From e7b326e7b694295ce2a2bac4874a71fd85e0ac49 Mon Sep 17 00:00:00 2001 From: zhangjinzn <2409663759@qq.com> Date: Fri, 31 Jul 2026 14:52:32 +0800 Subject: [PATCH] fix(security): harden local analysis boundaries Close the remaining PR #35 review gaps by canonicalizing cloc entrypoints, treating nested and safety-related scan skips as incomplete, containing Checkup backups, and redacting hierarchical URL userinfo. Also record the maintainer-requested task-scope rule and remove the proactive changelog entry. Implements AC-1 through AC-6 in docs/specs/2026-07-31-security-reliability-boundaries.md. Validated with Node 22.20.0 npm test (1031 tests), npm run pack:verify, the docs build, doc-link tests, and a complete changed-file secret scan. Co-authored-by: Codex (GPT 5.6 Sol) --- AGENTS.md | 6 ++ ...6-07-31-security-reliability-boundaries.md | 66 +++++++++++++ scripts/agent-guardrails/secret-scan.mjs | 27 ++++-- scripts/cloc/cli.mjs | 16 +++- .../coding-agent-practices/checkup/apply.mjs | 78 ++++++++++++++- .../session-analysis/privacy-safe-text.mjs | 2 + test/agent-guardrails-secret-scan.test.mjs | 96 ++++++++++++++++++- test/better-harness-cli.test.mjs | 37 +++++++ test/harness-checkup.test.mjs | 85 +++++++++++++++- test/session-analysis.test.mjs | 26 +++++ 10 files changed, 427 insertions(+), 12 deletions(-) create mode 100644 docs/specs/2026-07-31-security-reliability-boundaries.md diff --git a/AGENTS.md b/AGENTS.md index 13e11e6..dbac757 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -19,6 +19,12 @@ Adding support for a new Coding Agent host starts with - Keep `docs/specs/*.md` titles human-readable. Do not put Story ids, status, or review state in titles, and do not use YAML front matter by default. - Put traceability metadata in the body: `Spec ID`, optional `Story`, and `Status` in a short `## Traceability` section. +## Change Scope + +- Do not proactively edit `CHANGELOG.md`, release notes, version files, roadmap/status documents, or other task-external + project metadata. Change them only when the user explicitly requests it or a pre-existing issue, spec, or acceptance + criterion requires it; user-visible behavior alone is not authorization. + ## Test and Verify - Design scripts and code for AI-friendly automated use, and validate automation with an AI agent when relevant, diff --git a/docs/specs/2026-07-31-security-reliability-boundaries.md b/docs/specs/2026-07-31-security-reliability-boundaries.md new file mode 100644 index 0000000..767a259 --- /dev/null +++ b/docs/specs/2026-07-31-security-reliability-boundaries.md @@ -0,0 +1,66 @@ +# Harden local analysis and mutation boundaries + +## Traceability + +- Spec ID: security-reliability-boundaries +- Status: Implemented + +## Intent + +Restore a trustworthy local execution baseline for Better Harness by fixing the confirmed CLI entrypoint regression and closing three confirmed security/reliability gaps in source mutation, secret scanning, and session-text redaction. The outcome is that supported paths execute deterministically, authorized source and backup writes cannot escape through symbolic links, incomplete secret scans cannot report success, and review packets do not retain URL userinfo credentials. Keep task-external release metadata out of agent-authored changes unless a maintainer or pre-existing requirement authorizes it. + +## Acceptance Scenarios + +- AC-1: Direct and delegated `cloc --json` execution produces parser-safe JSON when the installation, repository, or target path contains spaces or is reached through an equivalent symbolic path. +- AC-2: A Checkup source patch rejects a source or backup path reached through a symbolic-link component or resolving outside its authorized root; ordinary in-root files and backups remain writable. +- AC-3: Secret Scan reports machine-readable incomplete coverage and returns a distinct non-zero exit code when an explicit target, nested symbolic link, or safety-sensitive file read cannot be inspected; a complete clean scan still exits zero, while secret findings preserve their existing finding exit behavior. +- AC-4: Session/review text redaction removes GitLab tokens and userinfo credentials from hierarchical URLs across HTTP, database, SSH, percent-encoded, and username/token-only forms. +- AC-5: Focused regression tests, the full root test suite, package verification, documentation build, and repository secret scan complete successfully. +- AC-6: Agent instructions prohibit proactive changelog, release-note, version, and task-external metadata edits unless the user or a pre-existing requirement authorizes them; user-visible behavior alone is insufficient. + +## Non-goals + +- Redesigning the Host capability registry, Evidence Bundle cache, report model, or CLI command registry. +- Changing scoring, finding generation, report prose, or host support claims. +- Adding remote Preview authentication or changing release automation in this change. +- Refactoring unrelated large modules or introducing a generic `scripts/core/` utility layer. +- Publishing an npm release. + +## Plan and Tasks + +1. Add RED coverage for a spaced symbolic installation path, then compare canonical real paths in the `cloc` direct-entrypoint check. +2. Add Checkup regressions for symbolic-link source parents and backup roots. Validate each backup directory component and canonical containment before writing without changing normal patch semantics. +3. Add Secret Scan CLI tests for unreadable/missing explicit paths, nested symbolic links, safety-sensitive read skips, and coverage status. Make incomplete scans fail closed with a stable distinct exit code while retaining current finding behavior. +4. Add parameterized privacy-safe text tests for GitLab and hierarchical URL userinfo credentials, then extend shared review-text sanitization to redact them. +5. Run focused tests after each RED→GREEN slice, then the complete verification gates. +6. Record the maintainer-requested change-scope rule in `AGENTS.md`, remove the proactive changelog entry, and perform traceability, security, and code-quality review before commit and merge. + +Affected owners are expected to remain limited to: + +- `scripts/cloc/` +- `scripts/coding-agent-practices/checkup/` +- `scripts/agent-guardrails/` +- `scripts/session-analysis/` +- their focused tests and this spec +- `AGENTS.md` for the explicit task-scope rule + +## Test and Review Evidence + +- AC-1: `node --test test/cloc.test.mjs test/better-harness-cli.test.mjs`, including the spaced symbolic installation path +- AC-2: focused Checkup tests in `test/harness-checkup.test.mjs` for source and backup symlink rejection +- AC-3: focused Secret Guard tests in `test/agent-guardrails-secret-scan.test.mjs`, including nested-link, safety-skip, exit-code, and JSON coverage assertions +- AC-4: focused privacy/session tests covering exact credential non-retention across hierarchical URL schemes +- AC-6: inspect the `AGENTS.md` diff and confirm `CHANGELOG.md` has no PR delta +- AC-5: + - `npm test` + - `npm run pack:verify` + - `node --test test/doc-link-graph.test.mjs` + - `npm ci && npm run build` from `docs/` + - repository secret scan over the final diff/worktree + +Risk notes: + +- Path containment changes must remain cross-platform and must not reject normal Windows/macOS/Linux in-root files. +- Secret Scan exit-code changes are intentional machine-contract behavior and require explicit regression coverage. +- Redaction must favor removing credential material over preserving diagnostic fidelity. +- No destructive external action is part of implementation; PR publication occurs only after verification and a sensitive-data scan. diff --git a/scripts/agent-guardrails/secret-scan.mjs b/scripts/agent-guardrails/secret-scan.mjs index fa3cd24..d52ccf1 100644 --- a/scripts/agent-guardrails/secret-scan.mjs +++ b/scripts/agent-guardrails/secret-scan.mjs @@ -392,6 +392,7 @@ export async function scanPaths(pathsToScan = ["."], options = {}) { return { tool: "secret-scan.mjs", version: VERSION, + coverageStatus: coverageStatus(stats), summary: summarize([], stats), findings: [], stats, @@ -415,12 +416,18 @@ export async function scanPaths(pathsToScan = ["."], options = {}) { return { tool: "secret-scan.mjs", version: VERSION, + coverageStatus: coverageStatus(stats), summary: summarize(filtered, stats), findings: filtered.map((finding) => publicFinding(finding, opts)), stats, }; } +function coverageStatus(stats) { + if (stats.errors.length === 0) return "complete"; + return stats.scannedFiles > 0 ? "partial" : "failed"; +} + function resolveHookPlatform({ platform, host } = {}) { if (platform && host && String(platform).trim().toLowerCase() !== String(host).trim().toLowerCase()) { throw new Error(`host (${host}) and platform (${platform}) must match when both are provided`); @@ -577,6 +584,11 @@ function isProtectedCredentialPath(filePath) { return PROTECTED_FILE_SEGMENTS.some((segment) => normalized === segment || normalized.endsWith(`/${segment}`) || normalized.includes(`/${segment}/`)); } +function recordCoverageGap(stats, file, reason) { + stats.skippedFiles += 1; + stats.errors.push(`${file}: ${reason}`); +} + async function collectFiles(absPath, out, opts, stats) { let stat; try { @@ -587,14 +599,14 @@ async function collectFiles(absPath, out, opts, stats) { } if (stat.isSymbolicLink()) { - stats.skippedFiles += 1; + recordCoverageGap(stats, absPath, "symbolic-link scan targets are not inspected"); return; } if (opts.containmentRoot) { try { const canonical = await fs.realpath(absPath); if (!isWithinRoot(opts.containmentRoot, canonical)) { - stats.skippedFiles += 1; + recordCoverageGap(stats, absPath, "scan target resolves outside the containment root"); return; } } catch (error) { @@ -632,6 +644,8 @@ async function collectFiles(absPath, out, opts, stats) { continue; } if (!shouldSkipFile(child, childStat, opts, stats)) out.push(child); + } else if (entry.isSymbolicLink()) { + await collectFiles(child, out, opts, stats); } } } @@ -663,18 +677,18 @@ async function readScannableFile(file, opts, stats) { try { beforeOpen = await fs.lstat(file); if (beforeOpen.isSymbolicLink() || !beforeOpen.isFile()) { - stats.skippedFiles += 1; + recordCoverageGap(stats, file, "scan target changed type before it could be read safely"); return null; } canonical = await fs.realpath(file); if (opts.containmentRoot && !isWithinRoot(opts.containmentRoot, canonical)) { - stats.skippedFiles += 1; + recordCoverageGap(stats, file, "scan target resolves outside the containment root"); return null; } handle = await openReadOnlyNoFollow(file); const opened = await handle.stat(); if (!opened.isFile() || !sameFileIdentity(beforeOpen, opened)) { - stats.skippedFiles += 1; + recordCoverageGap(stats, file, "scan target changed while it was being opened"); return null; } const buffer = await handle.readFile(); @@ -924,7 +938,8 @@ export async function main(argv = process.argv.slice(2)) { if (options.json) process.stdout.write(`${JSON.stringify(report, null, 2)}\n`); else process.stdout.write(printHuman(report, options)); - return report.findings.some((finding) => shouldFailFinding(finding, options.failOn)) ? 2 : 0; + if (report.findings.some((finding) => shouldFailFinding(finding, options.failOn))) return 2; + return report.coverageStatus === "complete" ? 0 : 3; } function validateFailOn(failOn) { diff --git a/scripts/cloc/cli.mjs b/scripts/cloc/cli.mjs index 75cd5d8..c6f6145 100644 --- a/scripts/cloc/cli.mjs +++ b/scripts/cloc/cli.mjs @@ -1,5 +1,9 @@ #!/usr/bin/env node +import { realpathSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + import { analyzeCloc } from "./analyze.mjs"; function parseArgs(argv = process.argv.slice(2)) { @@ -64,7 +68,17 @@ export async function main(argv = process.argv.slice(2)) { printTextReport(report); } -if (process.argv[1] && import.meta.url.endsWith(process.argv[1].replaceAll("\\", "/"))) { +function isDirectEntrypoint(entrypoint) { + if (!entrypoint) return false; + const currentFile = fileURLToPath(import.meta.url); + try { + return realpathSync(entrypoint) === realpathSync(currentFile); + } catch { + return path.resolve(entrypoint) === path.resolve(currentFile); + } +} + +if (isDirectEntrypoint(process.argv[1])) { main().catch((error) => { process.stderr.write(`cloc failed: ${error.stack ?? error.message}\n`); process.exitCode = 1; diff --git a/scripts/coding-agent-practices/checkup/apply.mjs b/scripts/coding-agent-practices/checkup/apply.mjs index 5d333bb..02d1907 100644 --- a/scripts/coding-agent-practices/checkup/apply.mjs +++ b/scripts/coding-agent-practices/checkup/apply.mjs @@ -1,10 +1,13 @@ import { spawnSync } from "node:child_process"; import { createHash, randomUUID } from "node:crypto"; +import { constants as fsConstants } from "node:fs"; import { chmod, copyFile, + lstat, mkdir, readFile, + realpath, rename, rm, stat, @@ -73,7 +76,68 @@ export function resolveSourceRef(sourceRef, options = {}) { if (!ALLOWED_SOURCE_EXTENSIONS.has(path.extname(filePath).toLowerCase())) { throw new Error("source patch target must be a supported instruction or hook source file"); } - return { filePath, workspace, key: sourceKey(sourceRef) }; + return { filePath, root, workspace, key: sourceKey(sourceRef) }; +} + +async function assertSafeSourceTarget({ filePath, root }) { + const canonicalRoot = await realpath(root); + const relativeTarget = path.relative(path.resolve(root), filePath); + let current = path.resolve(root); + for (const part of relativeTarget.split(path.sep)) { + current = path.join(current, part); + const metadata = await lstat(current); + if (metadata.isSymbolicLink()) { + throw new Error("source patch target path must not contain a symbolic link"); + } + } + const canonicalTarget = await realpath(filePath); + const canonicalRelative = path.relative(canonicalRoot, canonicalTarget); + if (canonicalRelative === "" || canonicalRelative === ".." || canonicalRelative.startsWith(`..${path.sep}`) || path.isAbsolute(canonicalRelative)) { + throw new Error("source patch target escapes its declared base"); + } +} + +function isWithinCanonicalRoot(root, target, { allowRoot = false } = {}) { + const relative = path.relative(root, target); + if (relative === "") return allowRoot; + return relative !== ".." && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative); +} + +async function ensureSafeBackupDirectory({ directoryPath, root }) { + const resolvedRoot = path.resolve(root); + const resolvedDirectory = path.resolve(directoryPath); + const relativeDirectory = path.relative(resolvedRoot, resolvedDirectory); + if (!isWithinCanonicalRoot(resolvedRoot, resolvedDirectory)) { + throw new Error("backup path escapes the workspace"); + } + + const canonicalRoot = await realpath(resolvedRoot); + let current = resolvedRoot; + for (const part of relativeDirectory.split(path.sep)) { + current = path.join(current, part); + let metadata; + try { + metadata = await lstat(current); + } catch (error) { + if (error.code !== "ENOENT") throw error; + try { + await mkdir(current); + } catch (mkdirError) { + if (mkdirError.code !== "EEXIST") throw mkdirError; + } + metadata = await lstat(current); + } + if (metadata.isSymbolicLink()) { + throw new Error("backup path must not contain a symbolic link"); + } + if (!metadata.isDirectory()) { + throw new Error("backup path components must be directories"); + } + const canonicalCurrent = await realpath(current); + if (!isWithinCanonicalRoot(canonicalRoot, canonicalCurrent, { allowRoot: true })) { + throw new Error("backup path escapes the workspace"); + } + } } function replaceLines(content, patch) { @@ -160,6 +224,7 @@ async function atomicReplace(filePath, content) { export async function applySourcePatch(action, options = {}) { const sourceRef = action.mutation?.sourceRef; const resolved = resolveSourceRef(sourceRef, options); + await assertSafeSourceTarget(resolved); const content = await readFile(resolved.filePath, "utf8"); const expectedFingerprint = action.sourceFingerprints?.[resolved.key]; if (!expectedFingerprint || sha256(content) !== expectedFingerprint) { @@ -172,8 +237,15 @@ export async function applySourcePatch(action, options = {}) { const backupRoot = path.join(resolved.workspace, ".better-harness-checkup-backups"); const relativeBackup = backupName(sourceRef, options.now); const backupPath = path.join(backupRoot, relativeBackup); - await mkdir(path.dirname(backupPath), { recursive: true }); - await copyFile(resolved.filePath, backupPath); + await ensureSafeBackupDirectory({ directoryPath: path.dirname(backupPath), root: resolved.workspace }); + await assertSafeSourceTarget(resolved); + await ensureSafeBackupDirectory({ directoryPath: path.dirname(backupPath), root: resolved.workspace }); + await copyFile(resolved.filePath, backupPath, fsConstants.COPYFILE_EXCL); + const canonicalWorkspace = await realpath(resolved.workspace); + const canonicalBackup = await realpath(backupPath); + if (!isWithinCanonicalRoot(canonicalWorkspace, canonicalBackup)) { + throw new Error("backup path escapes the workspace"); + } await atomicReplace(resolved.filePath, replacement); const verifiedContent = await readFile(resolved.filePath, "utf8"); let parser = "readable"; diff --git a/scripts/session-analysis/privacy-safe-text.mjs b/scripts/session-analysis/privacy-safe-text.mjs index 0e6ca5b..f20c482 100644 --- a/scripts/session-analysis/privacy-safe-text.mjs +++ b/scripts/session-analysis/privacy-safe-text.mjs @@ -64,6 +64,8 @@ export function sanitizePrivateReviewText(value, { limit = 800 } = {}) { .replace(/\b(?:authorization\s*:\s*)?bearer\s+[A-Za-z0-9._~+\/-]{8,}\b/giu, "Bearer ") .replace(/\b(api[_-]?key|access[_-]?token|auth[_-]?token|password|secret)\s*[:=]\s*[^\s,;]+/giu, "$1=") .replace(/\b(?:sk|ghp|github_pat|xox[abprs])[-_][A-Za-z0-9_-]{8,}\b/giu, "") + .replace(/\bglpat-[A-Za-z0-9_-]{20,}\b/giu, "") + .replace(/\b([a-z][a-z0-9+.-]*:\/\/)[^\s/?#@]+@/giu, "$1@") .replace(/\bAKIA[0-9A-Z]{12,}\b/gu, "") .replace(/(^|[\s("'`@])(?:\.{1,2}[\\/])?(?:[\p{L}\p{N}._-]+[\\/])+[\p{L}\p{N}._-]+/gmu, "$1") .replace(/(^|[^\p{L}\p{N}_])\/(?:Users|home|var|private|tmp|opt)\/[^\s"'`<>]+/gmu, "$1") diff --git a/test/agent-guardrails-secret-scan.test.mjs b/test/agent-guardrails-secret-scan.test.mjs index 8fb2874..bada576 100644 --- a/test/agent-guardrails-secret-scan.test.mjs +++ b/test/agent-guardrails-secret-scan.test.mjs @@ -195,7 +195,8 @@ test("workspace-contained scanning refuses a swap to an external symlink at read assert.equal(swapped, true); assert.equal(report.summary.totalFindings, 0); assert.equal(report.stats.scannedFiles, 0); - assert.ok(report.stats.errors.length > 0 || report.stats.skippedFiles > 0); + assert.equal(report.coverageStatus, "failed"); + assert.ok(report.stats.errors.length > 0); assert.doesNotMatch(JSON.stringify(report.findings), new RegExp(outsideKey)); }); @@ -258,6 +259,99 @@ test("secret-scan CLI hook mode reads JSON stdin and emits a block payload", () assert.equal(result.stderr, ""); }); +test("secret-scan CLI fails closed when an explicit path cannot be scanned", async (t) => { + const root = await mkdtemp(path.join(os.tmpdir(), "better-harness-secret-scan-missing-")); + t.after(() => rm(root, { recursive: true, force: true })); + const result = spawnSync(process.execPath, [scannerCli, "missing.txt", "--json"], { + cwd: root, + encoding: "utf8", + }); + + assert.equal(result.status, 3); + const report = JSON.parse(result.stdout); + assert.equal(report.coverageStatus, "failed"); + assert.ok(report.stats.errors.length > 0); + assert.equal(result.stderr, ""); +}); + +test("secret-scan CLI reports complete coverage and exits zero for a clean explicit file", async (t) => { + const root = await mkdtemp(path.join(os.tmpdir(), "better-harness-secret-scan-clean-")); + t.after(() => rm(root, { recursive: true, force: true })); + await writeFile(path.join(root, "clean.txt"), "ordinary fixture text\n"); + const result = spawnSync(process.execPath, [scannerCli, "clean.txt", "--json"], { + cwd: root, + encoding: "utf8", + }); + + assert.equal(result.status, 0); + const report = JSON.parse(result.stdout); + assert.equal(report.coverageStatus, "complete"); + assert.equal(report.summary.totalFindings, 0); + assert.deepEqual(report.stats.errors, []); + assert.equal(result.stderr, ""); +}); + +test("secret-scan CLI treats an explicitly supplied symbolic-link file as incomplete coverage", async (t) => { + const root = await mkdtemp(path.join(os.tmpdir(), "better-harness-secret-scan-link-")); + const target = path.join(root, "target.txt"); + const link = path.join(root, "linked.txt"); + t.after(() => rm(root, { recursive: true, force: true })); + await writeFile(target, "safe fixture\n"); + try { + await symlink(target, link); + } catch (error) { + t.skip(`symlink unavailable: ${error.message}`); + return; + } + + const result = spawnSync(process.execPath, [scannerCli, link, "--json"], { + cwd: root, + encoding: "utf8", + }); + + assert.equal(result.status, 3); + const report = JSON.parse(result.stdout); + assert.equal(report.coverageStatus, "failed"); + assert.ok(report.stats.errors.length > 0); +}); + +test("secret-scan CLI treats symbolic links nested in a scanned directory as incomplete coverage", async (t) => { + const root = await mkdtemp(path.join(os.tmpdir(), "better-harness-secret-scan-nested-link-")); + const scanRoot = path.join(root, "scan"); + const target = path.join(root, "target.txt"); + const link = path.join(scanRoot, "linked.txt"); + t.after(() => rm(root, { recursive: true, force: true })); + await mkdir(scanRoot, { recursive: true }); + await writeFile(target, "safe fixture\n"); + try { + await symlink(target, link); + } catch (error) { + t.skip(`symlink unavailable: ${error.message}`); + return; + } + + const result = spawnSync(process.execPath, [scannerCli, scanRoot, "--json"], { + encoding: "utf8", + }); + + assert.equal(result.status, 3); + const report = JSON.parse(result.stdout); + assert.equal(report.coverageStatus, "failed"); + assert.equal(report.stats.scannedFiles, 0); + assert.equal(report.stats.skippedFiles, 1); + assert.match(report.stats.errors.join("\n"), /symbolic-link scan targets are not inspected/u); + + await writeFile(path.join(scanRoot, "clean.txt"), "ordinary fixture text\n"); + const partialResult = spawnSync(process.execPath, [scannerCli, scanRoot, "--json"], { + encoding: "utf8", + }); + assert.equal(partialResult.status, 3); + const partialReport = JSON.parse(partialResult.stdout); + assert.equal(partialReport.coverageStatus, "partial"); + assert.equal(partialReport.stats.scannedFiles, 1); + assert.equal(partialReport.stats.skippedFiles, 1); +}); + test("install-secret-guard CLI keeps --host as a compatibility alias", () => { const result = spawnSync(process.execPath, [installerCli, "--host=codex", "--dry-run", "--json"], { encoding: "utf8", diff --git a/test/better-harness-cli.test.mjs b/test/better-harness-cli.test.mjs index 3d19c25..d98f7db 100644 --- a/test/better-harness-cli.test.mjs +++ b/test/better-harness-cli.test.mjs @@ -511,6 +511,43 @@ test("better-harness CLI preserves delegated cloc JSON output and spaced paths", } }); +test("cloc CLI runs from a spaced symlink installation path", async (t) => { + const root = await mkdtemp(path.join(os.tmpdir(), "better-harness cloc install-")); + const linkedClocDir = path.join(root, "linked cloc"); + const workspace = path.join(root, "workspace with spaces"); + t.after(() => rm(root, { recursive: true, force: true })); + await writeFixtureFile(workspace, "src/app.mjs", "export const value = 1;\n"); + try { + await symlink( + path.join(process.cwd(), "scripts", "cloc"), + linkedClocDir, + process.platform === "win32" ? "junction" : "dir", + ); + } catch (error) { + t.skip(`symlink unavailable: ${error.message}`); + return; + } + + const result = spawnSync(process.execPath, [ + path.join(linkedClocDir, "cli.mjs"), + "--cwd", + workspace, + "--json", + "--workers", + "1", + "--no-git", + ], { + cwd: workspace, + encoding: "utf8", + }); + + assert.equal(result.status, 0, result.stderr); + assert.notEqual(result.stdout.trim(), "", "cloc CLI must not silently skip direct execution"); + const report = JSON.parse(result.stdout); + assert.equal(report.kind, "cloc"); + assert.equal(report.totals.files, 1); +}); + test("better-harness CLI preserves deterministic delegated stdout byte-for-byte", () => { const input = `${JSON.stringify({ verdict: "consistent", diff --git a/test/harness-checkup.test.mjs b/test/harness-checkup.test.mjs index 550d32b..3d62b45 100644 --- a/test/harness-checkup.test.mjs +++ b/test/harness-checkup.test.mjs @@ -1,11 +1,12 @@ import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; -import { mkdtemp, mkdir, readFile, readdir, rm, writeFile } from "node:fs/promises"; +import { mkdtemp, mkdir, readFile, readdir, rm, symlink, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import test from "node:test"; import { + applySourcePatch, applyCheckupPlan, createQoderCliExecutor, resolveSourceRef, @@ -587,6 +588,88 @@ test("source patch resolution rejects traversal, generated runtime files, and pl ); }); +test("source patch rejects symbolic links in the target path", async (t) => { + const root = await mkdtemp(path.join(os.tmpdir(), "harness-checkup-symlink-")); + const workspace = path.join(root, "workspace"); + const outside = path.join(root, "outside"); + const original = '{\n "enabled": true\n}\n'; + const action = { + mutation: { + sourceRef: { base: "workspace", relativePath: "linked/settings.json" }, + patch: { + type: "replace-lines", + startLine: 2, + endLine: 2, + expectedHash: sha256(' "enabled": true'), + replacement: ' "enabled": false', + }, + }, + sourceFingerprints: { + "workspace:linked/settings.json": sha256(original), + }, + }; + t.after(() => rm(root, { recursive: true, force: true })); + await mkdir(workspace, { recursive: true }); + await mkdir(outside, { recursive: true }); + await writeFile(path.join(outside, "settings.json"), original); + try { + await symlink(outside, path.join(workspace, "linked"), "dir"); + } catch (error) { + t.skip(`symlink unavailable: ${error.message}`); + return; + } + + await assert.rejects( + applySourcePatch(action, { workspace, now: NOW }), + /symbolic link/u, + ); + assert.equal(await readFile(path.join(outside, "settings.json"), "utf8"), original); +}); + +test("source patch rejects a symbolic-link backup root", async (t) => { + const root = await mkdtemp(path.join(os.tmpdir(), "harness-checkup-backup-symlink-")); + const workspace = path.join(root, "workspace"); + const outside = path.join(root, "outside"); + const sourcePath = path.join(workspace, "settings.json"); + const original = '{\n "enabled": true\n}\n'; + const action = { + mutation: { + sourceRef: { base: "workspace", relativePath: "settings.json" }, + patch: { + type: "replace-lines", + startLine: 2, + endLine: 2, + expectedHash: sha256(' "enabled": true'), + replacement: ' "enabled": false', + }, + }, + sourceFingerprints: { + "workspace:settings.json": sha256(original), + }, + }; + t.after(() => rm(root, { recursive: true, force: true })); + await mkdir(workspace, { recursive: true }); + await mkdir(outside, { recursive: true }); + await writeFile(sourcePath, original); + try { + await symlink( + outside, + path.join(workspace, ".better-harness-checkup-backups"), + process.platform === "win32" ? "junction" : "dir", + ); + } catch (error) { + t.skip(`symlink unavailable: ${error.message}`); + return; + } + + await assert.rejects( + applySourcePatch(action, { workspace, now: NOW }), + /backup path must not contain a symbolic link/u, + ); + assert.equal(await readFile(sourcePath, "utf8"), original); + assert.deepEqual(await readdir(outside), []); +}); + test("same sanitized Hook identity with different arguments never produces a deduplication action", () => { const workspace = "/tmp/better-harness-checkup-hook-distinct"; const filePath = path.join(workspace, ".qoder", "settings.json"); diff --git a/test/session-analysis.test.mjs b/test/session-analysis.test.mjs index 40f7fe8..4c60671 100644 --- a/test/session-analysis.test.mjs +++ b/test/session-analysis.test.mjs @@ -399,6 +399,32 @@ test("privacy-safe user input summaries skip injected context and redact private assert.equal(hostContextSummary.includes("continuation state"), false); }); +test("privacy-safe user input summaries redact GitLab tokens and hierarchical URL userinfo", () => { + // secret-scan: allow synthetic provider-shaped regression fixture + const gitlabToken = ["glpat", "AbCdEfGhIjKlMnOpQrStUvWx"].join("-"); + const tokenSummary = privacySafeUserInputSummary([{ + type: "user", + userText: `Use ${gitlabToken}`, + }]); + + assert.ok(tokenSummary); + assert.doesNotMatch(tokenSummary, new RegExp(gitlabToken)); + + const cases = [ + ["https://build-user:url-password-value@git.example.test/repo.git", ["build-user", "url-password-value"]], + ["postgresql://db-user:db-password@db.example.test/database", ["db-user", "db-password"]], + ["ssh://ssh-user:ssh-password@git.example.test/repo", ["ssh-user", "ssh-password"]], + ["postgresql://db%2Duser:db%40password@db.example.test/database", ["db%2Duser", "db%40password"]], + ["https://token-only-value@git.example.test/repo", ["token-only-value"]], + ]; + for (const [url, credentials] of cases) { + const summary = privacySafeUserInputSummary([{ type: "user", userText: `Fetch ${url}` }]); + assert.ok(summary); + assert.match(summary, /@/u); + for (const credential of credentials) assert.equal(summary.includes(credential), false); + } +}); + async function makeQoderFixture() { const root = await mkdtemp(path.join(os.tmpdir(), "better-harness-session-analysis-")); const workspace = path.join(root, "workspace", "better-harness");