fix(redact): a short sha is not a phone number or a bc1 wallet; plan-reality receipt and pregate fixture flakes (#3998)

* fix(redact): a short sha is not a phone number or a bc1 wallet; plan-reality receipt and pregate fixture flakes (GSTA-279)

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

* docs: the review_loop_grant sha exemption and the fixture origin's maintenance setting (GSTA-279)

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

* docs(changelog): name the fragment for the branch and narrow its claim to the review_loop_grant position (GSTA-279)

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-05 00:01:33 +00:00
1 parent f86c9491b2
commit ec1f2aa6a4
9 files changed
+144 -7

No files matched your search

+5
View File
@@ -0,0 +1,5 @@
### Fixed
- **The secret scanner no longer reads the sha in an owner's `review_loop_grant` as a phone number, or a git sha that starts with `bc1` as a Bitcoin address. Both made free tests flaky.** About one 12-character sha in 300 is all digits, and `pii.phone.e164` flagged it, so `gstack-approvals add` refused the owner's `review_loop_grant <sha>` grant with APPROVAL_RECORD_SECRET (exit 3). A digits-only span right after `review_loop_grant` or `review_loop_exception` is now a sha. A phone number with a `+`, spaces or dashes, or the same digits anywhere else (an all-digit sha in other wording included), still reports (Free Tests 41667825418, 41766865679, 41680498938, 41843093520). A sha that starts with `bc1` was redacted as `<REDACTED-pii.wallet>` in CSO's history range. A `bc1` address now has to use the bech32 alphabet and carry a valid bech32 or bech32m checksum, which every real address has. An address with a mistyped character no longer reports (Free Tests 41715139988). Both rules have true-positive controls in `test/redact-context-exemptions.test.ts`.
- **`test/plan-reality-receipts.test.ts`**: the commit-sha case writes its receipt up to the sha's first hex letter. A bare all-digit prefix is not a receipt by design, and about one 10-character prefix in 110 is all digits (Free Tests 41669379416, 41790613651).
- **`test/helpers/pregate-fixture.ts`**: the fixture's bare origin turns off auto-maintenance, as `test/helpers/restamp-fixture.ts` already does. Git's local transport drops the free runner's maintenance-off setting, so each push into the origin started a detached `git maintenance run --auto` that could change `origin.git/objects` while the template was being copied (ENOENT, Free Tests 41760556132). A new test traces a push and checks that no maintenance starts.
+4 -1
View File
@@ -1556,7 +1556,10 @@ Free-suite flake rules:
replacement env never sees the backstop. Neither does the receiving side of
a push to a local path: git's local transport drops `GIT_CONFIG_*` from the
`receive-pack` it starts, so every push into a fixture's bare origin still
leaves a detached maintenance run in that origin. A fixture that clones such
leaves a detached maintenance run in that origin unless the origin's own
config turns it off (`maintenance.auto=false` and `gc.auto=0` on the bare
origin, as `test/helpers/pregate-fixture.ts` and
`test/helpers/restamp-fixture.ts` do). A fixture that clones such
an origin passes `--no-local` (`coordinatorClone` in
`test/helpers/ship-queue-fixture.ts`), so the clone reads it through git's
transport instead of copying `objects/` entry by entry.
+6
View File
@@ -2807,6 +2807,12 @@ exact tokens it finds in these positions:
- a 12-digit value right after `tree`, `gated_tree`, `head` or `predecessor`
with `=` or a space (`tree=704992822996`, `tree 704992822996`);
- an all-digit sha right after `review_loop_grant` or `review_loop_exception`
with a space, `=` or `:` (`review_loop_grant 367120488513`), so the owner's
grant records through `gstack-approvals add`. The scanner itself exempts it
(`isReviewLoopSha` in `lib/redact-patterns.ts`), and only that position
counts: the same digits elsewhere, or a number with a `+`, spaces or dashes
after the token, still report;
- an 11-digit id inside an `/actions/runs/<id>` URL or as that link's text;
- any id inside an `actions/runs/<id>` path, with or without a host, when
`actions/runs/` starts the line or follows whitespace, `/`, `(`, `[`,
+49 -5
View File
@@ -167,10 +167,36 @@ function looksLikeWallet(span: string): boolean {
const body = span.slice(2).toLowerCase();
return !/^(.)\1{39}$/.test(body);
}
// bech32 / base58 — length sanity only
if (span.startsWith("bc1")) return bech32ChecksumValid(span);
// base58 — length sanity only
return span.length >= 26 && span.length <= 62;
}
/**
* A bc1 address is bech32 (BIP 173) or bech32m (BIP 350): `bc`, the separator `1`, then data from a 32-letter alphabet
* with no `1`, `b`, `i` or `o`, ending in a six-letter checksum. A git sha that starts with `bc1` read as a wallet and
* was redacted out of `<sha>..<sha>` (Free Tests 41715139988, test/cso-cli.test.ts); the alphabet in the regex refuses
* most such shas, and the checksum, which a random string passes about once in 500 million tries, refuses the rest. Every real
* address carries a valid checksum, so none is lost.
*/
const BECH32_ALPHABET = "qpzry9x8gf2tvdw0s3jn54khce6mua7l";
const BECH32_GENERATOR = [0x3b6a57b2, 0x26508e6d, 0x1ea119fa, 0x3d4233dd, 0x2a1462b3];
function bech32ChecksumValid(address: string): boolean {
const sep = address.lastIndexOf("1");
const hrp = [...address.slice(0, sep)].map((c) => c.charCodeAt(0));
const data = [...address.slice(sep + 1)].map((c) => BECH32_ALPHABET.indexOf(c));
if (sep < 1 || data.length < 6 || data.some((v) => v < 0)) return false;
let chk = 1;
for (const v of [...hrp.map((c) => c >> 5), 0, ...hrp.map((c) => c & 31), ...data]) {
const top = chk >>> 25;
chk = ((chk & 0x1ffffff) << 5) ^ v;
BECH32_GENERATOR.forEach((g, i) => {
if ((top >>> i) & 1) chk ^= g;
});
}
return chk === 1 || chk === 0x2bc830a3;
}
// Compact log/backup stamps (`20260727202423` = YYYYMMDDHHMMSS) are bare digit
// runs that the phone regex happily eats. Only a SEPARATOR-FREE 14-digit span
// qualifies: E.164 tops out at 15 digits and real numbers carry a + or spacing,
@@ -392,6 +418,22 @@ export function isCiRunIdentifier(span: string, match: RegExpExecArray): boolean
return [...line.matchAll(CI_RUN_LIST)].some(m => m.index! < at && at + span.length <= m.index! + m[0].length);
}
/**
* The owner's grant of one scoped fix-check is recorded as `review_loop_grant <stop head sha>`, and an accepted stop as
* `review_loop_exception <sha>` (docs/review-policy.md). About one 12-character sha in 300 is all digits
* (`367120488513`), and pii.phone.e164 read it as a phone number, so gstack-approvals refused the owner's grant
* (APPROVAL_RECORD_SECRET; Free Tests 41667825418, 41766865679). Only the position decides: a digits-only span right
* after one of those two tokens and a space, `=` or `:`. The same digits anywhere else, or a number with a `+`,
* spaces or dashes after the token, still report.
*/
const REVIEW_LOOP_SHA_BEFORE = /\breview_loop_(?:grant|exception)(?:[ \t]+|[ \t]*[:=][ \t]*)$/;
function isReviewLoopSha(span: string, match: RegExpExecArray): boolean {
if (!/^\d+$/.test(span)) return false;
const input = match.input ?? "";
const { start } = spanBounds(match);
return REVIEW_LOOP_SHA_BEFORE.test(input.slice(Math.max(0, start - 40), start));
}
/**
* A four-part version (MAJOR.MINOR.PATCH.BUILD: .NET assembly versions,
* gstack's own VERSION) is byte-for-byte a dotted quad, and `1.128.1.0` is a
@@ -1276,8 +1318,9 @@ export const PATTERNS: RedactPattern[] = [
// so do a county tax-map parcel ID (see looksLikeParcelId), vector
// coordinates (looksLikeDecimalCoordinates) and seed/nonce/timestamp JSON
// values (isNumericMetadataValue), GitHub Actions run and job ids in
// their id positions (isCiRunIdentifier), and bare powers of two and
// hash/PRNG constants (isNumericConstant).
// their id positions (isCiRunIdentifier), a short sha after
// review_loop_grant or review_loop_exception (isReviewLoopSha), and bare
// powers of two and hash/PRNG constants (isNumericConstant).
validate: (span, match) =>
!insideUuid(match) &&
span.replace(/\D/g, "").length >= 10 &&
@@ -1286,7 +1329,8 @@ export const PATTERNS: RedactPattern[] = [
!looksLikeParcelId(span, match) &&
!looksLikeDecimalCoordinates(span) &&
!isNumericMetadataValue(span, match) &&
!isCiRunIdentifier(span, match),
!isCiRunIdentifier(span, match) &&
!isReviewLoopSha(span, match),
},
{
id: "pii.ssn",
@@ -1330,7 +1374,7 @@ export const PATTERNS: RedactPattern[] = [
tier: "MEDIUM",
category: "pii",
description: "Crypto wallet address (ETH/BTC)",
regex: /\b(0x[a-fA-F0-9]{40}|bc1[a-z0-9]{25,39}|[13][a-km-zA-HJ-NP-Z1-9]{25,34})\b/,
regex: /\b(0x[a-fA-F0-9]{40}|bc1[qpzry9x8gf2tvdw0s3jn54khce6mua7l]{25,39}|[13][a-km-zA-HJ-NP-Z1-9]{25,34})\b/,
validate: (span) => looksLikeWallet(span),
},
+12
View File
@@ -88,6 +88,18 @@ describe('gstack-approvals add and render', () => {
expect(control.err).toContain('pii.phone.e164');
});
test("the owner's grant for a stop head whose short sha is all digits records (Free Tests 41667825418, 41766865679)", () => {
const cwd = repo(); const state = tmp('appr-state-');
const ask = 'Owner grant: one scoped fix-check after round 2 (review_loop_grant 367120488513)';
const r = bin(['add', '--ask', ask, '--words', 'do it', '--by', 'GStack Minder', '--scope', 'this /ship invocation'], { cwd, env: { GSTACK_STATE_ROOT: state } });
expect(r.status, r.err).toBe(0);
const file = /^APPROVAL_RECORDED: a-[0-9a-f]{12} (.*\.jsonl)$/m.exec(r.out)![1]!;
expect(fs.readFileSync(file, 'utf8')).toContain('review_loop_grant 367120488513');
const control = bin(['add', '--ask', 'Owner grant (review_loop_grant 367120488513), call 367120488513 first', '--words', 'do it', '--by', 'g', '--scope', 's'], { cwd, env: { GSTACK_STATE_ROOT: state } });
expect(control.status).toBe(3);
expect(control.err).toContain('field ask: pii.phone.e164');
});
test('missing fields, bad outcomes and impossible dates are usage errors', () => {
const cwd = repo(); const state = tmp('appr-state-');
expect(bin(['add', '--ask', 'x'], { cwd, env: { GSTACK_STATE_ROOT: state } }).err).toContain('APPROVAL_RECORD_INVALID');
+2
View File
@@ -122,6 +122,8 @@ function seedFixtureRepo(root: string, opts: FixtureOptions): void {
fs.mkdirSync(work, { recursive: true });
fs.writeFileSync(env.GIT_CONFIG_GLOBAL, '[user]\n\tname = t\n\temail = t@test\n[commit]\n\tgpgsign = false\n[init]\n\tdefaultBranch = main\n');
spawnSync('git', ['init', '-q', '--bare', origin], { timeout: 30_000, env: { ...process.env, ...env } });
git('--git-dir', origin, 'config', 'maintenance.auto', 'false');
git('--git-dir', origin, 'config', 'gc.auto', '0');
git('init', '-q', '-b', 'main');
for (const dir of ['src', 'lib', 'bin', 'test', 'test/golden', 'test/nightly', 'generated', 'docs', 'scripts', '.gstack', '.github/workflows']) fs.mkdirSync(path.join(work, dir), { recursive: true });
write(work, 'src/version.js', "module.exports = { VERSION: '1.0.0' };\n");
+4 -1
View File
@@ -108,7 +108,10 @@ describe('resolveReceipts', () => {
gitOk(dir, 'add', '.');
gitOk(dir, '-c', 'user.name=t', '-c', 'user.email=t@t', 'commit', '-qm', 'seed');
const sha = gitOk(dir, 'rev-parse', 'HEAD').stdout.trim();
const out = resolveReceipts(parseRealityLines(`REALITY: numbers pass ok ${sha.slice(0, 10)} abcdef1234`), { roots: [dir] });
// A bare sha is a receipt only with a hex letter in it (RECEIPT_RE reads an all-digit run as a number unless `commit ` or
// backticks mark it), and about one 10-character prefix in 110 is all digits (Free Tests 41669379416, 41790613651).
const receipt = sha.slice(0, Math.max(10, sha.search(/[a-f]/) + 1));
const out = resolveReceipts(parseRealityLines(`REALITY: numbers pass ok ${receipt} abcdef1234`), { roots: [dir] });
expect(out.map(r => r.outcome)).toEqual(['resolved', 'unverifiable']);
});
});
+17
View File
@@ -91,6 +91,23 @@ describe('loadConfigFromUser', () => {
});
});
describe('fixture origin', () => {
// Value: protects=a push into the fixture's bare origin starts no detached auto-maintenance there, so copying the template never races
// a repack (ENOENT lstat origin.git/objects/1d, Free Tests 41760556132); fails_when=the origin loses maintenance.auto=false;
// why_new=local transport drops the runner's GIT_CONFIG_COUNT backstop, so only the origin's own config reaches receive-pack;
// seam=none; no_claim=which maintenance task removed the object directory on the CI runner's git
test('a push into the origin starts no auto-maintenance even when the runner backstop is dropped', () => {
const { repo } = setup();
const trace = path.join(repo.root, 'trace2.txt');
const backstop = { GIT_CONFIG_COUNT: '2', GIT_CONFIG_KEY_0: 'maintenance.auto', GIT_CONFIG_VALUE_0: 'false', GIT_CONFIG_KEY_1: 'gc.auto', GIT_CONFIG_VALUE_1: '0' };
const push = spawnSync('git', ['push', '-q', 'origin', 'HEAD:refs/heads/probe'], { cwd: repo.work, encoding: 'utf8', timeout: 30_000, env: { ...process.env, ...repo.env, ...backstop, GIT_TRACE2: trace } });
expect(push.status, push.stderr).toBe(0);
const events = fs.readFileSync(trace, 'utf8');
expect(events).toContain('receive-pack');
expect(events.split('\n').filter(l => l.includes('child_start') && l.includes('maintenance'))).toEqual([]);
});
});
describe('gstack-pregate with a user-owned policy', () => {
// Value: protects=with no base copy the user file is the policy and its `local` lanes run here, while an unnamed platform lane stays
// requires-remote; fails_when=the user fallback or the `local` branch in selectLanes is dropped; why_new=both are new;
+45
View File
@@ -352,3 +352,48 @@ describe("pii.phone.e164: run ids in a runs= list or an actions/runs/ path (GST-
],
);
});
describe("pii.phone.e164: a short sha after review_loop_grant or review_loop_exception (Free Tests 41667825418, 41766865679)", () => {
table(
"pii.phone.e164",
"MEDIUM",
[
// The ask gstack-approvals records for the owner's grant, here with a stop head whose 12-character sha is all digits.
"Owner grant: one scoped fix-check after round 2 (review_loop_grant 367120488513)",
"Owner accepts the stop after the fix-check (review_loop_exception 367120488513)",
"review_loop_grant=367120488513",
"review_loop_grant: 3671204885",
],
[
"review_loop_grant +14155552671",
"review_loop_grant 415-555-2671",
"review_loop_grant (415) 555-2671",
"call 367120488513 about the review_loop_grant",
"grant 367120488513",
"my_review_loop_grant 367120488513",
"review_loop_grant for 367120488513",
],
);
});
describe("pii.wallet: a bc1 address needs the bech32 alphabet and checksum (Free Tests 41715139988)", () => {
table(
"pii.wallet",
"MEDIUM",
[
// The sha the CSO history range printed as <REDACTED-pii.wallet>..<REDACTED-pii.wallet>; it holds 1 and b, outside bech32's alphabet.
"bc1f7576f1eaf9d548ddb6f742b85ec11b4a3e79..bc1f7576f1eaf9d548ddb6f742b85ec11b4a3e79",
// A sha inside the alphabet (no 1 or b after the prefix) fails the checksum.
"commit bc1f7576f2eaf9d548ddf6f742c85ec22d4a3e79",
// A real address with its last character changed fails the checksum too.
"bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t5",
],
[
"bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4",
"send it to bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq today",
"wallet: bc1qxy2kgdygjrsqtzq2n0yrf2493p83kkfjhx0wlh",
"0x52908400098527886E0F7030069857D2E4169EE7",
"1BvBMSEYstWetqTFn5Au4m4GFg7xJaNVN2",
],
);
});