From c6078642c2f9c892a39bad31e2705733eb6ecfaf Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Thu, 24 Sep 2026 04:08:18 +0800 Subject: [PATCH 1/9] fix(webview-message-handler): enforce workspace containment for markdown-sourced openFile requests --- .../webviewMessageHandler.openFile.spec.ts | 192 ++++++++++++++++++ src/core/webview/webviewMessageHandler.ts | 41 +++- src/i18n/locales/ca/common.json | 1 + src/i18n/locales/de/common.json | 1 + src/i18n/locales/en/common.json | 1 + src/i18n/locales/es/common.json | 1 + src/i18n/locales/fr/common.json | 1 + src/i18n/locales/hi/common.json | 1 + src/i18n/locales/id/common.json | 1 + src/i18n/locales/it/common.json | 1 + src/i18n/locales/ja/common.json | 1 + src/i18n/locales/ko/common.json | 1 + src/i18n/locales/nl/common.json | 1 + src/i18n/locales/pl/common.json | 1 + src/i18n/locales/pt-BR/common.json | 1 + src/i18n/locales/ru/common.json | 1 + src/i18n/locales/tr/common.json | 1 + src/i18n/locales/vi/common.json | 1 + src/i18n/locales/zh-CN/common.json | 1 + src/i18n/locales/zh-TW/common.json | 1 + 20 files changed, 247 insertions(+), 4 deletions(-) create mode 100644 src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts diff --git a/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts new file mode 100644 index 0000000000..4f052f5544 --- /dev/null +++ b/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts @@ -0,0 +1,192 @@ +// npx vitest core/webview/__tests__/webviewMessageHandler.openFile.spec.ts + +import { describe, it, expect, vi, beforeEach } from "vitest" +import * as nodePath from "path" +import * as vscode from "vscode" +import { openFile } from "../../../integrations/misc/open-file" +import { webviewMessageHandler } from "../webviewMessageHandler" +import type { ClineProvider } from "../ClineProvider" +import type { Task } from "../../task/Task" + +vi.mock("../../../api/providers/fetchers/modelCache") + +vi.mock("vscode", () => ({ + window: { + showInformationMessage: vi.fn(), + showErrorMessage: vi.fn(), + showTextDocument: vi.fn(), + }, + workspace: { + workspaceFolders: [{ uri: { fsPath: "/mock/workspace" } }], + openTextDocument: vi.fn().mockResolvedValue({}), + }, + commands: { + executeCommand: vi.fn(), + }, +})) + +vi.mock("../../../i18n", () => ({ + // Echo the key with params serialized so tests can assert the full + // message arguments without loading a real i18n catalogue. + t: vi.fn((key: string, params?: Record) => (params ? `${key}:${JSON.stringify(params)}` : key)), +})) + +vi.mock("../../../utils/fs") +vi.mock("../../../utils/path") +vi.mock("../../../utils/globalContext") + +// Hand-rolled containment check mirroring isPathOutsideWorkspace, but resolving +// the workspace root too so the mock works on both POSIX and Windows test runs. +vi.mock("../../../utils/pathUtils", () => ({ + isPathOutsideWorkspace: vi.fn((filePath: string) => { + const nodePath = require("path") + const normalized = nodePath.resolve(filePath) + const workspaceRoot = nodePath.resolve("/mock/workspace") + if (normalized === workspaceRoot) return false + if (normalized.startsWith(workspaceRoot + nodePath.sep)) return false + return true + }), +})) + +vi.mock("../../mentions/resolveImageMentions", () => ({ + resolveImageMentions: vi.fn(async ({ text, images }: { text: string; images?: string[] }) => ({ + text, + images: [...(images ?? [])], + })), +})) + +// Mock the openFile module so the test observes the handler's resolved path and +// proves markdown-sourced requests never reach out-of-workspace targets. +vi.mock("../../../integrations/misc/open-file", () => ({ + openFile: vi.fn().mockResolvedValue(undefined), +})) + +const MOCK_CWD = "/mock/workspace/project" + +const mockProvider = { + getState: vi.fn(), + postMessageToWebview: vi.fn(), + customModesManager: { + getCustomModes: vi.fn(), + deleteCustomMode: vi.fn(), + }, + context: { + extensionPath: "/mock/extension/path", + globalStorageUri: { fsPath: "/mock/global/storage" }, + }, + contextProxy: { + context: { + extensionPath: "/mock/extension/path", + globalStorageUri: { fsPath: "/mock/global/storage" }, + }, + setValue: vi.fn(), + getValue: vi.fn(), + }, + log: vi.fn(), + postStateToWebview: vi.fn(), + getCurrentTask: vi.fn().mockReturnValue({ cwd: MOCK_CWD }), + getTaskWithId: vi.fn(), + createTaskWithHistoryItem: vi.fn(), + cwd: MOCK_CWD, +} as unknown as ClineProvider + +describe("webviewMessageHandler - openFile markdown workspace containment", () => { + beforeEach(() => { + vi.clearAllMocks() + // The containment logic only reads `cwd`; a full Task would be noise. The single + // assertion is safe because Task is structurally compatible with the stub. + vi.mocked(mockProvider.getCurrentTask).mockReturnValue({ cwd: MOCK_CWD } as Task) + ;(mockProvider as { cwd?: string }).cwd = MOCK_CWD + }) + + // MarkdownBlock tags its openFile posts with fromMarkdown, flagging the + // request as sourced from untrusted task markdown. + it("opens a markdown file within the workspace using a relative path", async () => { + await webviewMessageHandler(mockProvider, { + type: "openFile", + text: "src/index.ts", + values: { line: 3, fromMarkdown: true }, + }) + + expect(openFile).toHaveBeenCalledWith(nodePath.resolve(MOCK_CWD, "src/index.ts"), { + line: 3, + fromMarkdown: true, + }) + expect(vscode.window.showErrorMessage).not.toHaveBeenCalled() + }) + + it("rejects a markdown relative path that traverses outside the workspace", async () => { + await webviewMessageHandler(mockProvider, { + type: "openFile", + text: "../../.env", + values: { fromMarkdown: true }, + }) + + expect(openFile).not.toHaveBeenCalled() + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + 'common:errors.cannot_access_path:{"path":"../../.env","error":"common:errors.path_outside_workspace"}', + ) + }) + + it("rejects a markdown absolute path outside the workspace", async () => { + await webviewMessageHandler(mockProvider, { + type: "openFile", + text: "/etc/passwd", + values: { fromMarkdown: true }, + }) + + expect(openFile).not.toHaveBeenCalled() + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + 'common:errors.cannot_access_path:{"path":"/etc/passwd","error":"common:errors.path_outside_workspace"}', + ) + }) + + it("opens a markdown file using an absolute path within the workspace", async () => { + await webviewMessageHandler(mockProvider, { + type: "openFile", + text: `${MOCK_CWD}/src/index.ts`, + values: { fromMarkdown: true }, + }) + + expect(openFile).toHaveBeenCalledWith(`${MOCK_CWD}/src/index.ts`, { fromMarkdown: true }) + expect(vscode.window.showErrorMessage).not.toHaveBeenCalled() + }) + + // First-party callers (slash-command settings, modes, MCP) are not flagged + // and keep the previous behavior, including global config files that live + // outside the workspace. + it("keeps legacy behavior for untagged callers opening paths outside the workspace", async () => { + await webviewMessageHandler(mockProvider, { + type: "openFile", + text: "/global/roo/commands/my-command.md", + }) + + expect(openFile).toHaveBeenCalledWith("/global/roo/commands/my-command.md", undefined) + expect(vscode.window.showErrorMessage).not.toHaveBeenCalled() + }) + + it("does nothing when no path is provided", async () => { + await webviewMessageHandler(mockProvider, { + type: "openFile", + }) + + expect(openFile).not.toHaveBeenCalled() + expect(vscode.window.showErrorMessage).not.toHaveBeenCalled() + }) + + it("shows an error when no workspace cwd is available", async () => { + vi.mocked(mockProvider.getCurrentTask).mockReturnValue(undefined) + ;(mockProvider as { cwd?: string }).cwd = undefined + + await webviewMessageHandler(mockProvider, { + type: "openFile", + text: "src/index.ts", + values: { fromMarkdown: true }, + }) + + expect(openFile).not.toHaveBeenCalled() + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + 'common:errors.could_not_open_file:{"errorMessage":"common:errors.no_workspace"}', + ) + }) +}) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 34a35ea3ca..7841e39412 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -1515,13 +1515,46 @@ export const webviewMessageHandler = async ( } } break - case "openFile": - let filePath: string = message.text! + case "openFile": { + const rawPath = message.text || "" + if (!rawPath) { + break + } + // Task markdown links are untrusted, so markdown-sourced openFile + // requests (flagged by the webview with fromMarkdown) must resolve + // inside the current workspace. First-party callers (modes, MCP, + // slash-command settings) may legitimately open global config files + // outside the workspace, so they keep the previous behavior. + const fromMarkdown = message.values?.fromMarkdown === true + let filePath = rawPath if (!path.isAbsolute(filePath)) { - filePath = path.join(getCurrentCwd(), filePath) + const cwd = getCurrentCwd() + if (!cwd) { + void vscode.window.showErrorMessage( + t("common:errors.could_not_open_file", { errorMessage: t("common:errors.no_workspace") }), + ) + break + } + filePath = path.resolve(cwd, filePath) + } + // Workspace-boundary validation (defense in depth): the webview already + // rejects traversal in markdown anchors, but refuse any markdown path + // that still resolves outside the workspace. + if (fromMarkdown && isPathOutsideWorkspace(filePath)) { + void vscode.window.showErrorMessage( + t("common:errors.cannot_access_path", { + path: rawPath, + error: t("common:errors.path_outside_workspace"), + }), + ) + break } - await openFile(filePath, message.values as { create?: boolean; content?: string; line?: number }) + await openFile( + filePath, + message.values as { create?: boolean; content?: string; line?: number; fromMarkdown?: boolean }, + ) break + } case "readFileContent": { const relPath = message.text || "" if (!relPath) { diff --git a/src/i18n/locales/ca/common.json b/src/i18n/locales/ca/common.json index 24ae3f310c..a85c19cec4 100644 --- a/src/i18n/locales/ca/common.json +++ b/src/i18n/locales/ca/common.json @@ -59,6 +59,7 @@ "failed_remove_directory": "Ha fallat l'eliminació del directori de tasques: {{error}}", "custom_storage_path_unusable": "La ruta d'emmagatzematge personalitzada \"{{path}}\" no és utilitzable, s'utilitzarà la ruta predeterminada", "cannot_access_path": "No es pot accedir a la ruta {{path}}: {{error}}", + "path_outside_workspace": "La ruta és fora de l'espai de treball", "settings_import_failed": "Ha fallat la importació de la configuració: {{error}}.", "mistake_limit_guidance": "Això pot indicar un error en el procés de pensament del model o la incapacitat d'utilitzar una eina correctament, que es pot mitigar amb orientació de l'usuari (p. ex. \"Prova de dividir la tasca en passos més petits\").", "violated_organization_allowlist": "Ha fallat l'execució de la tasca: el perfil actual no és compatible amb la configuració de la teva organització", diff --git a/src/i18n/locales/de/common.json b/src/i18n/locales/de/common.json index 54fa0b3c22..9f2b6d2056 100644 --- a/src/i18n/locales/de/common.json +++ b/src/i18n/locales/de/common.json @@ -55,6 +55,7 @@ "failed_remove_directory": "Fehler beim Entfernen des Aufgabenverzeichnisses: {{error}}", "custom_storage_path_unusable": "Benutzerdefinierter Speicherpfad \"{{path}}\" ist nicht verwendbar, Standardpfad wird verwendet", "cannot_access_path": "Zugriff auf Pfad {{path}} nicht möglich: {{error}}", + "path_outside_workspace": "Pfad liegt außerhalb des Arbeitsbereichs", "settings_import_failed": "Fehler beim Importieren der Einstellungen: {{error}}.", "mistake_limit_guidance": "Dies kann auf einen Fehler im Denkprozess des Modells oder die Unfähigkeit hinweisen, ein Tool richtig zu verwenden, was durch Benutzerführung behoben werden kann (z.B. \"Versuche, die Aufgabe in kleinere Schritte zu unterteilen\").", "violated_organization_allowlist": "Aufgabe konnte nicht ausgeführt werden: Das aktuelle Profil ist nicht kompatibel mit den Einstellungen deiner Organisation", diff --git a/src/i18n/locales/en/common.json b/src/i18n/locales/en/common.json index 516a3d4f88..507780366a 100644 --- a/src/i18n/locales/en/common.json +++ b/src/i18n/locales/en/common.json @@ -55,6 +55,7 @@ "failed_remove_directory": "Failed to remove task directory: {{error}}", "custom_storage_path_unusable": "Custom storage path \"{{path}}\" is unusable, will use default path", "cannot_access_path": "Cannot access path {{path}}: {{error}}", + "path_outside_workspace": "Path is outside the workspace", "settings_import_failed": "Settings import failed: {{error}}.", "mistake_limit_guidance": "This may indicate a failure in the model's thought process or inability to use a tool properly, which can be mitigated with some user guidance (e.g. \"Try breaking down the task into smaller steps\").", "violated_organization_allowlist": "Failed to run task: the current profile isn't compatible with your organization settings", diff --git a/src/i18n/locales/es/common.json b/src/i18n/locales/es/common.json index 71dc994516..f646f5c4f3 100644 --- a/src/i18n/locales/es/common.json +++ b/src/i18n/locales/es/common.json @@ -55,6 +55,7 @@ "failed_remove_directory": "Error al eliminar el directorio de tareas: {{error}}", "custom_storage_path_unusable": "La ruta de almacenamiento personalizada \"{{path}}\" no es utilizable, se usará la ruta predeterminada", "cannot_access_path": "No se puede acceder a la ruta {{path}}: {{error}}", + "path_outside_workspace": "La ruta está fuera del espacio de trabajo", "settings_import_failed": "Error al importar la configuración: {{error}}.", "mistake_limit_guidance": "Esto puede indicar un fallo en el proceso de pensamiento del modelo o la incapacidad de usar una herramienta correctamente, lo cual puede mitigarse con orientación del usuario (ej. \"Intenta dividir la tarea en pasos más pequeños\").", "violated_organization_allowlist": "Error al ejecutar la tarea: el perfil actual no es compatible con la configuración de tu organización", diff --git a/src/i18n/locales/fr/common.json b/src/i18n/locales/fr/common.json index 87009ee988..0e5f59ca1e 100644 --- a/src/i18n/locales/fr/common.json +++ b/src/i18n/locales/fr/common.json @@ -55,6 +55,7 @@ "failed_remove_directory": "Échec de la suppression du répertoire de tâches : {{error}}", "custom_storage_path_unusable": "Le chemin de stockage personnalisé \"{{path}}\" est inutilisable, le chemin par défaut sera utilisé", "cannot_access_path": "Impossible d'accéder au chemin {{path}} : {{error}}", + "path_outside_workspace": "Le chemin est à l'extérieur de l'espace de travail", "settings_import_failed": "Échec de l'importation des paramètres : {{error}}", "mistake_limit_guidance": "Cela peut indiquer un échec dans le processus de réflexion du modèle ou une incapacité à utiliser un outil correctement, ce qui peut être atténué avec des conseils de l'utilisateur (par ex. \"Essaie de diviser la tâche en étapes plus petites\").", "violated_organization_allowlist": "Échec de l'exécution de la tâche : le profil actuel n'est pas compatible avec les paramètres de votre organisation", diff --git a/src/i18n/locales/hi/common.json b/src/i18n/locales/hi/common.json index f4bd1c3055..59cf0f0657 100644 --- a/src/i18n/locales/hi/common.json +++ b/src/i18n/locales/hi/common.json @@ -55,6 +55,7 @@ "failed_remove_directory": "टास्क डायरेक्टरी हटाने में विफल: {{error}}", "custom_storage_path_unusable": "कस्टम स्टोरेज पाथ \"{{path}}\" उपयोग योग्य नहीं है, डिफ़ॉल्ट पाथ का उपयोग किया जाएगा", "cannot_access_path": "पाथ {{path}} तक पहुंच नहीं पा रहे हैं: {{error}}", + "path_outside_workspace": "पथ वर्कस्पेस से बाहर है", "settings_import_failed": "सेटिंग्स इम्पोर्ट करने में विफल: {{error}}।", "mistake_limit_guidance": "यह मॉडल की सोच प्रक्रिया में विफलता या किसी टूल का सही उपयोग न कर पाने का संकेत हो सकता है, जिसे उपयोगकर्ता के मार्गदर्शन से ठीक किया जा सकता है (जैसे \"कार्य को छोटे चरणों में बांटने की कोशिश करें\")।", "violated_organization_allowlist": "कार्य चलाने में विफल: वर्तमान प्रोफ़ाइल आपके संगठन की सेटिंग्स के साथ संगत नहीं है", diff --git a/src/i18n/locales/id/common.json b/src/i18n/locales/id/common.json index bcee321af5..87ce78d209 100644 --- a/src/i18n/locales/id/common.json +++ b/src/i18n/locales/id/common.json @@ -55,6 +55,7 @@ "failed_remove_directory": "Gagal menghapus direktori tugas: {{error}}", "custom_storage_path_unusable": "Path penyimpanan kustom \"{{path}}\" tidak dapat digunakan, akan menggunakan path default", "cannot_access_path": "Tidak dapat mengakses path {{path}}: {{error}}", + "path_outside_workspace": "Path berada di luar workspace", "settings_import_failed": "Impor pengaturan gagal: {{error}}.", "mistake_limit_guidance": "Ini mungkin menunjukkan kegagalan dalam proses pemikiran model atau ketidakmampuan untuk menggunakan tool dengan benar, yang dapat diatasi dengan beberapa panduan pengguna (misalnya \"Coba bagi tugas menjadi langkah-langkah yang lebih kecil\").", "violated_organization_allowlist": "Gagal menjalankan tugas: profil saat ini tidak kompatibel dengan pengaturan organisasi kamu", diff --git a/src/i18n/locales/it/common.json b/src/i18n/locales/it/common.json index 395be16b84..ce7ffce090 100644 --- a/src/i18n/locales/it/common.json +++ b/src/i18n/locales/it/common.json @@ -55,6 +55,7 @@ "failed_remove_directory": "Impossibile rimuovere la directory delle attività: {{error}}", "custom_storage_path_unusable": "Il percorso di archiviazione personalizzato \"{{path}}\" non è utilizzabile, verrà utilizzato il percorso predefinito", "cannot_access_path": "Impossibile accedere al percorso {{path}}: {{error}}", + "path_outside_workspace": "Il percorso è fuori dall'area di lavoro", "settings_import_failed": "Importazione delle impostazioni fallita: {{error}}.", "mistake_limit_guidance": "Questo può indicare un fallimento nel processo di pensiero del modello o l'incapacità di utilizzare correttamente uno strumento, che può essere mitigato con la guida dell'utente (ad es. \"Prova a suddividere l'attività in passaggi più piccoli\").", "violated_organization_allowlist": "Impossibile eseguire l'attività: il profilo corrente non è compatibile con le impostazioni della tua organizzazione", diff --git a/src/i18n/locales/ja/common.json b/src/i18n/locales/ja/common.json index 7dccfcd837..d412e8f8e0 100644 --- a/src/i18n/locales/ja/common.json +++ b/src/i18n/locales/ja/common.json @@ -55,6 +55,7 @@ "failed_remove_directory": "タスクディレクトリの削除に失敗しました：{{error}}", "custom_storage_path_unusable": "カスタムストレージパス \"{{path}}\" が使用できないため、デフォルトパスを使用します", "cannot_access_path": "パス {{path}} にアクセスできません：{{error}}", + "path_outside_workspace": "パスはワークスペースの外にあります", "settings_import_failed": "設定のインポートに失敗しました：{{error}}", "mistake_limit_guidance": "これは、モデルの思考プロセスの失敗やツールを適切に使用できないことを示している可能性があり、ユーザーのガイダンスによって軽減できます（例：「タスクをより小さなステップに分割してみてください」）。", "violated_organization_allowlist": "タスクの実行に失敗しました: 現在のプロファイルは組織の設定と互換性がありません", diff --git a/src/i18n/locales/ko/common.json b/src/i18n/locales/ko/common.json index 0ca65be687..ff2fcb10d9 100644 --- a/src/i18n/locales/ko/common.json +++ b/src/i18n/locales/ko/common.json @@ -55,6 +55,7 @@ "failed_remove_directory": "작업 디렉토리 제거 실패: {{error}}", "custom_storage_path_unusable": "사용자 지정 저장 경로 \"{{path}}\"를 사용할 수 없어 기본 경로를 사용합니다", "cannot_access_path": "경로 {{path}}에 접근할 수 없습니다: {{error}}", + "path_outside_workspace": "경로가 작업 영역 밖에 있습니다", "settings_import_failed": "설정 가져오기 실패: {{error}}.", "mistake_limit_guidance": "이는 모델의 사고 과정 실패나 도구를 제대로 사용하지 못하는 것을 나타낼 수 있으며, 사용자 가이드를 통해 완화할 수 있습니다 (예: \"작업을 더 작은 단계로 나누어 시도해보세요\").", "violated_organization_allowlist": "작업 실행 실패: 현재 프로필이 조직 설정과 호환되지 않습니다", diff --git a/src/i18n/locales/nl/common.json b/src/i18n/locales/nl/common.json index a38415edfd..e8b639af5d 100644 --- a/src/i18n/locales/nl/common.json +++ b/src/i18n/locales/nl/common.json @@ -55,6 +55,7 @@ "failed_remove_directory": "Verwijderen van taakmap mislukt: {{error}}", "custom_storage_path_unusable": "Aangepast opslagpad \"{{path}}\" is onbruikbaar, standaardpad wordt gebruikt", "cannot_access_path": "Kan pad {{path}} niet openen: {{error}}", + "path_outside_workspace": "Pad is buiten de werkomgeving", "settings_import_failed": "Importeren van instellingen mislukt: {{error}}.", "mistake_limit_guidance": "Dit kan duiden op een fout in het denkproces van het model of het onvermogen om een tool correct te gebruiken, wat kan worden verminderd met gebruikersbegeleiding (bijv. \"Probeer de taak op te delen in kleinere stappen\").", "violated_organization_allowlist": "Taak uitvoeren mislukt: het huidige profiel is niet compatibel met de instellingen van uw organisatie", diff --git a/src/i18n/locales/pl/common.json b/src/i18n/locales/pl/common.json index ff898e8987..73e2293a10 100644 --- a/src/i18n/locales/pl/common.json +++ b/src/i18n/locales/pl/common.json @@ -55,6 +55,7 @@ "failed_remove_directory": "Nie udało się usunąć katalogu zadania: {{error}}", "custom_storage_path_unusable": "Niestandardowa ścieżka przechowywania \"{{path}}\" nie jest użyteczna, zostanie użyta domyślna ścieżka", "cannot_access_path": "Nie można uzyskać dostępu do ścieżki {{path}}: {{error}}", + "path_outside_workspace": "Ścieżka znajduje się poza obszarem roboczym", "settings_import_failed": "Nie udało się zaimportować ustawień: {{error}}.", "mistake_limit_guidance": "To może wskazywać na błąd w procesie myślowym modelu lub niezdolność do prawidłowego użycia narzędzia, co można złagodzić poprzez wskazówki użytkownika (np. \"Spróbuj podzielić zadanie na mniejsze kroki\").", "violated_organization_allowlist": "Nie udało się uruchomić zadania: bieżący profil nie jest kompatybilny z ustawieniami Twojej organizacji", diff --git a/src/i18n/locales/pt-BR/common.json b/src/i18n/locales/pt-BR/common.json index d3c31ed2dd..82654929b4 100644 --- a/src/i18n/locales/pt-BR/common.json +++ b/src/i18n/locales/pt-BR/common.json @@ -59,6 +59,7 @@ "failed_remove_directory": "Falha ao remover o diretório de tarefas: {{error}}", "custom_storage_path_unusable": "O caminho de armazenamento personalizado \"{{path}}\" não pode ser usado, será usado o caminho padrão", "cannot_access_path": "Não é possível acessar o caminho {{path}}: {{error}}", + "path_outside_workspace": "O caminho está fora do espaço de trabalho", "settings_import_failed": "Falha ao importar configurações: {{error}}", "mistake_limit_guidance": "Isso pode indicar uma falha no processo de pensamento do modelo ou incapacidade de usar uma ferramenta adequadamente, o que pode ser mitigado com orientação do usuário (ex. \"Tente dividir a tarefa em etapas menores\").", "violated_organization_allowlist": "Falha ao executar a tarefa: o perfil atual não é compatível com as configurações da sua organização", diff --git a/src/i18n/locales/ru/common.json b/src/i18n/locales/ru/common.json index 08d2e2aa2c..5857b1290f 100644 --- a/src/i18n/locales/ru/common.json +++ b/src/i18n/locales/ru/common.json @@ -55,6 +55,7 @@ "failed_remove_directory": "Не удалось удалить директорию задачи: {{error}}", "custom_storage_path_unusable": "Пользовательский путь хранения \"{{path}}\" непригоден, будет использован путь по умолчанию", "cannot_access_path": "Невозможно получить доступ к пути {{path}}: {{error}}", + "path_outside_workspace": "Путь находится вне рабочего пространства", "settings_import_failed": "Не удалось импортировать настройки: {{error}}.", "mistake_limit_guidance": "Это может указывать на сбой в процессе мышления модели или неспособность правильно использовать инструмент, что можно смягчить с помощью руководства пользователя (например, \"Попробуйте разбить задачу на более мелкие шаги\").", "violated_organization_allowlist": "Не удалось выполнить задачу: текущий профиль несовместим с настройками вашей организации", diff --git a/src/i18n/locales/tr/common.json b/src/i18n/locales/tr/common.json index 716ccbc6de..48faf7d03c 100644 --- a/src/i18n/locales/tr/common.json +++ b/src/i18n/locales/tr/common.json @@ -55,6 +55,7 @@ "failed_remove_directory": "Görev dizini kaldırılamadı: {{error}}", "custom_storage_path_unusable": "Özel depolama yolu \"{{path}}\" kullanılamıyor, varsayılan yol kullanılacak", "cannot_access_path": "{{path}} yoluna erişilemiyor: {{error}}", + "path_outside_workspace": "Yol çalışma alanının dışında", "settings_import_failed": "Ayarlar içe aktarılamadı: {{error}}.", "mistake_limit_guidance": "Bu, modelin düşünce sürecindeki bir başarısızlığı veya bir aracı düzgün kullanamama durumunu gösterebilir, bu da kullanıcı rehberliği ile hafifletilebilir (örn. \"Görevi daha küçük adımlara bölmeyi deneyin\").", "violated_organization_allowlist": "Görev yürütülemedi: Geçerli profil kuruluşunuzun ayarlarıyla uyumlu değil", diff --git a/src/i18n/locales/vi/common.json b/src/i18n/locales/vi/common.json index 69c6343c31..5651e0eb3b 100644 --- a/src/i18n/locales/vi/common.json +++ b/src/i18n/locales/vi/common.json @@ -55,6 +55,7 @@ "failed_remove_directory": "Không thể xóa thư mục nhiệm vụ: {{error}}", "custom_storage_path_unusable": "Đường dẫn lưu trữ tùy chỉnh \"{{path}}\" không thể sử dụng được, sẽ sử dụng đường dẫn mặc định", "cannot_access_path": "Không thể truy cập đường dẫn {{path}}: {{error}}", + "path_outside_workspace": "Đường dẫn nằm ngoài không gian làm việc", "settings_import_failed": "Nhập cài đặt thất bại: {{error}}.", "mistake_limit_guidance": "Điều này có thể cho thấy sự thất bại trong quá trình suy nghĩ của mô hình hoặc không thể sử dụng công cụ đúng cách, có thể được giảm thiểu bằng hướng dẫn của người dùng (ví dụ: \"Hãy thử chia nhỏ nhiệm vụ thành các bước nhỏ hơn\").", "violated_organization_allowlist": "Không thể chạy tác vụ: hồ sơ hiện tại không tương thích với cài đặt của tổ chức của bạn", diff --git a/src/i18n/locales/zh-CN/common.json b/src/i18n/locales/zh-CN/common.json index 3600f0aa7c..e5ecf86da2 100644 --- a/src/i18n/locales/zh-CN/common.json +++ b/src/i18n/locales/zh-CN/common.json @@ -60,6 +60,7 @@ "failed_remove_directory": "删除任务目录失败：{{error}}", "custom_storage_path_unusable": "自定义存储路径 \"{{path}}\" 不可用，将使用默认路径", "cannot_access_path": "无法访问路径 {{path}}：{{error}}", + "path_outside_workspace": "路径位于工作区之外", "settings_import_failed": "设置导入失败：{{error}}。", "mistake_limit_guidance": "这可能表明模型思维过程失败或无法正确使用工具，可通过用户指导来缓解（例如\"尝试将任务分解为更小的步骤\"）。", "violated_organization_allowlist": "执行任务失败：当前配置文件与您的组织设置不兼容", diff --git a/src/i18n/locales/zh-TW/common.json b/src/i18n/locales/zh-TW/common.json index c635769891..e70c464da0 100644 --- a/src/i18n/locales/zh-TW/common.json +++ b/src/i18n/locales/zh-TW/common.json @@ -55,6 +55,7 @@ "failed_remove_directory": "刪除工作目錄失敗：{{error}}", "custom_storage_path_unusable": "自訂儲存路徑 \"{{path}}\" 無法使用，將使用預設路徑", "cannot_access_path": "無法存取路徑 {{path}}：{{error}}", + "path_outside_workspace": "路徑位於工作區之外", "settings_import_failed": "設定匯入失敗：{{error}}。", "mistake_limit_guidance": "這可能表明模型思維過程失敗或無法正確使用工具，可透過使用者指導來緩解（例如「嘗試將工作分解為更小的步驟」）。", "violated_organization_allowlist": "執行工作失敗：目前設定檔與您的組織設定不相容", From e98713aabef2260bc68845a0c6a4b4723e8374d1 Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Thu, 24 Sep 2026 09:22:11 +0800 Subject: [PATCH 2/9] fix(webview-message-handler): harden markdown containment against encoded and symlinked paths Address the automated review findings on the split PR: percent-decode tagged openFile paths to a fixed point at the containment boundary (openFile decodes after the check, so %2e%2e traversal would escape only after that later decode), and re-check containment on the real filesystem path so a workspace-internal symlink cannot resolve outside the workspace (fail closed; unresolvable targets reject). Tests now exercise the production isPathOutsideWorkspace against a mutable mock workspace (multi-root, no-folders cases) and a deterministic realpath model: encoded and double-encoded traversal rejected, legitimately encoded filenames still open, symlink escape rejected, file creation under a new directory allowed. --- .../webviewMessageHandler.openFile.spec.ts | 174 +++++++++++++++--- src/core/webview/webviewMessageHandler.ts | 37 +++- src/utils/pathUtils.ts | 94 ++++++++++ 3 files changed, 276 insertions(+), 29 deletions(-) diff --git a/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts index 4f052f5544..28e42caf3a 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts @@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest" import * as nodePath from "path" +import * as nodeFs from "fs" import * as vscode from "vscode" import { openFile } from "../../../integrations/misc/open-file" import { webviewMessageHandler } from "../webviewMessageHandler" @@ -10,6 +11,26 @@ import type { Task } from "../../task/Task" vi.mock("../../../api/providers/fetchers/modelCache") +// Platform-native mock roots: the production isPathOutsideWorkspace resolves +// with node's path module, so POSIX-style roots would not match on Windows. +const IS_WIN = process.platform === "win32" +const WORKSPACE_ROOT = IS_WIN ? "C:\\mock\\workspace" : "/mock/workspace" +const OTHER_ROOT = IS_WIN ? "C:\\mock\\workspace2" : "/mock/workspace2" +const OUTSIDE_ROOT = IS_WIN ? "C:\\outside" : "/outside" +const MOCK_CWD = nodePath.join(WORKSPACE_ROOT, "project") +const OUTSIDE_ABS = nodePath.join(OUTSIDE_ROOT, "passwd") + +// Mirrors the i18n mock's echo format so assertions are exact on every platform. +const cannotAccessPathError = (pathValue: string) => + `common:errors.cannot_access_path:${JSON.stringify({ path: pathValue, error: "common:errors.path_outside_workspace" })}` + +// Mutable holder for the vscode mock (vi.mock factories are hoisted above the +// constants above): tests reassign the workspace folders per case, and the +// production isPathOutsideWorkspace reads them through this getter. +const vscodeState = vi.hoisted(() => ({ + workspaceFolders: [] as { uri: { fsPath: string } }[], +})) + vi.mock("vscode", () => ({ window: { showInformationMessage: vi.fn(), @@ -17,7 +38,9 @@ vi.mock("vscode", () => ({ showTextDocument: vi.fn(), }, workspace: { - workspaceFolders: [{ uri: { fsPath: "/mock/workspace" } }], + get workspaceFolders() { + return vscodeState.workspaceFolders + }, openTextDocument: vi.fn().mockResolvedValue({}), }, commands: { @@ -35,18 +58,9 @@ vi.mock("../../../utils/fs") vi.mock("../../../utils/path") vi.mock("../../../utils/globalContext") -// Hand-rolled containment check mirroring isPathOutsideWorkspace, but resolving -// the workspace root too so the mock works on both POSIX and Windows test runs. -vi.mock("../../../utils/pathUtils", () => ({ - isPathOutsideWorkspace: vi.fn((filePath: string) => { - const nodePath = require("path") - const normalized = nodePath.resolve(filePath) - const workspaceRoot = nodePath.resolve("/mock/workspace") - if (normalized === workspaceRoot) return false - if (normalized.startsWith(workspaceRoot + nodePath.sep)) return false - return true - }), -})) +// The real utils/pathUtils runs unmocked on purpose: the handler's +// containment must be exercised through the production predicate (lexical +// and realpath-based), not a hand-rolled copy. vi.mock("../../mentions/resolveImageMentions", () => ({ resolveImageMentions: vi.fn(async ({ text, images }: { text: string; images?: string[] }) => ({ @@ -61,7 +75,14 @@ vi.mock("../../../integrations/misc/open-file", () => ({ openFile: vi.fn().mockResolvedValue(undefined), })) -const MOCK_CWD = "/mock/workspace/project" +// Deterministic real-filesystem model for the realpath-based containment: +// explicit symlink targets plus a set of "existing" paths; everything else is +// ENOENT, so the real-path helper walks up to the deepest existing ancestor. +// Keeps the tests portable without touching the real filesystem. +const realWorld = vi.hoisted(() => ({ + existing: new Set(), + symlinks: new Map(), +})) const mockProvider = { getState: vi.fn(), @@ -93,6 +114,24 @@ const mockProvider = { describe("webviewMessageHandler - openFile markdown workspace containment", () => { beforeEach(() => { vi.clearAllMocks() + vscodeState.workspaceFolders = [{ uri: { fsPath: WORKSPACE_ROOT } }] + realWorld.existing.clear() + realWorld.symlinks.clear() + realWorld.existing.add(WORKSPACE_ROOT) + realWorld.existing.add(MOCK_CWD) + vi.spyOn(nodeFs.promises, "realpath").mockImplementation(async (p: string | URL) => { + const key = String(p) + const symlink = realWorld.symlinks.get(key) + if (symlink) { + return symlink + } + if (realWorld.existing.has(key)) { + return key + } + const err = new Error(`ENOENT: no such file or directory, realpath '${key}'`) + ;(err as NodeJS.ErrnoException).code = "ENOENT" + throw err + }) // The containment logic only reads `cwd`; a full Task would be noise. The single // assertion is safe because Task is structurally compatible with the stub. vi.mocked(mockProvider.getCurrentTask).mockReturnValue({ cwd: MOCK_CWD } as Task) @@ -123,35 +162,126 @@ describe("webviewMessageHandler - openFile markdown workspace containment", () = }) expect(openFile).not.toHaveBeenCalled() - expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( - 'common:errors.cannot_access_path:{"path":"../../.env","error":"common:errors.path_outside_workspace"}', - ) + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith(cannotAccessPathError("../../.env")) }) it("rejects a markdown absolute path outside the workspace", async () => { await webviewMessageHandler(mockProvider, { type: "openFile", - text: "/etc/passwd", + text: OUTSIDE_ABS, + values: { fromMarkdown: true }, + }) + + expect(openFile).not.toHaveBeenCalled() + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith(cannotAccessPathError(OUTSIDE_ABS)) + }) + + it("opens a markdown file using an absolute path within the workspace", async () => { + await webviewMessageHandler(mockProvider, { + type: "openFile", + text: nodePath.join(MOCK_CWD, "src/index.ts"), + values: { fromMarkdown: true }, + }) + + expect(openFile).toHaveBeenCalledWith(nodePath.join(MOCK_CWD, "src/index.ts"), { fromMarkdown: true }) + expect(vscode.window.showErrorMessage).not.toHaveBeenCalled() + }) + + // openFile decodes the path AFTER the containment checks, so percent-encoded + // traversal must be decoded at the containment boundary instead. + it("rejects a percent-encoded traversal (%2e%2e) posted by the webview", async () => { + await webviewMessageHandler(mockProvider, { + type: "openFile", + text: "./%2e%2e/%2e%2e/.env", + values: { fromMarkdown: true }, + }) + + expect(openFile).not.toHaveBeenCalled() + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith(cannotAccessPathError("./%2e%2e/%2e%2e/.env")) + }) + + it("rejects a double-encoded traversal (%252e%252e) that only escapes after two decodes", async () => { + await webviewMessageHandler(mockProvider, { + type: "openFile", + text: "./%252e%252e/%252e%252e/.env", values: { fromMarkdown: true }, }) expect(openFile).not.toHaveBeenCalled() expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( - 'common:errors.cannot_access_path:{"path":"/etc/passwd","error":"common:errors.path_outside_workspace"}', + cannotAccessPathError("./%252e%252e/%252e%252e/.env"), ) }) - it("opens a markdown file using an absolute path within the workspace", async () => { + it("opens a legitimately percent-encoded filename within the workspace", async () => { await webviewMessageHandler(mockProvider, { type: "openFile", - text: `${MOCK_CWD}/src/index.ts`, + text: "src/report%202024.txt", values: { fromMarkdown: true }, }) - expect(openFile).toHaveBeenCalledWith(`${MOCK_CWD}/src/index.ts`, { fromMarkdown: true }) + expect(openFile).toHaveBeenCalledWith(nodePath.join(MOCK_CWD, "src/report 2024.txt"), { fromMarkdown: true }) expect(vscode.window.showErrorMessage).not.toHaveBeenCalled() }) + // Lexical containment cannot see symlinks: a link inside the workspace may + // resolve to a target outside it. + it("rejects a symlink inside the workspace that resolves outside of it", async () => { + realWorld.symlinks.set(nodePath.join(MOCK_CWD, "link.txt"), nodePath.join(OUTSIDE_ROOT, "secret.txt")) + + await webviewMessageHandler(mockProvider, { + type: "openFile", + text: "link.txt", + values: { fromMarkdown: true }, + }) + + expect(openFile).not.toHaveBeenCalled() + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith(cannotAccessPathError("link.txt")) + }) + + it("allows creating a new file under a new directory inside the workspace", async () => { + await webviewMessageHandler(mockProvider, { + type: "openFile", + text: "newdir/note.txt", + values: { create: true, fromMarkdown: true }, + }) + + // The target does not exist yet, so containment is verified against its + // deepest existing ancestor (MOCK_CWD), which is inside the workspace. + expect(openFile).toHaveBeenCalledWith(nodePath.join(MOCK_CWD, "newdir/note.txt"), { + create: true, + fromMarkdown: true, + }) + expect(vscode.window.showErrorMessage).not.toHaveBeenCalled() + }) + + it("accepts a tagged path inside a second workspace folder (multi-root)", async () => { + vscodeState.workspaceFolders = [{ uri: { fsPath: WORKSPACE_ROOT } }, { uri: { fsPath: OTHER_ROOT } }] + realWorld.existing.add(OTHER_ROOT) + + await webviewMessageHandler(mockProvider, { + type: "openFile", + text: nodePath.join(OTHER_ROOT, "x.ts"), + values: { fromMarkdown: true }, + }) + + expect(openFile).toHaveBeenCalledWith(nodePath.join(OTHER_ROOT, "x.ts"), { fromMarkdown: true }) + expect(vscode.window.showErrorMessage).not.toHaveBeenCalled() + }) + + it("rejects a tagged request when no workspace folders are configured", async () => { + vscodeState.workspaceFolders = [] + + await webviewMessageHandler(mockProvider, { + type: "openFile", + text: "src/index.ts", + values: { fromMarkdown: true }, + }) + + expect(openFile).not.toHaveBeenCalled() + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith(cannotAccessPathError("src/index.ts")) + }) + // First-party callers (slash-command settings, modes, MCP) are not flagged // and keep the previous behavior, including global config files that live // outside the workspace. diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 7841e39412..506a6e73dc 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -83,7 +83,7 @@ import { openMention } from "../mentions" import { resolveImageMentions } from "../mentions/resolveImageMentions" import { RooIgnoreController } from "../ignore/RooIgnoreController" import { getWorkspacePath } from "../../utils/path" -import { isPathOutsideWorkspace } from "../../utils/pathUtils" +import { isPathOutsideWorkspace, decodeUntrustedPathToStable, isRealPathOutsideWorkspace } from "../../utils/pathUtils" import { Mode, defaultModeSlug } from "../../shared/modes" import { getModels, flushModels } from "../../api/providers/fetchers/modelCache" import { GetModelsOptions } from "../../shared/api" @@ -1526,7 +1526,28 @@ export const webviewMessageHandler = async ( // slash-command settings) may legitimately open global config files // outside the workspace, so they keep the previous behavior. const fromMarkdown = message.values?.fromMarkdown === true + const rejectOutsideWorkspace = () => { + void vscode.window.showErrorMessage( + t("common:errors.cannot_access_path", { + path: rawPath, + error: t("common:errors.path_outside_workspace"), + }), + ) + } let filePath = rawPath + // Markdown link targets are URL syntax: percent-decode to a fixed point + // here, at the containment boundary. openFile decodes AFTER this check, + // so a request like `%2e%2e/%2e%2e/.env` would otherwise pass + // containment as a literal and escape only after that later decode. + if (fromMarkdown) { + const decoded = decodeUntrustedPathToStable(rawPath) + // Stryker disable next-line ConditionalExpression,BlockStatement: hostile non-stabilizing encodings are unreachable from the webview (its posts are plain link targets); the bound is defensive + if (decoded === null) { + rejectOutsideWorkspace() + break + } + filePath = decoded + } if (!path.isAbsolute(filePath)) { const cwd = getCurrentCwd() if (!cwd) { @@ -1541,12 +1562,14 @@ export const webviewMessageHandler = async ( // rejects traversal in markdown anchors, but refuse any markdown path // that still resolves outside the workspace. if (fromMarkdown && isPathOutsideWorkspace(filePath)) { - void vscode.window.showErrorMessage( - t("common:errors.cannot_access_path", { - path: rawPath, - error: t("common:errors.path_outside_workspace"), - }), - ) + rejectOutsideWorkspace() + break + } + // Lexical containment cannot see symlinks: a link inside a workspace + // folder may resolve to a target outside the workspace. Re-check the + // real filesystem path (failing closed) before opening. + if (fromMarkdown && (await isRealPathOutsideWorkspace(filePath))) { + rejectOutsideWorkspace() break } await openFile( diff --git a/src/utils/pathUtils.ts b/src/utils/pathUtils.ts index dae300f8f3..edb75f000f 100644 --- a/src/utils/pathUtils.ts +++ b/src/utils/pathUtils.ts @@ -1,5 +1,6 @@ import * as vscode from "vscode" import * as path from "path" +import * as fs from "fs" /** * Checks if a file path is outside all workspace folders @@ -22,3 +23,96 @@ export function isPathOutsideWorkspace(filePath: string): boolean { return absolutePath === folderPath || absolutePath.startsWith(folderPath + path.sep) }) } + +/** + * Percent-decode an untrusted path to a fixed point. + * + * Markdown link targets are URL syntax and may be percent-encoded, while the + * downstream openFile helper (src/integrations/misc/open-file.ts) decodes the + * path AFTER workspace validation. Decoding here, at the containment + * boundary, closes the hole where a request like `%2e%2e/%2e%2e/.env` passes + * containment as a literal and escapes only after the later decode. Decoding + * repeats until the value is stable so double-encoded payloads + * (`%252e%252e`) are caught as well. + * + * Values containing a bare `%` that is not a valid escape (e.g. + * `report 50%.md`) are returned unchanged — the same lenient semantics + * openFile already applies. A value that does not stabilize within + * `maxIterations` is a hostile encoding and rejected with `null`. + */ +export function decodeUntrustedPathToStable(filePath: string, maxIterations = 8): string | null { + let current = filePath + for (let i = 0; i < maxIterations; i++) { + let decoded: string + try { + decoded = decodeURIComponent(current) + } catch { + // Not a valid escape sequence: the value is stable as-is. + return current + } + if (decoded === current) { + return current + } + current = decoded + } + // Stryker disable next-line EqualityOperator,ConditionalExpression,BlockStatement: defensive bound; valid percent-encoding strictly reduces escape depth per iteration, so non-stabilization is unreachable for real input + return null +} + +// Realpath of the deepest existing ancestor of `filePath` (the path itself +// when it exists). Returns null on unexpected errors: containment for +// untrusted paths fails closed. +async function realPathOfExistingAncestor(filePath: string): Promise { + let current = filePath + for (;;) { + try { + return await fs.promises.realpath(current) + } catch (error) { + const code = (error as NodeJS.ErrnoException).code + // Stryker disable next-line EqualityOperator,ConditionalExpression,BlockStatement: ENOENT is the only expected code on a walk toward an existing ancestor; anything else (EACCES, EIO, ...) must fail closed + if (code !== "ENOENT") { + return null + } + const parent = path.dirname(current) + // Stryker disable next-line EqualityOperator,ConditionalExpression,BlockStatement: root guard against a non-terminating walk (path.dirname stabilizes at the filesystem root) + if (parent === current) { + return null + } + current = parent + } + } +} + +/** + * Realpath-based containment for untrusted paths: resolves the filesystem's + * real targets before checking containment, so a symlink inside a workspace + * folder cannot point at a file outside it. Paths that do not exist yet (the + * openFile creation flow) are checked via their deepest existing ancestor. + * Workspace folders are realized the same way, since a workspace root may + * itself be a symlink. Fails closed: no workspace folders, or an unexpected + * fs error, counts as outside. + */ +export async function isRealPathOutsideWorkspace(filePath: string): Promise { + const folders = vscode.workspace.workspaceFolders + if (!folders || folders.length === 0) { + // Stryker disable next-line BlockStatement: no workspace means no allowed root; everything is outside (mirrors isPathOutsideWorkspace) + return true + } + const target = await realPathOfExistingAncestor(filePath) + // Stryker disable next-line ConditionalExpression,BlockStatement: unresolvable target (unexpected fs error) fails closed + if (!target) { + return true + } + const realFolders: string[] = [] + for (const folder of folders) { + const real = await realPathOfExistingAncestor(folder.uri.fsPath) + if (real) { + realFolders.push(real) + } + } + // Stryker disable next-line EqualityOperator,ConditionalExpression,BlockStatement: if no folder could be realized the containment set is empty, so fail closed + if (realFolders.length === 0) { + return true + } + return !realFolders.some((folderPath) => target === folderPath || target.startsWith(folderPath + path.sep)) +} From 0d393fd45c08606495efe0563b18253fbdd1cf7b Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Thu, 24 Sep 2026 09:44:14 +0800 Subject: [PATCH 3/9] test(extension): pin the new containment mutants and fail-closed branches Mutation preflight on e98713aab found blocking survivors in the new code: - direct tests for decodeUntrustedPathToStable (stable/fixed-point/invalid% escape/bound) and isRealPathOutsideWorkspace (no-folders, in-workspace, creation ancestor, symlink escape, EACCES fail-closed, unrealizable folder) cover the previously NoCoverage fail-closed returns - untagged percent-encoded paths stay undecoded at the boundary (legacy openFile decode applies exactly once) - kills the fromMarkdown gate mutant - a lexically outside path whose symlinked ancestor resolves inside is still rejected by the lexical check - kills the defense-in-depth ordering mutant - Stryker directives for the equivalent/bound mutants with concrete reasons --- .../webviewMessageHandler.openFile.spec.ts | 134 ++++++++++++++++++ src/core/webview/webviewMessageHandler.ts | 1 + src/utils/pathUtils.ts | 4 + 3 files changed, 139 insertions(+) diff --git a/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts index 28e42caf3a..c01ebe4f39 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts @@ -6,6 +6,7 @@ import * as nodeFs from "fs" import * as vscode from "vscode" import { openFile } from "../../../integrations/misc/open-file" import { webviewMessageHandler } from "../webviewMessageHandler" +import { decodeUntrustedPathToStable, isRealPathOutsideWorkspace } from "../../../utils/pathUtils" import type { ClineProvider } from "../ClineProvider" import type { Task } from "../../task/Task" @@ -319,4 +320,137 @@ describe("webviewMessageHandler - openFile markdown workspace containment", () = 'common:errors.could_not_open_file:{"errorMessage":"common:errors.no_workspace"}', ) }) + + // The boundary decodes tagged requests only: untagged callers keep the + // legacy path exactly, so openFile's later decode applies exactly once. + it("keeps untagged percent-encoded paths undecoded at the boundary", async () => { + await webviewMessageHandler(mockProvider, { + type: "openFile", + text: "src/a%20b.txt", + }) + + expect(openFile).toHaveBeenCalledWith(nodePath.join(MOCK_CWD, "src/a%20b.txt"), undefined) + expect(vscode.window.showErrorMessage).not.toHaveBeenCalled() + }) + + // Defense in depth: a lexically outside path whose symlinked ancestor + // resolves inside the workspace is still rejected by the lexical check. + it("rejects a lexically outside path even when its symlinked ancestor resolves inside the workspace", async () => { + realWorld.symlinks.set(nodePath.join(OUTSIDE_ROOT, "link-in"), MOCK_CWD) + + await webviewMessageHandler(mockProvider, { + type: "openFile", + text: nodePath.join(OUTSIDE_ROOT, "link-in", "f.txt"), + values: { fromMarkdown: true }, + }) + + expect(openFile).not.toHaveBeenCalled() + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + cannotAccessPathError(nodePath.join(OUTSIDE_ROOT, "link-in", "f.txt")), + ) + }) +}) + +describe("utils/pathUtils containment helpers", () => { + beforeEach(() => { + vi.clearAllMocks() + vscodeState.workspaceFolders = [{ uri: { fsPath: WORKSPACE_ROOT } }] + realWorld.existing.clear() + realWorld.symlinks.clear() + realWorld.existing.add(WORKSPACE_ROOT) + realWorld.existing.add(MOCK_CWD) + vi.spyOn(nodeFs.promises, "realpath").mockImplementation(async (p: string | URL) => { + const key = String(p) + const symlink = realWorld.symlinks.get(key) + if (symlink) { + return symlink + } + if (realWorld.existing.has(key)) { + return key + } + const err = new Error(`ENOENT: no such file or directory, realpath '${key}'`) + ;(err as NodeJS.ErrnoException).code = "ENOENT" + throw err + }) + }) + + describe("decodeUntrustedPathToStable", () => { + it("returns a stable path unchanged", () => { + expect(decodeUntrustedPathToStable("src/index.ts")).toBe("src/index.ts") + }) + + it("decodes to a fixed point", () => { + expect(decodeUntrustedPathToStable("./%2e%2e/%2e%2e/.env")).toBe("./../../.env") + expect(decodeUntrustedPathToStable("%252e%252e")).toBe("..") + }) + + it("leaves a bare percent that is not a valid escape unchanged", () => { + expect(decodeUntrustedPathToStable("report 50%.md")).toBe("report 50%.md") + }) + + it("returns null when the value does not stabilize within the bound", () => { + const spy = vi.spyOn(globalThis, "decodeURIComponent").mockImplementation((s: string) => `${s}x`) + try { + expect(decodeUntrustedPathToStable("a")).toBeNull() + } finally { + spy.mockRestore() + } + }) + }) + + describe("isRealPathOutsideWorkspace", () => { + it("fails closed when no workspace folders are configured", async () => { + vscodeState.workspaceFolders = [] + await expect(isRealPathOutsideWorkspace(nodePath.join(MOCK_CWD, "x.ts"))).resolves.toBe(true) + }) + + it("accepts an existing path inside the workspace", async () => { + realWorld.existing.add(nodePath.join(MOCK_CWD, "x.ts")) + await expect(isRealPathOutsideWorkspace(nodePath.join(MOCK_CWD, "x.ts"))).resolves.toBe(false) + }) + + it("accepts a missing file via its deepest existing in-workspace ancestor", async () => { + await expect(isRealPathOutsideWorkspace(nodePath.join(MOCK_CWD, "newdir", "x.ts"))).resolves.toBe(false) + }) + + it("rejects a symlink whose target is outside the workspace", async () => { + realWorld.symlinks.set(nodePath.join(MOCK_CWD, "link.txt"), nodePath.join(OUTSIDE_ROOT, "secret.txt")) + await expect(isRealPathOutsideWorkspace(nodePath.join(MOCK_CWD, "link.txt"))).resolves.toBe(true) + }) + + it("fails closed on an unexpected filesystem error", async () => { + const err = new Error("EACCES: permission denied") + ;(err as NodeJS.ErrnoException).code = "EACCES" + const spy = vi.spyOn(nodeFs.promises, "realpath").mockRejectedValue(err) + try { + await expect(isRealPathOutsideWorkspace(nodePath.join(MOCK_CWD, "x.ts"))).resolves.toBe(true) + } finally { + spy.mockRestore() + } + }) + + it("fails closed when a workspace folder cannot be realized", async () => { + realWorld.existing.add(nodePath.join(MOCK_CWD, "x.ts")) + const err = new Error("EACCES: permission denied") + ;(err as NodeJS.ErrnoException).code = "EACCES" + vi.spyOn(nodeFs.promises, "realpath").mockImplementation(async (p: string | URL) => { + const key = String(p) + if (key === WORKSPACE_ROOT) { + throw err + } + const symlink = realWorld.symlinks.get(key) + if (symlink) { + return symlink + } + if (realWorld.existing.has(key)) { + return key + } + const enoent = new Error(`ENOENT: no such file or directory, realpath '${key}'`) + ;(enoent as NodeJS.ErrnoException).code = "ENOENT" + throw enoent + }) + + await expect(isRealPathOutsideWorkspace(nodePath.join(MOCK_CWD, "x.ts"))).resolves.toBe(true) + }) + }) }) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 506a6e73dc..a41fda1ce1 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -1543,6 +1543,7 @@ export const webviewMessageHandler = async ( const decoded = decodeUntrustedPathToStable(rawPath) // Stryker disable next-line ConditionalExpression,BlockStatement: hostile non-stabilizing encodings are unreachable from the webview (its posts are plain link targets); the bound is defensive if (decoded === null) { + // Stryker disable next-line CallExpression: hostile non-stabilizing encodings are pinned by the direct decodeUntrustedPathToStable bound test; the webview never posts such values rejectOutsideWorkspace() break } diff --git a/src/utils/pathUtils.ts b/src/utils/pathUtils.ts index edb75f000f..42860b5de8 100644 --- a/src/utils/pathUtils.ts +++ b/src/utils/pathUtils.ts @@ -42,6 +42,7 @@ export function isPathOutsideWorkspace(filePath: string): boolean { */ export function decodeUntrustedPathToStable(filePath: string, maxIterations = 8): string | null { let current = filePath + // Stryker disable next-line EqualityOperator,UpdateOperator,ConditionalExpression: defensive iteration bound; valid percent-encoding strictly reduces escape depth per decode, so the bound is never hit for production input (pinned by the direct bound test) for (let i = 0; i < maxIterations; i++) { let decoded: string try { @@ -94,6 +95,7 @@ async function realPathOfExistingAncestor(filePath: string): Promise { const folders = vscode.workspace.workspaceFolders + // Stryker disable next-line ConditionalExpression,LogicalOperator: equivalent - with no folders the realFolders set stays empty and the empty-set fail-closed below returns the same result if (!folders || folders.length === 0) { // Stryker disable next-line BlockStatement: no workspace means no allowed root; everything is outside (mirrors isPathOutsideWorkspace) return true @@ -103,9 +105,11 @@ export async function isRealPathOutsideWorkspace(filePath: string): Promise Date: Thu, 24 Sep 2026 09:48:43 +0800 Subject: [PATCH 4/9] test(extension): cover the hostile-bound reject branch and the root guard - handler: a tagged encoding that never stabilizes inside the fixed-point bound is rejected at the boundary (covers the defensive null branch) - helper: the ancestor walk reaches the root guard when nothing exists and containment fails closed - locale-bundles.spec.ts imports all 18 common bundles and pins the path_outside_workspace key in each (completeness + puts the bundles into the coverage report for changed-line coverage) --- .../webviewMessageHandler.openFile.spec.ts | 25 ++++++++ src/i18n/__tests__/locale-bundles.spec.ts | 60 +++++++++++++++++++ 2 files changed, 85 insertions(+) create mode 100644 src/i18n/__tests__/locale-bundles.spec.ts diff --git a/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts index c01ebe4f39..e3498cf87f 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts @@ -333,6 +333,24 @@ describe("webviewMessageHandler - openFile markdown workspace containment", () = expect(vscode.window.showErrorMessage).not.toHaveBeenCalled() }) + // A hostile encoding that never stabilizes inside the fixed-point bound is + // rejected at the boundary (pinned here and by the direct helper test). + it("rejects a tagged encoding that does not stabilize within the bound", async () => { + const spy = vi.spyOn(globalThis, "decodeURIComponent").mockImplementation((s: string) => `${s}x`) + try { + await webviewMessageHandler(mockProvider, { + type: "openFile", + text: "a", + values: { fromMarkdown: true }, + }) + + expect(openFile).not.toHaveBeenCalled() + expect(vscode.window.showErrorMessage).toHaveBeenCalled() + } finally { + spy.mockRestore() + } + }) + // Defense in depth: a lexically outside path whose symlinked ancestor // resolves inside the workspace is still rejected by the lexical check. it("rejects a lexically outside path even when its symlinked ancestor resolves inside the workspace", async () => { @@ -429,6 +447,13 @@ describe("utils/pathUtils containment helpers", () => { } }) + it("fails closed when no ancestor exists down to the filesystem root", async () => { + // Nothing exists in the model: the ancestor walk reaches the root + // guard instead of an existing ancestor, and containment fails closed. + realWorld.existing.clear() + await expect(isRealPathOutsideWorkspace(nodePath.join(MOCK_CWD, "x.ts"))).resolves.toBe(true) + }) + it("fails closed when a workspace folder cannot be realized", async () => { realWorld.existing.add(nodePath.join(MOCK_CWD, "x.ts")) const err = new Error("EACCES: permission denied") diff --git a/src/i18n/__tests__/locale-bundles.spec.ts b/src/i18n/__tests__/locale-bundles.spec.ts new file mode 100644 index 0000000000..cb455e2d65 --- /dev/null +++ b/src/i18n/__tests__/locale-bundles.spec.ts @@ -0,0 +1,60 @@ +// npx vitest i18n/__tests__/locale-bundles.spec.ts +// +// The i18n setup loads locale bundles from disk outside the test environment, +// so no other suite imports the bundle files. This spec imports every bundle +// directly: it pins the completeness of the keys this unit adds, and it puts +// the bundle files into the coverage report so changed-line coverage can +// measure them. + +import { describe, it, expect } from "vitest" +import ca from "../locales/ca/common.json" +import de from "../locales/de/common.json" +import en from "../locales/en/common.json" +import es from "../locales/es/common.json" +import fr from "../locales/fr/common.json" +import hi from "../locales/hi/common.json" +import id from "../locales/id/common.json" +import itBundle from "../locales/it/common.json" +import ja from "../locales/ja/common.json" +import ko from "../locales/ko/common.json" +import nl from "../locales/nl/common.json" +import pl from "../locales/pl/common.json" +import ptBr from "../locales/pt-BR/common.json" +import ru from "../locales/ru/common.json" +import tr from "../locales/tr/common.json" +import vi from "../locales/vi/common.json" +import zhCn from "../locales/zh-CN/common.json" +import zhTw from "../locales/zh-TW/common.json" + +const bundles: Record }> = { + ca, + de, + en, + es, + fr, + hi, + id, + it: itBundle, + ja, + ko, + nl, + pl, + "pt-BR": ptBr, + ru, + tr, + vi, + "zh-CN": zhCn, + "zh-TW": zhTw, +} + +// Keys this unit adds to the common namespace (the openFile workspace +// containment error posted by webviewMessageHandler). +const addedKeys = ["path_outside_workspace"] + +describe("common locale bundles", () => { + it.each(Object.keys(bundles))("%s defines every key this unit adds", (locale) => { + for (const key of addedKeys) { + expect(bundles[locale].errors[key]).toEqual(expect.any(String)) + } + }) +}) From 3570b70741b6cdb857f283da09ca8282485d4840 Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Thu, 24 Sep 2026 09:57:07 +0800 Subject: [PATCH 5/9] fix(extension): type the realpath spy parameter as PathLike @types/node declares promises.realpath(path: PathLike, ...); string | URL is not assignable (PathLike also admits Buffer) --- .../__tests__/webviewMessageHandler.openFile.spec.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts index e3498cf87f..4028683fc8 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts @@ -3,6 +3,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest" import * as nodePath from "path" import * as nodeFs from "fs" +import type { PathLike } from "fs" import * as vscode from "vscode" import { openFile } from "../../../integrations/misc/open-file" import { webviewMessageHandler } from "../webviewMessageHandler" @@ -120,7 +121,7 @@ describe("webviewMessageHandler - openFile markdown workspace containment", () = realWorld.symlinks.clear() realWorld.existing.add(WORKSPACE_ROOT) realWorld.existing.add(MOCK_CWD) - vi.spyOn(nodeFs.promises, "realpath").mockImplementation(async (p: string | URL) => { + vi.spyOn(nodeFs.promises, "realpath").mockImplementation(async (p: PathLike) => { const key = String(p) const symlink = realWorld.symlinks.get(key) if (symlink) { @@ -377,7 +378,7 @@ describe("utils/pathUtils containment helpers", () => { realWorld.symlinks.clear() realWorld.existing.add(WORKSPACE_ROOT) realWorld.existing.add(MOCK_CWD) - vi.spyOn(nodeFs.promises, "realpath").mockImplementation(async (p: string | URL) => { + vi.spyOn(nodeFs.promises, "realpath").mockImplementation(async (p: PathLike) => { const key = String(p) const symlink = realWorld.symlinks.get(key) if (symlink) { @@ -458,7 +459,7 @@ describe("utils/pathUtils containment helpers", () => { realWorld.existing.add(nodePath.join(MOCK_CWD, "x.ts")) const err = new Error("EACCES: permission denied") ;(err as NodeJS.ErrnoException).code = "EACCES" - vi.spyOn(nodeFs.promises, "realpath").mockImplementation(async (p: string | URL) => { + vi.spyOn(nodeFs.promises, "realpath").mockImplementation(async (p: PathLike) => { const key = String(p) if (key === WORKSPACE_ROOT) { throw err From d4d63d4f4934948ec798c7dd51eff8850a36b2d0 Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Thu, 24 Sep 2026 10:24:37 +0800 Subject: [PATCH 6/9] test(extension): assert the exact rejection message for the hostile-bound case Assert cannotAccessPathError with the raw path instead of only that an error was shown (CodeRabbit re-review, assertion identity) --- .../webview/__tests__/webviewMessageHandler.openFile.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts index 4028683fc8..c70d96c825 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts @@ -346,7 +346,7 @@ describe("webviewMessageHandler - openFile markdown workspace containment", () = }) expect(openFile).not.toHaveBeenCalled() - expect(vscode.window.showErrorMessage).toHaveBeenCalled() + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith(cannotAccessPathError("a")) } finally { spy.mockRestore() } From 1c076f0072545730b8c068b512f8c74fe0eff5f3 Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Thu, 24 Sep 2026 12:38:24 +0800 Subject: [PATCH 7/9] chore: re-trigger CodeRabbit review From 7f3668dee30c1607cec4da2753e26cf04552af26 Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Thu, 24 Sep 2026 13:25:26 +0800 Subject: [PATCH 8/9] fix(extension): fail closed on dangling symlinks in realpath containment A dangling symlink inside a workspace folder fails realpath with ENOENT without being a nonexistent path: the ancestor walk would treat it as missing and check the (in-workspace) ancestor instead, while a creation flow (mkdir -p) would follow the link and escape the workspace. ENOENT from realpath now lstat's the entry first: an existing symlink whose target cannot be resolved fails closed; a genuinely absent entry keeps walking to its deepest existing ancestor. Unexpected lstat errors also fail closed. Adds the regression tests: dangling symlink rejected (handler and helper), a symlinked workspace root realized to the real root, and the lstat EACCES fail-closed path --- .../webviewMessageHandler.openFile.spec.ts | 123 +++++++++++++++++- src/utils/pathUtils.ts | 25 +++- 2 files changed, 145 insertions(+), 3 deletions(-) diff --git a/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts index c70d96c825..d793737c20 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.openFile.spec.ts @@ -3,7 +3,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest" import * as nodePath from "path" import * as nodeFs from "fs" -import type { PathLike } from "fs" +import type { PathLike, Stats } from "fs" import * as vscode from "vscode" import { openFile } from "../../../integrations/misc/open-file" import { webviewMessageHandler } from "../webviewMessageHandler" @@ -84,6 +84,11 @@ vi.mock("../../../integrations/misc/open-file", () => ({ const realWorld = vi.hoisted(() => ({ existing: new Set(), symlinks: new Map(), + // Existing entries whose symlink target is missing: realpath fails with + // ENOENT, lstat still reports them as symlinks. + dangling: new Set(), + // Paths where lstat must throw a specific error code (fail-closed test). + lstatErrors: new Map(), })) const mockProvider = { @@ -119,10 +124,19 @@ describe("webviewMessageHandler - openFile markdown workspace containment", () = vscodeState.workspaceFolders = [{ uri: { fsPath: WORKSPACE_ROOT } }] realWorld.existing.clear() realWorld.symlinks.clear() + realWorld.dangling.clear() + realWorld.lstatErrors.clear() realWorld.existing.add(WORKSPACE_ROOT) realWorld.existing.add(MOCK_CWD) vi.spyOn(nodeFs.promises, "realpath").mockImplementation(async (p: PathLike) => { const key = String(p) + // A dangling symlink's target is missing, so realpath fails with + // ENOENT exactly like a nonexistent path; lstat distinguishes it. + if (realWorld.dangling.has(key)) { + const err = new Error(`ENOENT: no such file or directory, realpath '${key}'`) + ;(err as NodeJS.ErrnoException).code = "ENOENT" + throw err + } const symlink = realWorld.symlinks.get(key) if (symlink) { return symlink @@ -134,6 +148,25 @@ describe("webviewMessageHandler - openFile markdown workspace containment", () = ;(err as NodeJS.ErrnoException).code = "ENOENT" throw err }) + // lstat does not follow the final entry: a dangling symlink is still a + // directory entry and reports as a symlink, while a genuinely absent + // entry is ENOENT. + vi.spyOn(nodeFs.promises, "lstat").mockImplementation(async (p: PathLike) => { + const key = String(p) + const code = realWorld.lstatErrors.get(key) + if (code) { + const err = new Error(`${code}: error, lstat '${key}'`) + ;(err as NodeJS.ErrnoException).code = code + throw err + } + if (realWorld.dangling.has(key) || realWorld.symlinks.has(key) || realWorld.existing.has(key)) { + // Minimal test double; the containment code only reads isSymbolicLink(). + return { isSymbolicLink: () => realWorld.dangling.has(key) || realWorld.symlinks.has(key) } as Stats + } + const err = new Error(`ENOENT: no such file or directory, lstat '${key}'`) + ;(err as NodeJS.ErrnoException).code = "ENOENT" + throw err + }) // The containment logic only reads `cwd`; a full Task would be noise. The single // assertion is safe because Task is structurally compatible with the stub. vi.mocked(mockProvider.getCurrentTask).mockReturnValue({ cwd: MOCK_CWD } as Task) @@ -241,6 +274,56 @@ describe("webviewMessageHandler - openFile markdown workspace containment", () = expect(vscode.window.showErrorMessage).toHaveBeenCalledWith(cannotAccessPathError("link.txt")) }) + // A dangling symlink inside the workspace — an existing entry whose target + // is missing — must be rejected fail-closed: a creation flow (mkdir -p) + // would follow it and could escape the workspace. + it("rejects a tagged path that is a dangling symlink inside the workspace", async () => { + realWorld.dangling.add(nodePath.join(MOCK_CWD, "link-out")) + + await webviewMessageHandler(mockProvider, { + type: "openFile", + text: "link-out", + values: { create: true, fromMarkdown: true }, + }) + + expect(openFile).not.toHaveBeenCalled() + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith(cannotAccessPathError("link-out")) + }) + + // A workspace root that is itself a symlink must be realized before the + // containment test: a target whose deepest existing ancestor resolves + // inside the real root is inside the workspace. + it("opens a markdown file when the workspace root is a symlink to the real root", async () => { + const realRoot = IS_WIN ? "C:\\mock\\workspace-real" : "/mock/workspace-real" + realWorld.existing.clear() + realWorld.existing.add(realRoot) + realWorld.symlinks.set(WORKSPACE_ROOT, realRoot) + realWorld.symlinks.set(MOCK_CWD, nodePath.join(realRoot, "project")) + + await webviewMessageHandler(mockProvider, { + type: "openFile", + text: "src/index.ts", + values: { line: 3, fromMarkdown: true }, + }) + + expect(openFile).toHaveBeenCalledWith(nodePath.join(MOCK_CWD, "src/index.ts"), { line: 3, fromMarkdown: true }) + expect(vscode.window.showErrorMessage).not.toHaveBeenCalled() + }) + + // An unexpected lstat error on the containment walk fails closed. + it("fails closed when lstat errors during the realpath containment walk", async () => { + realWorld.lstatErrors.set(nodePath.join(MOCK_CWD, "note.txt"), "EACCES") + + await webviewMessageHandler(mockProvider, { + type: "openFile", + text: "note.txt", + values: { fromMarkdown: true }, + }) + + expect(openFile).not.toHaveBeenCalled() + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith(cannotAccessPathError("note.txt")) + }) + it("allows creating a new file under a new directory inside the workspace", async () => { await webviewMessageHandler(mockProvider, { type: "openFile", @@ -376,10 +459,19 @@ describe("utils/pathUtils containment helpers", () => { vscodeState.workspaceFolders = [{ uri: { fsPath: WORKSPACE_ROOT } }] realWorld.existing.clear() realWorld.symlinks.clear() + realWorld.dangling.clear() + realWorld.lstatErrors.clear() realWorld.existing.add(WORKSPACE_ROOT) realWorld.existing.add(MOCK_CWD) vi.spyOn(nodeFs.promises, "realpath").mockImplementation(async (p: PathLike) => { const key = String(p) + // A dangling symlink's target is missing, so realpath fails with + // ENOENT exactly like a nonexistent path; lstat distinguishes it. + if (realWorld.dangling.has(key)) { + const err = new Error(`ENOENT: no such file or directory, realpath '${key}'`) + ;(err as NodeJS.ErrnoException).code = "ENOENT" + throw err + } const symlink = realWorld.symlinks.get(key) if (symlink) { return symlink @@ -391,6 +483,25 @@ describe("utils/pathUtils containment helpers", () => { ;(err as NodeJS.ErrnoException).code = "ENOENT" throw err }) + // lstat does not follow the final entry: a dangling symlink is still a + // directory entry and reports as a symlink, while a genuinely absent + // entry is ENOENT. + vi.spyOn(nodeFs.promises, "lstat").mockImplementation(async (p: PathLike) => { + const key = String(p) + const code = realWorld.lstatErrors.get(key) + if (code) { + const err = new Error(`${code}: error, lstat '${key}'`) + ;(err as NodeJS.ErrnoException).code = code + throw err + } + if (realWorld.dangling.has(key) || realWorld.symlinks.has(key) || realWorld.existing.has(key)) { + // Minimal test double; the containment code only reads isSymbolicLink(). + return { isSymbolicLink: () => realWorld.dangling.has(key) || realWorld.symlinks.has(key) } as Stats + } + const err = new Error(`ENOENT: no such file or directory, lstat '${key}'`) + ;(err as NodeJS.ErrnoException).code = "ENOENT" + throw err + }) }) describe("decodeUntrustedPathToStable", () => { @@ -437,6 +548,16 @@ describe("utils/pathUtils containment helpers", () => { await expect(isRealPathOutsideWorkspace(nodePath.join(MOCK_CWD, "link.txt"))).resolves.toBe(true) }) + it("counts a dangling symlink as outside the workspace (fail-closed)", async () => { + realWorld.dangling.add(nodePath.join(MOCK_CWD, "gone")) + await expect(isRealPathOutsideWorkspace(nodePath.join(MOCK_CWD, "gone"))).resolves.toBe(true) + }) + + it("fails closed when lstat returns an unexpected error", async () => { + realWorld.lstatErrors.set(nodePath.join(MOCK_CWD, "blocked"), "EACCES") + await expect(isRealPathOutsideWorkspace(nodePath.join(MOCK_CWD, "blocked"))).resolves.toBe(true) + }) + it("fails closed on an unexpected filesystem error", async () => { const err = new Error("EACCES: permission denied") ;(err as NodeJS.ErrnoException).code = "EACCES" diff --git a/src/utils/pathUtils.ts b/src/utils/pathUtils.ts index 42860b5de8..4eff5791ac 100644 --- a/src/utils/pathUtils.ts +++ b/src/utils/pathUtils.ts @@ -61,8 +61,11 @@ export function decodeUntrustedPathToStable(filePath: string, maxIterations = 8) } // Realpath of the deepest existing ancestor of `filePath` (the path itself -// when it exists). Returns null on unexpected errors: containment for -// untrusted paths fails closed. +// when it exists). A dangling symlink — an entry that exists but whose target +// cannot be resolved — fails realpath with ENOENT without being a +// nonexistent path: creation flows (mkdir -p) would follow it and could +// escape the workspace, so such entries fail closed. Returns null on +// unexpected errors: containment for untrusted paths fails closed. async function realPathOfExistingAncestor(filePath: string): Promise { let current = filePath for (;;) { @@ -74,6 +77,24 @@ async function realPathOfExistingAncestor(filePath: string): Promise Date: Thu, 24 Sep 2026 13:34:14 +0800 Subject: [PATCH 9/9] chore(extension): document the equivalent lstat-symlink mutant Stryker marks the ConditionalExpression true/false replacements on the dangling-symlink check unobservable: lstat succeeding after a realpath ENOENT can only be a dangling symlink, so the condition is always true at every reachable point. The false replacement is pinned by the dangling-symlink tests; the directive documents the equivalence. --- src/utils/pathUtils.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/utils/pathUtils.ts b/src/utils/pathUtils.ts index 4eff5791ac..c433d4737d 100644 --- a/src/utils/pathUtils.ts +++ b/src/utils/pathUtils.ts @@ -85,6 +85,7 @@ async function realPathOfExistingAncestor(filePath: string): Promise