From 39dce15e010cf1d136410605c41f68698f7a8bc7 Mon Sep 17 00:00:00 2001
From: Charles Duffy <charles@dyfis.net>
Date: Sun, 30 Mar 2025 12:16:25 -0500
Subject: [PATCH] General cleanup of shell bits:

- Avoid `sed` use for functionality built into shell parameter expansion
- Avoid piping into a `while read` loop, which introduces the surprising behaviors discussed in [BashFAQ 24](https://mywiki.wooledge.org/BashFAQ/024)
- Switch from external command `which` to built-in, POSIX-standardized `command -v`
- Avoid using pushd and popd (which are interactive-extension options, not guaranteed to be compiled into a noninteractive shell at all).

With this applied, `shellcheck scripts/*` runs clean.

Not yet addressed:

- Failure to sanitize or escape strings substituted into JSON (base image does not provide jq or this would be trivial)
- Use of [`set -e`](https://mywiki.wooledge.org/BashFAQ/105)
---
 bin/setup-cgroups         | 51 +++++++++++++++++++++------------------
 scripts/build-image       |  5 ++--
 scripts/push-image        | 10 +++++---
 scripts/setup-buildkit.sh | 27 +++++++++++++--------
 scripts/test              |  3 ++-
 5 files changed, 55 insertions(+), 41 deletions(-)

diff --git a/bin/setup-cgroups b/bin/setup-cgroups
index 8fc5b99..c6b50f7 100755
--- a/bin/setup-cgroups
+++ b/bin/setup-cgroups
@@ -10,37 +10,40 @@ fi
 mkdir -p /sys/fs/cgroup
 mount -t tmpfs -o uid=0,gid=0,mode=0755 cgroup /sys/fs/cgroup
 
-sed -e 1d /proc/cgroups | while read sys hierarchy num enabled; do
-  if [ "$enabled" != "1" ]; then
-    # subsystem disabled; skip
-    continue
-  fi
+{
+  read -r _ # skip first line
+  while read -r sys _hierarchy _num enabled; do
+    if [ "$enabled" != "1" ]; then
+      # subsystem disabled; skip
+      continue
+    fi
 
-  grouping="$(cat /proc/self/cgroup | cut -d: -f2 | grep "\\<$sys\\>")" || true
-  if [ -z "$grouping" ]; then
-    # subsystem not mounted anywhere; mount it on its own
-    grouping="$sys"
-  fi
+    grouping="$(</proc/self/cgroup cut -d: -f2 | grep "\\<$sys\\>")" || true
+    if [ -z "$grouping" ]; then
+      # subsystem not mounted anywhere; mount it on its own
+      grouping="$sys"
+    fi
 
-  mountpoint="/sys/fs/cgroup/$grouping"
+    mountpoint="/sys/fs/cgroup/$grouping"
 
-  mkdir -p "$mountpoint"
+    mkdir -p "$mountpoint"
 
-  # clear out existing mount to make sure new one is read-write
-  if mountpoint -q "$mountpoint"; then
-    umount "$mountpoint"
-  fi
+    # clear out existing mount to make sure new one is read-write
+    if mountpoint -q "$mountpoint"; then
+      umount "$mountpoint"
+    fi
 
-  mount -n -t cgroup -o "$grouping" cgroup "$mountpoint"
+    mount -n -t cgroup -o "$grouping" cgroup "$mountpoint"
 
-  if [ "$grouping" != "$sys" ]; then
-    if [ -L "/sys/fs/cgroup/$sys" ]; then
-      rm "/sys/fs/cgroup/$sys"
-    fi
+    if [ "$grouping" != "$sys" ]; then
+      if [ -L "/sys/fs/cgroup/$sys" ]; then
+        rm "/sys/fs/cgroup/$sys"
+      fi
 
-    ln -s "$mountpoint" "/sys/fs/cgroup/$sys"
-  fi
-done
+      ln -s "$mountpoint" "/sys/fs/cgroup/$sys"
+    fi
+  done
+} </proc/cgroups
 
 if ! test -e /sys/fs/cgroup/systemd ; then
   mkdir /sys/fs/cgroup/systemd
diff --git a/scripts/build-image b/scripts/build-image
index 91562a2..1993b0a 100755
--- a/scripts/build-image
+++ b/scripts/build-image
@@ -2,7 +2,8 @@
 
 set -e -u -x
 
-cd $(dirname $0)/..
+script=${BASH_SOURCE[0]}
+cd "${script%/*}"/.. || exit
 
 export PATH=$PWD/bin:$PATH
 
@@ -10,4 +11,4 @@ export PATH=$PWD/bin:$PATH
 
 mkdir -p image
 
-build
+exec build
diff --git a/scripts/push-image b/scripts/push-image
index 174c0d2..23b0a2a 100755
--- a/scripts/push-image
+++ b/scripts/push-image
@@ -7,19 +7,21 @@ version=""
 
 case $GITHUB_REF in
   refs/heads/*)
-    tag=$(echo $GITHUB_REF | sed 's|refs/heads/||')
+    tag=${GITHUB_REF#refs/heads/}
     ;;
 
   refs/tags/v[0-9]*)
-    version=$(echo $GITHUB_REF | sed 's|refs/tags/v||')
+    version=${GITHUB_REF#refs/tags/v}
     ;;
 
   refs/tags/*)
-    tag=$(echo $GITHUB_REF | sed 's|refs/tags/||')
+    tag=${GITHUB_REF#refs/tags/}
     ;;
 
   refs/pull/[0-9]*/merge)
-    tag=pr$(echo $GITHUB_REF | sed 's|refs/pull/\([0-9]\+\)/merge|\1|')
+    tag=${GITHUB_REF#refs/pull/}
+    tag=${tag%/merge}
+    tag=pr$tag
     ;;
 
   *)
diff --git a/scripts/setup-buildkit.sh b/scripts/setup-buildkit.sh
index 057d0ed..f372cfe 100644
--- a/scripts/setup-buildkit.sh
+++ b/scripts/setup-buildkit.sh
@@ -1,21 +1,28 @@
-if ! which buildctl >/dev/null || ! which buildkitd >/dev/null; then
+#!/usr/bin/env bash
+# this is sourced, not executed; the shebang above is a hint for shellcheck and/or editors
+
+uname_arch=$(uname -m)
+case $uname_arch in
+  x86_64) arch=amd64;;
+  aarch64) arch=arm64;;
+  *) arch=$uname_arch;;
+esac
+
+if ! command -v buildctl >/dev/null || ! command -v buildkitd >/dev/null; then
   BUILDKIT_VERSION=0.9.1
