diff --git a/CHANGELOG.md b/CHANGELOG.md index 2dd3f6f2c..b6a4528ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +## [1.92.66.0] - 2026-11-02 + +**The secrets check reads Python and Dart string context from what git diffed, and a quote inside an interpolation no longer ends a string (GST-738).** env.kv's source-syntax exemption treats a `password:` line as code only when it sits outside every string. Two inputs could make the scanner place a committed string's contents outside a string. One was a working copy that no longer matched what git diffed. The other was a quote inside an f-string or Dart `${…}` expression. A scoped review pass now applies an explicit `--diff-base` to its Red Team as well. + +### Fixed +- **`lib/pregate/checks/secrets.ts`:** a changed Python or Dart file's whole text comes from a full-context `git diff`, never from the working copy. A `diff.suppressBlankEmpty` setting can't shift its lines. +- **`lib/redact-patterns.ts`:** a Python f-string's `{…}`, or t-string's, and a non-raw Dart string's `${…}` are read as code. Their nested strings count as strings, and a quote or comment inside them doesn't end the outer string. A Python format spec, such as `{n:#x}`, is text, and `\{` still opens a field. A `password:` line inside a Dart `/* */` comment is now read as text, not code. An expression left open, or nested more than 256 deep, runs to the end of the file, so the exemption fails closed. +- **`bin/gstack-review-dispatch`:** `pass --diff-base` now sets the scoped Red Team's base too. + ## [1.92.65.0] - 2026-11-02 - **Every /ship reviewer brief now starts with the `REVIEWED_TREE` step instead of saving it for the last paragraph** (`lib/review-brief.ts`, GST-897). Before, the brief only asked for the fingerprint line in its closing paragraph, after the whole diff and a rule that the reply ends with its `Recommendation:` line. On GST-850, two gpt-6.1-sol outside-voice replies never ran the command, so `collect` refused both (`COLLECT_WRONG_TREE reported="none"`) and their findings were lost. Other readers misread "the line prints" and pasted the command instead of its output. Now the brief opens with "First step, before anything else: run `cd && echo "REVIEWED_TREE: …"`", says the command outputs one `REVIEWED_TREE: ` line, and asks the reader to copy that output line, not the command, as the first line of its reply. The closing paragraph repeats the request with "(the output, not the command)". `collect` is unchanged and still refuses a reply without the line, and the dispatcher never stamps the tree for a reader. diff --git a/VERSION b/VERSION index b0f02decb..b0d5ebd0b 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.92.65.0 +1.92.66.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 686a682c7..01116c9ae 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.92.65.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.92.66.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/bin/gstack-review-dispatch b/bin/gstack-review-dispatch index 4a5b471e3..4697c43b9 100755 --- a/bin/gstack-review-dispatch +++ b/bin/gstack-review-dispatch @@ -65,6 +65,7 @@ const HELP = `gstack-review-dispatch — verbatim reviewer briefs and loss-free specialists (with the other --extras readers), red-team, voices (Step 11). Renders first and specialists now; prints the plan, MANIFEST: and one READER: line per reader. A plan whose NEXT is not full, scoped or fix-check prints PASS: none. A fix-check reader (stage specialists) renders from decision 3.5's --fix-list, --hunks, --tests and --callers files. + --diff-base sets every reader's base, a scoped pass's Red Team included (the targeted reader keeps the merge base). An existing manifest for the same --pass is reused (resumed). --extras: [{"name","prompt_file","first"?,"model"?}] --audit-only skips the plan: the --extras readers are the whole pass (Step 14.5's docs audit) show --manifest MANIFEST_JSON: @@ -193,7 +194,7 @@ function pass(): number { const extras = extrasFile ? JSON.parse(readFile(extrasFile, 'extras')!) as Extra[] : []; const m = buildManifest({ pass: id, out, ledger: flag('--ledger') ?? path.join(out, 'review-ledger.jsonl'), base, diffBase, root, checklistRoot: flag('--checklist-root') ?? ROOT, dispatches: step === 'fix-check' ? p.dispatches : p.dispatches.filter(d => d.role !== 'fix-check'), next: step, planLines, learnings: readFile(flag('--learnings-file'), 'learnings') ?? null, extras, - targetedClasses: p.targeted?.classes, mergeBase, rangeBase, + targetedClasses: p.targeted?.classes, mergeBase, rangeBase: flag('--diff-base') ? undefined : rangeBase, fixCheck: step === 'fix-check' && (flag('--fix-list') || flag('--hunks') || flag('--tests')) ? { fix_list: flag('--fix-list')!, hunks: flag('--hunks')!, tests: flag('--tests')!, callers: flag('--callers') } : undefined }); fs.mkdirSync(m.out, { recursive: true, mode: 0o700 }); const t = sectionTexts(ROOT); diff --git a/docs/pregate.md b/docs/pregate.md index e3b9393b6..c7238d475 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -195,7 +195,10 @@ Two stages with separate clocks, both printed: changed Python (`.py`, `.pyi`) or Dart file is scanned whole and only findings on its added lines count, so a line added inside an existing `"""` or `'''` string is still read as a literal; its rows give the file's - line number (`app.py, line 10`). + line number (`app.py, line 10`). A tracked file's whole text is the version + git diffed (a full-context `git diff`), never the working copy; an untracked + file is read whole from disk. A file over the size limit, or whose full diff + does not parse, is scanned on its added lines alone, without that context. - `strays`: a new file at the repository root, a new top-level directory, or a scratch name (`zz-*`, `tmp*`, `scratch*`) anywhere. - `literals`: for every changed exported constant or named timeout/retry @@ -939,8 +942,11 @@ those, each with its scope and its reason: (`session: HttpSession,`) and a dict entry or Dart named argument that reads a name (`"password": request.password`) are code, not credentials, as they are in TypeScript (#3630). Python's keyword form (`password=request.password`) - still reports. The same line inside a `"""` docstring is text and still - reports; allowlist it by its value. The pre-push hook sees only added lines, + still reports. The same line inside a `"""` docstring, any other string or + a Dart `/* */` comment is text and still reports; allowlist it by its value. + Code inside a Python f-string or t-string `{…}` field, or a non-raw Dart + string's `${…}`, is read as code, and its nested strings and quotes do not + end the outer string (GST-738). The pre-push hook sees only added lines, so it scans Python and Dart lines without this exemption and may still warn (MEDIUM, non-blocking) on them. diff --git a/docs/review-policy.md b/docs/review-policy.md index 41c088245..2cf6a453b 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -567,7 +567,8 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends: staged manifest (`/pass-manifest.json`): audit readers marked first, then the specialists with the other audit readers, then the Red Team, then Step 11's voices. A QA extra is refused before any reader is created: Step 9.2.1's probes - are the parent's. Each reviewer keeps its plan entry and is rendered by the same code as + are the parent's. `--diff-base` sets every reader's base, a scoped pass's Red + Team included; the targeted reader keeps the merge base. Each reviewer keeps its plan entry and is rendered by the same code as `render`, once its inputs exist; the Red Team's findings file is the specialists' `merged.jsonl` lines byte for byte. A brief leaves the bin in indexed chunks of at most 65,536 characters (`chunk`), so no tool output line carries a whole brief and a diff --git a/lib/pregate/checks/secrets.ts b/lib/pregate/checks/secrets.ts index 934def71c..17572c1f4 100644 --- a/lib/pregate/checks/secrets.ts +++ b/lib/pregate/checks/secrets.ts @@ -49,6 +49,26 @@ export function parseAddedLineNumbers(diff: string): Map { return files; } +/** Each file's whole postimage from a diff with full context: one hunk starting at line 1, its context and added lines; a file with any other shape is left out. */ +export function parsePostimages(diff: string): Map { + const files = new Map(); + let file = ''; + let lines: string[] | null = null; + for (const line of diff.split('\n')) { + if (line.startsWith('diff --')) { file = ''; lines = null; continue; } + if (line.startsWith('@@')) { + const first: boolean = lines === null && file !== '' && /^@@ -\d+(?:,\d+)? \+1(?:,\d+)? @@/.test(line); + files.delete(file); + lines = first ? [] : null; + if (lines) files.set(file, lines); else file = ''; + continue; + } + if (lines === null) { if (line.startsWith('+++ ')) file = line.startsWith('+++ b/') ? line.slice(6) : ''; continue; } + if (line.startsWith('+') || line.startsWith(' ')) lines.push(line.slice(1)); + } + return files; +} + /** Files whose source-syntax exemption needs the whole file: a line added inside an existing `'''`/`"""` block is a literal (#3630). */ const LEXICAL_CONTEXT_SOURCE = /\.(?:dart|pyi?)$/i; const WHOLE_FILE_MAX_BYTES = 4 * 1024 * 1024; @@ -58,10 +78,11 @@ const WHOLE_FILE_MAX_BYTES = 4 * 1024 * 1024; * untracked files, grouped by file so gstack-redact can use each file's type * (#3211). A tracked Dart or Python file is passed whole with `keep`, its * added line numbers, so the scanner sees the strings an added line sits in - * and only findings on those lines count (#3630); an untracked one keeps every - * line. One too large, unreadable, not a regular file, or whose working copy - * no longer holds the diff's added lines at their numbers - * to read whole is scanned under a neutral name, without that exemption. External diff and textconv + * and only findings on those lines count (#3630). The whole text is the + * postimage git diffed (a full-context diff), never the working copy, which + * assume-unchanged can hide from git (GST-738); an untracked file keeps every + * line. One too large, or whose full diff does not parse as one hunk holding + * its added lines, is scanned under a neutral name, without that exemption. External diff and textconv * drivers are off: either can empty or rewrite the diff, and an empty diff * reads as clean. */ @@ -70,16 +91,15 @@ export function addedLines(ctx: PregateContext): Array<{ file: string; text: str const files = parseAddedLines(diff.stdout ?? ''); const numbers = parseAddedLineNumbers(diff.stdout ?? ''); const whole = new Map(); - for (const [file, keep] of numbers) { - if (!LEXICAL_CONTEXT_SOURCE.test(file)) continue; - try { - const st = fs.lstatSync(path.join(ctx.repoRoot, file)); - if (!st.isFile() || st.size > WHOLE_FILE_MAX_BYTES) continue; - const text = fs.readFileSync(path.join(ctx.repoRoot, file), 'utf8'); - const lines = text.split('\n'); + const sources = [...numbers.keys()].filter(file => LEXICAL_CONTEXT_SOURCE.test(file)); + if (sources.length) { + const full = spawnSync('git', ['-c', 'diff.suppressBlankEmpty=false', 'diff', '--no-color', '--no-ext-diff', '--no-textconv', '--src-prefix=a/', '--dst-prefix=b/', '--unified=100000000', '--diff-filter=ACMR', ctx.mergeBase, '--', ...sources.map(file => `:(literal)${file}`)], { cwd: ctx.repoRoot, encoding: 'utf8', timeout: 60_000, maxBuffer: 256 * 1024 * 1024, env: ctx.env }); + for (const [file, post] of parsePostimages(full.status === 0 ? full.stdout ?? '' : '')) { + const keep = numbers.get(file) ?? []; const added = files.get(file) ?? []; - if (keep.length === added.length && keep.every((n, k) => lines[n - 1] === added[k])) whole.set(file, { text, keep }); - } catch { /* unreadable: its added lines are scanned alone */ } + const text = post.join('\n'); + if (text.length <= WHOLE_FILE_MAX_BYTES && keep.length === added.length && keep.every((n, k) => post[n - 1] === added[k])) whole.set(file, { text, keep }); + } } for (const t of ctx.touched) { if (!t.uncommitted || t.status === 'D') continue; diff --git a/lib/redact-patterns.ts b/lib/redact-patterns.ts index 66d837b71..1d7ef39b9 100644 --- a/lib/redact-patterns.ts +++ b/lib/redact-patterns.ts @@ -694,7 +694,8 @@ const CODE_NAME_CHAIN = /^[A-Za-z_$][\w$]*(?:\??\.[A-Za-z_$][\w$]*)*(?:<[\w$.,\s * text) records every string's content span: a `'''`/`"""` block, or a * `'`/`"` string that ends at its line unless a backslash continues it. * Comments are skipped (`#` in Python; `//` and nested `/* *\/` in Dart), so a - * delimiter inside one opens nothing; a backslash escapes the next character, except in a Dart raw + * delimiter inside one opens nothing, and a Dart block comment's own span is + * recorded too: a `password:` line in it is text, not code (GST-738); a backslash escapes the next character, except in a Dart raw * string (`r'''…'''`). The exemption needs the * whole file to see a block's opener: pregate passes Dart and Python files * whole, and the pre-push hook, which scans added lines alone, passes them @@ -708,34 +709,133 @@ export function clearStringSpanCache(): void { stringSpansCache = null; } -function stringSpans(text: string, lang: "py" | "dart"): Array<[number, number]> { +/** The index after the comment at `i`: a line comment ends at its newline, a Dart block comment at its matching `*\/` (they nest). */ +function commentEnd(text: string, i: number, lang: "py" | "dart"): number { + if (lang === "py" || !text.startsWith("/*", i)) { + const eol = text.indexOf("\n", i); + return eol < 0 ? text.length : eol; + } + let depth = 1; + let j = i + 2; + while (j < text.length && depth > 0) { + if (text.startsWith("/*", j)) { depth += 1; j += 2; } + else if (text.startsWith("*/", j)) { depth -= 1; j += 2; } + else j += 1; + } + return j; +} + +const MAX_INTERPOLATION_DEPTH = 256; + +type Spans = Array<[number, number]>; + +/** + * The index after a Dart `${…}` expression starting at `j`. Its code is not + * text; its strings and block comments add their own spans. + */ +function dartExpressionEnd(text: string, j: number, depth: number, spans: Spans): number { + if (depth > MAX_INTERPOLATION_DEPTH) return text.length; + for (let open = 1; j < text.length && open > 0; ) { + const d = text[j]!; + if (text.startsWith("//", j)) { j = commentEnd(text, j, "dart"); continue; } + if (text.startsWith("/*", j)) { const end = commentEnd(text, j, "dart"); spans.push([j, end]); j = end; continue; } + if (d === '"' || d === "'") { j = stringAt(text, j, "dart", depth, spans); continue; } + if (d === "{") open += 1; + else if (d === "}") open -= 1; + j += 1; + } + return Math.min(j, text.length); +} + +/** + * The index after a Python replacement field whose expression starts at `j`. + * The expression is code: its `#` comments are skipped and its strings add + * their own spans. A `:` or `!` outside its brackets starts the conversion and + * format spec, which is text apart from nested `{…}` fields (`{n:#x}`, `{n:'>10}`). + */ +function pythonFieldEnd(text: string, j: number, depth: number, spans: Spans): number { + const n = text.length; + if (depth > MAX_INTERPOLATION_DEPTH) return n; + let nest = 0; + while (j < n) { + const d = text[j]!; + if (d === "#") { j = commentEnd(text, j, "py"); continue; } + if (d === '"' || d === "'") { j = stringAt(text, j, "py", depth, spans); continue; } + if (d === "}" && nest === 0) return j + 1; + if (nest === 0 && (d === ":" || (d === "!" && text[j + 1] !== "="))) { + let from = j + 1; + for (j += 1; j < n && text[j] !== "}"; ) { + if (text[j] !== "{") { j += 1; continue; } + if (from < j) spans.push([from, j]); + j = pythonFieldEnd(text, j + 1, depth + 1, spans); + from = j; + } + if (from < Math.min(j, n)) spans.push([from, Math.min(j, n)]); + return Math.min(j + 1, n); + } + if ("([{".includes(d)) nest += 1; + else if (")]}".includes(d)) nest = Math.max(0, nest - 1); + j += 1; + } + return n; +} + +/** + * Adds the text spans of the string whose quote is at `i` and returns the index + * after its closing delimiter. An interpolated expression (`{…}` in a Python + * f- or t-string, `${…}` in a non-raw Dart string) is code: it is skipped + * whole, with its nested strings as their own spans, so a quote inside it never + * ends the outer string (GST-738). An expression left open, or nested past + * MAX_INTERPOLATION_DEPTH, is text to the end of the file. In a Python f-string + * `\{` still opens a field. + */ +function stringAt(text: string, i: number, lang: "py" | "dart", depth: number, spans: Spans): number { + const n = text.length; + const c = text[i]!; + const close = text.startsWith(c.repeat(3), i) ? c.repeat(3) : c; + const multiline = close.length === 3; + let k = i; + while (k > 0 && /[A-Za-z]/.test(text[k - 1]!)) k -= 1; + const prefix = k === 0 || !/[\w$]/.test(text[k - 1]!) ? text.slice(k, i) : ""; + const raw = lang === "dart" && prefix === "r"; + const interpolated = lang === "dart" ? !raw : /^(?:[rR]?[fFtT]|[fFtT][rR])$/.test(prefix); + let j = i + close.length; + let from = j; + while (j < n && !text.startsWith(close, j) && (multiline || text[j] !== "\n")) { + if (!raw && text[j] === "\\") { j += lang === "py" && interpolated && text[j + 1] === "{" ? 1 : 2; continue; } + if (!interpolated) { j += 1; continue; } + if (lang === "py" && text.startsWith("{{", j)) { j += 2; continue; } + if (lang === "py" ? text[j] !== "{" : !text.startsWith("${", j)) { j += 1; continue; } + if (from < j) spans.push([from, j]); + const field = j; + const mark = spans.length; + j = lang === "py" ? pythonFieldEnd(text, j + 1, depth + 1, spans) : dartExpressionEnd(text, j + 2, depth + 1, spans); + if (j >= n) { spans.length = mark; from = field; break; } + from = j; + } + const end = Math.min(j, n); + if (from < end) spans.push([from, end]); + return end + close.length; +} + +function stringSpans(text: string, lang: "py" | "dart"): Spans { if (stringSpansCache?.text === text && stringSpansCache.lang === lang) return stringSpansCache.spans; - const spans: Array<[number, number]> = []; + const spans: Spans = []; const n = text.length; for (let i = 0; i < n; ) { const c = text[i]!; if (lang === "py" ? c === "#" : text.startsWith("//", i)) { - const eol = text.indexOf("\n", i); - i = eol < 0 ? n : eol; + i = commentEnd(text, i, lang); continue; } if (lang === "dart" && text.startsWith("/*", i)) { - let depth = 1; - for (i += 2; i < n && depth > 0; ) { - if (text.startsWith("/*", i)) { depth += 1; i += 2; } - else if (text.startsWith("*/", i)) { depth -= 1; i += 2; } - else i += 1; - } + const end = commentEnd(text, i, lang); + spans.push([i, end]); + i = end; continue; } if (c !== '"' && c !== "'") { i += 1; continue; } - const close = text.startsWith(c.repeat(3), i) ? c.repeat(3) : c; - const multiline = close.length === 3; - const raw = lang === "dart" && text[i - 1] === "r" && !/[\w$]/.test(text[i - 2] ?? ""); - let j = i + close.length; - while (j < n && !text.startsWith(close, j) && (multiline || text[j] !== "\n")) j += !raw && text[j] === "\\" ? 2 : 1; - spans.push([i + close.length, Math.min(j, n)]); - i = j + close.length; + i = stringAt(text, i, lang, 0, spans); } stringSpansCache = { text, lang, spans }; return spans; diff --git a/package.json b/package.json index 4afa308b9..48bb0444d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.92.65", + "version": "1.92.66", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/test/pregate-secrets-source-context.test.ts b/test/pregate-secrets-source-context.test.ts index a46e1d178..27777514c 100644 --- a/test/pregate-secrets-source-context.test.ts +++ b/test/pregate-secrets-source-context.test.ts @@ -99,7 +99,7 @@ describe('pregate secrets: source context and diff integrity', () => { } finally { cleanup(fx); } }); - test('a Python file whose working copy no longer holds the diff\'s added lines is scanned by its added lines (#3630)', () => { + test('a Python file whose working copy no longer holds the diff\'s added lines is scanned as git diffed it (#3630, GST-738)', () => { const token = 'Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJx'; const fx: FixtureRepo = makeFixtureRepo({ pregate: BASE_PREGATE, baseFiles: { 'app.py': 'x = 1\n' } }); try { @@ -107,7 +107,31 @@ describe('pregate secrets: source context and diff integrity', () => { fx.git('update-index', '--assume-unchanged', 'app.py'); fs.writeFileSync(path.join(fx.work, 'app.py'), 'x = 1\ny = 2\n'); const check = JSON.parse(runBin(fx, 'gstack-pregate', ['--base', 'main', '--only', 'secrets', '--json']).out).checks.find((c: { id: string }) => c.id === 'secrets'); - expect({ status: check.status, kv: (check.lines as string[]).some((l) => l.includes('env.kv') && l.includes('app.py, added line 1')) }).toEqual({ status: 'fail', kv: true }); + expect({ status: check.status, kv: (check.lines as string[]).some((l) => l.includes('env.kv') && l.includes('app.py, line 2')) }).toEqual({ status: 'fail', kv: true }); + } finally { cleanup(fx); } + }); + + // Value: protects=the Python/Dart exemption reads the text git diffed, never a working copy that differs from it; fails_when=a committed credential inside a """ block passes because the working copy's other lines were edited under assume-unchanged; + // why_new=GST-738: the added lines matched, so the edited delimiters around them were trusted; seam=git diff; no_claim=a file edited after the scan + test('a committed credential in a Python string still fails when the working copy rewrites the lines around it (GST-738)', () => { + const token = 'Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJx'; + const fx: FixtureRepo = makeFixtureRepo({ pregate: BASE_PREGATE, baseFiles: { 'app.py': 'x = 1\nCONFIG = """\n"""\n' } }); + try { + commit(fx, { 'app.py': `x = 1\nCONFIG = """\npassword: ${token}\n"""\n` }); + fx.git('update-index', '--assume-unchanged', 'app.py'); + fs.writeFileSync(path.join(fx.work, 'app.py'), `x = 1\nCONFIG = 1 #\npassword: ${token}\n#\n`); + const check = JSON.parse(runBin(fx, 'gstack-pregate', ['--base', 'main', '--only', 'secrets', '--json']).out).checks.find((c: { id: string }) => c.id === 'secrets'); + expect({ status: check.status, kv: ((check.lines ?? []) as string[]).some((l) => l.includes('env.kv') && l.includes('app.py')) }).toEqual({ status: 'fail', kv: true }); + } finally { cleanup(fx); } + }); + + test('diff.suppressBlankEmpty does not shift the diffed Python text (GST-738)', () => { + const fx: FixtureRepo = makeFixtureRepo({ pregate: BASE_PREGATE, baseFiles: { 'app.py': 'x = 1\n\ny = 2\n' } }); + try { + commit(fx, { 'app.py': 'x = 1\n\ny = 2\nrows = {\n "password": session.password,\n}\n' }); + fx.git('config', 'diff.suppressBlankEmpty', 'true'); + const check = JSON.parse(runBin(fx, 'gstack-pregate', ['--base', 'main', '--only', 'secrets', '--json']).out).checks.find((c: { id: string }) => c.id === 'secrets'); + expect(check.status).toBe('pass'); } finally { cleanup(fx); } }); diff --git a/test/redact-engine.test.ts b/test/redact-engine.test.ts index b25ef1025..0bdef1556 100644 --- a/test/redact-engine.test.ts +++ b/test/redact-engine.test.ts @@ -347,6 +347,46 @@ describe("MEDIUM demoted credential-shaped patterns (TENSION-1)", () => { [` 'password: ${vIdent} more',`, "lib/a.dart"], [` "password: ${vIdent} more",`, "app.py"], ] as const) expect({ literal, file, kv: kv(literal, file) }).toEqual({ literal, file, kv: true }); }); + // Value: protects=an interpolated expression inside a Python f-string or Dart string is skipped whole, so a quote inside it never ends the outer string; fails_when=a """/''' delimiter in a nested string closes the block early and a password: line inside it reads as code; + // why_new=GST-738 (outside voice); seam=none; no_claim=Python before 3.12, where the nested quote is a syntax error + test("env.kv: a quote inside an f-string or Dart interpolation does not end the string (GST-738)", () => { + const kv = (text: string, sourcePath: string) => scan(text, { repoVisibility: "private", sourcePath, processEnv: {} }).findings.some((f) => f.id === "env.kv"); + const v = "Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJx"; + for (const [literal, file] of [ + [`CONFIG = f'''\n{"'''"}\npassword: ${v}\n'''\n`, "app.py"], + [`CONFIG = rf"""\n{x.replace('"""', "")}\npassword: ${v}\n"""\n`, "app.py"], + [`CONFIG = f'''\n{f"{'\'\'\''}"}\npassword: ${v}\n'''\n`, "app.py"], + [`const s = '''\${"'''"}\npassword: ${v}\n''';\n`, "lib/a.dart"], + [`const s = """\${m["k"] ?? '"""'}\npassword: ${v}\n""";\n`, "lib/a.dart"], + // A comment inside an interpolation holds no delimiter, and a Dart block comment is text. + [`CONFIG = f"""\n{1 # } """\n}\npassword: ${v}\n"""\n`, "app.py"], + [`const s = '''\${x // '''\n}\npassword: ${v}\n''';\n`, "lib/a.dart"], + [`void main() {}\n/*\npassword: ${v}\n*/\n`, "lib/a.dart"], + // A format spec is text: its # or quote opens nothing. + [`render({\n "id": f"{n:#x}",\n}, """\npassword: ${v}\n""")\n`, "app.py"], + [`render({\n "id": f"{n:'>10}",\n}, """\npassword: ${v}\n""")\n`, "app.py"], + [`render({\n "id": f"{n!r:{w}}",\n}, """\npassword: ${v}\n""")\n`, "app.py"], + // A # comment in a single-quoted f-string's multi-line field holds no delimiter either. + [`msg = f"{\n 1 # }" """\n}"\nCONFIG = """\npassword: ${v}\n"""\n`, "app.py"], + // In an f-string a backslash does not escape the brace after it. + [`CONFIG = f"""\\{"""\npassword: ${v}\n"""}"""\n`, "app.py"], + // Nesting past the depth cap runs to the end of the text instead of overflowing the stack. + ["const s = '" + "${'".repeat(20000) + `\npassword: ${v}\n`, "lib/a.dart"], + ["x = f'" + "{a:".repeat(100000) + `\npassword: ${v}\n`, "app.py"], + ] as const) expect({ literal, kv: kv(literal, file) }).toEqual({ literal, kv: true }); + for (const [code, file] of [ + ["msg = f'{a}:{b}'\ndef f(\n session: HttpSession,\n): pass\n", "app.py"], + ["msg = f'''{{literal brace'''\ndef f(\n session: HttpSession,\n): pass\n", "app.py"], + ["msg = f'{x:>{width}}'\ndef f(\n session: HttpSession,\n): pass\n", "app.py"], + ["msg = f\"{when:%b %d '%y} {d[':']} {a[1:2]} {a!=b}\"\nrows = {\n \"password\": session.password,\n}\n", "app.py"], + // Code inside a multi-line interpolation is code, not string text. + ["msg = f\"{\n {\n 'password': request.password,\n }\n}\"\n", "app.py"], + ["final s = \"${login(\n password: request.password,\n)}\";\n", "lib/a.dart"], + ["if'a' in s: pass\ndef f(\n session: HttpSession,\n): pass\n", "app.py"], + ["final s = 'hi ${user.name} and $id';\n login(password: request.password);\n", "lib/a.dart"], + ["final s = r'${raw}';\n login(password: request.password);\n", "lib/a.dart"], + ] as const) expect({ code, kv: kv(code, file) }).toEqual({ code, kv: false }); + }); test("env.kv's source-syntax check stays linear on a match-dense file (#3630)", () => { for (const file of ["app.py", "src/a.ts"]) { const text = "def f(\n" + " session: HttpSession,\n".repeat(12_000) + "): pass\n"; diff --git a/test/review-pass.test.ts b/test/review-pass.test.ts index ec2666aaf..dc367ddc0 100644 --- a/test/review-pass.test.ts +++ b/test/review-pass.test.ts @@ -841,7 +841,7 @@ describe('the Capy skill loader', () => { describe('a scoped pass after a blocker fix (GST-619)', () => { // Value: protects=the pass command accepts the scoped plan gstack-review-plan writes after a blocking fix, invoked as GEVA-2 ran it: the Red Team reviews the fix range, Step 11's voices still pending keep the whole branch, the fix-check is named outside the manifest for Step 3.5, and a resumed pass names it again; fails_when=pass refuses the scoped plan, stages the Red Team on the merge base, stages pending Step 11 voices on the range, drops the fix-check silently, or puts it in the manifest; // why_new=GEVA-82 on 1.91.379.0 hit BRIEF_RENDER_FAILED on every scoped pass and rendered briefs by hand; seam=none; no_claim=the fix-check's own brief - function scoped(flags: string[]) { + function scoped(flags: string[], explicitMergeBase = false) { const { dir, env } = repo(); const sh = (c: string) => { const r = spawnSync('bash', ['-c', c], { cwd: dir, env, encoding: 'utf8', timeout: 60_000 }); if (r.status !== 0) throw new Error(r.stderr); return r.stdout.trim(); }; const mergeBase = sh('git merge-base main HEAD'); @@ -852,7 +852,7 @@ describe('a scoped pass after a blocker fix (GST-619)', () => { fs.mkdirSync(path.join(dir, '.state'), { recursive: true }); fs.writeFileSync(path.join(dir, '.state/plan.md'), 'Check the plan items against the fix.\n'); fs.writeFileSync(extras, JSON.stringify([{ name: 'plan', prompt_file: path.join(dir, '.state/plan.md') }])); - const args = ['pass', '--base', 'main', '--out', out, '--pass', 'p2', '--extras', extras, '--', '--pass', '1', '--last', 'full', '--pass1-depth', 'standard', '--fixed-blocking', ...flags, '--range-base', rangeBase.slice(0, 7)]; + const args = ['pass', '--base', 'main', '--out', out, '--pass', 'p2', '--extras', extras, ...(explicitMergeBase ? ['--diff-base', mergeBase] : []), '--', '--pass', '1', '--last', 'full', '--pass1-depth', 'standard', '--fixed-blocking', ...flags, '--range-base', rangeBase.slice(0, 7)]; const p = bin(dir, env, args); expect(p.status, p.stdout + p.stderr).toBe(0); expect(p.stdout).toMatch(/^NEXT: scoped /m); @@ -893,6 +893,15 @@ describe('a scoped pass after a blocker fix (GST-619)', () => { expect(s.m.diff_base).toBe(s.rangeBase); expect(s.m.readers.map(r => [r.reader, r.diff_base ?? null])).toEqual([['red-team', s.rangeBase], ['adversarial-native', null], ['audit/plan', null]]); }, 120_000); + + // Value: protects=an explicit --diff-base reaches every reader, the Red Team included; fails_when=a scoped pass asked to read the whole branch still stages its Red Team on the fix range; why_new=GST-738 (outside voice); seam=none; no_claim=none + test('an explicit --diff-base widens the scoped Red Team too (GST-738)', () => { + const s = scoped([], true); + expect(s.m.diff_base).toBe(s.mergeBase); + const m = settle(s, 'specialists') && settle(s, 'red-team'); + const redTeam = fs.readFileSync(m.readers.find(r => r.reader === 'red-team')!.file!, 'utf8'); + expect({ diffBase: m.readers.find(r => r.reader === 'red-team')!.diff_base ?? null, reads: redTeam.includes(s.mergeBase) }).toEqual({ diffBase: null, reads: true }); + }, 120_000); }); describe('readers on a shared machine review the checkout the pass ships (GST-482)', () => {