Skip to content

FE-1320: Structural private-mission isolation for /compare-specs - #415

Open
lunelson wants to merge 4 commits into
ln/fe-1318-canonical-document-reconciliationfrom
ln/fe-1320-comparison-mission-isolation
Open

FE-1320: Structural private-mission isolation for /compare-specs#415
lunelson wants to merge 4 commits into
ln/fe-1318-canonical-document-reconciliationfrom
ln/fe-1320-comparison-mission-isolation

Conversation

@lunelson

@lunelson lunelson commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Why

Our /compare-specs workflow runs two AI products side by side on the same task to compare their output. The instructions that drive that comparison are private, sensitive context that must never leak into what either product being tested can see or touch — including via something as simple as a relative file path that happens to reach outside the test's own folder.

What

  • Each comparison run now gets its own fresh, temporary folder living completely outside our repository and its mission files, closing off any path from a tested product's workspace back to anything private.
  • Verified this holds even against a deliberately adversarial path designed to escape the sandboxed folder.
  • Confirmed that everything each tested product is supposed to produce is still preserved exactly, and that past historical test evidence is untouched by this change.
  • This closes the structural/file-placement half of the isolation problem. A live end-to-end run with real product accounts, and proving that revising a mission doesn't rewrite past results, is separate operator-scheduled follow-up work.

How to test

  1. npm run test:comparison passes.
  2. The isolation-specific cases (visibility boundary, adversarial path rejection, output/history preservation) are green.

lunelson and others added 4 commits August 5, 2026 15:57
Place each harness in a fresh external temporary root so controller-private mission material is absent from ordinary target-relative traversal while retained evidence stays exact.

Co-authored-by: Cursor <cursoragent@cursor.com>
Keep the external-target contract coupled to the existing single-actor, immutable-evidence, and prompt-owned workflow boundaries.

Co-authored-by: Cursor <cursoragent@cursor.com>
Return I67-L to its pre-frontier content so any evidence refresh remains owned by ln-sync.

Co-authored-by: Cursor <cursoragent@cursor.com>

lunelson commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

@lunelson lunelson changed the title FE-1320: Isolate comparison mission targets FE-1320: Structural private-mission isolation for /compare-specs Aug 7, 2026
@lunelson
lunelson marked this pull request as ready for review August 7, 2026 16:16
Copilot AI review requested due to automatic review settings August 7, 2026 16:16
@cursor

cursor Bot commented Aug 7, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes how comparison runs isolate sensitive mission data; impact is prompt-driven operator procedure plus tests, not product runtime code, but mistakes could still leak private missions or drop evidence if the procedure is misapplied.

Overview
FE-1320 hardens /compare-specs so private mission material is separated by filesystem placement, not only by agent conduct.

The operator prompt now requires each comparison harness to run in a fresh system-temporary target root outside the controller checkout, scratch comparisons tree, and retained run tree, with setup preflight that rejects ancestor paths that could reach controller-private files (including the witnessed ../../private-mission.md rival). Brunch still launches from the repo root and uses --workspace; other harnesses use that external directory as cwd. The run procedure also copies transcripts and harness-authored output into controller-owned evidence after process cleanup, then deletes the external target before the next harness. Docs explicitly bound this to ordinary relative traversal—not an OS sandbox or whole-host isolation.

Tests extend compare-specs-prompt.test.ts with prompt contract assertions plus temp-dir fixtures for target visibility, the CS2 rival path, and byte-stable retention after target cleanup.

Planning/spec updates mark comparison-mission-isolation-hardening complete, refresh D134-L/I67-L and verification prose, and close TESTING_FINDINGS CS2 placement risk; live Brunch + Claude witness work stays on saved-mission-comparison-witness.

Reviewed by Cursor Bugbot for commit 14deb5d. Bugbot is set up for automated code reviews on this repo. Configure here.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens /compare-specs private-mission isolation by changing the workflow to allocate each harness a fresh, system-temporary external target root outside controller-owned trees, and updates the written procedure + planning docs to match.

Changes:

  • Update the /compare-specs operator prompt to require per-harness external target roots and specify copy/cleanup sequencing.
  • Add/extend tests around the prompt contract and the external-target placement invariants.
  • Update SPEC/PLAN/testing notes to record FE-1320 as the closeout of the prior placement-risk finding.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
TESTING_FINDINGS.md Updates CS2 disposition to reflect FE-1320 closing the placement risk.
src/dev/tests/compare-specs-prompt.test.ts Adds prompt assertions and filesystem-based isolation/retention tests.
memory/SPEC.md Updates D134-L and I67-L to include external target placement boundary language.
memory/PLAN.md Marks the isolation hardening frontier as completed and removes it from “Next”.
docs/archive/PLAN_HISTORY.md Archives the FE-1320 closeout entry and its evidence summary.
.pi/prompts/compare-specs.md Implements the new external-target-root procedure and retention/cleanup ordering in the operator prompt.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +131 to +132
await rm(externalRoot, { recursive: true });
await cp(scratchRun, retainedRun, { recursive: true });

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 14deb5d. Configure here.

7. Acquire the harness-authored document if it exists. Never author, reconstruct, complete, rewrite, or improve it. Missing or partial output remains missing or partial.
8. On every outcome, retain state, harness-visible interaction, target-cwd/session identity, final process status, document that exists, and cleanup notes. Kill remaining processes, dismiss the completed shell record, and verify no comparison-harness shell or process remains before starting another.
7. Locate the harness-authored document if it exists without altering it. Never author, reconstruct, complete, rewrite, or improve it. Missing or partial output remains missing or partial.
8. On every outcome, kill remaining processes, dismiss the completed shell record, and verify no comparison-harness shell or process remains. Only after that process cleanup, copy the exact harness-visible transcript and any unchanged harness-authored document from the external target into controller-owned scratch evidence; retain target-cwd/session identity, final process status, and cleanup notes. Then remove that harness's external target root and verify it is gone before starting another harness.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Transcript salvage uses wrong source

Medium Severity

The new cleanup step says to copy the harness-visible transcript from the external target after dismissing the shell record. That transcript is the controller-authored disclosure record from the observed interaction, not a file in the target. Only the harness-authored document needs salvage from the external root, so this order and source can drop or miss the disclosure record.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 14deb5d. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants