Extend the path_within_root emit-safe guard to the translation/config (and middleware/feature/env) "Expected at:" diagnostic surfaces - #217
Merged
mikebronner merged 2 commits intoJun 18, 2026
Conversation
…afe guard Extend the path_within_root emit-safe containment guard (in_root_expected_path_hint, added in #202) to the translation, config, middleware, feature, and environment-variable "not found" diagnostic surfaces — the remaining members of the from_diagnostic -> CreateFile family that #201 deliberately left out of scope. Each surface echoed an unfiltered expected_path into its "Expected at:" message, which FileAction::from_diagnostic parses back into a CreateFile target. A user-registered out-of-root path (translation vendor_map, PSR-4 class resolution for middleware/feature) or a dangling under-root symlink could be echoed and followed out of the project tree. Routing every hint through in_root_expected_path_hint falls back to "unknown" for any candidate that is not emit-safe, while still admitting a genuinely-absent in-root create target. - translation/config: guard expected_path in create_*_diagnostic (root param) - middleware/feature: guard the inline resolve_class_to_file / root.join hints - env: guard the root.join(".env") hint (closes the dangling .env symlink) - add per-surface regression + positive-control tests mirroring #201 Fixes #214 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mikebronner
marked this pull request as ready for review
June 18, 2026 15:16
There was a problem hiding this comment.
✅ Approved
Review Summary
- Routes the five remaining
from_diagnostic → CreateFile"Expected at:" surfaces — translation, config, middleware, feature, env — through the emit-safein_root_expected_path_hintguard, completing the containment invariant #201 started for view/component. +368/−22 acrossmain.rsand a new test module. - All 8 acceptance criteria met. Translation (
create_translation_diagnostic) and config (create_config_diagnostic) gain aroot: &Pathparam and routecheck.expected_paththrough the guard; all three translation call sites + both config call sites passrootcorrectly. Middleware (both arms:mw_class_path,mw_file_path), feature (all three: PSR-4 class, string-keyapp/Features,@featuredirective), and env (env_expected_hintused in both message branches) route the correct path variable with the correct in-scoperoot. - Deliberate divergence, noted and correct: AC #3/#5 (env, config) are structurally
root.join-rooted, so the AC's "if user-controlled, route it" condition didn't strictly compel routing them — Watson routed them anyway for invariant uniformity and to close the dangling-under-root-symlink leaf. Strict improvement, nothing dropped. ✅ - Tests verified and honest. New
expected_path_diagnostic_containment.rsgives each of the 5 surfaces a regression case (all-candidates-out-of-root →"unknown") and a positive-control case (in-root-but-missing → returned as-is), plus a#[cfg(unix)]dangling-symlink case for env. Realassert_eq!on the return, soundcanonicalize().is_err()preconditions. They drive the helper directly — which I confirmed genuinely mirrors the #201view_diagnostic_containment.rsstyle (those tests import and callin_root_expected_path_hintdirectly too), so "same style" is accurate, not a shortcut. Module wired intomod.rs. The four pre-existing containment test files are untouched (AC #8);diagnostic_severity.rsis a mechanical signature fixup only. - CI green: LSP test/fmt/clippy ✅, Extension wasm/fmt/clippy ✅, CodeQL ✅.
📋 Non-blocking follow-ups
- Inertia "page not found"
Expected at:surface is the next sibling in this lineage and was not in #214's scope —main.rs:~15911emitspage_create_path(...)raw via.to_string_lossy(),from_diagnosticparses it into aCreateFiletarget, and thebuild_code_actionbackstop usespath_within_root_lexical(canonical_containment(...).unwrap_or(true),path_containment.rs:~104), which admits a dangling under-root symlink — the exact narrow casepath_within_root_emit_saferefuses. Defense-in-depth only (requires a hostile repo shipping a dangling symlink at the page path and a user clicking create;is_valid_page_namealready blocks../absolute escapes), same class and severity as the #130→#201→#214 lineage tracks as non-blocking. Outside #214's deliberately-scoped contract and named by no AC item here, so it does not block this PR — opening a tracked follow-up issue. (Adversarially verified: UPHELD.)
Ready for @mikebronner to merge.
6 tasks
mikebronner
deleted the
fix/214-extend-the-pathwithinroot-emit-safe-guard-to-the-t
branch
June 18, 2026 15:33
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements #214. Extends the
path_within_rootemit-safe containment guard (in_root_expected_path_hint, added in PR #202) to the remainingfrom_diagnostic → CreateFilediagnostic surfaces that #201 deliberately left out of scope: translation, config, middleware, feature, and environment-variable "not found" diagnostics.Each surface echoed an unfiltered
expected_pathinto itsExpected at:message, whichFileAction::from_diagnosticparses back into aCreateFiletarget a client could follow. A user-registered out-of-root path or a dangling under-root symlink could be echoed into the message and turned into a create target outside the project tree — the exact escape #201 closed for the view/component surfaces. Routing every hint throughin_root_expected_path_hintfalls back to"unknown"for any candidate that isn't emit-safe, while still admitting a genuinely-absent in-root create target.Audit findings (per surface)
expected_pathsourcevendor_map.get(namespace)root.join("config")resolve_class_to_file(PSR-4) + registry class file..-injected / out-of-root PSR-4 class nameresolve_class_to_file+root.join("app/Features")root.join(".env").envsymlink + uniform invariantThe
Copy from:(.env.example) line is left as-is — it is a copy source (read), structurally in-root, not thefrom_diagnostic → CreateFiletarget; out of this issue's scope.Changes
create_translation_diagnostic/create_config_diagnostic: take aroot: &Pathand build theExpected at:string viain_root_expected_path_hint(guards all 3 translation message sites + the config site at one point).resolve_class_to_file/root.joinhint.env_expected_hintand use it in both branches.src/tests/expected_path_diagnostic_containment.rs(+modregistration) with per-surface regression + positive-control tests mirroring the Extend the path_within_root containment guard to the remaining diagnostic surfaces (Livewire diagnostic fallback + "Expected at:" message hint) #201 component test, plus a dangling-.env-symlink case.Acceptance Criteria
expected_paththroughin_root_expected_path_hint(PSR-4-resolved class name — confirmed user-controllable).expected_paththrough the guard (PSR-4 +root.join).root.join-rooted, routed to close the dangling-.env-symlink vector and keep the invariant uniform).expected_path(vendor_map.get(namespace)) through the guard.expected_paththrough the guard with the same guarantee."unknown"regression test for each surface.view_diagnostic_containment.rs,view_navigation_containment.rs,component_navigation_containment.rs, andcode_action_create_containment.rspass without modification.Test Plan
cargo test --bin laravel-lsp— 416 tests pass (11 new), incl. the four AC-mandated suites unmodified.cargo clippy --all-targets— clean.cargo fmt --check— clean.Fixes #214