diff --git a/backend/scripts/p1-acceptance.mjs b/backend/scripts/p1-acceptance.mjs index 192eeeee..f086f89e 100755 --- a/backend/scripts/p1-acceptance.mjs +++ b/backend/scripts/p1-acceptance.mjs @@ -400,7 +400,7 @@ export async function resolveProductionExecutables({ repositoryRoot } = {}) { let fallbackGitHooksPath; function ownedFallbackGitHooksPath() { - if (!fallbackGitHooksPath) fallbackGitHooksPath = mkdtempSync(join(tmpdir(), `p1-git-hooks-${process.pid}-`)); + if (!fallbackGitHooksPath) fallbackGitHooksPath = realpathSync(mkdtempSync(join(tmpdir(), `p1-git-hooks-${process.pid}-`))); return fallbackGitHooksPath; } function gitSafeConfig(hooksPath) { @@ -533,6 +533,80 @@ function assertSafeGitRepositoryState(argv, cwd, runRoot, fixedHooksPath) { validateGitDirectoryState(realpathSync(target), runRoot); } } +function rawGitCommonDirectory(gitDirectory) { + const path = join(gitDirectory, "commondir"); + if (!existsSync(path)) return gitDirectory; + const entry = lstatSync(path); + const value = entry.isFile() && !entry.isSymbolicLink() ? readFileSync(path, "utf8") : ""; + if (!/^[^\0\n\r]+\n?$/.test(value)) throw new Error("unsafe Git repository state: common directory is unsafe"); + const common = resolve(gitDirectory, value.trimEnd()); + const commonEntry = lstatSync(common); + if (!commonEntry.isDirectory() || commonEntry.isSymbolicLink() || realpathSync(common) !== common) { + throw new Error("unsafe Git repository state: common directory is unsafe"); + } + return common; +} +function rawGitConfigEntries(path, required = false) { + if (!existsSync(path)) { + if (required) throw new Error("unsafe Git repository state: local config is unavailable"); + return []; + } + const entry = lstatSync(path); + if (!entry.isFile() || entry.isSymbolicLink()) throw new Error("unsafe Git repository state: local config is unsafe"); + return parseLocalGitConfig(readFileSync(path, "utf8")); +} +function unsafeRawGitConfigKey(key) { + const normalized = key.toLowerCase(); + return /^filter\..+\.(?:clean|smudge|process|required)$/.test(normalized) + || /^remote\..+\.(?:uploadpack|receivepack)$/.test(normalized) + || /^(?:core\.(?:hookspath|attributesfile)|diff\.external|interactive\.difffilter)$/.test(normalized) + || /^include(?:if\..+)?\.path$/.test(normalized); +} +function validateRawGitDirectoryState(gitDirectory) { + const common = rawGitCommonDirectory(gitDirectory); + const entries = [ + ...rawGitConfigEntries(join(common, "config"), true), + ...rawGitConfigEntries(join(gitDirectory, "config.worktree")), + ]; + if (entries.some(([key]) => unsafeRawGitConfigKey(key))) { + throw new Error("unsafe Git repository state: executable local config is present"); + } + for (const hooks of new Set([join(common, "hooks"), join(gitDirectory, "hooks")])) { + if (!existsSync(hooks)) continue; + const hooksEntry = lstatSync(hooks); + if (!hooksEntry.isDirectory() || hooksEntry.isSymbolicLink()) throw new Error("unsafe Git repository state: repository hooks are unsafe"); + for (const entry of readdirSync(hooks, { withFileTypes: true })) { + if (entry.isSymbolicLink() || !entry.isFile() || !entry.name.endsWith(".sample")) { + throw new Error("unsafe Git repository state: repository hook is present"); + } + } + } + return entries; +} +function rawRepositoryGitDirectory(argv, cwd) { + if (argv[0] === "--git-dir") return repositoryGitDirectory(argv, cwd); + let current = argv[0] === "-C" ? argv[1] : cwd; + if (!current) return undefined; + current = realpathSync(current); + while (true) { + if (existsSync(join(current, ".git"))) return repositoryGitDirectory(["-C", current]); + const parent = dirname(current); + if (parent === current) return undefined; + current = parent; + } +} +function assertSafeRawGitRepositoryState(argv, cwd, fixedHooksPath) { + assertEmptyHooksDirectory(fixedHooksPath); + const gitDirectory = rawRepositoryGitDirectory(argv, cwd); + const entries = gitDirectory ? validateRawGitDirectoryState(gitDirectory) : []; + const target = exactRemoteTarget(argv, entries); + if (target !== undefined) { + if (!ownedGitPath(target) || !existsSync(join(target, "config"))) { + throw new Error("unsafe Git repository state: remote target is not exact and owned"); + } + validateRawGitDirectoryState(realpathSync(target)); + } +} const FIXTURE_GIT_CONFIG = new Map([ ["user.name", new Set(["P1 Fixture Curator", "P1 Context Curator"])], @@ -847,16 +921,24 @@ export async function runCommand(options) { const allowedTht = activeExecutablePolicy?.thtPath; if (canonical !== allowedGit && canonical !== allowedTht) policyError("command executable is not allowlisted", details()); if (!Array.isArray(argv) || argv.some((value) => typeof value !== "string")) policyError("command argv must be a string array", details()); + const guardedByProduction = productionSurfaceOwner !== undefined; + let rawGitHooksPath; let rawGitArgv; try { - if (canonical === allowedGit) validateGitInvocation(argv, { runRoot: activeExecutablePolicy?.runRoot }); + if (canonical === allowedGit) { + validateGitInvocation(argv, { runRoot: activeExecutablePolicy?.runRoot }); + if (!guardedByProduction) { + rawGitHooksPath = ownedFallbackGitHooksPath(); + rawGitArgv = argv[0] === "-c" ? argv.slice(2) : argv; + assertSafeRawGitRepositoryState(rawGitArgv, cwd, rawGitHooksPath); + } + } if (canonical === allowedTht) validateThtInvocation(argv, { thtPath: allowedTht, runRoot: activeExecutablePolicy.runRoot, cwd }); } catch (error) { policyError(error.message, details()); } if (!Number.isSafeInteger(timeoutMs) || timeoutMs < 1 || timeoutMs > 300_000 || !Number.isSafeInteger(maxOutputBytes) || maxOutputBytes < 1 || maxOutputBytes > MAX_OUTPUT) { policyError("command bounds are invalid", details()); } return await new Promise((resolvePromise, reject) => { - const guardedByProduction = productionSurfaceOwner !== undefined; - const childArgv = canonical === allowedGit && !guardedByProduction ? hardenedGitArgv(argv, ownedFallbackGitHooksPath()) : argv; + const childArgv = canonical === allowedGit && !guardedByProduction ? hardenedGitArgv(rawGitArgv, rawGitHooksPath) : argv; const childEnv = canonical === allowedGit && !guardedByProduction ? { ...(env ?? {}), ...baseSafeGitEnvironment(canonical) } : env; const child = mutableChildProcess.execFile(canonical, childArgv, { cwd, env: childEnv, timeout: timeoutMs, maxBuffer: maxOutputBytes, encoding: "utf8", shell: false }, (error, stdout, stderr) => { const code = error && typeof error.code === "number" ? error.code : error ? 1 : 0; diff --git a/backend/scripts/p1-acceptance.test.mjs b/backend/scripts/p1-acceptance.test.mjs index 36050403..8ee57ecb 100644 --- a/backend/scripts/p1-acceptance.test.mjs +++ b/backend/scripts/p1-acceptance.test.mjs @@ -293,6 +293,30 @@ test("command helper accepts only executable plus separate argv", async () => { assert.equal(result.code, 0); }); +test("raw runCommand rejects a configured clean filter before exact Git add", async () => { + const repositoryRoot = await fakeRepository(); + const content = join(repositoryRoot, "workspace-content"); + const helper = join(repositoryRoot, "clean-helper"); + const marker = join(repositoryRoot, "clean-helper-ran"); + await execFileAsync("/usr/bin/git", ["init", "--initial-branch=main"], { cwd: repositoryRoot }); + await execFileAsync("/usr/bin/git", ["config", "user.name", "P1 Fixture Curator"], { cwd: repositoryRoot }); + await execFileAsync("/usr/bin/git", ["config", "user.email", "p1-curator@example.invalid"], { cwd: repositoryRoot }); + await mkdir(content); + await writeFile(join(content, "guide.md"), "content\n"); + await writeFile(join(repositoryRoot, ".gitattributes"), "workspace-content/** filter=bad\n"); + await writeFile(helper, `#!/bin/sh\nprintf ran > '${marker}'\ncat\n`, { mode: 0o700 }); + await execFileAsync("/usr/bin/git", ["config", "filter.bad.clean", `'${helper}'`], { cwd: repositoryRoot }); + + const { gitPath } = await resolveProductionExecutables({ + repositoryRoot: await realpath(join(dirname(fileURLToPath(import.meta.url)), "..", "..")), + }); + await assert.rejects( + runCommand({ executable: gitPath, argv: ["add", "workspace-content"], cwd: repositoryRoot, env: process.env }), + /unsafe Git repository state/, + ); + await assert.rejects(lstat(marker)); +}); + test("safe environment rejects ambient THT and keeps only strict process allowlist plus fixture values", () => { const safe = buildSafeEnvironment({ @@ -350,7 +374,7 @@ test("announce callback observes PASS and manual pending before non-keep cleanup test("public wrapper replaces ambient environment before invoking the runner", async () => { const wrapper = await readFile(join(dirname(fileURLToPath(import.meta.url)), "..", "..", "scripts", "p1-acceptance.sh"), "utf8"); assert.match(wrapper, /safe_env=\(\/usr\/bin\/env -i/); - assert.match(wrapper, /P1_ACCEPTANCE_FAIL_AT/); + assert.doesNotMatch(wrapper, /P1_ACCEPTANCE_FAIL_AT|LANG|LC_ALL|TZ/); assert.doesNotMatch(wrapper, /export THT_BIN/); }); @@ -842,15 +866,26 @@ test("secret scan fails closed on a recoverable symlink outside fixture-secrets" await assert.rejects(scanSecrets({ runRoot: run.root, forbiddenValues: [canary], expectedGitRepositories: [] }), /symlink outside fixture-secrets/); }); -test("direct public wrapper execution cannot source ambient BASH_ENV or ENV", async () => { +test("direct public wrapper clears startup files and exported functions before Bash starts", async () => { const root = await fakeRepository(); - const startup = join(root, "startup"); - const marker = join(root, "ambient-shell-ran"); - await writeFile(startup, `printf sourced > '${marker}'\n`); + const bashStartup = join(root, "bash-startup"); + const envStartup = join(root, "env-startup"); + const bashMarker = join(root, "bash-env-ran"); + const envMarker = join(root, "env-ran"); + const functionMarker = join(root, "exported-function-ran"); + await writeFile(bashStartup, `printf sourced > '${bashMarker}'\n`); + await writeFile(envStartup, `printf sourced > '${envMarker}'\n`); const wrapper = join(dirname(fileURLToPath(import.meta.url)), "..", "..", "scripts", "p1-acceptance.sh"); - await assert.rejects(execFileAsync(wrapper, ["invalid"], { env: { ...process.env, BASH_ENV: startup, ENV: startup } })); - await assert.rejects(lstat(marker)); - assert.match(await readFile(wrapper, "utf8"), /^#!\/usr\/bin\/env -S -u BASH_ENV -u ENV \/bin\/bash\n/); + await assert.rejects(execFileAsync(wrapper, ["invalid"], { + env: { + ...process.env, + BASH_ENV: bashStartup, + ENV: envStartup, + "BASH_FUNC_cd%%": `() { printf function > '${functionMarker}'; builtin cd "$@"; }`, + }, + })); + for (const marker of [bashMarker, envMarker, functionMarker]) await assert.rejects(lstat(marker)); + assert.match(await readFile(wrapper, "utf8"), /^#!\/usr\/bin\/env -S -i PATH=\/usr\/bin:\/bin \/bin\/bash\n/); }); test("final listener ownership state is a declared hash-bound report artifact", async () => { diff --git a/scripts/p1-acceptance.sh b/scripts/p1-acceptance.sh index 2114d27c..b58b3a01 100755 --- a/scripts/p1-acceptance.sh +++ b/scripts/p1-acceptance.sh @@ -1,4 +1,4 @@ -#!/usr/bin/env -S -u BASH_ENV -u ENV /bin/bash +#!/usr/bin/env -S -i PATH=/usr/bin:/bin /bin/bash set -euo pipefail script_path=${BASH_SOURCE[0]} script_dir=${script_path%/*} @@ -51,19 +51,12 @@ wrapper_root=$(/usr/bin/mktemp -d /tmp/thoth-p1-wrapper.XXXXXXXX) trap '/bin/rm -rf -- "$wrapper_root"' EXIT HUP INT TERM /bin/mkdir -m 700 "$wrapper_root/home" "$wrapper_root/tmp" owned_path="${node_path%/*}:/usr/bin:/bin" -build_env=(/usr/bin/env -i "PATH=$owned_path" "HOME=$wrapper_root/home" "TMPDIR=$wrapper_root/tmp" "LANG=${LANG:-C}") -for name in LC_ALL TZ; do - [[ -n "${!name:-}" ]] && build_env+=("$name=${!name}") -done +build_env=(/usr/bin/env -i "PATH=$owned_path" "HOME=$wrapper_root/home" "TMPDIR=$wrapper_root/tmp") /bin/rm -rf -- "$repo_root/backend/dist" "${build_env[@]}" "$node_path" "$npm_path" --prefix "$repo_root/backend" run build -safe_env=(/usr/bin/env -i "PATH=$owned_path" "HOME=$wrapper_root/home" "TMPDIR=$wrapper_root/tmp" "LANG=${LANG:-C}" +safe_env=(/usr/bin/env -i "PATH=$owned_path" "HOME=$wrapper_root/home" "TMPDIR=$wrapper_root/tmp" "THT_BIN=$repo_root/harness/.venv/bin/tht" "P1_ACCEPTANCE_NODE_PATH=$node_path" "P1_ACCEPTANCE_NPM_PATH=$npm_path") -for name in LC_ALL TZ; do - [[ -n "${!name:-}" ]] && safe_env+=("$name=${!name}") -done -[[ -n "${P1_ACCEPTANCE_FAIL_AT:-}" ]] && safe_env+=("P1_ACCEPTANCE_FAIL_AT=$P1_ACCEPTANCE_FAIL_AT") set +e "${safe_env[@]}" "$node_path" "$repo_root/backend/scripts/p1-acceptance.mjs" "$@" status=$?