Skip to content

Gate discovered paths in follow_links(true) directory walks against the project root #229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mikebronner merged 2 commits into from
Jun 18, 2026
Merged

Gate discovered paths in follow_links(true) directory walks against the project root #229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bf9c698
chore: start work on #228
mikebronner Jun 18, 2026
5fa3e5b
fix: 🔒️ Gate discovered paths in follow_links directory walks against…
mikebronner Jun 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 41 additions & 19 deletions laravel-lsp/src/component_completion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ use std::path::Path;

use walkdir::WalkDir;

use crate::path_containment::path_within_root_walk_entry;

/// What backs a completion candidate — drives the LSP item kind and the
/// dedup precedence when two sources produce the same tag.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
Expand Down Expand Up @@ -75,30 +77,42 @@ pub fn relative_path_to_tag_body(relative: &Path) -> Option<String> {
/// Walk `dir` for anonymous `.blade.php` components, emitting candidates named
/// `{tag_prefix}{body}` (`tag_prefix` is `""` for the root component path or
/// `"ns::"` for a namespaced one). `display_root` is the project root used to
/// shorten the detail path.
/// shorten the detail path; `root` is the project root the discovered entries
/// are containment-gated against (issue #228) — the two coincide today but stay
/// distinct so containment never rides on the display concern.
pub fn scan_anonymous_dir(
dir: &Path,
tag_prefix: &str,
display_root: &Path,
root: &Path,
) -> Vec<ComponentCandidate> {
scan_dir(dir, tag_prefix, display_root, CandidateKind::AnonymousView)
scan_dir(
dir,
tag_prefix,
display_root,
root,
CandidateKind::AnonymousView,
)
}

/// Walk `dir` for `.php` class-backed components (skipping `.blade.php` view
/// files), emitting `{tag_prefix}{body}` candidates with PascalCase filenames
/// kebab-cased.
/// kebab-cased. `display_root` shortens the detail path; `root` is the project
/// root the discovered entries are containment-gated against (issue #228).
pub fn scan_class_dir(
dir: &Path,
tag_prefix: &str,
display_root: &Path,
root: &Path,
) -> Vec<ComponentCandidate> {
scan_dir(dir, tag_prefix, display_root, CandidateKind::Class)
scan_dir(dir, tag_prefix, display_root, root, CandidateKind::Class)
}

fn scan_dir(
dir: &Path,
tag_prefix: &str,
display_root: &Path,
root: &Path,
kind: CandidateKind,
) -> Vec<ComponentCandidate> {
let mut out = Vec::new();
Expand All @@ -108,20 +122,17 @@ fn scan_dir(

let want_blade = kind == CandidateKind::AnonymousView;

// Containment scope (issue #226): the `dir` reached via the PSR-4
// namespace→directory path (`ComposerAutoload::resolve_namespace_dirs` →
// `scan_class_dir`) is now containment-gated by the fail-closed
// `path_within_root` guard at its source, so the *root* of this walk can no
// longer be an out-of-root directory. What is NOT yet gated here is each
// *discovered* path under `follow_links(true)`: a symlink encountered
// *inside* an in-root `dir` whose target escapes the project root would be
// followed and its files emitted as candidates. That symlink-escape leg is
// deferred scope — `scan_dir` is shared by callers (e.g.
// `collect_flux_components`) whose `dir`s are constructed in-root by
// different means, so gating discovered paths uniformly belongs in a
// dedicated follow-up rather than this resolver-scoped change. Low practical
// risk under the LSP threat model: the scanned tree is the developer's own
// project, and the probe is a read.
// Containment scope: the `dir` reached via the PSR-4 namespace→directory path
// (`ComposerAutoload::resolve_namespace_dirs` → `scan_class_dir`) is gated by
// the fail-closed `path_within_root` guard at its source (issue #226), so the
// *root* of this walk can no longer be an out-of-root directory. The
// *discovered* paths under `follow_links(true)` are gated below (issue #228):
// a symlink encountered *inside* an in-root `dir` whose target escapes the
// project root is still followed by `follow_links(true)`, so each emitted
// entry is checked against `root` before becoming a candidate. This closes the
// discovered-path leg #226 deferred and holds for every caller — including
// ones (e.g. `collect_flux_components`) whose `dir`s are constructed in-root
// by other means.
for entry in WalkDir::new(dir)
.follow_links(true)
.into_iter()
Expand All @@ -144,6 +155,17 @@ fn scan_dir(
continue;
}

// Containment gate (issue #228): drop any discovered entry whose real,
// symlink-resolved path escapes the project root — `follow_links(true)`
// would otherwise emit files reached through an under-root symlink whose
// target lives outside the tree. Runs after the cheap type/extension
// filters so only entries that would actually be emitted pay the
// `canonicalize` cost. Fail-closed and canonicalize-based, so an in-root
// symlink (target inside the root) still yields its candidates.
if !path_within_root_walk_entry(path, root) {
continue;
}

let Ok(relative) = path.strip_prefix(dir) else {
continue;
};
Expand Down Expand Up @@ -178,7 +200,7 @@ pub fn collect_flux_components(root: &Path) -> Vec<ComponentCandidate> {
];
let mut out = Vec::new();
for dir in &dirs {
out.extend(scan_anonymous_dir(dir, "", root));
out.extend(scan_anonymous_dir(dir, "", root, root));
}
dedup_and_sort(out)
}
Expand Down
8 changes: 4 additions & 4 deletions laravel-lsp/src/component_completion/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ fn scan_anonymous_dir_emits_namespaced_blade_components() {
]);
let scan_dir = root.join("views/test/components");

let mut got = scan_anonymous_dir(&scan_dir, "test::", &root);
let mut got = scan_anonymous_dir(&scan_dir, "test::", &root, &root);
got.sort_by(|a, b| a.name.cmp(&b.name));

assert_eq!(names(&got), vec!["test::backstage", "test::forms.input"]);
Expand All @@ -67,14 +67,14 @@ fn scan_anonymous_dir_emits_namespaced_blade_components() {
#[test]
fn scan_anonymous_dir_with_empty_prefix_for_root_components() {
let (_dir, root) = dir_with_files(&[("components/button.blade.php", "x")]);
let got = scan_anonymous_dir(&root.join("components"), "", &root);
let got = scan_anonymous_dir(&root.join("components"), "", &root, &root);
assert_eq!(names(&got), vec!["button"]);
}

#[test]
fn scan_missing_dir_is_empty() {
let (_dir, root) = dir_with_files(&[]);
assert!(scan_anonymous_dir(&root.join("nope"), "", &root).is_empty());
assert!(scan_anonymous_dir(&root.join("nope"), "", &root, &root).is_empty());
}

// ─── class scan ─────────────────────────────────────────────────────────
Expand All @@ -89,7 +89,7 @@ fn scan_class_dir_emits_kebab_class_components_and_skips_blade() {
]);
let scan_dir = root.join("app/View/Components");

let mut got = scan_class_dir(&scan_dir, "", &root);
let mut got = scan_class_dir(&scan_dir, "", &root, &root);
got.sort_by(|a, b| a.name.cmp(&b.name));

assert_eq!(names(&got), vec!["alert", "forms.input-text"]);
Expand Down
78 changes: 70 additions & 8 deletions laravel-lsp/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use laravel_lsp::middleware_parser::{middleware_base_alias, resolve_class_to_fil
use laravel_lsp::migration_index::{build_migration_index, MigrationIndex};
use laravel_lsp::path_containment::{
path_within_root, path_within_root_emit_safe, path_within_root_lexical,
path_within_root_walk_entry,
};
use laravel_lsp::query_chain::cursor::char_col_to_byte_offset;
use laravel_lsp::route_discovery::{
Expand Down Expand Up @@ -10995,6 +10996,15 @@ impl LaravelLanguageServer {
.unwrap_or(false)
})
{
// Containment gate (issue #228): the walk root is in-root, but
// `follow_links(true)` would follow a symlink *inside* it whose target
// escapes the project root and `read_to_string` its contents below —
// an out-of-root read primitive. Gate each discovered entry against the
// root before reading it; an in-root symlink (target inside the root)
// still passes.
if !path_within_root_walk_entry(entry.path(), root) {
continue;
}
if let Ok(content) = std::fs::read_to_string(entry.path()) {
if let Some(type_name) =
Self::extract_view_variable_type(&content, view_name, var_name)
Expand Down Expand Up @@ -12396,7 +12406,15 @@ impl LaravelLanguageServer {
continue;
}

// Walk the directory recursively
// Containment audit (issue #228): no walk-entry gate is needed here.
// A discovered path becomes only the `path` *display* string of a
// `ViewNameCompletion` — it is never read, opened, or resolved to an FS
// primitive at this site. Selecting a completion inserts its `name`;
// navigation later resolves that name through the independently
// containment-gated view resolver (`locate_view_file`,
// `path_within_root`). So even a `follow_links(true)` symlink escape
// could at most surface an out-of-root *display* string, never an
// out-of-root read.
for entry in walkdir::WalkDir::new(&view_path)
.follow_links(true)
.into_iter()
Expand Down Expand Up @@ -12444,6 +12462,12 @@ impl LaravelLanguageServer {
continue;
}

// Containment audit (issue #228): no walk-entry gate is needed
// here, same as the `view_path` walk above — a discovered path
// becomes only the `path` *display* string of a
// `ViewNameCompletion`, never a read or an FS primitive at this
// site; navigation resolves by `name` through the gated view
// resolver.
for entry in walkdir::WalkDir::new(package_path)
.follow_links(true)
.into_iter()
Expand Down Expand Up @@ -12544,27 +12568,42 @@ impl LaravelLanguageServer {
// 1. Root anonymous components — `resources/views/components/*.blade.php`
// (also where class-paired views live). No namespace prefix.
for (_namespace, component_path) in &component_paths {
candidates.extend(scan_anonymous_dir(component_path, "", &root));
candidates.extend(scan_anonymous_dir(component_path, "", &root, &root));
}

// 2. Package view namespaces (loadViewsFrom) — `{path}/components/`.
for (namespace, package_path) in &view_namespaces {
let dir = package_path.join("components");
candidates.extend(scan_anonymous_dir(&dir, &format!("{namespace}::"), &root));
candidates.extend(scan_anonymous_dir(
&dir,
&format!("{namespace}::"),
&root,
&root,
));
}

// 3. Anonymous component paths (Blade::anonymousComponentPath) — the
// registered directory IS the components dir.
for (namespace, dir) in &anon_paths {
candidates.extend(scan_anonymous_dir(dir, &format!("{namespace}::"), &root));
candidates.extend(scan_anonymous_dir(
dir,
&format!("{namespace}::"),
&root,
&root,
));
}

// 4. Anonymous component namespaces (Blade::anonymousComponentNamespace)
// — directory is relative to each view path.
for (namespace, rel_dir) in &anon_namespaces {
for view_path in &view_paths {
let dir = root.join(view_path).join(rel_dir);
candidates.extend(scan_anonymous_dir(&dir, &format!("{namespace}::"), &root));
candidates.extend(scan_anonymous_dir(
&dir,
&format!("{namespace}::"),
&root,
&root,
));
}
}

Expand All @@ -12574,15 +12613,25 @@ impl LaravelLanguageServer {
let autoload = laravel_lsp::composer_autoload::ComposerAutoload::for_project(&root);
for (namespace, php_namespace) in &class_namespaces {
for dir in autoload.resolve_namespace_dirs(php_namespace) {
candidates.extend(scan_class_dir(&dir, &format!("{namespace}::"), &root));
candidates.extend(scan_class_dir(
&dir,
&format!("{namespace}::"),
&root,
&root,
));
}
}
}

// 6. Non-namespaced class components — `app/View/Components/*.php`.
// These have no blade file of their own when render() is inline, so
// they're invisible to the view scans above.
candidates.extend(scan_class_dir(&root.join("app/View/Components"), "", &root));
candidates.extend(scan_class_dir(
&root.join("app/View/Components"),
"",
&root,
&root,
));

dedup_and_sort(candidates)
}
Expand Down Expand Up @@ -12623,7 +12672,13 @@ impl LaravelLanguageServer {

let mut completions = Vec::new();

// Walk the Livewire directory recursively
// Containment audit (issue #228): no walk-entry gate is needed here. A
// discovered path becomes only the `path` *display* string of a
// `LivewireComponentCompletion` — never read, opened, or resolved to an FS
// primitive at this site. Selecting a completion inserts its `name`;
// navigation resolves that name through the independently
// containment-gated Livewire resolver. So a `follow_links(true)` escape
// could at most surface an out-of-root display string, never a read.
for entry in walkdir::WalkDir::new(&livewire_path)
.follow_links(true)
.into_iter()
Expand Down Expand Up @@ -12686,6 +12741,13 @@ impl LaravelLanguageServer {

let mut completions = Vec::new();

// Containment audit (issue #228): no walk-entry gate is needed here. Each
// discovered path is reduced to a `base_dir`-*relative* string
// (`FilePathCompletion.path`) used purely as completion label/detail text
// for asset/vite/path helpers — it is never read, opened, or resolved to
// an FS primitive at this site, and the absolute discovered path is
// discarded. A `follow_links(true)` symlink escape would at most surface a
// relative path string, never an out-of-root read.
for entry in walkdir::WalkDir::new(base_dir)
.follow_links(true)
.into_iter()
Expand Down
Loading