diff --git a/sdk/typescript/src/cli.ts b/sdk/typescript/src/cli.ts index be9bea5a..192a0fda 100644 --- a/sdk/typescript/src/cli.ts +++ b/sdk/typescript/src/cli.ts @@ -7,9 +7,17 @@ import { existsSync, lstatSync, realpathSync, + type Stats, writeSync, } from "node:fs"; -import { mkdir, readFile, realpath, stat, writeFile } from "node:fs/promises"; +import { + lstat, + mkdir, + open, + readFile, + realpath, + writeFile, +} from "node:fs/promises"; import { basename, dirname, @@ -257,14 +265,18 @@ async function readPromptFiles( directory: string, scanPromptFile?: string, postScanPromptFile?: string, + repository = directory, ): Promise> { const [scanPrompt, postScanPrompt] = await Promise.all([ scanPromptFile === undefined ? undefined - : readFile(resolve(directory, scanPromptFile), "utf8"), + : readRegularInputFile(resolve(directory, scanPromptFile), repository), postScanPromptFile === undefined ? undefined - : readFile(resolve(directory, postScanPromptFile), "utf8"), + : readRegularInputFile( + resolve(directory, postScanPromptFile), + repository, + ), ]); return { ...(scanPrompt?.trim() ? { scanPrompt } : {}), @@ -272,6 +284,52 @@ async function readPromptFiles( }; } +async function readRegularInputFile( + path: string, + repository: string, + metadata?: Pick, +): Promise { + const selected = metadata ?? (await lstat(path)); + if (!selected.isFile()) { + throw new CodexSecurityError("Input files must be regular files."); + } + const canonicalRepository = await realpath(repository); + const canonicalParent = await realpath(dirname(path)); + if (isOutsidePath(relative(canonicalRepository, canonicalParent))) { + for (let ancestor = dirname(path); ; ancestor = dirname(ancestor)) { + if ( + !isOutsidePath(relative(canonicalRepository, await realpath(ancestor))) + ) { + throw new CodexSecurityError( + "Input files must not follow repository directory links outside the selected repository.", + ); + } + if (dirname(ancestor) === ancestor) { + break; + } + } + } + const file = await open( + join(canonicalParent, basename(path)), + constants.O_RDONLY | + (constants.O_NOFOLLOW ?? 0) | + (constants.O_NONBLOCK ?? 0), + ); + try { + const opened = await file.stat(); + if ( + !opened.isFile() || + opened.dev !== selected.dev || + opened.ino !== selected.ino + ) { + throw new CodexSecurityError("Input files must remain regular files."); + } + return await file.readFile({ encoding: "utf8" }); + } finally { + await file.close(); + } +} + interface ScanArguments extends DeepScanOptions { auth?: ScanAuthMode; verbose?: boolean; @@ -2370,7 +2428,7 @@ async function runSkill( localDeviceRoot !== normalizedDeviceRoot); if (!windowsNetworkPath) { const path = resolve(directory, input); - const metadata = await stat(path).catch((error: unknown) => { + const metadata = await lstat(path).catch((error: unknown) => { if ( typeof error === "object" && error !== null && @@ -2393,7 +2451,11 @@ async function runSkill( ); } try { - contentsOrLiteral = await readFile(path, "utf8"); + contentsOrLiteral = await readRegularInputFile( + path, + directory, + metadata, + ); } catch { throw new CodexSecurityError( "Could not read the finding or issue input.", @@ -2739,12 +2801,14 @@ async function runScan( let failed = false; let failure: unknown; try { - const repository = arguments_.repository ?? dependencies.currentDirectory(); + const directory = dependencies.currentDirectory(); + const repository = arguments_.repository ?? directory; const target = targetFromArguments(arguments_); const prompts = await readPromptFiles( - dependencies.currentDirectory(), + directory, arguments_.scanPromptFile, arguments_.postScanPromptFile, + resolve(directory, repository), ); const config: CodexSecurityConfig = { pluginPath: arguments_.pluginPath, diff --git a/sdk/typescript/tests-ts/cli-scan-prompts.test.ts b/sdk/typescript/tests-ts/cli-scan-prompts.test.ts index fae38608..b5fc2253 100644 --- a/sdk/typescript/tests-ts/cli-scan-prompts.test.ts +++ b/sdk/typescript/tests-ts/cli-scan-prompts.test.ts @@ -1,5 +1,12 @@ import { spawnSync } from "node:child_process"; -import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { + mkdir, + mkdtemp, + realpath, + rm, + symlink, + writeFile, +} from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, test } from "bun:test"; @@ -43,6 +50,98 @@ describe("CLI scan prompts", () => { } }); + test("rejects linked prompt files without rejecting selected external files", async () => { + const root = await mkdtemp(join(tmpdir(), "codex-security-cli-prompts-")); + try { + const repository = join(root, "repository"); + const repositoryAlias = join(root, "repository-alias"); + const externalDirectory = join(root, "external"); + const external = join(externalDirectory, "external-prompt.md"); + const linked = join(repository, "linked-prompt.md"); + await mkdir(repository); + await symlink( + repository, + repositoryAlias, + process.platform === "win32" ? "junction" : "dir", + ); + const physicalRepository = await realpath(repository); + await mkdir(externalDirectory); + await writeFile(external, "SYNTHETIC_EXTERNAL_PROMPT\n"); + await symlink(external, linked); + await symlink( + externalDirectory, + join(repository, "linked-directory"), + process.platform === "win32" ? "junction" : "dir", + ); + + for (const option of ["--scan-prompt-file", "--post-scan-prompt-file"]) { + for (const [directory, target, input] of [ + [repository, ".", "linked-prompt.md"], + [repository, ".", join("linked-directory", "external-prompt.md")], + [ + root, + repository, + join(repository, "linked-directory", "external-prompt.md"), + ], + [ + physicalRepository, + repositoryAlias, + join("linked-directory", "external-prompt.md"), + ], + [ + repositoryAlias, + physicalRepository, + join("linked-directory", "external-prompt.md"), + ], + ] as const) { + let started = false; + const stderr = capture(); + expect( + await main( + ["scan", target, option, input, "--json"], + capture().stream, + stderr.stream, + dependencies({ + currentDirectory: directory, + onTurn: () => { + started = true; + }, + }), + ), + ).toBe(2); + expect(stderr.text()).toContain("Input files must"); + expect(stderr.text()).not.toContain("SYNTHETIC_EXTERNAL_PROMPT"); + expect(started).toBe(false); + } + + for (const [directory, target] of [ + [repository, "."], + [physicalRepository, repositoryAlias], + [repositoryAlias, physicalRepository], + ] as const) { + let selected: unknown; + expect( + await main( + ["scan", target, option, external, "--json"], + capture().stream, + capture().stream, + dependencies({ + currentDirectory: directory, + onTurn: (_repository, value) => (selected = value), + }), + ), + ).toBe(0); + expect(selected).toMatchObject({ + [option === "--scan-prompt-file" ? "scanPrompt" : "postScanPrompt"]: + "SYNTHETIC_EXTERNAL_PROMPT\n", + }); + } + } + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + test("combines shared and repository-specific bulk scan prompts", async () => { const root = await mkdtemp(join(tmpdir(), "codex-security-cli-prompts-")); try { diff --git a/sdk/typescript/tests-ts/cli-skills.test.ts b/sdk/typescript/tests-ts/cli-skills.test.ts index 3ef574ad..f5b9457d 100644 --- a/sdk/typescript/tests-ts/cli-skills.test.ts +++ b/sdk/typescript/tests-ts/cli-skills.test.ts @@ -1,9 +1,10 @@ -import { spawn } from "node:child_process"; -import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { execFileSync, spawn } from "node:child_process"; +import { mkdir, mkdtemp, rm, symlink, writeFile } from "node:fs/promises"; +import * as filesystem from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { setTimeout as delay } from "node:timers/promises"; -import { describe, expect, test } from "bun:test"; +import { describe, expect, spyOn, test } from "bun:test"; import { main, readSkillCommandOutput, @@ -107,6 +108,137 @@ describe("CLI skill commands", () => { } }); + test("rejects linked findings while preserving selected external files", async () => { + const root = await mkdtemp(join(tmpdir(), "codex-security-skill-inputs-")); + try { + const repository = join(root, "repository"); + const externalDirectory = join(root, "external"); + const linkedDirectory = join(root, "external-alias"); + const finding = join(externalDirectory, "finding.txt"); + await mkdir(repository); + await mkdir(externalDirectory); + await writeFile(finding, "SYNTHETIC_EXTERNAL_FINDING\n"); + await symlink(finding, join(repository, "linked-finding.txt")); + await symlink( + externalDirectory, + linkedDirectory, + process.platform === "win32" ? "junction" : "dir", + ); + await symlink( + externalDirectory, + join(repository, "linked-directory"), + process.platform === "win32" ? "junction" : "dir", + ); + + for (const command of ["validate", "patch"] as const) { + let invocation: readonly string[] | undefined; + for (const input of [ + "linked-finding.txt", + join("linked-directory", "finding.txt"), + ]) { + const stderr = capture(); + expect( + await main( + [command, input], + capture().stream, + stderr.stream, + dependencies({ + currentDirectory: repository, + onCodex: (args) => { + invocation = args; + return 0; + }, + }), + ), + ).toBe(2); + expect(stderr.text()).not.toContain("SYNTHETIC_EXTERNAL_FINDING"); + expect(invocation).toBeUndefined(); + } + + for (const selected of [ + finding, + join("..", "external", "finding.txt"), + join(linkedDirectory, "finding.txt"), + ]) { + expect( + await main( + [command, selected], + capture().stream, + capture().stream, + dependencies({ + currentDirectory: repository, + onCodex: (args) => { + invocation = args; + return 0; + }, + }), + ), + ).toBe(0); + expect(JSON.parse(invocation!.at(-1)!.split("\n").at(-1)!)).toEqual([ + "SYNTHETIC_EXTERNAL_FINDING\n", + ]); + } + } + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + + test.each( + process.platform === "win32" + ? ["symbolic link"] + : ["symbolic link", "FIFO"], + )("rejects finding files replaced with a %s", async (replacement) => { + const root = await mkdtemp(join(tmpdir(), "codex-security-skill-inputs-")); + try { + const repository = join(root, "repository"); + const selected = join(repository, "finding.txt"); + const external = join(root, "external.txt"); + await mkdir(repository); + await writeFile(selected, "ordinary finding\n"); + await writeFile(external, "SYNTHETIC_EXTERNAL_FINDING\n"); + const canonicalSelected = await filesystem.realpath(selected); + + const originalOpen = filesystem.open; + const opening = spyOn(filesystem, "open").mockImplementation( + async (...args: Parameters) => { + if (String(args[0]) === canonicalSelected) { + opening.mockRestore(); + await rm(selected); + if (replacement === "FIFO") execFileSync("mkfifo", [selected]); + else await symlink(external, selected); + } + return await originalOpen(...args); + }, + ); + + try { + let started = false; + const stderr = capture(); + expect( + await main( + ["validate", "finding.txt"], + capture().stream, + stderr.stream, + dependencies({ + currentDirectory: repository, + onCodex: () => { + started = true; + return 0; + }, + }), + ), + ).toBe(2); + expect(stderr.text()).not.toContain("SYNTHETIC_EXTERNAL_FINDING"); + expect(started).toBe(false); + } finally { + opening.mockRestore(); + } + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + test("preserves Windows network paths without probing them as finding files", async () => { const directory = await mkdtemp( join(tmpdir(), "codex-security-network-input-"),