ci: add shfmt and shellcheck for shell scripts

.editorconfig

@@ -0,0 +1,20 @@
+# See https://EditorConfig.org
+
+# This is a top-most EditorConfig file.
+root = true
+
+# Ignore any "vendor" directories.
+[**/vendor/**]
+ignore = true
+
+# Bash, bats, and sh files use 4 spaces for indentation.
+[*.{bash,bats,sh}]
+indent_style = space
+indent_size = 4
+
+# This is for shell scripts with shebang but no extensions.
+# Not yet supported by editorconfig[1], only by shfmt.
+# https://github.com/editorconfig/editorconfig/issues/404
+[[shell]]
+indent_style = space
+indent_size = 4

.github/workflows/validate.yml

@@ -58,6 +58,26 @@ jobs:
         sudo apt-get install -y cppcheck libglib2.0-dev pkg-config
         make cppcheck
 
+  shfmt:
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+    steps:
+    - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+      with:
+        persist-credentials: false
+    - name: Check shell script formatting
+      run: make shfmt
+
+  shellcheck:
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+    steps:
+    - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+      with:
+        persist-credentials: false
+    - name: Lint shell scripts
+      run: make shellcheck
+
   meson-build:
     runs-on: ubuntu-latest
     timeout-minutes: 10
@@ -78,6 +98,8 @@ jobs:
       - cppcheck
       - lint
       - meson-build
+      - shellcheck
+      - shfmt
     runs-on: ubuntu-latest
     steps:
     - run: echo "All jobs completed"

.rpmbuild/prepare.sh

@@ -16,7 +16,7 @@ sed "s,#COMMIT#,${COMMIT},;
      s,#SHORTCOMMIT#,${COMMIT_SHORT},;
      s,#COMMITNUM#,${COMMIT_NUM},;
      s,#COMMITDATE#,${COMMIT_DATE}," \
-         contrib/spec/conmon.spec.in > contrib/spec/conmon.spec
+    contrib/spec/conmon.spec.in >contrib/spec/conmon.spec
 
 mkdir build/
 git archive --prefix "conmon-${COMMIT_SHORT}/" --format "tar.gz" HEAD -o "build/conmon-${COMMIT_SHORT}.tar.gz"

Makefile

@@ -182,6 +182,40 @@ cppcheck:
 		--suppress=missingIncludeSystem --suppress=checkersReport \
 		$(CPPCHECK_FLAGS) src/
 
+# Formatting for shell scripts. Uses a pinned image so that everyone, CI
+# included, gets identical output; run localshfmt to use a locally installed shfmt.
+# The list of files to format is shfmt's own, and covers .bats files as well
+# as shell scripts with a shebang and no extension.
+CONTAINER_ENGINE ?= podman
+SHFMT_IMAGE ?= docker.io/mvdan/shfmt:v3.13.1
+
+.PHONY: shfmt
+shfmt:
+	$(CONTAINER_ENGINE) run $(CONTAINER_ENGINE_RUN_FLAGS) \
+		--rm -v $(CURDIR):/src:z -w /src \
+		$(SHFMT_IMAGE) -d -w .
+
+.PHONY: localshfmt
+localshfmt:
+	shfmt -d -w .
+
+# Shell script linting, run the same way as shfmt above. .bats files have no
+# shebang, hence -s bash.
+SHELLCHECK_IMAGE ?= docker.io/koalaman/shellcheck:v0.11.0
+# The last entry is a shell script with a shebang and no extension, which
+# git ls-files cannot match by pattern.
+SHELL_SRC := $(shell git ls-files '*.bash' '*.bats' '*.sh') hack/github-actions-setup
+
+.PHONY: shellcheck
+shellcheck:
+	$(CONTAINER_ENGINE) run $(CONTAINER_ENGINE_RUN_FLAGS) \
+		--rm -v $(CURDIR):/src:z -w /src \
+		$(SHELLCHECK_IMAGE) -s bash $(SHELL_SRC)
+
+.PHONY: localshellcheck
+localshellcheck:
+	shellcheck -s bash $(SHELL_SRC)
+
 .PHONY: fmt
 fmt:
 	git ls-files -z \*.c \*.h | xargs -0 clang-format -i

hack/github-actions-setup

@@ -124,6 +124,7 @@ apt_get() {
 }
 
 install_packages() {
+    # shellcheck source=/dev/null
     . /etc/os-release
     CRIU_REPO="https://download.opensuse.org/repositories/devel:/tools:/criu/xUbuntu_$VERSION_ID"
 

test/01-basic.bats

@@ -171,4 +171,4 @@ teardown() {
         --log-path "k8s-file:$LOG_PATH" --log-path "$invalid_log_driver:$LOG_PATH"
     assert_failure
     assert_output_contains "No such log driver $invalid_log_driver"
-}
+}

test/02-ctr-logs.bats

@@ -153,7 +153,7 @@ run_conmon_with_log_opts() {
 @test "ctr logs: journald partial message" {
     # Print a message longer than the conmon buffer.
     # It should split it into multiple partial messages.
-    setup_container_env "printf '%*s' "65535" | tr ' ' '#'"
+    setup_container_env "printf '%*s' 65535 '' | tr ' ' '#'"
     run_conmon_with_default_args \
         --log-path "journald:"
 
@@ -164,7 +164,7 @@ run_conmon_with_log_opts() {
 @test "ctr logs: k8s partial message" {
     # Print a message longer than the conmon buffer.
     # It should split it into multiple partial messages.
-    setup_container_env "printf '%*s' "65535" | tr ' ' '#'"
+    setup_container_env "printf '%*s' 65535 '' | tr ' ' '#'"
     run_conmon_with_default_args \
         --log-path "k8s-file:$LOG_PATH"
 

test/03-k8s-log-rotation.bats

@@ -59,10 +59,10 @@ run_conmon_k8s_file() {
 
     # Test k8s log format handling
     local k8s_content='2023-07-23T18:00:00.000000000Z stdout F Test log message'
-    echo "$k8s_content" > "$LOG_PATH"
+    echo "$k8s_content" >"$LOG_PATH"
 
     # Verify content preservation
     local content
     content=$(<"$LOG_PATH")
     [ "$content" = "$k8s_content" ]
-}
+}

test/04-runtime.bats

@@ -18,15 +18,15 @@ teardown() {
     # Check that log file was created
     [ -f "$LOG_PATH" ]
     run cat "$LOG_PATH"
-    assert "${output}" =~ "hello from ubi10"  "'hello from ubi10' found in the log"
+    assert "${output}" =~ "hello from ubi10" "'hello from ubi10' found in the log"
 }
 
 @test "runtime: container execution with different log drivers" {
     # Test with journald log driver
     run_conmon_with_default_args --log-path "journald:"
 
     run journalctl --user CONTAINER_ID_FULL="$CTR_ID"
-    assert "${output}" =~ "hello from ubi10"  "'hello from ubi10' found in the journald"
+    assert "${output}" =~ "hello from ubi10" "'hello from ubi10' found in the journald"
 }
 
 @test "runtime: container execution with multiple log drivers" {
@@ -37,10 +37,10 @@ teardown() {
     [ -f "$LOG_PATH" ]
 
     run cat "$LOG_PATH"
-    assert "${output}" =~ "hello from ubi10"  "'hello from ubi10' found in the log"
+    assert "${output}" =~ "hello from ubi10" "'hello from ubi10' found in the log"
 
     run journalctl --user CONTAINER_ID_FULL="$CTR_ID"
-    assert "${output}" =~ "hello from ubi10"  "'hello from ubi10' found in the journald"
+    assert "${output}" =~ "hello from ubi10" "'hello from ubi10' found in the journald"
 }
 
 @test "runtime: container with log size limit" {
@@ -54,7 +54,7 @@ teardown() {
     [ -f "$LOG_PATH" ]
 
     run cat "$LOG_PATH"
-    assert "${output}" !~ "hello from ubi10 11"  "'hello from ubi10 11' not in the logs"
+    assert "${output}" !~ "hello from ubi10 11" "'hello from ubi10 11' not in the logs"
 }
 
 @test "runtime: invalid runtime binary should fail" {
@@ -87,7 +87,7 @@ teardown() {
 
     # Check that the pid is sent to the sync pipe.
     wait_for_syncpipe_output 1
-    run cat $TEST_TMPDIR/syncpipe-output
+    run cat "$TEST_TMPDIR/syncpipe-output"
     CONTAINER_PID=$(cat "$CONTAINER_PIDFILE")
     assert_json "${output}" =~ "\"pid\": $CONTAINER_PID"
 }
@@ -114,13 +114,13 @@ teardown() {
     # Give conmon some time to run the runtime and fail.
     sleep 1
 
-    assert_file_exists $CONMON_PID_FILE
+    assert_file_exists "$CONMON_PID_FILE"
     CONMON_PID=$(cat "$CONMON_PID_FILE")
     wait_for_conmon_exit "$CONMON_PID"
 
     # Check that the error is sent to the sync pipe.
     wait_for_syncpipe_output 1
-    run cat $TEST_TMPDIR/syncpipe-output
+    run cat "$TEST_TMPDIR/syncpipe-output"
     assert_json "${output}" =~ "\"pid\": -1"
     assert_json "${output}" =~ "\"message\":"
     assert_json "${output}" =~ "runc create failed"

test/05-oom-detection.bats

@@ -26,7 +26,7 @@ create_mock_memory_events() {
     local oom_kill_count="$2"
     local events_file="$MOCK_CGROUP_PATH/memory.events"
 
-    cat > "$events_file" << EOF
+    cat >"$events_file" <<EOF
 low 0
 high 0
 max 0
@@ -106,7 +106,7 @@ EOF
     # Test that malformed counter values are handled gracefully
     local events_file="$MOCK_CGROUP_PATH/memory.events"
 
-    cat > "$events_file" << EOF
+    cat >"$events_file" <<EOF
 low 0
 high 0
 max 0
@@ -195,4 +195,4 @@ EOF
         # We're not on cgroup v2, tests should be skipped
         skip "Not on cgroup v2 system"
     fi
-}
+}

test/06-exec-exit-status.bats

@@ -34,7 +34,6 @@ teardown() {
     assert_failure
 }
 
-
 # Integration test that can be run manually or in CI
 @test "integration: exec exit codes work correctly" {
     # This test can only run if podman is available and configured
@@ -43,7 +42,8 @@ teardown() {
     fi
 
     # Use the conmon binary from the build (using absolute path)
-    local conmon_path="$(dirname "$CONMON_BINARY")/conmon"
+    local conmon_path
+    conmon_path="$(dirname "$CONMON_BINARY")/conmon"
 
     if [ ! -f "$conmon_path" ]; then
         die "conmon binary not found for integration testing at $conmon_path"
@@ -55,7 +55,7 @@ teardown() {
     # of them has any business taking long, and an unbounded one hangs the
     # whole suite -- bats runs tests serially, and a command substitution
     # waits for stdout to be closed, which a misbehaving conmon may never do.
-    run timeout 10 podman --conmon $conmon_path run --rm "$UBI10_MICRO_IMAGE" true
+    run timeout 10 podman --conmon "$conmon_path" run --rm "$UBI10_MICRO_IMAGE" true
     if [ "$status" -ne 0 ]; then
         die "cannot create test containers with podman: $output"
     fi
@@ -64,37 +64,37 @@ teardown() {
 
     # Create a test container
     local container_id
-    container_id=$(timeout 60 podman --conmon $conmon_path run -dt "$UBI10_MICRO_IMAGE" sleep 30)
+    container_id=$(timeout 60 podman --conmon "$conmon_path" run -dt "$UBI10_MICRO_IMAGE" sleep 30)
 
     if [ -z "$container_id" ]; then
         die "failed to create test container"
     fi
 
     # Test 1: Success case
-    if ! timeout 60 podman --conmon $conmon_path exec "$container_id" true; then
-        timeout 60 podman --conmon $conmon_path rm -f "$container_id" >/dev/null 2>&1
+    if ! timeout 60 podman --conmon "$conmon_path" exec "$container_id" true; then
+        timeout 60 podman --conmon "$conmon_path" rm -f "$container_id" >/dev/null 2>&1
         echo "FAIL: true command should succeed"
         return 1
     fi
 
     # Test 2: Failure case - this would fail with the regression
-    if timeout 60 podman --conmon $conmon_path exec "$container_id" false; then
-        timeout 60 podman --conmon $conmon_path rm -f "$container_id" >/dev/null 2>&1
+    if timeout 60 podman --conmon "$conmon_path" exec "$container_id" false; then
+        timeout 60 podman --conmon "$conmon_path" rm -f "$container_id" >/dev/null 2>&1
         echo "FAIL: false command should fail (regression detected!)"
         echo "This indicates the fc0a342 regression where all exec commands return 0"
         return 1
     fi
 
     # Test 3: Custom exit code - this would return 0 with the regression
-    if timeout 60 podman --conmon $conmon_path exec "$container_id" sh -c 'exit 42'; then
-        timeout 60 podman --conmon $conmon_path rm -f "$container_id" >/dev/null 2>&1
+    if timeout 60 podman --conmon "$conmon_path" exec "$container_id" sh -c 'exit 42'; then
+        timeout 60 podman --conmon "$conmon_path" rm -f "$container_id" >/dev/null 2>&1
         echo "FAIL: 'exit 42' should fail with code 42 (regression detected!)"
         echo "This indicates the fc0a342 regression where all exec commands return 0"
         return 1
     fi
 
     # Clean up
-    timeout 60 podman --conmon $conmon_path rm -f "$container_id" >/dev/null 2>&1
+    timeout 60 podman --conmon "$conmon_path" rm -f "$container_id" >/dev/null 2>&1
 
     echo "Integration test passed: exec exit codes work correctly"
-}
+}