mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-08-28 19:11:03 +08:00
fix(vscode): scope file-link validation and fallback to the session root
Address markijbema review on #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.
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
import * as vscode from "vscode"
|
||||
import { buildPreviewPath, getPreviewCommand, getPreviewDir, parseImage, trimEntries } from "../image-preview"
|
||||
import { isAbsolutePath } from "../path-utils"
|
||||
import { escapeGlob, isAbsolutePath } from "../path-utils"
|
||||
import { validateFiles } from "./file-links"
|
||||
import type { DiffVirtualFile, DiffVirtualProvider } from "../DiffVirtualProvider"
|
||||
|
||||
@@ -139,14 +139,17 @@ function show(uri: vscode.Uri, line?: number, column?: number): void {
|
||||
}
|
||||
|
||||
/**
|
||||
* Fallback when the exact path does not exist: search the workspace by filename.
|
||||
* Opens the file directly on a single match, prompts on multiple, warns on none.
|
||||
* Fallback when the exact path does not exist: search the session directory by
|
||||
* filename. Opens the file directly on a single match, prompts on multiple,
|
||||
* warns on none. The search is scoped to `dir` (the active session's directory)
|
||||
* via a RelativePattern so it can't cross into another worktree/branch.
|
||||
*/
|
||||
function findFallback(filePath: string, line?: number, column?: number): void {
|
||||
function findFallback(dir: string, filePath: string, line?: number, column?: number): void {
|
||||
const name = filePath.split(/[\\/]/).pop() || filePath
|
||||
// Escape glob metacharacters so filenames like `[id].tsx` or `[...slug].tsx` resolve correctly.
|
||||
const escaped = name.replace(/[\[\]{}?*!()]/g, "\\$&")
|
||||
Promise.resolve(vscode.workspace.findFiles(`**/${escaped}`, "**/node_modules/**", 5)).then(
|
||||
// VS Code globs don't honor backslash escapes, so bracket-escape metacharacters
|
||||
// (e.g. `[id].tsx`) instead — otherwise such names never match.
|
||||
const pattern = new vscode.RelativePattern(vscode.Uri.file(dir), `**/${escapeGlob(name)}`)
|
||||
Promise.resolve(vscode.workspace.findFiles(pattern, "**/node_modules/**", 5)).then(
|
||||
(matches) => {
|
||||
if (matches.length === 1) {
|
||||
show(matches[0], line, column)
|
||||
@@ -178,6 +181,6 @@ function openFile(dir: string, filePath: string, line?: number, column?: number)
|
||||
}
|
||||
show(uri, line, column)
|
||||
},
|
||||
() => findFallback(filePath, line, column),
|
||||
() => findFallback(dir, filePath, line, column),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import * as vscode from "vscode"
|
||||
import { isAbsolutePath } from "../path-utils"
|
||||
import { contains, isAbsolutePath } from "../path-utils"
|
||||
|
||||
/**
|
||||
* Stat-check candidate paths and return which ones are actual files (not directories).
|
||||
@@ -7,16 +7,19 @@ import { isAbsolutePath } from "../path-utils"
|
||||
* The webview marks every inline code span as a file-link candidate; this confirms
|
||||
* which of those candidates resolve to a real file so the webview can promote them
|
||||
* to clickable links and leave the rest as plain code.
|
||||
*
|
||||
* Candidates that resolve outside the session `root` (absolute paths elsewhere,
|
||||
* UNC paths, or `../` traversal) are rejected without touching the filesystem, so
|
||||
* auto-validated model output can't probe arbitrary host paths.
|
||||
*/
|
||||
export function validateFiles(root: string, paths: string[]): Promise<string[]> {
|
||||
const resolve = (p: string) =>
|
||||
isAbsolutePath(p) ? vscode.Uri.file(p) : vscode.Uri.joinPath(vscode.Uri.file(root), p)
|
||||
return Promise.all(
|
||||
paths.map((p) =>
|
||||
Promise.resolve(vscode.workspace.fs.stat(resolve(p))).then(
|
||||
(s) => (s.type & vscode.FileType.File ? p : null),
|
||||
() => null,
|
||||
),
|
||||
),
|
||||
).then((r) => r.filter((x): x is string => x !== null))
|
||||
const check = (p: string): Promise<string | null> => {
|
||||
if (!contains(root, p)) return Promise.resolve(null)
|
||||
const uri = isAbsolutePath(p) ? vscode.Uri.file(p) : vscode.Uri.joinPath(vscode.Uri.file(root), p)
|
||||
return Promise.resolve(vscode.workspace.fs.stat(uri)).then(
|
||||
(s) => (s.type & vscode.FileType.File ? p : null),
|
||||
() => null,
|
||||
)
|
||||
}
|
||||
return Promise.all(paths.map(check)).then((r) => r.filter((x): x is string => x !== null))
|
||||
}
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import * as path from "node:path"
|
||||
|
||||
/**
|
||||
* Check whether a file path is absolute.
|
||||
*
|
||||
@@ -25,3 +27,29 @@ export function isAbsolutePath(filePath: string): boolean {
|
||||
return true
|
||||
return false
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether `candidate` resolves to a location inside `root`.
|
||||
*
|
||||
* Rejects UNC candidates, absolute paths outside the root, and `../` traversal
|
||||
* that escapes the root. Used to keep filesystem probes scoped to the trusted
|
||||
* session directory so model-generated paths can't reach arbitrary host files.
|
||||
*/
|
||||
export function contains(root: string, candidate: string): boolean {
|
||||
if (!root || !candidate) return false
|
||||
// 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))
|
||||
return rel !== "" && !rel.startsWith("..") && !path.isAbsolute(rel)
|
||||
}
|
||||
|
||||
/**
|
||||
* Escape glob metacharacters so a literal filename can be embedded in a VS Code
|
||||
* glob pattern. VS Code globs do not honor backslash escapes, so each special
|
||||
* character is wrapped in a single-character bracket expression — e.g.
|
||||
* `[id].tsx` becomes `[[]id[]].tsx`.
|
||||
*/
|
||||
export function escapeGlob(name: string): string {
|
||||
return name.replace(/[*?{}[\]]/g, (c) => (c === "]" ? "[]]" : `[${c}]`))
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, it } from "bun:test"
|
||||
import { isAbsolutePath } from "../../src/path-utils"
|
||||
import { contains, escapeGlob, isAbsolutePath } from "../../src/path-utils"
|
||||
|
||||
describe("isAbsolutePath", () => {
|
||||
// ── Unix absolute paths ──────────────────────────────────────────────
|
||||
@@ -163,3 +163,47 @@ describe("isAbsolutePath", () => {
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe("contains", () => {
|
||||
it("accepts relative paths inside the root", () => {
|
||||
expect(contains("/work", "src/a.ts")).toBe(true)
|
||||
expect(contains("/work", "./src/a.ts")).toBe(true)
|
||||
expect(contains("/work", "a.ts")).toBe(true)
|
||||
})
|
||||
|
||||
it("rejects parent traversal that escapes the root", () => {
|
||||
expect(contains("/work", "../etc/passwd")).toBe(false)
|
||||
expect(contains("/work", "../../secret.ts")).toBe(false)
|
||||
})
|
||||
|
||||
it("rejects absolute paths outside the root", () => {
|
||||
expect(contains("/work", "/etc/passwd")).toBe(false)
|
||||
})
|
||||
|
||||
it("rejects UNC candidates", () => {
|
||||
expect(contains("/work", "\\\\server\\share\\file.ts")).toBe(false)
|
||||
expect(contains("/work", "//server/share/file.ts")).toBe(false)
|
||||
})
|
||||
|
||||
it("rejects empty inputs", () => {
|
||||
expect(contains("", "a.ts")).toBe(false)
|
||||
expect(contains("/work", "")).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe("escapeGlob", () => {
|
||||
it("bracket-escapes dynamic-route filenames", () => {
|
||||
expect(escapeGlob("[id].tsx")).toBe("[[]id[]].tsx")
|
||||
expect(escapeGlob("[...slug].tsx")).toBe("[[]...slug[]].tsx")
|
||||
})
|
||||
|
||||
it("escapes wildcard and brace metacharacters", () => {
|
||||
expect(escapeGlob("a*b?.ts")).toBe("a[*]b[?].ts")
|
||||
expect(escapeGlob("{x,y}.ts")).toBe("[{]x,y[}].ts")
|
||||
})
|
||||
|
||||
it("leaves ordinary filenames unchanged", () => {
|
||||
expect(escapeGlob("index.ts")).toBe("index.ts")
|
||||
expect(escapeGlob("file-path.test.ts")).toBe("file-path.test.ts")
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user