From e74aca446ba36f45ee1c6dca9e7abb3a8237c5ee Mon Sep 17 00:00:00 2001 From: "Fredrik Liljegren (Claude Code Claude Opus 5)" Date: Fri, 21 Aug 2026 11:01:09 +0200 Subject: [PATCH] fix(tree): keep file reads inside the repository /api/tree/file/ and /api/tree/raw/ decodeURIComponent the path and joined it onto the repository root with no containment check. WHATWG URL parsing collapses a literal ../ before the handler sees it, but percent-encoded separators survive - and the UI itself sends encodeURIComponent(filePath), so %2F-encoded paths are the normal code path. Any file the user could read was readable. resolveInRepo resolves and then asserts containment, and both readers plus /api/open-in-editor go through it. Raw responses are also served inertly: only image types keep their content type, anything else is application/octet-stream with Content-Disposition: attachment, and every raw response carries nosniff and default-src 'none'; sandbox - a repository's own .html or .svg can no longer execute in this origin. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs --- packages/cli/src/server.ts | 39 ++++++++++++-- packages/git/src/index.ts | 2 +- packages/git/src/tree.ts | 23 ++++++-- packages/git/tests/resolve-in-repo.test.ts | 61 ++++++++++++++++++++++ 4 files changed, 115 insertions(+), 10 deletions(-) create mode 100644 packages/git/tests/resolve-in-repo.test.ts diff --git a/packages/cli/src/server.ts b/packages/cli/src/server.ts index e00476d..797f7c6 100644 --- a/packages/cli/src/server.ts +++ b/packages/cli/src/server.ts @@ -35,6 +35,7 @@ import { getTreeFingerprint, getWorkingTreeFileContent, getWorkingTreeRawFile, + resolveInRepo, WORKING_TREE_REFS, } from '@diffity/git'; import { @@ -72,6 +73,38 @@ const MIME_TYPES: Record = { '.pdf': 'application/pdf', }; +/** + * The tree browser only ever needs raw bytes for images. Anything else — a repository's own + * .html or .svg — would otherwise be rendered in this origin, where it can read the API. + */ +const INLINE_RAW_TYPES = new Set([ + '.png', + '.jpg', + '.jpeg', + '.gif', + '.webp', + '.avif', + '.ico', + '.svg', +]); + +function rawFileHeaders(ext: string): Record { + const headers: Record = { + 'X-Content-Type-Options': 'nosniff', + // An never runs an SVG's scripts, but navigating straight to the URL would. + 'Content-Security-Policy': "default-src 'none'; sandbox", + }; + + if (INLINE_RAW_TYPES.has(ext) && MIME_TYPES[ext]) { + headers['Content-Type'] = MIME_TYPES[ext]; + return headers; + } + + headers['Content-Type'] = 'application/octet-stream'; + headers['Content-Disposition'] = 'attachment'; + return headers; +} + export function getHost(): string { return process.env.DIFFITY_HOST?.trim() || 'localhost'; } @@ -286,7 +319,7 @@ export function startServer(options: ServerOptions): Promise { return; } const repoRoot = getRepoInfo().root; - const fullPath = filePath ? join(repoRoot, filePath) : repoRoot; + const fullPath = filePath ? resolveInRepo(filePath) : repoRoot; const gotoArg = line ? `${fullPath}:${line}` : fullPath; execFile('code', [repoRoot, '--goto', gotoArg], { timeout: 5000 }, () => {}); sendJson(res, { ok: true }); @@ -588,9 +621,7 @@ export function startServer(options: ServerOptions): Promise { ); try { const { data } = getWorkingTreeRawFile(filePath); - const ext = extname(filePath); - const mime = MIME_TYPES[ext] || 'application/octet-stream'; - res.writeHead(200, { 'Content-Type': mime }); + res.writeHead(200, rawFileHeaders(extname(filePath))); res.end(data); } catch { sendError(res, 404, `File not found: ${filePath}`); diff --git a/packages/git/src/index.ts b/packages/git/src/index.ts index 040f157..2031e21 100644 --- a/packages/git/src/index.ts +++ b/packages/git/src/index.ts @@ -5,5 +5,5 @@ export { getDiff, getDiffFiles, getDiffStat, getDiffStatForRef, getUntrackedFile export type { RefDiffArgs } from './diff.js'; export { getStagedFiles, getUnstagedFiles, isDirty } from './status.js'; export { getRecentCommits } from './commits.js'; -export { getTree, getTreeEntries, getTreeFingerprint, getWorkingTreeFileContent, getWorkingTreeRawFile } from './tree.js'; +export { getTree, getTreeEntries, getTreeFingerprint, getWorkingTreeFileContent, getWorkingTreeRawFile, resolveInRepo } from './tree.js'; export type { TreeEntry } from './tree.js'; diff --git a/packages/git/src/tree.ts b/packages/git/src/tree.ts index dec7055..6a1d3fe 100644 --- a/packages/git/src/tree.ts +++ b/packages/git/src/tree.ts @@ -1,5 +1,5 @@ import { readFileSync } from 'node:fs'; -import { join } from 'node:path'; +import { join, resolve, sep } from 'node:path'; import { execFileLarge } from './exec'; @@ -76,13 +76,26 @@ export function getTreeFingerprint(): string { return `${tracked.length}:${statOutput}`; } -export function getWorkingTreeFileContent(filePath: string): string { +/** + * Paths arrive from the URL, where `..` survives percent-encoding, so they cannot be joined + * onto the repository root and read as-is. + */ +export function resolveInRepo(filePath: string): string { const root = execFileLarge('git', ['rev-parse', '--show-toplevel']); - return readFileSync(join(root, filePath), 'utf-8'); + const fullPath = resolve(root, filePath); + + if (fullPath !== root && !fullPath.startsWith(root + sep)) { + throw new Error(`Path escapes the repository: ${filePath}`); + } + + return fullPath; +} + +export function getWorkingTreeFileContent(filePath: string): string { + return readFileSync(resolveInRepo(filePath), 'utf-8'); } export function getWorkingTreeRawFile(filePath: string): { data: Buffer; fullPath: string } { - const root = execFileLarge('git', ['rev-parse', '--show-toplevel']); - const fullPath = join(root, filePath); + const fullPath = resolveInRepo(filePath); return { data: readFileSync(fullPath), fullPath }; } diff --git a/packages/git/tests/resolve-in-repo.test.ts b/packages/git/tests/resolve-in-repo.test.ts new file mode 100644 index 0000000..0b4a6fa --- /dev/null +++ b/packages/git/tests/resolve-in-repo.test.ts @@ -0,0 +1,61 @@ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; + +let root: string; +let repoDir: string; +let origCwd: string; + +beforeAll(() => { + origCwd = process.cwd(); + root = mkdtempSync(join(tmpdir(), 'diffity-contain-')); + repoDir = join(root, 'repo'); + mkdirSync(repoDir); + + execFileSync('git', ['init', '-b', 'main', repoDir], { stdio: 'pipe' }); + writeFileSync(join(repoDir, 'inside.txt'), 'inside\n'); + writeFileSync(join(root, 'outside.txt'), 'outside\n'); + + process.chdir(repoDir); +}); + +afterAll(() => { + process.chdir(origCwd); + rmSync(root, { recursive: true, force: true }); +}); + +describe('resolveInRepo', () => { + it('resolves a path inside the repository', async () => { + const { resolveInRepo } = await import('../src/tree.js'); + + expect(resolveInRepo('inside.txt')).toBe(join(repoDir, 'inside.txt')); + }); + + it('rejects a traversal out of the repository', async () => { + const { resolveInRepo } = await import('../src/tree.js'); + + // What `decodeURIComponent('..%2f..%2fetc%2fpasswd')` hands the handler. + expect(() => resolveInRepo('../outside.txt')).toThrow(/escapes the repository/); + expect(() => resolveInRepo('../../../../etc/passwd')).toThrow(/escapes the repository/); + }); + + it('rejects an absolute path outside the repository', async () => { + const { resolveInRepo } = await import('../src/tree.js'); + + expect(() => resolveInRepo('/etc/passwd')).toThrow(/escapes the repository/); + }); + + it('rejects a sibling directory sharing the repository name as a prefix', async () => { + const { resolveInRepo } = await import('../src/tree.js'); + + expect(() => resolveInRepo('../repo-evil/secret.txt')).toThrow(/escapes the repository/); + }); + + it('refuses to read outside the repository through the file readers', async () => { + const { getWorkingTreeFileContent } = await import('../src/tree.js'); + + expect(() => getWorkingTreeFileContent('../outside.txt')).toThrow(/escapes the repository/); + }); +});