Skip to content

harden: make path_within_root fail-closed for security-guard callers (dangling-symlink leg) - #155

Merged
mikebronner merged 2 commits into
mainfrom
fix/134-make-path-within-root-fail-closed-for-security-guard
Jun 15, 2026
Merged

mikebronner merged 2 commits into
mainfrom
fix/134-make-path-within-root-fail-closed-for-security-guard

Conversation

@mikebronner

@mikebronner mikebronner commented Jun 15, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Implements #134

path_within_root is the shared root-containment guard used by every
security-sensitive disk-read path (Folio cursor route name, Folio prepare-rename
range, slot-navigation component resolution, blade-var rename view resolution).
Its fallback arm was fail-open: when canonicalize() failed it reverted to a
purely lexical path.starts_with(root) check. A dangling under-root symlink (a
link at <root>/… whose target no longer exists) fails to canonicalize, so the
lexical fallback admitted it even though its real target is unverifiable. PR
#132 closed the live-symlink escape leg; this closes the dangling leg.

Changes

  • Replace the _ => path.starts_with(root) fallback in path_within_root with _ => false — fail-closed when either side can't be canonicalized.
  • No lenient variant introduced: all four call sites use the helper as a security guard, and locate_view_file does not call it directly (the rename flow filters its result through the guard), so nothing needs graceful degradation.
  • Update the path_within_root doc comment to state the fail-closed contract.
  • Add a #[cfg(unix)] regression test (path_within_root_refuses_dangling_under_root_symlink) in folio_cursor_containment.rs.

Acceptance Criteria

  • path_within_root returns false when canonicalize fails — fallback arm replaced with _ => false
  • No lenient (fail-open) variant retained — all callers are security guards; the rename-guard call site uses only the fail-closed form
  • New #[cfg(unix)] unit test creates a dangling symlink and asserts path_within_root returns false
  • Test is discriminating: a companion assertion confirms the dangling path passes the old lexical starts_with check (would fail against the previous fallback)
  • Doc comment updated to state the fail-closed contract
  • All existing tests in folio_cursor_containment.rs and rename_integration.rs pass without regression

Test Plan

  • cargo fmt clean, cargo check clean, cargo clippy --all-targets clean
  • folio_cursor_containment module: 7/7 pass (incl. the new dangling-symlink test)
  • Full bin + lib suite green (1758 + 301 passing); rename_integration and slot_navigation_containment pass
  • The 8 integration_tests failures are pre-existing and environmental — they require an untracked laravel-lsp/test-project/ fixture (0 files tracked in git) and fail identically on the clean baseline; unrelated to this change

Fixes #134

The containment guard's fallback arm reverted to a purely lexical
`path.starts_with(root)` check whenever `canonicalize` failed, which
admitted a dangling under-root symlink — its link path is textually
inside the root — even though its real target is unverifiable. That is
a fail-open default for a guard every caller relies on for security.

Replace the fallback with `false` so an unprovable path is refused,
not admitted. No lenient variant is needed: all callers use
path_within_root as a security guard, and locate_view_file does not
call it directly (the rename flow filters its result with the guard).

Add a #[cfg(unix)] regression test that builds a dangling under-root
symlink and asserts path_within_root refuses it, with a companion
assertion proving the path passes the old lexical starts_with check —
so the test fails against the previous fallback and passes against the
fail-closed arm. Update the doc comment to state the fail-closed
contract.

Fixes: #134
@mikebronner
mikebronner marked this pull request as ready for review June 15, 2026 20:10

@mr-sherlock-holmes mr-sherlock-holmes Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approved

