diff --git a/.claude/skills/proofread-leaks/SKILL.md b/.claude/skills/proofread-leaks/SKILL.md index 06153be3b1..3eea65d42e 100644 --- a/.claude/skills/proofread-leaks/SKILL.md +++ b/.claude/skills/proofread-leaks/SKILL.md @@ -43,29 +43,37 @@ lines, including code fences and frontmatter. (`th_01JQZK4M8XN2VR7TDY0P3WGA6H`), `app.plain.com` links, or any equivalent from another helpdesk. -2. **Customer, org, and account identifiers.** GitHub +2. **Tracker ticket references.** A bare issue key + (`MRGFY-1234`, `HD-18`), linked or not, including + the lowercase form a branch name carries. Look + hardest inside example values — a key in a + `--reason` string or a commit message reads as a + plausible sample, which is how one reached the + published site. + +3. **Customer, org, and account identifiers.** GitHub org or repo names belonging to a customer, account IDs, subscription IDs, Stripe customer or invoice IDs, seat counts of a real account, email addresses. -3. **Private repositories and internal code paths.** +4. **Private repositories and internal code paths.** `Mergifyio/monorepo`, `mergify_shadow_office/...`, `mergify_engine/...`, dashboard source paths, internal module, class, or function names, and file:line references into private code. -4. **Internal URLs and tools.** Linear and Notion +5. **Internal URLs and tools.** Linear and Notion links, internal dashboards and admin consoles, staging or internal Mergify hostnames, cloud consoles, internal runbooks. -5. **Credentials.** Real tokens, API keys, private +6. **Credentials.** Real tokens, API keys, private keys, or webhook secrets. Placeholders like `ghp_*` or `` are fine; anything that could be a live secret is not. -6. **Internal-only framing.** Employee names in an +7. **Internal-only framing.** Employee names in an internal context, quotes from a support conversation, "as discussed with the customer", references to unreleased work, or an explanation diff --git a/AGENTS.md b/AGENTS.md index d482317cc0..028c6d837e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -168,6 +168,10 @@ comments, frontmatter, alt text, or screenshots: - **Support ticket and thread IDs** - Plain refs (`T-1234`), thread IDs (`th_01JQ...`), `app.plain.com` links, or the equivalent from any helpdesk. +- **Tracker ticket references** - a bare issue key (`MRGFY-1234`, `HD-18`), with + or without a link, including the lowercase form a branch name carries. A key + dropped into an example value - `--reason "flaky, see MRGFY-1234"` - is the + one that survives review, because it reads as a plausible sample. - **Customer, org, and account identifiers** - a customer's GitHub org or repo, account and subscription IDs, Stripe customer or invoice IDs, email addresses, real seat or contributor counts. diff --git a/scripts/check-internal-leaks.mjs b/scripts/check-internal-leaks.mjs index 11ba7d944a..3409c1b78c 100644 --- a/scripts/check-internal-leaks.mjs +++ b/scripts/check-internal-leaks.mjs @@ -1,8 +1,8 @@ #!/usr/bin/env node /** * Scan the docs for internal information that must never be published: support - * ticket and thread IDs, private repository and source paths, and internal - * tracker links. + * ticket and thread IDs, tracker ticket references, private repository and + * source paths, and internal tracker links. * * Docs are frequently written *from* internal material — a support case, a * private PR, an engine source file — and that material travels with the draft. @@ -22,7 +22,8 @@ * * {/* internal-leaks: allow internal-source-path — documenting the OSS engine layout *\/} * - * In YAML or shell fences, use `# internal-leaks: allow — why`. + * In YAML or shell fences, use `# internal-leaks: allow — why`. When one + * line trips more than one rule, name them separated by commas. */ import fs from 'node:fs'; @@ -45,6 +46,21 @@ export const RULES = [ label: 'support ticket or thread ID', re: /\bT-\d{3,6}\b|\bth_[0-9A-Za-z]{16,}\b|\bapp\.plain\.com\b/g, }, + { + id: 'ticket-ref', + label: 'internal tracker ticket reference', + // A bare issue key is the shape a ticket takes everywhere except a URL: in + // prose, inside a `--reason` string, in a branch name. `internal-tracker` + // below only sees the link form, so a bare key used to pass — one shipped to + // the public site in a CLI example. Prefixes are listed explicitly rather + // than matched as a generic `[A-Z]{3,}-\d+`: that shape reads as a plausible + // example value (`AES-256`, `WCAG-2`, a reader's own issue keys in a sample + // config), and every one of those false positives would land on a docs + // contributor. `mrgfy-` matches too, because that is the form the key takes + // in a branch name. `HD-` stays case-sensitive: lowercase `-hd-2` is a + // believable file or slug fragment, uppercase `HD-2` is not. + re: /\b(?:MRGFY|mrgfy)-\d+\b|\bHD-\d+\b/g, + }, { id: 'internal-source-path', label: 'private repository or internal source path', @@ -62,7 +78,11 @@ export const RULES = [ }, ]; -const ALLOW_RE = /internal-leaks:\s*allow\s+([\w-]+)/i; +// A single line can trip several rules at once — a tracker URL carries the +// ticket key inside it — so the directive takes a comma-separated list, not one +// rule. Commas rather than spaces: the reason that follows the ids is prose, and +// space-separated ids would swallow its first words as rule names. +const ALLOW_RE = /internal-leaks:\s*allow\s+([\w-]+(?:\s*,\s*[\w-]+)*)/i; /** Rule IDs allowed by a directive on the line above `index` (0-based). */ function allowedOnLine(lines, index) { @@ -70,7 +90,7 @@ function allowedOnLine(lines, index) { for (let p = index - 1; p >= 0; p -= 1) { if (!lines[p].trim()) continue; const m = lines[p].match(ALLOW_RE); - if (m) allowed.add(m[1].toLowerCase()); + if (m) for (const id of m[1].split(',')) allowed.add(id.trim().toLowerCase()); break; } return allowed; @@ -149,7 +169,7 @@ function main(argv) { '\nRewrite the line so it states the product behavior without the internal\n' + 'identifier (see .claude/skills/proofread-leaks). If a match is genuinely\n' + 'public, allow it on the line above:\n' + - ' {/* internal-leaks: allow — why */}' + ' {/* internal-leaks: allow [, ...] — why */}' ); return 1; } diff --git a/scripts/check-internal-leaks.test.mjs b/scripts/check-internal-leaks.test.mjs index 7ef01d2552..1285440cfb 100644 --- a/scripts/check-internal-leaks.test.mjs +++ b/scripts/check-internal-leaks.test.mjs @@ -10,6 +10,32 @@ describe('scanText', () => { expect(rulesOf('https://app.plain.com/workspace/x')).toEqual(['support-ticket']); }); + it('catches bare tracker ticket references', () => { + expect(rulesOf('Tracked in MRGFY-1234.')).toEqual(['ticket-ref']); + // The form that reached the public site: inside a CLI example, where it + // reads as a plausible value rather than as a leak. + expect(rulesOf('mergify tests quarantines add --reason "flaky, see MRGFY-1234"')).toEqual([ + 'ticket-ref', + ]); + // The branch-name spelling of the same key. + expect(rulesOf('git checkout devs/jd/mrgfy-8721-fix-the-thing')).toEqual(['ticket-ref']); + expect(rulesOf('helpdesk HD-18')).toEqual(['ticket-ref']); + // A tracker URL leaks twice over; both rules report it. + expect(rulesOf('https://linear.app/mergify/issue/MRGFY-1234')).toEqual([ + 'ticket-ref', + 'internal-tracker', + ]); + }); + + it('leaves example values that look like ticket keys alone', () => { + expect(rulesOf('encrypted with AES-256 and served as UTF-8')).toEqual([]); + expect(rulesOf('meets WCAG-2 contrast, over HTTP-2, dated 2026-07')).toEqual([]); + // Another tracker's keys are the reader's own, not ours. + expect(rulesOf('a Jira key such as PROJ-42 or ABC-7')).toEqual([]); + // Lowercase two-letter keys are indistinguishable from slug fragments. + expect(rulesOf('![diagram](./queue-hd-2.png)')).toEqual([]); + }); + it('catches private repositories and internal source paths', () => { expect(rulesOf('mergify_shadow_office/models/billing.py')).toEqual(['internal-source-path']); expect(rulesOf('cloned from Mergifyio/monorepo')).toEqual(['internal-source-path']); @@ -17,7 +43,7 @@ describe('scanText', () => { }); it('catches internal trackers and hostnames', () => { - expect(rulesOf('https://linear.app/mergifyio/issue/MRGFY-1')).toEqual(['internal-tracker']); + expect(rulesOf('https://linear.app/mergifyio/issue/1')).toEqual(['internal-tracker']); expect(rulesOf('https://www.notion.so/mergify/runbook')).toEqual(['internal-tracker']); expect(rulesOf('https://admin.mergify.com/orgs')).toEqual(['internal-host']); }); @@ -53,6 +79,17 @@ describe('scanText', () => { expect(rulesOf('{/* internal-leaks: allow support-ticket */}\nfine\nT-1234\n')).toEqual([ 'support-ticket', ]); + // A line that trips two rules needs both named, so the directive takes a + // comma-separated list. + expect( + rulesOf( + '{/* internal-leaks: allow ticket-ref, internal-tracker — public */}\nMRGFY-1 at linear.app/x\n' + ) + ).toEqual([]); + // The reason is prose: its words are not read as further rule ids. + expect(rulesOf('# internal-leaks: allow internal-tracker support-ticket\nT-1234\n')).toEqual([ + 'support-ticket', + ]); }); }); diff --git a/src/components/ScopesDetection.astro b/src/components/ScopesDetection.astro index b2cf66480c..a0f3355631 100644 --- a/src/components/ScopesDetection.astro +++ b/src/components/ScopesDetection.astro @@ -5,6 +5,14 @@ * batch-aware note are identical across build tools — only the command that * turns the BASE..HEAD diff into scopes.json differs, passed as `command`. * + * `--scopes-json` is the flag that reads the `{"scopes": [...]}` object every + * `command` writes; `--scopes-file` next to it takes one scope per line instead. + * The `--format json` refs are parsed with jq rather than scraped out of the + * default `Base: ` text, which is a human format with no stability promise. + * `printf '%s'` rather than `echo`, whose handling of backslashes in the JSON + * varies by shell; the format string stays `%s` with no `\n` because this + * template literal would turn that escape into a real newline inside the quotes. + * * Pass `command` with String.raw so the bash keeps its literal backslashes * (line continuations, `\n` in awk/jq) instead of the JS template literal eating * them. @@ -22,9 +30,9 @@ export interface Props { const { command } = Astro.props as Props; -const code = `REFS=$(mergify ci git-refs) -BASE=$(echo "$REFS" | awk '/^Base:/ {print $2}') -HEAD=$(echo "$REFS" | awk '/^Head:/ {print $2}') +const code = `REFS=$(mergify ci git-refs --format json) +BASE=$(printf '%s' "$REFS" | jq -r '.base') +HEAD=$(printf '%s' "$REFS" | jq -r '.head') ${command} diff --git a/src/content/docs/test-insights/quarantine.mdx b/src/content/docs/test-insights/quarantine.mdx index 20f13438ce..12495e1c93 100644 --- a/src/content/docs/test-insights/quarantine.mdx +++ b/src/content/docs/test-insights/quarantine.mdx @@ -35,7 +35,7 @@ which is handy for scripting or for letting an AI coding agent quarantine a flak ```bash # Quarantine a test. mergify tests quarantines add -r owner/repo \ - --reason "flaky — tracked in MRGFY-1234" \ + --reason "flaky under load, fix in progress" \ "tests/auth/test_login.py::test_login_timeout" # Remove it from quarantine.