Skip to content

Commit a607bdf

Browse files
committed
ci(audit): fail on an accepted advisory that is no longer reported
`bun audit --ignore <id>` accepts ids it never saw without complaining, so an inline ignore list keeps suppressing an advisory long after the dependency was patched and nothing reports that the entry is dead. Three of the five ignored advisories were in that state: `form-data`, `path-to-regexp`, and `undici` each had a patched release inside the range their parents already declare, so re-resolving those three lock entries clears them and the ignores go away. Accepted advisories now live in `.github/audit-allowlist.json`, one entry per directory with a written reason, and the wrapper fails both on an advisory that is not accepted and on an acceptance that no longer matches anything. The threshold is per directory because `vendor` is bundled and shipped, so a moderate advisory there reaches customers, while `--audit-level=high` had been filtering it out.
1 parent 98de8b7 commit a607bdf

6 files changed

Lines changed: 349 additions & 25 deletions

File tree

.github/audit-allowlist.json

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
{
2+
"$comment": [
3+
"Advisories that `node scripts/audit.js` is allowed to see without failing, per install directory.",
4+
"Every entry needs a reason naming why the advisory cannot simply be fixed by resolving a patched version.",
5+
"The script fails when an entry here is no longer reported, so a fixed advisory cannot silently rot in this file.",
6+
"`level` is the lowest severity that fails for that directory. Directories whose contents ship to customers are",
7+
"audited below `high`, because a moderate advisory in shipped code still reaches users."
8+
],
9+
"directories": {
10+
".": {
11+
"level": "high",
12+
"allow": [
13+
{
14+
"id": "GHSA-5c6j-r48x-rmvq",
15+
"package": "serialize-javascript",
16+
"reason": "Patched in 7.0.3, but mocha declares `serialize-javascript@^6.0.2` and cannot resolve a 7.x. Dev-only test runner; the RCE needs an attacker-controlled object to serialize, which only mocha's own parallel-mode test metadata reaches. Drops out when mocha widens the range."
17+
},
18+
{
19+
"id": "GHSA-mh99-v99m-4gvg",
20+
"package": "brace-expansion",
21+
"reason": "The advisory declares `<= 5.0.7` across every major, so the 1.x and 2.x copies that mocha, glob and @eslint/eslintrc pin can never clear it. The root copy is already on a patched 5.x. Dev-only, and the DoS needs a hostile glob pattern, which only this repository's own scripts supply."
22+
}
23+
]
24+
},
25+
"vendor": {
26+
"level": "moderate",
27+
"allow": [
28+
{
29+
"id": "GHSA-8988-4f7v-96qf",
30+
"package": "@opentelemetry/core",
31+
"reason": "Patched in 2.8.0, which is outside the `>=1.14.0 <1.31.0` range vendor/package.json pins for OpenTelemetry compatibility, and `@opentelemetry/resources@1.30.1` pins core to an exact 1.30.1. Clearing it needs a deliberate OpenTelemetry major upgrade of the vendored tree, not a lockfile bump. This code is bundled and shipped, so it is audited at moderate rather than filtered out by a high-only threshold."
32+
}
33+
]
34+
},
35+
"docs": {
36+
"level": "high",
37+
"allow": []
38+
},
39+
".github/all-green": {
40+
"level": "high",
41+
"allow": []
42+
},
43+
".github/actions/datadog-ci": {
44+
"level": "high",
45+
"allow": []
46+
}
47+
}
48+
}

.github/workflows/audit.yml

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,10 @@ jobs:
3434
steps:
3535
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
3636
- uses: ./.github/actions/node/latest
37-
# Root dev tooling reaches these through @actions/*, Axios, Mocha, and Express.
38-
# Keep them explicit so any newly introduced high or critical advisory still fails.
39-
# GHSA-mh99-v99m-4gvg declares `<= 5.0.7` across every brace-expansion major, so the 1.x
40-
# and 2.x copies Mocha, glob, and @eslint/eslintrc pin can never clear it; 5.x is patched.
41-
- run: |
42-
bun audit --audit-level=high \
43-
--ignore GHSA-vxpw-j846-p89q \
44-
--ignore GHSA-hmw2-7cc7-3qxx \
45-
--ignore GHSA-5c6j-r48x-rmvq \
46-
--ignore GHSA-j3q9-mxjg-w52f \
47-
--ignore GHSA-mh99-v99m-4gvg
48-
working-directory: ${{ matrix.directory }}
37+
# Severity threshold and accepted advisories both live in .github/audit-allowlist.json, per directory, each with
38+
# a written reason. The wrapper also fails once an accepted advisory stops being reported, which a bare
39+
# `--ignore` list cannot do: `bun audit` takes unknown ids without complaint, so a suppression silently outlives
40+
# the advisory it was added for.
41+
- run: node scripts/audit.js "$DIRECTORY"
42+
env:
43+
DIRECTORY: ${{ matrix.directory }}

