diff --git a/src/security/audit.test.ts b/src/security/audit.test.ts index 45198951a..537ccea93 100644 --- a/src/security/audit.test.ts +++ b/src/security/audit.test.ts @@ -2234,7 +2234,7 @@ describe("security audit", () => { } }); - it("flags plugins with dangerous code patterns (deep audit)", async () => { + it("does not scan plugin code safety findings when deep audit is disabled", async () => { const tmpDir = await makeTmpDir("audit-scanner-plugin"); const pluginDir = path.join(tmpDir, "extensions", "evil-plugin"); await fs.mkdir(path.join(pluginDir, ".hidden"), { recursive: true }); @@ -2260,20 +2260,7 @@ describe("security audit", () => { }); expect(nonDeepRes.findings.some((f) => f.checkId === "plugins.code_safety")).toBe(false); - const deepRes = await runSecurityAudit({ - config: cfg, - includeFilesystem: true, - includeChannelSecurity: false, - deep: true, - stateDir: tmpDir, - probeGatewayFn: async (opts) => successfulProbeResult(opts.url), - }); - - expect( - deepRes.findings.some( - (f) => f.checkId === "plugins.code_safety" && f.severity === "critical", - ), - ).toBe(true); + // Deep-mode positive coverage lives in the detailed plugin+skills code-safety test below. }); it("reports detailed code-safety issues for both plugins and skills", async () => { diff --git a/src/security/temp-path-guard.test.ts b/src/security/temp-path-guard.test.ts index cbc6a1e7c..78a45b497 100644 --- a/src/security/temp-path-guard.test.ts +++ b/src/security/temp-path-guard.test.ts @@ -1,9 +1,6 @@ -import { spawnSync } from "node:child_process"; -import fs from "node:fs/promises"; -import path from "node:path"; import { describe, expect, it } from "vitest"; +import { loadRuntimeSourceFilesForGuardrails } from "../test-utils/runtime-source-guardrail-scan.js"; -const RUNTIME_ROOTS = ["src", "extensions"]; const SKIP_PATTERNS = [ /\.test\.tsx?$/, /\.test-helpers\.tsx?$/, @@ -11,7 +8,7 @@ const SKIP_PATTERNS = [ /\.test-harness\.tsx?$/, /\.e2e\.tsx?$/, /\.d\.ts$/, - /[\\/](?:__tests__|tests)[\\/]/, + /[\\/](?:__tests__|tests|test-utils)[\\/]/, /[\\/][^\\/]*test-helpers(?:\.[^\\/]+)?\.ts$/, /[\\/][^\\/]*test-utils(?:\.[^\\/]+)?\.ts$/, /[\\/][^\\/]*test-harness(?:\.[^\\/]+)?\.ts$/, @@ -193,100 +190,6 @@ function hasDynamicTmpdirJoin(source: string): boolean { return false; } -async function listTsFiles(dir: string): Promise { - const entries = await fs.readdir(dir, { withFileTypes: true }); - const out: string[] = []; - for (const entry of entries) { - if (entry.name === "node_modules" || entry.name === "dist" || entry.name.startsWith(".")) { - continue; - } - const fullPath = path.join(dir, entry.name); - if (entry.isDirectory()) { - out.push(...(await listTsFiles(fullPath))); - continue; - } - if (!entry.isFile()) { - continue; - } - if (fullPath.endsWith(".ts") || fullPath.endsWith(".tsx")) { - out.push(fullPath); - } - } - return out; -} - -function parsePathList(stdout: string): Set { - const out = new Set(); - for (const line of stdout.split(/\r?\n/)) { - const trimmed = line.trim(); - if (!trimmed) { - continue; - } - out.add(path.resolve(trimmed)); - } - return out; -} - -function prefilterLikelyTmpdirJoinFiles(roots: readonly string[]): Set | null { - const commonArgs = [ - "--files-with-matches", - "--glob", - "*.ts", - "--glob", - "*.tsx", - "--glob", - "!**/*.test.ts", - "--glob", - "!**/*.test.tsx", - "--glob", - "!**/*.e2e.ts", - "--glob", - "!**/*.e2e.tsx", - "--glob", - "!**/*.d.ts", - "--glob", - "!**/*test-helpers*.ts", - "--glob", - "!**/*test-helpers*.tsx", - "--glob", - "!**/*test-utils*.ts", - "--glob", - "!**/*test-utils*.tsx", - "--glob", - "!**/*test-harness*.ts", - "--glob", - "!**/*test-harness*.tsx", - "--no-messages", - ]; - const strictDynamicCall = spawnSync( - "rg", - [ - ...commonArgs, - "-P", - "-U", - "(?s)path\\s*\\.\\s*join\\s*\\(\\s*os\\s*\\.\\s*tmpdir\\s*\\([^`]*`", - ...roots, - ], - { encoding: "utf8" }, - ); - if ( - !strictDynamicCall.error && - (strictDynamicCall.status === 0 || strictDynamicCall.status === 1) - ) { - return parsePathList(strictDynamicCall.stdout); - } - - const candidateCall = spawnSync( - "rg", - [...commonArgs, "path\\s*\\.\\s*join\\s*\\(\\s*os\\s*\\.\\s*tmpdir\\s*\\(", ...roots], - { encoding: "utf8" }, - ); - if (candidateCall.error || (candidateCall.status !== 0 && candidateCall.status !== 1)) { - return null; - } - return parsePathList(candidateCall.stdout); -} - describe("temp path guard", () => { it("skips test helper filename variants", () => { expect(shouldSkip("src/commands/test-helpers.ts")).toBe(true); @@ -316,42 +219,33 @@ describe("temp path guard", () => { expect(hasDynamicTmpdirJoin(fixture)).toBe(false); } }); - it("blocks dynamic template path.join(os.tmpdir(), ...) in runtime source files", async () => { - const repoRoot = process.cwd(); - const offenders: string[] = []; - const scanRoots = RUNTIME_ROOTS.map((root) => path.join(repoRoot, root)); - const rgPrefiltered = prefilterLikelyTmpdirJoinFiles(scanRoots); - const prefilteredByRoot = new Map(); - if (rgPrefiltered) { - for (const file of rgPrefiltered) { - for (const absRoot of scanRoots) { - if (file.startsWith(absRoot + path.sep)) { - const bucket = prefilteredByRoot.get(absRoot) ?? []; - bucket.push(file); - prefilteredByRoot.set(absRoot, bucket); - break; - } - } - } - } - for (const root of RUNTIME_ROOTS) { - const absRoot = path.join(repoRoot, root); - const files = rgPrefiltered - ? (prefilteredByRoot.get(absRoot) ?? []) - : await listTsFiles(absRoot); - for (const file of files) { - const relativePath = path.relative(repoRoot, file); - if (shouldSkip(relativePath)) { - continue; - } - const source = await fs.readFile(file, "utf8"); - if (hasDynamicTmpdirJoin(source)) { - offenders.push(relativePath); + it("enforces runtime guardrails for tmpdir joins and weak randomness", async () => { + const files = await loadRuntimeSourceFilesForGuardrails(process.cwd()); + const offenders: string[] = []; + const weakRandomMatches: string[] = []; + + for (const file of files) { + const relativePath = file.relativePath; + if (shouldSkip(relativePath)) { + continue; + } + if (hasDynamicTmpdirJoin(file.source)) { + offenders.push(relativePath); + } + if (file.source.includes("Date.now") && file.source.includes("Math.random")) { + const lines = file.source.split(/\r?\n/); + for (let idx = 0; idx < lines.length; idx += 1) { + const line = lines[idx] ?? ""; + if (!line.includes("Date.now") || !line.includes("Math.random")) { + continue; + } + weakRandomMatches.push(`${relativePath}:${idx + 1}`); } } } expect(offenders).toEqual([]); + expect(weakRandomMatches).toEqual([]); }); }); diff --git a/src/security/weak-random-patterns.test.ts b/src/security/weak-random-patterns.test.ts deleted file mode 100644 index 2bebf028f..000000000 --- a/src/security/weak-random-patterns.test.ts +++ /dev/null @@ -1,101 +0,0 @@ -import { spawnSync } from "node:child_process"; -import { describe, expect, it } from "vitest"; - -const SCAN_ROOTS = ["src", "extensions"] as const; -const RUNTIME_TS_GLOBS = [ - "*.ts", - "!*.test.ts", - "!*.test-helpers.ts", - "!*.test-utils.ts", - "!*.e2e.ts", - "!*.d.ts", - "!**/__tests__/**", - "!**/tests/**", - "!**/*test-helpers*.ts", - "!**/*test-utils*.ts", -] as const; - -const SKIP_RUNTIME_SOURCE_PATH_PATTERNS = [ - /\.test\.tsx?$/, - /\.test-helpers\.tsx?$/, - /\.test-utils\.tsx?$/, - /\.e2e\.tsx?$/, - /\.d\.ts$/, - /[\\/](?:__tests__|tests)[\\/]/, - /[\\/][^\\/]*test-helpers(?:\.[^\\/]+)?\.ts$/, - /[\\/][^\\/]*test-utils(?:\.[^\\/]+)?\.ts$/, -]; - -function shouldSkipRuntimeSourcePath(relativePath: string): boolean { - return SKIP_RUNTIME_SOURCE_PATH_PATTERNS.some((pattern) => pattern.test(relativePath)); -} - -async function findWeakRandomPatternMatches(repoRoot: string): Promise { - const rgResult = spawnSync( - "rg", - [ - "--line-number", - "--no-heading", - "--color=never", - ...RUNTIME_TS_GLOBS.flatMap((glob) => ["--glob", glob]), - "Date\\.now.*Math\\.random|Math\\.random.*Date\\.now", - ...SCAN_ROOTS, - ], - { - cwd: repoRoot, - encoding: "utf8", - }, - ); - if (!rgResult.error && (rgResult.status === 0 || rgResult.status === 1)) { - const matches: string[] = []; - const lines = rgResult.stdout.split(/\r?\n/); - for (const line of lines) { - const text = line.trim(); - if (!text) { - continue; - } - const parsed = /^(.*?):(\d+):(.*)$/.exec(text); - if (!parsed) { - continue; - } - const relativePath = parsed[1] ?? ""; - const lineNumber = parsed[2] ?? ""; - if (shouldSkipRuntimeSourcePath(relativePath)) { - continue; - } - matches.push(`${relativePath}:${lineNumber}`); - } - return matches; - } - - const [{ default: fs }, pathModule, { listRuntimeSourceFiles }] = await Promise.all([ - import("node:fs/promises"), - import("node:path"), - import("../test-utils/repo-scan.js"), - ]); - - const matches: string[] = []; - const files = await listRuntimeSourceFiles(repoRoot, { - roots: SCAN_ROOTS, - extensions: [".ts"], - }); - for (const filePath of files) { - const lines = (await fs.readFile(filePath, "utf8")).split(/\r?\n/); - for (let idx = 0; idx < lines.length; idx += 1) { - const line = lines[idx] ?? ""; - if (!line.includes("Date.now") || !line.includes("Math.random")) { - continue; - } - matches.push(`${pathModule.relative(repoRoot, filePath)}:${idx + 1}`); - } - } - return matches; -} - -describe("weak random pattern guardrail", () => { - it("rejects Date.now + Math.random token/id patterns in runtime code", async () => { - const repoRoot = process.cwd(); - const matches = await findWeakRandomPatternMatches(repoRoot); - expect(matches).toEqual([]); - }); -}); diff --git a/src/test-utils/runtime-source-guardrail-scan.ts b/src/test-utils/runtime-source-guardrail-scan.ts new file mode 100644 index 000000000..667ed4f0b --- /dev/null +++ b/src/test-utils/runtime-source-guardrail-scan.ts @@ -0,0 +1,64 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import { listRuntimeSourceFiles } from "./repo-scan.js"; + +export type RuntimeSourceGuardrailFile = { + relativePath: string; + source: string; +}; + +const runtimeSourceGuardrailCache = new Map>(); +const FILE_READ_CONCURRENCY = 32; + +async function readRuntimeSourceFiles( + repoRoot: string, + absolutePaths: string[], +): Promise { + const output: Array = Array.from({ + length: absolutePaths.length, + }); + let nextIndex = 0; + + const worker = async () => { + for (;;) { + const index = nextIndex; + nextIndex += 1; + if (index >= absolutePaths.length) { + return; + } + const absolutePath = absolutePaths[index]; + if (!absolutePath) { + continue; + } + const source = await fs.readFile(absolutePath, "utf8"); + output[index] = { + relativePath: path.relative(repoRoot, absolutePath), + source, + }; + } + }; + + const workers = Array.from( + { length: Math.min(FILE_READ_CONCURRENCY, Math.max(1, absolutePaths.length)) }, + () => worker(), + ); + await Promise.all(workers); + return output.filter((entry): entry is RuntimeSourceGuardrailFile => entry !== undefined); +} + +export async function loadRuntimeSourceFilesForGuardrails( + repoRoot: string, +): Promise { + let pending = runtimeSourceGuardrailCache.get(repoRoot); + if (!pending) { + pending = (async () => { + const files = await listRuntimeSourceFiles(repoRoot, { + roots: ["src", "extensions"], + extensions: [".ts", ".tsx"], + }); + return await readRuntimeSourceFiles(repoRoot, files); + })(); + runtimeSourceGuardrailCache.set(repoRoot, pending); + } + return await pending; +}