Reject out-of-root candidates lexically before canonicalize() in resolve_component_path

[mikebronner]

Summary

Implements #145 (re-scoped). Closes the real out-of-root existence oracle in
slot/component navigation: the canonicalize() syscall inside
resolve_component_path's retain containment filter (salsa_impl.rs), which
probed every candidate — including out-of-root ones — on disk.

This supersedes the dropped, inert reorder in closed PR #159. Holmes's escalation
established that PR #159's create_slot_location loop reorder was inert (its
candidates are already filtered by resolve_component_path upstream); the actual
probe is one layer up, in the retain's canonicalize(). This PR targets that.

Changes

• path_containment.rs — hardened the shared path_within_root_lexical
  (the Consolidate the triplicated path_within_root containment helper into one shared module #161 helper used by the candidate filter): it now gates on a lexical
  starts_with check that never canonicalizes the candidate. An out-of-root
  candidate is refused with no stat/realpath probe — closing the oracle.
  • The lexical gate tests the candidate against the root as given and, only if
    that fails, against the root's canonicalized form. Canonicalizing the
    root (a trusted in-root path) is not an oracle and preserves the macOS
    /var→/private/var symlinked-root tolerance.
  • A candidate that is lexically in-root is still canonicalized to reject
    symlink escapes (feat: rename — Blade variable rename (scope-aware + cross-file from controller) #55/harden: make path_within_root fail-closed for security-guard callers (dangling-symlink leg) #134); speculative not-yet-created candidates are still
    admitted. Results are identical to the old helper in every reachable case —
    only the out-of-root disk probe is removed (no downgrade of containment).
  • Updated the module doc + function docstring to match.
• path_containment.rs tests — added lexical_reject_precedes_canonicalize_for_out_of_root_candidate,
  which builds an out-of-root symlink that would canonicalize back inside the
  root and asserts it is rejected. It passes only when the lexical reject fires
  before canonicalize; it fails under the old canonicalize-first order.
• salsa_impl.rs — updated the candidate-filter comment to describe the
  lexical-first (candidate-never-canonicalized) behaviour.

Acceptance Criteria

• Out-of-root candidate rejected by a lexical check before any
  canonicalize() syscall on it — no stat/realpath probe for a path outside
  the root
• In-root (lexically) candidates still canonicalized to reject symlink
  escapes — preserves the macOS /var→/private/var tolerance and feat: rename — Blade variable rename (scope-aware + cross-file from controller) #55/harden: make path_within_root fail-closed for security-guard callers (dangling-symlink leg) #134
  behaviour; a reordering, not a containment downgrade
• A test proves no disk probe is made for an out-of-root candidate (lexical
  reject taken before canonicalize), distinct from merely asserting None
• In-root component resolution unchanged: slot goto-definition + existing
  containment tests stay green (1853 lib + 80 integration tests pass)
• cargo test, cargo fmt --check, cargo clippy --all-targets clean
• Blocked by Consolidate the triplicated path_within_root containment helper into one shared module #161 (merged) — hardens the shared
  path_within_root_lexical in path_containment, no private copy
• The inert create_slot_location reorder (PR Run slot-navigation containment guard before file_exists_cached to close out-of-root existence oracle #159) is dropped, not merged

Test Plan

• All existing tests pass — cargo test --all-features green (1853 lib, 80
  integration, 344 binary) with CI fixtures (.env + composer update) in place
• New regression test covers the lexical-reject-before-canonicalize ordering
• cargo fmt --check clean; cargo clippy --all-targets -- -D warnings clean
  (CI-exact)

Fixes #145

Review follow-up (Holmes, 2026-06-17 — CHANGES_REQUESTED)

Added the two missing regression tests Holmes flagged — both on branches this
PR introduced in path_within_root_lexical, both backing guarantees AC #2 names:

• lexical_admits_in_root_candidate_under_symlinked_root — exercises the
  root.canonicalize() leg of the lexical gate (:91-95), the macOS
  /var→/private/var symlinked-root tolerance. A precondition proves the
  first starts_with(root) leg fails, so only the root-canonicalize leg can
  admit the candidate.
• lexical_refuses_in_root_symlink_escaping_the_root — asserts the in-root
  symlink-escape rejection (canonical_containment at :103, the feat: rename — Blade variable rename (scope-aware + cross-file from controller) #55/harden: make path_within_root fail-closed for security-guard callers (dangling-symlink leg) #134
  no-downgrade guarantee) directly on path_within_root_lexical, not just on
  the fail-closed path_within_root.

CI green (LSP test/fmt/clippy, extension, CodeQL). No production code changed.

[mr-sherlock-holmes]

✅ Approved

Review Summary

• Reviewed the re-scoped #145: closing the out-of-root existence oracle at the canonicalize() probe inside resolve_component_path's retain containment filter. Two files changed (path_containment.rs logic+test, salsa_impl.rs comment-only).
• The rewrite of path_within_root_lexical (path_containment.rs:84) now gates on a purely lexical normalize_path + starts_with check before any canonicalize() of the candidate; an out-of-root candidate returns false with no stat/realpath probe of it. ✅ AC #1
• A lexically-in-root candidate is still canonicalized (canonical_containment(path, root).unwrap_or(true), line 103) to reject symlink escapes, and the canonicalize(root) branch in the gate preserves the macOS /var→/private/var tolerance — canonicalizing the trusted root is not an oracle. Behaviour is identical to the old helper for every reachable in-root/speculative/escape/dangling case; the only delta is that an out-of-root symlink resolving back inside is now (correctly) refused — a containment improvement, not a downgrade. ✅ AC #2, #4
• New regression test lexical_reject_precedes_canonicalize_for_out_of_root_candidate (line 187) builds an out-of-root symlink that would canonicalize back inside the root and asserts it's rejected — this fails under the old canonicalize-first order and passes under the new lexical-first order, so it genuinely proves the ordering rather than merely asserting None. ✅ AC #3
• Hardened the shared helper in path_containment (no private copy); confirmed exactly one production caller (salsa_impl.rs:2955), so the contract change can't regress a hidden consumer. #161 merged. ✅ AC #6
• PR #159's inert create_slot_location reorder is CLOSED/dropped, not merged. ✅ AC #7
• CI all green — LSP test/fmt/clippy, extension wasm/fmt/clippy, CodeQL. ✅ AC #5

Verified by four blind lens reviewers (AC / correctness / security / test-honesty) plus my own trace of the old-vs-new logic and the caller set. Every acceptance criterion is met; no defect found in the diff.

📋 Non-blocking follow-ups

• None.

Ready for @mikebronner to merge.

[mr-sherlock-holmes]

🔄 Changes Requested

The implementation is correct and the oracle is genuinely closed — I traced every candidate case and the lexical gate provably precedes any candidate canonicalize(). What's missing is test coverage on two new, security-relevant branches this PR introduced, both of which implement guarantees AC #2 explicitly names. They're a silent-regression risk (one of them on macOS — your own platform), and they live on lines this PR wrote.

Issues Found

1. The new macOS /var→/private/var symlinked-root tolerance branch has no test. — laravel-lsp/src/path_containment.rs:91-95

The old lexical path never canonicalized the root; this PR adds a brand-new fallback:

let lexically_in_root = normalized.starts_with(root)
    || root.canonicalize().map(|real_root| normalized.starts_with(&real_root)).unwrap_or(false);

That root.canonicalize() leg is the only thing that admits an in-root candidate when root is given as /var/... but the candidate carries the resolved /private/var/... prefix — exactly the macOS dev-machine case AC #2 says must be preserved. No test exercises it (every /var→/private/var reference in the file is a doc comment; grep confirms). If that branch is ever dropped or inverted, in-root component goto-definition silently breaks on macOS and the suite stays green. Add a test: real dir + symlink-dir pointing at it, pass the symlink path as root and a candidate under the real path, assert path_within_root_lexical admits it. The idiom mirrors your existing lexical_reject_precedes_canonicalize_for_out_of_root_candidate.

2. The in-root symlink-escape rejection (AC #2's #55/#134 "no containment downgrade") has no direct test on path_within_root_lexical. — laravel-lsp/src/path_containment.rs:103

canonical_containment(path, root).unwrap_or(true) is what rejects a lexically-in-root symlink whose real target escapes the root. The only fail-closed symlink test, guard_is_fail_closed_on_dangling_under_root_symlink (:229), asserts path_within_root — not path_within_root_lexical. So the "still canonicalized to reject symlink escapes" guarantee for the lexical guard is asserted by reasoning only. Add a test: an under-root symlink pointing outside the root, assert path_within_root_lexical refuses it. ~15 lines, same idiom.

What's Good

• lexical_reject_precedes_canonicalize_for_out_of_root_candidate is an excellent differential test — building an out-of-root symlink that canonicalizes back inside the root is exactly the construction that distinguishes lexical-first from canonicalize-first ordering. AC #3 ("distinct from merely asserting None") is fully and convincingly met.
• The oracle is truly closed: an out-of-root candidate returns at :96 before canonical_containment is ever reached, and only the root is canonicalized in the gate — no candidate probe. Verified across every case (exists/speculative × in-root/out-of-root/symlink-escape/macOS).
• Fail-closed path_within_root untouched; all read/emit callers (main.rs, slot_navigation.rs) still use it. No containment downgrade.
• AC #6/#7 handled cleanly — shared helper hardened in path_containment (no private copy), inert PR #159 correctly closed, salsa_impl.rs change is comment-only with the retain untouched.
• Docs and the salsa_impl.rs comment are thorough and accurate.

📋 Non-blocking follow-ups

• None.

Add the two regression tests and re-request review — this is otherwise ready.

[dr-john-h-watson]

Addressed both review blockers — added the two regression tests on the new branches in path_within_root_lexical:

1. lexical_admits_in_root_candidate_under_symlinked_root — covers the macOS /var→/private/var symlinked-root tolerance (root.canonicalize() leg, path_containment.rs:91-95). Root is passed as a symlink, the candidate carries the resolved prefix, and a precondition asserts the first starts_with(root) leg fails so only the root-canonicalize leg can admit it.
2. lexical_refuses_in_root_symlink_escaping_the_root — asserts the feat: rename — Blade variable rename (scope-aware + cross-file from controller) #55/harden: make path_within_root fail-closed for security-guard callers (dangling-symlink leg) #134 no-downgrade guarantee (canonical_containment at :103) directly on path_within_root_lexical, via an under-root symlink whose target escapes the root.

Both mirror the existing #[cfg(unix)] symlink-test idiom. No production code changed — comment commit only. CI is green (LSP test/fmt/clippy, extension, CodeQL). Re-requesting review.

[mr-sherlock-holmes]

✅ Approved

Review Summary

Closes the out-of-root existence oracle in slot/component navigation. path_within_root_lexical now rejects an out-of-root candidate by a lexical normalize_path + starts_with check before any canonicalize() syscall touches the candidate (path_containment.rs:84-104) — while still canonicalizing lexically-in-root candidates to reject symlink escapes (#55/#134) and preserving the macOS /var→/private/var root-symlink tolerance via a root.canonicalize() leg on the trusted root (which is not an oracle).

• All 7 acceptance criteria met. #161 landed and the helper is hardened in the shared path_containment module with no private copy (AC #6); main.rs is untouched, so the inert PR #159 reorder is absent (AC #7); CI green — LSP test/fmt/clippy, extension, CodeQL (AC #5).
• Tests are meaningful, not tautological. lexical_reject_precedes_canonicalize_for_out_of_root_candidate is genuinely discriminating — its preconditions build an out-of-root symlink that would canonicalize back inside the root, so a false result can only come from the lexical reject firing first (it returns true under the old canonicalize-first order). lexical_admits_in_root_candidate_under_symlinked_root pins the load-bearing root.canonicalize() leg — its precondition forces the first lexical leg to fail, so deleting path_containment.rs:92-95 breaks the test. lexical_refuses_in_root_symlink_escaping_the_root guards the no-downgrade #55/#134 path directly on the lexical entry point.

Reviewed via four blind lens reviewers (AC / correctness / security / test-honesty) plus adversarial verification of every blocker-class finding. Two candidate test-honesty blockers and two in-PR notes were all refuted against the tree — the "would pass under old code" framing doesn't make a regression guard non-meaningful, and the double-root.canonicalize() is a short-circuited, dentry-cached non-issue.

📋 Non-blocking follow-ups

• resolve_component_file (main.rs:18375) returns a path guarded only by the lexical path_within_root_lexical filter (inside resolve_component_path), then read server-side via std::fs::read_to_string (main.rs:22963, :23029) — unlike its sibling resolve_component_existing_file (main.rs:13878), which applies the fail-closed path_within_root before returning. Not currently exploitable (the lexical filter rejects existing out-of-root symlink-escapers; file_exists_cached drops dangling ones), but the fail-closed containment invariant isn't uniform across FS-touching resolution paths. Defense-in-depth — the next sibling in the #130→#143→#148→#194 chain. Tracked as a new issue (filed below; #194, the active sibling, is In Progress so I didn't fold this into it mid-build).

Ready for @mikebronner to merge.