diff --git a/package-lock.json b/package-lock.json index 06f2f3ca..1c95f46f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1096,7 +1096,7 @@ "typebox": "1.1.38" }, "bin": { - "pi-ai": "./dist/cli.js" + "pi-ai": "dist/cli.js" }, "engines": { "node": ">=22.19.0" @@ -1354,7 +1354,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/api/-/api-1.9.0.tgz", "integrity": "sha512-3giAOQvZiH5F9bMlMiv8+GSPMeqg0dbaeo58/0SlA9sxSqZhnUtxzX9/2FzyhS9sWQf5S0GJE0AKBrFqjpeYcg==", "license": "Apache-2.0", - "peer": true, "engines": { "node": ">=8.0.0" } @@ -2399,7 +2398,6 @@ "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz", "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==", "license": "MIT", - "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } @@ -3165,7 +3163,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/api/-/api-1.9.0.tgz", "integrity": "sha512-3giAOQvZiH5F9bMlMiv8+GSPMeqg0dbaeo58/0SlA9sxSqZhnUtxzX9/2FzyhS9sWQf5S0GJE0AKBrFqjpeYcg==", "license": "Apache-2.0", - "peer": true, "engines": { "node": ">=8.0.0" } @@ -3390,7 +3387,6 @@ "integrity": "sha512-xRQbDb9BnwDafYNn6Vwl839DYVjqXYb1XVGtWAZ1kcDc6iwAL4hg3B1dZlRiuENFeO2H53gFG3in621AdERVAg==", "dev": true, "license": "MIT", - "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -3499,11 +3495,14 @@ } }, "node_modules/balanced-match": { - "version": "1.0.2", - "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz", - "integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==", + "version": "4.0.4", + "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-4.0.4.tgz", + "integrity": "sha512-BLrgEcRTwX2o6gGxGOCNyMvGSp35YofuYzw9h1IMTRmKqttAZZVU67bdb9Pr2vUHA8+j3i2tJfjO6C6+4myGTA==", "dev": true, - "license": "MIT" + "license": "MIT", + "engines": { + "node": "18 || 20 || >=22" + } }, "node_modules/base64-js": { "version": "1.5.1", @@ -3568,14 +3567,16 @@ "license": "MIT" }, "node_modules/brace-expansion": { - "version": "1.1.16", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.16.tgz", - "integrity": "sha512-IDw48K2/2kRkg9LdJxurvq3lV3aBgq0REY89duEqFRthjlPdXHKMj7EnQOXVckxzgisinf3nHfrcE2FufFLXMw==", + "version": "5.0.8", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.8.tgz", + "integrity": "sha512-JZyDyq3D4AUifKTPOB7DELf6XsB3WdPuNxCtob1vFXPsSXhdAiHBWJ/tJ8HAc9aH84BK+5JFZLNkJKx3G9kzQg==", "dev": true, "license": "MIT", "dependencies": { - "balanced-match": "^1.0.0", - "concat-map": "0.0.1" + "balanced-match": "^4.0.2" + }, + "engines": { + "node": "20 || >=22" } }, "node_modules/braces": { @@ -3764,13 +3765,6 @@ "node": ">=18" } }, - "node_modules/concat-map": { - "version": "0.0.1", - "resolved": "https://registry.npmjs.org/concat-map/-/concat-map-0.0.1.tgz", - "integrity": "sha512-/Srv4dswyQNBfohGpz9o6Yb3Gz3SrUDqBH5rTuhGR7ahtlbYKnVxw2bCFMRljaA7EXHaXZ8wsHdodFvbkhKmqg==", - "dev": true, - "license": "MIT" - }, "node_modules/core-util-is": { "version": "1.0.3", "resolved": "https://registry.npmjs.org/core-util-is/-/core-util-is-1.0.3.tgz", @@ -3980,7 +3974,6 @@ "integrity": "sha512-DgZS62aPLXKlnxILS/AYCoRvHaZeXceIzlXPkkGGzJWSow1aEk0lbTlxUSlyjC8jcaKxAdOnTDz+o1JFSBsyjw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.1", @@ -6487,7 +6480,6 @@ "resolved": "https://registry.npmjs.org/zod/-/zod-4.4.3.tgz", "integrity": "sha512-ytENFjIJFl2UwYglde2jchW2Hwm4GJFLDiSXWdTrJQBIN9Fcyp7n4DhxJEiWNAJMV1/BqWfW/kkg71UDcHJyTQ==", "license": "MIT", - "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } diff --git a/package.json b/package.json index ead7b8ab..042dc2c8 100644 --- a/package.json +++ b/package.json @@ -114,6 +114,7 @@ "overrides": { "esbuild": ">=0.28.1", "protobufjs": ">=7.6.4 <8", - "ws": ">=8.21.0" + "ws": ">=8.21.0", + "brace-expansion": "5.0.8" } } diff --git a/scripts/security-audit.mjs b/scripts/security-audit.mjs index 9b6bc6a5..60503fb4 100644 --- a/scripts/security-audit.mjs +++ b/scripts/security-audit.mjs @@ -13,13 +13,14 @@ // fails the build. Tolerated advisories are printed so they stay visible. import { execFileSync } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; // Packages whose own subtree we cannot patch downstream: pi-coding-agent ships // an npm-shrinkwrap.json, which npm honors absolutely — root `overrides` are // ignored inside it exactly like a bundled tarball (verified empirically: a // fresh resolve with a matching override still installs the shrinkwrapped // version). Fixes there must come from an upstream pi release. -const BUNDLED_ROOTS = [ +export const BUNDLED_ROOTS = [ 'node_modules/@earendil-works/pi-coding-agent/node_modules/', ]; @@ -28,22 +29,29 @@ const BUNDLED_ROOTS = [ // per-advisory allowlist is deliberate: a brand-new high/critical for the // same package — let alone any other package — still fails the gate, forcing // a conscious decision rather than silent acceptance. -// - brace-expansion GHSA-3jxr-9vmj-r5cp (DoS): pinned 5.0.6 by pi's -// shrinkwrap; every non-shrinkwrapped path in our tree is on the fixed line. +// - brace-expansion GHSA-3jxr-9vmj-r5cp + GHSA-mh99-v99m-4gvg (both DoS): +// pinned 5.0.6 by pi's shrinkwrap, both advisories confined to that one +// bundled node (verified: the root `overrides` entry below forces every +// non-shrinkwrapped path in our tree onto the patched 5.0.8 line, which +// fixes both GHSAs — 5.0.8 itself declares `engines.node: "20 || >=22"`, +// dropping Node 18, but it's a devDependency-only lint/audit-tooling path +// with no `engine-strict` set, so npm installs it without error on the +// Node 18 CI lane; this is a deliberate, documented trade-off since no +// brace-expansion release fixes both GHSAs while keeping Node 18 support). // - protobufjs / ws: previously tolerated by name while their advisories were // live in the bundled subtree; currently dormant, so no ids are listed — if // one refires, the gate fails loudly and the specific GHSA gets re-added. -const TOLERATED_BUNDLED_ADVISORIES = new Map([ - ['brace-expansion', new Set(['GHSA-3jxr-9vmj-r5cp'])], +export const TOLERATED_BUNDLED_ADVISORIES = new Map([ + ['brace-expansion', new Set(['GHSA-3jxr-9vmj-r5cp', 'GHSA-mh99-v99m-4gvg'])], ]); -function advisoryIds(info) { +export function advisoryIds(info) { return (info.via ?? []) .filter((entry) => typeof entry === 'object' && entry?.url) .map((entry) => String(entry.url).split('/').pop()); } -const BLOCKING = new Set(['high', 'critical']); +export const BLOCKING = new Set(['high', 'critical']); function runAudit() { try { @@ -60,43 +68,61 @@ function runAudit() { } } -function isFullyBundled(nodes) { +export function isFullyBundled(nodes, bundledRoots = BUNDLED_ROOTS) { if (!nodes || !nodes.length) return false; - return nodes.every((path) => BUNDLED_ROOTS.some((root) => path.includes(root))); + return nodes.every((path) => bundledRoots.some((root) => path.includes(root))); } -const report = runAudit(); -const vulns = report.vulnerabilities || {}; -const blocking = []; -const tolerated = []; +// Pure decision logic (no subprocess, no process.exit) — the part every +// tolerate/block call actually has to get right. Kept separate from +// runAudit()/main() so it can be unit-tested against a synthetic `npm audit +// --json` shape without shelling out. +export function evaluateAdvisories( + vulnerabilities, + { toleratedBundledAdvisories = TOLERATED_BUNDLED_ADVISORIES, bundledRoots = BUNDLED_ROOTS } = {}, +) { + const blocking = []; + const tolerated = []; -for (const [name, info] of Object.entries(vulns)) { - if (!BLOCKING.has(info.severity)) continue; - const allowedIds = TOLERATED_BUNDLED_ADVISORIES.get(name); - const ids = advisoryIds(info); - const everyIdTolerated = Boolean(allowedIds) - && ids.length > 0 - && ids.every((id) => allowedIds.has(id)); - if (everyIdTolerated && isFullyBundled(info.nodes)) { - tolerated.push({ name, severity: info.severity, ids }); - } else { - blocking.push({ name, severity: info.severity, nodes: info.nodes }); + for (const [name, info] of Object.entries(vulnerabilities || {})) { + if (!BLOCKING.has(info.severity)) continue; + const allowedIds = toleratedBundledAdvisories.get(name); + const ids = advisoryIds(info); + const everyIdTolerated = Boolean(allowedIds) + && ids.length > 0 + && ids.every((id) => allowedIds.has(id)); + if (everyIdTolerated && isFullyBundled(info.nodes, bundledRoots)) { + tolerated.push({ name, severity: info.severity, ids }); + } else { + blocking.push({ name, severity: info.severity, nodes: info.nodes }); + } } -} -if (tolerated.length) { - console.log('Tolerated advisories (vendored/bundled, unpatchable downstream — track upstream):'); - for (const t of tolerated) console.log(` - ${t.name} (${t.severity}) [${t.ids.join(', ')}]`); + return { blocking, tolerated }; } -if (blocking.length) { - console.error('\nBlocking high/critical advisories in our dependency tree:'); - for (const b of blocking) { - console.error(` - ${b.name} (${b.severity})`); - for (const n of b.nodes || []) console.error(` ${n}`); +function main() { + const report = runAudit(); + const { blocking, tolerated } = evaluateAdvisories(report.vulnerabilities || {}); + + if (tolerated.length) { + console.log('Tolerated advisories (vendored/bundled, unpatchable downstream — track upstream):'); + for (const t of tolerated) console.log(` - ${t.name} (${t.severity}) [${t.ids.join(', ')}]`); + } + + if (blocking.length) { + console.error('\nBlocking high/critical advisories in our dependency tree:'); + for (const b of blocking) { + console.error(` - ${b.name} (${b.severity})`); + for (const n of b.nodes || []) console.error(` ${n}`); + } + console.error('\nRun `npm audit` for details, then bump or override the offending package.'); + process.exit(1); } - console.error('\nRun `npm audit` for details, then bump or override the offending package.'); - process.exit(1); + + console.log(`\nSecurity baseline OK — ${tolerated.length} tolerated bundled advisory(ies), 0 blocking.`); } -console.log(`\nSecurity baseline OK — ${tolerated.length} tolerated bundled advisory(ies), 0 blocking.`); +if (process.argv[1] === fileURLToPath(import.meta.url)) { + main(); +} diff --git a/src/notifications/channels/discord.js b/src/notifications/channels/discord.js index 5d875ddc..847c1daf 100644 --- a/src/notifications/channels/discord.js +++ b/src/notifications/channels/discord.js @@ -21,7 +21,8 @@ export function convertSlackToDiscord(slackPayload) { if (block.type === 'section' && block.text) { descriptionLines.push(block.text.text); } - if (block.type === 'fields' && block.fields) { + // Fields ride on a section block; 'fields' is tolerated for older payloads. + if ((block.type === 'section' || block.type === 'fields') && block.fields) { for (const f of block.fields) { const raw = f.text || ''; const parts = raw.split('\n'); diff --git a/src/notifications/channels/teams.js b/src/notifications/channels/teams.js index 28b86a5e..0b6bc4b9 100644 --- a/src/notifications/channels/teams.js +++ b/src/notifications/channels/teams.js @@ -24,7 +24,8 @@ export function convertSlackToTeams(slackPayload) { facts: [] }); } - if (block.type === 'fields' && block.fields) { + // Fields ride on a section block; 'fields' is tolerated for older payloads. + if ((block.type === 'section' || block.type === 'fields') && block.fields) { const currentSection = sections[sections.length - 1] || { facts: [] }; if (!sections.includes(currentSection)) { sections.push(currentSection); diff --git a/src/notifications/channels/text.js b/src/notifications/channels/text.js index adad2663..2814c08f 100644 --- a/src/notifications/channels/text.js +++ b/src/notifications/channels/text.js @@ -23,7 +23,8 @@ export function slackPayloadToText(slackPayload) { if (block.type === 'section' && block.text?.text) { lines.push(stripMrkdwn(block.text.text)); } - if (block.type === 'fields' && Array.isArray(block.fields)) { + // Fields ride on a section block; 'fields' is tolerated for older payloads. + if ((block.type === 'section' || block.type === 'fields') && Array.isArray(block.fields)) { for (const field of block.fields) { const parts = String(field.text ?? '').split('\n'); const name = stripMrkdwn(parts[0]); diff --git a/src/notifications/index.js b/src/notifications/index.js index 14e3a2ea..1c20acef 100644 --- a/src/notifications/index.js +++ b/src/notifications/index.js @@ -89,7 +89,8 @@ export function formatSlackStageMessage(runId, stageId, status, details = {}) { }, }, { - type: 'fields', + // Block Kit has no 'fields' block type — fields live on a section block. + type: 'section', fields, }, ]; @@ -146,7 +147,8 @@ export function formatSlackTaskReportMessage(runId, taskId, trace) { }, }, { - type: 'fields', + // Block Kit has no 'fields' block type — fields live on a section block. + type: 'section', fields: [ { type: 'mrkdwn', text: `*Run ID:*\n${runId}` }, { type: 'mrkdwn', text: `*Task:*\n${taskId}` }, diff --git a/tests/notifications-channels.test.js b/tests/notifications-channels.test.js index 5ffc63a9..4282eab1 100644 --- a/tests/notifications-channels.test.js +++ b/tests/notifications-channels.test.js @@ -41,6 +41,30 @@ test('discord and teams converters keep their original behavior', () => { assert.ok(teams.sections[0].facts.some((fact) => fact.name === 'Run ID:')); }); +test('legacy block.type === "fields" payloads are still converted (backward compatibility)', () => { + const legacyPayload = { + attachments: [{ + color: '#22c55e', + blocks: [ + { type: 'section', text: { type: 'mrkdwn', text: 'Legacy stage update' } }, + { type: 'fields', fields: [{ text: '*Run ID:*\nrun-legacy-001' }] }, + ], + }], + }; + + const text = slackPayloadToText(legacyPayload); + assert.ok(text.includes('Legacy stage update')); + assert.ok(text.includes('Run ID: run-legacy-001'), 'legacy fields block flattened into text'); + + const discord = convertSlackToDiscord(legacyPayload); + assert.equal(discord.embeds[0].fields.length, 1); + assert.equal(discord.embeds[0].fields[0].name, 'Run ID:'); + assert.equal(discord.embeds[0].fields[0].value, 'run-legacy-001'); + + const teams = convertSlackToTeams(legacyPayload); + assert.ok(teams.sections[0].facts.some((fact) => fact.name === 'Run ID:' && fact.value === 'run-legacy-001')); +}); + test('resolveChannels: env vars, URL sniffing, and file config precedence', () => { const root = mkdtempSync(join(tmpdir(), 'rstack-notify-')); mkdirSync(join(root, '.rstack'), { recursive: true }); diff --git a/tests/security-audit.test.js b/tests/security-audit.test.js new file mode 100644 index 00000000..ee8ec61f --- /dev/null +++ b/tests/security-audit.test.js @@ -0,0 +1,112 @@ +/** + * Security baseline gate (#472, Qodo-flagged): scripts/security-audit.mjs + * decides whether an `npm audit` high/critical advisory is tolerated + * (fully confined to a bundled/shrinkwrapped subtree we can't patch) or + * blocks CI. That decision — not the `npm audit` subprocess call — is what + * TOLERATED_BUNDLED_ADVISORIES changes actually affect, so it's exercised + * here directly against synthetic `npm audit --json` shapes rather than + * shelling out. + * + * owner: RStack developed by Richardson Gunde + */ + +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { evaluateAdvisories, isFullyBundled, advisoryIds } from '../scripts/security-audit.mjs'; + +const BUNDLED_ROOT = 'node_modules/@earendil-works/pi-coding-agent/node_modules/'; + +function adv({ severity = 'high', ids = [], nodes = [] } = {}) { + return { + severity, + nodes, + via: ids.map((id) => ({ url: `https://github.com/advisories/${id}` })), + }; +} + +test('tolerates an advisory whose every install path is bundled and whose id is allowlisted', () => { + const vulns = { + 'brace-expansion': adv({ + ids: ['GHSA-3jxr-9vmj-r5cp'], + nodes: [`${BUNDLED_ROOT}brace-expansion`], + }), + }; + const { blocking, tolerated } = evaluateAdvisories(vulns, { + toleratedBundledAdvisories: new Map([['brace-expansion', new Set(['GHSA-3jxr-9vmj-r5cp'])]]), + bundledRoots: [BUNDLED_ROOT], + }); + assert.equal(blocking.length, 0); + assert.equal(tolerated.length, 1); + assert.equal(tolerated[0].name, 'brace-expansion'); +}); + +test('blocks the same advisory when a reachable (non-bundled) install path also carries it', () => { + const vulns = { + 'brace-expansion': adv({ + ids: ['GHSA-3jxr-9vmj-r5cp'], + nodes: [`${BUNDLED_ROOT}brace-expansion`, 'node_modules/brace-expansion'], + }), + }; + const { blocking, tolerated } = evaluateAdvisories(vulns, { + toleratedBundledAdvisories: new Map([['brace-expansion', new Set(['GHSA-3jxr-9vmj-r5cp'])]]), + bundledRoots: [BUNDLED_ROOT], + }); + assert.equal(tolerated.length, 0); + assert.equal(blocking.length, 1); + assert.equal(blocking[0].name, 'brace-expansion'); +}); + +test('blocks a bundled advisory whose id is NOT on the allowlist — a new GHSA forces a conscious decision', () => { + const vulns = { + 'brace-expansion': adv({ + ids: ['GHSA-brand-new-id'], + nodes: [`${BUNDLED_ROOT}brace-expansion`], + }), + }; + const { blocking, tolerated } = evaluateAdvisories(vulns, { + toleratedBundledAdvisories: new Map([['brace-expansion', new Set(['GHSA-3jxr-9vmj-r5cp'])]]), + bundledRoots: [BUNDLED_ROOT], + }); + assert.equal(tolerated.length, 0); + assert.equal(blocking.length, 1); +}); + +test('ignores advisories below the blocking severity threshold', () => { + const vulns = { lodash: adv({ severity: 'moderate', ids: ['GHSA-whatever'], nodes: ['node_modules/lodash'] }) }; + const { blocking, tolerated } = evaluateAdvisories(vulns); + assert.equal(blocking.length, 0); + assert.equal(tolerated.length, 0); +}); + +test('blocks a package with no allowlist entry at all', () => { + const vulns = { minimatch: adv({ ids: ['GHSA-3jxr-9vmj-r5cp'], nodes: ['node_modules/minimatch'] }) }; + const { blocking, tolerated } = evaluateAdvisories(vulns, { + toleratedBundledAdvisories: new Map([['brace-expansion', new Set(['GHSA-3jxr-9vmj-r5cp'])]]), + }); + assert.equal(tolerated.length, 0); + assert.equal(blocking.length, 1); + assert.equal(blocking[0].name, 'minimatch'); +}); + +test('isFullyBundled requires every node to match a bundled root', () => { + assert.equal(isFullyBundled([`${BUNDLED_ROOT}x`], [BUNDLED_ROOT]), true); + assert.equal(isFullyBundled([`${BUNDLED_ROOT}x`, 'node_modules/x'], [BUNDLED_ROOT]), false); + assert.equal(isFullyBundled([], [BUNDLED_ROOT]), false); +}); + +test('advisoryIds extracts the GHSA id from each via entry URL', () => { + const ids = advisoryIds({ via: [{ url: 'https://github.com/advisories/GHSA-abcd-1234-efgh' }, 'some-transitive-package-name'] }); + assert.deepEqual(ids, ['GHSA-abcd-1234-efgh']); +}); + +// The live #472 regression this gate protects: brace-expansion currently +// carries TWO tolerated GHSAs, both confined to pi-coding-agent's +// shrinkwrapped subtree. Pin this shape so a future edit to the allowlist +// (adding/removing an id) is a conscious, reviewed change. +test('the real TOLERATED_BUNDLED_ADVISORIES allowlist matches the documented brace-expansion GHSAs', async () => { + const { TOLERATED_BUNDLED_ADVISORIES } = await import('../scripts/security-audit.mjs'); + assert.deepEqual( + [...TOLERATED_BUNDLED_ADVISORIES.get('brace-expansion')].sort(), + ['GHSA-3jxr-9vmj-r5cp', 'GHSA-mh99-v99m-4gvg'].sort(), + ); +});