Add the fail-closed path_within_root guard to find_php_class_file_by_fqcn (FQCN → file resolution containment)

[mikebronner]

Summary

Implements #218 — extends the fail-closed path_within_root containment lineage (#130 → #143 → #148 → #194 → #199 → #201 → #214) to find_php_class_file_by_fqcn, the last FS-touching resolver that lacked the guard.

find_php_class_file_by_fqcn maps an FQCN to a candidate path by splitting on \ and PathBuf::join-ing each segment. join does not resolve .. on a non-absolute segment — it appends it literally — so an FQCN carrying .. segments yields a path like <root>/app/../../etc/secret.php that the subsequent path.exists()/read then stats: a read primitive that can escape the project root. The same holds for a candidate whose path crosses an under-root symlink resolving outside root.

Changes

• class_locator.rs: gate every candidate with the fail-closed path_within_root(&path, project_root) guard before the path.exists() check, in both the app (!search_vendor) and vendor (search_vendor) branches. A candidate that canonicalizes outside root — or can't be proven in-root — is skipped. Doc comment updated to record the guard.
• tests/fqcn_resolution_containment.rs (new): ..-escape negatives for both branches, an under-root-symlink escape negative (#[cfg(unix)]), and in-root positive controls. Each negative is discriminating — the escaping file is written to disk outside root, so None can only come from the guard, not absence (asserted via preconditions).
• tests/mod.rs: register the new module.

Implementation note for review

The new tests drive the public entry points (find_php_class_file → app branch; find_php_class_file_in_app_or_vendor → vendor branch) rather than calling the private find_php_class_file_by_fqcn directly. Reaching the helper from the test module (which compiles into the binary crate and imports via laravel_lsp::) would require widening it to pub across the crate boundary. Keeping it private matches class_locator's existing test style (class_locator_and_properties.rs drives the public find_php_class_file) and avoids exposing an internal heuristic fallback. The guard lives in the helper either way; the public callers exercise the exact branch each test targets. Open to making it pub and testing it directly if you'd prefer the AC-literal form.

Acceptance Criteria

• In find_php_class_file_by_fqcn, after constructing each candidate PathBuf and before path.exists(), apply the fail-closed path_within_root(&path, project_root) guard — continue when it returns false
• Guard applies in both the app (!search_vendor) and vendor (search_vendor) branches
• New *_containment.rs test file (tests/fqcn_resolution_containment.rs):
  • Negative: FQCN with .. segments resolving outside root → None (app + vendor branches)
  • Positive control: a normal in-root FQCN with the file present → Some(path) (app + vendor)
  • #[cfg(unix)] negative: an under-root symlink whose target resolves outside root → None
• New tests registered in tests/mod.rs
• No regression in existing class_locator unit tests
• The Composer-autoload branch (ComposerAutoload::resolve) is unchanged — out of scope for this guard

Test Plan

• cargo test --lib — 1894 passed (includes existing class_locator unit tests)
• cargo test --bin laravel-lsp — 421 passed (includes the 5 new containment tests)
• cargo clippy --all-targets — clean
• cargo fmt — clean
• Note: 8 pre-existing failures in tests/integration_tests.rs are unrelated — they depend on a gitignored, untracked test-project/.env fixture absent from any fresh clone, and reference class_locator zero times. They fail on a clean main checkout regardless of this change.

Fixes #218

[mr-sherlock-holmes]

✅ Approved

Review Summary

• Reviewed PR #221 (+233/−1, 3 files) adding the fail-closed path_within_root guard to find_php_class_file_by_fqcn — the last FS-touching resolver in the containment lineage (#130 → #143 → #148 → #194 → #199 → #201 → #214) that lacked it. Fanned out four blind lens reviewers (AC / correctness / security / test-honesty) over the checkout.
• CI green — LSP test/fmt/clippy, wasm check, and CodeQL (js-ts + python) all pass. This is the first review round; no prior change-requests.

Acceptance criteria — all met

1. ✅ Guard placed correctly — if !path_within_root(&path, project_root) { continue; } sits after each candidate is constructed and immediately before path.exists() (class_locator.rs:148, :181).
2. ✅ Both branches — app (!search_vendor, :148) and vendor (search_vendor, :181). Each continue is correctly scoped inside its own for loop (["app","src"] / ["src",""]), so refusing one candidate still tries the sibling rather than aborting.
3. ✅ Containment test file — tests/fqcn_resolution_containment.rs covers, for both public entry points: negative ..-escape (App\..\..\secret, Vendor\Pkg\..\..\..\..\secret), positive control (App\Models\User, Laravel\Passport\Token), and a #[cfg(unix)] under-root-symlink escape — a superset of the three cases the AC asked for.
4. ✅ Registered — mod fqcn_resolution_containment; in tests/mod.rs:23.
5. ✅ No regression — existing class_locator.rs unit tests untouched; CI's LSP test job is green.
6. ✅ Composer-autoload branch unchanged — the guard lives only inside find_php_class_file_by_fqcn; the ComposerAutoload::resolve call sites are untouched (see follow-up below).

Why the guard-before-exists() ordering is sound

path_within_root → canonical_containment canonicalizes both path and root and is fail-closed (unwrap_or(false)). Because canonicalize() requires the target to exist, placing the guard before path.exists() drops no legitimate resolution: an in-root candidate that doesn't exist would also fail exists(), and the only behaviour that changes is an existing out-of-root candidate — now refused instead of read. Exactly the intended fix, nothing lost.

Tests are discriminating (not false-passing)

Each negative test writes the escaping file to disk outside the root and asserts a precondition (via canonicalize equality) that the constructed candidate resolves out-of-root and exists — so a None result can only come from the guard firing, never from mere absence. The basename-walk fallback can't rescue the out-of-root file either (WalkDir defaults to follow_links=false). Positive controls guard against over-fitting.

📋 Non-blocking follow-ups

• ComposerAutoload::resolve is the one FS-touching resolver in the lineage with no containment guard — composer_autoload.rs:64-74. It pushes each \-split FQCN segment literally (so .. is appended verbatim) and returns candidate on a bare candidate.exists(), with no path_within_root gate. This is the higher-priority branch that runs before the heuristic this PR just guarded, so #218's premise that it "returns a verified path" is inaccurate — it verifies existence, not containment. Low practical risk under the LSP threat model (composer.json/installed.json is developer-controlled, not an untrusted boundary), so non-blocking and out of #218's scope — but it's the natural next anchor for the lineage's stated goal that the invariant hold uniformly. Filed as a new issue (linked below).

Ready for @mikebronner to merge.