ReviewOS

also looking at this

craft-native/craft

Decide dependency advisories instead of counting them

#320
Merged glennmichael123 wants to merge fix/advisory-retention-policy into main
6 files +324 -1
.github/workflows/ci.ymlmodified+9-0
Changes to .github/workflows/ci.yml
@@ -297,6 +297,15 @@ jobs:
297297 - name: Test agent environment briefing
298298 run: bun test scripts/agents-guidelines.test.ts
299299
300 - name: Test dependency advisory policy
301 run: bun test scripts/advisory-policy.test.ts
302
303 # Reads the registry, so this is the step that turns a newly published
304 # advisory into a red build rather than a larger number in a report.
305 # Clearing it is one reviewed entry in scripts/retained-advisories.ts.
306 - name: Review dependency advisories
307 run: bun scripts/advisory-policy.ts
308
300309 - name: Test merged SBOM inventory
301310 run: bun test scripts/merge-sbom.test.ts
302311
.github/workflows/release.ymlmodified+5-0
Changes to .github/workflows/release.yml
@@ -22,6 +22,11 @@ jobs:
2222 - name: Validate tag and workspace versions
2323 run: bun scripts/release-version.ts
2424
25 # A release must not carry an advisory nobody has reviewed, or a review
26 # that expired while the release was being prepared.
27 - name: Review dependency advisories
28 run: bun scripts/advisory-policy.ts
29
2530 release-sbom:
2631 needs: validate-release
2732 uses: ./.github/workflows/sbom.yml
docs/VULNERABILITY_SCANNING.mdmodified+25-1
Changes to docs/VULNERABILITY_SCANNING.md
@@ -23,7 +23,31 @@ tracks the reviewed High/Critical policy and shipping-scope analysis; issue
2323[#276](https://github.com/craft-native/craft/issues/276) tracks locked dependency
2424advisories. Native binary scanning records exact hashes and detected components,
2525but static Zig code may not be identifiable from the binary alone; retain the
26full-source review. No baseline or vulnerability exemption is added by this workflow.
26full-source review.
27
28## Reviewed advisory retention
29
30Lockfile advisories are decided rather than counted. `scripts/advisory-policy.ts`
31reads `bun audit --json` and fails when an advisory has not been reviewed, when a
32review has passed its expiry, when a retained entry no longer matches any current
33advisory, and on any High or Critical finding, which can never be retained. It runs
34in CI on every pull request and again in `validate-release`, so a release cannot
35carry an unreviewed advisory.
36
37Retentions live in `scripts/retained-advisories.ts`, one entry per advisory, each
38answering a single question: by what path would an installed Craft package reach
39this code? An advisory that reaches a published artifact is fixed instead of
40retained. Entries carry a review date and an expiry so that the reasoning is
41re-examined rather than inherited.
42
43A newly published advisory therefore turns the build red until someone either
44fixes it or writes down why it is unreachable. That is deliberate: the previous
45arrangement reported a count and stayed green, so a new finding was invisible.
46
47`bun audit fix` is not part of any gate. It is willing to *downgrade* a package so
48that it falls below an advisory window rather than upgrading past it, and reports
49that as a fix, so remediation stays a reviewed decision. Dependency updates go
50through buddy-bot.
2751
2852Syft and Grype Linux x64 downloads are pinned by release version and SHA-256 in
2953`.github/workflows/sbom.yml`. Tool updates must update both the asset URL and its
scripts/advisory-policy.test.tsadded+70-0
Changes to scripts/advisory-policy.test.ts
@@ -0,0 +1,70 @@
1import { expect, test } from 'bun:test'
2import { assertAdvisoryPolicy, collectAdvisories, reviewAdvisories } from './advisory-policy'
3import { retainedAdvisories } from './retained-advisories'
4
5const low = { id: 1, url: 'https://example.test/a', title: 'dev server read', severity: 'low' }
6const moderate = { id: 2, url: 'https://example.test/b', title: 'ssr xss', severity: 'moderate' }
7const high = { id: 3, url: 'https://example.test/c', title: 'rce', severity: 'high' }
8
9const reviewed = (id: number, pkg: string, expires: string) => ({
10 id,
11 package: pkg,
12 ghsa: `GHSA-test-${id}`,
13 reason: 'dev only, reaches no published artifact',
14 reviewed: '2026-09-01',
15 expires,
16})
17
18test('a reviewed, in-date, unreachable advisory passes', () => {
19 const review = assertAdvisoryPolicy({ esbuild: [low] }, [reviewed(1, 'esbuild', '2026-12-31')], '2026-09-25')
20 expect(review.retained).toHaveLength(1)
21 expect(review.unreviewed).toHaveLength(0)
22})
23
24test('an advisory nobody reviewed fails, and the message names it', () => {
25 expect(() => assertAdvisoryPolicy({ svelte: [moderate] }, [], '2026-09-25'))
26 .toThrow('nobody has reviewed')
27})
28
29test('high and critical can never be retained, however carefully worded', () => {
30 expect(() => assertAdvisoryPolicy({ tar: [high] }, [reviewed(3, 'tar', '2099-01-01')], '2026-09-25'))
31 .toThrow('cannot be retained')
32})
33
34test('a retention past its expiry fails, so the reasoning gets re-examined', () => {
35 expect(() => assertAdvisoryPolicy({ esbuild: [low] }, [reviewed(1, 'esbuild', '2026-09-24')], '2026-09-25'))
36 .toThrow('past review')
37})
38
39test('a retention matching no current advisory fails as stale', () => {
40 expect(() => assertAdvisoryPolicy({}, [reviewed(1, 'esbuild', '2026-12-31')], '2026-09-25'))
41 .toThrow('match no current advisory')
42})
43
44test('a retention is scoped to its own package, not the id alone', () => {
45 expect(() => assertAdvisoryPolicy({ svelte: [low] }, [reviewed(1, 'esbuild', '2026-12-31')], '2026-09-25'))
46 .toThrow('nobody has reviewed')
47})
48
49test('an unreadable or failed scan is never an empty report', () => {
50 expect(() => collectAdvisories(null)).toThrow('expected an object')
51 expect(() => collectAdvisories([])).toThrow('expected an object')
52 expect(() => collectAdvisories({ esbuild: 'none' })).toThrow('does not list advisories')
53 expect(() => collectAdvisories({ esbuild: [{ id: 1 }] })).toThrow('missing id or severity')
54 expect(() => collectAdvisories({ esbuild: [{ id: 1, severity: 'spicy' }] })).toThrow('Invalid bun audit severity')
55})
56
57test('no advisories at all is a clean pass', () => {
58 expect(reviewAdvisories([], [], '2026-09-25').retained).toHaveLength(0)
59})
60
61test('every retained advisory carries a reason, a review date and an expiry', () => {
62 expect(retainedAdvisories.length).toBeGreaterThan(0)
63 for (const entry of retainedAdvisories) {
64 expect(entry.ghsa).toMatch(/^GHSA-/)
65 expect(entry.reason.length).toBeGreaterThan(30)
66 expect(entry.reviewed).toMatch(/^\d{4}-\d{2}-\d{2}$/)
67 expect(entry.expires).toMatch(/^\d{4}-\d{2}-\d{2}$/)
68 expect(entry.expires > entry.reviewed).toBe(true)
69 }
70})
scripts/advisory-policy.tsadded+131-0
Changes to scripts/advisory-policy.ts
@@ -0,0 +1,131 @@
1/**
2 * Turns `bun audit --json` into a decision, so a dependency advisory is either
3 * fixed or reviewed, never merely unnoticed.
4 *
5 * The scan this replaces reported counts and stayed green. Counting is not a
6 * policy: a report of six moderate findings looks the same on the day a seventh
7 * arrives. This fails on anything nobody has looked at, and on any review that
8 * has gone stale, so the green state means "reviewed" rather than "nothing
9 * critical today".
10 *
11 * Note what is deliberately absent: `bun audit fix`. It is willing to *downgrade*
12 * a package to fall below an advisory window and report that as fixed, which is
13 * why remediation stays a human decision and this only ever reports.
14 */
15
16import type { RetainedAdvisory } from './retained-advisories'
17
18/** One advisory against one package, flattened out of the report's per-package keys. */
19export type Advisory = {
20 package: string
21 id: number
22 severity: string
23 title: string
24 url: string
25}
26
27const severities = ['critical', 'high', 'moderate', 'low', 'info'] as const
28
29/** Severities that may never be retained, whatever the reasoning says. */
30const unretainable = ['critical', 'high']
31
32/**
33 * Flatten and validate the report.
34 *
35 * Validation is not ceremony here: a scanner that fails, or a future Bun that
36 * changes this shape, must not read as an empty report. Anything unrecognised
37 * throws rather than defaulting to zero findings.
38 */
39export function collectAdvisories(report: unknown): Advisory[] {
40 if (!report || typeof report !== 'object' || Array.isArray(report))
41 throw new Error('Invalid bun audit report: expected an object keyed by package name')
42 const advisories: Advisory[] = []
43 for (const [name, list] of Object.entries(report as Record<string, unknown>)) {
44 if (!Array.isArray(list))
45 throw new Error(`Invalid bun audit report: ${name} does not list advisories`)
46 for (const entry of list) {
47 const advisory = entry as Partial<Advisory>
48 if (typeof advisory?.id !== 'number' || typeof advisory?.severity !== 'string')
49 throw new Error(`Invalid bun audit advisory for ${name}: missing id or severity`)
50 if (!severities.includes(advisory.severity as typeof severities[number]))
51 throw new Error(`Invalid bun audit severity for ${name}: ${advisory.severity}`)
52 advisories.push({
53 package: name,
54 id: advisory.id,
55 severity: advisory.severity,
56 title: typeof advisory.title === 'string' ? advisory.title : '',
57 url: typeof advisory.url === 'string' ? advisory.url : '',
58 })
59 }
60 }
61 return advisories
62}
63
64export type AdvisoryReview = {
65 /** Reviewed, unreachable, and still in date. */
66 retained: Advisory[]
67 /** Nobody has reviewed these. Any one of them fails the gate. */
68 unreviewed: Advisory[]
69 /** Reviewed once, but the review is past its expiry date. */
70 expired: RetainedAdvisory[]
71 /** Retained entries that match no current advisory, so the note is stale. */
72 stale: RetainedAdvisory[]
73 /** High or critical findings, which are never retainable. */
74 blocking: Advisory[]
75}
76
77export function reviewAdvisories(advisories: Advisory[], retained: RetainedAdvisory[], today: string): AdvisoryReview {
78 const byId = new Map(retained.map(entry => [`${entry.package}#${entry.id}`, entry]))
79 const seen = new Set<string>()
80 const review: AdvisoryReview = { retained: [], unreviewed: [], expired: [], stale: [], blocking: [] }
81
82 for (const advisory of advisories) {
83 const key = `${advisory.package}#${advisory.id}`
84 const entry = byId.get(key)
85 if (unretainable.includes(advisory.severity)) {
86 review.blocking.push(advisory)
87 if (entry) seen.add(key)
88 continue
89 }
90 if (!entry) {
91 review.unreviewed.push(advisory)
92 continue
93 }
94 seen.add(key)
95 if (entry.expires < today) review.expired.push(entry)
96 else review.retained.push(advisory)
97 }
98
99 for (const entry of retained) {
100 if (!seen.has(`${entry.package}#${entry.id}`)) review.stale.push(entry)
101 }
102 return review
103}
104
105/** Throw a message that says which of the four ways the policy was broken. */
106export function assertAdvisoryPolicy(report: unknown, retained: RetainedAdvisory[], today: string): AdvisoryReview {
107 const review = reviewAdvisories(collectAdvisories(report), retained, today)
108 const problems: string[] = []
109 if (review.blocking.length)
110 problems.push(`${review.blocking.length} high or critical advisories, which cannot be retained: ${review.blocking.map(a => `${a.package} ${a.url || a.id}`).join(', ')}`)
111 if (review.unreviewed.length)
112 problems.push(`${review.unreviewed.length} advisories nobody has reviewed; add them to scripts/retained-advisories.ts with a reason, or fix them: ${review.unreviewed.map(a => `${a.package} ${a.url || a.id}`).join(', ')}`)
113 if (review.expired.length)
114 problems.push(`${review.expired.length} retentions are past review; re-examine and move the expiry: ${review.expired.map(e => `${e.package} ${e.ghsa} expired ${e.expires}`).join(', ')}`)
115 if (review.stale.length)
116 problems.push(`${review.stale.length} retentions match no current advisory and should be deleted: ${review.stale.map(e => `${e.package} ${e.ghsa}`).join(', ')}`)
117 if (problems.length)
118 throw new Error(`Dependency advisory policy failed.\n- ${problems.join('\n- ')}`)
119 return review
120}
121
122if (import.meta.main) {
123 const { retainedAdvisories } = await import('./retained-advisories')
124 const audit = Bun.spawnSync(['bun', 'audit', '--json'], { stdout: 'pipe', stderr: 'pipe' })
125 const text = audit.stdout.toString().trim()
126 // No advisories at all is a valid, empty report rather than a scanner failure.
127 const report: unknown = text.length ? JSON.parse(text) : {}
128 const today = new Date().toISOString().slice(0, 10)
129 const review = assertAdvisoryPolicy(report, retainedAdvisories, today)
130 console.log(`Advisory policy satisfied: ${review.retained.length} reviewed and unreachable, 0 unreviewed, 0 expired`)
131}
scripts/retained-advisories.tsadded+84-0
Changes to scripts/retained-advisories.ts
@@ -0,0 +1,84 @@
1/**
2 * Advisories this repository knowingly ships with, and why each is unreachable.
3 *
4 * An entry here is a review, not a mute. `scripts/advisory-policy.ts` fails the
5 * build when one expires, when it stops matching a real advisory, and when an
6 * advisory appears that nobody has reviewed — so the list cannot quietly become
7 * the reason a scan is green.
8 *
9 * What may be retained: an advisory in a package that reaches no published
10 * artifact. What may not: anything high or critical, which the policy refuses
11 * regardless of what is written here.
12 *
13 * Adding an entry means answering one question in `reason`: by what path would
14 * an installed Craft package reach this code? If the answer is "it would", the
15 * advisory gets fixed instead.
16 */
17
18export type RetainedAdvisory = {
19 /** Bun's numeric advisory id, which is what the report is keyed by. */
20 id: number
21 /** The package the advisory is against, as the report names it. */
22 package: string
23 /** The GHSA identifier, for a human following this up. */
24 ghsa: string
25 /** Why an installed Craft package cannot reach it. */
26 reason: string
27 /** When this was last reviewed, ISO date. */
28 reviewed: string
29 /** When the reasoning must be re-examined, ISO date. Past this, the build fails. */
30 expires: string
31}
32
33/**
34 * `esbuild` arrives under `tsup`, which builds `@craft-native/react` and is a
35 * devDependency of that workspace. No published tarball contains esbuild, and
36 * the advisory is a development-server file read on Windows — a machine and a
37 * process that no consumer of a Craft package runs.
38 *
39 * Not fixed rather than fixed, deliberately: the only in-range remedy bun
40 * offers is a *downgrade* to 0.27.2, which falls below the advisory window
41 * instead of rising above it. Moving the build toolchain backwards to satisfy a
42 * scanner is a worse position than this entry. A `tsup` release that resolves
43 * esbuild >= 0.28.1 removes the finding properly, and buddy-bot will carry it.
44 */
45const esbuildDevServer: RetainedAdvisory[] = [
46 {
47 id: 1120680,
48 package: 'esbuild',
49 ghsa: 'GHSA-g7r4-m6w7-qqqr',
50 reason: 'Dev-only under tsup, in no published tarball; the advisory needs a Windows dev server, and the only in-range fix is a downgrade to 0.27.2.',
51 reviewed: '2026-09-25',
52 expires: '2026-12-31',
53 },
54]
55
56/**
57 * Svelte is a peer and dev dependency of `@craft-native/svelte`, whose
58 * published `files` are `dist` and `src`. The framework itself is never
59 * bundled, so an installed Craft package carries no Svelte code; the version a
60 * consumer runs is the one they chose, and these advisories are theirs to
61 * resolve against their own Svelte.
62 *
63 * Every one is an SSR or DOM-clobbering issue in Svelte's own renderer, which
64 * the binding does not call: it wraps the Craft bridge, not Svelte's compiler
65 * or its server renderer.
66 *
67 * Kept rather than remedied because there is nothing to move to inside the
68 * supported range. The advisories reach up to 5.55.6, so the first unaffected
69 * version is 5.55.7 — outside the declared peer range `^3.0.0 || ^4.0.0`
70 * entirely. Closing them means widening that range to Svelte 5, which breaks
71 * every consumer on 3.x or 4.x. That is an API decision about what the binding
72 * supports, and it should be made for its own reasons rather than to satisfy a
73 * scanner about code Craft does not ship.
74 */
75const svelteRenderer: RetainedAdvisory[] = [
76 { id: 1113416, package: 'svelte', ghsa: 'GHSA-crpf-4hrx-3jrp', reason: 'SSR attribute spreading; Svelte is a peer dependency and is bundled in no published artifact.', reviewed: '2026-09-25', expires: '2026-12-31' },
77 { id: 1113418, package: 'svelte', ghsa: 'GHSA-m56q-vw4c-c2cp', reason: 'SSR dynamic element tag names; peer dependency only, and the binding never invokes Svelte SSR.', reviewed: '2026-09-25', expires: '2026-12-31' },
78 { id: 1113419, package: 'svelte', ghsa: 'GHSA-f7gr-6p89-r883', reason: 'XSS via SSR spread attributes; peer dependency only, not bundled.', reviewed: '2026-09-25', expires: '2026-12-31' },
79 { id: 1120446, package: 'svelte', ghsa: 'GHSA-rcqx-6q8c-2c42', reason: 'DOM clobbering of Svelte internal state; peer dependency only, and no patched version exists inside the supported range.', reviewed: '2026-09-25', expires: '2026-12-31' },
80 { id: 1120449, package: 'svelte', ghsa: 'GHSA-pr6f-5x2q-rwfp', reason: 'XSS via SSR spread attributes; peer dependency only, and no patched version exists inside the supported range.', reviewed: '2026-09-25', expires: '2026-12-31' },
81 { id: 1114402, package: 'svelte', ghsa: 'GHSA-phwv-c562-gvmh', reason: 'XSS during SSR with contenteditable bindings; peer dependency only, and the binding exposes no such binding.', reviewed: '2026-09-25', expires: '2026-12-31' },
82]
83
84export const retainedAdvisories: RetainedAdvisory[] = [...esbuildDevServer, ...svelteRenderer]