The git-diff viewer can now diff the current branch against a base commit-ish
(e.g. main) — review an agent's whole branch from your phone before merging, not
just uncommitted changes. Completes the long-deferred FR-B1.9.
- src/http/diff.ts: getDiff() gains an optional base. Three-layer defense so an
attacker-supplied base never reaches a shell or acts as a git option:
(1) isPlausibleRev() rejects leading '-', '..'/'...' ranges, metachars, control
chars, >250 chars; (2) git rev-parse --verify --quiet --end-of-options
<base>^{commit} — only a resolved 7-64 hex sha is accepted, else empty result;
(3) git diff --no-color <sha>... -- (three-dot = the branch's changes since
divergence, PR-style). execFile, no shell, timeout + maxBuffer bound.
- src/types.ts: additive optional base on DiffResult.
- src/server.ts GET /projects/diff reads ?base (400 on !isPlausibleRev); read-only.
- public/diff.ts: a "compare base" <select> (Working tree + one option per base),
disables Working/Staged tabs in base mode. public/projects.ts derives bases from
the worktree branches ∪ current branch.
Verified: typecheck + build:web clean, 1763 pass (diff tests 87 green). The 1 red
in the plain full run is the pre-existing real-PTY "ring buffer" flake (times out
at default 5s under sandbox load; 27/27 at --test-timeout=30000) — unrelated.
459 lines
16 KiB
TypeScript
459 lines
16 KiB
TypeScript
/**
|
||
* test/http/diff.test.ts (N-diff-be, B1) — read-only git diff parsing + getDiff.
|
||
*
|
||
* Two layers:
|
||
* 1. Pure parsers (parseNumstat / parseUnifiedDiff) fed canned git output — the
|
||
* deterministic core (AC-B1.5: rename/new/delete/binary/empty/untracked, +/-
|
||
* consistent with `git diff --numstat`; never throws; <script> stays text).
|
||
* 2. getDiff against a REAL throwaway git repo in os.tmpdir (AC-B1.1, L3).
|
||
*/
|
||
|
||
import { describe, it, expect, beforeAll, afterAll } from 'vitest'
|
||
import os from 'node:os'
|
||
import path from 'node:path'
|
||
import fs from 'node:fs/promises'
|
||
import { execFile } from 'node:child_process'
|
||
import { promisify } from 'node:util'
|
||
import {
|
||
parseNumstat,
|
||
parseUnifiedDiff,
|
||
getDiff,
|
||
isPlausibleRev,
|
||
type NumstatEntry,
|
||
type GetDiffOptions,
|
||
} from '../../src/http/diff.js'
|
||
|
||
const execFileAsync = promisify(execFile)
|
||
|
||
const LIMITS: GetDiffOptions['cfg'] = {
|
||
diffTimeoutMs: 5000,
|
||
diffMaxBytes: 2 * 1024 * 1024,
|
||
diffMaxFiles: 300,
|
||
}
|
||
|
||
// ── isPlausibleRev (FR-B1.9 syntactic allow-list) ────────────────────────────
|
||
|
||
describe('isPlausibleRev', () => {
|
||
it('accepts ordinary commit-ish forms', () => {
|
||
for (const ok of ['main', 'feature/x', 'HEAD~3', 'v1.2.0', 'main^', 'HEAD@{1}',
|
||
'0123456789abcdef0123456789abcdef01234567']) {
|
||
expect(isPlausibleRev(ok)).toBe(true)
|
||
}
|
||
})
|
||
|
||
it('rejects the empty string', () => {
|
||
expect(isPlausibleRev('')).toBe(false)
|
||
})
|
||
|
||
it('rejects an over-long revision (>250 chars)', () => {
|
||
expect(isPlausibleRev('a'.repeat(300))).toBe(false)
|
||
expect(isPlausibleRev('a'.repeat(250))).toBe(true) // boundary
|
||
})
|
||
|
||
it('rejects flag/option injection (leading hyphen)', () => {
|
||
expect(isPlausibleRev('-rf')).toBe(false)
|
||
expect(isPlausibleRev('--output=/etc/passwd')).toBe(false)
|
||
expect(isPlausibleRev('--upload-pack=touch /tmp/pwn')).toBe(false)
|
||
})
|
||
|
||
it('rejects range syntax (.. and ...)', () => {
|
||
expect(isPlausibleRev('a..b')).toBe(false)
|
||
expect(isPlausibleRev('a...b')).toBe(false)
|
||
})
|
||
|
||
it('rejects whitespace and shell metacharacters', () => {
|
||
for (const bad of ['x y', 'a\tb', '`id`', '$(x)', 'a;b', 'a|b', 'a&b', 'a>b', "a'b", 'a"b',
|
||
'main; rm -rf /', 'a\x00b']) {
|
||
expect(isPlausibleRev(bad)).toBe(false)
|
||
}
|
||
})
|
||
})
|
||
|
||
// ── parseNumstat ──────────────────────────────────────────────────────────────
|
||
|
||
describe('parseNumstat', () => {
|
||
it('parses a basic added/removed line', () => {
|
||
const m = parseNumstat('2\t1\tfoo.txt\n')
|
||
expect(m.get('foo.txt')).toEqual<NumstatEntry>({ added: 2, removed: 1, binary: false })
|
||
})
|
||
|
||
it('flags binary files (-\\t-) with zero counts', () => {
|
||
const m = parseNumstat('-\t-\timg.png\n')
|
||
expect(m.get('img.png')).toEqual<NumstatEntry>({ added: 0, removed: 0, binary: true })
|
||
})
|
||
|
||
it('keys a plain-arrow rename under both old and new paths', () => {
|
||
const m = parseNumstat('1\t1\told.txt => new.txt\n')
|
||
const entry = { added: 1, removed: 1, binary: false }
|
||
expect(m.get('old.txt')).toEqual(entry)
|
||
expect(m.get('new.txt')).toEqual(entry)
|
||
})
|
||
|
||
it('expands a braced rename path (common prefix/suffix)', () => {
|
||
const m = parseNumstat('3\t4\tsrc/{a => b}/file.js\n')
|
||
expect(m.has('src/a/file.js')).toBe(true)
|
||
expect(m.has('src/b/file.js')).toBe(true)
|
||
expect(m.get('src/b/file.js')).toEqual({ added: 3, removed: 4, binary: false })
|
||
})
|
||
|
||
it('collapses double slashes when a brace side is empty', () => {
|
||
const m = parseNumstat('1\t0\tsrc/{ => sub}/file.txt\n')
|
||
expect(m.has('src/file.txt')).toBe(true)
|
||
expect(m.has('src/sub/file.txt')).toBe(true)
|
||
})
|
||
|
||
it('returns an empty map for empty input', () => {
|
||
expect(parseNumstat('').size).toBe(0)
|
||
})
|
||
|
||
it('skips malformed lines without throwing', () => {
|
||
const m = parseNumstat('garbage\n2\t1\tok.txt\n\n')
|
||
expect(m.size).toBe(1)
|
||
expect(m.get('ok.txt')).toEqual({ added: 2, removed: 1, binary: false })
|
||
})
|
||
})
|
||
|
||
// ── parseUnifiedDiff ─────────────────────────────────────────────────────────
|
||
|
||
const MODIFIED = [
|
||
'diff --git a/foo.txt b/foo.txt',
|
||
'index e69de29..d95f3ad 100644',
|
||
'--- a/foo.txt',
|
||
'+++ b/foo.txt',
|
||
'@@ -1,2 +1,3 @@',
|
||
' keep',
|
||
'-old',
|
||
'+new',
|
||
'+added',
|
||
'',
|
||
].join('\n')
|
||
|
||
describe('parseUnifiedDiff', () => {
|
||
it('returns [] for empty / non-diff input', () => {
|
||
expect(parseUnifiedDiff('')).toEqual([])
|
||
expect(parseUnifiedDiff('not a diff at all')).toEqual([])
|
||
})
|
||
|
||
it('parses a modified file into one hunk with typed lines', () => {
|
||
const [file] = parseUnifiedDiff(MODIFIED)
|
||
expect(file).toBeDefined()
|
||
expect(file?.oldPath).toBe('foo.txt')
|
||
expect(file?.newPath).toBe('foo.txt')
|
||
expect(file?.status).toBe('modified')
|
||
expect(file?.binary).toBe(false)
|
||
expect(file?.hunks).toHaveLength(1)
|
||
expect(file?.hunks[0]?.header).toBe('@@ -1,2 +1,3 @@')
|
||
expect(file?.hunks[0]?.lines).toEqual([
|
||
{ kind: 'context', text: 'keep' },
|
||
{ kind: 'removed', text: 'old' },
|
||
{ kind: 'added', text: 'new' },
|
||
{ kind: 'added', text: 'added' },
|
||
])
|
||
})
|
||
|
||
it('counts +/- from the hunk when no numstat is supplied', () => {
|
||
const [file] = parseUnifiedDiff(MODIFIED)
|
||
expect(file?.added).toBe(2)
|
||
expect(file?.removed).toBe(1)
|
||
})
|
||
|
||
it('prefers numstat counts over hunk counts', () => {
|
||
const numstat = parseNumstat('99\t88\tfoo.txt\n')
|
||
const [file] = parseUnifiedDiff(MODIFIED, numstat)
|
||
expect(file?.added).toBe(99)
|
||
expect(file?.removed).toBe(88)
|
||
})
|
||
|
||
it('detects an added file via /dev/null source', () => {
|
||
const patch = [
|
||
'diff --git a/new.txt b/new.txt',
|
||
'new file mode 100644',
|
||
'index 0000000..1234567',
|
||
'--- /dev/null',
|
||
'+++ b/new.txt',
|
||
'@@ -0,0 +1,2 @@',
|
||
'+line one',
|
||
'+line two',
|
||
'',
|
||
].join('\n')
|
||
const [file] = parseUnifiedDiff(patch)
|
||
expect(file?.status).toBe('added')
|
||
expect(file?.newPath).toBe('new.txt')
|
||
expect(file?.added).toBe(2)
|
||
expect(file?.removed).toBe(0)
|
||
})
|
||
|
||
it('detects a deleted file via /dev/null target', () => {
|
||
const patch = [
|
||
'diff --git a/del.txt b/del.txt',
|
||
'deleted file mode 100644',
|
||
'index 1234567..0000000',
|
||
'--- a/del.txt',
|
||
'+++ /dev/null',
|
||
'@@ -1,2 +0,0 @@',
|
||
'-line one',
|
||
'-line two',
|
||
'',
|
||
].join('\n')
|
||
const [file] = parseUnifiedDiff(patch)
|
||
expect(file?.status).toBe('deleted')
|
||
expect(file?.oldPath).toBe('del.txt')
|
||
expect(file?.removed).toBe(2)
|
||
})
|
||
|
||
it('detects a rename and maps numstat by the new path', () => {
|
||
const patch = [
|
||
'diff --git a/old/name.txt b/new/name.txt',
|
||
'similarity index 95%',
|
||
'rename from old/name.txt',
|
||
'rename to new/name.txt',
|
||
'index 111..222 100644',
|
||
'--- a/old/name.txt',
|
||
'+++ b/new/name.txt',
|
||
'@@ -1 +1 @@',
|
||
'-hello',
|
||
'+hello world',
|
||
'',
|
||
].join('\n')
|
||
const numstat = parseNumstat('1\t1\t{old => new}/name.txt\n')
|
||
const [file] = parseUnifiedDiff(patch, numstat)
|
||
expect(file?.status).toBe('renamed')
|
||
expect(file?.oldPath).toBe('old/name.txt')
|
||
expect(file?.newPath).toBe('new/name.txt')
|
||
expect(file?.added).toBe(1)
|
||
expect(file?.removed).toBe(1)
|
||
})
|
||
|
||
it('detects a binary file (Binary files … differ + numstat -\\t-)', () => {
|
||
const patch = [
|
||
'diff --git a/img.png b/img.png',
|
||
'index 111..222 100644',
|
||
'Binary files a/img.png and b/img.png differ',
|
||
'',
|
||
].join('\n')
|
||
const numstat = parseNumstat('-\t-\timg.png\n')
|
||
const [file] = parseUnifiedDiff(patch, numstat)
|
||
expect(file?.status).toBe('binary')
|
||
expect(file?.binary).toBe(true)
|
||
expect(file?.hunks).toEqual([])
|
||
expect(file?.added).toBe(0)
|
||
expect(file?.removed).toBe(0)
|
||
})
|
||
|
||
it('parses multiple hunks and a no-newline meta marker', () => {
|
||
const patch = [
|
||
'diff --git a/m.txt b/m.txt',
|
||
'index 111..222 100644',
|
||
'--- a/m.txt',
|
||
'+++ b/m.txt',
|
||
'@@ -1,2 +1,2 @@',
|
||
' a',
|
||
'-b',
|
||
'+B',
|
||
'@@ -10,2 +10,3 @@',
|
||
' x',
|
||
'+y',
|
||
'\\ No newline at end of file',
|
||
'',
|
||
].join('\n')
|
||
const [file] = parseUnifiedDiff(patch)
|
||
expect(file?.hunks).toHaveLength(2)
|
||
const meta = file?.hunks[1]?.lines.find((l) => l.kind === 'meta')
|
||
expect(meta?.text).toContain('No newline')
|
||
})
|
||
|
||
it('classifies an unrecognised hunk line as context (never throws)', () => {
|
||
const patch = [
|
||
'diff --git a/x.txt b/x.txt',
|
||
'--- a/x.txt',
|
||
'+++ b/x.txt',
|
||
'@@ -1 +1 @@',
|
||
'?weird line',
|
||
'',
|
||
].join('\n')
|
||
const [file] = parseUnifiedDiff(patch)
|
||
expect(file?.hunks[0]?.lines[0]).toEqual({ kind: 'context', text: '?weird line' })
|
||
})
|
||
|
||
it('preserves <script>/ANSI/backtick content verbatim as text (AC-B1.4)', () => {
|
||
const payload = '<script>alert(1)</script> `cmd` [31mred'
|
||
const patch = [
|
||
'diff --git a/x.txt b/x.txt',
|
||
'--- a/x.txt',
|
||
'+++ b/x.txt',
|
||
'@@ -0,0 +1 @@',
|
||
'+' + payload,
|
||
'',
|
||
].join('\n')
|
||
const [file] = parseUnifiedDiff(patch)
|
||
expect(file?.hunks[0]?.lines[0]).toEqual({ kind: 'added', text: payload })
|
||
})
|
||
|
||
it('parses several files in one patch', () => {
|
||
const patch = MODIFIED + [
|
||
'diff --git a/bar.txt b/bar.txt',
|
||
'--- a/bar.txt',
|
||
'+++ b/bar.txt',
|
||
'@@ -1 +1 @@',
|
||
'-one',
|
||
'+two',
|
||
'',
|
||
].join('\n')
|
||
const files = parseUnifiedDiff(patch)
|
||
expect(files.map((f) => f.newPath)).toEqual(['foo.txt', 'bar.txt'])
|
||
})
|
||
})
|
||
|
||
// ── getDiff (integration against a real git repo) ────────────────────────────
|
||
|
||
async function git(cwd: string, ...args: string[]): Promise<void> {
|
||
await execFileAsync('git', args, { cwd })
|
||
}
|
||
|
||
describe('getDiff (real git repo)', () => {
|
||
let repo: string
|
||
|
||
beforeAll(async () => {
|
||
repo = await fs.mkdtemp(path.join(os.tmpdir(), 'webterm-diff-'))
|
||
await git(repo, 'init', '-q', '-b', 'main')
|
||
await git(repo, 'config', 'user.email', 'test@example.com')
|
||
await git(repo, 'config', 'user.name', 'Test')
|
||
await git(repo, 'config', 'commit.gpgsign', 'false')
|
||
await fs.writeFile(path.join(repo, 'tracked.txt'), 'one\ntwo\nthree\n')
|
||
await git(repo, 'add', '.')
|
||
await git(repo, 'commit', '-q', '-m', 'init')
|
||
})
|
||
|
||
afterAll(async () => {
|
||
await fs.rm(repo, { recursive: true, force: true })
|
||
})
|
||
|
||
it('returns structured working-tree diff with numstat-consistent counts', async () => {
|
||
await fs.writeFile(path.join(repo, 'tracked.txt'), 'one\nTWO\nthree\nfour\n')
|
||
const result = await getDiff(repo, { staged: false, cfg: LIMITS })
|
||
expect(result.staged).toBe(false)
|
||
expect(result.truncated).toBe(false)
|
||
const file = result.files.find((f) => f.newPath === 'tracked.txt')
|
||
expect(file).toBeDefined()
|
||
expect(file?.status).toBe('modified')
|
||
// changed line two + added line four = 2 added, 1 removed
|
||
expect(file?.added).toBe(2)
|
||
expect(file?.removed).toBe(1)
|
||
// revert for later tests
|
||
await fs.writeFile(path.join(repo, 'tracked.txt'), 'one\ntwo\nthree\n')
|
||
})
|
||
|
||
it('lists untracked files via porcelain (L3, working tree only)', async () => {
|
||
await fs.writeFile(path.join(repo, 'fresh.txt'), 'brand new\n')
|
||
const result = await getDiff(repo, { staged: false, cfg: LIMITS })
|
||
const untracked = result.files.find((f) => f.newPath === 'fresh.txt')
|
||
expect(untracked?.status).toBe('untracked')
|
||
await fs.rm(path.join(repo, 'fresh.txt'))
|
||
})
|
||
|
||
it('does not include untracked files in the staged diff', async () => {
|
||
await fs.writeFile(path.join(repo, 'staged-only.txt'), 'queued\n')
|
||
await git(repo, 'add', 'staged-only.txt')
|
||
const result = await getDiff(repo, { staged: true, cfg: LIMITS })
|
||
expect(result.staged).toBe(true)
|
||
const file = result.files.find((f) => f.newPath === 'staged-only.txt')
|
||
expect(file?.status).toBe('added')
|
||
expect(result.files.every((f) => f.status !== 'untracked')).toBe(true)
|
||
await git(repo, 'reset', '-q')
|
||
await fs.rm(path.join(repo, 'staged-only.txt'))
|
||
})
|
||
|
||
it('marks truncated when the file count exceeds diffMaxFiles', async () => {
|
||
for (let i = 0; i < 5; i++) {
|
||
await fs.writeFile(path.join(repo, `gen-${i}.txt`), `content ${i}\n`)
|
||
}
|
||
const result = await getDiff(repo, {
|
||
staged: false,
|
||
cfg: { ...LIMITS, diffMaxFiles: 2 },
|
||
})
|
||
expect(result.truncated).toBe(true)
|
||
expect(result.files.length).toBeLessThanOrEqual(2)
|
||
for (let i = 0; i < 5; i++) await fs.rm(path.join(repo, `gen-${i}.txt`))
|
||
})
|
||
|
||
it('never throws for a non-existent / non-git path (returns empty)', async () => {
|
||
const result = await getDiff(path.join(os.tmpdir(), 'definitely-not-here-xyz'), {
|
||
staged: false,
|
||
cfg: LIMITS,
|
||
})
|
||
expect(result.files).toEqual([])
|
||
expect(result.truncated).toBe(false)
|
||
})
|
||
})
|
||
|
||
// ── getDiff against a base revision (FR-B1.9, real git repo) ──────────────────
|
||
|
||
describe('getDiff against a base revision (FR-B1.9)', () => {
|
||
let repo: string
|
||
|
||
beforeAll(async () => {
|
||
repo = await fs.mkdtemp(path.join(os.tmpdir(), 'webterm-basediff-'))
|
||
await git(repo, 'init', '-q', '-b', 'main')
|
||
await git(repo, 'config', 'user.email', 'test@example.com')
|
||
await git(repo, 'config', 'user.name', 'Test')
|
||
await git(repo, 'config', 'commit.gpgsign', 'false')
|
||
await fs.writeFile(path.join(repo, 'app.txt'), 'base one\nbase two\n')
|
||
await git(repo, 'add', '.')
|
||
await git(repo, 'commit', '-q', '-m', 'base commit on main')
|
||
// Branch off and add a feature-only change (committed on `feature`, not main).
|
||
await git(repo, 'checkout', '-q', '-b', 'feature')
|
||
await fs.writeFile(path.join(repo, 'feature.txt'), 'brand new feature file\n')
|
||
await git(repo, 'add', '.')
|
||
await git(repo, 'commit', '-q', '-m', 'feature work')
|
||
})
|
||
|
||
afterAll(async () => {
|
||
await fs.rm(repo, { recursive: true, force: true })
|
||
})
|
||
|
||
it('diffs the whole branch against main (three-dot), echoing base', async () => {
|
||
const result = await getDiff(repo, { staged: false, base: 'main', cfg: LIMITS })
|
||
expect(result.base).toBe('main')
|
||
expect(result.staged).toBe(false)
|
||
expect(result.truncated).toBe(false)
|
||
const feat = result.files.find((f) => f.newPath === 'feature.txt')
|
||
expect(feat).toBeDefined()
|
||
expect(feat?.status).toBe('added')
|
||
// numstat-consistent counts for the committed feature file
|
||
expect(feat?.added).toBe(1)
|
||
expect(feat?.removed).toBe(0)
|
||
// A base diff never lists untracked entries.
|
||
expect(result.files.every((f) => f.status !== 'untracked')).toBe(true)
|
||
})
|
||
|
||
it('does NOT include working-tree-only untracked files in a base diff', async () => {
|
||
await fs.writeFile(path.join(repo, 'scratch.txt'), 'not committed\n')
|
||
const result = await getDiff(repo, { staged: false, base: 'main', cfg: LIMITS })
|
||
expect(result.files.find((f) => f.newPath === 'scratch.txt')).toBeUndefined()
|
||
await fs.rm(path.join(repo, 'scratch.txt'))
|
||
})
|
||
|
||
it('returns an empty diff when HEAD equals the base (no branch changes)', async () => {
|
||
const result = await getDiff(repo, { staged: false, base: 'feature', cfg: LIMITS })
|
||
expect(result.base).toBe('feature')
|
||
expect(result.files).toEqual([])
|
||
expect(result.truncated).toBe(false)
|
||
})
|
||
|
||
it('returns an empty result for an unknown/unresolvable base (rev-parse miss)', async () => {
|
||
const result = await getDiff(repo, { staged: false, base: 'no-such-branch', cfg: LIMITS })
|
||
expect(result.base).toBe('no-such-branch')
|
||
expect(result.files).toEqual([])
|
||
expect(result.truncated).toBe(false)
|
||
})
|
||
|
||
it('never runs a diff for an implausible base (defense-in-depth, no throw)', async () => {
|
||
// The route rejects this with 400 before getDiff; getDiff must still be safe.
|
||
for (const bad of ['-x', '--output=/tmp/x', 'main; rm -rf /', 'a..b']) {
|
||
const result = await getDiff(repo, { staged: false, base: bad, cfg: LIMITS })
|
||
expect(result.files).toEqual([])
|
||
expect(result.base).toBe(bad)
|
||
}
|
||
})
|
||
})
|