bun.lock

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

scripts/audit.js

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
'use strict'
2+
3+
/* eslint-disable no-console */
4+
5+
// Wraps `bun audit` so an accepted advisory has to carry a written reason, and so an acceptance that is no longer
6+
// needed fails instead of lingering. `bun audit --ignore <id>` accepts ids it never saw without complaining, so an
7+
// inline ignore list keeps suppressing an advisory long after the dependency was patched and nothing ever reports that
8+
// the entry is dead. Severity is resolved per directory because a moderate advisory in `vendor` is bundled and reaches
9+
// customers, while the same severity in the dev-only trees does not.
10+
//
11+
// node scripts/audit.js [--allowlist <path>] [<directory> ...]
12+
//
13+
// Directories resolve against the working directory and default to every directory the allowlist names.
14+
15+
const { spawnSync } = require('node:child_process')
16+
const { readFileSync } = require('node:fs')
17+
const path = require('node:path')
18+
19+
const { getBunBinary } = require('./bun')
20+
21+
const SEVERITIES = ['info', 'low', 'moderate', 'high', 'critical']
22+
const DEFAULT_ALLOWLIST = path.join(__dirname, '..', '.github', 'audit-allowlist.json')
23+
24+
/**
25+
* @param {string[]} argv
26+
* @returns {{ allowlistPath: string, directories: string[] }}
27+
*/
28+
function parseArguments (argv) {
29+
let allowlistPath = DEFAULT_ALLOWLIST
30+
const directories = []
31+
for (let i = 0; i < argv.length; i++) {
32+
if (argv[i] === '--allowlist') {
33+
allowlistPath = path.resolve(argv[++i])
34+
} else {
35+
directories.push(argv[i])
36+
}
37+
}
38+
return { allowlistPath, directories }
39+
}
40+
41+
/**
42+
* @param {string} url
43+
* @returns {string} The GHSA identifier the advisory URL ends with.
44+
*/
45+
function ghsaId (url) {
46+
return String(url).split('/').pop()
47+
}
48+
49+
/**
50+
* @param {string} directory
51+
* @returns {Map<string, { id: string, package: string, severity: string, title: string }>}
52+
*/
53+
function runAudit (directory) {
54+
const result = spawnSync(getBunBinary(), ['audit', '--json'], {
55+
cwd: path.resolve(directory),
56+
encoding: 'utf8',
57+
// `bun audit` exits non-zero whenever it finds anything, so the status alone cannot tell "advisories present" from
58+
// "the command failed"; parsing the report is what decides.
59+
stdio: ['ignore', 'pipe', 'pipe'],
60+
})
61+
62+
if (!result.stdout?.trim()) {
63+
// A clean tree still prints an empty object, so missing output means bun itself failed.
64+
throw new Error(`\`bun audit\` produced no output in '${directory}':\n${result.stderr ?? ''}`)
65+
}
66+
67+
let report
68+
try {
69+
report = JSON.parse(result.stdout)
70+
} catch {
71+
throw new Error(`Could not parse \`bun audit --json\` output in '${directory}':\n${result.stdout}`)
72+
}
73+
74+
const advisories = new Map()
75+
for (const [packageName, entries] of Object.entries(report)) {
76+
for (const advisory of [entries].flat()) {
77+
// Without a URL there is no GHSA id to match an acceptance against, and a synthesized one would silently never
78+
// match, so surface it instead.
79+
if (!advisory?.url) throw new Error(`Advisory for ${packageName} in '${directory}' has no URL to identify it.`)
80+
const id = ghsaId(advisory.url)
81+
advisories.set(id, { id, package: packageName, severity: advisory.severity, title: advisory.title })
82+
}
83+
}
84+
return advisories
85+
}
86+
87+
/**
88+
* @param {string} directory
89+
* @param {{ level: string, allow: Array<{ id: string, package: string, reason: string }> }} config
90+
* @param {string} allowlistLabel
91+
* @returns {string[]} One description per failure, empty when the directory is clean.
92+
*/
93+
function auditDirectory (directory, config, allowlistLabel) {
94+
const threshold = SEVERITIES.indexOf(config.level)
95+
if (threshold === -1) throw new Error(`Unknown level '${config.level}' for '${directory}'`)
96+
97+
const allowed = new Map()
98+
for (const entry of config.allow) {
99+
if (!entry.reason?.trim()) throw new Error(`Allowlist entry ${entry.id} for '${directory}' needs a reason.`)
100+
allowed.set(entry.id, entry)
101+
}
102+
103+
const advisories = runAudit(directory)
104+
const problems = []
105+
106+
for (const advisory of advisories.values()) {
107+
if (allowed.has(advisory.id) || SEVERITIES.indexOf(advisory.severity) < threshold) continue
108+
problems.push(
109+
`${directory}: unaccepted ${advisory.severity} advisory ${advisory.id} in ${advisory.package} ` +
110+
`(${advisory.title}). Resolve a patched version, or add it to ${allowlistLabel} with a reason why it cannot ` +
111+
'be resolved.'
112+
)
113+
}
114+
115+
for (const entry of allowed.values()) {
116+
if (advisories.has(entry.id)) continue
117+
problems.push(
118+
`${directory}: accepted advisory ${entry.id} (${entry.package}) is no longer reported. ` +
119+
`Remove it from ${allowlistLabel}.`
120+
)
121+
}
122+
123+
return problems
124+
}
125+
126+
const { allowlistPath, directories: requested } = parseArguments(process.argv.slice(2))
127+
const allowlistLabel = path.relative(process.cwd(), allowlistPath) || allowlistPath
128+
const { directories } = JSON.parse(readFileSync(allowlistPath, 'utf8'))
129+
const selected = requested.length > 0 ? requested : Object.keys(directories)
130+
const problems = []
131+
132+
for (const directory of selected) {
133+
const config = directories[directory]
134+
if (!config) throw new Error(`'${directory}' has no entry in ${allowlistLabel}.`)
135+
problems.push(...auditDirectory(directory, config, allowlistLabel))
136+
}
137+
138+
if (problems.length > 0) {
139+
for (const problem of problems) console.error(problem)
140+
process.exit(1)
141+
}
142+
143+
console.log(`No unaccepted advisories in: ${selected.join(', ')}`)

