From 2edd1c0cfbed0b8234cdf390b8c5baee3b6c277c Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 26 Oct 2026 00:55:45 +0000 Subject: [PATCH] fix(post): the repo's release versions and a receipt's tree hash / run ids are not pii.ip_public or pii.phone.e164 Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 1 + docs/troubleshooting.md | 6 +++- lib/gstack-post.ts | 59 +++++++++++++++++++++++++++++++++-- test/gstack-post.test.ts | 67 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 129 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4dd025c73..72e0c2d94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ ### Fixed - **`gstack-gate-diff ` in a clone with no local `` branch.** Before, `gstack-gate-diff main` in a checkout that cloned only its feature branch printed `GATE_ERROR=no_base ref=main fix=git fetch origin main`, and that fix did not help: the tool only tried the local branch, and in a single-branch clone a bare `git fetch origin main` updates FETCH_HEAD alone. Now `lib/gate-diff/git.ts` resolves the base to its remote-tracking branch (`origin/main`, origin first, then the other remotes) when no local branch exists, says so once on stderr, and the merge-base is the same commit. When the remote-tracking branch is missing too, the fix line fetches into it explicitly: `git fetch origin +main:refs/remotes/origin/main` (with `--unshallow` on a shallow clone), which works in every clone shape. `test/gate-diff-cli.test.ts` adds a single-branch clone fixture with no local `main` and no `origin/main`: it asserts the error, runs the printed fix verbatim, and expects the next run to list the fixture's RH-1 hunk; two more clones cover a second remote that carries the branch when origin does not, and an upstream-only clone of a master-default repository whose default base (no argument) now comes from the same remote list as the fix line. The refspec is forced (`+main:refs/remotes/origin/main`) so the line also repairs a tracking ref that exists but shares no history with HEAD. +- **`gstack-post` no longer flags the repository's own release versions as public IPs.** A four-part version is byte-for-byte a dotted quad, so "(1.91.191.0)" in a PR body raised `pii.ip_public` (MEDIUM) and stopped an unattended docs audit that cannot confirm a MEDIUM finding. The engine's version exemption only reads the declaration right before the span (`"version": "`, `## [`), so prose that merely names a release got no help. Now `lib/gstack-post.ts` reads the root `VERSION` value and every version in a root `CHANGELOG.md` release heading (`## [1.2.3.4]`, `## v1.2.3.4`, through the same `parseChangelog` gstack-restamp uses) and passes those exact strings to the scan's allowlist. Verdict: this narrows one detector by the repo's own release set, and nothing else. The allowlist is exact-string and context-free, so a released quad is exempt wherever it appears in the text, `host: 1.91.191.0` included; the set is the repo's own releases and nothing wider. `8.8.8.8` and `203.0.113.7` in prose, a quad the repo never released (`1.128.1.0`), and the same text outside a repository or in a repo whose VERSION is three-part all still need confirmation; `test/gstack-post.test.ts` carries those controls beside the four benign bodies (`(1.91.191.0)`, `v1.91.226.0`, a bare `1.91.191.0`, `release 1.91.190.0`) against a synthetic CHANGELOG in which `1.91.192.0` is not a release. The same post path also stops reading a ship receipt's all-digit tree hash (`tree=704992822996`) and an 11-digit GitHub Actions run id in prose or link text (`run 19368712345`, `[run 19368712345](…/actions/runs/19368712345)`) as `pii.phone.e164`: only the exact tokens in those positions are allowlisted, and `+14155551234`, a bare 11-digit number and a 10-digit number after `run` still need confirmation (GSTA-94 hit this on #3424). ## [1.91.226.0] - 2026-10-26 diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index f9ca4cef2..4597984fd 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -2239,7 +2239,11 @@ entry that no longer applies; rotate a credential that is real. ### `pii.ip_public` MEDIUM on a four-part version number -**Meaning.** The line does not declare the value as a version. +**Meaning.** The line does not declare the value as a version. When the +text goes through `gstack-post` from inside the repository, four-part versions +equal to the root `VERSION` or to a `## [x.y.z.w]` / `## vx.y.z.w` CHANGELOG.md heading are +exempt wherever they appear, so there the finding means the quad is not a +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`. MEDIUM findings do not block pushes. diff --git a/lib/gstack-post.ts b/lib/gstack-post.ts index 7c0f7e17d..7430c6a1e 100644 --- a/lib/gstack-post.ts +++ b/lib/gstack-post.ts @@ -16,6 +16,8 @@ import { createHash } from "crypto"; import { scan, type Finding, type RepoVisibility } from "./redact-engine"; import { canonicalRemote } from "./remote-identity"; +import { parseChangelog } from "./changelog-check"; +import { versionWidth } from "./version-source"; export const OPS = ["pr-comment", "issue-comment", "reply", "pr-title", "pr-body", "pr-create", "issue-create"] as const; export type Op = (typeof OPS)[number]; @@ -198,11 +200,61 @@ function visibility(env: PostEnv, req: Request, kind: HostKind, configBin: strin type Part = "title" | "body"; interface Located { part: Part; finding: Finding } -function scanParts(parts: Array<[Part, string]>, vis: RepoVisibility, selfEmail: string | undefined) { +/** + * The repository's own four-part release versions: the root VERSION file's + * value and every version in a root CHANGELOG.md release heading, read with + * the same parser gstack-restamp uses (`## [1.2.3.4]`, `## v1.2.3.4`, an + * optional `- date`; a heading that only mentions a quad is not one). A + * four-part version is byte-for-byte a dotted quad, and `1.91.191.0` is a + * public address, so "(1.91.191.0)" in a PR body raised pii.ip_public + * (MEDIUM), which an unattended /ship cannot confirm. The engine's own version + * exemption reads only the declaration right before the span, so prose that + * merely names a release gets no help from it. These exact strings are + * allowlisted for the scan wherever they appear (the allowlist is exact-string + * and context-free, so `host: ` is exempt too, a trade the + * repo's own release set is small enough to carry); any other dotted quad + * (8.8.8.8, 203.0.113.7, a version this repo never released) still reports. + * The files are read from the checked-out tree, so a branch that adds a + * four-part heading widens the set for posts made from that checkout. + */ +function repoReleaseVersions(env: PostEnv): string[] { + const top = env.run("git", ["rev-parse", "--show-toplevel"]); + const root = top.status === 0 ? top.stdout.trim() : ""; + if (!root) return []; + const read = (name: string) => { try { return env.readFile(`${root}/${name}`); } catch { return ""; } }; + const versions = new Set(); + const current = read("VERSION").trim(); + if (versionWidth(current) === 4) versions.add(current); + for (const s of parseChangelog(read("CHANGELOG.md")).sections) if (s.version && versionWidth(s.version) === 4) versions.add(s.version); + return [...versions]; +} + +/** + * All-digit identifiers a ship receipt or a CI link names, which pii.phone.e164 + * reads as phone numbers: a 12-hex receipt sha that happens to be all digits + * (`tree=704992822996`, `tree 704992822996`, and the receipt's `head=`, + * `gated_tree=` and `predecessor=` keys) and an 11-digit GitHub Actions + * run id as prose or link text (`run 19368712345`, `[run 19368712345](…/actions/runs/19368712345)`; + * the engine already exempts the id inside the URL itself). Only the exact + * tokens found in those positions are allowlisted, so a bare 11-digit number, + * a 10-digit national number and an E.164 `+14155551234` still report. + */ +function receiptDigitTokens(texts: string[]): string[] { + const tokens = new Set(); + for (const text of texts) { + for (const m of text.matchAll(/(?:^|[^\w])(?:tree|gated_tree|head|predecessor)[= ](\d{12})\b/g)) tokens.add(m[1]!); + for (const m of text.matchAll(/\brun #?(\d{11})\b/g)) tokens.add(m[1]!); + for (const m of text.matchAll(/\/actions\/runs\/(\d{11})\b/g)) tokens.add(m[1]!); + } + return [...tokens]; +} + +function scanParts(parts: Array<[Part, string]>, vis: RepoVisibility, selfEmail: string | undefined, releaseVersions: string[]) { const found: Located[] = []; let oversize = false; + const allowlist = [...releaseVersions, ...receiptDigitTokens(parts.map(([, text]) => text))]; for (const [part, text] of parts) { - const result = scan(text, { repoVisibility: vis, ...(selfEmail ? { selfEmail } : {}) }); + const result = scan(text, { repoVisibility: vis, allowlist, ...(selfEmail ? { selfEmail } : {}) }); oversize ||= result.oversize; for (const finding of result.findings) found.push({ part, finding }); } @@ -303,7 +355,8 @@ export function runPost(argv: string[], env: PostEnv, configBin = "gstack-config const vis = visibility(env, req, remote.kind, configBin); const email = env.run("git", ["config", "user.email"]); - const { found, oversize } = scanParts(parts, vis, email.status === 0 ? email.stdout.trim() || undefined : undefined); + const hasQuad = parts.some(([, text]) => /\b\d{1,3}(?:\.\d{1,3}){3}\b/.test(text)); + const { found, oversize } = scanParts(parts, vis, email.status === 0 ? email.stdout.trim() || undefined : undefined, hasQuad ? repoReleaseVersions(env) : []); env.out(`REPO_VISIBILITY: ${vis}\n`); const line = (m: Located) => `RULE: ${m.finding.id} LINE: ${m.finding.line} PART: ${m.part}\n`; const high = found.filter(m => m.finding.severity === "HIGH"); diff --git a/test/gstack-post.test.ts b/test/gstack-post.test.ts index 52b05153f..485094b27 100644 --- a/test/gstack-post.test.ts +++ b/test/gstack-post.test.ts @@ -16,6 +16,8 @@ function fakeEnv(opts: { files?: Record; visibility?: string; email?: string; + /** The repository root `git rev-parse --show-toplevel` reports; VERSION and CHANGELOG.md are read from `files` under it. */ + root?: string; respond?: (c: Call) => { status: number; stdout?: string; stderr?: string } | undefined; } = {}) { const calls: Call[] = []; @@ -30,6 +32,7 @@ function fakeEnv(opts: { if (custom) return { stdout: "", stderr: "", ...custom }; if (cmd === "git" && args[0] === "remote") return { status: 0, stdout: (opts.remote ?? "git@github.com:acme/widget.git") + "\n", stderr: "" }; if (cmd === "git" && args[0] === "config") return { status: opts.email ? 0 : 1, stdout: (opts.email ?? "") + "\n", stderr: "" }; + if (cmd === "git" && args[0] === "rev-parse") return { status: opts.root ? 0 : 128, stdout: (opts.root ?? "") + "\n", stderr: opts.root ? "" : "fatal: not a git repository" }; if (cmd === "cfg") return { status: 0, stdout: (opts.visibility ?? "private") + "\n", stderr: "" }; if (cmd === "gh" && args[0] === "pr" && args[1] === "edit") { const t = args.find(a => a.startsWith("--title=")); @@ -159,6 +162,70 @@ describe("gstack-post owns the redaction scan", () => { } }); + test("the repo's own release versions in prose are not public IPs; other dotted quads still need confirmation", () => { + const repo = { + "/repo/VERSION": "1.91.226.0\n", + "/repo/CHANGELOG.md": "# Changelog\n\n## [Unreleased]\n\n## [1.91.191.0] - 2026-10-24\n- x\n\n## v1.91.190.0\n\n## [1.128.1.0 mirror]\n", + }; + const clean = [ + "The receipt is written to a file (1.91.191.0).", + "Since v1.91.226.0 the queue refuses a READY without it.", + "1.91.191.0", + "landed in release 1.91.190.0 and 1.91.226.0", + ]; + for (const body of clean) { + const f = fakeEnv({ files: { ...repo, "/b": body + "\n" }, root: "/repo", visibility: "public" }); + expect([body, run(["pr-comment", "12", "--body-file", "/b"], f)]).toEqual([body, EXIT.posted]); + expect(f.posts()).toHaveLength(1); + } + const controls = [ + "resolver 8.8.8.8 answered", + "egress is 203.0.113.7 in prod", + "the mirror at 1.128.1.0 is ours", + "host: 1.91.192.0", + ]; + for (const body of controls) { + const f = fakeEnv({ files: { ...repo, "/b": body + "\n" }, root: "/repo", visibility: "public" }); + expect([body, run(["pr-comment", "12", "--body-file", "/b"], f)]).toEqual([body, EXIT.confirm]); + expect(f.stdout()).toContain("RULE: pii.ip_public LINE: 1 PART: body"); + expect(f.posts()).toEqual([]); + } + const elsewhere = fakeEnv({ files: { "/b": "The receipt is written to a file (1.91.191.0).\n" }, visibility: "public" }); + expect(run(["pr-comment", "12", "--body-file", "/b"], elsewhere)).toBe(EXIT.confirm); + expect(elsewhere.stdout()).toContain("RULE: pii.ip_public LINE: 1 PART: body"); + const noVersion = fakeEnv({ files: { "/repo/VERSION": "2.4.0\n", "/b": "see (1.91.191.0)\n" }, root: "/repo", visibility: "public" }); + expect(run(["pr-comment", "12", "--body-file", "/b"], noVersion)).toBe(EXIT.confirm); + }); + + test("a receipt's all-digit tree hash and GitHub run ids in prose or link text are not phone numbers; real numbers still are", () => { + const clean = [ + "SHIP_RECEIPT: pr=3424 head=abc tree=704992822996", + "receipt tree 704992822996 verified", + "head=193687123456 gated_tree=704992822996 predecessor=123456789012 (gate-reuse: eligible)", + "cleared by run 19368712345", + "see [run 19368712345](https://github.com/garrytan/gstack/actions/runs/19368712345) for the gate", + "run #19368712345 is green", + ]; + for (const body of clean) { + const f = fakeEnv({ files: { "/b": body + "\n" }, visibility: "public" }); + expect([body, run(["pr-comment", "12", "--body-file", "/b"], f)]).toEqual([body, EXIT.posted]); + expect(f.posts()).toHaveLength(1); + } + const controls = [ + "call +14155551234 for the on-call", + "the id is 19368712345", + "run 4155551234 answered", + "tree=70499282299 is short", + "subtree=704992822996 is not a receipt key", + ]; + for (const body of controls) { + const f = fakeEnv({ files: { "/b": body + "\n" }, visibility: "public" }); + expect([body, run(["pr-comment", "12", "--body-file", "/b"], f)]).toEqual([body, EXIT.confirm]); + expect(f.stdout()).toContain("RULE: pii.phone.e164 LINE: 1 PART: body"); + 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);