fix(lsp): re-check physical containment after server startup, right before didOpen (#1268)

[Juliusolsson05]

Refs #1268. OWNER-APPROVED (B6 proxy, 2026-09-27): narrowing accepted. This PR NARROWS the LSP open window; it does not close #1268 (see the limit below).

Plan: docs/plans/2026-09-27-lsp-open-containment.md.

Problem

authorizeContext (src/main/ipc/lsp.ts) validated the physical target and returned the checked relative path. LspManager.openDocumentNow then awaited server startup, which can take seconds on a cold spawn. Only after that did it build a lexical file:// URI and send didOpen. If a directory on the path was swapped for a symlink to an outside directory in that window, the server was handed a URI whose realpath is outside the root. A fresh open at that moment was correctly refused, so only the in-flight one escaped.

What merges

• OpenDocumentParams.assertPhysicalTarget: a callback from the authorizing caller. The manager awaits it inside the per-document queue, after server startup and immediately before every didOpen, including shared joins (review a). A refusal fails open like every other LSP failure (returns false: no LSP for that document) and names nothing to the server.
• lspPhysicalTargetAssertion(context) (src/main/ipc/lsp.ts), passed by both open paths (lsp:open-document, lsp:reopen-document):
  • first, the canonical root must still resolve to itself (review b: a root renamed away and replaced by a symlink out);
  • then the authorization rule is re-run: resolveInsideRoot + validateExistingTarget (no symlink, canonical inside the root), a regular file;
  • a pathless (virtual) document checks that its .agent-code-lsp directory is not a symlink out, AND that the virtual-<hash>.<ext> leaf didOpen names is absent or a regular file inside the root (review c: a leaf symlink created in advance escaped with no timing window). The name comes from the manager's own lspVirtualDocumentName. A pathless open without it is refused;
  • a case-only rename is allowed.
• Why a callback and not a filesystem check in the manager: the IPC layer owns authorization, and the manager's tests run on fake roots.

The limit (why Refs, not Fixes)

LSP is path-based: the language server re-resolves every path itself, later, and the server starts in the root path (cwd, rootUri). A local actor who can rename directories inside the project can still swap a path between the last check here and the server's own read, or during server startup (review b, findings 1 and 3). No check in main can close that. Closing it needs isolation (descriptor passing, which LSP has no concept of, or a sandboxed server), which is a different design. This PR narrows the window: authorization, a re-check before every didOpen, and a root-identity check.

Tests

• src/main/ipc/lspPhysicalTarget.test.ts, real filesystem, LSP open: physical containment is checked before server startup, not when the URI is used (TOCTOU) #1268's probe:
  • authorize src/a.ts, then swap src for a symlink to an outside dir → refused;
  • a leaf that became a symlink → refused;
  • an untouched file → passes;
  • a virtual document with a symlinked virtual directory → refused;
  • a pathless document whose root was replaced by a symlink out → refused (review b; red before the fix, and the mutation that drops the root check fails);
  • a virtual leaf that is a symlink out or a directory → refused; a regular leaf passes; no leaf name → refused (review c; red before the fix, and the leaf-check mutation fails);
  • an authorized file that became a directory → refused (pins the file-type re-check).
• lspManager.test.ts:
  • the re-check runs after initialized and before didOpen;
  • a refusal returns false with no notification;
  • a passing check opens normally.
• lspDocumentOrdering.test.ts: its IPC tests now open under a real, canonical temp root. They used a made-up /repo, which the new root-identity check (realpath(root) === root) correctly refuses, so CI failed 9 of them at 8c2aaf07. Production stays strict.
• Mutations killed: no re-check in the manager; no physical validation in the assertion.

Residuals:

• The TOCTOU limit above.
• The IPC wiring of the callback has no committed test. A wiring test is feasible with the existing Electron mock (review c verified the wiring with a probe); it is left out under the freeze.

Gates: npx tsc -b + src/main/ipc + src/main/lspManager.test.ts + src/main/lspDocumentOrdering.test.ts + src/main/lspServerGeneration.test.ts + src/main/lspServerCreation.system.test.ts pass at 0659b362 (19 files, 176 tests).

🤖 Generated with Claude Code

[Juliusolsson05]

OWNER-APPROVED: (B6 proxy) narrowing accepted. #1412 narrows the LSP open window (authorization, a root-identity check, and a physical re-check before every didOpen) and is Refs #1268, not Fixes: a path-based LSP server re-resolves paths itself, so a local directory swap after the last check (or during server startup) cannot be closed from main. #1268 stays open for an isolation design.

[Juliusolsson05]

Review disposition (head 8c2aaf0). Refs #1268, OWNER-APPROVED narrowing

Reviewer	Verdict	Findings and disposition
a r1	FIX	Re-check every open (including shared joins), the virtual directory, a case-only rename: fixed (c92ebe8)
a r2	MERGE-READY	(c92ebe8)
b r1	FIX	2: a pathless document skipped the root check: fixed (1cb8b3c; assertRootUnchanged). 1 and 3: server startup in a swapped root, and a swap between the check and the server's own read: accepted as the path-based LSP limit (OWNER-APPROVED, B6 proxy) and stated in the body
c r1	FIX	The virtual LEAF was unchecked, so a pre-created leaf symlink escaped deterministically: fixed (8c2aaf0; the leaf name comes from the manager's builder and must be absent or a regular contained file). Minors: the file-type re-check is pinned; the body wording and plan are corrected
c r2	MERGE-READY	(8c2aaf0; C7/C8 probes now refused)

Accepted limit: LSP is path-based, and the server re-resolves paths itself. A local directory swap after the last check, or during server startup, cannot be closed from main. #1268 stays open for an isolation design.

Residual: the callback's IPC wiring has no committed test (feasible with the existing Electron mock; left out under the freeze).

Gate at 8c2aaf0: npx tsc -b clean; 130 tests.

[Juliusolsson05]

CI failure at 8c2aaf0, cause and fix (0659b36): 9 tests in src/main/lspDocumentOrdering.test.ts drove the real IPC handlers with a made-up workspace root /repo. The new root-identity re-check (realpath(root) === root) found that root does not exist and refused every open, correctly. So it was not a /var vs /private/var issue in production code. The fix is test-only: those tests now open under a realpath'd mkdtemp root. Production stays strict. My earlier gate missed this file; it now runs every src/main/lsp* test (19 files, 176 tests) plus tsc -b, all green at 0659b36.

🤖 Generated with Claude Code