scripts/test/audit.spec.js

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
'use strict'
2+
3+
const assert = require('node:assert/strict')
4+
const { spawnSync } = require('node:child_process')
5+
const fs = require('node:fs')
6+
const { tmpdir } = require('node:os')
7+
const path = require('node:path')
8+
9+
const repoRoot = path.join(__dirname, '..', '..')
10+
const auditScript = path.join(repoRoot, 'scripts', 'audit.js')
11+
const bunVersion = require('../../package.json').devDependencies.bun
12+
13+
/**
14+
* @param {string} id
15+
* @param {string} severity
16+
*/
17+
function advisory (id, severity) {
18+
return { id: 1, url: `https://github.com/advisories/${id}`, severity, title: `${id} title` }
19+
}
20+
21+
describe('scripts/audit.js', () => {
22+
let fixtureDirectory
23+
24+
beforeEach(() => {
25+
fixtureDirectory = fs.mkdtempSync(path.join(tmpdir(), 'dd-trace-audit-'))
26+
})
27+
28+
afterEach(() => {
29+
fs.rmSync(fixtureDirectory, { recursive: true, force: true })
30+
})
31+
32+
it('passes when every advisory at or above the threshold is accepted', () => {
33+
const result = runAudit({
34+
report: {
35+
'some-package': [advisory('GHSA-aaaa-aaaa-aaaa', 'high')],
36+
'noisy-package': [advisory('GHSA-cccc-cccc-cccc', 'low')],
37+
},
38+
allow: [{ id: 'GHSA-aaaa-aaaa-aaaa', package: 'some-package', reason: 'upstream ships no patched release' }],
39+
})
40+
41+
assert.strictEqual(result.status, 0, result.stderr)
42+
})
43+
44+
it('fails on an advisory at the threshold that is not accepted', () => {
45+
const result = runAudit({
46+
report: { 'some-package': [advisory('GHSA-bbbb-bbbb-bbbb', 'high')] },
47+
allow: [],
48+
})
49+
50+
assert.strictEqual(result.status, 1)
51+
assert.match(result.stderr, /unaccepted high advisory GHSA-bbbb-bbbb-bbbb in some-package/)
52+
})
53+
54+
it('ignores an advisory below the directory threshold', () => {
55+
const result = runAudit({
56+
report: { 'some-package': [advisory('GHSA-cccc-cccc-cccc', 'moderate')] },
57+
allow: [],
58+
})
59+
60+
assert.strictEqual(result.status, 0, result.stderr)
61+
})
62+
63+
it('fails on an accepted advisory that is no longer reported', () => {
64+
// The reason this wrapper exists: `bun audit --ignore <id>` takes ids it never saw without complaining, so a
65+
// suppression keeps hiding an advisory long after the dependency was patched and nothing reports the entry is dead.
66+
const result = runAudit({
67+
report: {},
68+
allow: [{ id: 'GHSA-dddd-dddd-dddd', package: 'gone', reason: 'was unpatched when this was added' }],
69+
})
70+
71+
assert.strictEqual(result.status, 1)
72+
assert.match(result.stderr, /GHSA-dddd-dddd-dddd \(gone\) is no longer reported/)
73+
})
74+
75+
it('fails when an accepted advisory carries no reason', () => {
76+
const result = runAudit({
77+
report: { 'some-package': [advisory('GHSA-eeee-eeee-eeee', 'high')] },
78+
allow: [{ id: 'GHSA-eeee-eeee-eeee', package: 'some-package', reason: ' ' }],
79+
})
80+
81+
assert.strictEqual(result.status, 1)
82+
assert.match(result.stderr, /needs a reason/)
83+
})
84+
85+
it('fails when an audited directory has no allowlist entry', () => {
86+
const result = runAudit({ report: {}, allow: [], directory: 'unlisted' })
87+
88+
assert.strictEqual(result.status, 1)
89+
assert.match(result.stderr, /has no entry in/)
90+
})
91+
92+
it('fails when bun produces no output rather than reporting a clean tree', () => {
93+
const result = runAudit({ report: {}, allow: [], bunOutput: '' })
94+
95+
assert.strictEqual(result.status, 1)
96+
assert.match(result.stderr, /produced no output/)
97+
})
98+
99+
/**
100+
* Runs the real script against a stub `bun` so the assertions cover the wrapper's decisions rather than whatever the
101+
* repository's own lockfiles happen to report today.
102+
*
103+
* @param {{ report: object, allow: object[], directory?: string, bunOutput?: string }} options
104+
* @returns {import('node:child_process').SpawnSyncReturns<string>}
105+
*/
106+
function runAudit ({ report, allow, directory = 'audited', bunOutput }) {
107+
const allowlistPath = path.join(fixtureDirectory, 'audit-allowlist.json')
108+
fs.writeFileSync(allowlistPath, JSON.stringify({ directories: { audited: { level: 'high', allow } } }))
109+
fs.mkdirSync(path.join(fixtureDirectory, 'audited'), { recursive: true })
110+
111+
// `getBunBinary()` accepts the `bun` on PATH only when it reports the pinned version, so the stub answers
112+
// `--version` as well as `audit --json`.
113+
const stubDirectory = path.join(fixtureDirectory, 'bin')
114+
fs.mkdirSync(stubDirectory, { recursive: true })
115+
const stub = path.join(stubDirectory, 'bun')
116+
fs.writeFileSync(stub, [
117+
'#!/bin/sh',
118+
'if [ "$1" = "--version" ]; then',
119+
` echo ${bunVersion}`,
120+
' exit 0',
121+
'fi',
122+
"cat <<'REPORT'",
123+
bunOutput ?? JSON.stringify(report),
124+
'REPORT',
125+
].join('\n'))
126+
fs.chmodSync(stub, 0o755)
127+
128+
return spawnSync(process.execPath, [auditScript, '--allowlist', allowlistPath, directory], {
129+
cwd: fixtureDirectory,
130+
encoding: 'utf8',
131+
env: { ...process.env, PATH: `${stubDirectory}:${process.env.PATH}` },
132+
})
133+
}
134+
})

0 commit comments

Comments
 (0)