Skip to content

Commit d32ed5d

Browse files
fix(ci): scan both platforms upstream and derive the gate's trigger
Two review findings on the upstream scan, both real. The `release-images` path filter listed the gate's entry point but not the modules it loads, so a pull request touching `scripts/lib/json.ts` or `scripts/lib/process.ts` passed `release_images_changed: false` and skipped the scan. The closure is five files. Rather than trust a hand-maintained list — the same drift this PR exists to prevent — `ci-contract.test.ts` now re-walks the imports from the entry point and asserts every file it reaches appears in the filter, so the enumeration cannot fall behind the code. `scanAll` resolved only `linux/amd64`, while an exception matches on `image | platform | vulnerability | package | installedVersion`. A single-platform pre-release scan half-checks each subject, leaving an arm64-only finding — or an arm64 exception nobody wrote — to the release gate by construction. The upstream scan now covers both platforms, and `ScanOutcome` carries the platform so a result can name which half it is. `scan-main-images.ts` stays `linux/amd64`: a finding on either architecture of an image we build is fixed by the same rebuild, and the Version PR preflight scans both before a release is cut. A pinned upstream digest has no rebuild and its fix arrives as a digest bump in a pull request, so that scan cannot defer half a subject. Both the script and the remediation guide now say so. Verified: arm64 evidence re-gathered rather than assumed — qemu is unavailable, so the arm64 binaries were extracted and read with `readelf`. nats-server and traefik are static AArch64 executables with no dynamic section at all, nginx links libssl/libcrypto and not libexpat, and its embedded configure line carries the same --with-http_v3_module. Every justification holds unchanged, the arm64 finding sets are identical to amd64, and the gate reports all eight subjects pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 5a42a7b commit d32ed5d

7 files changed

Lines changed: 98 additions & 18 deletions

File tree

.github/workflows/ci-security-scan.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ jobs:
5454
if: ${{ !cancelled() }}
5555
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
5656
with:
57-
name: vulnerability-policy-upstream-linux-amd64
57+
name: vulnerability-policy-upstream
5858
path: reports
5959
if-no-files-found: warn
6060
retention-days: 7

.github/workflows/cicd.yml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,12 +199,16 @@ jobs:
199199
- '.github/workflows/ci-security-scan.yml'
200200
release-images:
201201
# The pinned upstream digests need no build, so the pull request that edits them —
202-
# a Renovate digest bump included — is the one that scans them (#1741).
202+
# a Renovate digest bump included — is the one that scans them (#1741). The script
203+
# entries are the gate's whole transitive import closure, which ci-contract.test.ts
204+
# re-derives from the imports and asserts against this list.
203205
- 'security/release-images.json'
204206
- 'security/vulnerability-policy.json'
205207
- 'scripts/scan-upstream-images.ts'
206-
- 'scripts/lib/image-scan.ts'
207208
- 'scripts/check-release-vulnerabilities.ts'
209+
- 'scripts/lib/image-scan.ts'
210+
- 'scripts/lib/json.ts'
211+
- 'scripts/lib/process.ts'
208212
- '.github/workflows/ci-security-scan.yml'
209213
docker-config:
210214
- '.github/workflows/ci-docker-build.yml'

