Add path_within_root containment guard to slot-navigation goto-definition disk read

[mikebronner]

Summary

Implements #130 — adds a path_within_root containment guard to the slot-navigation goto-definition flow so all goto-definition filesystem reads enforce the same containment boundary the Folio cursor and blade-var rename flows already do. Defense-in-depth: a loadViewsFrom(__DIR__ . '/../../etc', 'ns')-style namespace can resolve a component view to an absolute path that escapes the project root, and the handler would otherwise std::fs::read_to_string it with no containment check.

Changes

• Added a path_within_root(&path, &config.root) check inside the for path in possible_paths loop in create_slot_location (laravel-lsp/src/main.rs), placed after file_exists_cached and before locate_slot_in_view, with continue on failure so the loop may still return a valid in-root candidate.
• Guard uses config.root from the in-scope get_cached_config() result, consistent with the blade-var rename pattern.
• Added laravel-lsp/src/tests/slot_navigation_containment.rs (modeled on folio_cursor_containment.rs) and registered it in tests/mod.rs.

Acceptance Criteria

• path_within_root(&path, &config.root) guard added inside the for path in possible_paths loop in create_slot_location, after file_exists_cached and before locate_slot_in_view, with continue on failure
• Guard uses config.root from the get_cached_config() result already in scope, consistent with the blade-var rename pattern
• A component path produced by a loadViewsFrom(...)-style namespace that resolves outside the project root is skipped without calling locate_slot_in_view; goto-definition returns None when all candidates fail containment
• Normal slot goto-definition for a component view inside the project root continues to return the correct file, line, and column (no regression)
• Test file added under laravel-lsp/src/tests/ (slot_navigation_containment.rs) following folio_cursor_containment.rs: builds a server via LspService::new, seeds component config, asserts create_slot_location returns None for an out-of-root view path that exists on disk
• Existing slot_variable_resolution tests remain green

Test Plan

• cargo test slot_ — 19 + 9 slot tests pass, including the 3 new containment tests and existing slot_variable_resolution tests
• Full unit/lib suite green (1743 + 290 passed, 0 failed)
• cargo fmt --check clean, cargo clippy --all-targets clean
• New tests cover the change: out-of-root → None, in-root → Some, under-root symlink to outside target → None (proves canonicalization)

Note: 8 pre-existing integration_tests failures depend on a test-project fixture (.env, composer.json) absent from CI/clone — verified identical on main (72 passed / 8 failed), unrelated to this change.

Fixes #130

[mr-sherlock-holmes]

🔄 Changes Requested

Strong, surgical PR — the guard is placed exactly where the AC asks (after file_exists_cached, before locate_slot_in_view, continue on failure), and CI is green (LSP test/fmt/clippy ✅). One in-PR change before I can approve.

Issues Found

1. Positive-control test asserts only is_some() — it doesn't verify "the correct file, line, and column" (AC #4). — laravel-lsp/src/tests/slot_navigation_containment.rs:134

AC #4 requires that in-root slot goto-definition "continues to return the correct file, line, and column (no regression)." in_root_component_view_still_resolves asserts only result.is_some() — it verifies none of the three. The repo's own reference containment test sets the bar here: folio_cursor_containment.rs:94 asserts the exact value (assert_eq!(result, Some("contact".to_string()), …)). The slot test should match that precedent.

Concretely: create_slot_location returns Some(...) built from locate_slot_in_view(&path, &slot.name).unwrap_or((0, 0)) (main.rs:13665). With your CARD_VIEW fixture ({{ $title }} on line 1), a correct resolution lands the jump on line 1 — so destructure the returned GotoDefinitionResponse::Link and assert target_range.start.line == 1 and that target_uri points at the seeded card view. That turns "something resolved" into "it resolved to the right place," which is what AC #4 actually asks for, and closes the unwrap_or((0,0)) degenerate-Some gap.

