Skip to content

Commit 05a7fcf

Browse files
committed
ci: check the analyses reached code scanning
What the workflow declares and what code scanning received are different claims, and only the second is evidence. A scan that runs and uploads nothing, or whose category merges with another tool's so that both are filed as one, is invisible to anything that reads the workflow file. A third job waits for both uploads and asserts that the commit this run scanned carries analyses under exactly the three expected categories, retrying while indexing catches up. It holds read on security-events, not write: it must not be able to delete what it is checking. It runs unless the run was cancelled, because needs: alone would skip it when a scan fails, and a skipped job reports success to whatever requires it. verify-analyses.test.sh stands gh in front of canned analyses and pins the verdict for each way the expected set can be wrong: a category filed on an earlier commit or another ref, a set split over pages, indexing that lands an attempt late, a call that fails once, a call that fails every time, and an API that stops answering after it has answered. An error on one attempt has to be retried; an error on every attempt still fails the job. The interval between attempts is recorded rather than waited out, and the test cannot speak to a body written to recognise its own launch and skip in the job, which review decides rather than a sibling test. An API that never answered is reported as such rather than as an empty set of analyses, and the fixtures carry an unexpected category at each end of the sort order and a substitution at each end, where a comparison that tolerates a longer list, or counts rather than compares, would pass. A commit sharing seven hex with the scanned one carries the whole expected set in another, and a category differing only in case appears beside the expected one as well as in its place, so an abbreviated match and a case-folded comparison are each red. The analyses are read with --paginate: the ref lists every analysis ever filed on it, newest first, and this pull request's ref passed a hundred in a little over a day, so a single page is a truncated answer that hides an older commit's analyses. Signed-off-by: Nikita Kalyazin <nikita.kalyazin@e2b.dev>
1 parent a00522e commit 05a7fcf

5 files changed

Lines changed: 373 additions & 10 deletions

File tree

‎.github/scripts/security-workflow.test.py‎

