v1.95.1.0 review-depth: executables stay code, sealed custody and egress are security, quoted names in slices (GST-1177, GSTA-258 L7) (#3972)

* review-depth: an exec bit or shebang keeps logs/ and .out paths code (GSTA-258 L7 Addendum 1, 7)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* review-brief: sliceDiff splits and names git-quoted and spaced filenames (GSTA-258 L7 Addendum 1.2)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* review-risk: sealed custody, redaction and outside-model egress code class as security (GST-1177)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs: review key row, review_depth receipt fields, executable and egress classing (GSTA-258 L7 Addendum 2, GST-1177)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* review pins: GSTA-258-L7 planner no-loss replay admits the new depth rules; changelog fragment

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: pre-landing review fixes (deleted paths are not executables, read two bytes of candidates only, wider egress line rules)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: Step 11 review fixes (executables and symlinks are code with their lines read whatever their docs, data or config name; never open a symlink)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: second review exception (docs credential check reads every docs path whatever its class; the live SVG reader never opens a symlink or non-regular file)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* changelog: L7 fragment matches the final change

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

---------

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
Garry Tanandcapy-ai[bot] authored and GitHub committed 2026-11-04 00:11:07 +00:00
1 parent 26a4e59165
commit 20645bce84
17 files changed
+203 -26

No files matched your search

+9 -2
View File
@@ -43,14 +43,21 @@ export class BriefError extends Error {}
*/
export function sliceDiff(text: string, cap: number): { text: string; sliced: boolean; files: number; kept: number; bytes: number; total: number; left: string[] } {
const total = Buffer.byteLength(text);
const starts = [...text.matchAll(/^diff --git a\/(\S+)/gm)];
const starts = [...text.matchAll(/^diff --git (.*)$/gm)];
const name = (rest: string) => {
const quoted = /^"a\/((?:[^"\\]|\\.)*)"/.exec(rest);
if (quoted) return `"${quoted[1]}"`;
const half = (rest.length - 1) / 2;
if (rest.startsWith('a/') && rest.slice(half) === ` b/${rest.slice(2, half)}`) return rest.slice(2, half);
return rest.startsWith('a/') ? rest.slice(2).split(' b/')[0]! : rest;
};
if (total <= cap || !starts.length) return { text, sliced: false, files: starts.length, kept: starts.length, bytes: total, total, left: [] };
let out = '', bytes = 0;
const left: string[] = [];
starts.forEach((m, k) => {
const section = text.slice(m.index!, k + 1 < starts.length ? starts[k + 1]!.index! : text.length);
const n = Buffer.byteLength(section);
if (bytes + n <= cap) { out += section; bytes += n; } else left.push(m[1]!);
if (bytes + n <= cap) { out += section; bytes += n; } else left.push(name(m[1]!));
});
return { text: out, sliced: true, files: starts.length, kept: starts.length - left.length, bytes, total, left };
}
+46 -6
View File
@@ -13,7 +13,7 @@ import * as path from 'node:path';
import { resolveBase, type ResolvedBase } from './gate-diff/git';
import { GENERATED_PATH, parseRegistry } from './regen';
import { RESULT_CODES } from './result-codes';
import { classifyPath, dataCandidate, svgCandidate, svgDrawing, type ClassInput, type DepthError, type PathLines, type RegistryInfo } from './review-depth';
import { BINARY_DATA_PATH, classifyPath, dataCandidate, svgCandidate, svgDrawing, type ClassInput, type DepthError, type PathLines, type RegistryInfo } from './review-depth';
export interface Collected extends ClassInput {
perPath: PathLines[];
@@ -32,6 +32,7 @@ export interface Collected extends ClassInput {
untracked: string[];
consumed: string[] | 'all';
drawings: string[];
executables: string[];
/** collectLive only: the ref resolveBase read for the base name (GST-1036), null when nothing resolved. */
base?: ResolvedBase | null;
}
@@ -232,9 +233,36 @@ function drawingsAmong(perPath: readonly PathLines[], classRoot: string | readon
}).sort();
}
function result(diff: ReturnType<typeof readDiff>, extra: Pick<Collected, 'root' | 'registry' | 'errors' | 'mergeBase' | 'untracked' | 'consumed'> & { drawings?: string[] }): Collected {
/**
* The changed paths a docs, data or run-log rule could move out of code that are executables: the exec bit, a symlink, or text
* that starts with "#!" (GSTA-258 L7). `isExec` answers for one path and fails closed (true) when it cannot read it.
*/
function executablesAmong(perPath: readonly PathLines[], classRoot: string | readonly string[], isExec: (e: PathLines) => boolean): string[] {
return perPath.filter(e => (BINARY_DATA_PATH.test(e.path) && classifyPath(e.path, classRoot) === 'code') || dataCandidate(e.path, classRoot) || svgCandidate(e.path, classRoot) || classifyPath(e.path, classRoot) === 'docs').filter(isExec).map(e => e.path).sort();
}
/**
* The executables among the changed paths at `headSha`: tree mode 100755 or 120000 (a symlink, as collectLive sees it), or a text
* blob whose first two bytes are "#!". A path the head tree lacks is a deletion, never an executable; an unreadable tree or blob counts as one.
*/
function executablesAt(root: string, headSha: string, perPath: readonly PathLines[], classRoot: readonly string[]): string[] {
const tree = git(root, ['ls-tree', '-r', '-z', headSha]);
const entries = new Map(tree.out.split('\0').flatMap(rec => { const m = /^(\d+) blob ([0-9a-f]+)\t(.*)$/s.exec(rec); return m ? [[m[3]!, { mode: m[1]!, sha: m[2]! }] as const] : []; }));
return executablesAmong(perPath, classRoot, e => {
if (!tree.ok) return true;
const t = entries.get(e.path);
if (!t) return false;
if (t.mode === '100755' || t.mode === '120000') return true;
if (e.binary) return false;
const r = spawnSync('git', ['cat-file', 'blob', t.sha], { cwd: root, timeout: 30_000, maxBuffer: 2 });
const read = r.status === 0 || (r.error as NodeJS.ErrnoException | undefined)?.code === 'ENOBUFS';
return !read || (r.stdout ?? Buffer.alloc(0)).subarray(0, 2).toString('latin1') === '#!';
});
}
function result(diff: ReturnType<typeof readDiff>, extra: Pick<Collected, 'root' | 'registry' | 'errors' | 'mergeBase' | 'untracked' | 'consumed'> & { drawings?: string[]; executables?: string[] }): Collected {
const paths = diff.perPath.map(e => e.path).sort();
return { ...diff, drawings: [], ...extra, paths, gateTagged: null, diffLines: diff.perPath.reduce((n, e) => n + e.lines, 0) };
return { ...diff, drawings: [], executables: [], ...extra, paths, gateTagged: null, diffLines: diff.perPath.reduce((n, e) => n + e.lines, 0) };
}
/** The range `baseSha`...`headSha` from git objects alone; the registry is the base commit's, never the head's. */
@@ -255,7 +283,7 @@ export function collectCommit(root: string, baseSha: string, headSha: string): C
const blob = spawnSync('git', ['cat-file', 'blob', `${headSha}:${p}`], { cwd: root, timeout: 60_000, maxBuffer: DRAWING_READ_CAP + 1 });
return blob.status === 0 && !blob.error ? blob.stdout : null;
});
return result(diff, { root: skillDirs, registry, errors, mergeBase, untracked: [], consumed, drawings });
return result(diff, { root: skillDirs, registry, errors, mergeBase, untracked: [], consumed, drawings, executables: executablesAt(root, headSha, diff.perPath, skillDirs) });
}
/**
@@ -289,7 +317,19 @@ export function collectLive(root: string, base: string): Collected {
}
const consumed = scanConsumers(root, [...diff.perPath.map(e => e.path), ...diff.deleted], [mergeBase], true, root, errors);
const drawings = drawingsAmong(diff.perPath, root, p => {
try { return fs.statSync(path.join(root, p)).size <= DRAWING_READ_CAP ? fs.readFileSync(path.join(root, p)) : null; } catch { return null; }
try { const st = fs.lstatSync(path.join(root, p)); return st.isFile() && st.size <= DRAWING_READ_CAP ? fs.readFileSync(path.join(root, p)) : null; } catch { return null; }
});
return { ...result(diff, { root, registry, errors, mergeBase, untracked, consumed, drawings }), base: resolved };
const executables = executablesAmong(diff.perPath, root, e => {
try {
const f = path.join(root, e.path);
const st = fs.lstatSync(f);
if (!st.isFile() || st.mode & 0o111) return true;
if (e.binary) return false;
const fd = fs.openSync(f, 'r');
const buf = Buffer.alloc(2);
try { fs.readSync(fd, buf, 0, 2, 0); } finally { fs.closeSync(fd); }
return buf.toString('latin1') === '#!';
} catch (err) { return (err as NodeJS.ErrnoException).code !== 'ENOENT'; }
});
return { ...result(diff, { root, registry, errors, mergeBase, untracked, consumed, drawings, executables }), base: resolved };
}
+7 -5
View File
@@ -237,6 +237,8 @@ export interface ClassInput {
created?: readonly string[];
/** The SVG candidates (svgCandidate) whose whole text the collector read as a drawing (svgDrawing); any other SVG stays code. */
drawings?: readonly string[];
/** Docs-, data- or log-named paths the collector found executable (exec bit, a symlink or a "#!" first line): code whatever their name or directory, never judged by path alone. */
executables?: readonly string[];
/** Added and deleted lines per path (lib/review-collect.ts): the risk classes read them; without them no line signal applies. */
added?: Record<string, readonly string[]>;
removed?: Record<string, readonly string[]>;
@@ -252,9 +254,9 @@ export interface ClassResult { class: DiffClass; reasons: string[]; errors: Dept
const sanitize = (s: string) => s.replace(/[\0-\x1f]+/g, ' ').slice(0, 200);
const isOutput = (i: ClassInput, p: string) => !!i.registry?.outputs.length && matchesAny(i.registry.outputs, p);
const unconsumedData = (i: ClassInput, p: string) => Array.isArray(i.consumed) && !i.consumed.includes(p) && (dataCandidate(p, i.root) || (!!i.drawings?.includes(p) && svgCandidate(p, i.root)));
const binaryData = (i: ClassInput, p: string) => BINARY_DATA_PATH.test(p) && classifyPath(p, i.root) === 'code' && ![...AGENT_PROSE, ...GATE_PATH, ...DATA_CONFIG].some(r => r.test(p));
const pathClass = (i: ClassInput, p: string): PathClass => (i.excluded?.has(p) || (i.stampOnly ?? []).includes(p) || unconsumedData(i, p) || binaryData(i, p) ? 'docs' : classifyPath(p, i.root));
const unconsumedData = (i: ClassInput, p: string) => !i.executables?.includes(p) && Array.isArray(i.consumed) && !i.consumed.includes(p) && (dataCandidate(p, i.root) || (!!i.drawings?.includes(p) && svgCandidate(p, i.root)));
const binaryData = (i: ClassInput, p: string) => !i.executables?.includes(p) && BINARY_DATA_PATH.test(p) && classifyPath(p, i.root) === 'code' && ![...AGENT_PROSE, ...GATE_PATH, ...DATA_CONFIG].some(r => r.test(p));
const pathClass = (i: ClassInput, p: string): PathClass => i.executables?.includes(p) && !i.excluded?.has(p) ? 'code' : (i.excluded?.has(p) || (i.stampOnly ?? []).includes(p) || unconsumedData(i, p) || binaryData(i, p) ? 'docs' : classifyPath(p, i.root));
/** The diff's class, raised to gate by any gate signal; `errors` lists every input that fails the depth closed; `runtimeLines` counts code-class lines outside the base registry's outputs. */
export function classifyDiff(i: ClassInput): ClassResult {
@@ -285,8 +287,8 @@ export function classifyDiff(i: ClassInput): ClassResult {
for (const p of i.paths) if (GATE_PATH.some(r => r.test(p))) { raise('gate', `gate path: ${p}`); break; }
if (i.registry?.error) errors.push(i.registry.error);
}
const riskHits = CLASS_RANK[cls] >= CLASS_RANK.code ? matchRisk({ codePaths: i.paths.filter(p => pathClass(i, p) === 'code'), added: i.added, removed: i.removed, scopeAuth: s.SCOPE_AUTH === 'true', ...(i.riskPaths?.length ? { isRiskPath: (p: string) => matchesAny(i.riskPaths!, p) && !DATA_PATH.test(p) } : {}) }) : new Map<RiskClass, string>();
const docSecret = i.paths.find(p => pathClass(i, p) === 'docs' && DOC_PATH.test(p) && (i.added?.[p] ?? []).some(l => DOC_SECRET_LINE.some(r => r.test(l))));
const riskHits = CLASS_RANK[cls] >= CLASS_RANK.code ? matchRisk({ codePaths: i.paths.filter(p => pathClass(i, p) === 'code'), executables: i.executables, added: i.added, removed: i.removed, scopeAuth: s.SCOPE_AUTH === 'true', ...(i.riskPaths?.length ? { isRiskPath: (p: string) => matchesAny(i.riskPaths!, p) && !DATA_PATH.test(p) } : {}) }) : new Map<RiskClass, string>();
const docSecret = i.paths.find(p => DOC_PATH.test(p) && (i.added?.[p] ?? []).some(l => DOC_SECRET_LINE.some(r => r.test(l))));
if (docSecret && !riskHits.has('security')) riskHits.set('security', `docs that can print a credential: ${docSecret}`);
const byRank = (a: RiskClass, b: RiskClass) => RISK_CLASSES.indexOf(a) - RISK_CLASSES.indexOf(b);
const detected = [...riskHits.keys()].sort(byRank);
+9 -3
View File
@@ -51,7 +51,8 @@ export const SECRET_LINE = [
/\b(?:keychain|keytar|find-generic-password|find-internet-password|secret-tool|git-credential|credential\.helper)\b/i, /\bgh auth token\b/, /(?:^|[\s'"/])\.netrc\b/,
];
const SECURITY_PATH_TOKENS = ['trust', 'redact', 'redaction', 'redactor', 'sanitize', 'sanitizer', 'neutralize', 'neutralizer', 'consent', 'escape'];
/** Sealed custody (harness-evals#237: scripts/sealed/seal.ts) and egress code are trust boundaries too (GST-1177). */
const SECURITY_PATH_TOKENS = ['trust', 'redact', 'redaction', 'redactor', 'sanitize', 'sanitizer', 'neutralize', 'neutralizer', 'consent', 'escape', 'seal', 'sealed', 'custody', 'egress'];
/** Coarse, linear-time rules (GST-850 B+): any changed line naming the child_process module, any outbound HTTP client call; quantifiers bounded. */
export const SECURITY_LINE = [
/\bshell\s*:\s*true\b/, /\bshell\s*=\s*True\b/, /(?<![.\w$])exec(?:Sync)?\s*\(/, /\b(?:child_process|childProcess|cp)\.exec(?:Sync)?\s*\(/, /\bos\.system\s*\(/,
@@ -67,6 +68,8 @@ export const SECURITY_LINE = [
/(?<![.\w])eval\s*\(/, /(?:^|[;&|])\s*eval\s+["'$]/, /\bnew\s+Function\s*\(/,
/\b(?:spawn|spawnSync|execFile|execFileSync)\s*\(\s*['"](?:\/bin\/)?(?:ba|z)?sh['"]/, /\bBun\.spawn(?:Sync)?\s*\(\s*\[\s*['"](?:\/bin\/)?(?:ba|z)?sh['"]/,
/\brmSync\s*\((?:.{0,500}?recursive\s*:\s*true|[^)]{501})/, /\brm\s*\((?:.{0,500}?recursive\s*:\s*true|[^)]{501})/, /\brm\s+-(?=[a-z]*r)(?=[a-z]*f)[a-z]*[rf]\b/,
/\bredact\w{0,40}\s{0,20}\(/, /(?:\b(?:from|import)\s{1,20}|\b(?:require|import)\s{0,20}\(\s{0,20})['"][^'"]{0,300}(?:redact|outside-voice)[^'"]{0,300}['"]/, /\b(?:from|import)\s{1,20}[\w.]{0,200}(?:redact|outside_voice)/,
/\b[A-Z0-9_]{0,60}(?:SECRET|CREDENTIAL)[A-Z0-9_]{0,60}\s{0,20}=\s{0,20}(?:\[\s{0,20})?(?:\/|new\s{1,20}RegExp\b)/,
/\b(?:innerHTML|outerHTML)\s*\+?=/, /\bdangerouslySetInnerHTML\b/, /\bv-html\b/, /\{@html\b/, /\binsertAdjacentHTML\s*\(/, /\bdocument\.write\s*\(/,
];
/** A recursive delete split across lines (a formatter puts `recursive: true` on its own line), read over a file's changed lines joined. */
@@ -131,6 +134,8 @@ export interface RiskInput {
removed?: Record<string, readonly string[]>;
/** gstack-diff-scope's SCOPE_AUTH: the auth class needs it and an auth token in the path. */
scopeAuth: boolean;
/** Executables (lib/review-collect.ts): never a config file or results script, so their lines are always read. */
executables?: readonly string[];
/** The base policy's review.risk_paths matcher: a code path it matches is risk class `policy`. */
isRiskPath?: (p: string) => boolean;
}
@@ -141,9 +146,10 @@ export function matchRisk(i: RiskInput): Map<RiskClass, string> {
const hit = (c: RiskClass, reason: string) => { if (!why.has(c)) why.set(c, reason); };
for (const p of i.codePaths) {
if (i.isRiskPath?.(p)) hit('policy', 'risk_paths: ' + p);
const stats = STATS_SCRIPT.test(p);
const exe = !!i.executables?.includes(p);
const stats = !exe && STATS_SCRIPT.test(p);
const t = stats ? [] : tokens(p);
const config = CONFIG_DATA_PATH.test(p);
const config = !exe && CONFIG_DATA_PATH.test(p);
const pathOnly = config || stats;
const prose = PROSE_PATH.test(p);
if (i.scopeAuth && t.some(x => AUTH_PATH_TOKENS.includes(x) || AUTH_PATH_PREFIXES.some(a => x.startsWith(a)))) hit('auth', 'auth path: ' + p);
+1 -1
View File
@@ -152,7 +152,7 @@ function merge(cwd: string, base: string, r: Report): boolean {
let runtime = 'unknown';
try {
const j = JSON.parse(plan.stdout);
const c = classifyDiff({ paths: j.paths, perPath: j.per_path, stampOnly: j.stamp_only, registry: j.registry, consumed: j.consumed, drawings: j.drawings, root: cwd, gateTagged: 0 });
const c = classifyDiff({ paths: j.paths, perPath: j.per_path, stampOnly: j.stamp_only, registry: j.registry, consumed: j.consumed, drawings: j.drawings, executables: j.executables, root: cwd, gateTagged: 0 });
runtime = String(c.runtimeLines ?? 'unknown');
} catch { /* unknown */ }
r.add(`RUNTIME_LINES: ${runtime} (gstack-ship-budget check --runtime-lines at the Step 9 and 12 boundaries)`);