Skip to content

Commit a00522e

Browse files
kalyazinkalyazin
committed
ci: move the scan's exit-code gate into a tested script
The gate decides whether a scan that could not run is allowed to publish a clean-looking result, and it was a shell body embedded in YAML — so the only way to assert its behaviour was to pin the body's text and re-execute it from the workflow's own shape test, under a stubbed docker. As a script it can be tested directly: scan.test.sh feeds it every exit code, checks that the ones meaning "did not scan" fail the job, checks the argv the scanner receives and the lockfile it is handed at the moment of the call, and checks that the results are not rewritten after it, on both codes that pass. Being a script is also what makes the container's reach assertable. It is given the lockfile read-only and an output directory of its own, under the runner's user rather than root, because the checkout's post step runs git in that workspace with the token in its environment, so an image able to write there is inside the reach of a step this file does not control. The argv is pinned, so widening the mount again is red, and the script lint's container runs as the same user, over the whole checkout read-only rather than the one file. What comes back out is checked too. The image decides what is left in its output directory, so the results are copied only when they are a file the scanner wrote rather than a link it aimed at one of the runner's. A body that leaves YAML also leaves actionlint's reach, so the runner lints the scripts with shellcheck, and the shape test requires each script it names to be linted and to have a sibling test that a step runs. The runner's bodies are an allowlist for the same reason its steps are: a step running anything else runs before them and can rewrite what they assert. The shape test reads scalars the way GitHub does rather than the way PyYAML would: a timeout written 0400 is 400 to GitHub and 256 to PyYAML, while hex and 0o octal are refused by the pinned actionlint in the same job, as are cron field ranges like */0. The stub records what the scanner receives at the moment it is called, so a body that swaps the lockfile around the call and puts it back is not invisible to a check made afterwards, and it writes more than the results into its output directory, so what crosses back into the workspace is asserted to be only them; it also leaves a link there, and nothing at all, because both are what an image can hand back in place of a scan. Every call is recorded rather than the last, and exactly one is expected, since a container started before the pinned one reaches the workspace while the argv still reads as pinned. The output directory has to be a different path on each run, because a fixed one can be waited at. After each exit code the test also looks for a process still holding that directory, because work backgrounded and not waited for reaches the results after the gate has passed on them; four decoys prove the four ways it looks. That is aimed at a later edit leaving something running, not at an author, who has easier ways and whom no test here constrains. The image the test names is deliberately not the one the workflow ships, so a body that hardcodes a digest rather than reading the one it is given is red, and the link left at the results path is tested live as well as dangling, because cp refuses the second and writes through the first. shellcheck runs with --norc for the same reason actionlint runs with -config-file /dev/null: a config file committed beside them would decide what the lint reports. The scripts and their tests are a pinned list, so a script named there is linted, run, and has a test of its own. It is a guard against drift rather than against whoever writes the diff, who can edit the list in the same commit. shellcheck is pinned by the digest its own tag resolves to, so the version named in the comment is one a reader can verify. Signed-off-by: Nikita Kalyazin <nikita.kalyazin@e2b.dev>
1 parent ad45e84 commit a00522e

5 files changed

Lines changed: 373 additions & 141 deletions

File tree

‎.github/scripts/scan.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+
# The exit code is the verdict, not the results file's content: an unreachable api.osv.dev still writes a
3+
# well-formed zero-result SARIF, and uploading that files a clean analysis of the branch.
4+
set -euo pipefail
5+
6+
SCANNER=${SCANNER:?the scanner image must be pinned by digest}
7+
LOCKFILE=Cargo.lock
8+
OUTPUT=results.sarif
9+
WORKSPACE=${GITHUB_WORKSPACE:-$PWD}
10+
11+
out=$(mktemp -d)
12+
trap 'rm -rf "$out"' EXIT
13+
14+
status=0
15+
# Only the lockfile goes in, so a committed osv-scanner.toml never reaches the scan; --config holds
16+
# if that widens. Unwrapped, because the published action pins its image by tag rather than digest.
17+
docker run --rm \
18+
--user "$(id -u):$(id -g)" \
19+
--volume "$WORKSPACE/$LOCKFILE:/scan/$LOCKFILE:ro" \
20+
--volume "$out:/out" \
21+
--workdir /scan \
22+
"$SCANNER" \
23+
scan source \
24+
--config=/dev/null \
25+
--lockfile="/scan/$LOCKFILE" \
26+
--format=sarif \
27+
--output-file="/out/$OUTPUT" || status=$?
28+
29+
case "$status" in
30+
0 | 1) ;;
31+
*)
32+
echo "::error::OSV-Scanner exited $status; its results are not trustworthy"
33+
exit 1
34+
;;
35+
esac
36+
37+
# The image chooses what it leaves in its output directory, so what is copied out has to be a file
38+
# the scanner wrote rather than a link it aimed at one of the runner's.
39+
if [ ! -f "$out/$OUTPUT" ] || [ -L "$out/$OUTPUT" ]; then
40+
echo "::error::OSV-Scanner exited $status leaving no results of its own at $OUTPUT"
41+
exit 1
42+
fi
43+
44+
# Removed rather than written through, so a link left at the path does not redirect the results.
45+
rm -f "${WORKSPACE:?}/$OUTPUT"
46+
cp "$out/$OUTPUT" "$WORKSPACE/$OUTPUT"

