feat(vscode): add filesystem validation protocol for file links - #11218
Conversation
Add a validateFiles request/response round-trip between the webview and the extension so the webview can confirm which inline code-span candidates are real files before promoting them to clickable links. The extension stat-checks candidate paths (new file-links.ts) and replies with the subset that exist. Routing lives in editor-actions alongside the other editor open actions, and openFile now falls back to a workspace filename search (single match opens, multiple prompts) with a "File not found" warning when a clicked path cannot be resolved.
dc5ccb4 to
ed5fc5a
Compare
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| */ | ||
| function findFallback(filePath: string, line?: number, column?: number): void { | ||
| const name = filePath.split(/[\\/]/).pop() || filePath | ||
| Promise.resolve(vscode.workspace.findFiles(`**/${name}`, "**/node_modules/**", 5)).then( |
There was a problem hiding this comment.
WARNING: Glob-special characters in filenames are not escaped before building the pattern.
Filenames common in Next.js / Remix projects — [id].tsx, [...slug].tsx — contain [, ] which are glob metacharacters. Passing them unescaped in **/${name} will produce an invalid glob and findFiles will return zero matches, falling through to the "File not found" warning even when the file exists.
Consider escaping metacharacters before constructing the pattern:
const escaped = name.replace(/[\[\]{}?*!()]/g, "\\$&")
vscode.workspace.findFiles(`**/${escaped}`, "**/node_modules/**", 5)Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Previous Review Summaries (2 snapshots, latest commit d497be6)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit d497be6)Status: 1 Issue Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Resolved in
Issue Details (click to expand)WARNING
Other Observations (not in diff)None. Files Reviewed (4 files)
Previous review (commit 81aad79)Status: No Issues Found | Recommendation: Merge All four issues from the previous review have been resolved in commit
Files Reviewed (1 file)
Reviewed by gpt-5.4-2026-03-05 · 170,066 tokens Review guidance: REVIEW.md from base branch |
Address kilo-code-bot review on Kilo-Org#11218: - escape glob metacharacters in the workspace filename search so names like [id].tsx / [...slug].tsx resolve instead of falling through to the "File not found" warning - add rejection handlers to showTextDocument (in show()) and showQuickPick so VS Code API errors surface instead of being silently swallowed - type the validateFiles message fields on the handleEditorAction parameter instead of inline casts
Address kilo-code-bot review on Kilo-Org#11218: - escape glob metacharacters in the workspace filename search so names like [id].tsx / [...slug].tsx resolve instead of falling through to the "File not found" warning - add rejection handlers to showTextDocument (in show()) and showQuickPick so VS Code API errors surface instead of being silently swallowed - type the validateFiles message fields on the handleEditorAction parameter instead of inline casts
|
Thanks for the review — addressed all four in 81aad79:
The same fix is applied to the stacked #11219, which carries its own copy of |
markijbema
left a comment
markijbema
left a comment
There was a problem hiding this comment.
Thanks for splitting it up, i find a few issues still, could you look into that?
Hi @markijbema ! Thanks for the feedback, I'll look into it later this day :) |
Address markijbema review on Kilo-Org#11218: - validateFiles now rejects candidates that resolve outside the session root (absolute paths elsewhere, UNC paths, ../ traversal) before any fs.stat, so auto-validated model output can't probe arbitrary host paths - the openFile dead-link fallback searches the session dir via a RelativePattern instead of the whole opened workspace, so it can't cross into another worktree/branch - use VS Code-compatible bracket glob escaping (`[id].tsx` -> `[[]id[]].tsx`) so dynamic-route filenames resolve instead of falling through Adds focused unit tests for the new vscode-free `contains` and `escapeGlob` helpers in path-utils.
Address markijbema review on Kilo-Org#11218: - validateFiles now rejects candidates that resolve outside the session root (absolute paths elsewhere, UNC paths, ../ traversal) before any fs.stat, so auto-validated model output can't probe arbitrary host paths - the openFile dead-link fallback searches the session dir via a RelativePattern instead of the whole opened workspace, so it can't cross into another worktree/branch - use VS Code-compatible bracket glob escaping (`[id].tsx` -> `[[]id[]].tsx`) so dynamic-route filenames resolve instead of falling through Adds focused unit tests for the new vscode-free `contains` and `escapeGlob` helpers in path-utils.
|
Thanks for the thorough review — addressed all three in d497be6: F1 (path containment): F2 (worktree scoping): the F3 (glob escaping + test): replaced the backslash escaping with VS Code-compatible bracket escaping ( The same fix is applied to the stacked #11219. |
| // UNC candidates can trigger outbound filesystem requests on Windows — never allow them. | ||
| if (candidate.startsWith("\\\\") || candidate.startsWith("//")) return false | ||
| const base = path.resolve(root) | ||
| const rel = path.relative(base, path.resolve(base, candidate)) |
There was a problem hiding this comment.
WARNING: contains() only enforces lexical containment, so symlink escapes can still probe files outside the session root.
path.resolve() / path.relative() treat root/link/passwd as in-tree even when link is a symlink to /etc, so validateFiles() will still call workspace.fs.stat() on an external target through any checked-in symlink. Once the stacked UI auto-validates code spans, model output can still confirm arbitrary host files this way. Consider resolving the real path before the containment check, or rejecting symlinked path segments entirely.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Address kilo-code-bot review on Kilo-Org#11218: contains() is purely lexical, so a checked-in symlink (e.g. root/link -> /etc) would pass the check and let validateFiles stat an external target. validateFiles now resolves each candidate's real path (symlinks included) and requires it to stay inside the real session root before any stat, on top of the existing lexical pre-check that still rejects UNC / absolute-outside / ../ without touching the disk.
Address kilo-code-bot review on Kilo-Org#11218: contains() is purely lexical, so a checked-in symlink (e.g. root/link -> /etc) would pass the check and let validateFiles stat an external target. validateFiles now resolves each candidate's real path (symlinks included) and requires it to stay inside the real session root before any stat, on top of the existing lexical pre-check that still rejects UNC / absolute-outside / ../ without touching the disk.
|
Good catch — addressed in c2076c8.
So a checked-in symlink like |
|
Thanks for your approval, @markijbema ! I'll update the latter PR with images later today so that you may start reviewing it. Btw, should I do something about the failing check "Check forbidden strings / Check forbidden strings (pull_request)"? |
|
Nopes, that check was already fixed on master, and i can override it. Thanks! |
Address kilo-code-bot review on Kilo-Org#11218: - escape glob metacharacters in the workspace filename search so names like [id].tsx / [...slug].tsx resolve instead of falling through to the "File not found" warning - add rejection handlers to showTextDocument (in show()) and showQuickPick so VS Code API errors surface instead of being silently swallowed - type the validateFiles message fields on the handleEditorAction parameter instead of inline casts
Address markijbema review on Kilo-Org#11218: - validateFiles now rejects candidates that resolve outside the session root (absolute paths elsewhere, UNC paths, ../ traversal) before any fs.stat, so auto-validated model output can't probe arbitrary host paths - the openFile dead-link fallback searches the session dir via a RelativePattern instead of the whole opened workspace, so it can't cross into another worktree/branch - use VS Code-compatible bracket glob escaping (`[id].tsx` -> `[[]id[]].tsx`) so dynamic-route filenames resolve instead of falling through Adds focused unit tests for the new vscode-free `contains` and `escapeGlob` helpers in path-utils.
Address kilo-code-bot review on Kilo-Org#11218: contains() is purely lexical, so a checked-in symlink (e.g. root/link -> /etc) would pass the check and let validateFiles stat an external target. validateFiles now resolves each candidate's real path (symlinks included) and requires it to stay inside the real session root before any stat, on top of the existing lexical pre-check that still rejects UNC / absolute-outside / ../ without touching the disk.
Address kilo-code-bot review on Kilo-Org#11218: - escape glob metacharacters in the workspace filename search so names like [id].tsx / [...slug].tsx resolve instead of falling through to the "File not found" warning - add rejection handlers to showTextDocument (in show()) and showQuickPick so VS Code API errors surface instead of being silently swallowed - type the validateFiles message fields on the handleEditorAction parameter instead of inline casts
Address markijbema review on Kilo-Org#11218: - validateFiles now rejects candidates that resolve outside the session root (absolute paths elsewhere, UNC paths, ../ traversal) before any fs.stat, so auto-validated model output can't probe arbitrary host paths - the openFile dead-link fallback searches the session dir via a RelativePattern instead of the whole opened workspace, so it can't cross into another worktree/branch - use VS Code-compatible bracket glob escaping (`[id].tsx` -> `[[]id[]].tsx`) so dynamic-route filenames resolve instead of falling through Adds focused unit tests for the new vscode-free `contains` and `escapeGlob` helpers in path-utils.
Address kilo-code-bot review on Kilo-Org#11218: contains() is purely lexical, so a checked-in symlink (e.g. root/link -> /etc) would pass the check and let validateFiles stat an external target. validateFiles now resolves each candidate's real path (symlinks included) and requires it to stay inside the real session root before any stat, on top of the existing lexical pre-check that still rejects UNC / absolute-outside / ../ without touching the disk.
…links-2-fs-validation feat(vscode): add filesystem validation protocol for file links
2 participants
Issue
Re-split of #10340 per maintainer feedback (no separate tracking issue exists). PR 1 of 2.
Stack (review/merge in order): #11218 (this PR) → #11219
The maintainer suggested three pieces (path parsing, filesystem validation, UI rendering). Filesystem validation is genuinely self-contained and is this PR. Path-parsing and UI rendering had to be combined in #11219: the path-parsing API change and its only consumers (
marked.tsx,message-part.tsx) can't compile apart, and the UI also depends on the data-context hook added here — so a fully separate path-parsing PR can't pass CI. This 2-PR stack keeps both PRs green while still isolating the validation layer for separate review.Context
The extension-side plumbing that confirms which inline-code candidates are real files, plus a fallback for opening links whose exact path is missing. No parsing or rendering changes here — this layer is independent and compiles on its own.
Implementation
validateFilesrequest/response round-trip (webview ↔ extension) so the webview can ask "which of these candidate paths exist?" and only promote real ones.kilo-provider/file-links.tsstat-checks candidate paths (files only, not directories) relative to the session/workspace root.kilo-provider/editor-actions.ts(wheremainnow keeps the editor open/openFile logic after its refactor), soKiloProvideronly needs a one-linepostcallback rather than the larger change from the original PR.openFilegains a dead-link fallback: workspace filename search (opens on a single match, quick-pick on multiple, "File not found" warning on none).packages/ui/src/context/data.tsxexposes thevalidateFilesfunction on the data context (andApp.tsxwires it to the extension).Screenshots / Video
N/A for the core plumbing. The visible surfaces (the "File not found" warning / multi-match quick-pick) are exercised end-to-end in #11219.
How to Test
Manual/local verification
bun run typecheckin bothpackages/ui/andpackages/kilo-vscode/(extension + webview) — green. This branch compiles standalone (contrast with the original 3-way split, where the intermediate PRs could not).bun test tests/unit/kilo-ui-contract.test.tsinpackages/kilo-vscode/— 28 pass.Reviewer test steps
The behavior you can exercise directly here is the
openFiledead-link fallback. File references in chat are already clickable onmain(the existing renderer), and every click routes through theopenFilethis PR enhances. Launch the dev extension from this branch (bun run extensionfrom the repo root), open a real project, and in a chat:`package.json`and click it → the file opens.`src/totally-missing.ts`and click it → a "File not found: src/totally-missing.ts" warning. (Onmainthis silently did nothing.)`index.ts`and click it → a quick-pick of matching files; choosing one opens it.The same fallback also covers file mentions and tool-output paths (same
openFileentry point) — e.g. a tool-output path that has since been moved/deleted.Blocked checks and substitute verification
@kilocode/kilo-jetbrains(needs JDK 21, not installed on this machine) — unrelated to this change. Substitute: ran theui+kilo-vscode(extension + webview) typechecks directly (agent-executed, green); CI runs the full typecheck.Checklist