mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-09 18:28:15 +01:00
Fewer false privacy holds when posting PRs and running pre-merge checks (GST-2047) (#4198)
* Stop flagging test run numbers in PR descriptions as phone numbers (GST-2047) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Let approval import record autoplan run ids that look like phone or card numbers (GST-2047) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Read versions beside a VERSION file or a semver as versions, not public IPs (GST-2047) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Say pre-merge checks in the changelog entry (GST-2047) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Keep the phone-number check on run-id fields that hold other text and on run id names (GST-2047) Review-Finding: p1/red-team#1 reader=red-team Review-Finding: p1/specialist-batched#1 reader=security Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Exempt only 11 to 20 digit run numbers in a ship summary block from the phone-number check (GST-2047) Review-Finding: v1/adversarial-native#2 reader=adversarial-native 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
1 parent
434ead2183
commit
a8e36b9815
9 files changed
+283
-13
No files matched your search
@@ -0,0 +1,3 @@
|
||||
- **Fewer false privacy holds when posting PR descriptions and running the pre-merge checks.** Three kinds of machine-written numbers no longer read as personal data. GitHub Actions run numbers in the JSON summary /ship writes into a PR description (`ci_carried[].run`, `flaky_exception[].runs`, `issues.flakes[].run`) no longer count as phone numbers, so a post with reused CI no longer asks for `--confirm` once per run; only a block exactly as /ship's writer renders it qualifies, and the same number in prose, next to the block, under any other key, in one of those fields holding anything but an 11-to-20-digit run number, or inside a hand-written or broken block still needs confirmation. A four-part version written to or read from a `VERSION` file, or set right beside a three-part version (`path.join(dir, "VERSION"), "1.98.0.0\n"`, `v1.97.2 to 1.98.0.0`), and a `VERSION` file holding only the version, no longer count as public IP addresses; a public IP with no version beside it still reports.
|
||||
- **`gstack-approvals import` records plans whose autoplan run id looks like a phone or card number.** A run id such as `HARN-64-20261109177601` ends in a 14-digit time stamp, and the scanner could read it, alone or with the digits before it, as a phone or card number, so import refused the approval. Import now skips those two findings only in the time stamp (and the thread number right before it) where it places the run id itself, and only for an id of the minted `<name>-<14 digits>` shape. A phone number in the rest of the run id's name, in the same approval's words or plan title, or the same id typed into `gstack-approvals add`, still refuses.
|
||||
- Home-directory paths and other low-severity findings stay notes in both `gstack-post` and the pre-merge checks, now pinned by tests, and keys still refuse everywhere, inside /ship's summary block included.
|
||||
+23
-3
@@ -2041,7 +2041,13 @@ session, outside the lane's /ship run.
|
||||
|
||||
**Meaning.** A field matched gstack's secret scanner (the same one the push guard
|
||||
uses). The line names the field and the detector rule, never the matched text. Exit 3.
|
||||
Nothing was written, locally or to the Drive mirror.
|
||||
Nothing was written, locally or to the Drive mirror. `import --autoplan` skips a
|
||||
`pii.phone.e164` or `pii.cc` finding that starts inside the autoplan run id's
|
||||
14-digit stamp, or the thread-code digits right before it, where import places
|
||||
the id (the ask's `autoplan run <id>` and the source's `autoplan:<id>@`), when
|
||||
the id has the minted shape `<name>-<14 digits>` (GST-2047); a phone number in
|
||||
the rest of the directory name, the same id anywhere else, and every other
|
||||
finding in the row still refuse.
|
||||
|
||||
**Fix.** Record the words with the secret replaced by `[redacted]`, or ask the person to
|
||||
restate them. Rotate the secret if it was real.
|
||||
@@ -2963,7 +2969,12 @@ release of this repository (or the post ran outside the checkout).
|
||||
**Fix.** Write it as `version: 1.2.3.4`, `"version": "..."` or `v1.2.3.4`.
|
||||
A gstack release named right after `gstack` (`gstack 1.94.4.0`, the install
|
||||
check's `gstack: ok 1.94.4.0 for=ship`) is exempt too, but only for majors 0 to 2.
|
||||
MEDIUM findings do not block pushes.
|
||||
So is a quad a few quotes, brackets, arrows or spaces (or the word `to`) from a
|
||||
`VERSION` file name or path, or from a three-part version, on the same line
|
||||
(`path.join(dir, "VERSION"), "1.98.0.0\n"`, `echo 1.98.0.0 > VERSION`,
|
||||
`v1.97.2 to 1.98.0.0`), and a `VERSION` file that holds only the quad (GST-2047).
|
||||
Any other word between them, or another quad as the only neighbour, keeps the
|
||||
finding. MEDIUM findings do not block pushes.
|
||||
|
||||
<a id="redact-run-id-as-phone"></a>
|
||||
### `pii.phone.e164` MEDIUM on a receipt token or a GitHub Actions run id
|
||||
@@ -3006,7 +3017,16 @@ exact tokens it finds in these positions:
|
||||
accepted residual of the ruling that a run id is not a phone number;
|
||||
- every 11-digit id chained after an allowlisted one through `, `, ` and `,
|
||||
` & `, ` or ` or ` / `, optionally with a `(#1234)` between
|
||||
(`screen 40269992077 (#3434) and 40275443393 (#3453)`).
|
||||
(`screen 40269992077 (#3434) and 40275443393 (#3453)`);
|
||||
- a run id in a ship receipt's run-id fields (`ci_carried[].run`,
|
||||
`flaky_exception[].runs`, `issues.flakes[].run`) inside a
|
||||
`gstack-ship-receipt` block the receipt writer rendered: the block must
|
||||
close, pass the receipt schema and match `renderReceiptBlock`'s output byte
|
||||
for byte. Only those values count, and only when they are run ids (11 to 20
|
||||
digits, with an optional `/attempt`), so the same number in prose, beside
|
||||
the block, under any other key, a run-id field holding other text or a
|
||||
10-digit number, or a hand-written, schema-invalid or malformed block still
|
||||
reports (GST-2047).
|
||||
|
||||
A bare 11-digit number with none of that context, a 10-digit national number
|
||||
and an E.164 `+14155551234` still report, so the finding means the id stands
|
||||
|
||||
+21
-6
@@ -32,7 +32,7 @@ import { capyDriveFolder } from './autoplan-checkpoint';
|
||||
import { reviewedPlan } from './captain-brief';
|
||||
import { slugFromEnvironment } from './bin-context';
|
||||
import { firstTitle } from './markdown-sections';
|
||||
import { scan } from './redact-engine';
|
||||
import { scan, type Finding } from './redact-engine';
|
||||
import type { ResultCodeName } from './result-codes';
|
||||
import { resolveStateRoot } from './state-root';
|
||||
|
||||
@@ -151,8 +151,19 @@ function validDate(d: string): boolean {
|
||||
|
||||
export interface NewRow { ask: string; words: string; by: string; scope: string; pending?: string; outcome?: string; date?: string; source?: string }
|
||||
|
||||
/** Validate and secret-scan one row; the error names the field and rule, never the text. */
|
||||
export function buildRow(input: NewRow, branch: string, now = new Date()): ApprovalRow | RecordError {
|
||||
/** An autoplan run id as gstack-autoplan mints it: the run directory's name, a hyphen, then a 14-digit time stamp. */
|
||||
const MINTED_RUN_ID = /^[A-Za-z0-9._-]{1,64}-\d{14}$/;
|
||||
/** The part of a minted run id the exemption covers: the 14-digit stamp and, when the name ends in a thread code, its digits. */
|
||||
const RUN_ID_STAMP = /(?:(?<!\d)\d{1,6}-)?\d{14}$/;
|
||||
|
||||
/**
|
||||
* Validate and secret-scan one row; the error names the field and rule, never the text. `runId` (import only) is a
|
||||
* minted autoplan run id and the offset where the importer placed it in each field: pii.phone.e164 and pii.cc read its
|
||||
* stamp, alone or with the thread-code digits of the directory name before it, as a phone or card number (GST-2047), so
|
||||
* a finding of those two rules that starts inside that stamp part of the id, at that offset, is skipped. Anything else
|
||||
* in the field, the rest of the directory name included, and the same id typed anywhere else, is scanned as usual.
|
||||
*/
|
||||
export function buildRow(input: NewRow, branch: string, now = new Date(), runId?: { id: string; at: Partial<Record<'ask' | 'source', number>> }): ApprovalRow | RecordError {
|
||||
const fields = { ask: input.ask, words: input.words, by: input.by, scope: input.scope, pending: input.pending ?? 'none', source: input.source ?? 'manual' };
|
||||
for (const [k, v] of Object.entries(fields)) {
|
||||
if (typeof v !== 'string' || !v.trim()) return { code: 'APPROVAL_RECORD_INVALID', message: `--${k} is required` };
|
||||
@@ -165,7 +176,10 @@ export function buildRow(input: NewRow, branch: string, now = new Date()): Appro
|
||||
if (!validDate(date)) return { code: 'APPROVAL_RECORD_INVALID', message: `--date must be a real YYYY-MM-DD day` };
|
||||
for (const [k, v] of Object.entries(fields)) {
|
||||
const s = scan(v);
|
||||
const hits = s.findings.filter(f => f.severity === 'HIGH' || f.severity === 'MEDIUM');
|
||||
const at = runId && MINTED_RUN_ID.test(runId.id) ? runId.at[k as 'ask' | 'source'] : undefined;
|
||||
const stamp = at === undefined ? 0 : RUN_ID_STAMP.exec(runId!.id)![0].length;
|
||||
const inRunId = (f: Finding) => at !== undefined && (f.id === 'pii.phone.e164' || f.id === 'pii.cc') && f.line === 1 && f.col - 1 >= at + runId!.id.length - stamp && f.col - 1 < at + runId!.id.length;
|
||||
const hits = s.findings.filter(f => (f.severity === 'HIGH' || f.severity === 'MEDIUM') && !inRunId(f));
|
||||
if (s.oversize || hits.length) return { code: 'APPROVAL_RECORD_SECRET', message: `field ${k}: ${s.oversize ? 'oversize input' : [...new Set(hits.map(f => f.id))].join(', ')}` };
|
||||
}
|
||||
const id = 'a-' + createHash('sha256').update([fields.source, date, fields.ask, fields.words, fields.by].join('\u0000')).digest('hex').slice(0, 12);
|
||||
@@ -348,8 +362,9 @@ export function importAutoplan(runDir: string, branch: string, by?: string): App
|
||||
const checked = [...new Set(items.map(r => r.carry_check).filter((c): c is string => !!c))];
|
||||
const scope = [`the reviewed plan (sha256 ${planSha}), ${changed.length ? `${changed.length} recommendation(s) changed: ${changed.map(r => stripRef(r.title ?? '')).join('; ')}` : 'every recommendation taken'}`,
|
||||
unseen.length ? `taken under "do it", not seen by the owner: ${unseen.join('; ')}` : '', checked.length ? `post-cap delta checked by ${checked.join(', ')}` : ''].filter(Boolean).join('; ');
|
||||
const askHead = `Approve the plan "${planTitle}" (autoplan run `;
|
||||
const built = buildRow({
|
||||
ask: `Approve the plan "${planTitle}" (autoplan run ${runId}, ${carried ? `gate_rev ${rev} carried forward from gate_rev ${carried.carried_gate_rev}, ` : ''}${items.length + (p1 ? 1 : 0)} decisions)`.slice(0, CAPS.ask),
|
||||
ask: `${askHead}${runId}, ${carried ? `gate_rev ${rev} carried forward from gate_rev ${carried.carried_gate_rev}, ` : ''}${items.length + (p1 ? 1 : 0)} decisions)`.slice(0, CAPS.ask),
|
||||
words: first.owner_reply ?? first.answer_words ?? first.reply ?? '(no words recorded)',
|
||||
by: who.slice(0, CAPS.by),
|
||||
scope: scope.slice(0, CAPS.scope),
|
||||
@@ -357,7 +372,7 @@ export function importAutoplan(runDir: string, branch: string, by?: string): App
|
||||
outcome,
|
||||
date: first.answered_at!.slice(0, 10),
|
||||
source: `autoplan:${runId}@gate_rev=${rev}`,
|
||||
}, branch, new Date(first.answered_at!));
|
||||
}, branch, new Date(first.answered_at!), { id: runId, at: { ask: askHead.length, source: 'autoplan:'.length } });
|
||||
if ('code' in built) return built;
|
||||
out.push(first.words_by ? { ...built, words_by: first.words_by } : built);
|
||||
}
|
||||
|
||||
+50
-2
@@ -31,7 +31,8 @@ import { scan, type Finding, type RepoVisibility } from "./redact-engine";
|
||||
import { CI_RUN_WORD_BEFORE } from "./redact-patterns";
|
||||
import { agentNotesLines } from "./agent-notes";
|
||||
import { checkTitle, loadPlainTitlePolicy, PLAIN_TITLE_POLICY_PATH } from "./plain-title";
|
||||
import { RECEIPT_FENCE, RUNS_FENCE, extractReceipt, placeReceipt } from "./ship-receipt";
|
||||
import { RECEIPT_FENCE, RUNS_FENCE, extractReceipt, placeReceipt, renderReceiptBlock, validateReceipt } from "./ship-receipt";
|
||||
import type { ShipReceipt } from "./headless-artifacts";
|
||||
import { canonicalRemote } from "./remote-identity";
|
||||
import { parseChangelog } from "./changelog-check";
|
||||
import { versionWidth } from "./version-source";
|
||||
@@ -506,6 +507,49 @@ function receiptDigitTokens(texts: string[]): string[] {
|
||||
return [...tokens];
|
||||
}
|
||||
|
||||
/**
|
||||
* Where a ship receipt's run-id fields hold GitHub Actions run ids, which pii.phone.e164 reads as phone numbers
|
||||
* (GST-2047): the 1-based line and column span of each ci_carried[].run, flaky_exception[].runs[] and
|
||||
* issues.flakes[].run value that is a run id (11 to 20 digits, with an optional /attempt), bounded to that value, in
|
||||
* every closed ```gstack-ship-receipt``` block whose JSON passes the receipt schema and is byte-identical to
|
||||
* renderReceiptBlock's rendering of it. Only the receipt writer's own output qualifies, so a
|
||||
* hand-written, schema-invalid, malformed or unclosed block exempts nothing, and the same number in prose, beside the
|
||||
* block or under any other key still reports.
|
||||
*/
|
||||
function receiptRunIdSpans(text: string): Array<{ line: number; from: number; to: number }> {
|
||||
const lines = text.split("\n");
|
||||
const rows = (v: unknown): number => v !== null && typeof v === "object" && Object.keys(v).length ? 2 + Object.values(v).reduce((n: number, c) => n + rows(c), 0) : 1;
|
||||
const spans: Array<{ line: number; from: number; to: number }> = [];
|
||||
for (let open = 0; open < lines.length; open++) {
|
||||
if (lines[open] !== "```" + RECEIPT_FENCE) continue;
|
||||
const close = lines.indexOf("```", open + 1);
|
||||
if (close < 0) break;
|
||||
const raw = lines.slice(open + 1, close).join("\n");
|
||||
let receipt: ShipReceipt;
|
||||
try { receipt = JSON.parse(raw); } catch { continue; }
|
||||
if (validateReceipt(receipt).length || renderReceiptBlock(receipt) !== "```" + RECEIPT_FENCE + "\n" + raw + "\n```\n") continue;
|
||||
const span = (path: Array<string | number>) => {
|
||||
let at = open + 1;
|
||||
let v: any = receipt;
|
||||
for (const key of path) {
|
||||
at++;
|
||||
for (const [k, c] of Object.entries(v)) { if (k === String(key)) break; at += rows(c); }
|
||||
v = v[key];
|
||||
}
|
||||
if (!/^\d{11,20}(?:\/\d{1,4})?$/.test(typeof v === "number" ? String(v) : typeof v === "string" ? v : "")) return;
|
||||
const t = lines[at]!;
|
||||
const key = path.at(-1);
|
||||
const from = t.length - t.trimStart().length + (typeof key === "string" ? JSON.stringify(key).length + 2 : 0) + 1;
|
||||
spans.push({ line: at + 1, from, to: from + JSON.stringify(v).length - 1 });
|
||||
};
|
||||
receipt.ci_carried?.forEach((_, i) => span(["ci_carried", i, "run"]));
|
||||
receipt.flaky_exception?.forEach((e, i) => e.runs.forEach((_, j) => span(["flaky_exception", i, "runs", j])));
|
||||
receipt.issues?.flakes?.forEach((_, i) => span(["issues", "flakes", i, "run"]));
|
||||
open = close;
|
||||
}
|
||||
return spans;
|
||||
}
|
||||
|
||||
function scanParts(parts: Array<[Part, string]>, vis: RepoVisibility, selfEmail: string | undefined, releaseVersions: string[]) {
|
||||
const found: Located[] = [];
|
||||
let oversize = false;
|
||||
@@ -513,7 +557,11 @@ function scanParts(parts: Array<[Part, string]>, vis: RepoVisibility, selfEmail:
|
||||
for (const [part, text] of parts) {
|
||||
const result = scan(text, { repoVisibility: vis, allowlist, ...(selfEmail ? { selfEmail } : {}) });
|
||||
oversize ||= result.oversize;
|
||||
for (const finding of result.findings) found.push({ part, finding });
|
||||
const runIds = result.findings.some(f => f.id === "pii.phone.e164") ? receiptRunIdSpans(text) : [];
|
||||
for (const finding of result.findings) {
|
||||
if (finding.id === "pii.phone.e164" && runIds.some(s => s.line === finding.line && s.from <= finding.col && finding.col <= s.to)) continue;
|
||||
found.push({ part, finding });
|
||||
}
|
||||
}
|
||||
return { found, oversize };
|
||||
}
|
||||
|
||||
+24
-1
@@ -474,6 +474,29 @@ export function isDeclaredVersion(match: RegExpExecArray): boolean {
|
||||
return declaredAt(match.input ?? "", start, end, true);
|
||||
}
|
||||
|
||||
/**
|
||||
* A quad written to or read from a VERSION file, or set beside a semver (GST-2047): the file's name, alone or ending a
|
||||
* path, or a three-part version (`v1.97.2`, `1.98.0-rc.1`) within a short gap of the quad on the same line
|
||||
* (`path.join(dir, "VERSION"), "1.98.0.0\n"`, `echo 1.98.0.0 > VERSION`, `v1.97.2 to 1.98.0.0`). The gap holds only
|
||||
* quotes, brackets, commas, arrows, spaces, a `\n` escape or the word `to`, so a quad set apart by any other word
|
||||
* (`VERSION bumped; the DNS server 8.8.8.8`, `node 22.11.0 talks to 8.8.8.8`) still reports, and so does a quad
|
||||
* next to another quad. A file named VERSION that holds only the quad is one too; a VERSION file with any other line is
|
||||
* not a whole-file version (#2856), so each of its quads reports.
|
||||
*/
|
||||
const VERSION_GAP = String.raw`(?:[^A-Za-z0-9\n]|\\[nrt]|\bto\b){0,12}`;
|
||||
const VERSION_NEIGHBOUR = String.raw`(?:(?<![\w.-])(?:[\w.-]+\/)*VERSION(?![\w.-])|(?<![\w.])v?\d+\.\d+\.\d+(?:-[0-9A-Za-z.]+)?(?![\w.-]))`;
|
||||
const VERSION_NEIGHBOUR_BEFORE = new RegExp(VERSION_NEIGHBOUR + VERSION_GAP + "$");
|
||||
const VERSION_NEIGHBOUR_AFTER = new RegExp("^" + VERSION_GAP + VERSION_NEIGHBOUR);
|
||||
export function isVersionContext(match: RegExpExecArray, sourcePath?: string): boolean {
|
||||
const input = match.input ?? "";
|
||||
const { start, end } = spanBounds(match);
|
||||
const lineStart = input.lastIndexOf("\n", start - 1) + 1;
|
||||
const lineEnd = input.indexOf("\n", end);
|
||||
const line = input.slice(lineStart, lineEnd < 0 ? input.length : lineEnd);
|
||||
if (sourcePath && /(?:^|[\\/])VERSION$/.test(sourcePath) && input.trim() === input.slice(start, end)) return true;
|
||||
return VERSION_NEIGHBOUR_BEFORE.test(input.slice(Math.max(lineStart, start - 80), start)) || VERSION_NEIGHBOUR_AFTER.test(line.slice(end - lineStart, end - lineStart + 80));
|
||||
}
|
||||
|
||||
// ── Placeholder suppression (per-matched-span, NOT per-line) ─────────────────
|
||||
|
||||
/**
|
||||
@@ -1630,7 +1653,7 @@ export const PATTERNS: RedactPattern[] = [
|
||||
category: "pii",
|
||||
description: "Public IPv4 address",
|
||||
regex: /\b(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})\b/,
|
||||
validate: (span, match) => isPublicIPv4(span) && !isDeclaredVersion(match),
|
||||
validate: (span, match, opts) => isPublicIPv4(span) && !isDeclaredVersion(match) && !isVersionContext(match, opts?.sourcePath),
|
||||
},
|
||||
{
|
||||
id: "pii.wallet",
|
||||
|
||||
@@ -355,6 +355,40 @@ describe('import --autoplan', () => {
|
||||
expect(rows[0]!.source).toBe('autoplan:run-20261024131504@gate_rev=1');
|
||||
});
|
||||
|
||||
// Value: protects=an autoplan run id (<out dir name>-<14-digit stamp>) in the two places import puts it (the ask's "autoplan run <id>" and the
|
||||
// source's "autoplan:<id>@") never trips pii.phone.e164 or pii.cc, so a crew whose thread code ends in digits can record its gate (GST-2047,
|
||||
// HARN-1); fails_when=import refuses HARN-64-20261109177601 (phone) or GEVA-2-20261109173601 (Luhn), or the exemption reaches past those two
|
||||
// positions: a phone number in the same row's words or plan title, in the directory-name half of a minted id, a run id that is not the minted shape, or the same id typed into add
|
||||
// --ask; why_new=the Luhn-stamp test covers only a stamp the scanner already skips; seam=none; no_claim=other callers of the scanner
|
||||
test('a minted autoplan run id in its own fields imports; a phone number in the same row, or the id anywhere else, still refuses', () => {
|
||||
const importWith = (o: { runId: string; words?: string; title?: string }) => {
|
||||
const run = tmp('appr-run-');
|
||||
autoplanRun(run, [{ by: 'agent:GSTA-36', words: o.words ?? 'do it', at: '2026-11-09T17:36:01.000Z', p1: 'a' }]);
|
||||
fs.writeFileSync(path.join(run, 'run.json'), JSON.stringify({ run: o.runId, status: 'complete' }));
|
||||
if (o.title) fs.writeFileSync(path.join(run, 'plan-active.md'), `# ${o.title}\n\n## Implementation plan\n\n1. Fix it.\n`);
|
||||
return { run, rows: importAutoplan(run, 'b', 'Garry') };
|
||||
};
|
||||
for (const runId of ['HARN-64-20261109177601', 'GEVA-2-20261109173601']) {
|
||||
const { run, rows } = importWith({ runId });
|
||||
if ('code' in rows) throw new Error(`${runId}: ${rows.message}`);
|
||||
expect([rows[0]!.ask, rows[0]!.source]).toEqual([`Approve the plan "Widget fix" (autoplan run ${runId}, 2 decisions)`, `autoplan:${runId}@gate_rev=1`]);
|
||||
expect(bin(['import', '--autoplan', run, '--by', 'Garry'], { cwd: repo(), env: { GSTACK_STATE_ROOT: tmp('appr-state-') } }).out).toMatch(/^APPROVAL_IMPORTED: n=1 skipped=0 /);
|
||||
}
|
||||
const refused: Array<[{ runId: string; words?: string; title?: string }, string]> = [
|
||||
[{ runId: 'HARN-64-20261109177601', words: 'do it, call +14155552671 if it breaks' }, 'field words: pii.phone.e164'],
|
||||
[{ runId: 'HARN-64-20261109177601', words: 'do it, call 14155552671 if it breaks' }, 'field words: pii.phone.e164'],
|
||||
[{ runId: 'HARN-64-20261109177601', title: 'Call 14155552671 first' }, 'field ask: pii.phone.e164'],
|
||||
[{ runId: 'call-14155552671' }, 'field ask: pii.phone.e164'],
|
||||
[{ runId: '+14155552671-20261109173601' }, 'field ask: pii.phone.e164'],
|
||||
[{ runId: '14155552671-20261109173601' }, 'field source: pii.phone.e164'],
|
||||
[{ runId: 'call-415-555-2671-20261109173601' }, 'field ask: pii.phone.e164'],
|
||||
];
|
||||
for (const [o, message] of refused) expect([o, importWith(o).rows]).toEqual([o, { code: 'APPROVAL_RECORD_SECRET', message }]);
|
||||
const typed = bin(['add', '--ask', 'Approve the plan (autoplan run HARN-64-20261109177601, 2 decisions)', '--words', 'do it', '--by', 'Garry', '--scope', 'the plan'], { cwd: repo(), env: { GSTACK_STATE_ROOT: tmp('appr-state-') } });
|
||||
expect([typed.status, typed.out + typed.err]).toEqual([typed.status, expect.stringContaining('field ask: pii.phone.e164')]);
|
||||
expect(typed.status).not.toBe(0);
|
||||
});
|
||||
|
||||
test('an unanswered gate imports nothing and says why', () => {
|
||||
const run = tmp('appr-run-');
|
||||
fs.writeFileSync(path.join(run, 'decisions.jsonl'), JSON.stringify({ schema_version: 1, run: 'run-1', id: 'run-1-p1', label: 'P1', status: 'pending', gate_rev: 1 }) + '\n');
|
||||
|
||||
@@ -11,7 +11,8 @@ import * as fs from "node:fs";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import { EXIT, attributionCallout, mergeBodySections, runPost, type PostEnv } from "../lib/gstack-post";
|
||||
import { extractReceipt, receiptRuns } from "../lib/ship-receipt";
|
||||
import { extractReceipt, orderReceipt, placeReceipt, receiptRuns } from "../lib/ship-receipt";
|
||||
import type { ShipReceipt } from "../lib/headless-artifacts";
|
||||
|
||||
interface Call { cmd: string; args: string[]; input?: string }
|
||||
|
||||
@@ -293,6 +294,83 @@ describe("gstack-post owns the redaction scan", () => {
|
||||
}
|
||||
});
|
||||
|
||||
// Value: protects=a ship receipt the writer renders (lib/ship-receipt.ts placeReceipt) posts through pr-body without a pii.phone.e164 confirmation
|
||||
// when its run-id fields (ci_carried[].run, flaky_exception[].runs, issues.flakes[].run) hold GitHub Actions run ids (GST-2047), while a phone
|
||||
// number in prose, beside the block, in any other receipt field, in a run-id field holding something other than a run id, or inside a forged, hand-written or malformed receipt block still needs one;
|
||||
// fails_when=every ship tail with carried CI holds once per carried run, or the exemption reaches past the run-id values of a writer-rendered,
|
||||
// schema-valid block (a run id copied into prose, a non-run-id key, a fake block); why_new=the run-word and runs= tests read prose, and the
|
||||
// receipt's JSON puts a quote between the key and the id; seam=fake run; no_claim=real gh
|
||||
test("run ids in a writer-rendered receipt's run-id fields are not phone numbers; the same number anywhere else, or in a fake block, still is", () => {
|
||||
const CARRIED = [42574388503, 42574388504, 42574391177, 42574402011];
|
||||
const receipt = (extra: Partial<ShipReceipt> = {}) => orderReceipt({
|
||||
schema_version: 1, pr: 4200, head: "68b1d0560a3f4e2b9c7d1e5f8a2b3c4d5e6f7a8b", base: "main",
|
||||
ci_carried: CARRIED.map((run, i) => ({ lane: ["free-tests", "windows-smoke", "e2e-gate", "llm-judge"][i]!, run, attempt: i === 1 ? 2 : 1, tree: "9f2c4e1a7b3d5c8e0f6a2b4d6e8f0a1c3e5b7d9f" })),
|
||||
...extra,
|
||||
});
|
||||
const body = (r: ShipReceipt, prose = "Narrows one false positive.") => placeReceipt(`## What changed\n\n${prose}\n`, r);
|
||||
const post = (text: string) => {
|
||||
const f = fakeEnv({ files: { "/b": text }, visibility: "public" });
|
||||
return { code: run(["pr-body", "4200", "--replace", "--body-file", "/b"], f), phones: f.stdout().split("\n").filter(l => l.startsWith("RULE: pii.phone.e164 ")), f };
|
||||
};
|
||||
const lineOf = (text: string, needle: string) => text.split("\n").findIndex(l => l.includes(needle)) + 1;
|
||||
|
||||
const flaky = { flaky_exception: [{ lane: "free-tests", runs: ["42574388503/1", "42574388511/2"], cases: ["test/retry.test.ts::retries"], grant: "a-1", by: "Garry" }], issues: { fixes: [4100], refs: [], flakes: [{ issue: 4101, run: "42574388519" }] } };
|
||||
for (const text of [body(receipt()), body(receipt(flaky))]) {
|
||||
expect(extractReceipt(text)).toMatchObject({ found: true, errors: [] });
|
||||
const { code, phones, f } = post(text);
|
||||
expect([phones, code]).toEqual([[], EXIT.posted]);
|
||||
expect(f.posts()).toHaveLength(1);
|
||||
}
|
||||
|
||||
const holds: Array<[string, string, string]> = [
|
||||
["E.164 in prose", body(receipt(), "Call +14155552671 for the on-call."), "+14155552671"],
|
||||
["bare 11-digit in prose", body(receipt(), "Call 14155552671 for the on-call."), "14155552671"],
|
||||
["a carried run id copied into prose", body(receipt(), "Call 42574388503 after five."), "Call 42574388503"],
|
||||
["thread field", body(receipt({ thread: "14155552671" })), "\"thread\": \"14155552671\""],
|
||||
["ci_carried attempt", body(receipt({ ci_carried: [{ lane: "free-tests", run: 42574388503, attempt: 14155552671, tree: "9f2c4e1a7b3d" }] })), "\"attempt\": 14155552671"],
|
||||
["ci_carried lane", body(receipt({ ci_carried: [{ lane: "+14155552671", run: 42574388503, attempt: 1, tree: "9f2c4e1a7b3d" }] })), "\"lane\": \"+14155552671\""],
|
||||
["issues.flakes issue", body(receipt({ issues: { fixes: [], refs: [], flakes: [{ issue: 14155552671, run: "42574388519" }] } })), "\"issue\": 14155552671"],
|
||||
["a phone-shaped issues.flakes run", body(receipt({ issues: { fixes: [], refs: [], flakes: [{ issue: 4101, run: "call 415-555-0100" }] } })), "call 415-555-0100"],
|
||||
["a phone-shaped flaky_exception run", body(receipt({ flaky_exception: [{ lane: "free-tests", runs: ["call 415-555-0100"], cases: ["t"], grant: "a-1", by: "Garry" }] })), "call 415-555-0100"],
|
||||
["a 10-digit ci_carried run", body(receipt({ ci_carried: [{ lane: "free-tests", run: 4155552671, attempt: 1, tree: "9f2c4e1a7b3d" }] })), "\"run\": 4155552671"],
|
||||
["a 10-digit flaky_exception run", body(receipt({ flaky_exception: [{ lane: "free-tests", runs: ["4155552671"], cases: ["t"], grant: "a-1", by: "Garry" }] })), "\"4155552671\""],
|
||||
["a 10-digit issues.flakes run", body(receipt({ issues: { fixes: [], refs: [], flakes: [{ issue: 4101, run: "4155552671" }] } })), "\"run\": \"4155552671\""],
|
||||
["line before the block", body(receipt()).replace("```gstack-ship-receipt\n", "14155552671\n```gstack-ship-receipt\n"), "14155552671"],
|
||||
["line after the block", body(receipt()).replace(/\n```\n/, "\n```\n+14155552671\n"), "+14155552671"],
|
||||
];
|
||||
const forgedRun = (json: string) => "## What changed\n\nNarrows one false positive.\n\n```gstack-ship-receipt\n" + json + "\n```\n";
|
||||
const valid = receipt({ ci_carried: [{ lane: "free-tests", run: 14155552671, attempt: 1, tree: "9f2c4e1a7b3d" }] });
|
||||
holds.push(
|
||||
["schema-invalid block", forgedRun(JSON.stringify({ schema_version: 1, ci_carried: valid.ci_carried }, null, 2)), "14155552671"],
|
||||
["hand-written one-line block", forgedRun(JSON.stringify(valid)), "14155552671"],
|
||||
["malformed JSON block", forgedRun(JSON.stringify(valid, null, 2).replace(/\n}$/, ",\n}")), "14155552671"],
|
||||
["unclosed block", forgedRun(JSON.stringify(valid, null, 2)).replace(/\n```\n$/, "\n"), "14155552671"],
|
||||
);
|
||||
for (const [name, text, needle] of holds) {
|
||||
const { code, phones, f } = post(text);
|
||||
expect([name, phones, code]).toEqual([name, [`RULE: pii.phone.e164 LINE: ${lineOf(text, needle)} PART: body`], EXIT.confirm]);
|
||||
expect(f.posts()).toEqual([]);
|
||||
}
|
||||
});
|
||||
|
||||
// Value: protects=a LOW privacy finding (a home path) is a stderr note and never holds a post, while a credential still refuses even inside a
|
||||
// writer-rendered receipt block, whose exemption covers only its run ids (GST-2047); fails_when=a home path needs --confirm, or a key in a
|
||||
// receipt field posts or only asks for confirmation; why_new=no test pinned LOW as non-holding or a key inside a generated block; seam=fake
|
||||
// run; no_claim=real gh
|
||||
test("a LOW home path only notes; a key still refuses, inside a writer-rendered receipt too", () => {
|
||||
const low = fakeEnv({ files: { "/b": "logs are under /home/garry/work/run.log\n" }, visibility: "public" });
|
||||
expect(run(["pr-body", "12", "--replace", "--body-file", "/b"], low)).toBe(EXIT.posted);
|
||||
expect(low.stderr()).toContain("note: LOW RULE: internal.user_path LINE: 1 PART: body");
|
||||
const receipt = (extra: Partial<ShipReceipt>) => placeReceipt("## What changed\n\nOne fix.\n", orderReceipt({ schema_version: 1, pr: 12, head: "68b1d0560a3f4e2b9c7d1e5f8a2b3c4d5e6f7a8b", base: "main", ci_carried: [{ lane: "free-tests", run: 42574388503, attempt: 1, tree: "9f2c4e1a7b3d" }], ...extra }));
|
||||
for (const body of [receipt({ thread: AWS }), receipt({ flaky_exception: [{ lane: "free-tests", runs: [AWS], cases: ["t"], grant: "a-1", by: "Garry" }] })]) {
|
||||
expect(extractReceipt(body)).toMatchObject({ found: true, errors: [] });
|
||||
const f = fakeEnv({ files: { "/b": body }, visibility: "public" });
|
||||
expect(run(["pr-body", "12", "--replace", "--body-file", "/b"], f)).toBe(EXIT.refused);
|
||||
expect(f.stdout()).toMatch(/^HIGH RULE: aws\./m);
|
||||
expect(f.posts()).toEqual([]);
|
||||
}
|
||||
});
|
||||
|
||||
test("the title is scanned too, and the pusher's own email is not a finding", () => {
|
||||
const f = fakeEnv({ files: { "/t": "v1 fix for jane.roe@acme-corp.io", "/b": "body" } });
|
||||
expect(run(["pr-create", "--base", "main", "--title-file", "/t", "--body-file", "/b"], f)).toBe(EXIT.confirm);
|
||||
|
||||
@@ -75,6 +75,19 @@ describe('pregate secrets: source context and diff integrity', () => {
|
||||
expect(r.lines.join('\n')).toContain('app.py, line 5)');
|
||||
});
|
||||
|
||||
// Value: protects=pregate's secrets check reads a four-part version written to a VERSION file, or the file itself, as a version, and lists a
|
||||
// LOW privacy finding as a warning that never fails or warns the check, while a public IP with no version context still warns and a key
|
||||
// still fails (GST-2047); fails_when=gstack-evals' pv-sdk.ts VERSION write warns as pii.ip_public, a home path turns the check warn or fail,
|
||||
// or 8.8.8.8 and AKIA keys stop reporting; why_new=the engine tables test the scanner, not the check's status; seam=fixture repo; no_claim=CI
|
||||
test('a version beside a VERSION file passes and a home path only warns; a bare public IP still warns and a key still fails (GST-2047)', () => {
|
||||
expect(repo({ 'pv-sdk.ts': 'fs.writeFileSync(path.join(dir, "VERSION"), "1.98.0.0\\n");\n', VERSION: '1.98.0.0\n' }).secrets()).toMatchObject({ status: 'pass', lines: [] });
|
||||
const low = repo({ 'notes.md': 'logs are under /home/garry/work/run.log\n' }).secrets();
|
||||
expect([low.status, low.detail]).toEqual(['pass', expect.stringContaining('(1 WARN)')]);
|
||||
const ip = repo({ 'dns.ts': 'const resolver = "8.8.8.8";\n' }).secrets();
|
||||
expect([ip.status, ip.lines.join('\n')]).toEqual(['warn', expect.stringContaining('pii.ip_public')]);
|
||||
expect(repo({ 'notes.md': `logs are under /home/garry/work/run.log\nkey ${AWS}\n` }).secrets().status).toBe('fail');
|
||||
});
|
||||
|
||||
test('parseAddedLineNumbers numbers each added line from its hunk header', () => {
|
||||
const diff = ['diff --git a/a.py b/a.py', '--- a/a.py', '+++ b/a.py', '@@ -3,0 +4,2 @@', '+one', '+two', '@@ -9 +11 @@', '-old', '+new'].join('\n');
|
||||
expect([...parseAddedLineNumbers(diff)]).toEqual([['a.py', [4, 5, 11]]]);
|
||||
|
||||
@@ -308,6 +308,42 @@ describe("pii.ip_public: a four-part version needs a version declaration (#2784)
|
||||
);
|
||||
});
|
||||
|
||||
describe("pii.ip_public: a four-part version beside a VERSION file or a semver is not an address (GST-2047)", () => {
|
||||
table(
|
||||
"pii.ip_public",
|
||||
"MEDIUM",
|
||||
[
|
||||
'fs.writeFileSync(path.join(dir, "VERSION"), "1.98.0.0\\n");',
|
||||
"echo 1.98.0.0 > VERSION",
|
||||
"cat pkg/VERSION -> 1.98.0.0",
|
||||
"write('1.98.0.0', 'VERSION')",
|
||||
"bumps 1.98.0.0 (1.98.0 on npm)",
|
||||
"from v1.97.2 to 1.98.0.0",
|
||||
"1.98.0.0 / 1.98.0-rc.1",
|
||||
],
|
||||
[
|
||||
"resolver = 8.8.8.8",
|
||||
'fs.writeFileSync(path.join(dir, "HOSTS"), "8.8.8.8\\n");',
|
||||
"VERSION bumped; the DNS server 8.8.8.8 answered",
|
||||
"8.8.8.8 is in VERSION notes",
|
||||
"echo 8.8.8.8 > VERSIONS",
|
||||
"cat MY_VERSION_HOST 8.8.8.8",
|
||||
"node 22.11.0 talks to 8.8.8.8 over the egress proxy",
|
||||
"1.1.1.1, 8.8.8.8",
|
||||
],
|
||||
);
|
||||
|
||||
test("a VERSION file holding only a quad is a version; the same quad in any other file, or in a VERSION file with other lines, still reports", () => {
|
||||
const ids = (text: string, sourcePath: string) => scan(text, { repoVisibility: "private", sourcePath }).findings.map((f) => f.id);
|
||||
expect(ids("1.98.0.0\n", "VERSION")).not.toContain("pii.ip_public");
|
||||
expect(ids("1.98.0.0\n", "/repo/pkg/VERSION")).not.toContain("pii.ip_public");
|
||||
expect(ids("1.98.0.0\n", "HOSTS")).toContain("pii.ip_public");
|
||||
expect(ids("1.98.0.0\n", "NOT_VERSION")).toContain("pii.ip_public");
|
||||
expect(ids("host 8.8.8.8\n", "VERSION")).toContain("pii.ip_public");
|
||||
expect(scan("1.98.0.0\nupstream 8.8.8.8\n", { repoVisibility: "private", sourcePath: "VERSION" }).findings.filter((f) => f.id === "pii.ip_public").map((f) => f.line)).toEqual([1, 2]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("pii.ip_public: a four-part version gstack names is not an address (GST-1204)", () => {
|
||||
table(
|
||||
"pii.ip_public",
|
||||
|
||||
Reference in new issue
Block a user