Skip to content

Commit 27dd41d

Browse files
jservjserv
committed
Overhaul pre-commit hook and CI checks
This rewrites pre-commit hook from 7 basic checks to 11, adding compiler-based syntax verification, security pattern detection, and expanded banned function enforcement. All checks now operate on staged index content via a materialized temp tree. New checks: - Compiler -fsyntax-only pass (Linux only) with -Werror for format strings, implicit declarations, pointer type mismatches, and VLA - Security pattern scan on newly-added lines: non-literal format strings, missing O_CLOEXEC, positive errno returns, unchecked malloc multiplication, unbounded scanf, thread-unsafe functions - Expanded banned function list (13 functions) - Whitespace error detection via git diff --check - TODO/FIXME enforcement on new lines Change-Id: Ic25657aaee3df44b155e92ae69e695c1c139f546
1 parent 6b9076a commit 27dd41d

File tree

13 files changed

+705
-180
lines changed

13 files changed

+705
-180
lines changed

‎.ci/check-cppcheck.sh‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
#!/usr/bin/env bash
2+
3+
# Run cppcheck static analysis on kbox source files (src/ only).
4+
# Tests are excluded -- they use stubs and have different rules.
5+
6+
set -e -u -o pipefail
7+
8+
mapfile -t SOURCES < <(git ls-files -z -- 'src/*.c' | tr '\0' '\n')
9+
10+
if [ ${#SOURCES[@]} -eq 0 ]; then
11+
echo "No tracked C source files found."
12+
exit 0
13+
fi
14+
15+
NPROC=$(nproc 2>/dev/null || echo 1)
16+
17+
cppcheck -j"$NPROC" -I. -Iinclude -Isrc --enable=all --error-exitcode=1 \
18+
--max-configs=4 --inline-suppr \
19+
--suppress=checkersReport --suppress=unmatchedSuppression \
20+
--suppress=missingIncludeSystem --suppress=noValidConfiguration \
21+
--suppress=unusedFunction --suppress=syntaxError \
22+
--suppress=preprocessorErrorDirective \
23+
--suppress=normalCheckLevelMaxBranches \
24+
--suppress=constParameterPointer --suppress=constParameterCallback \
25+
--suppress=constVariablePointer --suppress=constParameter \
26+
--suppress=unusedStructMember --suppress=redundantAssignment \
27+
--suppress=staticFunction --suppress=checkLevelNormal \
28+
--suppress=variableScope --suppress=compareValueOutOfTypeRangeError \
29+
--suppress=constVariable --suppress=knownConditionTrueFalse \
30+
--suppress=unreadVariable --suppress=redundantInitialization \
31+
--suppress=shadowVariable \
32+
-D_GNU_SOURCE -DKBOX_UNIT_TEST \
33+
"${SOURCES[@]}"

‎.ci/check-format.sh‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
#!/usr/bin/env bash
2+
3+
# Verify clang-format-20 conformance for all tracked C/H files.
4+
5+
# Require clang-format-20 -- older versions produce different output.
6+
if [ -z "${CLANG_FORMAT:-}" ]; then
7+
if command -v clang-format-20 >/dev/null 2>&1; then
8+
CLANG_FORMAT="clang-format-20"
9+
else
10+
echo "Error: clang-format-20 is required (older versions differ in style)" >&2
11+
exit 1
12+
fi
13+
fi
14+
15+
ret=0
16+
while IFS= read -r -d '' file; do
17+
expected=$(mktemp)
18+
"$CLANG_FORMAT" "$file" >"$expected" 2>/dev/null
19+
if ! diff -u -p --label="$file" --label="expected coding style" "$file" "$expected"; then
20+
ret=1
21+
fi
22+
rm -f "$expected"
23+
done < <(git ls-files -z -- '*.c' '*.h')
24+
25+
exit $ret

‎.ci/check-newline.sh‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
#!/usr/bin/env bash
2+
3+
# Ensure all tracked C/H files end with a newline.
4+
5+
set -e -u -o pipefail
6+
7+
ret=0
8+
while IFS= read -rd '' f; do
9+
if file --mime-encoding "$f" | grep -qv binary; then
10+
if [ -n "$(tail -c1 < "$f")" ]; then
11+
echo "Warning: No newline at end of file $f"
12+
ret=1
13+
fi
14+
fi
15+
done < <(git ls-files -z -- '*.c' '*.h')
16+
17+
exit $ret

‎.ci/check-security.sh‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
#!/usr/bin/env bash
2+
3+
# Security checks for kbox source files (src/ and include/ only).
4+
# Tests and guest/stress binaries are excluded -- they have different rules.
5+
#
6+
# 1. Banned functions -- unsafe libc calls with safer alternatives.
7+
# 2. Credential / secret patterns -- catch accidental key leaks.
8+
# 3. Dangerous preprocessor -- detect disabled security features.
9+
10+
set -u -o pipefail
11+
12+
failed=0
13+
14+
# --- Patterns ---
15+
banned='(^|[^[:alnum:]_])(gets|sprintf|vsprintf|strcpy|stpcpy|strcat|atoi|atol|atoll|atof|mktemp|tmpnam|tempnam)[[:space:]]*\('
16+
secrets='(password|secret|api_key|private_key|token)[[:space:]]*=[[:space:]]*"[^"]+'
17+
dangerous_pp='#[[:space:]]*(undef|define)[[:space:]]+((_FORTIFY_SOURCE[[:space:]]+0)|(__SSP__))'
18+
comment_only='^[[:space:]]*(//|/\*|\*|\*/)'
19+
20+
# Only scan kbox source, not tests/guest/stress.
21+
while IFS= read -r -d '' f; do
22+
# Skip comment-only matches before checking.
23+
code=$(grep -vE "$comment_only" "$f")
24+
25+
if echo "$code" | grep -qE "$banned"; then
26+
echo "Banned function in $f:"
27+
grep -nE "$banned" "$f" | grep -vE "$comment_only"
28+
failed=1
29+
fi
30+
if echo "$code" | grep -iqE "$secrets"; then
31+
echo "Possible hardcoded secret in $f:"
32+
grep -inE "$secrets" "$f" | grep -vE "$comment_only"
33+
failed=1
34+
fi
35+
if echo "$code" | grep -qE "$dangerous_pp"; then
36+
echo "Dangerous preprocessor directive in $f:"
37+
grep -nE "$dangerous_pp" "$f" | grep -vE "$comment_only"
38+
failed=1
39+
fi
40+
done < <(git ls-files -z -- 'src/*.c' 'src/*.h' 'include/*.h' 'include/**/*.h')
41+
42+
if [ $failed -eq 0 ]; then
43+
echo "Security checks passed."
44+
fi
45+
46+
exit $failed

‎.editorconfig‎

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,40 @@
1-
# Top-level EditorConfig file
21
root = true
32

4-
# Shell script-specific settings
3+
[*]
4+
end_of_line = lf
5+
insert_final_newline = true
6+
trim_trailing_whitespace = true
7+
8+
[Makefile]
9+
indent_style = tab
10+
indent_size = 4
11+
12+
[mk/*]
13+
indent_style = tab
14+
indent_size = 4
15+
16+
[*.[ch]]
17+
indent_style = space
18+
indent_size = 4
19+
max_line_length = 80
20+
21+
[*.py]
22+
indent_style = space
23+
indent_size = 4
24+
525
[*.sh]
626
indent_style = space
727
indent_size = 4
8-
end_of_line = lf
9-
trim_trailing_whitespace = true
10-
insert_final_newline = true
11-
function_next_line = true
12-
switch_case_indent = true
13-
space_redirects = true
14-
binary_next_line = true
28+
29+
[*.hook]
30+
indent_style = space
31+
indent_size = 4
32+
33+
[*.yml]
34+
indent_style = space
35+
indent_size = 2
36+
# test
37+
# test
38+
# test
39+
# test
40+
# x

‎.github/workflows/build-kbox.yml‎

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
# Build kbox and run full test suite.
22
# Zero root required -- everything runs as an unprivileged user.
33
#
4-
# Parallelism:
5-
# unit-tests -- no LKL, no rootfs, runs immediately
6-
# build-kbox -- fetches LKL, compiles kbox + guest/stress bins, builds rootfs
7-
# integration -- needs build-kbox artifacts, runs integration + stress tests
4+
# Parallelism (5 independent jobs, 1 sequential):
5+
# coding-style -- clang-format + newline checks (fast)
6+
# static-analysis -- security patterns + cppcheck
7+
# unit-tests -- no LKL dependency, ASAN/UBSAN
8+
# build-kbox -- fetches LKL, compiles kbox + guest/stress bins, builds rootfs
9+
# integration -- needs build-kbox artifacts, runs integration + stress tests
810
#
9-
# unit-tests and build-kbox run in parallel. integration waits for build-kbox.
11+
# coding-style, static-analysis, unit-tests, and build-kbox run in parallel.
12+
# integration-tests waits for build-kbox only.
1013
name: Build and Test
1114

1215
on:
@@ -16,6 +19,42 @@ on:
1619
branches: [main]
1720

1821
jobs:
22+
# ---- Coding style: formatting checks (fast, ~10s) ----
23+
coding-style:
24+
runs-on: ubuntu-24.04
25+
steps:
26+
- name: Checkout
27+
uses: actions/checkout@v6
28+
29+
- name: Install clang-format
30+
run: |
31+
sudo apt-get update
32+
sudo apt-get install -y clang-format-20
33+
34+
- name: Check trailing newline
35+
run: .ci/check-newline.sh
36+
37+
- name: Check clang-format
38+
run: .ci/check-format.sh
39+
40+
# ---- Static analysis: security patterns + cppcheck ----
41+
static-analysis:
42+
runs-on: ubuntu-24.04
43+
steps:
44+
- name: Checkout
45+
uses: actions/checkout@v6
46+
47+
- name: Install cppcheck
48+
run: |
49+
sudo apt-get update
50+
sudo apt-get install -y cppcheck
51+
52+
- name: Security checks
53+
run: .ci/check-security.sh
54+
55+
- name: Static analysis (cppcheck)
56+
run: .ci/check-cppcheck.sh
57+
1958
# ---- Unit tests: no LKL dependency, fast ----
2059
unit-tests:
2160
runs-on: ubuntu-24.04

‎Makefile‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ KCONFIG_CONF := configs/Kconfig
1515

1616
# Targets that don't require .config
1717
CONFIG_TARGETS := config defconfig oldconfig savedefconfig clean distclean indent \
18-
check-unit fetch-lkl build-lkl fetch-minislirp install-hooks \
19-
guest-bins stress-bins rootfs
18+
check-unit check-syntax fetch-lkl build-lkl fetch-minislirp \
19+
install-hooks guest-bins stress-bins rootfs
2020
CONFIG_GENERATORS := config defconfig oldconfig
2121

2222
# Require .config for build targets.

‎mk/tests.mk‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,4 +111,45 @@ $(ROOTFS): scripts/mkrootfs.sh scripts/alpine-sha256.txt $(GUEST_BINS) $(STRESS_
111111
@echo " GEN $@"
112112
$(Q)ALPINE_ARCH=$(ARCH) ./scripts/mkrootfs.sh
113113

114-
.PHONY: check check-unit check-integration check-stress guest-bins stress-bins rootfs
114+
# ---- Syntax-only compilation check (used by pre-commit hook) ----
115+
# Usage: make check-syntax CHK_SOURCES="src/foo.c src/bar.c"
116+
# Uses the project's real CFLAGS with -fsyntax-only (no linking, no .o output).
117+
# Skips gracefully on non-Linux compilers.
118+
119+
CHK_CC_TARGET := $(shell $(CC) -dumpmachine 2>/dev/null)
120+
check-syntax:
121+
ifeq ($(findstring linux,$(CHK_CC_TARGET)),)
122+
@echo " SKIP check-syntax (non-Linux compiler: $(CHK_CC_TARGET))"
123+
else
124+
ifdef CHK_SOURCES
125+
@echo " SYNTAX $(words $(CHK_SOURCES)) file(s)"
126+
$(Q)$(CC) $(CFLAGS) -DKBOX_UNIT_TEST -fsyntax-only \
127+
-Werror=implicit-function-declaration \
128+
-Werror=incompatible-pointer-types \
129+
-Werror=int-conversion \
130+
-Werror=return-type \
131+
-Werror=format=2 \
132+
-Werror=format-security \
133+
-Werror=strict-prototypes \
134+
-Werror=old-style-definition \
135+
-Werror=sizeof-pointer-memaccess \
136+
-Werror=vla \
137+
$(CHK_SOURCES)
138+
else
139+
@echo " SYNTAX all source files"
140+
$(Q)$(CC) $(CFLAGS) -DKBOX_UNIT_TEST -fsyntax-only \
141+
-Werror=implicit-function-declaration \
142+
-Werror=incompatible-pointer-types \
143+
-Werror=int-conversion \
144+
-Werror=return-type \
145+
-Werror=format=2 \
146+
-Werror=format-security \
147+
-Werror=strict-prototypes \
148+
-Werror=old-style-definition \
149+
-Werror=sizeof-pointer-memaccess \
150+
-Werror=vla \
151+
$(SRCS)
152+
endif
153+
endif
154+
155+
.PHONY: check check-unit check-integration check-stress guest-bins stress-bins rootfs check-syntax

‎scripts/build-lkl.sh‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
set -eu
1313

1414
case "${1:-$(uname -m)}" in
15-
x86_64 | amd64) ARCH="x86_64" ;;
15+
x86_64 | amd64) ARCH="x86_64" ;;
1616
aarch64 | arm64) ARCH="aarch64" ;;
1717
*)
1818
echo "error: unsupported architecture: ${1:-$(uname -m)}" >&2
@@ -24,7 +24,11 @@ LKL_DIR="${LKL_DIR:-lkl-${ARCH}}"
2424
LKL_SRC="${LKL_SRC:-build/lkl-src}"
2525
LKL_UPSTREAM="https://github.com/lkl/linux"
2626

27-
die() { echo "error: $*" >&2; exit 1; }
27+
die()
28+
{
29+
echo "error: $*" >&2
30+
exit 1
31+
}
2832

2933
# ---- Clone or update source tree ----------------------------------------
3034

@@ -66,8 +70,7 @@ for opt in \
6670
CONFIG_PRINTK \
6771
CONFIG_TRACEPOINTS \
6872
CONFIG_FTRACE \
69-
CONFIG_DEBUG_FS \
70-
; do
73+
CONFIG_DEBUG_FS; do
7174
"${LKL_SRC}/scripts/config" --file "${LKL_SRC}/.config" --enable "${opt}"
7275
done
7376

@@ -80,16 +83,15 @@ for opt in \
8083
CONFIG_USB_SUPPORT \
8184
CONFIG_INPUT \
8285
CONFIG_NFS_FS \
83-
CONFIG_CIFS \
84-
; do
86+
CONFIG_CIFS; do
8587
"${LKL_SRC}/scripts/config" --file "${LKL_SRC}/.config" --disable "${opt}"
8688
done
8789

8890
make -C "${LKL_SRC}" ARCH=lkl olddefconfig
8991

9092
# ---- Build ---------------------------------------------------------------
9193

92-
NPROC=$(nproc 2>/dev/null || echo 1)
94+
NPROC=$(nproc 2> /dev/null || echo 1)
9395

9496
echo " BUILD ARCH=lkl kernel (-j${NPROC})"
9597
make -C "${LKL_SRC}" ARCH=lkl -j"${NPROC}"
@@ -104,10 +106,10 @@ test -f "${LKL_SRC}/tools/lkl/liblkl.a" \
104106

105107
echo " VERIFY symbols"
106108
for sym in lkl_init lkl_start_kernel lkl_cleanup lkl_syscall \
107-
lkl_strerror lkl_disk_add lkl_mount_dev \
108-
lkl_host_ops lkl_dev_blk_ops; do
109-
if ! nm "${LKL_SRC}/tools/lkl/liblkl.a" 2>/dev/null \
110-
| awk -v s="$sym" '$3==s && $2~/^[TtDdBbRr]$/{found=1} END{exit !found}'; then
109+
lkl_strerror lkl_disk_add lkl_mount_dev \
110+
lkl_host_ops lkl_dev_blk_ops; do
111+
if ! nm "${LKL_SRC}/tools/lkl/liblkl.a" 2> /dev/null \
112+
| awk -v s="$sym" '$3==s && $2~/^[TtDdBbRr]$/{found=1} END{exit !found}'; then
111113
die "MISSING symbol: ${sym}"
112114
fi
113115
done
@@ -117,17 +119,17 @@ done
117119
echo " INSTALL ${LKL_DIR}/"
118120
mkdir -p "${LKL_DIR}"
119121

120-
cp "${LKL_SRC}/tools/lkl/liblkl.a" "${LKL_DIR}/"
121-
cp "${LKL_SRC}/tools/lkl/include/lkl.h" "${LKL_DIR}/" 2>/dev/null || true
122-
cp "${LKL_SRC}/tools/lkl/include/lkl/autoconf.h" "${LKL_DIR}/" 2>/dev/null || true
123-
cp "${LKL_SRC}/scripts/gdb/vmlinux-gdb.py" "${LKL_DIR}/" 2>/dev/null || true
122+
cp "${LKL_SRC}/tools/lkl/liblkl.a" "${LKL_DIR}/"
123+
cp "${LKL_SRC}/tools/lkl/include/lkl.h" "${LKL_DIR}/" 2> /dev/null || true
124+
cp "${LKL_SRC}/tools/lkl/include/lkl/autoconf.h" "${LKL_DIR}/" 2> /dev/null || true
125+
cp "${LKL_SRC}/scripts/gdb/vmlinux-gdb.py" "${LKL_DIR}/" 2> /dev/null || true
124126

125127
printf 'commit=%s\ndate=%s\narch=%s\n' \
126128
"$(git -C "${LKL_SRC}" rev-parse HEAD)" \
127129
"$(date -u +%Y-%m-%dT%H:%M:%SZ)" \
128130
"${ARCH}" \
129131
> "${LKL_DIR}/BUILD_INFO"
130132

131-
( cd "${LKL_DIR}" && sha256sum ./* > sha256sums.txt )
133+
(cd "${LKL_DIR}" && sha256sum ./* > sha256sums.txt)
132134

133135
echo "OK: ${LKL_DIR}/liblkl.a"

0 commit comments

Comments
 (0)