Review Summary

  • 🎯 Tight, surgical hardening: path_within_root's fallback arm flips from fail-open (_ => path.starts_with(root)) to fail-closed (_ => false), closing the dangling-under-root-symlink admission leg that PR #132 left open. main.rs:18861.
  • ✅ All six acceptance criteria met:
    1. Fallback is now _ => false — no lexical fallback on canonicalize failure (main.rs:18861).
    2. No lenient variant was retained; the single fail-closed function is the only form, and the rename-guard call site (main.rs:19303) uses it. AC satisfied by absence of a lenient variant.
    3. New #[cfg(unix)] test path_within_root_refuses_dangling_under_root_symlink creates a dangling symlink and asserts refusal (folio_cursor_containment.rs:258).
    4. Genuinely discriminating: the companion assert!(dangling.starts_with(root.path())) proves the old lexical fallback would have admitted the path, so the test fails against the pre-#134 code (folio_cursor_containment.rs:284).
    5. Doc comment rewritten to state the fail-closed contract and the motivating dangling-symlink case (main.rs:18846).
    6. CI green — LSP — test, fmt, clippy passed (1m40s); existing folio_cursor_containment.rs / rename_integration.rs tests unmodified and passing.
  • 🔬 Three blocker candidates raised during review were each put to an adversarial check and refuted: (a) the fail-closed change does not regress unsaved-in-buffer files (open files were loaded from disk and keep their inode, so canonicalize succeeds; the path at 13700 comes from resolve_component_path, not buffers); (b) the doc claim "every caller uses this as a security guard" holds — all four call sites are annotated containment guards; (c) the structurally-similar salsa_impl.rs:2953 lexical fallback is not exploitable (downstream file_exists_cached filters dangling symlinks; live out-of-root symlinks are caught by the primary (Ok,Ok) arm).