docs/contributor/vulnerability-remediation.mdx

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,9 @@ attached to the release; the filter is in the evaluator, not a `--ignore-unfixed
2929
| When | Subject | Effect |
3030
| --- | --- | --- |
3131
| Every image build (`reusable-docker-build.yml`) | the `linux/amd64` image the run just pushed | fails the run |
32-
| A pull request touching `security/release-images.json` (`ci-security-scan.yml`) | the `linux/amd64` side of each pinned upstream digest | fails the run |
32+
| A pull request touching `security/release-images.json` (`ci-security-scan.yml`) | both platforms of each pinned upstream digest | fails the run |
3333
| Release evidence (`release.yml`) | both platforms of every first-party and upstream image | no release is created |
34-
| Weekly (`rescan-main-images.yml`) | the `linux/amd64` image behind each `:main` tag, and each pinned upstream digest | opens or updates a tracking issue |
34+
| Weekly (`rescan-main-images.yml`) | the `linux/amd64` image behind each `:main` tag, and both platforms of each pinned upstream digest | opens or updates a tracking issue |
3535
| Weekly (`rescan-release-images.yml`) | every subject in the latest published release | fails and comments on [the vulnerability response issue](https://github.com/ls1intum/Hephaestus/issues/1369) |
3636

3737
All five evaluate `security/vulnerability-policy.json` through `scripts/check-release-vulnerabilities.ts`;
@@ -48,6 +48,15 @@ A pinned upstream image cannot be patched by rebuilding anything here. Its remed
4848
`security/release-images.json` — which Renovate proposes, and which the pull request proposing it now
4949
scans — and until upstream republishes the tag there may be no digest to bump to.
5050

51+
That is also why the upstream scan covers both platforms while the scans of images we build cover
52+
`linux/amd64` only. An exception matches on `image | platform | vulnerability | package |
53+
installedVersion`, so a single-platform scan can only ever half-check a subject. For an image we
54+
build, the other half costs little to leave to the release: a finding on either architecture is
55+
fixed by the same rebuild, and the Version PR preflight scans both before the release is cut. For a
56+
pinned upstream digest there is no rebuild, the fix is a digest bump nobody can make until upstream
57+
publishes one, and the bump arrives in a pull request — so an arm64-only finding has to fail that
58+
pull request rather than the release.
59+
5160
## Exceptions
5261

5362
An exception lets one fixable finding through for at most 90 days. It is an entry in

scripts/ci-contract.test.ts

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
import assert from "node:assert/strict";
2+
import { existsSync } from "node:fs";
23
import { glob, readFile } from "node:fs/promises";
4+
import path from "node:path";
35
import { describe, test } from "node:test";
46

57
import { type Document, isMap, isSeq, parseDocument, type YAMLMap } from "yaml";
68

79
import { planRelease, releaseOutputs } from "./plan-release.ts";
810
import { planSubjects } from "./scan-main-images.ts";
9-
import { planUpstreamSubjects } from "./scan-upstream-images.ts";
11+
import { PLATFORMS, planUpstreamSubjects } from "./scan-upstream-images.ts";
1012
import { validateManifest } from "./verify-release-evidence.ts";
1113

1214
function job(source: string, name: string): string {
@@ -40,6 +42,30 @@ function escapeRegExp(value: string): string {
4042
return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
4143
}
4244

45+
/**
46+
* Every repository script a given entry point loads, transitively — its static relative imports,
47+
* plus any sibling script it names as a string, which is how the scanners reach the policy
48+
* evaluator (they spawn it rather than importing it, so that its exit status is the verdict).
49+
*/
50+
async function importClosure(entry: string): Promise<string[]> {
51+
const seen = new Set<string>();
52+
const queue = [entry];
53+
while (queue.length > 0) {
54+
const file = queue.shift();
55+
if (file === undefined || seen.has(file)) continue;
56+
seen.add(file);
57+
const source = await readFile(file, "utf8");
58+
const directory = path.dirname(file);
59+
for (const [, specifier] of source.matchAll(/from\s+"(\.[^"]+\.ts)"/g))
60+
queue.push(path.normalize(path.join(directory, specifier ?? "")));
61+
for (const [, name] of source.matchAll(/"([\w.-]+\.ts)"/g)) {
62+
const candidate = path.normalize(path.join(directory, "..", name ?? ""));
63+
if (candidate.startsWith("scripts/") && existsSync(candidate)) queue.push(candidate);
64+
}
65+
}
66+
return [...seen].toSorted();
67+
}
68+
4369
/** The `with` map of the first step in a job whose `uses` starts with `action`. */
4470
function step(workflow: Document, jobPath: string[], action: string): YAMLMap {
4571
const steps = workflow.getIn([...jobPath, "steps"]);
@@ -445,10 +471,28 @@ void describe("CI contract", () => {
445471
/run: node scripts\/scan-upstream-images\.ts reports --report-only\n/,
446472
);
447473
const detection = job(await readFile(".github/workflows/cicd.yml", "utf8"), "detect-changes");
474+
const filter = pathFilter(detection, "release-images");
448475
assert.match(
449-
pathFilter(detection, "release-images"),
476+
filter,
450477
/- 'security\/release-images\.json'[\s\S]*- 'security\/vulnerability-policy\.json'/,
451478
);
479+
// The trigger is derived, not trusted: a filter that lists the entry point but not the module
480+
// it parses JSON with skips the gate on the pull request that breaks the parser. Re-walk the
481+
// imports and require every file the gate actually loads to appear.
482+
for (const file of await importClosure("scripts/scan-upstream-images.ts"))
483+
assert.ok(
484+
filter.includes(`- '${file}'`),
485+
`release-images must trigger on ${file}, which the upstream scan loads`,
486+
);
487+
});
488+
489+
void test("scans both released platforms before the release, not just linux/amd64", async () => {
490+
// The policy match key is `image | platform | vulnerability | package | installedVersion`, so
491+
// a single-platform pre-release scan leaves an arm64-only finding — or an arm64 exception
492+
// nobody wrote — to be discovered by the release gate, which is the failure this PR removes.
493+
assert.deepEqual([...PLATFORMS], ["linux/amd64", "linux/arm64"]);
494+
const source = await readFile("scripts/scan-upstream-images.ts", "utf8");
495+
assert.match(source, /for \(const platform of PLATFORMS\)/);
452496
});
453497

454498
void test("rescans main's images weekly and reports drift to an issue, not a status", async () => {

scripts/lib/image-scan.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ import path from "node:path";
1515
import { asRecord, isRecord } from "./json.ts";
1616
import { output, run, succeeds } from "./process.ts";
1717

18-
/** The one platform these scans cover. Alpine and Debian ship the same package versions across
19-
* architectures, and the release still scans both, where the evidence bundle must be complete. */
18+
/** The platform a caller gets when it names none. Callers that must match the release evidence
19+
* gate, which is keyed per platform, name both — see `scan-upstream-images.ts`. */
2020
export const PLATFORM = "linux/amd64";
2121

2222
const DIGEST = /^sha256:[a-f0-9]{64}$/;
@@ -32,6 +32,9 @@ export interface ScanOutcome {
3232
readonly image: string;
3333
/** `false` when the policy evaluator rejected something; never throws the run. */
3434
readonly passed: boolean;
35+
/** Carried because the policy match key is per platform, so an outcome that does not name one
36+
* cannot be reported or compared against the release gate's subjects. */
37+
readonly platform: string;
3538
}
3639

3740
export interface ScanOptions {
@@ -130,15 +133,16 @@ async function scan(
130133
"security/vulnerability-policy.json",
131134
result,
132135
];
133-
if (await evaluatorPassed(evaluator, annotate)) return { image: subject.image, passed: true };
136+
if (await evaluatorPassed(evaluator, annotate))
137+
return { image: subject.image, passed: true, platform };
134138
if (!existsSync(result)) {
135139
// It threw before writing anything — a malformed report or policy, not a finding. Re-run so
136140
// the reason reaches the log, then fail: this is an infrastructure failure, and unlike a CVE
137141
// it is fixed by a commit.
138142
await run("node", evaluator);
139143
throw new Error(`vulnerability policy evaluation produced no result for ${subject.image}`);
140144
}
141-
return { image: subject.image, passed: false };
145+
return { image: subject.image, passed: false, platform };
142146
}
143147

144148
export async function scanAll(

scripts/scan-main-images.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@
1111
* the release evidence gate demands. Both share `lib/image-scan.ts`, which hands each Trivy report
1212
* to `check-release-vulnerabilities.ts`.
1313
*
14+
* Only `linux/amd64`, unlike the upstream scan, which covers both. A finding on either architecture
15+
* of an image we build is fixed by the same rebuild, and the Version PR preflight scans both before
16+
* a release is cut; a pinned upstream digest has no rebuild and its fix arrives as a digest bump in
17+
* a pull request, so that scan cannot leave half a subject to the release gate.
18+
*
1419
* A finding never fails this run. `report-vulnerability-drift.ts` reads the `.policy.json` files
1520
* written here and routes them to a tracking issue; only an infrastructure failure — a missing tag, an
1621
* unreachable registry, a Trivy crash — is worth a red status on a schedule nobody triggered.

scripts/scan-upstream-images.ts

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88
* been there for days (issue #1741).
99
*
1010
* They need no build — a digest in a committed file is the whole subject — so this is cheap and
11-
* deterministic, and it blocks by default. A Renovate digest bump edits this very file, so the pull
12-
* request proposing the bump is the one that scans it.
11+
* deterministic, it covers both released platforms, and it blocks by default. A Renovate digest
12+
* bump edits this very file, so the pull request proposing the bump is the one that scans it.
1313
*
1414
* `--report-only` is the weekly rescan, where a finding is routed to a tracking issue rather than a
1515
* red status: a CVE published after the digest was pinned belongs to no commit, and a pinned
@@ -18,11 +18,19 @@
1818
import { writeFile } from "node:fs/promises";
1919
import path from "node:path";
2020

21-
import { PLATFORM, scanAll, type Subject } from "./lib/image-scan.ts";
21+
import { type ScanOutcome, scanAll, type Subject } from "./lib/image-scan.ts";
2222
import { asArray, asRecord, asString, readJsonFile } from "./lib/json.ts";
2323

2424
const DIGEST = /^sha256:[a-f0-9]{64}$/;
2525

26+
/**
27+
* Both platforms, because the release evidence gate scans both and an exception matches on
28+
* `image | platform | vulnerability | package | installedVersion`. Scanning only `linux/amd64`
29+
* would leave an arm64-only finding — or an arm64 exception nobody wrote — discoverable for the
30+
* first time at the release, which is the failure this whole script exists to remove.
31+
*/
32+
export const PLATFORMS = ["linux/amd64", "linux/arm64"] as const;
33+
2634
export interface UpstreamSubject extends Subject {
2735
/** The multi-architecture index digest pinned in the inventory, which is what the release
2836
* manifest records as each platform subject's `indexDigest`. */
@@ -53,14 +61,20 @@ if (import.meta.main) {
5361
throw new Error("usage: scan-upstream-images <directory> [--report-only]");
5462
const reportOnly = option === "--report-only";
5563
const subjects = planUpstreamSubjects(await readJsonFile("security/release-images.json"));
56-
const outcomes = await scanAll(subjects, directory, { annotate: !reportOnly });
64+
const outcomes: ScanOutcome[] = [];
65+
for (const platform of PLATFORMS)
66+
outcomes.push(...(await scanAll(subjects, directory, { annotate: !reportOnly, platform })));
5767
await writeFile(
5868
path.join(directory, "upstream-scan.json"),
59-
`${JSON.stringify({ platform: PLATFORM, scannedAt: new Date().toISOString(), subjects }, null, 2)}\n`,
69+
`${JSON.stringify({ platforms: PLATFORMS, scannedAt: new Date().toISOString(), subjects }, null, 2)}\n`,
6070
);
6171
for (const outcome of outcomes)
62-
process.stdout.write(`${outcome.image}: ${outcome.passed ? "pass" : "fail"}\n`);
63-
const failed = outcomes.filter((outcome) => !outcome.passed).map((outcome) => outcome.image);
72+
process.stdout.write(
73+
`${outcome.image} (${outcome.platform}): ${outcome.passed ? "pass" : "fail"}\n`,
74+
);
75+
const failed = outcomes
76+
.filter((outcome) => !outcome.passed)
77+
.map((outcome) => `${outcome.image} (${outcome.platform})`);
6478
if (failed.length > 0 && !reportOnly) {
6579
process.stderr.write(
6680
`::error::pinned upstream images do not satisfy the vulnerability policy: ${failed.join(", ")}\n`,

0 commit comments

Comments
 (0)