Merge pull request #31 from sysprog21/pre-commit

Overhaul pre-commit hook and CI checks

Author: jserv

.ci/check-cppcheck.sh

@@ -0,0 +1,32 @@
+#!/usr/bin/env bash
+
+# Run cppcheck static analysis on kbox source files (src/ only).
+# Tests are excluded -- they use stubs and have different rules.
+#
+# CI mode: --max-configs=1 + --enable=warning for speed.
+# Deep analysis runs in the pre-commit hook on individual changed files.
+
+set -e -u -o pipefail
+
+# Exclude syscall-nr.c: it's a data table that causes cppcheck to hang
+# on older versions (>100s). The pre-commit hook checks it per-file.
+mapfile -t SOURCES < <(git ls-files -z -- 'src/*.c' | tr '\0' '\n' | grep -v 'syscall-nr\.c')
+
+if [ ${#SOURCES[@]} -eq 0 ]; then
+    echo "No tracked C source files found."
+    exit 0
+fi
+
+# Run cppcheck with a timeout to prevent CI hangs.
+# 120s is generous -- this should finish in <30s with --max-configs=1.
+timeout 120 cppcheck \
+    -I. -Iinclude -Isrc \
+    --platform=unix64 \
+    --enable=warning \
+    --max-configs=1 --error-exitcode=1 --inline-suppr \
+    --suppress=checkersReport --suppress=unmatchedSuppression \
+    --suppress=missingIncludeSystem --suppress=noValidConfiguration \
+    --suppress=normalCheckLevelMaxBranches \
+    --suppress=preprocessorErrorDirective \
+    -D_GNU_SOURCE -D__linux__ \
+    "${SOURCES[@]}"

.ci/check-format.sh

@@ -0,0 +1,25 @@
+#!/usr/bin/env bash
+
+# Verify clang-format-20 conformance for all tracked C/H files.
+
+# Require clang-format-20 -- older versions produce different output.
+if [ -z "${CLANG_FORMAT:-}" ]; then
+    if command -v clang-format-20 >/dev/null 2>&1; then
+        CLANG_FORMAT="clang-format-20"
+    else
+        echo "Error: clang-format-20 is required (older versions differ in style)" >&2
+        exit 1
+    fi
+fi
+
+ret=0
+while IFS= read -r -d '' file; do
+    expected=$(mktemp)
+    "$CLANG_FORMAT" "$file" >"$expected" 2>/dev/null
+    if ! diff -u -p --label="$file" --label="expected coding style" "$file" "$expected"; then
+        ret=1
+    fi
+    rm -f "$expected"
+done < <(git ls-files -z -- '*.c' '*.h')
+
+exit $ret

.ci/check-newline.sh

@@ -0,0 +1,17 @@
+#!/usr/bin/env bash
+
+# Ensure all tracked C/H files end with a newline.
+
+set -e -u -o pipefail
+
+ret=0
+while IFS= read -rd '' f; do
+    if file --mime-encoding "$f" | grep -qv binary; then
+        if [ -n "$(tail -c1 < "$f")" ]; then
+            echo "Warning: No newline at end of file $f"
+            ret=1
+        fi
+    fi
+done < <(git ls-files -z -- '*.c' '*.h')
+
+exit $ret

.ci/check-security.sh

@@ -0,0 +1,46 @@
+#!/usr/bin/env bash
+
+# Security checks for kbox source files (src/ and include/ only).
+# Tests and guest/stress binaries are excluded -- they have different rules.
+#
+# 1. Banned functions -- unsafe libc calls with safer alternatives.
+# 2. Credential / secret patterns -- catch accidental key leaks.
+# 3. Dangerous preprocessor -- detect disabled security features.
+
+set -u -o pipefail
+
+failed=0
+
+# --- Patterns ---
+banned='(^|[^[:alnum:]_])(gets|sprintf|vsprintf|strcpy|stpcpy|strcat|atoi|atol|atoll|atof|mktemp|tmpnam|tempnam)[[:space:]]*\('
+secrets='(password|secret|api_key|private_key|token)[[:space:]]*=[[:space:]]*"[^"]+'
+dangerous_pp='#[[:space:]]*(undef|define)[[:space:]]+((_FORTIFY_SOURCE[[:space:]]+0)|(__SSP__))'
+comment_only='^[[:space:]]*(//|/\*|\*|\*/)'
+
+# Only scan kbox source, not tests/guest/stress.
+while IFS= read -r -d '' f; do
+    # Skip comment-only matches before checking.
+    code=$(grep -vE "$comment_only" "$f")
+
+    if echo "$code" | grep -qE "$banned"; then
+        echo "Banned function in $f:"
+        grep -nE "$banned" "$f" | grep -vE "$comment_only"
+        failed=1
+    fi
+    if echo "$code" | grep -iqE "$secrets"; then
+        echo "Possible hardcoded secret in $f:"
+        grep -inE "$secrets" "$f" | grep -vE "$comment_only"
+        failed=1
+    fi
+    if echo "$code" | grep -qE "$dangerous_pp"; then
+        echo "Dangerous preprocessor directive in $f:"
+        grep -nE "$dangerous_pp" "$f" | grep -vE "$comment_only"
+        failed=1
+    fi
+done < <(git ls-files -z -- 'src/*.c' 'src/*.h' 'include/*.h' 'include/**/*.h')
+
+if [ $failed -eq 0 ]; then
+    echo "Security checks passed."
+fi
+
+exit $failed

.editorconfig

@@ -1,14 +1,40 @@
-# Top-level EditorConfig file
 root = true
 
-# Shell script-specific settings
+[*]
+end_of_line = lf
+insert_final_newline = true
+trim_trailing_whitespace = true
+
+[Makefile]
+indent_style = tab
+indent_size = 4
+
+[mk/*]
+indent_style = tab
+indent_size = 4
+
+[*.[ch]]
+indent_style = space
+indent_size = 4
+max_line_length = 80
+
+[*.py]
+indent_style = space
+indent_size = 4
+
 [*.sh]
 indent_style = space
 indent_size = 4
-end_of_line = lf
-trim_trailing_whitespace = true
-insert_final_newline = true
-function_next_line = true
-switch_case_indent = true
-space_redirects = true
-binary_next_line = true
+
+[*.hook]
+indent_style = space
+indent_size = 4
+
+[*.yml]
+indent_style = space
+indent_size = 2
+# test
+# test
+# test
+# test
+# x

.github/workflows/build-kbox.yml

@@ -1,12 +1,15 @@
 # Build kbox and run full test suite.
 # Zero root required -- everything runs as an unprivileged user.
 #
-# Parallelism:
-#   unit-tests   -- no LKL, no rootfs, runs immediately
-#   build-kbox   -- fetches LKL, compiles kbox + guest/stress bins, builds rootfs
-#   integration  -- needs build-kbox artifacts, runs integration + stress tests
+# Parallelism (5 independent jobs, 1 sequential):
+#   coding-style     -- clang-format + newline checks (fast)
+#   static-analysis  -- security patterns + cppcheck
+#   unit-tests       -- no LKL dependency, ASAN/UBSAN
+#   build-kbox       -- fetches LKL, compiles kbox + guest/stress bins, builds rootfs
+#   integration      -- needs build-kbox artifacts, runs integration + stress tests
 #
-# unit-tests and build-kbox run in parallel. integration waits for build-kbox.
+# coding-style, static-analysis, unit-tests, and build-kbox run in parallel.
+# integration-tests waits for build-kbox only.
 name: Build and Test
 
 on:
@@ -16,6 +19,42 @@ on:
     branches: [main]
 
 jobs:
+  # ---- Coding style: formatting checks (fast, ~10s) ----
+  coding-style:
+    runs-on: ubuntu-24.04
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v6
+
+      - name: Install clang-format
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y clang-format-20
+
+      - name: Check trailing newline
+        run: .ci/check-newline.sh
+
+      - name: Check clang-format
+        run: .ci/check-format.sh
+
+  # ---- Static analysis: security patterns + cppcheck ----
+  static-analysis:
+    runs-on: ubuntu-24.04
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v6
+
+      - name: Install cppcheck
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y cppcheck
+
+      - name: Security checks
+        run: .ci/check-security.sh
+
+      - name: Static analysis (cppcheck)
+        run: .ci/check-cppcheck.sh
+
   # ---- Unit tests: no LKL dependency, fast ----
   unit-tests:
     runs-on: ubuntu-24.04

Makefile

@@ -15,8 +15,8 @@ KCONFIG_CONF := configs/Kconfig
 
 # Targets that don't require .config
 CONFIG_TARGETS    := config defconfig oldconfig savedefconfig clean distclean indent \
-                     check-unit fetch-lkl build-lkl fetch-minislirp install-hooks \
-                     guest-bins stress-bins rootfs
+                     check-unit check-syntax fetch-lkl build-lkl fetch-minislirp \
+                     install-hooks guest-bins stress-bins rootfs
 CONFIG_GENERATORS := config defconfig oldconfig
 
 # Require .config for build targets.

mk/tests.mk

@@ -6,6 +6,7 @@ TEST_DIR   = tests/unit
 TEST_SRCS  = $(TEST_DIR)/test-runner.c \
              $(TEST_DIR)/test-fd-table.c \
              $(TEST_DIR)/test-path.c \
+             $(TEST_DIR)/test-cli.c \
              $(TEST_DIR)/test-identity.c \
              $(TEST_DIR)/test-syscall-nr.c \
              $(TEST_DIR)/test-elf.c \
@@ -29,6 +30,7 @@ endif
 # Unit tests link only the pure-computation sources (no LKL)
 TEST_SUPPORT_SRCS = $(SRC_DIR)/fd-table.c \
                     $(SRC_DIR)/path.c \
+                    $(SRC_DIR)/cli.c \
                     $(SRC_DIR)/identity.c \
                     $(SRC_DIR)/syscall-nr.c \
                     $(SRC_DIR)/elf.c \
@@ -111,4 +113,45 @@ $(ROOTFS): scripts/mkrootfs.sh scripts/alpine-sha256.txt $(GUEST_BINS) $(STRESS_
 	@echo "  GEN     $@"
 	$(Q)ALPINE_ARCH=$(ARCH) ./scripts/mkrootfs.sh
 
-.PHONY: check check-unit check-integration check-stress guest-bins stress-bins rootfs
+# ---- Syntax-only compilation check (used by pre-commit hook) ----
+# Usage: make check-syntax CHK_SOURCES="src/foo.c src/bar.c"
+# Uses the project's real CFLAGS with -fsyntax-only (no linking, no .o output).
+# Skips gracefully on non-Linux compilers.
+
+CHK_CC_TARGET := $(shell $(CC) -dumpmachine 2>/dev/null)
+check-syntax:
+ifeq ($(findstring linux,$(CHK_CC_TARGET)),)
+	@echo "  SKIP    check-syntax (non-Linux compiler: $(CHK_CC_TARGET))"
+else
+ifdef CHK_SOURCES
+	@echo "  SYNTAX  $(words $(CHK_SOURCES)) file(s)"
+	$(Q)$(CC) $(CFLAGS) -DKBOX_UNIT_TEST -fsyntax-only \
+	    -Werror=implicit-function-declaration \
+	    -Werror=incompatible-pointer-types \
+	    -Werror=int-conversion \
+	    -Werror=return-type \
+	    -Werror=format=2 \
+	    -Werror=format-security \
+	    -Werror=strict-prototypes \
+	    -Werror=old-style-definition \
+	    -Werror=sizeof-pointer-memaccess \
+	    -Werror=vla \
+	    $(CHK_SOURCES)
+else
+	@echo "  SYNTAX  all source files"
+	$(Q)$(CC) $(CFLAGS) -DKBOX_UNIT_TEST -fsyntax-only \
+	    -Werror=implicit-function-declaration \
+	    -Werror=incompatible-pointer-types \
+	    -Werror=int-conversion \
+	    -Werror=return-type \
+	    -Werror=format=2 \
+	    -Werror=format-security \
+	    -Werror=strict-prototypes \
+	    -Werror=old-style-definition \
+	    -Werror=sizeof-pointer-memaccess \
+	    -Werror=vla \
+	    $(SRCS)
+endif
+endif
+
+.PHONY: check check-unit check-integration check-stress guest-bins stress-bins rootfs check-syntax