From 1351cf7dd48115b6c4ee5b7ba3424fc95d35058f Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sat, 4 Apr 2026 21:18:10 -0700 Subject: [PATCH] =?UTF-8?q?fix:=20validateOutputPath=20symlink=20bypass=20?= =?UTF-8?q?=E2=80=94=20resolve=20real=20path=20before=20safe-dir=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cherry-pick PR #745 by @Gonzih. Adds a second pass using fs.realpathSync() to resolve symlinks after lexical path validation. Co-Authored-By: Gonzih Co-Authored-By: Claude Opus 4.6 (1M context) --- browse/src/write-commands.ts | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/browse/src/write-commands.ts b/browse/src/write-commands.ts index 19283fef..5314795e 100644 --- a/browse/src/write-commands.ts +++ b/browse/src/write-commands.ts @@ -18,10 +18,39 @@ const SAFE_DIRECTORIES = [TEMP_DIR, process.cwd()]; function validateOutputPath(filePath: string): void { const resolved = path.resolve(filePath); + + // Basic containment check using lexical resolution only. + // This catches obvious traversal (../../../etc/passwd) but NOT symlinks. const isSafe = SAFE_DIRECTORIES.some(dir => isPathWithin(resolved, dir)); if (!isSafe) { throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`); } + + // Symlink check: resolve the real path of the nearest existing ancestor + // directory and re-validate. This closes the symlink bypass where a + // symlink inside /tmp or cwd points outside the safe zone. + // + // We resolve the parent dir (not the file itself — it may not exist yet). + // If the parent doesn't exist either we fall back up the tree. + let dir = path.dirname(resolved); + let realDir: string; + try { + realDir = fs.realpathSync(dir); + } catch { + // Parent doesn't exist — check the grandparent, or skip if inaccessible + try { + realDir = fs.realpathSync(path.dirname(dir)); + } catch { + // Can't resolve — fail safe + throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`); + } + } + + const realResolved = path.join(realDir, path.basename(resolved)); + const isRealSafe = SAFE_DIRECTORIES.some(dir => isPathWithin(realResolved, dir)); + if (!isRealSafe) { + throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')} (symlink target blocked)`); + } } /**