Lines changed: 51 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434
SCAN_PERMISSIONS = {
3535
"codeql": {"contents": "read", "actions": "read", "security-events": "write"},
3636
"osv": {"contents": "read", "actions": "read", "security-events": "write"},
37+
# Reads what the two above wrote; write here would let this job close what it is checking.
38+
"verify": {"contents": "read", "security-events": "read"},
3739
}
3840
PACKS = {
3941
"rust": "codeql/rust-queries:codeql-suites/rust-code-scanning.qls",
@@ -57,12 +59,14 @@
5759
JOB_KEYS = {
5860
"codeql": {"runs-on", "timeout-minutes", "permissions", "strategy", "steps"},
5961
"osv": {"runs-on", "timeout-minutes", "permissions", "steps"},
62+
"verify": {"runs-on", "timeout-minutes", "permissions", "needs", "if", "steps"},
6063
}
6164
RUNNER_JOB_KEYS = {"runs-on", "timeout-minutes", "steps"}
6265
STEP_SHAPES = ({"name", "uses", "with"}, {"name", "run"}, {"name", "run", "env"})
6366
JOB_STEPS = {
6467
"codeql": ("actions/checkout", "github/codeql-action/init", "github/codeql-action/analyze"),
6568
"osv": ("actions/checkout", "run", "run", "github/codeql-action/upload-sarif"),
69+
"verify": ("actions/checkout", "run"),
6670
}
6771
JOB_ACTIONS = {
6872
"codeql": ("actions/checkout", "github/codeql-action/init", "github/codeql-action/analyze"),
@@ -84,31 +88,34 @@
8488
MATRIX_KEYS = {"language", "include"}
8589
GENERATE_BODY = 'cd "${RUNNER_TEMP:?}" && cargo generate-lockfile --manifest-path "${GITHUB_WORKSPACE:?}/Cargo.toml"'
8690
SCAN_SCRIPT = ".github/scripts/scan.sh"
87-
SCAN_BODIES = (GENERATE_BODY, f"bash {SCAN_SCRIPT}")
88-
SCRIPTS = pathlib.Path(__file__).absolute().parent
89-
TESTS = sorted(p.relative_to(ROOT).as_posix() for p in SCRIPTS.glob("*.test.*"))
90-
SHELL_SCRIPTS = sorted(p.relative_to(ROOT).as_posix() for p in SCRIPTS.glob("*.sh"))
91+
VERIFY_SCRIPT = ".github/scripts/verify-analyses.sh"
92+
SCAN_BODIES = (GENERATE_BODY, f"bash {SCAN_SCRIPT}", f"bash {VERIFY_SCRIPT}")
93+
SHELL_SCRIPTS = [
94+
".github/scripts/scan.sh",
95+
".github/scripts/scan.test.sh",
96+
".github/scripts/verify-analyses.sh",
97+
".github/scripts/verify-analyses.test.sh",
98+
]
9199
# A `# shellcheck disable=` line silences the lint from inside the file it lints, so what any of them may silence is pinned here.
92100
SILENCED = []
101+
TESTS = [s for s in SHELL_SCRIPTS if s.endswith(".test.sh")] + [".github/scripts/security-workflow.test.py"]
93102
UNTESTED = [s for s in SHELL_SCRIPTS if not s.endswith(".test.sh") and s[:-3] + ".test.sh" not in TESTS]
94-
EXPECTED_TESTS = [
95-
".github/scripts/scan.test.sh",
96-
".github/scripts/security-workflow.test.py",
97-
]
98103
SHELLCHECK_BODY = 'docker run --rm --user "$(id -u):$(id -g)" --volume "$GITHUB_WORKSPACE:$GITHUB_WORKSPACE:ro" --workdir "$GITHUB_WORKSPACE" "$SHELLCHECK" \\\n' + " \\\n".join([" --norc"] + [f" {script}" for script in SHELL_SCRIPTS])
99104
INTERPRETERS = {"sh": "bash", "py": "python3"}
100105
SHELLCHECK_REPO = "koalaman/shellcheck"
101106
SHELLCHECK_DIGEST = "sha256:61862eba1fcf09a484ebcc6feea46f1782532571a34ed51fedf90dd25f925a8d"
102107
# Keyed by what the step runs: the env names that step may set. The runner sets the rest.
103108
STEP_ENV = {
104109
SCAN_SCRIPT: {"SCANNER"},
110+
VERIFY_SCRIPT: {"GH_TOKEN", "REF", "SHA", "EXPECTED"},
105111
'"$LINTER"': {"LINTER"},
106112
'"$SHELLCHECK"': {"SHELLCHECK"},
107113
}
108114
INIT_KEYS = {"languages", "build-mode", "config"}
109115
ANALYZE_KEYS = {"category"}
110116
# Pinned: another ${{ }} naming matrix.language reads as per-language but resolves to one.
111117
ANALYZE_CATEGORY = "/language:${{ matrix.language }}"
118+
VERIFY_IF = "${{ !cancelled() }}"
112119
UPLOAD_KEYS = {"sarif_file", "category"}
113120
# The whole file, compared as one value: only github-actions belongs here, the consuming fork's
114121
# lockfile owning dependency versions.
@@ -478,6 +485,42 @@ def minutes(value):
478485
)
479486

480487

488+
verify_steps = jobs["verify"]["steps"] if "verify" in jobs else []
489+
check(
490+
jobs.get("verify", {}).get("needs") == ["codeql", "osv"],
491+
f"verify: it needs {jobs.get('verify', {}).get('needs')}, not ['codeql', 'osv']; the list is "
492+
"pinned because a verify that does not wait for both uploads reads an empty set as a missing scan",
493+
)
494+
check(
495+
expr(jobs.get("verify", {}).get("if")) == VERIFY_IF,
496+
f"verify: it runs if {jobs.get('verify', {}).get('if')!r}, not {VERIFY_IF}; the guard is pinned "
497+
"because a verify that a failed scan skips reports that skip as success to whatever requires it",
498+
)
499+
verify_step = next((st for st in verify_steps if launches(str(st.get("run", "")), VERIFY_SCRIPT)), None)
500+
check(verify_step is not None, f"verify: no step runs {VERIFY_SCRIPT}, so nothing reads what was uploaded")
501+
if verify_step is not None:
502+
env = verify_step.get("env") or {}
503+
check(
504+
set(str(env.get("EXPECTED", "")).split(" ")) == CATEGORIES,
505+
f"verify: it expects {str(env.get('EXPECTED', '')).split(' ')}, not {sorted(CATEGORIES)}; the "
506+
"categories it waits for are what makes a missing or merged upload visible",
507+
)
508+
check(
509+
expr(env.get("SHA")) == "${{ github.sha }}",
510+
f"verify: it reads the analyses of {env.get('SHA')!r}, not the commit this run scanned",
511+
)
512+
# Already refs/pull/<n>/merge on a pull request, which is where those analyses are filed.
513+
check(
514+
expr(env.get("REF")) == "${{ github.ref }}",
515+
f"verify: it reads the analyses of {env.get('REF')!r}, not the ref this run scanned",
516+
)
517+
# The scopes above bound this job only while it runs on the token they apply to.
518+
check(
519+
expr(env.get("GH_TOKEN")) == "${{ github.token }}",
520+
f"verify: it authenticates with {env.get('GH_TOKEN')!r}; only the job's own token carries "
521+
"the scopes declared above, and only the one spelling of it is read here",
522+
)
523+
481524
osv_steps = jobs["osv"]["steps"]
482525
gen_i = next((i for i, s in enumerate(osv_steps) if "generate-lockfile" in str(s.get("run", ""))), None)
483526
scan_i = next((i for i, s in enumerate(osv_steps) if SCAN_SCRIPT in str(s.get("run", ""))), None)

‎.github/scripts/verify-analyses.sh‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
#!/usr/bin/env bash
2+
# Reading the workflow cannot see a scan that uploaded nothing, or one whose category merged
3+
# with another tool's so that both were filed as one.
4+
set -euo pipefail
5+
6+
REPO=${GITHUB_REPOSITORY:?}
7+
REF=${REF:?}
8+
SHA=${SHA:?}
9+
EXPECTED=${EXPECTED:?the categories that must exist, space separated}
10+
11+
oneline() { printf '%s' "$1" | tr '\n' ' '; }
12+
13+
# Compared as JSON text, because a category is free text: joined by any character a category may
14+
# itself contain, one filed as "a b c" would read as three present and hide two missing.
15+
read -ra expected <<<"$EXPECTED"
16+
want=$(printf '"%s"\n' "${expected[@]}" | sort -u)
17+
found=""
18+
answered=""
19+
20+
# Both uploads wait for processing, but that wait gives up after about two and a half minutes and
21+
# passes anyway, so this covers processing as well as listing lag. No run has needed a second
22+
# attempt, which is why the one reached is printed rather than assumed.
23+
for attempt in $(seq 10); do
24+
# The ref lists every analysis ever filed on it, and one page is a truncated answer.
25+
# A failed call says nothing about what is filed, so the last answer the API gave is kept.
26+
if page=$(gh api --paginate "repos/$REPO/code-scanning/analyses?ref=$REF&per_page=100" \
27+
--jq ".[] | select(.commit_sha == \"$SHA\") | .category | @json"); then
28+
found=$(printf '%s\n' "$page" | sort -u)
29+
answered=yes
30+
fi
31+
[ "$found" = "$want" ] && break
32+
[ "$attempt" = 10 ] || sleep 15
33+
done
34+
35+
if [ "$found" != "$want" ]; then
36+
if [ -n "$answered" ]; then
37+
echo "::error::$SHA on $REF carries analyses under [$(oneline "$found")], not [$(oneline "$want")]"
38+
else
39+
echo "::error::the code scanning API never answered for $SHA on $REF, so what it carries is unknown"
40+
fi
41+
exit 1
42+
fi
43+
echo "ok: $(oneline "$want"), all on $SHA, at attempt $attempt"

0 commit comments

Comments
 (0)