Skip to content

Commit c88b510

Browse files
committed
Reject borrowed Git object stores and verify rename evidence
1 parent 9020224 commit c88b510

4 files changed

Lines changed: 31 additions & 1 deletion

File tree

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ 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. Reads are bounded to 500 commits, 32 MiB per Git response, 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 are rejected. Reads are bounded to 500 commits, 32 MiB per Git response, and a 2-second budget per line diff; oversized work fails explicitly.
4747
- 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.
4848
- 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.
4949
- 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: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,12 @@ Declined the claim that repository aliases can override `rev-parse`, `rev-list`,
4141

4242
Fixed inherited Git environment redirection. A regression with `GIT_DIR` pointing at a second repository initially returned the foreign repository's content. The adapter now drops inherited `GIT_*` variables, ignores global/system Git configuration, and disables lazy fetch and transport access. The same regression now reads only the requested repository.
4343

44+
## Review rounds 3–4
45+
46+
Round 3 reviewed the pre-fix commit and repeated the environment finding; the existing fix resolved it. Round 4 exposed repository-local object alternates, reproduced by reading borrowed history from a second repository. The adapter now rejects an alternates file before resolving commits.
47+
48+
Declined adding rename-only ownership to later text edits. The approved design represents a no-content rename as a separate file-change segment. A real-Git test confirms that a foreign rename remains an Unplanned file card while the later P2 text edit belongs to P2; the rename has not disappeared from review. Text edits before a rename still retain their line ancestry. Scope tests now explicitly require nonempty P2 rows.
49+
4450
## Remaining gates
4551

4652
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: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import { execFileSync } from 'node:child_process';
2+
import { lstatSync } from 'node:fs';
3+
import { resolve as resolvePath } from 'node:path';
24
import type { FileDelta, FileVersion, History } from '../core/linking.ts';
35

46
/** Read-only Git adapter. Never follows working-tree symlinks or runs diff helpers. */
@@ -11,6 +13,8 @@ export function readHistory(repo: string, baseRef: string, headRef = 'HEAD'): Hi
1113
GIT_NO_LAZY_FETCH: '1', GIT_CONFIG_NOSYSTEM: '1', GIT_CONFIG_GLOBAL: '/dev/null' },
1214
stdio: ['ignore', 'pipe', 'pipe'],
1315
});
16+
const alternates = resolvePath(repo, run('rev-parse', '--git-path', 'objects/info/alternates').toString().trim());
17+
if (lstatSync(alternates, { throwIfNoEntry: false })) throw new Error('Review repositories must not use object alternates.');
1418
const resolve = (ref: string) => run('rev-parse', '--verify', '--end-of-options', `${ref}^{commit}`).toString().trim();
1519
const base = resolve(baseRef), head = resolve(headRef);
1620
const records = run('rev-list', '--reverse', '--parents', `${base}..${head}`).toString().trim().split('\n').filter(Boolean);

‎test/history.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ it('checks scope at each owning commit, not against both ends of a final rename'
142142
f.plan.items[1]!.files = [{ path: 'b.txt', kind: 'edit', renamed_from: null, change: 'Edit new name' }];
143143
renameSync(join(f.dir, 'a.txt'), join(f.dir, 'b.txt')); f.commit('P1');
144144
f.write('b.txt', 'ONE\ntwo\nthree\n'); f.commit('P2');
145+
expect(f.segments().some(s => s.row === 'P2')).toBe(true);
145146
expect(f.segments().filter(s => s.row === 'P2').every(s => s.scope === 'in-scope')).toBe(true);
146147
// Declaring only the old name must not authorize edits to the new one.
147148
f.plan.items[1]!.files[0]!.path = 'a.txt';
@@ -152,6 +153,7 @@ it('checks scope of deletion after a rename using the deleted current path', ()
152153
f.plan.items[1]!.files = [{ path: 'b.txt', kind: 'delete', renamed_from: null, change: 'Delete' }];
153154
renameSync(join(f.dir, 'a.txt'), join(f.dir, 'b.txt')); f.commit('P1');
154155
rmSync(join(f.dir, 'b.txt')); f.commit('P2');
156+
expect(f.segments().some(s => s.row === 'P2')).toBe(true);
155157
expect(f.segments().filter(s => s.row === 'P2').every(s => s.scope === 'in-scope')).toBe(true);
156158
});
157159

@@ -173,3 +175,21 @@ it('isolates repository selection from inherited Git environment variables', ()
173175
if (previous === undefined) delete process.env.GIT_DIR; else process.env.GIT_DIR = previous;
174176
}
175177
});
178+
179+
it('rejects repository-local object alternates before reading borrowed history', () => {
180+
const source = fixture(); source.write('a.txt', 'borrowed content\n'); source.commit();
181+
const borrower = fixture();
182+
writeFileSync(join(borrower.dir, '.git/objects/info/alternates'), join(source.dir, '.git/objects') + '\n');
183+
expect(() => readHistory(borrower.dir, source.base, source.git('rev-parse', 'HEAD'))).toThrow(/alternates/i);
184+
});
185+
it('keeps a foreign rename on its file card while attributing later text edits', () => {
186+
const f = fixture();
187+
renameSync(join(f.dir, 'a.txt'), join(f.dir, 'b.txt')); f.commit();
188+
f.plan.items[1]!.files[0]!.path = 'b.txt';
189+
f.write('b.txt', 'ONE\ntwo\nthree\n'); f.commit('P2');
190+
const parts = f.segments();
191+
expect(parts.find(s => s.kind === 'file')?.row).toBe('Unplanned');
192+
const text = parts.filter(s => s.kind === 'text');
193+
expect(text.length).toBeGreaterThan(0);
194+
expect(text.every(s => s.row === 'P2' && s.scope === 'in-scope')).toBe(true);
195+
});

0 commit comments

Comments
 (0)