From 211c392ec1ce923169bda8917c8c99070a291802 Mon Sep 17 00:00:00 2001 From: Mike Bronner Date: Mon, 15 Jun 2026 13:02:58 -0700 Subject: [PATCH 1/2] chore: start work on #134 From 64199b3d524510d02bd736a3b3bc3837a7d70825 Mon Sep 17 00:00:00 2001 From: Mike Bronner Date: Mon, 15 Jun 2026 13:10:29 -0700 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20=F0=9F=94=92=EF=B8=8F=20Make=20path?= =?UTF-8?q?=5Fwithin=5Froot=20fail-closed=20on=20canonicalize=20failure.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- laravel-lsp/src/main.rs | 19 +++++-- .../src/tests/folio_cursor_containment.rs | 51 ++++++++++++++++++- 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/laravel-lsp/src/main.rs b/laravel-lsp/src/main.rs index 649ce067..90bd9f76 100644 --- a/laravel-lsp/src/main.rs +++ b/laravel-lsp/src/main.rs @@ -18841,13 +18841,24 @@ fn zero_anchor() -> Range { /// canonicalized first — `locate_view_file` builds the path by joining and /// `Path::starts_with` is purely textual, so a symlink under the project could /// otherwise resolve outside the root and leak an absolute, out-of-project path -/// into a `WorkspaceEdit` (issue #55 cross-file binding rename). When either -/// side can't be canonicalized (e.g. the file vanished mid-edit) we fall back -/// to the textual prefix check rather than admitting an unverified path. +/// into a `WorkspaceEdit` (issue #55 cross-file binding rename). +/// +/// **Fail-closed.** When either side can't be canonicalized this returns +/// `false` rather than falling back to a textual prefix check (issue #134). +/// The motivating case is a *dangling* under-root symlink — a symlink at +/// `/…` whose target no longer exists, so `canonicalize` returns +/// `Err(ENOENT)`. A lexical `path.starts_with(root)` fallback would *admit* +/// it (its link path is textually inside the root) even though its real +/// target is unverifiable, a surprising fail-open default for a containment +/// guard. Every caller uses this as a security guard, so an unprovable path is +/// refused, not admitted. fn path_within_root(path: &std::path::Path, root: &std::path::Path) -> bool { match (path.canonicalize(), root.canonicalize()) { (Ok(real_path), Ok(real_root)) => real_path.starts_with(&real_root), - _ => path.starts_with(root), + // Fail-closed: a path we can't canonicalize (missing, or a dangling + // symlink) is unverifiable, so refuse it rather than admit it via a + // textual prefix check that a dangling under-root symlink would pass. + _ => false, } } diff --git a/laravel-lsp/src/tests/folio_cursor_containment.rs b/laravel-lsp/src/tests/folio_cursor_containment.rs index 1ac3aefb..92ab78c4 100644 --- a/laravel-lsp/src/tests/folio_cursor_containment.rs +++ b/laravel-lsp/src/tests/folio_cursor_containment.rs @@ -8,7 +8,7 @@ //! tests drive the private methods directly by building the server through //! `tower_lsp::LspService` and reaching its inner value with `inner()`. -use crate::LaravelLanguageServer; +use crate::{path_within_root, LaravelLanguageServer}; use laravel_lsp::route_discovery::{RouteDefinition, RouteIndex, PRIORITY_APP}; use std::fs; use std::path::Path; @@ -244,3 +244,52 @@ async fn decl_range_under_root_symlink_to_outside_target_returns_none() { inside the root" ); } + +// --- path_within_root: fail-closed on canonicalize failure (issue #134) --- +// +// The guard above proved the *live*-symlink escape leg (a link whose target +// resolves outside the root). This exercises the helper directly on the +// *dangling*-symlink leg: a link under the root whose target does not exist, so +// `canonicalize` returns `Err(ENOENT)`. The fail-closed contract must refuse it +// rather than fall back to a lexical prefix check that would admit it. + +#[cfg(unix)] +#[test] +fn path_within_root_refuses_dangling_under_root_symlink() { + // A symlink at `/dangling` pointing at a target that was never + // created. `dangling.canonicalize()` fails (the target is missing), so the + // (Err, _) arm of `path_within_root` decides the outcome. + let root = TempDir::new().unwrap(); + let missing_target = root.path().join("never-created.blade.php"); + let dangling = root.path().join("dangling"); + std::os::unix::fs::symlink(&missing_target, &dangling).unwrap(); + + // Sanity: the link itself exists on disk, but it cannot be canonicalized + // because its target is missing — this is exactly the case the old + // `_ => path.starts_with(root)` fallback admitted. + assert!( + std::fs::symlink_metadata(&dangling).is_ok(), + "the dangling symlink must exist on disk for this to be a real test" + ); + assert!( + dangling.canonicalize().is_err(), + "a dangling symlink must fail to canonicalize" + ); + + // Discriminating companion assertion: the dangling path PASSES the old + // lexical `path.starts_with(root)` check (its link path is textually inside + // the root). So this test would FAIL against the previous + // `_ => path.starts_with(root)` fallback, which admitted the path, and only + // passes against the fail-closed `_ => false`. + assert!( + dangling.starts_with(root.path()), + "precondition: the dangling link path is lexically inside the root, so \ + the old lexical fallback would have admitted it" + ); + + assert!( + !path_within_root(&dangling, root.path()), + "a dangling under-root symlink (canonicalize fails) must be refused — \ + path_within_root is fail-closed, not fail-open via a lexical prefix check" + ); +}