mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 16:34:00 +01:00
fix(sealed): compare landed patches byte for byte (latin1), so bytes that are not UTF-8 cannot collapse into a false match
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
0a68d03217
commit
f020d995b8
3 files changed
+23
-7
No files matched your search
@@ -5,4 +5,4 @@
|
||||
- **`gstack-pregate sealed`**: the `SEALED:` line gains `skipped=<n>` (and `commits=` counts the commits scanned), stderr names each skipped group, and `--no-skip` scans every listed commit.
|
||||
|
||||
### Tests
|
||||
- `test/sealed-skip.test.ts`: the #22 shape is clear with seven commits skipped as `refs/pull/6/head`, the hook and the preflight agree, `--no-skip` refuses, no fetched ref is left behind and the egress ledger records the lookup. The same parent never published refuses although its squash equals the combined diff. An unreachable remote scans everything and says so. A cherry-pick that landed on the base is skipped and named. A whitespace near-miss (`/home/alice/` against `/home/ alice/`) is scanned and refuses.
|
||||
- `test/sealed-skip.test.ts`: the #22 shape is clear with seven commits skipped as `refs/pull/6/head`, the hook and the preflight agree, `--no-skip` refuses, no fetched ref is left behind and the egress ledger records the lookup. The same parent never published refuses although its squash equals the combined diff. An unreachable remote scans everything and says so. A cherry-pick that landed on the base is skipped and named. A whitespace near-miss (`/home/alice/` against `/home/ alice/`) is scanned and refuses, and so is a landed commit that differs only in bytes that aren't valid UTF-8.
|
||||
+6
-6
@@ -82,8 +82,8 @@ export function sealedErrors(raw: unknown): Array<{ path: string; message: strin
|
||||
return errors;
|
||||
}
|
||||
|
||||
function git(repoRoot: string, args: string[], input?: string): { ok: boolean; out: string; err: string } {
|
||||
const r = spawnSync('git', args, { cwd: repoRoot, encoding: 'utf8', input, timeout: 120_000, maxBuffer: 256 * 1024 * 1024 });
|
||||
function git(repoRoot: string, args: string[], input?: string, encoding: BufferEncoding = 'utf8'): { ok: boolean; out: string; err: string } {
|
||||
const r = spawnSync('git', args, { cwd: repoRoot, encoding, input, timeout: 120_000, maxBuffer: 256 * 1024 * 1024 });
|
||||
return { ok: r.status === 0, out: r.stdout ?? '', err: (r.stderr ?? '').trim() };
|
||||
}
|
||||
|
||||
@@ -307,7 +307,7 @@ function publishedViaPulls(o: EvaluateOptions, listed: Set<string>, mbs: string[
|
||||
/** Diff text compared after a patch-id match: no commit line, blob ids or hunk line numbers. */
|
||||
const patchBody = (diff: string) => diff.split('\n').filter(l => !HEX.test(l) && !l.startsWith('index ')).map(l => l.replace(/^@@ -\d+(?:,\d+)? \+\d+(?:,\d+)? @@/, '@@')).join('\n');
|
||||
|
||||
/** Listed commits whose own patch (`--verbatim` patch-id, then equal diff text) is a base commit since the merge-base; merges never match. */
|
||||
/** Listed commits whose own patch (`--verbatim` patch-id, then equal diff text) is a base commit since the merge-base; merges never match. Diffs are read as latin1 so every byte counts: UTF-8 decoding folds invalid bytes into one replacement character. */
|
||||
function landedOnBase(o: EvaluateOptions, rest: string[], mbs: string[], notes: string[]): Map<string, string> {
|
||||
const out = new Map<string, string>();
|
||||
if (!rest.length) return out;
|
||||
@@ -318,15 +318,15 @@ function landedOnBase(o: EvaluateOptions, rest: string[], mbs: string[], notes:
|
||||
if (baseCommits.length > MAX_LANDED_BASE || rest.length > MAX_LANDED_LISTED) { notes.push(`${baseCommits.length} base and ${rest.length} listed commits since the merge-base, over ${MAX_LANDED_BASE} or ${MAX_LANDED_LISTED}: commits whose patch landed are scanned`); return out; }
|
||||
const diffArgs = ['diff-tree', '-r', '--root', '-p', '--no-color', '--no-ext-diff', '--no-textconv'];
|
||||
const ids = (list: string[]): string[][] => {
|
||||
const d = git(o.repoRoot, [...diffArgs, '--stdin'], list.join('\n') + '\n');
|
||||
const d = git(o.repoRoot, [...diffArgs, '--stdin'], list.join('\n') + '\n', 'latin1');
|
||||
if (!d.ok) throw new Error(`git diff-tree -p failed: ${d.err.slice(0, 200)}`);
|
||||
const p = git(o.repoRoot, ['patch-id', '--verbatim'], d.out);
|
||||
const p = git(o.repoRoot, ['patch-id', '--verbatim'], d.out, 'latin1');
|
||||
if (!p.ok) throw new Error(`git patch-id failed: ${p.err.slice(0, 200)}`);
|
||||
return p.out.split('\n').map(l => l.split(' ')).filter(f => f.length === 2);
|
||||
};
|
||||
const byId = new Map<string, string>();
|
||||
for (const [id, c] of ids(baseCommits)) if (!byId.has(id!)) byId.set(id!, c!);
|
||||
const body = (c: string) => { const r = git(o.repoRoot, [...diffArgs, c]); return r.ok && r.out.trim() ? patchBody(r.out) : null; };
|
||||
const body = (c: string) => { const r = git(o.repoRoot, [...diffArgs, c], undefined, 'latin1'); return r.ok && r.out.trim() ? patchBody(r.out) : null; };
|
||||
for (const [id, c] of ids(rest)) {
|
||||
const b = byId.get(id!);
|
||||
if (!b) continue;
|
||||
|
||||
@@ -155,4 +155,20 @@ describe('sealed skips what the destination already holds', () => {
|
||||
expect(h.err).toMatch(/sealed\.local_path\s+notes\.md:2/);
|
||||
expect(preflight(s).out).toMatch(/^SEALED: refused findings=1 commits=1 skipped=0 /m);
|
||||
});
|
||||
|
||||
test('a landed commit that differs only in bytes that are not UTF-8 is scanned and refuses', () => {
|
||||
const s = scratch();
|
||||
sh(s, ['checkout', '-q', '-b', 'results']);
|
||||
fs.writeFileSync(path.join(s.work, 'notes.md'), Buffer.from('intro\nraw at /home/alice/evals/raw\xe8.jsonl\n', 'latin1'));
|
||||
sh(s, ['add', 'notes.md']); sh(s, ['commit', '-q', '-m', 'notes']);
|
||||
sh(s, ['checkout', '-q', 'main']);
|
||||
fs.writeFileSync(path.join(s.work, 'notes.md'), Buffer.from('intro\nraw at /home/alice/evals/raw\xe9.jsonl\n', 'latin1'));
|
||||
sh(s, ['add', 'notes.md']); sh(s, ['commit', '-q', '-m', 'notes']);
|
||||
sh(s, ['push', '-q', 'origin', 'main']);
|
||||
sh(s, ['checkout', '-q', 'results']);
|
||||
const h = hook(s);
|
||||
expect(h.code).toBe(1);
|
||||
expect(h.err).toMatch(/sealed\.local_path\s+notes\.md:2/);
|
||||
expect(preflight(s).out).toMatch(/^SEALED: refused findings=1 commits=1 skipped=0 /m);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user