-  BUILDKIT_URL=https://github.com/moby/buildkit/releases/download/v$BUILDKIT_VERSION/buildkit-v$BUILDKIT_VERSION.linux-amd64.tar.gz
+  BUILDKIT_URL=https://github.com/moby/buildkit/releases/download/v$BUILDKIT_VERSION/buildkit-v$BUILDKIT_VERSION.linux-${arch}.tar.gz
 
   curl -fL "$BUILDKIT_URL" | tar zxf -
 fi
 
-if [ "$(id -u)" != "0" ]; then
-  if ! which newuidmap >/dev/null || ! which newgidmap >/dev/null; then
-    echo "newuidmap and newgidmap must be installed"
+if [ "$UID" != "0" ]; then
+  if ! command -v newuidmap >/dev/null || ! command -v newgidmap >/dev/null; then
+    echo "newuidmap and newgidmap must be installed" >&2
     exit 1
   fi
 
-  if ! which rootlesskit >/dev/null; then
-    pushd rootlesskit
-      make
-    popd
-
+  if ! command -v rootlesskit >/dev/null; then
+    (cd rootlesskit && exec make)
     cp rootlesskit/bin/* bin/
   fi
 fi
diff --git a/scripts/test b/scripts/test
index 3482294..63250a5 100755
--- a/scripts/test
+++ b/scripts/test
@@ -2,7 +2,8 @@
 
 set -e -u
 
-cd $(dirname $0)/..
+script=${BASH_SOURCE[0]}
+cd "${script%/*}/.." || exit
 
 export PATH=$PWD/bin:$PATH