Skip to content

Commit 86fc293

Browse files
mchwangclaude
andcommitted
Read the trusted plan context with main's hardened Git invocation
main (#82, #83) replaced isolatedGitEnvironment with HARDENED_GIT_OPTIONS and hardenedGitEnvironment. planContext() now uses both. That also adds --no-replace-objects: before, a replace ref for the snapshot's base made ls-tree list the replacement's tree as the trusted base entries. A test replaces the base with the head commit and fails on the old invocation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 4e2e27a commit 86fc293

2 files changed

Lines changed: 23 additions & 3 deletions

File tree

‎runner/review.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { Store, type ReviewState, type SnippetReference } from './store.ts';
55
import type { PlanIdentity } from '../core/identity.ts';
66
import { readHistory } from '../git/history.ts';
77
import { execFileSync } from 'node:child_process';
8-
import { isolatedGitEnvironment } from '../scripts/git-environment.ts';
8+
import { HARDENED_GIT_OPTIONS, hardenedGitEnvironment } from '../scripts/git-environment.ts';
99
import type { BaseEntry, PlanContext } from '../core/plan.ts';
1010
import { linkHistory } from '../core/linking.ts';
1111
import { applyChoices, approvalStates, approveItem, choiceKeys, fingerprint, reviewedSegment, stable } from '../core/approvals.ts';
@@ -122,11 +122,13 @@ export class ReviewService {
122122
const normalized = pathIdentity.unicodeNormalization === 'NFC' ? path.normalize('NFC') : path;
123123
return pathIdentity.caseSensitive ? normalized : normalized.toLowerCase();
124124
};
125-
const listing = execFileSync('git', ['-c', 'core.hooksPath=/dev/null', 'ls-tree', '-rz', snapshot.base], { cwd: repository, env: isolatedGitEnvironment(), encoding: 'utf8', maxBuffer: 64 * 1024 * 1024 });
125+
// Hardened like every repository Git call (#82, #83): no replace objects, hooks, network or inherited environment.
126+
const git = (args: string[], maxBuffer?: number) => execFileSync('git', [...HARDENED_GIT_OPTIONS, ...args], { cwd: repository, env: hardenedGitEnvironment(), encoding: 'utf8', maxBuffer, stdio: ['ignore', 'pipe', 'pipe'] });
127+
const listing = git(['ls-tree', '-rz', snapshot.base], 64 * 1024 * 1024);
126128
const baseEntries: BaseEntry[] = listing.split('\0').filter(Boolean).map(record => {
127129
const split = record.indexOf('\t'), [mode, , oid] = record.slice(0, split).split(' '), path = record.slice(split + 1);
128130
if (mode === '160000') return { path, kind: 'gitlink' };
129-
if (mode === '120000') return { path, kind: 'symlink', target: execFileSync('git', ['cat-file', 'blob', oid!], { cwd: repository, env: isolatedGitEnvironment(), encoding: 'utf8' }) };
131+
if (mode === '120000') return { path, kind: 'symlink', target: git(['cat-file', 'blob', oid!]) };
130132
return { path, kind: 'file' };
131133
});
132134
return { identity, issue: plan.issue, baseEntries, pathKey, allowedCommands: [] };

‎test/runner-planning-feedback.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import { afterEach, describe, expect, it, vi } from 'vitest';
99
import { createDemo } from '../scripts/demo.ts';
1010
import { startServer, type PlanningDeps } from '../web/server.ts';
1111
import { Store } from '../runner/store.ts';
12+
import { ReviewService } from '../runner/review.ts';
13+
import { fixtureGit } from './fixtures/git.ts';
1214
import type { AuthorProvider } from '../core/planning-author.ts';
1315
import type { EditReply } from '../core/plan.ts';
1416

@@ -95,6 +97,22 @@ describe('feedback from review actions', () => {
9597
});
9698
});
9799

100+
describe('trusted plan context', () => {
101+
it('reads the base tree itself, not a replacement object for it', () => {
102+
const root = mkdtempSync(join(tmpdir(), 'codeboost-f1e-')); roots.push(root);
103+
const config = createDemo(join(root, 'demo')), service = new ReviewService(config);
104+
try {
105+
const snapshot = service.store.getSnapshot(config.identity);
106+
const paths = () => service.planContext().baseEntries.map(entry => entry.path).sort();
107+
const expected = paths();
108+
expect(expected).not.toContain('debug.log');
109+
// A replace ref makes plain Git read the head commit (which adds debug.log) wherever the base is named.
110+
fixtureGit(config.repository, 'replace', snapshot.base, snapshot.head);
111+
expect(paths()).toEqual(expected);
112+
} finally { service.close(); }
113+
});
114+
});
115+
98116
describe('planning API for lane G', () => {
99117
const reply = (revision: number): EditReply => ({ schema_version: 1, base_revision: revision, reply: 'Suggestion', edits: [{ op: 'set_field',
100118
item: 'P1', summary: 'Rename', reason: 'Clearer', field: 'title', value: 'Clearer title', file: null, check: null, check_index: null, depends_on: null, new_item: null }] });

0 commit comments

Comments
 (0)