Skip to content

Commit 8cf8320

Browse files
committed
Reject symlinked Git object storage before history reads
1 parent 7efeeba commit 8cf8320

4 files changed

Lines changed: 33 additions & 5 deletions

File tree

‎README.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ Inputs such as `planText` and the ledger must come from the caller. The future `
4343

4444
## Current limits and safety
4545

46-
- History must be linear and descend from the requested base. Merge histories are rejected with a rebase instruction; repositories using object alternates are rejected. Reads are bounded to 500 commits, 32 MiB per Git response, 64 MiB of unique blob bytes across the history (callers may lower `maxBlobBytes`), and a 2-second budget per line diff; oversized work fails explicitly.
46+
- History must be linear and descend from the requested base. Merge histories are rejected with a rebase instruction; repositories using object alternates or symlinked object storage are rejected (storage inspection is limited to 100,000 entries). Reads are bounded to 500 commits, 32 MiB per Git response, 64 MiB of unique blob bytes across the history (callers may lower `maxBlobBytes`), and a 2-second budget per line diff; oversized work fails explicitly.
47+
- Object storage must remain unchanged during a read; these library checks do not isolate a concurrently hostile filesystem.
4748
- Ownership uses line diffs, not semantic inference. Within one replacement block, new lines inherit all affected owners conservatively. Function context comes from Git hunk headers, not an AST.
4849
- The importer requires an accurate base-file list. It rejects path traversal, Git metadata paths, and traversal through a listed file/symlink/submodule. Runtime symlink and write-scope enforcement belong to the future container/runner; plan validation alone is not a sandbox.
4950
- Allowed commands restrict accidents, not hostile programs or changed scripts. Parsing returns argv and never executes it. An unlisted valid command is a warning and must not run until allowed.

‎docs/implementation/build-step-1.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ Declined adding rename-only ownership to later text edits. The approved design r
5151

5252
Fixed unbounded accumulation of unique blobs across a history. The adapter checks object size before loading it and enforces a cumulative 64 MiB byte budget; callers can choose a smaller positive limit. A small-budget fixture failed before the fix, then passed with explicit rejection below the required total and success at the exact total. Repeated references to the same blob do not count twice.
5353

54+
## Review round 6
55+
56+
Fixed symlinked object storage bypassing the alternates check. Root, loose-directory, and pack-directory symlink regressions all reproduced the problem. The adapter now inspects the object store without following symlinks, rejects links at any depth, and bounds inspection to 100,000 entries before resolving commits. The caller must keep storage stable during reads; concurrent filesystem isolation belongs to the runner.
57+
5458
## Remaining gates
5559

5660
This is a working foundation, not a completed application or a claim that all implementation tasks are done. T18's pure validation/edit core is present; its agent adapters, import UI, and persistence are pending. Ledger storage, rebase mappings, and the read-only review screen remain next. The already-fixed GitHub check belongs to the later GitHub/runner integration.

‎git/history.ts‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { execFileSync } from 'node:child_process';
2-
import { lstatSync } from 'node:fs';
3-
import { resolve as resolvePath } from 'node:path';
2+
import { lstatSync, readdirSync } from 'node:fs';
3+
import { resolve as resolvePath, join } from 'node:path';
44
import type { FileDelta, FileVersion, History } from '../core/linking.ts';
55

66
/** Read-only Git adapter. Never follows working-tree symlinks or runs diff helpers. */
@@ -16,7 +16,21 @@ export function readHistory(repo: string, baseRef: string, headRef = 'HEAD', lim
1616
GIT_NO_LAZY_FETCH: '1', GIT_CONFIG_NOSYSTEM: '1', GIT_CONFIG_GLOBAL: '/dev/null' },
1717
stdio: ['ignore', 'pipe', 'pipe'],
1818
});
19-
const alternates = resolvePath(repo, run('rev-parse', '--git-path', 'objects/info/alternates').toString().trim());
19+
// Inspect storage without following links before any object-resolving command.
20+
const objects = resolvePath(repo, run('rev-parse', '--git-path', 'objects').toString().trim());
21+
const pending = [objects];
22+
let inspected = 0;
23+
while (pending.length) {
24+
const path = pending.pop()!;
25+
if (++inspected > 100_000) throw new Error('Object storage inspection exceeds 100000 entries.');
26+
const stat = lstatSync(path);
27+
if (stat.isSymbolicLink()) throw new Error('Review repositories must not use symlinked object storage.');
28+
if (stat.isDirectory()) for (const entry of readdirSync(path)) {
29+
if (pending.length + inspected >= 100_000) throw new Error('Object storage inspection exceeds 100000 entries.');
30+
pending.push(join(path, entry));
31+
}
32+
}
33+
const alternates = join(objects, 'info/alternates');
2034
if (lstatSync(alternates, { throwIfNoEntry: false })) throw new Error('Review repositories must not use object alternates.');
2135
const resolve = (ref: string) => run('rev-parse', '--verify', '--end-of-options', `${ref}^{commit}`).toString().trim();
2236
const base = resolve(baseRef), head = resolve(headRef);

‎test/history.test.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { mkdtempSync, mkdirSync, writeFileSync, existsSync, rmSync, chmodSync, symlinkSync, renameSync } from 'node:fs';
1+
import { mkdtempSync, readdirSync, mkdirSync, writeFileSync, existsSync, rmSync, chmodSync, symlinkSync, renameSync } from 'node:fs';
22
import { tmpdir } from 'node:os';
33
import { join, dirname } from 'node:path';
44
import { execFileSync } from 'node:child_process';
@@ -199,3 +199,12 @@ it('fails explicitly when cumulative unique blob bytes exceed the history budget
199199
expect(() => readHistory(f.dir, f.base, 'HEAD', { maxBlobBytes: 20 })).toThrow(/blob byte budget/i);
200200
expect(readHistory(f.dir, f.base, 'HEAD', { maxBlobBytes: 27 }).final).toHaveLength(1);
201201
});
202+
203+
it.each(['root', 'loose', 'pack'])('rejects symlinked %s object storage', kind => {
204+
const f = fixture(); f.write('a.txt', 'changed\n'); f.commit('P1');
205+
const objects = join(f.dir, '.git/objects');
206+
const storage = kind === 'root' ? objects : join(objects, kind === 'pack' ? 'pack' : readdirSync(objects).find(name => /^[0-9a-f]{2}$/.test(name))!);
207+
const borrowed = join(f.dir, 'borrowed-objects');
208+
renameSync(storage, borrowed); symlinkSync(borrowed, storage);
209+
expect(() => readHistory(f.dir, f.base)).toThrow(/symlinked object storage/i);
210+
});

0 commit comments

Comments
 (0)