📋 Non-blocking follow-ups

  • View-rename arm lacks the path_within_root containment guard — main.rs:21509. The SymbolRef::View rename path guards its locate_view_file result with only is_under_vendor, whereas the binding-rename arm (main.rs:19303) filters through path_within_root. A live under-root symlink resolving outside root could let the rename operate on an out-of-tree file. Outside this PR's diff and not in #134's AC; opening a tracked issue. (Not covered by #148/#139, which address goto-definition and route-declaration flows.)
  • Stale "Mirrors path_within_root" comment — salsa_impl.rs:2947-2953. resolve_component_path's fallback is still fail-open lexical and its comment claims it mirrors path_within_root, which is now inaccurate after this PR made the helper fail-closed. Verified non-exploitable above; the underlying triplication is already tracked by #156 (consolidation), so no new issue — flagging for that work to also refresh the comment.
  • (awareness, no action) is_in_routes_dir (main.rs:18330) shares the lexical-fallback shape but is a route-file classifier, not a containment guard — a fail-open mis-classification is benign, not an out-of-root read.

Ready for @mikebronner to merge.

@mikebronner
mikebronner merged commit d7f960e into main Jun 15, 2026
5 checks passed
@mikebronner
mikebronner deleted the fix/134-make-path-within-root-fail-closed-for-security-guard branch June 15, 2026 21:28
mikebronner added a commit that referenced this pull request Jun 16, 2026
…module.

The canonical-first root-containment check lived in three copies with three
divergent fallbacks: main.rs (fail-closed, #155), slot_navigation.rs
(raw-textual), and an inline `retain` in salsa_impl.rs (lexical, for
speculative candidates). Three copies can drift, and any future hardening
had to land in all three and stay in sync.

Extract one `path_containment` module with a shared canonical-first core and
two public entry points:
- `path_within_root` — fail-closed; the security guard, used by main.rs (no
  behavior change) and slot_navigation.rs (upgrade: raw-textual → fail-closed).
- `path_within_root_lexical` — normalize_path lexical fallback that admits
  not-yet-created candidates, used by salsa_impl.rs's component-path filter
  (which must not fail-close).

All main.rs call-sites (including the one #157 added in
collect_route_declaration_targets) now route through the shared guard. Unit
tests cover in-root, sibling-root, interior-`..` escape, and the fail-closed
dangling-under-root-symlink leg.

Fixes: #156
mikebronner added a commit that referenced this pull request Jun 16, 2026
…one shared module (#161)

* chore: start work on #156

* refactor: ♻️ Consolidate triplicated path_within_root guard into one module.

The canonical-first root-containment check lived in three copies with three
divergent fallbacks: main.rs (fail-closed, #155), slot_navigation.rs
(raw-textual), and an inline `retain` in salsa_impl.rs (lexical, for
speculative candidates). Three copies can drift, and any future hardening
had to land in all three and stay in sync.

Extract one `path_containment` module with a shared canonical-first core and
two public entry points:
- `path_within_root` — fail-closed; the security guard, used by main.rs (no
  behavior change) and slot_navigation.rs (upgrade: raw-textual → fail-closed).
- `path_within_root_lexical` — normalize_path lexical fallback that admits
  not-yet-created candidates, used by salsa_impl.rs's component-path filter
  (which must not fail-close).

All main.rs call-sites (including the one #157 added in
collect_route_declaration_targets) now route through the shared guard. Unit
tests cover in-root, sibling-root, interior-`..` escape, and the fail-closed
dangling-under-root-symlink leg.

Fixes: #156

* test: ✅ Cover find_block_terminator block-comment and double-quoted heredoc legs.

Two already-working but uncovered paths in `find_block_terminator`'s @php-block
masking: a literal @endphp inside a `/* … */` block comment, and inside a
double-quoted-label heredoc (`<<<"LABEL"`). Existing tests only reach the `//`
line-comment, bare-label heredoc, and nowdoc legs.

Bundled from #93/#164 (unrelated area, folded into #156's PR by request).
mikebronner added a commit that referenced this pull request Jun 18, 2026
…gnostic hint

The "Expected at:" create-this-file hint baked into a "view not found"
diagnostic is parsed back into a CreateFile target by `from_diagnostic`, so
it is an *emitted* path. The branch sourced it via `path_within_root_lexical`,
whose docstring explicitly disclaims emitted paths — its `unwrap_or(true)`
admits a dangling under-root symlink whose target is missing, which a client
`CreateFile` could follow out of the project tree (issues #134/#155).

Add a third containment entry point, `path_within_root_emit_safe`: it shares
the lexical out-of-root gate and still admits a genuinely-absent in-root path
(the hint is for a missing view, so fail-closing would break "create view"),
but refuses a leaf dangling symlink via `symlink_metadata` — the residual the
lexical guard leaves open. Point `in_root_expected_path_hint` at it (both
"Expected at:" call sites share the helper, AC #3/#4). Extract the shared
lexical gate into `lexically_in_root` to keep the module's single-source-of-
truth invariant.

Closes Holmes's escalated AC-vs-documented-contract dispute on PR #202, per
the decision to close the residual in this PR rather than defer it.

Refs #201
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mikebronner added a commit that referenced this pull request Jun 18, 2026
…stic surfaces (Livewire diagnostic fallback + "Expected at:" message hint) (#202)

* chore: start work on #201

* fix: 🔒️ Extend path containment guard to remaining diagnostic surfaces.

Route the Livewire "component not found" diagnostic fallback through
any_in_root_candidate_exists so an out-of-root loadViewsFrom/
component_namespaces registration can no longer make diagnostics
stat-probe files outside the project tree, nor let an out-of-root view
that exists on disk silently satisfy the check.

Source the "Expected at:" message hint from the first in-root candidate
(new in_root_expected_path_hint helper, shared by the view() and
@extends/@include loops) so a maliciously-registered out-of-root
namespace path is never echoed back to the client in the message text.

Add Livewire-fallback and message-hint cases to view_diagnostic_containment.

Fixes: #201

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix: 🔒️ Refuse dangling under-root symlinks in the "Expected at:" diagnostic hint

The "Expected at:" create-this-file hint baked into a "view not found"
diagnostic is parsed back into a CreateFile target by `from_diagnostic`, so
it is an *emitted* path. The branch sourced it via `path_within_root_lexical`,
whose docstring explicitly disclaims emitted paths — its `unwrap_or(true)`
admits a dangling under-root symlink whose target is missing, which a client
`CreateFile` could follow out of the project tree (issues #134/#155).

Add a third containment entry point, `path_within_root_emit_safe`: it shares
the lexical out-of-root gate and still admits a genuinely-absent in-root path
(the hint is for a missing view, so fail-closing would break "create view"),
but refuses a leaf dangling symlink via `symlink_metadata` — the residual the
lexical guard leaves open. Point `in_root_expected_path_hint` at it (both
"Expected at:" call sites share the helper, AC #3/#4). Extract the shared
lexical gate into `lexically_in_root` to keep the module's single-source-of-
truth invariant.

Closes Holmes's escalated AC-vs-documented-contract dispute on PR #202, per
the decision to close the residual in this PR rather than defer it.

Refs #201
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix: 🔒️ Refuse non-NotFound lstat errors in emit-safe containment guard

`path_within_root_emit_safe`'s `None` arm tested `symlink_metadata().is_err()`,
which is true for EVERY lstat error — not just `NotFound`. A lexically-in-root
candidate that fails to canonicalize and whose lstat returns `EACCES` (a
no-search-permission parent hiding a dangling under-root symlink), `ENOTDIR`,
or `ELOOP` was therefore ADMITTED and echoed into the "Expected at:" diagnostic
hint, where a client `CreateFile` could follow it out of the project tree —
the exact escape this guard exists to refuse. The doc comment already specified
the discriminating condition as `NotFound`; the code failed open against it.

Discriminate on `NotFound` so the guard fails closed on anything it cannot
prove absent, admitting only a genuinely-absent speculative create target.

- `is_err()` → `is_err_and(|e| e.kind() == ErrorKind::NotFound)`
- Tighten the doc comment to state non-`NotFound` errors fail closed
- Add two `#[cfg(unix)]` tests pinning the corrected contract: an unverifiable
  candidate behind a file path component (`ENOTDIR`, deterministic) and a
  dangling under-root symlink behind a no-search-permission parent (`EACCES`,
  the case Holmes named) must both be refused

Addresses Holmes's review blocker on PR #202.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix: 🔒️ Route the "Blade component not found" diagnostic hint through the emit-safe guard.

The component-diagnostic loop (main.rs:16683) built its "Expected at:" hint
from an unfiltered `possible_paths.first()` — the fourth and last surface in
the `from_diagnostic` → `CreateFile` family, still echoing a raw candidate
after the view()/@extends/@include loops were closed. A dangling under-root
symlink (or out-of-root namespace path) planted at the conventional component
path was echoed into the message, parsed back into a create target, passed the
lexical-only #199 backstop, and a client `CreateFile` could follow it out of
the project tree. Route it through `in_root_expected_path_hint` so all four
"Expected at:" surfaces share the emit-safe guard.

Also folds in the test hardenings from the same review round:
- New `emit_safe_refuses_unverifiable_candidate_behind_a_symlink_loop` pins the
  `ELOOP` fail-closed case of the emit-safe `None` arm (deterministic on every
  unix uid), completing the EACCES/ENOTDIR/ELOOP trio named in its contract.
- `emit_safe_refuses_out_of_root_candidate` now writes the escapee to disk and
  asserts the precondition, so it unambiguously proves an *existing* out-of-root
  file is refused, not merely an absent one.
- `expected_path_hint_picks_first_in_root_candidate` gains the
  `canonicalize().is_err()` precondition matching its sibling tests.
- New `component_expected_path_hint_is_unknown_when_all_candidates_out_of_root`
  regression test mirrors AC #5 for the component surface.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
mikebronner added a commit that referenced this pull request Aug 29, 2026
Item 1's containment rework gated `contained_class_path` with
`path_within_root_lexical`, which admits every in-root path it cannot
canonicalize. A DANGLING under-root symlink therefore minted a registration
where `main` refused one — verified against both trees with the same probe:

    dangling under-root symlink   main: refused   branch: ADMITTED
    live symlink escaping root    main: refused   branch: refused

That value is not a speculative candidate. `contained_class_path`'s own doc
says it is consumed without further gating by the component-completion walk
and `try_namespaced_class`, so it becomes a read primitive; a target created
after the check could then resolve outside the module (#134/#155). The
docstring claiming "fail-closed via path_within_root_lexical" was false —
`path_containment`'s module doc states that guard is "not a security guard
for paths that will be read or emitted".

`path_containment` documented four entry points over one canonical-first
core, differing only in what they do with an uncanonicalizable path. Two
axes vary — whether an out-of-root candidate is probed on disk, and whether
an unverifiable in-root path is admitted — and "never probe, never admit"
had no member. `path_within_root_registration` fills it, composing the two
halves that already existed. No new containment logic.

Item 1's fix is untouched: gated against the owning module, a symlinked
composer path repository still passes because both sides canonicalize to the
real target, and all three of item 1's regression tests stay green.

Eight tests added. Mutation-verified: dropping the lexical pre-gate fails
only the no-probe test; flipping `unwrap_or(false)` to `true` fails only the
dangling and absent tests; reverting the call site to
`path_within_root_lexical` fails both new call-site tests.

The module's stale "**three** public entry points" count is corrected to five.

Refs: #354
mikebronner added a commit that referenced this pull request Aug 29, 2026
…#360)

* chore: start work on #354

Watson-Branch: #354

* feat(config): ✨ add owning_module, the shared module-ownership lookup

Items 1-3 of #354 are one mistake three times: a precedence or containment
rule re-implemented away from the helper that owns it. Both the Livewire
containment gate and the Salsa registration merge need the same fact —
which configured module owns a provider file, and where that module sits
in `modules.paths` order — so give them one lookup to read instead of two
path-prefix implementations that drift apart.

Rank is 1-based so 0 means "no owning module" in the tuple comparison the
merge tie-break will use. Matching is lexical on collapsed paths: a module
may be a symlinked composer path repository, and canonicalizing either side
would move a provider out from under its own module.

* fix(livewire): 🐛 gate class paths against the owning module, not the root

Item 1 of #354. `contained_class_path` canonicalized the resolved class path
and checked it against the project ROOT. A module symlinked in from a composer
path repository canonicalizes outside the root, so every Livewire registration
it made was dropped silently, with no diagnostic — the exact layout
`expand_module_dirs` documents as deliberately supported.

Gate lexically against the provider's owning module before canonicalizing,
which is what the PSR-4 provider branch (`resolve_provider_class_file`) has
always done, falling back to the root for app providers that have no owning
module. The gate gets stricter for module providers, not looser: a class path
reaching into a sibling module is in-root but outside its own module, and is
now dropped.

Three tests: the symlinked path-repo module (with an in-root control that
isolates the gate as the cause), the sibling-module reach (which a gate stubbed
to always fall back to the root would admit), and the unchanged app-provider
escape case. Mutation-verified against both the pre-#354 gate and a stubbed
ownership lookup.

* fix(salsa): 🐛 one tie-break rule for all five registration merges

Items 2, 3 and 4 of #354.

Item 2: anonymous_component_paths and anonymous_component_namespaces used a
bare `.entry().or_insert()` — priority was never read. Provider files iterate
in lexicographic path order, so a module's app/Legal/… provider was visited
before app/Providers/AppServiceProvider.php and the module won, against the
documented "an app/Providers registration always wins".

Item 3: the view- and component-namespace merges broke an equal-priority tie
by provider path sort, while the docs promise `modules.paths` glob-match
order. The actor now holds the configured module directories and reads a
provider's rank through config::owning_module — the same lookup the Livewire
containment gate uses.

Item 4: four registries ran three tie-break rules (last-wins, first-wins,
priority ignored). All five now route through one `wins()` comparison on one
`MergeRank` — priority, then modules.paths rank, then last-wins — stated once
above the loop instead of restated per map. class_component_files changes
direction from first-wins to match.

Tests: both "later module wins" tests are rewritten onto a three-module
fixture listed Alpha, Gamma, Beta, so the winner is neither the first nor the
last by name and neither the old sort nor a reversed sort can produce it. One
parametrized test constructs the same collision in all five registries and
asserts one winner across them; another pins app-over-module in all five.
Mutation-verified by dropping the rank and by restoring the or_insert.

* docs: 📝 correct the post-#336 priority tiers, and fix two follow-ups

Items 5, 6 and 7 of #354.

Item 5: #336 renumbered service-provider priority to 0=framework, 1=package,
2=module, 3=app but left the doc sites behind. Swept the crate for the old
order rather than fixing the three the issue named: main.rs's registrar
docblock and its six inline section comments, the SalsaRequest priority
comment, build_macro_registry, the salsa_impl test comment (whose fixture
registered an app provider at 2 — corrected with it), and CLAUDE.md's
convention line.

Two sites keep their own numbers: command_index and member_resolver::
impl_priority each run an independent path-derived scale that really is
0/1/2, so renumbering them would have made the prose wrong about the code.
What was stale there was the claim to be following the service-provider
convention — that is what is corrected.

Item 6: the cached middleware and binding entries labelled priority 2 "app
level" were sitting at the module tier after the renumber. Inert today (no
reader compares those priorities) and a real bug the moment one does.

Item 7: psr4_entries_escaping_the_module_resolve_nothing's traversal case
used ../../outside, which normalized to proj/app/outside — nowhere near the
decoy provider the test writes to tmp/outside. It passed with or without the
containment gate. Four levels reach the decoy; verified by deleting the gate
and watching the test go red.

* docs: 📝 close item 5 on the semantic claim, not the literal spelling

The three-literal sweep (`app=2`, `2=app`, `App=2`) could not see a tier
claim written as prose, and two sites spelled it that way.

- `salsa_impl.rs:3563` — the macro/mixin walk's coverage-boundaries doc still
  carried the pre-#336 three-tier order, four lines above a cross-reference to
  `build_macro_registry`, whose docstring was already corrected. Now names all
  four tiers, and keeps "the last two" pointing at the vendor-scanned pair.
- `tests/integration_tests.rs:1336` — `priority_merging` documented and
  encoded `app (2) > package (1) > framework (0)`: the module tier absent and
  app pinned at what is now the module value. A stale tier oracle in the test
  tree, which a `src/`-scoped sweep is structurally blind to.

The integration test is self-referential by construction, so it records the
order rather than observing it; its doc now says so and points at the tests
that enforce the order against the real merge. The new module assertion is
mutation-verified — swapping the app and module constants fails the build.

Left alone, each checked: the env scale (`0=.env.example, 1=.env.local,
2=.env`), `command_index`'s own three-tier path scale, `route_discovery`'s
route-name scale, and the `BindingRegistrationData` fixture literals, which
assert no tier semantics.

* test: ✅ Correct the stale pre-#336 app provider tier in the location oracle.

`test_service_provider_priority_by_location` still encoded the three-tier
scale item 5 exists to retire: app providers at 2 — now the module value —
and no module tier at all. It is the immediate sibling of
`test_priority_ordering_constants`, corrected one commit earlier, so
`mod priority_merging` documented two different scales six lines apart.

App is now 3, matching `register_service_provider_files_with_salsa`.

The module tier (2) is deliberately not added as a branch: no path substring
identifies a module provider — they come from the `modules.paths` globs plus
each module's composer `extra.laravel.providers`, resolved by
`config::module_provider_files`. A `contains("modules/")` branch would encode
a rule the real classifier does not implement, replacing a stale oracle with
a false one. The doc now states that limit and points at
`module_view_namespaces` and `module_livewire_namespaces`, which enforce the
module tier against the real merge.

Mutation-verified: restoring `2` in the classifier fails the assertion.

Refs: #354

* fix: 🐛 Fail closed on unverifiable Livewire class paths.

Item 1's containment rework gated `contained_class_path` with
`path_within_root_lexical`, which admits every in-root path it cannot
canonicalize. A DANGLING under-root symlink therefore minted a registration
where `main` refused one — verified against both trees with the same probe:

    dangling under-root symlink   main: refused   branch: ADMITTED
    live symlink escaping root    main: refused   branch: refused

That value is not a speculative candidate. `contained_class_path`'s own doc
says it is consumed without further gating by the component-completion walk
and `try_namespaced_class`, so it becomes a read primitive; a target created
after the check could then resolve outside the module (#134/#155). The
docstring claiming "fail-closed via path_within_root_lexical" was false —
`path_containment`'s module doc states that guard is "not a security guard
for paths that will be read or emitted".

`path_containment` documented four entry points over one canonical-first
core, differing only in what they do with an uncanonicalizable path. Two
axes vary — whether an out-of-root candidate is probed on disk, and whether
an unverifiable in-root path is admitted — and "never probe, never admit"
had no member. `path_within_root_registration` fills it, composing the two
halves that already existed. No new containment logic.

Item 1's fix is untouched: gated against the owning module, a symlinked
composer path repository still passes because both sides canonicalize to the
real target, and all three of item 1's regression tests stay green.

Eight tests added. Mutation-verified: dropping the lexical pre-gate fails
only the no-probe test; flipping `unwrap_or(false)` to `true` fails only the
dangling and absent tests; reverting the call site to
`path_within_root_lexical` fails both new call-site tests.

The module's stale "**three** public entry points" count is corrected to five.

Refs: #354

* fix: 🐛 Refuse component names that name a path instead of a component.

`dotted_to_class_path` promised a relative class path and returned whatever
it was given. It splits on `.`, kebab→Pascal's each segment and rejoins with
`/` — which destroys `..` by accident, but passes `/` and a leading `/`
through untouched. Every caller then does `base.join(converted)`, and
`Path::join` REPLACES the base when the right-hand side is absolute.

Proven against the real resolver before the fix — no race, no write access,
just a name in a file you opened:

    ui::/tmp/outside/Secret  ->  paths: ["/tmp/outside/Secret.php"]
       /tmp/outside/Secret   ->  paths: ["/tmp/outside/Secret.php"]

The name is discovered data: whatever follows `::` in a `<livewire:…>` tag
or an `@livewire('…')` literal.

Four call sites trusted the converter, not the two the probe found:

  - livewire_resolver::try_namespaced_class / try_v3_class — goto-definition
  - component_declaration_locator::conventional_class_file_path — lookup AND
    rename target
  - livewire_declaration_locator — rename target

Gating only the resolver would have left both rename paths open, so the fix
is at the converter, where the contract is. It now returns `Option<String>`
and refuses any segment that is empty or contains `/`, `\` or `:`. Rejecting
rather than sanitizing keeps it total — there is no "cleaned" name that
silently resolves somewhere the author did not write. `\` and `:` are refused
on every platform so behaviour does not diverge by host.

All four callers fail closed; the two rename paths decline to move a file
rather than compute a destination for it.

Mutation-verified. The first version of the resolver regression test passed
WITHOUT the guard: `TempDir::new()` names its directory `.tmpXXXX`, so the
dot-split mangled the probe path before it could escape. It now uses a
dot-free prefix and asserts that precondition, so it cannot go vacuous again.

Refs: #354

* fix: 🐛 Close the remaining escapes and silences the review surfaced.

Three reviewers attacked the two preceding commits; each found something,
and every finding was reproduced by probe before being fixed.

1. `dotted_to_class_path` checked the segments going IN and returned the
   join of the segments coming OUT. `kebab_to_pascal` maps an all-dashes
   segment to "", and an empty FIRST segment makes the join absolute, so
   `"-.foo"` minted `"/Foo"` — renaming a component to `-.foo` wrote its
   class file to `/Foo.php` while the blade file stayed put. The check now
   runs on the converted segments.

2. The previous commit hardened only the class branch. The V4 SFC/MFC/Volt
   branch runs FIRST and builds its search directory with `parents_to_path`,
   which uses `PathBuf::push` — an absolute segment replaces the path.
   Verified escaping the project root; goto-definition was saved by a guard
   at main.rs:16573, hover was not. `resolve_component` now refuses any raw
   segment carrying path syntax, covering every branch at once.

3. The Livewire gate re-derived a provider's owning module by PATH PREFIX.
   `modules.paths` is user-written and the settings doc offers `app/*/*`, so
   `app/*` is a shape people write — and it expands to include
   `app/Providers`. An APP provider then looked like a module provider and
   was gated against `app/Providers`, silently dropping the most ordinary
   registration there is. Provenance now comes from the discovery that found
   the provider. `owning_module` keeps its one honest consumer, the Salsa
   tie-break, where a prefix match is the only signal available.

4. The registration guard returns the canonical path it verified instead of
   a bool. The caller canonicalized a second time to obtain the value, so a
   symlink swapped between the two calls stored an unapproved path.

5. A provider changing on disk now drops the cached Livewire namespace map,
   not just the vendor translation namespaces. Registrations are gated on
   the class directory existing, so one that failed the gate at index time
   stayed failed for the session: `artisan module:make-livewire` creates the
   directory through the watcher, never the editor, leaving the component
   dead until restart.

6. A dropped registration logs a warning naming the path and the gate it
   failed, and a `modules.paths` entry resolving outside the project root
   announces itself. Both were silent, and there is no Livewire
   "component not found" diagnostic to make them visible. Neither is
   refused: outward-linking module directories are the composer
   path-repository layout this supports.

Each behavioural fix is mutation-verified and fails only its own test.
Item 1's regression tests stay green, including the symlinked path-repo
module and the sibling-module drop.

Refs: #354
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

harden: make path_within_root fail-closed for security-guard callers (dangling-symlink leg)

1 participant