From 100a3bb6ba98e95393775fe8711692a6e3cf4351 Mon Sep 17 00:00:00 2001 From: DongHyeonka Date: Sun, 2 Aug 2026 05:40:58 +0900 Subject: [PATCH] fix: make security fixtures fail closed --- config/ci/gates.json | 3 +- scripts/check-security-fixtures.ts | 71 +------- scripts/lib/repository-file-inventory.ts | 20 ++- scripts/lib/secret-scan-policy.ts | 59 ++++++ scripts/lib/security-fixture-check.ts | 170 ++++++++++++++++++ scripts/security-scan.ts | 20 +-- .../secret-detection/nonmatching-policy.json | 12 ++ tests/unit/supply-chain.test.ts | 102 +++++++++++ 8 files changed, 374 insertions(+), 83 deletions(-) create mode 100644 scripts/lib/secret-scan-policy.ts create mode 100644 scripts/lib/security-fixture-check.ts create mode 100644 tests/fixtures/security/secret-detection/nonmatching-policy.json diff --git a/config/ci/gates.json b/config/ci/gates.json index a5401a3..86842f5 100644 --- a/config/ci/gates.json +++ b/config/ci/gates.json @@ -275,13 +275,12 @@ "script": "check:supply-chain:provider-fixtures", "expect": "pass" }, - { "script": "scan:security:fixture", "expect": "fail", "expectedExitCode": 1, "expectedDiagnosticId": "Security scan found" }, + { "script": "check:security:fixtures", "expect": "pass" }, { "script": "check:browser-security", "expect": "pass" } ], "logPath": "artifacts/quality/gates/FE-GATE-013.txt", "evidence": [ "artifacts/security/scan.sarif", - "artifacts/security/scan-fixture.sarif", "artifacts/release/dependency-inventory.json", "artifacts/release/sbom.cdx.json", "artifacts/release/provenance.json", diff --git a/scripts/check-security-fixtures.ts b/scripts/check-security-fixtures.ts index 4bd9ddf..f46417e 100644 --- a/scripts/check-security-fixtures.ts +++ b/scripts/check-security-fixtures.ts @@ -1,65 +1,10 @@ -import { spawnSync } from "node:child_process"; -import { readFile } from "node:fs/promises"; +import { + checkNonmatchingSecurityIncludeFixture, + checkSecurityFixtures, +} from "./lib/security-fixture-check.ts"; -type Document = Record; - -function record(value: unknown, label: string): Document { - if (!value || typeof value !== "object" || Array.isArray(value)) { - throw new TypeError(`${label} must be an object`); - } - return value as Document; -} - -const artifact = "artifacts/security/scan-fixture.sarif"; -const scan = spawnSync( - "node", - [ - "scripts/security-scan.ts", - "--policy", - "tests/fixtures/security/secret-detection/forbidden-policy.json", - "--artifact", - artifact, - ], - { encoding: "utf8" }, +await checkNonmatchingSecurityIncludeFixture(); +await checkSecurityFixtures(); +process.stdout.write( + "Security fixtures: unmatched include rejected; 3 forbidden files detected\n", ); -if (scan.error || scan.signal || scan.status !== 1) { - throw new Error( - `forbidden security fixture did not fail exactly: ${scan.error?.message ?? scan.stderr}`, - ); -} - -const sarif = record( - JSON.parse(await readFile(artifact, "utf8")), - "security fixture SARIF", -); -const runs = Array.isArray(sarif.runs) ? sarif.runs : []; -const run = record(runs[0], "security fixture SARIF run"); -const results = Array.isArray(run.results) ? run.results : []; -const actual = results - .map((rawResult) => { - const result = record(rawResult, "security fixture result"); - const locations = Array.isArray(result.locations) ? result.locations : []; - const location = record(locations[0], "security fixture location"); - const physical = record( - location.physicalLocation, - "security fixture physical location", - ); - const artifactLocation = record( - physical.artifactLocation, - "security fixture artifact location", - ); - return `${String(artifactLocation.uri)}:${String(result.ruleId)}`; - }) - .sort(); -const root = "tests/fixtures/security/secret-detection/forbidden"; -const expected = [ - `${root}/config.json:assigned-secret`, - `${root}/dist.ts:assigned-secret`, - `${root}/source.ts:aws-access-key`, -].sort(); -if (JSON.stringify(actual) !== JSON.stringify(expected)) { - throw new Error( - `security fixture findings mismatch: expected ${expected.join(", ")}; received ${actual.join(", ")}`, - ); -} -process.stdout.write("Security fixtures: 3 forbidden files detected\n"); diff --git a/scripts/lib/repository-file-inventory.ts b/scripts/lib/repository-file-inventory.ts index aa99dfd..c557265 100644 --- a/scripts/lib/repository-file-inventory.ts +++ b/scripts/lib/repository-file-inventory.ts @@ -107,13 +107,17 @@ async function defaultAssertReadable(target: string): Promise { await handle.close(); } -function normalizeRepositoryPath(value: string, label: string): string { +export function normalizeRepositoryRelativePath( + value: string, + label = "repository path", +): string { if ( value.length === 0 || path.posix.isAbsolute(value) || path.win32.isAbsolute(value) || value.includes("\\") || - value.includes("\0") + value.includes("\0") || + value.endsWith("/") ) { throw new TypeError(`${label} must be a repository-relative POSIX path`); } @@ -122,7 +126,7 @@ function normalizeRepositoryPath(value: string, label: string): string { normalized === "." || normalized === ".." || normalized.startsWith("../") || - normalized !== value.replace(/\/$/u, "") + normalized !== value ) { throw new TypeError(`${label} must be a repository-relative POSIX path`); } @@ -168,7 +172,7 @@ function parseGitFileList(result: GitFileListResult): string[] { throw new TypeError("git ls-files returned an empty NUL-delimited path"); } const normalized = rows.map((row) => - normalizeRepositoryPath(row, "git ls-files path"), + normalizeRepositoryRelativePath(row, "git ls-files path"), ); if (new Set(normalized).size !== normalized.length) { throw new TypeError("git ls-files returned a duplicate path"); @@ -193,14 +197,14 @@ export async function buildRepositoryFileInventory( const realpathPath = options.realpathPath ?? realpath; const assertReadable = options.assertReadable ?? defaultAssertReadable; const trackedRoots = options.trackedRoots.map((root) => - normalizeRepositoryPath(root, "tracked root"), + normalizeRepositoryRelativePath(root, "tracked root"), ); const generatedRoots = (options.generatedRoots ?? []).map((root) => - normalizeRepositoryPath(root, "generated root"), + normalizeRepositoryRelativePath(root, "generated root"), ); const optionalRoots = new Set( (options.optionalRoots ?? []).map((root) => - normalizeRepositoryPath(root, "optional root"), + normalizeRepositoryRelativePath(root, "optional root"), ), ); for (const root of optionalRoots) { @@ -290,7 +294,7 @@ export async function buildRepositoryFileInventory( const entries = await readdir(absoluteTarget, { withFileTypes: true }); for (const entry of entries.sort((left, right) => left.name.localeCompare(right.name))) { await collectGenerated( - normalizeRepositoryPath( + normalizeRepositoryRelativePath( `${relativeTarget}/${entry.name}`, "generated inventory path", ), diff --git a/scripts/lib/secret-scan-policy.ts b/scripts/lib/secret-scan-policy.ts new file mode 100644 index 0000000..0b2e42b --- /dev/null +++ b/scripts/lib/secret-scan-policy.ts @@ -0,0 +1,59 @@ +import { normalizeRepositoryRelativePath } from "./repository-file-inventory.ts"; + +export function parseSecretScanIncludedPaths( + value: unknown, +): readonly string[] | null { + if (value === undefined) return null; + if ( + !Array.isArray(value) || + value.length === 0 || + value.some( + (entry) => + typeof entry !== "string" || + entry.length === 0 || + entry.trim() !== entry, + ) + ) { + throw new TypeError( + "includedPaths must be a non-empty array of repository-relative POSIX paths", + ); + } + const normalized = value.map((entry) => + normalizeRepositoryRelativePath(entry as string, "included path"), + ); + if (new Set(normalized).size !== normalized.length) { + throw new TypeError("includedPaths must not contain duplicate paths"); + } + return Object.freeze(normalized); +} + +export function selectIncludedInventoryFiles( + inventoryFiles: readonly string[], + includedPaths: readonly string[] | null, +): readonly string[] { + if (includedPaths === null) return Object.freeze([...inventoryFiles]); + const validatedIncludedPaths = parseSecretScanIncludedPaths(includedPaths); + if (validatedIncludedPaths === null) { + throw new TypeError("includedPaths unexpectedly omitted"); + } + for (const includedPath of validatedIncludedPaths) { + if ( + !inventoryFiles.some( + (file) => + file === includedPath || file.startsWith(`${includedPath}/`), + ) + ) { + throw new Error( + `secret scan included path matches no inventory file: ${includedPath}`, + ); + } + } + return Object.freeze( + inventoryFiles.filter((file) => + validatedIncludedPaths.some( + (includedPath) => + file === includedPath || file.startsWith(`${includedPath}/`), + ), + ), + ); +} diff --git a/scripts/lib/security-fixture-check.ts b/scripts/lib/security-fixture-check.ts new file mode 100644 index 0000000..ee378bc --- /dev/null +++ b/scripts/lib/security-fixture-check.ts @@ -0,0 +1,170 @@ +import { spawnSync } from "node:child_process"; +import { mkdtemp, readFile, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; + +type ScanResult = Readonly<{ + error?: Error; + status: number | null; + signal: NodeJS.Signals | null; + stdout: string; + stderr: string; +}>; + +type SecurityFixtureCheckDependencies = Readonly<{ + createTempDirectory?: () => Promise; + runScan?: (artifactPath: string, policyPath: string) => ScanResult; + readArtifact?: (artifactPath: string) => Promise; + cleanup?: (directory: string) => Promise; +}>; + +type Document = Record; + +function record(value: unknown, label: string): Document { + if (!value || typeof value !== "object" || Array.isArray(value)) { + throw new TypeError(`${label} must be an object`); + } + return value as Document; +} + +function defaultScan(artifactPath: string, policyPath: string): ScanResult { + const scan = spawnSync( + "node", + [ + "scripts/security-scan.ts", + "--policy", + policyPath, + "--artifact", + artifactPath, + ], + { encoding: "utf8" }, + ); + return { + ...(scan.error ? { error: scan.error } : {}), + status: scan.status, + signal: scan.signal, + stdout: scan.stdout ?? "", + stderr: scan.stderr ?? "", + }; +} + +function assertExactFindings(rawArtifact: string): void { + const sarif = record(JSON.parse(rawArtifact), "security fixture SARIF"); + const runs = Array.isArray(sarif.runs) ? sarif.runs : []; + const run = record(runs[0], "security fixture SARIF run"); + const results = Array.isArray(run.results) ? run.results : []; + const actual = results + .map((rawResult) => { + const result = record(rawResult, "security fixture result"); + const locations = Array.isArray(result.locations) ? result.locations : []; + const location = record(locations[0], "security fixture location"); + const physical = record( + location.physicalLocation, + "security fixture physical location", + ); + const artifactLocation = record( + physical.artifactLocation, + "security fixture artifact location", + ); + return `${String(artifactLocation.uri)}:${String(result.ruleId)}`; + }) + .sort(); + const root = "tests/fixtures/security/secret-detection/forbidden"; + const expected = [ + `${root}/config.json:assigned-secret`, + `${root}/dist.ts:assigned-secret`, + `${root}/source.ts:aws-access-key`, + ].sort(); + if (JSON.stringify(actual) !== JSON.stringify(expected)) { + throw new Error( + `security fixture findings mismatch: expected ${expected.join(", ")}; received ${actual.join(", ")}`, + ); + } +} + +export async function checkSecurityFixtures( + dependencies: SecurityFixtureCheckDependencies = {}, +): Promise { + const createTempDirectory = + dependencies.createTempDirectory ?? + (() => mkdtemp(path.join(tmpdir(), "ca-security-fixture-"))); + const runScan = dependencies.runScan ?? defaultScan; + const readArtifact = dependencies.readArtifact ?? ((target) => readFile(target, "utf8")); + const cleanup = + dependencies.cleanup ?? + ((directory) => rm(directory, { recursive: true, force: true })); + const directory = await createTempDirectory(); + const artifactPath = path.join(directory, "scan-fixture.sarif"); + try { + const scan = runScan( + artifactPath, + "tests/fixtures/security/secret-detection/forbidden-policy.json", + ); + const expectedDiagnostic = "Security scan found 3 blocking result(s)."; + if ( + scan.error || + scan.status !== 1 || + scan.signal !== null || + scan.stderr !== `${expectedDiagnostic}\n` + ) { + throw new Error( + `forbidden security fixture did not fail exactly: ${scan.error?.message ?? scan.stderr}`, + ); + } + assertExactFindings(await readArtifact(artifactPath)); + } finally { + await cleanup(directory); + } +} + +export async function checkNonmatchingSecurityIncludeFixture( + dependencies: SecurityFixtureCheckDependencies = {}, +): Promise { + const createTempDirectory = + dependencies.createTempDirectory ?? + (() => mkdtemp(path.join(tmpdir(), "ca-security-include-fixture-"))); + const runScan = dependencies.runScan ?? defaultScan; + const readArtifact = + dependencies.readArtifact ?? ((target) => readFile(target, "utf8")); + const cleanup = + dependencies.cleanup ?? + ((directory) => rm(directory, { recursive: true, force: true })); + const directory = await createTempDirectory(); + const artifactPath = path.join(directory, "scan-fixture.sarif"); + try { + const includedPath = + "tests/fixtures/security/secret-detection/misspelled"; + const scan = runScan( + artifactPath, + "tests/fixtures/security/secret-detection/nonmatching-policy.json", + ); + if ( + scan.error || + scan.status !== 1 || + scan.signal !== null || + !scan.stderr.includes( + `secret scan included path matches no inventory file: ${includedPath}`, + ) + ) { + throw new Error( + `nonmatching security include fixture did not fail closed: ${scan.error?.message ?? scan.stderr}`, + ); + } + try { + await readArtifact(artifactPath); + } catch (error) { + if ( + typeof error === "object" && + error !== null && + "code" in error && + error.code === "ENOENT" + ) { + return; + } + throw error; + } + throw new Error("nonmatching security include fixture wrote an artifact"); + } finally { + await cleanup(directory); + } +} diff --git a/scripts/security-scan.ts b/scripts/security-scan.ts index 7208c59..91beeb8 100644 --- a/scripts/security-scan.ts +++ b/scripts/security-scan.ts @@ -10,6 +10,10 @@ import { secretScanRules, type SecretFinding, } from "./lib/secret-scan.ts"; +import { + parseSecretScanIncludedPaths, + selectIncludedInventoryFiles, +} from "./lib/secret-scan-policy.ts"; type AllowlistEntry = Readonly<{ path: string; @@ -23,7 +27,7 @@ type SecretPolicy = Readonly<{ trackedRoots: readonly string[]; generatedRoots: readonly string[]; optionalRoots: readonly string[]; - includedPaths: readonly string[]; + includedPaths: readonly string[] | null; allowlist: readonly AllowlistEntry[]; }>; @@ -65,7 +69,7 @@ function parsePolicy(value: unknown): SecretPolicy { trackedRoots: inventoryPolicy.trackedRoots, generatedRoots: inventoryPolicy.generatedRoots, optionalRoots: inventoryPolicy.optionalRoots, - includedPaths: Object.freeze(strings(document.includedPaths)), + includedPaths: parseSecretScanIncludedPaths(document.includedPaths), allowlist: Object.freeze(allowlist), }); } @@ -87,9 +91,6 @@ const patterns = secretScanRules(); const excluded = new Set( policy.excludedPaths.map((entry) => entry.replaceAll("\\", "/")), ); -const included = policy.includedPaths.map((entry) => - entry.replaceAll("\\", "/"), -); const allowlist = policy.allowlist; for (const entry of allowlist) { const expiry = Date.parse(entry.expiresAt); @@ -111,14 +112,13 @@ const inventory = await buildRepositoryFileInventory({ generatedRoots: policy.generatedRoots, optionalRoots: policy.optionalRoots, }); -const scanFiles = inventory.files; +const scanFiles = selectIncludedInventoryFiles( + inventory.files, + policy.includedPaths, +); for (const scanFile of [...new Set(scanFiles)].sort()) { const normalized = scanFile.replaceAll("\\", "/"); if ( - (included.length > 0 && - !included.some( - (entry) => normalized === entry || normalized.startsWith(`${entry}/`), - )) || [...excluded].some( (entry) => normalized === entry || normalized.startsWith(`${entry}/`), ) || diff --git a/tests/fixtures/security/secret-detection/nonmatching-policy.json b/tests/fixtures/security/secret-detection/nonmatching-policy.json new file mode 100644 index 0000000..9a8d3ae --- /dev/null +++ b/tests/fixtures/security/secret-detection/nonmatching-policy.json @@ -0,0 +1,12 @@ +{ + "schemaVersion": 1, + "trackedRoots": [ + "tests/fixtures/security/secret-detection/forbidden" + ], + "generatedRoots": [], + "includedPaths": [ + "tests/fixtures/security/secret-detection/misspelled" + ], + "excludedPaths": [], + "allowlist": [] +} diff --git a/tests/unit/supply-chain.test.ts b/tests/unit/supply-chain.test.ts index 26c2b27..15308c0 100644 --- a/tests/unit/supply-chain.test.ts +++ b/tests/unit/supply-chain.test.ts @@ -12,6 +12,11 @@ import { } from "../../scripts/lib/supply-chain.ts"; import { digestReleaseInputFiles } from "../../scripts/lib/release-input-evidence.ts"; import { findSecretMatches } from "../../scripts/lib/secret-scan.ts"; +import { + parseSecretScanIncludedPaths, + selectIncludedInventoryFiles, +} from "../../scripts/lib/secret-scan-policy.ts"; +import { checkSecurityFixtures } from "../../scripts/lib/security-fixture-check.ts"; const integrity = `sha512-${Buffer.alloc(64, 7).toString("base64")}`; const dependency = { @@ -26,6 +31,103 @@ const dependency = { }; describe("supply-chain policy", () => { + it.each([ + ["empty", []], + ["empty entry", [""]], + ["blank entry", [" "]], + ["absolute", ["/src"]], + ["backslash", ["src\\file.ts"]], + ["dot", ["."]], + ["dotdot", [".."]], + ["traversal", ["src/../docs"]], + ["trailing slash", ["src/"]], + ["mixed", ["src", 42]], + ["duplicate", ["src", "src"]], + ])("rejects %s secret-scan include paths", (_name, includedPaths) => { + expect(() => parseSecretScanIncludedPaths(includedPaths)).toThrow(); + }); + + it("requires every configured include path to match the inventory", () => { + expect( + selectIncludedInventoryFiles( + ["README.md", "src/app.ts"], + ["src"], + ), + ).toEqual(["src/app.ts"]); + expect(() => + selectIncludedInventoryFiles( + ["README.md", "src/app.ts"], + ["misspelled"], + ), + ).toThrow(/misspelled/u); + expect( + selectIncludedInventoryFiles( + ["README.md", "src/app.ts"], + null, + ), + ).toEqual(["README.md", "src/app.ts"]); + }); + + it("rejects a crashed fixture scan and cannot reuse a stale repository artifact", async () => { + const cleaned: string[] = []; + await expect( + checkSecurityFixtures({ + createTempDirectory: async () => "/tmp/fresh-security-fixture", + runScan: () => ({ + status: 1, + signal: null, + stdout: "", + stderr: "Security scan found 3 blocking result(s).\n", + }), + readArtifact: async (artifactPath) => { + expect(artifactPath).toBe( + "/tmp/fresh-security-fixture/scan-fixture.sarif", + ); + throw Object.assign(new Error("fresh artifact missing"), { + code: "ENOENT", + }); + }, + cleanup: async (directory) => { + cleaned.push(directory); + }, + }), + ).rejects.toThrow(/fresh artifact missing/u); + expect(cleaned).toEqual(["/tmp/fresh-security-fixture"]); + + await expect( + checkSecurityFixtures({ + createTempDirectory: async () => "/tmp/fresh-security-fixture", + runScan: () => ({ + status: null, + signal: "SIGTERM", + stdout: "", + stderr: "Security scan found 3 blocking result(s).\n", + }), + readArtifact: async () => "{}", + cleanup: async () => undefined, + }), + ).rejects.toThrow(/did not fail exactly/u); + }); + + it("wires the exact security fixture checker as a passing CI gate", async () => { + const gates = JSON.parse(await readFile("config/ci/gates.json", "utf8")) as { + gates: Record; + }; + const securityGate = gates.gates["FE-GATE-013"]!; + expect(securityGate.steps).toContainEqual({ + script: "check:security:fixtures", + expect: "pass", + }); + expect(securityGate.steps).not.toEqual( + expect.arrayContaining([ + expect.objectContaining({ script: "scan:security:fixture" }), + ]), + ); + expect(securityGate.evidence).not.toContain( + "artifacts/security/scan-fixture.sarif", + ); + }); + it("uses one fail-closed repository inventory for provenance and secret scanning", async () => { const [provenanceSource, securitySource] = await Promise.all([ readFile("scripts/generate-supply-chain.ts", "utf8"),