‎.github/scripts/scan.test.sh‎

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
#!/usr/bin/env bash
2+
set -euo pipefail
3+
4+
here=$(cd "$(dirname "$0")" && pwd)
5+
tmp=$(mktemp -d)
6+
trap 'rm -rf "$tmp"' EXIT
7+
mkdir -p "$tmp/bin" "$tmp/work/.github/scripts"
8+
failures=0
9+
# Not the shipped digest: a body hardcoding the real one would match a pin that named it too.
10+
SCANNER_PIN="example.invalid/scanner@sha256:0000000000000000000000000000000000000000000000000000000000000000"
11+
12+
fail() {
13+
echo " $1"
14+
failures=$((failures + 1))
15+
}
16+
17+
# Real shapes: anything conditioned on a lockfile's or a SARIF result's structure is a no-op on a
18+
# token, and passes a test written with one.
19+
cat >"$tmp/lockfile" <<'EOF'
20+
[[package]]
21+
name = "smallvec"
22+
version = "1.6.0"
23+
source = "registry+https://github.com/rust-lang/crates.io-index"
24+
checksum = "fe0f37c9e8f3c5a4a66ad655a93c74daac4ad00c441533bf5c6e7990bb42604e"
25+
EOF
26+
cat >"$tmp/sarif" <<'EOF'
27+
{"version":"2.1.0","runs":[{"tool":{"driver":{"name":"osv-scanner","rules":[{"id":"RUSTSEC-2021-0003"}]}},
28+
"results":[{"ruleId":"RUSTSEC-2021-0003","level":"warning","message":{"text":"smallvec@1.6.0 is affected"},
29+
"locations":[{"physicalLocation":{"artifactLocation":{"uri":"Cargo.lock"}}}]}]}]}
30+
EOF
31+
32+
stub() {
33+
cat >"$tmp/bin/docker" <<EOF
34+
#!/usr/bin/env bash
35+
{ printf 'CALL\n'; printf '%s\n' "\$@"; } >>"$tmp/argv"
36+
lock=""
37+
outdir=""
38+
for arg in "\$@"; do
39+
case "\$arg" in
40+
*:/scan/Cargo.lock | *:/scan/Cargo.lock:ro) lock=\${arg%%:*} ;;
41+
*:/out) outdir=\${arg%%:*} ;;
42+
esac
43+
done
44+
cat "\$lock" >"$tmp/lockfile-seen" 2>/dev/null || printf 'MISSING' >"$tmp/lockfile-seen"
45+
if [ -n "\$outdir" ]; then
46+
case ${2:-results} in
47+
results) cp "$tmp/sarif" "\$outdir/results.sarif" ;;
48+
link) ln -s "$tmp/private" "\$outdir/results.sarif" ;;
49+
nothing) : ;;
50+
esac
51+
printf 'x' >"\$outdir/decoy"
52+
mkdir -p "\$outdir/.git" && printf 'x' >"\$outdir/.git/config"
53+
fi
54+
exit $1
55+
EOF
56+
chmod +x "$tmp/bin/docker"
57+
}
58+
59+
run_gate() {
60+
stub "$1" "${2:-results}"
61+
: >"$tmp/argv"
62+
cp "$tmp/lockfile" "$tmp/work/Cargo.lock"
63+
printf '[package]\n' >"$tmp/work/Cargo.toml"
64+
rm -rf "$tmp/elsewhere" "$tmp/work/decoy" "$tmp/work/.git"
65+
ln -sf "${3:-$tmp/elsewhere}" "$tmp/work/results.sarif"
66+
cp "$here/scan.sh" "$tmp/work/.github/scripts/scan.sh"
67+
(cd "$tmp/work" && PATH="$tmp/bin:$PATH" \
68+
env SCANNER="$SCANNER_PIN" GITHUB_WORKSPACE="$tmp/work" \
69+
bash .github/scripts/scan.sh) >"$tmp/out" 2>&1
70+
}
71+
72+
out_mount() { sed -n 's#^\(/.*\):/out$#\1#p' "$tmp/argv"; }
73+
74+
# Either way the image holds a writable path in the checked-out tree, which is the whole of what
75+
# mounting one file instead avoids.
76+
outside_the_workspace() {
77+
local real work
78+
real=$(readlink -m "$1")
79+
work=$(readlink -m "$tmp/work")
80+
case "$real" in "$work" | "$work"/*) return 1 ;; esac
81+
case "$work" in "$real"/*) return 1 ;; esac
82+
}
83+
84+
# A child that kept the workspace in a file of its own, or only in its own memory, is out of reach
85+
# of all four probes below, so this catches a leftover, not a deliberate one.
86+
outlived_the_gate() {
87+
local pids real
88+
# A /proc link reads canonically, so a TMPDIR reached through a symlink never matches $tmp.
89+
real=$(readlink -m "$tmp")
90+
mapfile -t pids < <({
91+
set +e # a /proc entry belonging to someone else is a non-zero grep, not a reason to stop
92+
grep -laF "$tmp" /proc/[0-9]*/environ /proc/[0-9]*/cmdline
93+
find /proc -maxdepth 2 -name cwd \( -lname "$tmp*" -o -lname "$real*" \)
94+
find /proc -maxdepth 3 -path '/proc/[0-9]*/fd/*' \( -lname "$tmp*" -o -lname "$real*" \)
95+
} 2>/dev/null | sed 's#/proc/\([0-9]*\)/.*#\1#' | sort -u)
96+
[ "${#pids[@]}" -eq 0 ] || kill "${pids[@]}" 2>/dev/null || true
97+
[ "${#pids[@]}" -ne 0 ]
98+
}
99+
100+
# A broken sweep answers "no survivors" too, and one working carrier answers for three broken ones,
101+
# so each gets a decoy holding it alone — rescanned, not slept on, being invisible until it execs.
102+
control() {
103+
for _ in 1 2 3 4 5; do
104+
outlived_the_gate && return 0
105+
done
106+
fail "a process holding the workspace only $1 outlives the gate unseen"
107+
}
108+
109+
( cd / && exec env -i HOLD="$tmp" sleep 60 ) >/dev/null 2>&1 &
110+
control "in its environment"
111+
( cd / && exec env -i sh -c ": $tmp; sleep 60; :" ) >/dev/null 2>&1 &
112+
control "on its command line"
113+
( cd "$tmp" && exec env -i sleep 60 ) >/dev/null 2>&1 &
114+
control "as its directory"
115+
( cd / && exec env -i sleep 60 9<"$tmp/lockfile" ) >/dev/null 2>&1 &
116+
control "as an open descriptor"
117+
118+
for code in 0 1; do
119+
if ! run_gate "$code"; then
120+
fail "a scanner exit of $code should pass the job, because the scan happened"
121+
elif [ "$(grep -c '^CALL$' "$tmp/argv")" != 1 ]; then
122+
fail "on exit $code the gate ran docker $(grep -c '^CALL$' "$tmp/argv") times, and only one call is pinned below"
123+
elif ! cmp -s "$tmp/work/results.sarif" "$tmp/sarif"; then
124+
fail "on exit $code the results file is not what the scanner wrote by the time the gate passed it"
125+
elif ! cmp -s "$tmp/lockfile-seen" "$tmp/lockfile"; then
126+
fail "on exit $code the scanner did not read the job's lockfile at the path the argv pins"
127+
elif [ -L "$tmp/work/results.sarif" ] || [ -e "$tmp/elsewhere" ]; then
128+
fail "on exit $code the results were written through a link left at their path"
129+
elif [ "$(out_mount | wc -l)" != 1 ]; then
130+
fail "on exit $code the scanner was given $(out_mount | wc -l) output mounts, and one is asserted below"
131+
elif ! outside_the_workspace "$(out_mount)"; then
132+
fail "on exit $code the scanner could write to $(out_mount), which is not outside the workspace"
133+
elif [ -e "$tmp/work/decoy" ] || [ -e "$tmp/work/.git" ]; then
134+
fail "on exit $code something the scanner wrote beside the results crossed into the workspace"
135+
elif [ -d "$(out_mount)" ]; then
136+
fail "on exit $code the scanner's output directory outlived the gate"
137+
elif outlived_the_gate; then
138+
fail "on exit $code the gate left work running, which can still reach the results it passed on"
139+
fi
140+
done
141+
142+
# 127 is the code an unreachable api.osv.dev really exits with; the ends bracket it.
143+
for code in 2 127 255; do
144+
if run_gate "$code"; then
145+
fail "a scanner exit of $code passed the job, but the results cannot be trusted"
146+
elif ! grep -q "::error::OSV-Scanner exited $code" "$tmp/out"; then
147+
fail "a scanner exit of $code failed the job without annotating the run"
148+
elif [ -d "$(out_mount)" ]; then
149+
fail "a scanner exit of $code left the output directory behind"
150+
fi
151+
done
152+
153+
printf 'PRIVATE TO THE RUNNER\n' >"$tmp/private"
154+
for leaves in link nothing; do
155+
if run_gate 0 "$leaves"; then
156+
fail "a scanner that left $leaves in place of its results passed the job"
157+
elif ! grep -q "::error::" "$tmp/out"; then
158+
fail "a scanner that left $leaves in place of its results failed without saying so"
159+
elif cmp -s "$tmp/work/results.sarif" "$tmp/private"; then
160+
fail "the scanner named a file of the runner's and the gate filed it as the scan"
161+
elif [ -d "$(out_mount)" ]; then
162+
fail "a scanner that left $leaves in place of its results left the output directory behind"
163+
fi
164+
done
165+
166+
# cp refuses a dangling link and writes through a live one, so only the removal covers both.
167+
printf 'BESIDE THE WORKSPACE\n' >"$tmp/live"
168+
if ! run_gate 0 results "$tmp/live"; then
169+
fail "a live link at the results path failed the job"
170+
elif ! cmp -s "$tmp/work/results.sarif" "$tmp/sarif"; then
171+
fail "with a live link at the results path the results are not what the scanner wrote"
172+
elif [ "$(cat "$tmp/live")" != "BESIDE THE WORKSPACE" ]; then
173+
fail "the results were written through a live link, into a file outside the workspace"
174+
fi
175+
176+
run_gate 0 || fail "the gate failed on exit 0, so the output directory below is not one a pass makes"
177+
first=$(out_mount)
178+
run_gate 0 || fail "the gate failed on exit 0 the second time"
179+
[ "$first" != "$(out_mount)" ] ||
180+
fail "the scanner's output directory is $first on both runs, so a link can be waiting at it"
181+
182+
# In order and entire: the scanner takes the last --config it is given.
183+
run_gate 0 || fail "the gate failed on exit 0, so the pinned argv below is not what a pass sends"
184+
expected=$(printf '%s\n' CALL run --rm --user "$(id -u):$(id -g)" \
185+
--volume "$tmp/work/Cargo.lock:/scan/Cargo.lock:ro" --volume "OUT:/out" --workdir /scan \
186+
"$SCANNER_PIN" scan source --config=/dev/null --lockfile=/scan/Cargo.lock --format=sarif \
187+
--output-file=/out/results.sarif)
188+
seen=$(sed 's#^/.*:/out$#OUT:/out#' "$tmp/argv")
189+
if [ "$seen" != "$expected" ]; then
190+
fail "docker is not invoked with exactly the pinned arguments:"
191+
diff <(printf '%s\n' "$expected") <(printf '%s\n' "$seen") | sed 's/^/ /'
192+
fi
193+
194+
if [ "$failures" -ne 0 ]; then
195+
echo "$failures assertion(s) failed"
196+
exit 1
197+
fi
198+
echo ok

0 commit comments

Comments
 (0)