Fail-Closed Review and Privacy Boundaries

Traceability

• Spec ID: review-boundary-hardening-11-18
• Story: #11, #12, #13, #14, #15, #16, #17, #18
• Status: Implemented

Intent

Make Better Harness honor its declared privacy controls and fail closed when review, filesystem, or report-integrity boundaries cannot be verified. The change fixes eight independently reproduced High findings without expanding the product surface or changing successful-path report semantics.

Acceptance Scenarios

• AC-01 (#11): Secret Guard normalizes supported snake_case and camelCase tool payloads, scans secret-bearing Write/Edit/apply_patch content, blocks synthetic secrets without echoing them, and preserves existing Bash/path decisions.
• AC-02 (#12): Workspace-scoped Qoder analysis excludes home-only sessions that have no verified relationship to the requested workspace across sessions, events/show, facets, insights, file-reads, and usage-summary. A matching record cannot authorize foreign-cwd records from the same home session, and cwd-less records require a workspace-linked session source instead of being assigned to the requested workspace. An explicit global-capability pass can retain home-only sessions only as user-global evidence.
• AC-03 (#13): report --no-sessions executes no session probe and always uses the static software-fluency route even when local sessions exist.
• AC-04 (#14): Blast-radius collection distinguishes an invalid/unavailable base ref from a real empty diff and returns an explicit fail-closed error.
• AC-05 (#15): review-trigger argument, Git worktree probes, and runtime failures exit non-zero while successful findings retain their documented non-blocking result.
• AC-06 (#16): report --cwd rejects empty, missing, and non-directory targets before starting evidence collectors; valid-directory fallback behavior remains available.
• AC-07 (#17): findings repair never lowers Critical or arbitrary unknown severity to Medium; unsupported values fail closed or use an explicit conservative mapping.
• AC-08 (#18): Git-backed cloc never follows a tracked path to a target outside the repository, reads a non-regular file, or reads a regular file beyond the configured size boundary; skipped results remain bounded and do not reveal the external target.
• AC-09: Focused tests, the full Node test suite, package verification, syntax checks, and review-readiness checks pass on Linux-compatible local tooling; cross-platform path behavior remains covered by portable fixtures.

Non-goals

• Do not redesign report scoring, add a new severity level to the public findings schema, or alter valid High/Medium/Low findings.
• Do not add authentication or network sharing to Canvas preview.
• Do not change the Medium large-untracked-file finding or architecture watchlist items from the originating review.
• Do not add dependencies.
• Do not refactor unrelated session, report, or Git-analysis behavior.

Plan and Tasks

1. Add failing regression tests for each issue before changing implementation.
2. Normalize Secret Guard tool events once, then scan only the content fields owned by matched write tools.
3. Apply workspace ownership at both Qoder home-session discovery and per-event hydration, keeping explicit user-global behavior separate.
4. Make quickstart privacy and empty/missing cwd validation explicit at its CLI boundary.
5. Replace Git diff ambiguity with a structured failure from blast-radius collection.
6. Separate review-trigger argument, Git-probe, and execution failures from successful non-blocking findings.
7. Make severity repair conservative and make cloc file reads type-, size-, and containment-aware.
8. Run focused tests per module, then the complete repository checks.
9. Perform independent code-review and architecture lanes, address all Critical/High findings and any architectural Block, then run Review Readiness before commit and PR.

Decision rationale:

• Fail closed only when the tool cannot establish the requested boundary; preserve supported fallback behavior for valid inputs with partial evidence.
• Prefer small checks at existing owner boundaries over new shared abstraction layers.
• Use synthetic credentials and temporary repositories only; tests must not retain private prompts, paths, or secrets.

Test and Review Evidence

• AC-01: node --test test/agent-guardrails-secret-scan.test.mjs
• AC-02: node --test test/session-analysis.test.mjs test/session-usage-summary.test.mjs including mixed-cwd, cwd-less, and explicit user-global home-session cases
• AC-03 and AC-06: node --test test/harness-quickstart.test.mjs test/better-harness-cli.test.mjs
• AC-04: node --test test/blast-radius.test.mjs
• AC-05: node --test test/review-trigger.test.mjs, including a non-Git directory
• AC-07: node --test test/harness-findings-repair.test.mjs test/harness-report-render-cli.test.mjs
• AC-08: node --test test/cloc.test.mjs, including an oversized regular file
• AC-09: npm testnpm run pack:verifynode --test test/doc-link-graph.test.mjs

Review evidence:

• Review follow-up (2026-07-29): mixed-cwd and cwd-less home-session hydration, explicit user-global analysis, failed Git worktree probes, empty --cwd=, and bounded cloc reads are covered by regressions and resolved before merge.
• npm run check passed with the full Node suite and package verification.
• The review follow-up focused run passed 80 tests across Qoder session analysis, session usage summary, review-trigger, quickstart, and cloc; syntax and diff checks also passed.
• Code-reviewer recommendation: APPROVE after both reported High findings were fixed and re-reviewed.
• Architect status: non-blocking WATCH; the previous fail-open BLOCK was resolved. The remaining watch is limited to the concurrent swap-to-symlink race on platforms where O_NOFOLLOW is unavailable.
• The final diff must contain no generated runtime state, credentials, or unrelated source changes.

Risk notes:

• Qoder home-session filtering can reduce previously over-broad results; tests must prove verified workspace sessions remain visible.
• Hook exit-code changes can expose previously hidden configuration errors; success-path non-blocking tests guard the intended contract.
• Symlink policy differs by platform; fixtures must avoid requiring privileged symlink creation where the platform does not support it.