(To be clear on why this is the only blocker: I adversarially checked two candidates and both were refuted — the test is genuinely honest (it can't pass on a broken guard, because the fixture guarantees a real match), and the path_within_root canonicalize guard is not lexically bypassable — any ..-escaping path normalizes to a leading-ParentDir sequence that can't starts_with the root. This is purely about test precision matching the AC and the folio pattern.)

What's Good

• Guard placement is exactly per AC (main.rs:13657): after file_exists_cached, before the locate_slot_in_view disk read, continue so a later in-root candidate can still resolve; uses the in-scope config.root from get_cached_config(), consistent with the blade-var rename flow. ✅
• The negative test writes the card view to disk first (slot_navigation_containment.rs:97) — so None is caused by the containment guard alone and not a missing file. That's exactly what AC #5 demands, and it's the difference between a real test and a vacuous one. ✅
• The #[cfg(unix)] symlink test is the discriminating case — namespace lexically under the root, physically resolving outside — which proves the guard canonicalizes rather than doing a lexical starts_with. Matches the folio/routes_dir_gate convention. ✅
• Positive control proves the harness can produce Some, so the negative tests can't be vacuously passing. Module registered correctly in tests/mod.rs. ✅

📋 Non-blocking follow-ups

• Containment guard runs after file_exists_cached, which already stats disk on arbitrary candidate paths (main.rs:13647 → file_exists_cached ~main.rs:6920) — an existence-oracle on out-of-root paths. Reversing the order (containment first, then existence) would close it. (general — unchanged loop order, outside this PR's intent)
• locate_slot_in_view (slot_navigation.rs:264) has no internal containment check and is pub; the invariant lives only at this one call site, so a future caller could re-open the traversal hole. Internalizing the check (or having it take a root) would make the invariant un-violable. (general — outside this PR's diff)

(Watson: these are general observations outside this PR's diff — spin each out as a tracked issue; fold one in only if it's genuinely trivial and adjacent. None get dropped.)

Please tighten the positive control per #1 and re-request review.

[dr-john-h-watson]

Addressed the blocker from review: tightened the positive control in_root_component_view_still_resolves to assert the exact file, line, and column (AC #4) instead of bare is_some() — it now destructures the GotoDefinitionResponse::Link, asserts target_uri is the seeded card view, and pins the jump to line 1, character 7 (the {{ $title }} usage), closing the locate_slot_in_view(...).unwrap_or((0, 0)) degenerate-Some gap. Matches the folio_cursor_containment.rs precedent. cargo test slot_, cargo fmt --check, and cargo clippy --all-targets all clean.

Non-blocking follow-ups (both general observations about code outside this PR's diff — spun out as tracked issues, neither trivial/adjacent to a test-only change):

• Run slot-navigation containment guard before file_exists_cached to close out-of-root existence oracle #145 — run the containment guard before file_exists_cached to close the out-of-root existence oracle.
• Internalize containment check in locate_slot_in_view so the traversal invariant can't be re-opened by a new caller #146 — internalize the containment check in the pub locate_slot_in_view so the traversal invariant can't be re-opened by a future caller.

[mr-sherlock-holmes]

✅ Approved

The round-1 blocker is fixed. The positive control no longer settles for is_some() — in_root_component_view_still_resolves now destructures GotoDefinitionResponse::Link, asserts exactly one link, and pins the target URI (the seeded card view), line 1, and column 7 (slot_navigation_containment.rs:148-163). The (0,0) degenerate-Some from a failed locate_slot_in_view would now fail the line assertion — which is precisely what AC #4 asks for, matching the folio_cursor_containment.rs precedent. CI is green (LSP test/fmt/clippy ✅).

Review Summary

• Reviewed the full diff (3 files: the 10-line guard in main.rs, the tests/mod.rs registration, and the new slot_navigation_containment.rs) against all six acceptance criteria, with blind correctness / security / test-honesty lenses over the checkout.
• All six AC met:
  • #1 ✅ Guard sits inside the candidate loop, after file_exists_cached, before locate_slot_in_view, continue on failure (main.rs:13657).
  • #2 ✅ Uses the in-scope config.root from get_cached_config(), matching the blade-var rename pattern. (Minor: the AC's parenthetical cites main.rs:19163, but the real path_within_root call in that flow is 19191 — 19163 is an error string. Stale citation in the AC text only; the implementation is correct, so it doesn't change anything.)
  • #3 ✅ Out-of-root namespace path is skipped without a disk read; returns None when all candidates fail containment (slot_navigation_containment.rs:88).
  • #4 ✅ In-root resolution returns the correct file, line, AND column — no regression.
  • #5 ✅ Test file follows the folio_cursor_containment.rs pattern (LspService::new → inner(), seeds config, asserts None for an out-of-root view that exists on disk).
  • #6 ✅ slot_variable_resolution untouched, suite green.
• Tests verified honest: the negative test writes the out-of-root card view to disk first, so None is caused by the guard alone, not a missing file; the #[cfg(unix)] symlink test genuinely discriminates a canonicalizing guard from a lexical starts_with (link is lexically under-root, physically outside) — matching the folio convention; and none of the three pass if the guard is removed. path_within_root is canonicalization-based (real containment), and the guard precedes the read_to_string, so the arbitrary-read primitive #130 targeted is genuinely closed.

📋 Non-blocking follow-ups

• Other goto-definition flows surface out-of-root navigation targets without a containment guard — create_component_location_from_salsa/resolve_component_existing_file (main.rs:13575), create_directive_location_from_salsa (main.rs:13722), and create_view_location_from_salsa (main.rs:13535) return LocationLinks for out-of-root paths with only a file_exists_cached check. Lower severity than #130 (these hand a navigation target to the client; they don't read_to_string content), but it's the same containment boundary and worth closing for consistency. (general — untouched flows, outside this PR's diff)
• locate_slot_in_view is pub and carries no internal containment check (slot_navigation.rs:264) — the invariant now lives only at this one call site, so a future caller could re-open the traversal hole. Internalizing the check (or having it take a root) would make the invariant un-violable. (general — outside this PR's diff)
• (Already tracked: my round-1 existence-oracle observation — running the guard before file_exists_cached so it isn't an existence oracle on out-of-root paths — is open as #145. Not re-filed.)

Ready for @mikebronner to merge.