diff --git a/Dockerfile b/Dockerfile index bc95d696..f8ab5770 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,6 +2,10 @@ FROM node:22-bookworm-slim@sha256:6c74791e557ce11fc957704f6d4fe134a7bc8d6f5ca4403205b2966bd488f6b3 AS package +RUN apt-get update \ + && apt-get install --no-install-recommends --yes python3 \ + && rm -rf /var/lib/apt/lists/* + WORKDIR /build/sdk/typescript COPY sdk/typescript/package.json sdk/typescript/pnpm-lock.yaml ./ diff --git a/README.md b/README.md index 14d6eb8f..760914fe 100644 --- a/README.md +++ b/README.md @@ -97,10 +97,20 @@ npx @openai/codex-security policy . --headless --output-dir /path/outside/reposi The command reads the source, describes the system, builds a detailed threat model, and drafts a short `SECURITY.md`. In a terminal, it asks about important -facts the code cannot establish and shows the proposed diff. It does not change -repository files. Review the saved policy and check for other policy files that -link to the target before copying it. Later scans read the root and nested -`SECURITY.md` files. +facts the code cannot establish, shows the proposed diff, and asks before +writing. Later scans read the approved root and nested `SECURITY.md` files. + +To review a saved draft, edit its `SECURITY.md`, then run: + +```bash +npx @openai/codex-security policy . --apply /path/outside/repository/policy --write +``` + +Use the same repository and `--path` as generation. Applying does not call the +model. It checks that the original and inherited policies are unchanged, keeps +the write inside the approved scope, and verifies the result. Updates retain the +previous file at the reported recovery path. Keep it until other writers have +closed it and any edits are reconciled. Drafts are stored outside the repository and any enclosing Git checkout. The same private directory contains `project-spec.md`, `THREAT_MODEL.md`, and review diff --git a/sdk/typescript/README.md b/sdk/typescript/README.md index 78633671..b0a1c694 100644 --- a/sdk/typescript/README.md +++ b/sdk/typescript/README.md @@ -201,9 +201,9 @@ Trusted Access for Cyber. To apply or check your access, visit ## Generate a security policy -`policy` drafts a `SECURITY.md` for owner review. It uses the same Codex runtime, -authentication, model settings, and security guidance as scans, but does not -look for vulnerabilities or create a scan record. Codex can read files but +`policy` drafts or updates a `SECURITY.md` for owner review. It uses the same +Codex runtime, authentication, model settings, and security guidance as scans, +but does not look for vulnerabilities or create a scan record. Codex can read files but cannot write them. Network access, web search, apps, and MCP servers are disabled. The SDK saves the responses in a private directory outside the checkout. @@ -227,15 +227,13 @@ that would spread a component policy to a wider scope. Generation has three stages: describe the system, build a detailed threat model, and draft the policy. In a terminal, the command asks about important facts the source cannot establish, then shows the exact diff and decisions that need -review. If both a ChatGPT sign-in and an API key are available, it asks which to -use. Set `--auth chatgpt` or `--auth api-key` to choose explicitly. +review. It asks before writing to the repository. If both a ChatGPT sign-in and +an API key are available, it asks which to use. Set `--auth chatgpt` or +`--auth api-key` to choose explicitly. -### Review the draft +### Review and apply a saved draft -The command never changes repository files. Review the saved `SECURITY.md` -before copying it to the reported target path. Check whether another policy, -such as `.github/SECURITY.md` or `docs/SECURITY.md`, links to that target; a -manual copy can change the linked policy too. Preserve existing reporting +Review the saved `SECURITY.md` before applying it. Preserve existing reporting instructions and obtain owner approval for exclusions, accepted risks, and severity decisions. Later scans read the approved policy. @@ -243,16 +241,48 @@ Preview checks that the selected policy and its parent policies have not changed. Other source files are not frozen; generate a new draft if relevant source or neighboring policy files change. -Use `--headless` or an explicit output format to skip questions. Unanswered -questions remain in the review notes. Drafts default to the Codex Security state -directory; `--output-dir` selects an empty directory outside every enclosing -Git checkout and its Git metadata. +Use `--headless` or an explicit output format to skip questions and write +prompts. Unanswered questions remain in the review notes. Drafts default to the +Codex Security state directory; `--output-dir` selects an empty directory +outside every enclosing Git checkout and its Git metadata. ```bash npx @openai/codex-security policy . --path services/api \ --headless --output-dir /path/outside/repository/api-policy --json + +# Review or edit the saved SECURITY.md. +npx @openai/codex-security policy . --path services/api \ + --apply /path/outside/repository/api-policy --write ``` +`--apply` loads the saved draft without starting Codex. Omit `--write` to review +and confirm interactively. `--write` requires `--apply`; it cannot write an +unseen model response. The repository and component must match the draft. +Before writing, the command checks that the original policy and inherited +guidance have not changed. It also rejects links that would change another +component's guidance or a separate reporting policy in `.github` or `docs`. +Fix those links before applying. It writes the reviewed bytes, verifies that +the policy resolver can read them, and does not stage, commit, or publish them. +If a write succeeded but verification failed, fix the reported problem and +retry the same saved draft with `--write`. An exact-content retry verifies the +installed policy without replacing it. Keep any previously reported recovery +file until concurrent edits have been reconciled. + +Updates keep the previous file so a late save through an open editor handle is +not lost. The command moves it into the private artifact directory when possible; +otherwise it stays beside the target as `.SECURITY.md.*.previous`. The CLI prints +the path and returns `recoveryPath` in JSON. Keep that file until other writers +have closed it and any edits are reconciled. A `recovery_required` result means +the replacement needs manual reconciliation. A `written_unverified` result means +the new policy was written but verification failed. Inspect the reported paths +before retrying. Once a write commits, SDK cancellation does not skip the +remaining checks. A terminal interrupt or process failure can still leave a +`written_unverified` result. A later Ctrl-C or SIGTERM forces the CLI to stop. + +Save edited drafts as UTF-8. If generation used a custom `--plugin-path`, select +it again when applying a saved draft. Saved metadata cannot choose executable +plugin code. Both plugin directories and ZIP files are supported. + The artifact directory contains: | File | Purpose | @@ -260,9 +290,11 @@ The artifact directory contains: | `SECURITY.md` | Editable policy draft. | | `THREAT_MODEL.md` | Detailed threat model with source references. | | `project-spec.md` | System description and security boundaries. | -| `previous-SECURITY.md` | Original policy used for the diff. | +| `previous-SECURITY.md` | Original policy used for the diff and overwrite checks. | | `policy-draft.json` | Target, policy hashes, revision, model, and review notes. | +An update may also retain `recovery-SECURITY-*.md` files. + Keep detailed models and intermediate files private until they have been reviewed for disclosure. Generation does not imply owner approval or confirm that a threat scenario is a vulnerability. @@ -270,14 +302,15 @@ that a threat scenario is a vulnerability. `--format md` writes the draft to stdout. `--json` returns paths, review notes, status, and estimated cost. Global filters and token options work with these formats. Progress goes to stderr. `--full-output` reports failures with -`ok: false`. `--max-cost` applies to the whole generation. If a stage cannot +`ok: false`. Plain `--json` retains recovery status and paths when a write +needs attention. `--max-cost` applies to the whole generation. If a stage cannot inspect required source evidence, generation stops and preserves completed documents. Fix the reported problem and use a new output directory to retry. ### Generate a policy from TypeScript ```ts -import { CodexSecurity } from "@openai/codex-security"; +import { CodexSecurity, applySecurityPolicy } from "@openai/codex-security"; const security = new CodexSecurity(); try { @@ -289,6 +322,8 @@ try { console.log(await security.previewPolicy(draft)); // Open draft.draftPath in an editor to review the saved policy. + // Obtain approval for this exact draft before calling: + // await applySecurityPolicy(draft, { pythonPath: security.config.pythonPath }); } finally { await security.close(); } @@ -298,11 +333,20 @@ try { `previewPolicy()` uses the client's Python setting and makes terminal control characters visible. The standalone `securityPolicyDiff()` returns a raw diff for files or other non-terminal uses; pass an interpreter explicitly if needed. -`generatePolicy()` accepts `auth`, `path`, `knowledgeBasePaths`, `outputDir`, -`maxCostUsd`, `signal`, and progress and cost callbacks. An optional +`generatePolicy()` never edits the repository. It accepts `auth`, `path`, +`knowledgeBasePaths`, `outputDir`, `maxCostUsd`, `signal`, and progress and cost +callbacks. An optional `answerQuestions` callback receives each group of up to three owner questions and a cancellation signal. Without it, the questions remain unresolved. +Use `loadSecurityPolicyDraft(repository, artifactDirectory, { path })` to load +an edited draft. `applySecurityPolicy()` returns `{ status, targetPath, recoveryPath }`; +`status` is `written` or `unchanged`. +`recoveryPath` is `null` when no existing file was replaced. Pass `{ pluginPath }` +when applying a saved custom-plugin draft. `SecurityPolicyVerificationError` and +`SecurityPolicyRecoveryError` identify files that need inspection or +reconciliation. + ## CLI ```bash diff --git a/sdk/typescript/scripts/smoke-package.mjs b/sdk/typescript/scripts/smoke-package.mjs index 6e93c16d..befa3c4a 100644 --- a/sdk/typescript/scripts/smoke-package.mjs +++ b/sdk/typescript/scripts/smoke-package.mjs @@ -1,5 +1,6 @@ import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; +import { createHash } from "node:crypto"; import { chmod, cp, @@ -71,7 +72,13 @@ async function resolveArchive() { function run( command, args, - { cwd, env, capture = false, windowsVerbatimArguments = false } = {}, + { + cwd, + env, + capture = false, + windowsVerbatimArguments = false, + expectedStatus = 0, + } = {}, ) { const result = spawnSync(command, args, { cwd, @@ -93,10 +100,10 @@ function run( if (result.error !== undefined) { throw new Error(`Failed to run ${command}.`, { cause: result.error }); } - if (result.status !== 0) { + if (result.status !== expectedStatus) { const details = capture ? `\n${result.stderr.trim()}` : ""; throw new Error( - `${command} exited with status ${result.status}.${details}`, + `${command} exited with status ${result.status} (expected ${expectedStatus}).${details}`, ); } @@ -349,7 +356,7 @@ try { "--eval", [ `const sdk = await import(${JSON.stringify(packageManifest.name)});`, - `for (const name of ${JSON.stringify(["CodexSecurity", "publishScan", "securityPolicyDiff"])}) {`, + `for (const name of ${JSON.stringify(["CodexSecurity", "publishScan", "applySecurityPolicy", "loadSecurityPolicyDraft", "securityPolicyDiff", "SecurityPolicyRecoveryError", "SecurityPolicyVerificationError"])}) {`, ' if (typeof sdk[name] !== "function") throw new Error(`The installed package does not export ${name}.`);', "}", 'if (typeof sdk.CodexSecurity.prototype.generatePolicy !== "function") throw new Error("The installed package does not export generatePolicy.");', @@ -443,7 +450,194 @@ try { join(await realpath(policyTarget), "SECURITY.md"), ); assert.equal(policyPreflight.dryRun, true); + + const policyArtifacts = join(consumer, "policy-draft"); + const policyMarkdown = + "# Security Policy\n\n## Security invariants\n\nCallers must authorize access to another account's records.\n"; + await mkdir(policyArtifacts, { mode: 0o700 }); + for (const [name, contents] of Object.entries({ + "SECURITY.md": policyMarkdown, + "previous-SECURITY.md": "", + "project-spec.md": "# Synthetic architecture\n", + "THREAT_MODEL.md": "# Synthetic threat model\n", + "policy-draft.json": JSON.stringify({ + documentType: "codex-security.policy-draft", + schemaVersion: "1.0", + repository: policyPreflight.repository, + scope: ".", + createdAt: "2026-01-01T00:00:00.000Z", + revision: null, + previousPolicySha256: null, + inheritedPolicySha256: createHash("sha256").update("[]").digest("hex"), + model: "synthetic-model", + reasoningEffort: "high", + pluginVersion: packageManifest.version, + customPlugin: false, + reviewNotes: [], + }), + })) { + await writeFile(join(policyArtifacts, name), contents, { mode: 0o600 }); + } + const savedPolicyEnvironment = { + ...process.env, + CODEX_CLI_PATH: join(consumer, "codex-must-not-run"), + OPENAI_API_KEY: "", + CODEX_API_KEY: "", + CODEX_SECURITY_STATE_DIR: join(consumer, "policy-state"), + }; + const previewPolicy = (args) => + run( + process.execPath, + [launcher, "policy", policyTarget, "--apply", policyArtifacts, ...args], + { cwd: consumer, capture: true, env: savedPolicyEnvironment }, + ); + assert.equal(previewPolicy(["--format", "md"]), policyMarkdown); + assert.match(previewPolicy(["--format=toon"]), /status: draft/u); + assert.equal( + JSON.parse(previewPolicy(["--json", "--filter-output", "status"])), + "draft", + ); + for (const format of [[], ["--format", "md"]]) { + const count = previewPolicy([...format, "--token-count"]).trim(); + assert.match(count, /^\d+$/u); + assert.ok(Number(count) > 0); + assert.match( + previewPolicy([...format, "--token-limit", "4"]), + /\[truncated: showing tokens /u, + ); + } + assert.equal( + JSON.parse(previewPolicy(["--json", "--full-output"])).data.status, + "draft", + ); + const failedPolicy = JSON.parse( + run( + process.execPath, + [ + launcher, + "policy", + policyTarget, + "--apply", + join(consumer, "missing-policy-draft"), + "--json", + "--full-output", + ], + { + cwd: consumer, + capture: true, + env: savedPolicyEnvironment, + expectedStatus: 2, + }, + ), + ); + assert.equal(failedPolicy.ok, false); + assert.equal(failedPolicy.error.code, "POLICY_FAILED"); + const invalidPolicy = JSON.parse( + run( + process.execPath, + [launcher, "policy", policyTarget, "--write", "--json", "--full-output"], + { + cwd: consumer, + capture: true, + env: savedPolicyEnvironment, + expectedStatus: 2, + }, + ), + ); + assert.equal(invalidPolicy.ok, false); + assert.match(invalidPolicy.error.message, /--write requires --apply/u); assert.deepEqual(await readdir(policyTarget), []); + // Node rejects Python's flags before reading stdin. Report that failure + // without an uncaught stream error in the installed Node.js entrypoint. + run( + process.execPath, + [ + "--input-type=module", + "--eval", + [ + 'import assert from "node:assert/strict";', + `const { loadSecurityPolicyDraft, securityPolicyDiff } = await import(${JSON.stringify(packageManifest.name)});`, + "const draft = await loadSecurityPolicyDraft(process.argv[1], process.argv[2]);", + 'draft.content = "# Policy\\n" + "x".repeat(900_000);', + "await assert.rejects(securityPolicyDiff(draft, process.execPath));", + ].join("\n"), + policyTarget, + policyArtifacts, + ], + { cwd: consumer }, + ); + const appliedPolicy = JSON.parse( + run( + process.execPath, + [ + launcher, + "policy", + policyTarget, + "--apply", + policyArtifacts, + "--write", + "--json", + ], + { + cwd: consumer, + capture: true, + env: savedPolicyEnvironment, + }, + ), + ); + assert.equal(appliedPolicy.status, "written"); + assert.equal( + await readFile(policyPreflight.targetPath, "utf8"), + policyMarkdown, + ); + run( + process.execPath, + [ + "--input-type=module", + "--eval", + [ + 'import assert from "node:assert/strict";', + 'import { createHash } from "node:crypto";', + 'import { mkdir, readFile, realpath, writeFile } from "node:fs/promises";', + 'import { dirname, join } from "node:path";', + `const { CodexSecurity, loadSecurityPolicyDraft, applySecurityPolicy } = await import(${JSON.stringify(packageManifest.name)});`, + "const repository = await realpath(process.argv[1]);", + "const artifacts = await realpath(process.argv[2]);", + 'const target = join(repository, "SECURITY.md");', + 'const previous = await readFile(target, "utf8");', + 'const next = previous + "\\nOwner-reviewed update.\\n";', + 'const manifestPath = join(artifacts, "policy-draft.json");', + 'const manifest = JSON.parse(await readFile(manifestPath, "utf8"));', + 'manifest.previousPolicySha256 = createHash("sha256").update(previous).digest("hex");', + 'await writeFile(join(artifacts, "previous-SECURITY.md"), previous);', + 'await writeFile(join(artifacts, "SECURITY.md"), next);', + "await writeFile(manifestPath, JSON.stringify(manifest));", + "const applied = await applySecurityPolicy(await loadSecurityPolicyDraft(repository, artifacts));", + "assert.equal(applied.targetPath, target);", + "assert.equal(dirname(applied.recoveryPath), artifacts);", + 'assert.equal(await readFile(applied.recoveryPath, "utf8"), previous);', + 'assert.equal(await readFile(target, "utf8"), next);', + 'await mkdir(join(repository, "component"));', + "const security = new CodexSecurity();", + "try {", + " await writeFile(target, Buffer.from([0xff]));", + ' await assert.rejects(security.preflightPolicy(repository, { path: "component" }), /valid UTF-8/);', + "} finally {", + " await security.close();", + " await writeFile(target, next);", + "}", + ].join("\n"), + policyTarget, + policyArtifacts, + ], + { + cwd: consumer, + env: { + ...process.env, + CODEX_CLI_PATH: join(consumer, "codex-must-not-run"), + }, + }, + ); const publicationScan = join(consumer, "publication-scan"); await cp( diff --git a/sdk/typescript/src/cli.ts b/sdk/typescript/src/cli.ts index 80269379..57cac88a 100644 --- a/sdk/typescript/src/cli.ts +++ b/sdk/typescript/src/cli.ts @@ -208,6 +208,7 @@ const VALUE_OPTIONS = new Set([ "--auth", "--path", "--knowledge-base", + "--apply", "--scan-prompt-file", "--post-scan-prompt-file", "--diff", @@ -1935,7 +1936,7 @@ export async function main( }, }) .command("policy", { - description: "Draft a source-backed SECURITY.md for owner review.", + description: "Generate or review a source-backed SECURITY.md policy.", destructive: true, mcp: false, args: z.object({ @@ -1946,67 +1947,106 @@ export async function main( "Repository or component directory (default: current directory).", ), }), - options: z.object({ - path: optionValue("--path") - .optional() - .describe( - "Generate SECURITY.md for this repository-relative component directory.", - ), - knowledgeBase: z - .array(optionValue("--knowledge-base")) - .default([]) - .describe( - "Add architecture or security-context files; repeat for multiple paths.", - ), - outputDir: optionValue("--output-dir") - .optional() - .describe( - "Private artifact directory outside the repository (default: Codex Security state).", - ), - headless: z - .boolean() - .default(false) - .describe("Do not ask owner questions."), - dryRun: z - .boolean() - .default(false) - .describe("Validate local generation inputs without starting Codex."), - auth: z - .enum(["auto", "chatgpt", "api-key"]) - .default("auto") - .describe("Select ChatGPT, API-key, or automatic authentication."), - model: optionValue("--model") - .optional() - .describe( - `Model to use (default: ${DEFAULT_SCAN_MODEL_CONFIGURATION.model}).`, + options: z + .object({ + path: optionValue("--path") + .optional() + .describe( + "Generate SECURITY.md for this repository-relative component directory.", + ), + knowledgeBase: z + .array(optionValue("--knowledge-base")) + .default([]) + .describe( + "Add architecture or security-context files; repeat for multiple paths.", + ), + outputDir: optionValue("--output-dir") + .optional() + .describe( + "Private artifact directory outside the repository (default: Codex Security state).", + ), + apply: optionValue("--apply") + .optional() + .describe( + "Review a saved policy artifact directory without calling the model.", + ), + write: z + .boolean() + .default(false) + .describe( + "Apply the reviewed --apply draft without an interactive confirmation.", + ), + headless: z + .boolean() + .default(false) + .describe("Do not ask questions or offer to write the policy."), + dryRun: z + .boolean() + .default(false) + .describe( + "Validate local generation inputs without starting Codex.", + ), + auth: z + .enum(["auto", "chatgpt", "api-key"]) + .default("auto") + .describe("Select ChatGPT, API-key, or automatic authentication."), + model: optionValue("--model") + .optional() + .describe( + `Model to use (default: ${DEFAULT_SCAN_MODEL_CONFIGURATION.model}).`, + ), + effort: effortOption(), + provider: PROVIDER_OPTION.describe( + "Inference provider for policy generation.", ), - effort: effortOption(), - provider: PROVIDER_OPTION.describe( - "Inference provider for policy generation.", + maxCost: z + .number() + .positive() + .optional() + .describe("Stop if estimated USD cost exceeds AMOUNT."), + pluginPath: optionValue("--plugin-path") + .optional() + .describe(PLUGIN_PATH_DESCRIPTION), + python: optionValue("--python") + .optional() + .describe(PYTHON_PATH_DESCRIPTION), + codex: z + .array(optionValue("--codex")) + .default([]) + .describe(CODEX_OVERRIDE_DESCRIPTION), + }) + .refine((options) => !options.write || options.apply !== undefined, { + message: + "--write requires --apply. Generate and review a draft first.", + }) + .refine( + (options) => + options.apply === undefined || + (!options.dryRun && + options.outputDir === undefined && + options.knowledgeBase.length === 0 && + options.auth === "auto" && + options.model === undefined && + options.effort === undefined && + options.provider === "openai" && + options.maxCost === undefined && + options.codex.length === 0), + { + message: "--apply cannot be combined with generation options.", + }, ), - maxCost: z - .number() - .positive() - .optional() - .describe("Stop if estimated USD cost exceeds AMOUNT."), - pluginPath: optionValue("--plugin-path") - .optional() - .describe(PLUGIN_PATH_DESCRIPTION), - python: optionValue("--python") - .optional() - .describe(PYTHON_PATH_DESCRIPTION), - codex: z - .array(optionValue("--codex")) - .default([]) - .describe(CODEX_OVERRIDE_DESCRIPTION), - }), examples: [ { args: { repository: "." } }, { args: { repository: "." }, options: { path: "services/api" } }, + { + args: { repository: "." }, + options: { apply: "/path/outside/repository/policy" }, + }, ], hint: - "Save a draft for review:\n" + - " codex-security policy . --headless --output-dir /path/outside/repository/policy --json", + "Noninteractive review:\n" + + " codex-security policy . --headless --output-dir /path/outside/repository/policy --json\n" + + " codex-security policy . --apply /path/outside/repository/policy --write", output: z .union([z.record(z.string(), z.unknown()), z.string()]) .optional(), @@ -2061,6 +2101,11 @@ export async function main( : resolve(directory, expandHome(options.outputDir)), maxCostUsd: options.maxCost, }, + apply: + options.apply === undefined + ? undefined + : resolve(directory, expandHome(options.apply)), + write: options.write, headless: options.headless || explicitOutput, dryRun: options.dryRun, format, @@ -2105,7 +2150,10 @@ export async function main( ), ); exitCode = outcome.exitCode; - if (exitCode !== 0) { + if ( + exitCode !== 0 && + (policyFullOutput || outcome.data === undefined) + ) { return fail(outcome.error ?? "Policy command failed.", exitCode); } if ( diff --git a/sdk/typescript/src/errors.ts b/sdk/typescript/src/errors.ts index 7a198816..8215e673 100644 --- a/sdk/typescript/src/errors.ts +++ b/sdk/typescript/src/errors.ts @@ -90,3 +90,39 @@ export class ScanCostLimitExceededError extends ScanInterruptedError { this.cost = cost; } } + +export class SecurityPolicyVerificationError extends CodexSecurityError { + public readonly recoveryPath?: string; + + public constructor( + public readonly targetPath: string, + options?: ErrorOptions & { recoveryPath?: string }, + ) { + const detail = + options?.cause === undefined + ? "" + : ` Cause: ${safeErrorMessage(options.cause)}`; + super( + `SECURITY.md was written to ${targetPath}, but verification failed.${options?.recoveryPath === undefined ? "" : ` Recovery file: ${options.recoveryPath}.`} Review the file before retrying.${detail}`, + options, + ); + this.recoveryPath = options?.recoveryPath; + } +} + +export class SecurityPolicyRecoveryError extends CodexSecurityError { + public constructor( + public readonly targetPath: string, + public readonly recoveryPath: string, + options?: ErrorOptions, + ) { + const detail = + options?.cause === undefined + ? "" + : ` Cause: ${safeErrorMessage(options.cause)}`; + super( + `Could not safely finish replacing ${targetPath}. The previous file is preserved at ${recoveryPath}. Review both paths before retrying.${detail}`, + options, + ); + } +} diff --git a/sdk/typescript/src/index.ts b/sdk/typescript/src/index.ts index 00166803..50d7a9f0 100644 --- a/sdk/typescript/src/index.ts +++ b/sdk/typescript/src/index.ts @@ -36,6 +36,8 @@ export { PluginPythonUnavailableError, ScanCostLimitExceededError, ScanInterruptedError, + SecurityPolicyRecoveryError, + SecurityPolicyVerificationError, } from "./errors.js"; export type { ProtectedScanPathKind } from "./errors.js"; export { @@ -48,10 +50,13 @@ export { loadContract, requireScanFile } from "./contract.js"; export type { LoadedContract, ScanExpectation } from "./contract.js"; export type * from "./models.js"; export { + applySecurityPolicy, + loadSecurityPolicyDraft, resolveSecurityPolicyTarget, securityPolicyDiff, } from "./security-policy.js"; export type { + SecurityPolicyApplication, SecurityPolicyDraft, SecurityPolicyOptions, SecurityPolicyPreflight, diff --git a/sdk/typescript/src/runtime.ts b/sdk/typescript/src/runtime.ts index 3efa5dc1..24e59faa 100644 --- a/sdk/typescript/src/runtime.ts +++ b/sdk/typescript/src/runtime.ts @@ -1744,18 +1744,7 @@ export async function importAmbientAuth( await copyFile(source, temporary, constants.COPYFILE_EXCL); await chmod(temporary, 0o600); try { - try { - await link(temporary, destination); - } catch (error) { - if ( - !["EPERM", "ENOTSUP", "EOPNOTSUPP", "EXDEV", "EMLINK"].includes( - nodeErrorCode(error) ?? "", - ) - ) { - throw error; - } - await copyFile(temporary, destination, constants.COPYFILE_EXCL); - } + await installFileNoClobber(temporary, destination); } catch (error) { if ( nodeErrorCode(error) === "EEXIST" && @@ -1779,6 +1768,25 @@ export async function importAmbientAuth( } } +export async function installFileNoClobber( + source: string, + destination: string, +): Promise { + try { + await link(source, destination); + } catch (error) { + // Windows reports unsupported FAT/exFAT hard links as EISDIR. + if ( + !["EPERM", "ENOTSUP", "EOPNOTSUPP", "EXDEV", "EMLINK", "EISDIR"].includes( + nodeErrorCode(error) ?? "", + ) + ) { + throw error; + } + await copyFile(source, destination, constants.COPYFILE_EXCL); + } +} + export async function extractPluginZip( archive: string, destination: string, @@ -1954,7 +1962,7 @@ async function rejectBackslashZipNames( export async function resolvePluginPath( pluginPath: string | undefined, - workspace: string, + workspace: string | (() => Promise), signal?: AbortSignal, ): Promise { if (pluginPath === undefined) { @@ -1964,9 +1972,11 @@ export async function resolvePluginPath( const path = resolve(expandHome(pluginPath)); const metadata = await lstat(path).catch(() => null); if (metadata?.isFile() && extname(path).toLowerCase() === ".zip") { + const extractionRoot = + typeof workspace === "function" ? await workspace() : workspace; return await extractPluginZip( path, - join(workspace, "extracted-plugin"), + join(extractionRoot, "extracted-plugin"), signal, ); } diff --git a/sdk/typescript/src/security-policy-cli.ts b/sdk/typescript/src/security-policy-cli.ts index 3c6e6896..62e0aeec 100644 --- a/sdk/typescript/src/security-policy-cli.ts +++ b/sdk/typescript/src/security-policy-cli.ts @@ -2,10 +2,17 @@ import type { CodexSecurity, ScanAuthMode } from "./api.js"; import type { BulkScanPrompt } from "./bulk-scan-discovery.js"; import type { CodexSecurityConfig } from "./config.js"; import { formatUsd } from "./cost.js"; -import { safeErrorMessage } from "./errors.js"; import { + SecurityPolicyRecoveryError, + SecurityPolicyVerificationError, + safeErrorMessage, +} from "./errors.js"; +import { + applySecurityPolicy, + loadSecurityPolicyDraft, formatSecurityPolicyText as display, securityPolicyDiff, + type SecurityPolicyDraft, type SecurityPolicyOptions, type SecurityPolicyStage, } from "./security-policy.js"; @@ -14,7 +21,10 @@ import { enclosingGitWorktreeRoots } from "./targets.js"; type SignalName = "SIGINT" | "SIGTERM"; type Output = { write(value: string): unknown }; -export type PolicyPrompt = Pick; +export type PolicyPrompt = Pick< + BulkScanPrompt, + "isInteractive" | "input" | "confirm" +>; export type PolicySecurity = Pick< CodexSecurity, "generatePolicy" | "preflightPolicy" | "close" @@ -24,6 +34,8 @@ export interface PolicyCommandOptions { repository: string; config: CodexSecurityConfig; generation: SecurityPolicyOptions; + apply?: string; + write: boolean; headless: boolean; dryRun: boolean; format: string; @@ -73,6 +85,7 @@ export async function runPolicyCommand( const started = dependencies.now(); let security: PolicySecurity | undefined; let outputDir: string | undefined; + let applyingTarget: string | undefined; const write = (message: string): void => { try { errorOutput.write(`${message}\n`); @@ -87,6 +100,10 @@ export async function runPolicyCommand( dependencies.now() - firstSignalAt < 500 ) return; + if (applyingTarget !== undefined) + write( + `Policy application is being stopped. Check ${display(applyingTarget)}${outputDir === undefined ? "" : ` and saved artifacts at ${display(outputDir)}`} for recovery files before retrying.`, + ); removeSignalListeners(); dependencies.forceExit(signal); return; @@ -103,82 +120,90 @@ export async function runPolicyCommand( dependencies.addSignalListener("SIGINT", interrupt); dependencies.addSignalListener("SIGTERM", terminate); try { - const auth = - interactive && !options.dryRun - ? await dependencies.chooseAuthentication( - options.config, - options.generation.auth, - controller.signal, - ) - : options.generation.auth; - controller.signal.throwIfAborted(); - security = dependencies.createSecurity(options.config); - if (options.dryRun) { - const preflight = await security.preflightPolicy(options.repository, { - ...options.generation, + let draft: SecurityPolicyDraft; + if (options.apply !== undefined) { + draft = await loadSecurityPolicyDraft(options.repository, options.apply, { + path: options.generation.path, signal: controller.signal, }); + outputDir = draft.outputDir; + } else { + const auth = + interactive && !options.dryRun + ? await dependencies.chooseAuthentication( + options.config, + options.generation.auth, + controller.signal, + ) + : options.generation.auth; controller.signal.throwIfAborted(); - return { - exitCode: 0, - data: { - ...preflight, - dryRun: true, + security = dependencies.createSecurity(options.config); + if (options.dryRun) { + const preflight = await security.preflightPolicy(options.repository, { + ...options.generation, + signal: controller.signal, + }); + controller.signal.throwIfAborted(); + return { + exitCode: 0, + data: { + ...preflight, + dryRun: true, + }, + }; + } + draft = await security.generatePolicy(options.repository, { + ...options.generation, + auth, + signal: controller.signal, + onOutputDirReady: (directory) => { + outputDir = directory; + write(`Policy artifacts: ${display(directory)}`); }, - }; - } - const draft = await security.generatePolicy(options.repository, { - ...options.generation, - auth, - signal: controller.signal, - onOutputDirReady: (directory) => { - outputDir = directory; - write(`Policy artifacts: ${display(directory)}`); - }, - onStage: (stage) => write(STAGES[stage]), - onWarning: (warning) => - write(`codex-security: ${display(safeErrorMessage(warning))}`), - ...(interactive - ? { - answerQuestions: async ( - questions: readonly string[], - signal: AbortSignal, - ) => { - write( - "A few details could change this policy. Leave an answer blank to keep it unresolved.", - ); - const answers: string[] = []; - for (const question of questions) { - signal.throwIfAborted(); - const answer = await prompt.input( - display(question), - undefined, - signal, + onStage: (stage) => write(STAGES[stage]), + onWarning: (warning) => + write(`codex-security: ${display(safeErrorMessage(warning))}`), + ...(interactive + ? { + answerQuestions: async ( + questions: readonly string[], + signal: AbortSignal, + ) => { + write( + "A few details could change this policy. Leave an answer blank to keep it unresolved.", ); - if (answer.trim()) answers.push(`${question}\n${answer}`); - } - return answers.join("\n\n"); - }, - } - : {}), - }); + const answers: string[] = []; + for (const question of questions) { + signal.throwIfAborted(); + const answer = await prompt.input( + display(question), + undefined, + signal, + ); + if (answer.trim()) answers.push(`${question}\n${answer}`); + } + return answers.join("\n\n"); + }, + } + : {}), + }); + } controller.signal.throwIfAborted(); const cost = draft.cost; + let python: string | undefined; + const resolvePython = async () => + (python ??= await (dependencies.resolvePython ?? resolvePluginPython)({ + configuredPath: options.config.pythonPath, + environment: dependencies.environment, + protectedRoot: + ( + await enclosingGitWorktreeRoots(draft.repository, controller.signal) + ).at(-1) ?? draft.repository, + signal: controller.signal, + })); const diff = await securityPolicyDiff( draft, - async () => - await (dependencies.resolvePython ?? resolvePluginPython)({ - configuredPath: options.config.pythonPath, - environment: dependencies.environment, - protectedRoot: - ( - await enclosingGitWorktreeRoots( - draft.repository, - controller.signal, - ) - ).at(-1) ?? draft.repository, - signal: controller.signal, - }), + resolvePython, controller.signal, ); const changed = diff.length > 0; @@ -196,24 +221,54 @@ export async function runPolicyCommand( ...draft.reviewNotes.map((note) => `- ${display(note)}`), ]), ].join("\n"); - if (interactive) + if (interactive && !options.write) await dependencies.writePreview(`${preview}\n`, controller.signal); else write(preview); } controller.signal.throwIfAborted(); - const status = changed ? "draft" : "unchanged"; - if (humanOutput) { + const approved = + options.write || + (changed && + interactive && + (await prompt.confirm( + `Write this policy to ${display(draft.targetPath)}?`, + false, + controller.signal, + ))); + controller.signal.throwIfAborted(); + let status: "draft" | "written" | "unchanged" = changed + ? "draft" + : "unchanged"; + let recoveryPath: string | null = null; + if (approved) { + applyingTarget = draft.targetPath; + const applied = await applySecurityPolicy(draft, { + pythonPath: python ?? options.config.pythonPath, + pluginPath: options.config.pluginPath, + environment: dependencies.environment, + signal: controller.signal, + }); + recoveryPath = applied.recoveryPath; + status = applied.status; + write( + `${status === "written" ? "Wrote and verified" : "Verified"} ${display(draft.targetPath)}`, + ); + if (recoveryPath !== null) + write(`Previous policy kept at ${display(recoveryPath)}`); + } else if (humanOutput) { write(`\nDraft: ${display(draft.draftPath)}`); write(`Threat model: ${display(draft.threatModelPath)}`); if (changed) write( - "No repository files changed. Review the saved SECURITY.md before copying it into the repository.", + "No repository files changed. Review the draft, then run policy with --apply --write.", ); } - const seconds = Math.max(0, (dependencies.now() - started) / 1000); - write( - `Policy generation finished in ${seconds.toFixed(1)}s${cost === null ? "" : ` (${formatUsd(cost.estimatedUsd)} estimated)`}.`, - ); + if (options.apply === undefined) { + const seconds = Math.max(0, (dependencies.now() - started) / 1000); + write( + `Policy generation finished in ${seconds.toFixed(1)}s${cost === null ? "" : ` (${formatUsd(cost.estimatedUsd)} estimated)`}.`, + ); + } return { exitCode: 0, markdown: draft.content, @@ -222,6 +277,7 @@ export async function runPolicyCommand( repository: draft.repository, scope: draft.scope, targetPath: draft.targetPath, + ...(recoveryPath === null ? {} : { recoveryPath }), outputDir: draft.outputDir, draftPath: draft.draftPath, specificationPath: draft.specificationPath, @@ -232,8 +288,10 @@ export async function runPolicyCommand( }, }; } catch (error) { + const written = error instanceof SecurityPolicyVerificationError; + const recovery = error instanceof SecurityPolicyRecoveryError; const signal = - controller.signal.reason ?? + (written || recovery ? undefined : controller.signal.reason) ?? (error instanceof Error && error.name === "ExitPromptError" ? "SIGINT" : undefined); @@ -250,6 +308,18 @@ export async function runPolicyCommand( return { exitCode, error: message, + ...(written || recovery + ? { + data: { + status: written ? "written_unverified" : "recovery_required", + targetPath: error.targetPath, + ...(error.recoveryPath === undefined + ? {} + : { recoveryPath: error.recoveryPath }), + ...(outputDir === undefined ? {} : { outputDir }), + }, + } + : {}), }; } finally { removeSignalListeners(); diff --git a/sdk/typescript/src/security-policy.ts b/sdk/typescript/src/security-policy.ts index 0597bf9c..a4e07927 100644 --- a/sdk/typescript/src/security-policy.ts +++ b/sdk/typescript/src/security-policy.ts @@ -1,23 +1,43 @@ import { execFile } from "node:child_process"; -import { createHash } from "node:crypto"; +import { createHash, randomUUID } from "node:crypto"; import { constants } from "node:fs"; import { + chmod, + copyFile, lstat, open, + readFile, readdir, readlink, realpath, + rename, + rm, stat, writeFile, } from "node:fs/promises"; +import { tmpdir } from "node:os"; import { basename, dirname, isAbsolute, join, relative, sep } from "node:path"; import { promisify } from "node:util"; import { z } from "incur"; import type { ScanAuthentication, ScanOptions } from "./api.js"; import { jsonForPrompt, pluginPythonCommand } from "./codex-prompt.js"; import type { ScanCost } from "./cost.js"; -import { CodexSecurityError, InvalidTargetError } from "./errors.js"; -import { resolvePluginPython, type ProcessEnvironment } from "./runtime.js"; +import { requireScanFile } from "./contract.js"; +import { + CodexSecurityError, + InvalidTargetError, + SecurityPolicyRecoveryError, + SecurityPolicyVerificationError, +} from "./errors.js"; +import { + cleanupSdkDirectory, + createIsolatedHome, + installFileNoClobber, + requireOutputOutsideRepositories, + resolvePluginPath, + resolvePluginPython, + type ProcessEnvironment, +} from "./runtime.js"; import { abortable, enclosingGitWorktreeRoot, @@ -127,6 +147,12 @@ export interface SecurityPolicyDraft cost: Readonly | null; } +export interface SecurityPolicyApplication { + status: "written" | "unchanged"; + targetPath: string; + recoveryPath: string | null; +} + const execFileAsync = promisify(execFile); const MANIFEST_NAME = "policy-draft.json"; const ORIGINAL_NAME = "previous-SECURITY.md"; @@ -556,6 +582,13 @@ export async function requireUnchangedSecurityPolicy( signal?: AbortSignal, ): Promise { const current = await readSecurityPolicySnapshot(target, signal); + requirePolicySnapshot(current, snapshot); +} + +function requirePolicySnapshot( + current: SecurityPolicySnapshot, + snapshot: SecurityPolicySnapshot, +): void { if (current.previousContent !== snapshot.previousContent) { throw new CodexSecurityError( "SECURITY.md changed after its contents were read. Reconcile the changes and generate a new draft before writing.", @@ -568,6 +601,22 @@ export async function requireUnchangedSecurityPolicy( } } +async function readDraftContent( + target: SecurityPolicyTarget, + draft: SecurityPolicyDraft, + signal?: AbortSignal, +): Promise { + const current = await readSecurityPolicySnapshot(target, signal); + requirePolicySnapshot(current, { + previousContent: + current.previousContent === draft.content + ? draft.content + : draft.previousContent, + inheritedPolicySha256: draft.inheritedPolicySha256, + }); + return current.previousContent; +} + export async function resolveSecurityPolicyGuidance( target: SecurityPolicyTarget, python: string, @@ -721,7 +770,7 @@ export async function runSecurityPolicyStages(options: { "Retain their full repository-relative citations where they support policy decisions; do not shorten nested source paths.", ownerContext, `Threat-model questions and review notes (JSON data): ${jsonForPrompt({ questions: threatModel.questions, reviewNotes: threatModel.reviewNotes })}`, - "Use the define-security-policy skill to draft the complete SECURITY.md for the selected component. This request authorizes a draft only; the host will save it for owner review.", + "Use the define-security-policy skill to draft the complete SECURITY.md for the selected component. This request authorizes a draft only; the host will preview the exact diff and obtain approval before applying it.", "Preserve useful existing guidance, private-reporting instructions, and confirmed owner decisions. Write concise, source-backed scope, trust boundaries, named security invariants, reportability and severity context, owner-confirmed exclusions, limitations, and open decisions. Do not copy the full threat model, exploit narratives, or private artifact paths into SECURITY.md.", "Mark new or changed policy decisions as requiring owner review. Never turn an assumption or missing evidence into permission to suppress findings. List new exclusions, accepted risks, severity changes, and material unanswered questions in reviewNotes.", ].join("\n"), @@ -781,6 +830,65 @@ export async function runSecurityPolicyStages(options: { }; } +export async function loadSecurityPolicyDraft( + repository: string, + outputDir: string, + options: Pick = {}, +): Promise { + const target = await resolveSecurityPolicyTarget( + repository, + options.path, + options.signal, + ); + const manifestPath = await requireScanFile( + outputDir, + MANIFEST_NAME, + MANIFEST_NAME, + options.signal, + ); + const directory = dirname(manifestPath); + const file = (name: string) => + requireScanFile(directory, name, name, options.signal); + const manifest = manifestSchema.parse( + JSON.parse(await readFile(manifestPath, "utf8")), + ); + if ( + manifest.repository !== target.repository || + manifest.scope !== target.scope + ) { + throw new CodexSecurityError( + "The saved policy draft belongs to a different repository or component. Select its original target explicitly.", + ); + } + const originalPath = await file(ORIGINAL_NAME); + const original = await readPolicyFile(originalPath); + if ( + manifest.previousPolicySha256 === null + ? original !== "" + : digest(original) !== manifest.previousPolicySha256 + ) { + throw new CodexSecurityError( + "The saved policy's original-content checkpoint has changed.", + ); + } + const draftPath = await file("SECURITY.md"); + const content = await readPolicyFile(draftPath); + validatePolicyContent(content); + return { + ...target, + outputDir: directory, + draftPath, + specificationPath: await file("project-spec.md"), + threatModelPath: await file("THREAT_MODEL.md"), + content, + previousContent: manifest.previousPolicySha256 === null ? null : original, + inheritedPolicySha256: manifest.inheritedPolicySha256, + customPlugin: manifest.customPlugin, + reviewNotes: manifest.reviewNotes, + cost: null, + }; +} + /** Raw unified diff. A Python resolver is called only when there is a change. * Use CodexSecurity.previewPolicy() for terminal output. */ export async function securityPolicyDiff( @@ -790,8 +898,8 @@ export async function securityPolicyDiff( ): Promise { draft = { ...draft }; const target = await resolveDraftTarget(draft, signal); - await requireUnchangedSecurityPolicy(target, draft, signal); - if (draft.previousContent === draft.content) return ""; + if ((await readDraftContent(target, draft, signal)) === draft.content) + return ""; const selectedPython = typeof python === "function" ? await python() : python; const interpreter = selectedPython ?? @@ -835,12 +943,331 @@ export async function securityPolicyDiff( ]), ); }); - await requireUnchangedSecurityPolicy( + const current = await readDraftContent( await resolveDraftTarget(draft, signal), draft, signal, ); - return diff; + return current === draft.content ? "" : diff; +} + +async function validatePolicyLinks( + target: SecurityPolicyTarget, + signal?: AbortSignal, +): Promise { + const repositories = await enclosingGitWorktreeRoots( + target.repository, + signal, + ); + if (repositories.length === 0) repositories.push(target.repository); + const protectedRoot = repositories.at(-1)!; + const component = dirname(target.targetPath); + const canonicalTarget = await realpath(target.targetPath).catch( + (error: NodeJS.ErrnoException) => { + if (error.code === "ENOENT") return null; + throw error; + }, + ); + for await (const entry of securityPolicyPaths( + protectedRoot, + repositories, + signal, + )) { + if (!entry.reportingPolicy && !entry.isSymbolicLink) continue; + const boundary = repositories.find( + (root) => !relativePathIsOutside(relative(root, entry.path)), + )!; + const alias = await policyLinkSnapshot(entry.path, boundary, signal); + const destination = await policyLinkDestination(boundary, alias); + const reportingPolicy = + entry.reportingPolicy && entry.path !== target.targetPath; + const outsideScope = + entry.repository !== target.repository || + relativePathIsOutside(relative(component, dirname(entry.path))); + if ( + (outsideScope || reportingPolicy) && + destination !== null && + policyPathsMatch( + canonicalTarget ?? target.targetPath, + destination, + canonicalTarget === null && alias.status === "missing", + ) + ) { + const policyPath = relative(protectedRoot, entry.path) + .split(sep) + .join("/"); + throw new CodexSecurityError( + `SECURITY.md ${JSON.stringify(policyPath)} points to the selected policy and would change ${reportingPolicy ? "a separate vulnerability-reporting policy" : "guidance outside the selected component"}. Fix the link before applying a policy.`, + ); + } + } +} + +export async function applySecurityPolicy( + draft: SecurityPolicyDraft, + options: { + pythonPath?: string; + pluginPath?: string; + environment?: ProcessEnvironment; + signal?: AbortSignal; + } = {}, +): Promise { + draft = { ...draft }; + validatePolicyContent(draft.content); + const target = await resolveDraftTarget(draft, options.signal); + const alreadyApplied = + (await readSecurityPolicy(target.targetPath)) === draft.content; + let written = alreadyApplied && draft.previousContent !== draft.content; + let recoveryPath: string | null = null; + let pluginWorkspace: string | undefined; + try { + await readDraftContent(target, draft, options.signal); + if (draft.previousContent === draft.content) + return { + status: "unchanged", + targetPath: target.targetPath, + recoveryPath: null, + }; + await validatePolicyLinks(target, options.signal); + const protectedRoots = await securityPolicyProtectedRoots( + target.repository, + options.signal, + ); + const protectedRoot = protectedRoots[0]!; + const recoveryDirectory = + alreadyApplied || draft.previousContent === null + ? null + : dirname( + await requireScanFile( + draft.outputDir, + MANIFEST_NAME, + MANIFEST_NAME, + options.signal, + ), + ); + if (recoveryDirectory !== null) + requireOutputOutsideRepositories(protectedRoots, recoveryDirectory); + const pluginPath = options.pluginPath ?? draft.pluginPath; + if (draft.customPlugin && pluginPath === undefined) { + throw new CodexSecurityError( + "This draft used a custom plugin. Select it explicitly with --plugin-path or the SDK's pluginPath option before applying.", + ); + } + const python = await resolvePluginPython({ + configuredPath: options.pythonPath, + environment: options.environment, + protectedRoot, + signal: options.signal, + }); + const pluginRoot = await resolvePluginPath( + pluginPath, + async () => { + const temporaryRoot = await realpath(tmpdir()); + requireOutputOutsideRepositories( + protectedRoots, + temporaryRoot, + "temporary", + ); + pluginWorkspace = await createIsolatedHome(temporaryRoot, (path) => + requireOutputOutsideRepositories(protectedRoots, path, "runtime"), + ); + return pluginWorkspace; + }, + options.signal, + ); + if (!alreadyApplied) { + await resolveSecurityPolicyGuidance( + target, + python, + pluginRoot, + options.environment, + options.signal, + ); + options.signal?.throwIfAborted(); + const temporary = join( + dirname(target.targetPath), + `.SECURITY.md.${randomUUID()}.tmp`, + ); + try { + await writeFile(temporary, draft.content, { + flag: "wx", + mode: draft.previousContent === null ? 0o644 : 0o600, + signal: options.signal, + }); + if ( + (await realpath(dirname(target.targetPath))) !== + dirname(target.targetPath) + ) { + throw new CodexSecurityError( + "The security-policy destination changed. Review a new draft before writing.", + ); + } + await resolveDraftTarget(draft, options.signal); + await validatePolicyLinks(target, options.signal); + await requireUnchangedSecurityPolicy(target, draft, options.signal); + options.signal?.throwIfAborted(); + if (draft.previousContent === null) { + try { + await installPolicyFile(temporary, target.targetPath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "EEXIST") { + // A failed copy fallback can leave a partial destination. + written = + (await lstat(target.targetPath).catch( + (inspectError: NodeJS.ErrnoException) => { + if ( + inspectError.code === "ENOENT" || + inspectError.code === "ENOTDIR" + ) + return null; + throw inspectError; + }, + )) !== null; + } + throw error; + } + } else + recoveryPath = await replaceExistingPolicy( + temporary, + target.targetPath, + draft.previousContent, + recoveryDirectory!, + options.signal, + ); + written = true; + if (recoveryPath !== null) + recoveryPath = await retainPolicyRecovery( + recoveryPath, + recoveryDirectory!, + ); + } finally { + // Preserve the write or recovery outcome if temporary cleanup fails. + await rm(temporary, { force: true }).catch(() => undefined); + } + } + // SDK cancellation must not skip post-write checks. Process interruption + // can still leave a written policy that needs verification on retry. + if ((await readSecurityPolicy(target.targetPath)) !== draft.content) { + throw new CodexSecurityError( + "The written policy contents do not match the reviewed draft.", + ); + } + if ( + recoveryPath !== null && + (await readSecurityPolicy(recoveryPath)) !== draft.previousContent + ) { + throw new CodexSecurityError( + "The previous SECURITY.md changed while the replacement was being installed.", + ); + } + await resolveSecurityPolicyGuidance( + target, + python, + pluginRoot, + options.environment, + ); + await resolveDraftTarget(draft); + await validatePolicyLinks(target); + await requireUnchangedSecurityPolicy(target, { + previousContent: draft.content, + inheritedPolicySha256: draft.inheritedPolicySha256, + }); + return { + status: alreadyApplied ? "unchanged" : "written", + targetPath: target.targetPath, + recoveryPath, + }; + } catch (error) { + if (written) + throw new SecurityPolicyVerificationError(target.targetPath, { + cause: error, + ...(recoveryPath === null ? {} : { recoveryPath }), + }); + throw error; + } finally { + if (pluginWorkspace !== undefined) + await cleanupSdkDirectory(pluginWorkspace).catch(() => undefined); + } +} + +async function installPolicyFile( + temporary: string, + targetPath: string, +): Promise { + // Windows may make a read-only file writable before removing it. Keep the + // temporary inode separate so cleanup cannot change the installed mode. + if (((await stat(temporary)).mode & 0o200) === 0) + await copyFile(temporary, targetPath, constants.COPYFILE_EXCL); + else await installFileNoClobber(temporary, targetPath); +} + +async function replaceExistingPolicy( + temporary: string, + targetPath: string, + previousContent: string, + recoveryDirectory: string, + signal?: AbortSignal, +): Promise { + const recoveryPath = `${temporary}.previous`; + await writeFile(recoveryPath, "", { flag: "wx", mode: 0o600 }); + try { + signal?.throwIfAborted(); + // Check the displaced file, then install without replacing a newer save. + await rename(targetPath, recoveryPath); + } catch (error) { + await rm(recoveryPath, { force: true }).catch(() => undefined); + throw error; + } + try { + if ((await readSecurityPolicy(recoveryPath)) !== previousContent) { + throw new CodexSecurityError( + "SECURITY.md changed while the policy was being applied. Review a new draft before writing.", + ); + } + const mode = (await stat(recoveryPath)).mode & 0o777; + await chmod(temporary, mode); + signal?.throwIfAborted(); + await installPolicyFile(temporary, targetPath); + } catch (error) { + let cause = error; + try { + const metadata = await lstat(recoveryPath); + if (!metadata.isFile() || metadata.isSymbolicLink()) { + throw new CodexSecurityError( + "The recovery path is not a regular file.", + ); + } + await installFileNoClobber(recoveryPath, targetPath); + } catch (restoreError) { + cause = new AggregateError([error, restoreError]); + } + throw new SecurityPolicyRecoveryError( + targetPath, + await retainPolicyRecovery(recoveryPath, recoveryDirectory), + { cause }, + ); + } + return recoveryPath; +} + +async function retainPolicyRecovery( + recoveryPath: string, + directory: string, +): Promise { + const retained = join(directory, `recovery-SECURITY-${randomUUID()}.md`); + try { + await writeFile(retained, "", { flag: "wx", mode: 0o600 }); + } catch { + return recoveryPath; + } + try { + // Preserve the inode: copying it would lose writes through an open handle. + await rename(recoveryPath, retained); + return retained; + } catch { + await rm(retained, { force: true }).catch(() => undefined); + return recoveryPath; + } } export function formatSecurityPolicyText( diff --git a/sdk/typescript/tests-ts/api-policy.test.ts b/sdk/typescript/tests-ts/api-policy.test.ts index 7c2f846a..ec980068 100644 --- a/sdk/typescript/tests-ts/api-policy.test.ts +++ b/sdk/typescript/tests-ts/api-policy.test.ts @@ -17,7 +17,9 @@ import type { import Ajv, { type AnySchema } from "ajv"; import { afterEach, describe, expect, test } from "bun:test"; import { + applySecurityPolicy, CodexSecurity, + loadSecurityPolicyDraft, OutputDirectoryNotEmptyError, securityPolicyDiff, writeCodexConfig, @@ -575,7 +577,7 @@ describe("CodexSecurity policy API", () => { await f.security.close(); }); - test("keeps literal component names intact through generation and preview", async () => { + test("keeps literal component names intact through generation and apply", async () => { for (const scope of ["-component", "~component", "~", "~/child"]) { let prepared = false; const f = await setup({ @@ -597,10 +599,14 @@ describe("CodexSecurity policy API", () => { const generated = await f.security.generatePolicy(f.repository, options); expect(generated.scope).toBe(scope); expect(f.prompts[0]).toContain("Inherited guidance."); - expect(await securityPolicyDiff(generated, PYTHON)).toContain( + const saved = await loadSecurityPolicyDraft(f.repository, f.outputDir, { + path: options.path, + }); + expect(await securityPolicyDiff(saved, PYTHON)).toContain( `b/${scope}/SECURITY.md`, ); - expect(await readdir(component)).toEqual([]); + await applySecurityPolicy(saved, { pythonPath: PYTHON }); + expect(await readFile(saved.targetPath, "utf8")).toBe(POLICY); await f.security.close(); } }); diff --git a/sdk/typescript/tests-ts/cli-policy.test.ts b/sdk/typescript/tests-ts/cli-policy.test.ts index 29af8d52..8232fb66 100644 --- a/sdk/typescript/tests-ts/cli-policy.test.ts +++ b/sdk/typescript/tests-ts/cli-policy.test.ts @@ -6,10 +6,15 @@ import { symlink, writeFile, } from "node:fs/promises"; +import * as fsPromises from "node:fs/promises"; import { delimiter, dirname, join } from "node:path"; import { Writable } from "node:stream"; -import { afterEach, describe, expect, test } from "bun:test"; +import { afterEach, describe, expect, mock, test } from "bun:test"; import { main } from "../src/cli.js"; +import { + SecurityPolicyRecoveryError, + SecurityPolicyVerificationError, +} from "../src/errors.js"; import type { SecurityPolicyDraft, SecurityPolicyOptions, @@ -17,12 +22,14 @@ import type { import type { PolicyPrompt } from "../src/security-policy-cli.js"; import { resolvePluginPython } from "../src/runtime.js"; import { capture, dependencies, FakeSignals } from "./cli-fixtures.js"; +import { runMockInSubprocess } from "./support/isolated-mock.js"; import { POLICY, PYTHON, addPolicySubmodule, policyFixture, policyGit, + policyPlugin, stageResult, } from "./support/security-policy.js"; @@ -42,6 +49,9 @@ function prompt(overrides: Partial = {}): PolicyPrompt { input: async () => { throw new Error("Unexpected input prompt"); }, + confirm: async () => { + throw new Error("Unexpected confirmation"); + }, ...overrides, }; } @@ -68,6 +78,7 @@ function policyDependencies( ...dependencies({ currentDirectory: f.repository, signals: options.signals, + environment: { PYTHON }, }), policyPrompt: options.prompt ?? prompt(), resolvePolicyPython: async () => PYTHON, @@ -129,13 +140,13 @@ describe("policy CLI", () => { ), ).toBe(0); expect(stdout.text()).toContain("SECURITY.md"); - expect(stdout.text()).not.toContain("--apply"); - expect(stdout.text()).not.toContain("--write"); + expect(stdout.text()).toContain("--apply"); + expect(stdout.text()).toContain("--write"); expect(stdout.text()).toContain("--headless"); expect(stdout.text()).not.toContain("--outputDir"); expect(stdout.text()).not.toContain("--write true"); expect(stdout.text()).toContain( - "--headless --output-dir /path/outside/repository/policy --json", + "--apply /path/outside/repository/policy --write", ); }); @@ -200,6 +211,7 @@ describe("policy CLI", () => { draft, prompt: prompt({ isInteractive: () => true, + confirm: async () => false, }), onGenerate: (_repository, options) => { selected = options.auth; @@ -233,7 +245,7 @@ describe("policy CLI", () => { expect(await readdir(f.repository)).toEqual([]); }); - test("does not choose credentials for automated, explicit policy requests", async () => { + test("does not choose credentials for automated, explicit, or saved policy requests", async () => { const f = await fixture(); const draft = await f.generate(); for (const scenario of [ @@ -244,6 +256,7 @@ describe("policy CLI", () => { { args: ["--auth", "chatgpt"] }, { args: ["--auth", "api-key"] }, { args: ["--provider", "openrouter", "--model", "vendor/model"] }, + { args: ["--apply", f.outputDir] }, { args: [], ci: true }, { args: [], stored: false }, { args: [], key: false }, @@ -255,6 +268,7 @@ describe("policy CLI", () => { draft, prompt: prompt({ isInteractive: () => scenario.inputInteractive !== false, + confirm: async () => false, }), }); deps.environment = { @@ -423,12 +437,13 @@ describe("policy CLI", () => { expect(JSON.parse(stdout.text()).status).toBe("draft"); }); - test("reports a failed interactive preview without changing source", async () => { + test("does not offer an interactive write if the diff preview fails", async () => { const f = await fixture(); - const draft = await f.generate(); + await f.generate(); + let asked = false; expect( await main( - ["policy"], + ["policy", "--apply", f.outputDir], capture(true).stream, { isTTY: true, @@ -437,11 +452,17 @@ describe("policy CLI", () => { }, }, policyDependencies(f, { - draft, - prompt: prompt({ isInteractive: () => true }), + prompt: prompt({ + isInteractive: () => true, + confirm: async () => { + asked = true; + return true; + }, + }), }), ), ).toBe(2); + expect(asked).toBe(false); expect(await readdir(f.repository)).toEqual([]); }); @@ -491,7 +512,7 @@ describe("policy CLI", () => { } }); - test("asks owner questions and previews the exact draft without writing source", async () => { + test("offers source-backed questions and shows the exact diff before approval", async () => { const f = await fixture(); const stderr = capture(true); let asked = 0; @@ -508,19 +529,46 @@ describe("policy CLI", () => { expect(question).toContain("internet-facing"); return "Private service"; }, + confirm: async (_question, defaultValue) => { + expect(defaultValue).toBe(false); + expect(stderr.text()).toContain("--- /dev/null"); + expect(stderr.text()).toContain("+Requests must be authorized"); + expect(stderr.text()).toContain("Owner review:"); + expect(await readdir(f.repository)).toEqual([]); + return true; + }, }), }), ), ).toBe(0); expect(asked).toBe(1); - expect(stderr.text()).toContain("--- /dev/null"); - expect(stderr.text()).toContain("+Requests must be authorized"); - expect(stderr.text()).toContain("Owner review:"); - expect(stderr.text()).toContain("No repository files changed"); - expect(await readFile(join(f.outputDir, "SECURITY.md"), "utf8")).toBe( + expect(await readFile(join(f.repository, "SECURITY.md"), "utf8")).toBe( POLICY, ); + expect(stderr.text()).toContain("Wrote and verified"); + }); + + test("declining approval leaves the policy draft available", async () => { + const f = await fixture(); + const draft = await f.generate(); + const stderr = capture(true); + expect( + await main( + ["policy"], + capture(true).stream, + stderr.stream, + policyDependencies(f, { + draft, + prompt: prompt({ + isInteractive: () => true, + confirm: async () => false, + }), + }), + ), + ).toBe(0); expect(await readdir(f.repository)).toEqual([]); + expect(stderr.text()).toContain("No repository files changed"); + expect(await readFile(draft.draftPath, "utf8")).toBe(POLICY); }); test("preserves significant trailing spaces in the proposed diff", async () => { @@ -543,6 +591,7 @@ describe("policy CLI", () => { draft, prompt: prompt({ isInteractive: () => true, + confirm: async () => false, }), }), ), @@ -550,6 +599,250 @@ describe("policy CLI", () => { expect(stderr.text()).toContain("+Last line \n"); }); + test("uses the selected plugin when approving a generated policy", async () => { + const f = await fixture(); + const log = join(f.root, "resolver.log"); + const pluginPath = await policyPlugin( + f.root, + [ + "import os, pathlib", + "with pathlib.Path(os.environ['POLICY_TEST_LOG']).open('a') as output: output.write('used\\n')", + "print('custom guidance')", + ].join("\n"), + ); + const draft = await f.generate({ pluginPath }); + const deps = policyDependencies(f, { + draft, + prompt: prompt({ isInteractive: () => true, confirm: async () => true }), + }); + deps.environment = { ...deps.environment, POLICY_TEST_LOG: log }; + expect( + await main( + ["policy", "--plugin-path", pluginPath], + capture(true).stream, + capture(true).stream, + deps, + ), + ).toBe(0); + expect((await readFile(log, "utf8")).trimEnd().split(/\r?\n/u)).toEqual([ + "used", + "used", + ]); + expect(await readFile(draft.targetPath, "utf8")).toBe(POLICY); + }); + + test("applies a reviewed, edited saved draft without initializing Codex", async () => { + const f = await fixture(); + await mkdir(join(f.repository, "component")); + const draft = await f.generate({ path: "component" }); + const edited = `${POLICY}\nReviewed by the component owner.\n`; + await writeFile(draft.draftPath, edited); + const stdout = capture(); + const deps = policyDependencies(f); + deps.createPolicySecurity = () => { + throw new Error("Must not initialize Codex for --apply"); + }; + expect( + await main( + [ + "policy", + ".", + "--path", + "component", + "--apply", + f.outputDir, + "--write", + "--json", + ], + stdout.stream, + capture().stream, + deps, + ), + ).toBe(0); + expect(JSON.parse(stdout.text()).status).toBe("written"); + expect(await readFile(draft.targetPath, "utf8")).toBe(edited); + }); + + test("reports the retained previous file after updating a policy", async () => { + const f = await fixture(); + const original = "# Original policy\n"; + await writeFile(join(f.repository, "SECURITY.md"), original); + const draft = await f.generate(); + const stdout = capture(); + const stderr = capture(); + expect( + await main( + ["policy", "--apply", f.outputDir, "--write", "--json"], + stdout.stream, + stderr.stream, + policyDependencies(f), + ), + ).toBe(0); + const result = JSON.parse(stdout.text()); + expect(result.status).toBe("written"); + expect(dirname(result.recoveryPath)).toBe(f.outputDir); + expect(stderr.text()).toContain(result.recoveryPath); + expect(await readFile(result.recoveryPath, "utf8")).toBe(original); + expect(await readFile(draft.targetPath, "utf8")).toBe(POLICY); + expect(await readdir(f.repository)).toEqual(["SECURITY.md"]); + }); + + test("reports a written policy when verification fails after cancellation", async () => { + const name = + "reports a written policy when verification fails after cancellation"; + if (runMockInSubprocess(import.meta.path, name)) return; + const f = await fixture(); + const pluginPath = await policyPlugin( + f.root, + [ + "import pathlib, sys", + "root = pathlib.Path(sys.argv[sys.argv.index('--repo') + 1])", + "if (root / 'SECURITY.md').exists(): raise SystemExit('synthetic verification failure')", + "print('preflight passed')", + ].join("\n"), + ); + const draft = await f.generate({ pluginPath }); + const signals = new FakeSignals(); + const deps = policyDependencies(f, { signals }); + deps.createPolicySecurity = () => { + throw new Error("Must not initialize Codex for --apply"); + }; + const originalLink = fsPromises.link; + mock.module("node:fs/promises", () => ({ + ...fsPromises, + link: async (source: string, destination: string) => { + await originalLink(source, destination); + if (destination === draft.targetPath) signals.emit("SIGINT"); + }, + })); + try { + const stdout = capture(); + const stderr = capture(); + expect( + await main( + [ + "policy", + "--apply", + f.outputDir, + "--plugin-path", + pluginPath, + "--write", + "--json", + ], + stdout.stream, + stderr.stream, + deps, + ), + ).toBe(2); + expect(JSON.parse(stdout.text())).toMatchObject({ + status: "written_unverified", + targetPath: draft.targetPath, + }); + expect(stderr.text()).toContain("was written"); + expect(stderr.text()).not.toContain("canceled by Ctrl-C"); + expect(await readFile(draft.targetPath, "utf8")).toBe(POLICY); + for (const flags of [ + [], + [ + "--plugin-path", + pluginPath, + "--python", + join(f.root, "missing-python"), + ], + ]) { + const failedRetry = capture(); + expect( + await main( + ["policy", "--apply", f.outputDir, "--write", "--json", ...flags], + failedRetry.stream, + capture().stream, + { + ...policyDependencies(f), + resolvePolicyPython: resolvePluginPython, + }, + ), + ).toBe(2); + expect(JSON.parse(failedRetry.text())).toMatchObject({ + status: "written_unverified", + targetPath: draft.targetPath, + }); + } + await writeFile( + join(pluginPath, "scripts", "resolve_security_md.py"), + "print('resolver accepted the policy')\n", + ); + const retry = capture(); + expect( + await main( + [ + "policy", + "--apply", + f.outputDir, + "--plugin-path", + pluginPath, + "--write", + "--json", + ], + retry.stream, + capture().stream, + policyDependencies(f), + ), + ).toBe(0); + expect(JSON.parse(retry.text()).status).toBe("unchanged"); + expect(await readFile(draft.targetPath, "utf8")).toBe(POLICY); + } finally { + mock.module("node:fs/promises", () => ({ + ...fsPromises, + link: originalLink, + })); + } + }); + + test("rejects writing an unseen model-generated policy", async () => { + const f = await fixture(); + const stderr = capture(); + let generated = false; + expect( + await main( + ["policy", "--write"], + capture().stream, + stderr.stream, + policyDependencies(f, { + onGenerate: () => { + generated = true; + }, + }), + ), + ).toBe(2); + expect(stderr.text()).toContain("--write requires --apply"); + expect(generated).toBe(false); + }); + + test("does not silently ignore generation options when applying a saved draft", async () => { + const f = await fixture(); + const deps = policyDependencies(f); + deps.createPolicySecurity = () => { + throw new Error("Must not initialize Codex for --apply"); + }; + for (const option of [ + ["--model", "gpt-5.6-terra"], + ["--auth", "chatgpt"], + ["--provider", "fireworks"], + ["--output-dir", f.outputDir], + ]) { + const stderr = capture(); + expect( + await main( + ["policy", "--apply", f.outputDir, ...option], + capture().stream, + stderr.stream, + deps, + ), + ).toBe(2); + expect(stderr.text()).toContain("generation options"); + } + }); + test("preflights without generation or Python discovery", async () => { const f = await fixture(); const stdout = capture(); @@ -580,10 +873,10 @@ describe("policy CLI", () => { f.repository, join(f.root, "submodule-source"), ); - const draft = await f.generate({ path: "services/api" }); + await f.generate({ path: "services/api" }); const protectedRoots: (string | undefined)[] = []; const deps = { - ...policyDependencies(f, { draft }), + ...policyDependencies(f), resolvePolicyPython: async ( options: Parameters[0], ) => { @@ -597,7 +890,15 @@ describe("policy CLI", () => { ] as const) { expect( await main( - ["policy", repository, "--path", path, "--json"], + [ + "policy", + repository, + "--path", + path, + "--apply", + f.outputDir, + "--json", + ], capture().stream, capture().stream, deps, @@ -619,7 +920,7 @@ describe("policy CLI", () => { f.repository, join(f.root, "submodule-source"), ); - const draft = await f.generate({ path: "services/api" }); + await f.generate({ path: "services/api" }); const unsafeBin = join(f.repository, ".venv", "bin"); const trustedBin = join(f.root, "trusted-bin"); const unsafePython = join(unsafeBin, "python3"); @@ -634,7 +935,7 @@ describe("policy CLI", () => { for (const explicit of [false, true]) { const stdout = capture(); const deps = { - ...policyDependencies(f, { draft }), + ...policyDependencies(f), environment: { PATH: [unsafeBin, trustedBin].join(delimiter), ...(explicit ? { PYTHON: unsafePython } : {}), @@ -645,7 +946,7 @@ describe("policy CLI", () => { await resolvePluginPython({ ...options, managedRuntimeRoots: [] }), }; const code = await main( - ["policy", nested, "--json", "--full-output"], + ["policy", nested, "--apply", f.outputDir, "--json", "--full-output"], stdout.stream, capture().stream, deps, @@ -920,9 +1221,13 @@ describe("policy CLI", () => { throw new Error("Validation must finish before initializing Codex"); }; for (const [args, message] of [ - [["policy", "--write"], "Unknown flag"], + [["policy", "--write"], "--write requires --apply"], + [ + ["policy", "--apply", f.outputDir, "--model", "synthetic-model"], + "--apply cannot be combined", + ], [["policy", "--path"], "Missing value"], - [["policy", "--path", "--headless"], "Missing value"], + [["policy", "--path", "--write"], "Missing value"], [["policy", ".", "extra"], "Unexpected positional argument"], [["policy", "--unknown-policy-option"], "Unknown flag"], [["policy", "--max-cost", "0"], "Too small"], @@ -949,6 +1254,66 @@ describe("policy CLI", () => { expect(await readdir(f.repository)).toEqual([]); }); + test("preserves recovery records and useful sanitized error causes", async () => { + const f = await fixture(); + const targetPath = join(f.repository, "SECURITY.md"); + const recoveryPath = join(f.outputDir, "recovery-SECURITY.md"); + for (const [cause, diagnostic] of [ + [ + new Error("synthetic resolver unavailable"), + "synthetic resolver unavailable", + ], + [new Error("api_key=synthetic-test-value"), "[redacted]"], + ] as const) { + for (const [error, status] of [ + [ + new SecurityPolicyVerificationError(targetPath, { + recoveryPath, + cause, + }), + "written_unverified", + ], + [ + new SecurityPolicyRecoveryError(targetPath, recoveryPath, { cause }), + "recovery_required", + ], + ] as const) { + const deps = policyDependencies(f, { + onGenerate: () => { + throw error; + }, + }); + for (const fullOutput of [false, true]) { + const stdout = capture(); + const stderr = capture(); + expect( + await main( + ["policy", "--json", ...(fullOutput ? ["--full-output"] : [])], + stdout.stream, + stderr.stream, + deps, + ), + ).toBe(2); + expect(stderr.text()).toContain(diagnostic); + expect(stdout.text() + stderr.text()).not.toContain( + "synthetic-test-value", + ); + const result = JSON.parse(stdout.text()); + if (fullOutput) { + expect(result).toMatchObject({ + ok: false, + error: { code: "POLICY_FAILED", message: error.message }, + }); + expect(result.error.message).toContain(diagnostic); + expect(result).not.toHaveProperty("data"); + } else { + expect(result).toMatchObject({ status, targetPath, recoveryPath }); + } + } + } + } + }); + test("returns a full-output error when policy setup fails", async () => { const f = await fixture(); const deps = policyDependencies(f); @@ -977,6 +1342,116 @@ describe("policy CLI", () => { }); }); + test("keeps the written and previous files after a full-output verification error", async () => { + const f = await fixture(); + const original = "# Original policy\n"; + await writeFile(join(f.repository, "SECURITY.md"), original); + const pluginPath = await policyPlugin( + f.root, + [ + "import pathlib, sys", + "root = pathlib.Path(sys.argv[sys.argv.index('--repo') + 1])", + "if (root / 'SECURITY.md').read_text() != '# Original policy\\n': raise SystemExit('synthetic verification failure')", + "print('preflight passed')", + ].join("\n"), + ); + const draft = await f.generate({ pluginPath }); + const stdout = capture(); + expect( + await main( + [ + "policy", + "--apply", + f.outputDir, + "--plugin-path", + pluginPath, + "--write", + "--json", + "--full-output", + ], + stdout.stream, + capture().stream, + policyDependencies(f), + ), + ).toBe(2); + const result = JSON.parse(stdout.text()); + expect(result).toMatchObject({ + ok: false, + error: { code: "POLICY_FAILED" }, + }); + expect(result.error.message).toContain(draft.targetPath); + const recovery = (await readdir(f.outputDir)).find((name) => + name.startsWith("recovery-SECURITY-"), + ); + expect(recovery).toBeDefined(); + const recoveryPath = join(f.outputDir, recovery!); + expect(result.error.message).toContain(recoveryPath); + expect(await readFile(recoveryPath, "utf8")).toBe(original); + expect(await readFile(draft.targetPath, "utf8")).toBe(POLICY); + }); + + test("reports an unchanged saved policy without starting Codex or Python", async () => { + for (const alreadyApplied of [false, true]) { + const f = await fixture(); + const target = join(f.repository, "SECURITY.md"); + if (!alreadyApplied) await writeFile(target, POLICY); + await f.generate(); + if (alreadyApplied) await writeFile(target, POLICY); + const stdout = capture(); + const deps = policyDependencies(f); + deps.createPolicySecurity = () => { + throw new Error("Must not initialize Codex for --apply"); + }; + deps.resolvePolicyPython = async () => { + throw new Error("Must not resolve Python for an unchanged preview"); + }; + expect( + await main( + [ + "policy", + "--apply", + f.outputDir, + ...(alreadyApplied ? [] : ["--write"]), + "--python", + "missing-python", + "--json", + ], + stdout.stream, + capture().stream, + deps, + ), + ).toBe(0); + expect(JSON.parse(stdout.text()).status).toBe("unchanged"); + expect(await readFile(target, "utf8")).toBe(POLICY); + } + }); + + test("does not overwrite source edited during the confirmation", async () => { + const f = await fixture(); + const draft = await f.generate(); + const stderr = capture(true); + expect( + await main( + ["policy", "--apply", f.outputDir], + capture(true).stream, + stderr.stream, + policyDependencies(f, { + prompt: prompt({ + isInteractive: () => true, + confirm: async () => { + await writeFile(draft.targetPath, "# Concurrent change\n"); + return true; + }, + }), + }), + ), + ).toBe(2); + expect(stderr.text()).toContain("changed after"); + expect(await readFile(draft.targetPath, "utf8")).toBe( + "# Concurrent change\n", + ); + }); + test("renders terminal controls visibly without changing reviewed bytes", async () => { const f = await fixture(); const controls = @@ -989,10 +1464,10 @@ describe("policy CLI", () => { const stderr = capture(); expect( await main( - ["policy", "--path", scope], + ["policy", "--path", scope, "--apply", f.outputDir, "--write"], capture().stream, stderr.stream, - policyDependencies(f, { draft: { ...draft, content: controlled } }), + policyDependencies(f), ), ).toBe(0); expect(stderr.text()).not.toContain("\u001b"); @@ -1002,8 +1477,7 @@ describe("policy CLI", () => { expect(stderr.text()).toContain( `\\u${character.charCodeAt(0).toString(16).padStart(4, "0")}`, ); - expect(await readFile(draft.draftPath, "utf8")).toBe(controlled); - expect(await readdir(dirname(draft.targetPath))).toEqual([]); + expect(await readFile(draft.targetPath, "utf8")).toBe(controlled); }); test("returns the interrupt exit code and removes signal listeners", async () => { @@ -1036,18 +1510,140 @@ describe("policy CLI", () => { expect(await readdir(f.repository)).toEqual([]); }); + test("lets a later interrupt escape post-write verification", async () => { + const name = "lets a later interrupt escape post-write verification"; + if (runMockInSubprocess(import.meta.path, name)) return; + const f = await fixture(); + const draft = await f.generate(); + const signals = new FakeSignals(); + const forced: string[] = []; + let now = 0; + const deps = policyDependencies(f, { signals }); + deps.now = () => now; + deps.forceExit = (signal) => { + expect(signals.listeners.get("SIGINT")?.size).toBe(0); + expect(signals.listeners.get("SIGTERM")?.size).toBe(0); + forced.push(signal); + }; + const originalLink = fsPromises.link; + mock.module("node:fs/promises", () => ({ + ...fsPromises, + link: async (source: string, destination: string) => { + await originalLink(source, destination); + if (destination !== draft.targetPath) return; + signals.emit("SIGINT"); + signals.emit("SIGINT"); + expect(forced).toEqual([]); + now = 1_000; + signals.emit("SIGINT"); + }, + })); + try { + const stderr = capture(); + expect( + await main( + ["policy", "--apply", f.outputDir, "--write", "--json"], + capture().stream, + stderr.stream, + deps, + ), + ).toBe(0); + expect(forced).toEqual(["SIGINT"]); + expect(stderr.text()).toContain("recovery files before retrying"); + expect(await readFile(draft.targetPath, "utf8")).toBe(POLICY); + } finally { + mock.module("node:fs/promises", () => ({ + ...fsPromises, + link: originalLink, + })); + } + }); + + test("reports recovery paths even when a conflict also receives cancellation", async () => { + const name = + "reports recovery paths even when a conflict also receives cancellation"; + if (runMockInSubprocess(import.meta.path, name)) return; + const f = await fixture(); + const original = "# Original policy\n"; + const concurrent = "# Concurrent save\n"; + await writeFile(join(f.repository, "SECURITY.md"), original); + const draft = await f.generate(); + const signals = new FakeSignals(); + const originalLink = fsPromises.link; + mock.module("node:fs/promises", () => ({ + ...fsPromises, + link: async (source: string, destination: string) => { + if (destination === draft.targetPath && source.endsWith(".tmp")) { + await writeFile(destination, concurrent); + signals.emit("SIGINT"); + } + await originalLink(source, destination); + }, + })); + try { + const stdout = capture(); + const stderr = capture(); + expect( + await main( + ["policy", "--apply", f.outputDir, "--write", "--json"], + stdout.stream, + stderr.stream, + policyDependencies(f, { signals }), + ), + ).toBe(2); + const result = JSON.parse(stdout.text()); + expect(result.status).toBe("recovery_required"); + expect(result.targetPath).toBe(draft.targetPath); + expect(stderr.text()).toContain(result.recoveryPath); + expect(await readFile(result.recoveryPath, "utf8")).toBe(original); + expect(await readFile(draft.targetPath, "utf8")).toBe(concurrent); + } finally { + mock.module("node:fs/promises", () => ({ + ...fsPromises, + link: originalLink, + })); + } + }); + + test("cancels a pending review prompt on SIGTERM", async () => { + const f = await fixture(); + await f.generate(); + const signals = new FakeSignals(); + expect( + await main( + ["policy", "--apply", f.outputDir], + capture(true).stream, + capture(true).stream, + policyDependencies(f, { + signals, + prompt: prompt({ + isInteractive: () => true, + confirm: async (_question, _defaultValue, signal) => { + expect(signal).toBeDefined(); + signals.emit("SIGTERM"); + signal!.throwIfAborted(); + return true; + }, + }), + }), + ), + ).toBe(143); + expect(await readdir(f.repository)).toEqual([]); + }); + test("treats Inquirer's Ctrl-C error as cancellation without a process signal", async () => { const f = await fixture(); + await f.generate(); const stderr = capture(true); expect( await main( - ["policy"], + ["policy", "--apply", f.outputDir], capture(true).stream, stderr.stream, policyDependencies(f, { prompt: prompt({ isInteractive: () => true, - input: async () => { + confirm: async () => { throw Object.assign(new Error("Prompt closed"), { name: "ExitPromptError", }); diff --git a/sdk/typescript/tests-ts/security-policy.test.ts b/sdk/typescript/tests-ts/security-policy.test.ts index 22582203..c75a7db3 100644 --- a/sdk/typescript/tests-ts/security-policy.test.ts +++ b/sdk/typescript/tests-ts/security-policy.test.ts @@ -1,17 +1,31 @@ import { execFileSync } from "node:child_process"; +import { createHash } from "node:crypto"; import { + chmod, + link, + lstat, mkdir, + open, readFile, readdir, + readlink, rename, rm, stat, symlink, writeFile, } from "node:fs/promises"; -import { join } from "node:path"; -import { afterEach, describe, expect, test } from "bun:test"; +import * as fsPromises from "node:fs/promises"; +import { dirname, join, sep } from "node:path"; +import { afterEach, describe, expect, mock, test } from "bun:test"; +import { strToU8, zipSync } from "fflate"; import { + SecurityPolicyRecoveryError, + SecurityPolicyVerificationError, +} from "../src/errors.js"; +import { + applySecurityPolicy, + loadSecurityPolicyDraft, readSecurityPolicy, resolveSecurityPolicyGuidance, resolveSecurityPolicyTarget, @@ -27,6 +41,7 @@ import { addPolicySubmodule, policyFixture, policyGit, + policyPlugin, stageResult, } from "./support/security-policy.js"; @@ -431,38 +446,1014 @@ describe("security policy generation", () => { }); }); -describe("security policy preview", () => { +describe("security policy review and application", () => { + test("protects enclosing-checkout policies when a nested checkout is selected", async () => { + for (const kind of ["repository", "submodule", "worktree"]) { + for (const existing of [false, true]) { + const f = await fixture(); + policyGit(f.repository, "init", "--quiet"); + policyGit( + f.repository, + "commit", + "--allow-empty", + "--quiet", + "-m", + "initial", + ); + const nested = join(f.repository, "services", "api"); + if (kind === "submodule") + await addPolicySubmodule( + f.repository, + join(f.root, "submodule-source"), + ); + else if (kind === "worktree") + policyGit( + f.repository, + "worktree", + "add", + "--quiet", + "--detach", + nested, + "HEAD", + ); + else { + await mkdir(nested, { recursive: true }); + policyGit(nested, "init", "--quiet"); + } + const target = join(nested, "SECURITY.md"); + const original = "# Existing nested policy\n"; + if (existing) await writeFile(target, original); + const alias = join(f.repository, "SECURITY.md"); + await symlink(target, alias, "file"); + const draft = await f.generate({ path: "services/api" }); + expect(await securityPolicyDiff(draft, PYTHON)).toContain( + "b/SECURITY.md", + ); + await expect(applySecurityPolicy(draft)).rejects.toThrow( + "outside the selected component", + ); + expect(await readSecurityPolicy(target)).toBe( + existing ? original : null, + ); + expect((await lstat(alias)).isSymbolicLink()).toBe(true); + } + } + }); + + test("protects reporting-policy aliases in enclosing checkouts", async () => { + const f = await fixture(); + const middle = join(f.repository, "services"); + const nested = join(middle, "api"); + await mkdir(nested, { recursive: true }); + for (const repository of [f.repository, middle, nested]) + policyGit(repository, "init", "--quiet"); + await symlink( + nested, + join(middle, ".github"), + process.platform === "win32" ? "junction" : "dir", + ); + const draft = await f.generate({ path: "services/api" }); + await expect(applySecurityPolicy(draft)).rejects.toThrow( + "separate vulnerability-reporting policy", + ); + expect(await readSecurityPolicy(draft.targetPath)).toBe(null); + }); + + test("allows an explicitly selected reporting policy", async () => { + for (const path of ["docs", "Docs", ".github", ".GITHUB"]) { + for (const existing of [false, true]) { + const f = await fixture(); + const directory = join(f.repository, path); + await mkdir(directory); + if (existing) + await writeFile( + join(directory, "SECURITY.md"), + "# Existing policy\n", + ); + const draft = await f.generate({ path }); + await applySecurityPolicy(draft); + expect(await readFile(draft.targetPath, "utf8")).toBe(POLICY); + } + } + }); + + test("keeps linked reporting directories distinct from the selected directory", async () => { + const f = await fixture(); + const component = join(f.repository, "Docs"); + await mkdir(component); + const lowerCaseExists = await lstat(join(f.repository, "docs")).then( + () => true, + (error: NodeJS.ErrnoException) => { + if (error.code === "ENOENT") return false; + throw error; + }, + ); + const draft = await f.generate({ path: "Docs" }); + await symlink( + component, + join(f.repository, lowerCaseExists ? ".github" : "docs"), + process.platform === "win32" ? "junction" : "dir", + ); + await expect(applySecurityPolicy(draft)).rejects.toThrow( + "separate vulnerability-reporting policy", + ); + expect(await readSecurityPolicy(draft.targetPath)).toBe(null); + }); + + test("enforces the resolver byte limit on saved draft files", async () => { + const header = "# Policy\n"; + const maximum = + header + "x".repeat(1024 * 1024 - Buffer.byteLength(header)); + const saved = await fixture(); + const draft = await saved.generate(); + await writeFile(draft.draftPath, `${maximum}x`); + await expect( + loadSecurityPolicyDraft(saved.repository, saved.outputDir), + ).rejects.toThrow("1 MiB limit"); + await writeFile(draft.draftPath, POLICY); + await writeFile( + join(saved.outputDir, "previous-SECURITY.md"), + `${maximum}x`, + ); + await expect( + loadSecurityPolicyDraft(saved.repository, saved.outputDir), + ).rejects.toThrow("1 MiB limit"); + }); + test("accepts policy Markdown without a hash-style heading", async () => { for (const content of [ "Security policy\n===============\n\nReport vulnerabilities privately.\n", "Report vulnerabilities privately.\n", ]) { const f = await fixture(); - const draft = await f.generate({ + await f.generate({ run: async (stage) => ({ ...stageResult(stage), ...(stage === "policy" ? { markdown: content } : {}), }), }); + const draft = await loadSecurityPolicyDraft(f.repository, f.outputDir); expect(draft.content).toBe(content); - expect(await readFile(draft.draftPath, "utf8")).toBe(content); - expect(await securityPolicyDiff(draft, PYTHON)).toContain( - "+Report vulnerabilities privately.", - ); - expect(await readdir(f.repository)).toEqual([]); + await applySecurityPolicy(draft, { pythonPath: PYTHON }); + expect(await readFile(draft.targetPath, "utf8")).toBe(content); + expect( + await resolveSecurityPolicyGuidance(draft, PYTHON, PLUGIN_ROOT), + ).toContain(content); } }); - test("previews the exact proposed policy without changing source", async () => { + test("previews a real diff and applies a new policy accepted by the resolver", async () => { const f = await fixture(); const draft = await f.generate(); const diff = await securityPolicyDiff(draft, PYTHON); expect(diff).toContain("--- /dev/null\n+++ b/SECURITY.md\n"); expect(diff).toContain("+Requests must be authorized"); + expect(await applySecurityPolicy(draft)).toEqual({ + status: "written", + targetPath: draft.targetPath, + recoveryPath: null, + }); + expect(await readFile(draft.targetPath, "utf8")).toBe(POLICY); + expect( + await resolveSecurityPolicyGuidance(draft, PYTHON, PLUGIN_ROOT), + ).toContain(POLICY.trim()); + expect(await readdir(f.repository)).toEqual(["SECURITY.md"]); + }); + + test("allows edits to a saved draft and writes the exact reviewed bytes", async () => { + const f = await fixture(); + const original = "# Security Policy\n\nOriginal guidance.\n"; + await writeFile(join(f.repository, "SECURITY.md"), original); + if (process.platform !== "win32") + await chmod(join(f.repository, "SECURITY.md"), 0o640); + await f.generate(); + const originalAlias = join(f.root, "original-policy.md"); + await link(join(f.repository, "SECURITY.md"), originalAlias); + const edited = `${POLICY}\nOwner-confirmed scope.\n`; + await writeFile(join(f.outputDir, "SECURITY.md"), edited); + const draft = await loadSecurityPolicyDraft(f.repository, f.outputDir); + await writeFile(draft.draftPath, "# Later unreviewed edit\n"); + await applySecurityPolicy(draft); + expect(await readFile(draft.targetPath, "utf8")).toBe(edited); + expect(await readFile(originalAlias, "utf8")).toBe(original); + if (process.platform !== "win32") + expect((await stat(draft.targetPath)).mode & 0o777).toBe(0o640); + }); + + test("rejects linked saved-draft inputs before reading them", async () => { + const f = await fixture(); + await f.generate(); + const alias = join(f.root, "linked-policy"); + await symlink( + f.outputDir, + alias, + process.platform === "win32" ? "junction" : "dir", + ); + await expect(loadSecurityPolicyDraft(f.repository, alias)).rejects.toThrow( + "non-symlink directory", + ); + for (const name of ["SECURITY.md", "previous-SECURITY.md"]) { + const path = join(f.outputDir, name); + const outside = join(f.root, `outside-${name}`); + await rename(path, outside); + await link(outside, path); + await expect( + loadSecurityPolicyDraft(f.repository, f.outputDir), + ).rejects.toThrow("hard-linked"); + await rm(path); + await rename(outside, path); + } + expect(await readdir(f.repository)).toEqual([]); + }); + + test("applies the SDK draft snapshot that was validated", async () => { + const f = await fixture(); + const draft = await f.generate(); + const target = draft.targetPath; + const other = join(f.repository, "unreviewed.md"); + const application = applySecurityPolicy(draft); + draft.content = ""; + draft.previousContent = POLICY; + draft.targetPath = other; + expect(await application).toMatchObject({ + status: "written", + targetPath: target, + }); + expect(await readFile(target, "utf8")).toBe(POLICY); + await expect(lstat(other)).rejects.toMatchObject({ code: "ENOENT" }); + }); + + test("rejects malformed UTF-8 in existing policies and saved drafts", async () => { + const f = await fixture(); + const malformed = Buffer.concat([ + Buffer.from("# Policy\n"), + Buffer.from([0xe9]), + ]); + const draft = await f.generate(); + await writeFile(draft.draftPath, malformed); + await expect( + loadSecurityPolicyDraft(f.repository, f.outputDir), + ).rejects.toThrow("valid UTF-8"); + expect(await readdir(f.repository)).toEqual([]); + await writeFile(draft.targetPath, malformed); + await expect(resolveSecurityPolicyTarget(f.repository)).rejects.toThrow( + "valid UTF-8", + ); + expect(await readFile(draft.targetPath)).toEqual(malformed); + }); + + test("preserves a valid UTF-8 byte-order mark in a reviewed draft", async () => { + const f = await fixture(); + const draft = await f.generate(); + const bytes = Buffer.from( + "\uFEFF# Security Policy\r\n\r\nReviewed text.\r\n", + ); + await writeFile(draft.draftPath, bytes); + const loaded = await loadSecurityPolicyDraft(f.repository, f.outputDir); + await applySecurityPolicy(loaded); + expect(await readFile(draft.targetPath)).toEqual(bytes); + }); + + test("uses the selected plugin and requires an explicit selection for saved custom drafts", async () => { + const f = await fixture(); + const log = join(f.root, "resolver.log"); + const pluginPath = await policyPlugin( + f.root, + [ + "import os, pathlib", + "with pathlib.Path(os.environ['POLICY_TEST_LOG']).open('a') as output:", + " output.write('custom resolver\\n')", + "print('custom guidance')", + ].join("\n"), + ); + const draft = await f.generate({ pluginPath }); + const manifestPath = join(f.outputDir, "policy-draft.json"); + const manifest = JSON.parse(await readFile(manifestPath, "utf8")); + expect(manifest.customPlugin).toBe(true); + expect(manifest).not.toHaveProperty("pluginPath"); + await writeFile( + manifestPath, + JSON.stringify({ ...manifest, pluginPath: "/unapproved/plugin" }), + ); + const saved = await loadSecurityPolicyDraft(f.repository, f.outputDir); + expect(saved.pluginPath).toBeUndefined(); + await expect(applySecurityPolicy(saved)).rejects.toThrow( + "Select it explicitly", + ); expect(await readdir(f.repository)).toEqual([]); + const temporaryVariables = ["TMPDIR", "TMP", "TEMP"] as const; + const previousTemporary = temporaryVariables.map((key) => process.env[key]); + try { + for (const key of temporaryVariables) process.env[key] = f.repository; + await applySecurityPolicy(draft, { + pythonPath: PYTHON, + environment: { ...process.env, POLICY_TEST_LOG: log }, + }); + } finally { + for (const [index, key] of temporaryVariables.entries()) { + const previous = previousTemporary[index]; + if (previous === undefined) delete process.env[key]; + else process.env[key] = previous; + } + } + expect((await readFile(log, "utf8")).trimEnd().split(/\r?\n/u)).toEqual([ + "custom resolver", + "custom resolver", + ]); + expect(await readFile(draft.targetPath, "utf8")).toBe(POLICY); + }); + + test("applies a saved draft with an explicitly selected plugin ZIP", async () => { + const f = await fixture(); + const log = join(f.root, "resolver-paths.log"); + const archive = join(f.root, "policy-plugin.zip"); + const script = [ + "import os, pathlib", + "with pathlib.Path(os.environ['POLICY_TEST_LOG']).open('a') as output:", + " output.write(str(pathlib.Path(__file__).resolve()) + '\\n')", + "print('custom guidance')", + ].join("\n"); + await writeFile( + archive, + zipSync({ + ".codex-plugin/plugin.json": strToU8( + JSON.stringify({ + name: "codex-security", + version: "test-policy-plugin", + }), + ), + "scripts/resolve_security_md.py": strToU8(script), + }), + ); + await f.generate({ pluginPath: archive }); + const saved = await loadSecurityPolicyDraft(f.repository, f.outputDir); + await applySecurityPolicy(saved, { + pluginPath: archive, + pythonPath: PYTHON, + environment: { ...process.env, POLICY_TEST_LOG: log }, + }); + expect(await readFile(saved.targetPath, "utf8")).toBe(POLICY); + const resolverPaths = (await readFile(log, "utf8")).trim().split(/\r?\n/u); + expect(resolverPaths).toHaveLength(2); + for (const path of resolverPaths) + await expect(stat(path)).rejects.toMatchObject({ code: "ENOENT" }); }); - test("rejects preview after a component changes Git roots", async () => { + test("checks the selected resolver before changing repository files", async () => { + const f = await fixture(); + const pluginPath = await policyPlugin( + f.root, + "raise SystemExit('synthetic preflight failure')\n", + ); + const draft = await f.generate({ pluginPath }); + await expect(applySecurityPolicy(draft)).rejects.toThrow( + "synthetic preflight failure", + ); + expect(await readdir(f.repository)).toEqual([]); + }); + + test("reports a committed policy when verification fails or is interrupted", async () => { + for (const failure of [ + "raise SystemExit('synthetic verification failure')", + "signal.raise_signal(signal.SIGINT)", + ]) { + const f = await fixture(); + const pluginPath = await policyPlugin( + f.root, + [ + "import pathlib, signal, sys", + "root = pathlib.Path(sys.argv[sys.argv.index('--repo') + 1])", + `if (root / 'SECURITY.md').exists(): ${failure}`, + "print('preflight passed')", + ].join("\n"), + ); + const draft = await f.generate({ pluginPath }); + const error = await applySecurityPolicy(draft).catch( + (value: unknown) => value, + ); + expect(error).toBeInstanceOf(SecurityPolicyVerificationError); + expect(error).toMatchObject({ targetPath: draft.targetPath }); + expect(await readFile(draft.targetPath, "utf8")).toBe(POLICY); + } + }); + + test("retries verification without replacing an already-installed draft", async () => { + for (const existing of [false, true]) { + const f = await fixture(); + if (existing) + await writeFile( + join(f.repository, "SECURITY.md"), + "# Existing policy\n", + ); + const blocked = join(f.root, "block-verification"); + await writeFile(blocked, ""); + const pluginPath = await policyPlugin( + f.root, + [ + "import pathlib, sys", + "target = pathlib.Path(sys.argv[sys.argv.index('--scope') + 1]) / 'SECURITY.md'", + `if target.exists() and target.read_text() == ${JSON.stringify(POLICY)} and pathlib.Path(${JSON.stringify(blocked)}).exists():`, + " raise SystemExit('synthetic verification failure')", + "print('resolver accepted the policy')", + ].join("\n"), + ); + const draft = await f.generate({ pluginPath }); + await expect(applySecurityPolicy(draft)).rejects.toBeInstanceOf( + SecurityPolicyVerificationError, + ); + const installed = await stat(draft.targetPath); + const artifacts = (await readdir(f.outputDir)).sort(); + const saved = await loadSecurityPolicyDraft(f.repository, f.outputDir); + expect(await securityPolicyDiff(draft, PYTHON)).toBe(""); + await expect(applySecurityPolicy(draft)).rejects.toBeInstanceOf( + SecurityPolicyVerificationError, + ); + for (const options of [ + { pythonPath: PYTHON }, + { pythonPath: join(f.root, "missing-python"), pluginPath }, + { pythonPath: PYTHON, pluginPath: join(f.root, "missing-plugin.zip") }, + ]) + await expect( + applySecurityPolicy(saved, options), + ).rejects.toBeInstanceOf(SecurityPolicyVerificationError); + await rm(blocked); + expect( + await applySecurityPolicy(saved, { pythonPath: PYTHON, pluginPath }), + ).toEqual({ + status: "unchanged", + targetPath: draft.targetPath, + recoveryPath: null, + }); + expect((await stat(draft.targetPath)).ino).toBe(installed.ino); + expect((await readdir(f.outputDir)).sort()).toEqual(artifacts); + } + }); + + test("rechecks the reviewed bytes after the resolver returns", async () => { + for (const change of ["remove", "replace"] as const) { + const f = await fixture(); + const original = "# Original policy\n"; + await writeFile(join(f.repository, "SECURITY.md"), original); + const pluginPath = await policyPlugin( + f.root, + [ + "import pathlib, sys", + "root = pathlib.Path(sys.argv[sys.argv.index('--repo') + 1])", + "target = root / 'SECURITY.md'", + `if target.read_text() == ${JSON.stringify(POLICY)}:`, + change === "remove" + ? " target.unlink()" + : " target.write_bytes(b'# Concurrent policy\\n')", + "print('resolver accepted the current policy chain')", + ].join("\n"), + ); + const draft = await f.generate({ pluginPath }); + const error = await applySecurityPolicy(draft).catch( + (value: unknown) => value, + ); + expect(error).toBeInstanceOf(SecurityPolicyVerificationError); + const recovery = error as SecurityPolicyVerificationError; + expect(await readFile(recovery.recoveryPath!, "utf8")).toBe(original); + expect(await readSecurityPolicy(draft.targetPath)).toBe( + change === "remove" ? null : "# Concurrent policy\n", + ); + } + }); + + test("handles unavailable hard links without clobbering policy files", async () => { + const name = + "handles unavailable hard links without clobbering policy files"; + if (runMockInSubprocess(import.meta.path, name)) return; + const originalLink = fsPromises.link; + const originalCopyFile = fsPromises.copyFile; + let linkErrorCode = "ENOTSUP"; + let collision = false; + let copyFailure = false; + mock.module("node:fs/promises", () => ({ + ...fsPromises, + link: async (_source: string, destination: string) => { + if (collision) await writeFile(destination, "# Concurrent policy\n"); + throw Object.assign(new Error("hard links are unsupported"), { + code: linkErrorCode, + }); + }, + copyFile: async (source: string, destination: string, mode?: number) => { + if (copyFailure) { + await writeFile(destination, "# Partial policy\n", { flag: "wx" }); + throw Object.assign(new Error("synthetic copy failure"), { + code: "EIO", + }); + } + await originalCopyFile(source, destination, mode); + }, + })); + try { + for (linkErrorCode of ["ENOTSUP", "EISDIR"]) { + const f = await fixture(); + const draft = await f.generate(); + await applySecurityPolicy(draft); + expect(await readFile(draft.targetPath, "utf8")).toBe(POLICY); + const existing = await fixture(); + await writeFile( + join(existing.repository, "SECURITY.md"), + "# Existing policy\n", + ); + const replacement = await existing.generate(); + await applySecurityPolicy(replacement); + expect(await readFile(replacement.targetPath, "utf8")).toBe(POLICY); + expect(await readdir(existing.repository)).toEqual(["SECURITY.md"]); + } + linkErrorCode = "ENOTSUP"; + const other = await fixture(); + const racing = await other.generate(); + collision = true; + await expect(applySecurityPolicy(racing)).rejects.toMatchObject({ + code: "EEXIST", + }); + expect(await readFile(racing.targetPath, "utf8")).toBe( + "# Concurrent policy\n", + ); + collision = false; + copyFailure = true; + const failed = await fixture(); + const partial = await failed.generate(); + const error = await applySecurityPolicy(partial).catch( + (value: unknown) => value, + ); + expect(error).toBeInstanceOf(SecurityPolicyVerificationError); + expect(error).toMatchObject({ + targetPath: partial.targetPath, + cause: { code: "EIO" }, + }); + expect(await readFile(partial.targetPath, "utf8")).toBe( + "# Partial policy\n", + ); + expect(await readdir(failed.repository)).toEqual(["SECURITY.md"]); + } finally { + mock.module("node:fs/promises", () => ({ + ...fsPromises, + link: originalLink, + copyFile: originalCopyFile, + })); + } + }); + + test("restores a concurrent save captured immediately before replacement", async () => { + const name = + "restores a concurrent save captured immediately before replacement"; + if (runMockInSubprocess(import.meta.path, name)) return; + const f = await fixture(); + await writeFile(join(f.repository, "SECURITY.md"), "# Original policy\n"); + const draft = await f.generate(); + const concurrent = "# Concurrent save\n"; + const originalRename = fsPromises.rename; + mock.module("node:fs/promises", () => ({ + ...fsPromises, + rename: async (source: string, destination: string) => { + if (source === draft.targetPath) await writeFile(source, concurrent); + await originalRename(source, destination); + }, + })); + try { + const error = await applySecurityPolicy(draft).catch( + (value: unknown) => value, + ); + expect(error).toBeInstanceOf(SecurityPolicyRecoveryError); + const recovery = error as SecurityPolicyRecoveryError; + expect(dirname(recovery.recoveryPath)).toBe(f.outputDir); + expect(await readFile(recovery.recoveryPath, "utf8")).toBe(concurrent); + expect(await readFile(draft.targetPath, "utf8")).toBe(concurrent); + expect(await readdir(f.repository)).toEqual(["SECURITY.md"]); + } finally { + mock.module("node:fs/promises", () => ({ + ...fsPromises, + rename: originalRename, + })); + } + }); + + test("keeps both files when a concurrent writer claims the destination", async () => { + const name = + "keeps both files when a concurrent writer claims the destination"; + if (runMockInSubprocess(import.meta.path, name)) return; + const f = await fixture(); + const original = "# Original policy\n"; + const concurrent = "# Concurrent save\n"; + await writeFile(join(f.repository, "SECURITY.md"), original); + const draft = await f.generate(); + const originalLink = fsPromises.link; + mock.module("node:fs/promises", () => ({ + ...fsPromises, + link: async (source: string, destination: string) => { + if (destination === draft.targetPath && source.endsWith(".tmp")) + await writeFile(destination, concurrent); + await originalLink(source, destination); + }, + })); + try { + const error = await applySecurityPolicy(draft).catch( + (value: unknown) => value, + ); + expect(error).toBeInstanceOf(SecurityPolicyRecoveryError); + const recovery = error as SecurityPolicyRecoveryError; + expect(recovery.targetPath).toBe(draft.targetPath); + expect(dirname(recovery.recoveryPath)).toBe(f.outputDir); + expect(await readFile(recovery.recoveryPath, "utf8")).toBe(original); + expect(await readFile(draft.targetPath, "utf8")).toBe(concurrent); + expect(await readdir(f.repository)).toEqual(["SECURITY.md"]); + } finally { + mock.module("node:fs/promises", () => ({ + ...fsPromises, + link: originalLink, + })); + } + }); + + test("keeps a recovery copy changed through an already-open file", async () => { + const name = "keeps a recovery copy changed through an already-open file"; + if (runMockInSubprocess(import.meta.path, name)) return; + const f = await fixture(); + await writeFile(join(f.repository, "SECURITY.md"), "# Original policy\n"); + const draft = await f.generate(); + const concurrent = "# Concurrent in-place save\n"; + const writer = await open(draft.targetPath, "r+"); + const originalLink = fsPromises.link; + mock.module("node:fs/promises", () => ({ + ...fsPromises, + link: async (source: string, destination: string) => { + await originalLink(source, destination); + if (destination === draft.targetPath && source.endsWith(".tmp")) { + await writer.truncate(0); + await writer.writeFile(concurrent); + } + }, + })); + try { + const error = await applySecurityPolicy(draft).catch( + (value: unknown) => value, + ); + expect(error).toBeInstanceOf(SecurityPolicyVerificationError); + const recovery = error as SecurityPolicyVerificationError; + expect(recovery.targetPath).toBe(draft.targetPath); + expect(dirname(recovery.recoveryPath!)).toBe(f.outputDir); + expect(await readFile(recovery.recoveryPath!, "utf8")).toBe(concurrent); + expect(await readFile(draft.targetPath, "utf8")).toBe(POLICY); + } finally { + await writer.close(); + mock.module("node:fs/promises", () => ({ + ...fsPromises, + link: originalLink, + })); + } + }); + + test("retains late writes to the displaced file after successful application", async () => { + const f = await fixture(); + const original = "# Original policy\n"; + const late = "# Save after application completed\n"; + await writeFile(join(f.repository, "SECURITY.md"), original); + const draft = await f.generate(); + const writer = await open(draft.targetPath, "r+"); + try { + const applied = await applySecurityPolicy(draft); + expect(applied.targetPath).toBe(draft.targetPath); + expect(dirname(applied.recoveryPath!)).toBe(f.outputDir); + await writer.truncate(0); + await writer.writeFile(late); + expect(await readFile(applied.recoveryPath!, "utf8")).toBe(late); + expect(await readFile(draft.targetPath, "utf8")).toBe(POLICY); + expect( + await readFile(join(f.outputDir, "previous-SECURITY.md"), "utf8"), + ).toBe(original); + expect(await readdir(f.repository)).toEqual(["SECURITY.md"]); + } finally { + await writer.close(); + } + }); + + test("keeps the original inode beside the target across filesystem boundaries", async () => { + const name = + "keeps the original inode beside the target across filesystem boundaries"; + if (runMockInSubprocess(import.meta.path, name)) return; + const f = await fixture(); + await writeFile(join(f.repository, "SECURITY.md"), "# Original policy\n"); + const draft = await f.generate(); + const writer = await open(draft.targetPath, "r+"); + const originalRename = fsPromises.rename; + mock.module("node:fs/promises", () => ({ + ...fsPromises, + rename: async (source: string, destination: string) => { + if ( + source.endsWith(".previous") && + dirname(destination) === f.outputDir + ) + throw Object.assign(new Error("different filesystem"), { + code: "EXDEV", + }); + await originalRename(source, destination); + }, + })); + try { + const applied = await applySecurityPolicy(draft); + expect(dirname(applied.recoveryPath!)).toBe(f.repository); + await writer.truncate(0); + await writer.writeFile("# Late save\n"); + expect(await readFile(applied.recoveryPath!, "utf8")).toBe( + "# Late save\n", + ); + expect(await readFile(draft.targetPath, "utf8")).toBe(POLICY); + expect( + (await readdir(f.outputDir)).filter((path) => + path.startsWith("recovery-SECURITY-"), + ), + ).toEqual([]); + } finally { + await writer.close(); + mock.module("node:fs/promises", () => ({ + ...fsPromises, + rename: originalRename, + })); + } + }); + + test("retains open-writer data when rollback must copy instead of hard-link", async () => { + const name = + "retains open-writer data when rollback must copy instead of hard-link"; + if (runMockInSubprocess(import.meta.path, name)) return; + const f = await fixture(); + const original = "# Original policy\n"; + await writeFile(join(f.repository, "SECURITY.md"), original); + const draft = await f.generate(); + const writer = await open(draft.targetPath, "r+"); + const controller = new AbortController(); + const originalRename = fsPromises.rename; + const originalLink = fsPromises.link; + mock.module("node:fs/promises", () => ({ + ...fsPromises, + rename: async (source: string, destination: string) => { + await originalRename(source, destination); + if (source === draft.targetPath) + controller.abort("cancel before install"); + }, + link: async () => { + throw Object.assign(new Error("hard links are unsupported"), { + code: "ENOTSUP", + }); + }, + })); + try { + const error = await applySecurityPolicy(draft, { + signal: controller.signal, + }).catch((value: unknown) => value); + expect(error).toBeInstanceOf(SecurityPolicyRecoveryError); + const recovery = error as SecurityPolicyRecoveryError; + await writer.truncate(0); + await writer.writeFile("# Late rollback save\n"); + expect(await readFile(recovery.recoveryPath, "utf8")).toBe( + "# Late rollback save\n", + ); + expect(await readFile(draft.targetPath, "utf8")).toBe(original); + } finally { + await writer.close(); + mock.module("node:fs/promises", () => ({ + ...fsPromises, + rename: originalRename, + link: originalLink, + })); + } + }); + + test("keeps recovery files outside checkouts and Git metadata", async () => { + for (const kind of ["root", "submodule", "external_git"]) { + const f = await fixture(); + const path = kind === "submodule" ? "services/api" : "."; + let inside = join(f.repository, "artifacts"); + if (kind === "submodule") { + policyGit(f.repository, "init", "--quiet"); + await addPolicySubmodule( + f.repository, + join(f.root, "submodule-source"), + ); + } else if (kind === "external_git") { + const metadata = join(f.root, "git-data"); + policyGit( + f.repository, + "init", + "--quiet", + "--separate-git-dir", + metadata, + ); + inside = join(metadata, "artifacts"); + } + const original = "# Original policy\n"; + await writeFile(join(f.repository, path, "SECURITY.md"), original); + const draft = await f.generate({ path }); + await mkdir(inside, { mode: 0o700 }); + await writeFile( + join(inside, "policy-draft.json"), + await readFile(join(f.outputDir, "policy-draft.json")), + ); + const before = (await readdir(f.repository)).sort(); + await expect( + applySecurityPolicy({ ...draft, outputDir: inside }), + ).rejects.toThrow("outside the protected scan root"); + expect(await readFile(draft.targetPath, "utf8")).toBe(original); + expect((await readdir(f.repository)).sort()).toEqual(before); + expect(await readdir(inside)).toEqual(["policy-draft.json"]); + } + }); + + test("restores the original policy when canceled after moving it", async () => { + const name = "restores the original policy when canceled after moving it"; + if (runMockInSubprocess(import.meta.path, name)) return; + const f = await fixture(); + const original = "# Original policy\n"; + await writeFile(join(f.repository, "SECURITY.md"), original); + const draft = await f.generate(); + const controller = new AbortController(); + const originalRename = fsPromises.rename; + mock.module("node:fs/promises", () => ({ + ...fsPromises, + rename: async (source: string, destination: string) => { + await originalRename(source, destination); + if (source === draft.targetPath) + controller.abort(new Error("cancel before install")); + }, + })); + try { + const error = await applySecurityPolicy(draft, { + signal: controller.signal, + }).catch((value: unknown) => value); + expect(error).toBeInstanceOf(SecurityPolicyRecoveryError); + expect( + await readFile( + (error as SecurityPolicyRecoveryError).recoveryPath, + "utf8", + ), + ).toBe(original); + expect(await readFile(draft.targetPath, "utf8")).toBe(original); + expect(await readdir(f.repository)).toEqual(["SECURITY.md"]); + } finally { + mock.module("node:fs/promises", () => ({ + ...fsPromises, + rename: originalRename, + })); + } + }); + + test("does not follow a symlink that races with an existing policy", async () => { + const name = "does not follow a symlink that races with an existing policy"; + if (runMockInSubprocess(import.meta.path, name)) return; + const f = await fixture(); + await writeFile(join(f.repository, "SECURITY.md"), "# Original policy\n"); + const draft = await f.generate(); + const outside = join(f.root, "outside-policy.md"); + await writeFile(outside, "# Outside policy\n"); + const originalRename = fsPromises.rename; + mock.module("node:fs/promises", () => ({ + ...fsPromises, + rename: async (source: string, destination: string) => { + if (source === draft.targetPath) { + await rm(source); + await symlink(outside, source, "file"); + } + await originalRename(source, destination); + }, + })); + try { + const error = await applySecurityPolicy(draft).catch( + (value: unknown) => value, + ); + expect(error).toBeInstanceOf(SecurityPolicyRecoveryError); + expect( + ( + await lstat((error as SecurityPolicyRecoveryError).recoveryPath) + ).isSymbolicLink(), + ).toBe(true); + expect(await readFile(outside, "utf8")).toBe("# Outside policy\n"); + await expect(lstat(draft.targetPath)).rejects.toMatchObject({ + code: "ENOENT", + }); + } finally { + mock.module("node:fs/promises", () => ({ + ...fsPromises, + rename: originalRename, + })); + } + }); + + test.skipIf(process.platform === "win32")( + "preserves an existing policy mode under a restrictive umask", + async () => { + const name = + "preserves an existing policy mode under a restrictive umask"; + if (runMockInSubprocess(import.meta.path, name)) return; + const f = await fixture(); + const target = join(f.repository, "SECURITY.md"); + await writeFile(target, "# Existing policy\n"); + await chmod(target, 0o644); + const draft = await f.generate(); + const previous = process.umask(0o077); + try { + await applySecurityPolicy(draft); + expect((await stat(target)).mode & 0o777).toBe(0o644); + } finally { + process.umask(previous); + } + }, + ); + + test("preserves read-only mode when temporary cleanup changes permissions", async () => { + const name = + "preserves read-only mode when temporary cleanup changes permissions"; + if (runMockInSubprocess(import.meta.path, name)) return; + const originalRm = fsPromises.rm; + const originalWriteFile = fsPromises.writeFile; + for (const existing of [false, true]) { + const f = await fixture(); + const target = join(f.repository, "SECURITY.md"); + if (existing) { + await writeFile(target, "# Existing policy\n"); + await chmod(target, 0o444); + } + const draft = await f.generate(); + mock.module("node:fs/promises", () => ({ + ...fsPromises, + writeFile: async (...args: Parameters) => { + await originalWriteFile(...args); + const path = args[0]; + if (!existing && typeof path === "string" && path.endsWith(".tmp")) + await chmod(path, 0o444); + }, + rm: async (path: string, options: Parameters[1]) => { + if (path.endsWith(".tmp")) await chmod(path, 0o666); + return await originalRm(path, options); + }, + })); + try { + await applySecurityPolicy(draft); + expect(await readFile(target, "utf8")).toBe(POLICY); + expect((await stat(target)).mode & 0o200).toBe(0); + expect(await readdir(f.repository)).toEqual(["SECURITY.md"]); + } finally { + mock.module("node:fs/promises", () => ({ + ...fsPromises, + rm: originalRm, + writeFile: originalWriteFile, + })); + await chmod(target, 0o644).catch(() => undefined); + } + } + }); + + test("finishes verification when cancellation arrives after the write commits", async () => { + const name = + "finishes verification when cancellation arrives after the write commits"; + if (runMockInSubprocess(import.meta.path, name)) return; + const originalLink = fsPromises.link; + const originalRename = fsPromises.rename; + for (const existing of [false, true]) { + const f = await fixture(); + if (existing) + await writeFile( + join(f.repository, "SECURITY.md"), + "# Existing policy\n", + ); + const draft = await f.generate(); + const controller = new AbortController(); + mock.module("node:fs/promises", () => ({ + ...fsPromises, + link: async (source: string, destination: string) => { + await originalLink(source, destination); + if (destination === draft.targetPath) + controller.abort(new Error("cancel after commit")); + }, + rename: async (source: string, destination: string) => { + await originalRename(source, destination); + if (destination === draft.targetPath) + controller.abort(new Error("cancel after commit")); + }, + })); + try { + const applied = await applySecurityPolicy(draft, { + pythonPath: PYTHON, + signal: controller.signal, + }); + expect(applied.targetPath).toBe(draft.targetPath); + expect(applied.recoveryPath === null).toBe(!existing); + expect(controller.signal.aborted).toBe(true); + expect(await readFile(draft.targetPath, "utf8")).toBe(POLICY); + } finally { + mock.module("node:fs/promises", () => ({ + ...fsPromises, + link: originalLink, + rename: originalRename, + })); + } + } + }); + + test("rejects preview and application after a component changes Git roots", async () => { for (const change of ["add", "remove"] as const) { const f = await fixture(); policyGit(f.repository, "init", "--quiet"); @@ -475,6 +1466,9 @@ describe("security policy preview", () => { await expect(securityPolicyDiff(draft, PYTHON)).rejects.toThrow( "destination changed", ); + await expect(applySecurityPolicy(draft)).rejects.toThrow( + "destination changed", + ); expect(await readSecurityPolicy(draft.targetPath)).toBe(null); } }); @@ -590,32 +1584,40 @@ describe("security policy preview", () => { throw new Error("An unchanged preview must not resolve Python"); }), ).toBe(""); + expect( + await applySecurityPolicy(draft, { pythonPath: "missing-python" }), + ).toEqual({ + status: "unchanged", + targetPath: draft.targetPath, + recoveryPath: null, + }); await writeFile(draft.targetPath, "# Concurrent policy\n"); await expect(securityPolicyDiff(draft, PYTHON)).rejects.toThrow( "changed after", ); + await expect(applySecurityPolicy(draft)).rejects.toThrow("changed after"); }); - test("rejects changes made while preparing a policy diff", async () => { - for (const changed of ["target", "inherited"]) { + test("rechecks policy changes made while preparing a diff", async () => { + for (const changed of ["target", "inherited", "applied"]) { const f = await fixture(); await mkdir(join(f.repository, "component")); const rootPolicy = join(f.repository, "SECURITY.md"); await writeFile(rootPolicy, "# Original root policy\n"); const draft = await f.generate({ path: "component" }); - await expect( - securityPolicyDiff(draft, async () => { - await writeFile( - changed === "target" ? draft.targetPath : rootPolicy, - "# Concurrent policy\n", - ); - return PYTHON; - }), - ).rejects.toThrow("changed after"); + const preview = securityPolicyDiff(draft, async () => { + await writeFile( + changed === "inherited" ? rootPolicy : draft.targetPath, + changed === "applied" ? draft.content : "# Concurrent policy\n", + ); + return PYTHON; + }); + if (changed === "applied") expect(await preview).toBe(""); + else await expect(preview).rejects.toThrow("changed after"); } }); - test("invalidates component previews when inherited policies change", async () => { + test("invalidates saved component drafts when inherited policies change", async () => { for (const change of ["edit", "add", "remove"] as const) { const f = await fixture(); const component = join(f.repository, "services", "api"); @@ -624,7 +1626,11 @@ describe("security policy preview", () => { await writeFile(rootPolicy, "# Root policy\n"); if (change === "edit") await writeFile(join(component, "SECURITY.md"), POLICY); - const draft = await f.generate({ path: "services/api" }); + const generated = await f.generate({ path: "services/api" }); + const draft = await loadSecurityPolicyDraft(f.repository, f.outputDir, { + path: "services/api", + }); + expect(draft.inheritedPolicySha256).toBe(generated.inheritedPolicySha256); if (change === "edit") await writeFile(rootPolicy, "# New root policy\n"); else if (change === "add") await writeFile( @@ -635,12 +1641,43 @@ describe("security policy preview", () => { await expect(securityPolicyDiff(draft, "missing-python")).rejects.toThrow( "inherited SECURITY.md changed", ); + await expect( + applySecurityPolicy(draft, { pythonPath: "missing-python" }), + ).rejects.toThrow("inherited SECURITY.md changed"); expect(await readSecurityPolicy(draft.targetPath)).toBe( draft.previousContent, ); } }); + test("applies a component policy without changing a safe inherited link", async () => { + const f = await fixture(); + const ownerPolicy = join(f.repository, "owner-policy.md"); + const inherited = join(f.repository, "SECURITY.md"); + await mkdir(join(f.repository, "component")); + await writeFile(ownerPolicy, "# Owner policy\n"); + await symlink(ownerPolicy, inherited, "file"); + const draft = await f.generate({ path: "component" }); + const hash = (text: string) => + createHash("sha256").update(text).digest("hex"); + const links = { + links: [["SECURITY.md", await readlink(inherited)]], + destination: "owner-policy.md", + }; + expect(draft.inheritedPolicySha256).toBe( + hash( + JSON.stringify([ + ["SECURITY.md", `link:${hash(JSON.stringify(links))}`], + ["SECURITY.md", hash("# Owner policy\n")], + ]), + ), + ); + await applySecurityPolicy(draft); + expect(await readFile(draft.targetPath, "utf8")).toBe(POLICY); + expect(await readFile(inherited, "utf8")).toBe("# Owner policy\n"); + expect((await lstat(inherited)).isSymbolicLink()).toBe(true); + }); + test("rejects inherited aliases that would widen the policy scope", async () => { for (const [ancestor, existing, chained, name] of [ [".", false, false, "SECURITY.md"], @@ -682,7 +1719,7 @@ describe("security policy preview", () => { "b/component/SECURITY.md", ); await writeFile(linkedPolicy, "# Changed owner policy\n"); - await expect(securityPolicyDiff(draft, PYTHON)).rejects.toThrow( + await expect(applySecurityPolicy(draft)).rejects.toThrow( "inherited SECURITY.md changed", ); @@ -701,6 +1738,354 @@ describe("security policy preview", () => { expect(await readdir(outside.outputDir)).toEqual([]); }); + test("rejects policy aliases in a sibling component", async () => { + for (const [existing, chained] of [ + [false, false], + [true, false], + [false, true], + [true, true], + ]) { + const f = await fixture(); + const component = join(f.repository, "component"); + const target = join(component, "SECURITY.md"); + const alias = join(f.repository, "component-other", "SECURITY.md"); + await mkdir(component); + await mkdir(dirname(alias)); + if (existing) await writeFile(target, "# Original policy\n"); + const destination = chained + ? join(f.repository, "policy-link.md") + : target; + if (chained) await symlink(target, destination, "file"); + await symlink(destination, alias, "file"); + const draft = await f.generate({ path: "component" }); + await expect(applySecurityPolicy(draft)).rejects.toThrow( + "outside the selected component", + ); + expect((await lstat(alias)).isSymbolicLink()).toBe(true); + expect(await readSecurityPolicy(target)).toBe( + existing ? "# Original policy\n" : null, + ); + } + }); + + test("allows aliases within the selected policy scope", async () => { + for (const scope of [".", "component"]) { + const f = await fixture(); + const component = join(f.repository, scope); + const descendant = join(component, "child", "SECURITY.md"); + const target = join(component, "SECURITY.md"); + await mkdir(dirname(descendant), { recursive: true }); + await symlink(target, descendant, "file"); + const draft = await f.generate({ path: scope }); + await applySecurityPolicy(draft); + expect(await readFile(descendant, "utf8")).toBe(POLICY); + expect((await lstat(descendant)).isSymbolicLink()).toBe(true); + } + }); + + test("keeps descendant checkout reporting policies out of scope", async () => { + for (const kind of ["nested", "submodule"] as const) { + for (const linkedDirectory of [false, true]) { + const f = await fixture(); + policyGit(f.repository, "init", "--quiet"); + let child: string; + if (kind === "submodule") { + child = await addPolicySubmodule( + f.repository, + join(f.root, "submodule-source"), + "child", + ); + } else { + child = join(f.repository, "child"); + await mkdir(child); + policyGit(child, "init", "--quiet"); + } + const reporting = linkedDirectory + ? join(f.repository, "reporting") + : join(child, ".github"); + await mkdir(reporting); + const target = join(f.repository, "SECURITY.md"); + const draft = await f.generate(); + await symlink(target, join(reporting, "SECURITY.md"), "file"); + if (linkedDirectory) + await symlink( + reporting, + join(child, ".github"), + process.platform === "win32" ? "junction" : "dir", + ); + await expect(applySecurityPolicy(draft)).rejects.toThrow( + "separate vulnerability-reporting policy", + ); + expect(await readSecurityPolicy(target)).toBe(null); + } + } + }); + + test("leaves unrelated aliases in descendant checkouts alone", async () => { + const f = await fixture(); + policyGit(f.repository, "init", "--quiet"); + const child = join(f.repository, "child"); + await mkdir(child); + policyGit(child, "init", "--quiet"); + const separate = join(f.repository, "child-policy.md"); + await writeFile(separate, "# Child policy\n"); + await symlink(separate, join(child, "SECURITY.md"), "file"); + await applySecurityPolicy(await f.generate()); + expect(await readFile(separate, "utf8")).toBe("# Child policy\n"); + }); + + test("preserves separate reporting policies when applying a root draft", async () => { + const f = await fixture(); + for (const directory of [".github", "docs"]) { + await mkdir(join(f.repository, directory)); + await writeFile( + join(f.repository, directory, "SECURITY.md"), + "# Reporting a vulnerability\n", + ); + } + const draft = await f.generate(); + await applySecurityPolicy(draft); + for (const directory of [".github", "docs"]) + expect( + await readFile(join(f.repository, directory, "SECURITY.md"), "utf8"), + ).toBe("# Reporting a vulnerability\n"); + }); + + test("treats non-directory reporting-policy paths as absent", async () => { + for (const entry of [".github", "docs"]) { + const f = await fixture(); + const path = join(f.repository, entry); + await writeFile(path, "A regular source file.\n"); + const draft = await f.generate(); + expect(await securityPolicyDiff(draft, PYTHON)).toContain( + "b/SECURITY.md", + ); + await applySecurityPolicy(draft); + expect(await readFile(path, "utf8")).toBe("A regular source file.\n"); + } + }); + + test("validates descendant policy links before applying a draft", async () => { + for (const scope of [".", "component"]) { + for (const existing of [false, true]) { + const f = await fixture(); + const component = join(f.repository, scope); + const alias = join(component, "child", "SECURITY.md"); + const outside = join(f.root, "outside-policy.md"); + await mkdir(dirname(alias), { recursive: true }); + if (existing) await writeFile(outside, "# Outside policy\n"); + const draft = await f.generate({ path: scope }); + await symlink(outside, alias, "file"); + expect(await securityPolicyDiff(draft, PYTHON)).toContain( + "SECURITY.md", + ); + await expect( + applySecurityPolicy(draft, { pythonPath: "missing-python" }), + ).rejects.toThrow("outside the repository"); + expect(await readSecurityPolicy(draft.targetPath)).toBe(null); + } + } + }); + + test("rejects root drafts that would change a linked reporting policy", async () => { + for (const directory of [".github", "docs"]) { + for (const [existing, chained] of [ + [false, false], + [true, false], + [false, true], + [true, true], + ]) { + const f = await fixture(); + const target = join(f.repository, "SECURITY.md"); + const reporting = join(f.repository, directory, "SECURITY.md"); + await mkdir(dirname(reporting)); + if (existing) await writeFile(target, "# Original policy\n"); + const draft = await f.generate(); + const destination = chained + ? join(f.repository, "policy-link.md") + : target; + if (chained) await symlink(target, destination, "file"); + await symlink(destination, reporting, "file"); + expect(await securityPolicyDiff(draft, PYTHON)).toContain( + "b/SECURITY.md", + ); + await expect( + applySecurityPolicy(draft, { pythonPath: "missing-python" }), + ).rejects.toThrow("separate vulnerability-reporting policy"); + expect(await readSecurityPolicy(target)).toBe( + existing ? "# Original policy\n" : null, + ); + } + } + }); + + test("rejects reporting-policy aliases through directory links", async () => { + for (const directory of [".github", "docs"]) { + const f = await fixture(); + const draft = await f.generate(); + await symlink( + f.repository, + join(f.repository, directory), + process.platform === "win32" ? "junction" : "dir", + ); + await expect(applySecurityPolicy(draft)).rejects.toThrow( + "separate vulnerability-reporting policy", + ); + expect(await readSecurityPolicy(draft.targetPath)).toBe(null); + } + }); + + test("stops policy-link walks before inspecting another target", async () => { + const name = "stops policy-link walks before inspecting another target"; + if (runMockInSubprocess(import.meta.path, name)) return; + const originalLstat = fsPromises.lstat; + const originalReadlink = fsPromises.readlink; + const originalRealpath = fsPromises.realpath; + const inspected: string[] = []; + let outside = ""; + const record = (path: unknown) => { + const value = String(path); + if (value === outside || value.startsWith(`${outside}${sep}`)) + inspected.push(value); + }; + mock.module("node:fs/promises", () => ({ + ...fsPromises, + lstat: (...args: Parameters) => { + record(args[0]); + return originalLstat(...args); + }, + readlink: (...args: Parameters) => { + record(args[0]); + return originalReadlink(...args); + }, + realpath: (...args: Parameters) => { + record(args[0]); + return originalRealpath(...args); + }, + })); + try { + for (const viaDirectory of [false, true]) { + for (const existing of [false, true]) { + const f = await fixture(); + outside = join(f.root, "outside"); + const target = join(f.repository, "component", "SECURITY.md"); + const alias = join(f.repository, "sibling", "SECURITY.md"); + await mkdir(dirname(target)); + await mkdir(dirname(alias)); + await mkdir(outside); + if (existing) await writeFile(target, "# Original policy\n"); + const externalLink = join(outside, "policy-link.md"); + await symlink(target, externalLink, "file"); + let destination = externalLink; + if (viaDirectory) { + const directoryLink = join(f.repository, "outside-link"); + await symlink( + outside, + directoryLink, + process.platform === "win32" ? "junction" : "dir", + ); + destination = join(directoryLink, "policy-link.md"); + } + await symlink(destination, alias, "file"); + const draft = await f.generate({ path: "component" }); + inspected.length = 0; + await expect(applySecurityPolicy(draft)).rejects.toThrow( + "outside the repository", + ); + expect(inspected).toEqual([]); + } + } + } finally { + mock.module("node:fs/promises", () => ({ + ...fsPromises, + lstat: originalLstat, + readlink: originalReadlink, + realpath: originalRealpath, + })); + } + }); + + test("ignores unrelated broken policies, Git metadata, and directory links", async () => { + const f = await fixture(); + execFileSync("git", ["init", "--quiet", f.repository]); + const target = join(f.repository, "component", "SECURITY.md"); + const cycle = join(f.repository, "unrelated", "SECURITY.md"); + const intermediate = join(f.repository, "unrelated", "cycle.md"); + const outside = join(f.root, "linked-directory"); + await mkdir(dirname(target)); + await mkdir(dirname(cycle)); + await mkdir(outside); + await symlink( + join(f.repository, "missing", "owner-policy.md"), + join(f.repository, "SECURITY.md"), + "file", + ); + await symlink(intermediate, cycle, "file"); + await symlink(cycle, intermediate, "file"); + await symlink(target, join(f.repository, ".git", "SECURITY.md"), "file"); + await symlink(target, join(outside, "SECURITY.md"), "file"); + await symlink( + outside, + join(f.repository, "linked-directory"), + process.platform === "win32" ? "junction" : "dir", + ); + const draft = await f.generate({ path: "component" }); + await applySecurityPolicy(draft); + expect(await readFile(target, "utf8")).toBe(POLICY); + expect((await lstat(cycle)).isSymbolicLink()).toBe(true); + }); + + test("ignores case-equivalent Git metadata without hiding ordinary directories", async () => { + const f = await fixture(); + policyGit(f.repository, "init", "--quiet"); + const target = join(f.repository, "component", "SECURITY.md"); + await mkdir(dirname(target)); + await rename( + join(f.repository, ".git"), + join(f.repository, "git-metadata"), + ); + await rename( + join(f.repository, "git-metadata"), + join(f.repository, ".GIT"), + ); + await symlink(target, join(f.repository, ".GIT", "SECURITY.md"), "file"); + const gitRecognizesDirectory = await lstat(join(f.repository, ".git")).then( + () => true, + (error: NodeJS.ErrnoException) => { + if (error.code === "ENOENT") return false; + throw error; + }, + ); + const draft = await f.generate({ path: "component" }); + if (gitRecognizesDirectory) { + await applySecurityPolicy(draft); + expect(await readFile(target, "utf8")).toBe(POLICY); + } else { + await expect(applySecurityPolicy(draft)).rejects.toThrow( + /outside the selected component|Git metadata/u, + ); + expect(await readSecurityPolicy(target)).toBe(null); + } + }); + + test("rejects case aliases to a missing component policy on every platform", async () => { + for (const name of ["security.md", "\u017fECURITY.md"]) { + const f = await fixture(); + await mkdir(join(f.repository, "component")); + await mkdir(join(f.repository, "sibling")); + await symlink( + join(f.repository, "component", name), + join(f.repository, "sibling", "SECURITY.md"), + "file", + ); + const draft = await f.generate({ path: "component" }); + await expect( + applySecurityPolicy(draft, { pythonPath: "missing-python" }), + ).rejects.toThrow("outside the selected component"); + expect(await readSecurityPolicy(draft.targetPath)).toBe(null); + } + }); + test("treats inherited links through regular files as absent", async () => { const f = await fixture(); policyGit(f.repository, "init", "--quiet"); @@ -715,6 +2100,8 @@ describe("security policy preview", () => { expect(await securityPolicyDiff(draft, PYTHON)).toContain( "b/component/SECURITY.md", ); + await applySecurityPolicy(draft); + expect(await readFile(draft.targetPath, "utf8")).toBe(POLICY); }); test("invalidates component drafts when inherited links change", async () => { @@ -741,10 +2128,37 @@ describe("security policy preview", () => { await expect(securityPolicyDiff(draft, "missing-python")).rejects.toThrow( "inherited SECURITY.md changed", ); + await expect( + applySecurityPolicy(draft, { pythonPath: "missing-python" }), + ).rejects.toThrow("inherited SECURITY.md changed"); expect(await readFile(target, "utf8")).toBe("# Original policy\n"); } }); + test("rejects saved drafts when an outside scope starts linking to the target", async () => { + for (const policyDirectory of [".", "sibling"]) { + for (const existing of [false, true]) { + const f = await fixture(); + await mkdir(join(f.repository, "component")); + const target = join(f.repository, "component", "SECURITY.md"); + const alias = join(f.repository, policyDirectory, "SECURITY.md"); + await mkdir(dirname(alias), { recursive: true }); + if (existing) await writeFile(target, "# Original policy\n"); + await f.generate({ path: "component" }); + await symlink(target, alias, "file"); + const draft = await loadSecurityPolicyDraft(f.repository, f.outputDir, { + path: "component", + }); + await expect( + applySecurityPolicy(draft, { pythonPath: "missing-python" }), + ).rejects.toThrow("outside the selected component"); + expect(await readSecurityPolicy(target)).toBe( + existing ? "# Original policy\n" : null, + ); + } + } + }); + test("rejects cycles in inherited policy links", async () => { const f = await fixture(); await mkdir(join(f.repository, "component")); @@ -755,4 +2169,141 @@ describe("security policy preview", () => { await expect(f.generate({ path: "component" })).rejects.toThrow("cycle"); expect(await readdir(f.outputDir)).toEqual([]); }); + + test("checks policy aliases before and after a policy write", async () => { + for (const policyDirectory of [".", "sibling"]) { + for (const timing of ["before", "after"] as const) { + const f = await fixture(); + await mkdir(join(f.repository, "component")); + await mkdir(join(f.repository, policyDirectory), { recursive: true }); + const pluginPath = await policyPlugin( + f.root, + [ + "import pathlib, sys", + "root = pathlib.Path(sys.argv[sys.argv.index('--repo') + 1])", + "target = root / 'component' / 'SECURITY.md'", + `if ${timing === "before" ? "not " : ""}target.exists():`, + ` (root / ${JSON.stringify(policyDirectory)} / 'SECURITY.md').symlink_to(target)`, + "print('resolver accepted the current policy chain')", + ].join("\n"), + ); + const draft = await f.generate({ path: "component", pluginPath }); + await expect(applySecurityPolicy(draft)).rejects.toThrow( + timing === "before" + ? "outside the selected component" + : "was written", + ); + expect(await readSecurityPolicy(draft.targetPath)).toBe( + timing === "before" ? null : POLICY, + ); + if (timing === "after") + await expect(applySecurityPolicy(draft)).rejects.toBeInstanceOf( + SecurityPolicyVerificationError, + ); + } + } + }); + + test("checks inherited policies around application and verification", async () => { + for (const timing of ["before", "after"] as const) { + const f = await fixture(); + await mkdir(join(f.repository, "component")); + await writeFile(join(f.repository, "SECURITY.md"), "# Root policy\n"); + const pluginPath = await policyPlugin( + f.root, + [ + "import pathlib, sys", + "root = pathlib.Path(sys.argv[sys.argv.index('--repo') + 1])", + "target = root / 'component' / 'SECURITY.md'", + `if ${timing === "before" ? "not " : ""}target.exists():`, + " (root / 'SECURITY.md').write_text('# New root policy\\n')", + "print('resolver accepted the current policy chain')", + ].join("\n"), + ); + const draft = await f.generate({ path: "component", pluginPath }); + const error = await applySecurityPolicy(draft).catch( + (value: unknown) => value, + ); + if (timing === "before") + expect(String(error)).toContain("inherited SECURITY.md changed"); + else expect(error).toBeInstanceOf(SecurityPolicyVerificationError); + expect(await readSecurityPolicy(draft.targetPath)).toBe( + timing === "before" ? null : POLICY, + ); + } + }); + + test("honors cancellation before applying a draft", async () => { + const f = await fixture(); + const draft = await f.generate(); + const signal = AbortSignal.abort(new Error("canceled")); + await expect( + applySecurityPolicy(draft, { pythonPath: PYTHON, signal }), + ).rejects.toThrow("canceled"); + expect(await readdir(f.repository)).toEqual([]); + }); + + test("does not overwrite a policy changed after generation", async () => { + const f = await fixture(); + const draft = await f.generate(); + await writeFile(draft.targetPath, "# Someone else's new policy\n"); + await expect(applySecurityPolicy(draft)).rejects.toThrow("changed after"); + expect(await readFile(draft.targetPath, "utf8")).toBe( + "# Someone else's new policy\n", + ); + }); + + test("binds saved drafts to the explicitly selected repository and component", async () => { + const f = await fixture(); + await mkdir(join(f.repository, "component")); + await f.generate({ path: "component" }); + await expect( + loadSecurityPolicyDraft(f.repository, f.outputDir), + ).rejects.toThrow("different repository or component"); + const draft = await loadSecurityPolicyDraft(f.repository, f.outputDir, { + path: "component", + }); + expect(draft.scope).toBe("component"); + const other = await fixture(); + await expect( + loadSecurityPolicyDraft(other.repository, f.outputDir), + ).rejects.toThrow("different repository or component"); + }); + + test("rejects linked policy files and replaced component directories", async () => { + const f = await fixture(); + const component = join(f.repository, "component"); + await mkdir(component); + const draft = await f.generate({ path: "component" }); + const external = join(f.root, "external"); + await mkdir(external); + const externalPolicy = join(external, "SECURITY.md"); + await writeFile(externalPolicy, "# External policy\n"); + await symlink(externalPolicy, draft.targetPath); + await expect(applySecurityPolicy(draft)).rejects.toThrow("regular file"); + await rm(draft.targetPath); + await rename(component, join(f.repository, "old-component")); + await symlink( + external, + component, + process.platform === "win32" ? "junction" : "dir", + ); + await expect(applySecurityPolicy(draft)).rejects.toThrow( + "outside the repository", + ); + expect(await readFile(externalPolicy, "utf8")).toBe("# External policy\n"); + expect((await lstat(component)).isSymbolicLink()).toBe(true); + }); + + test("rejects a modified original-content checkpoint", async () => { + const f = await fixture(); + await f.generate(); + await writeFile( + join(f.outputDir, "previous-SECURITY.md"), + "# Forged baseline\n", + ); + await expect( + loadSecurityPolicyDraft(f.repository, f.outputDir), + ).rejects.toThrow("checkpoint has changed"); + }); });