From d468ef847b830f72eb8059823aa5bb8673bbbac7 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 03:35:29 +0000 Subject: [PATCH] fix: pre-landing review fixes (check the origin repository's visibility too, GST-1160) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- .../garrytan-approvals-public-repo-privacy.md | 2 +- lib/approval-record.ts | 20 ++++++++++++++----- test/approval-record.test.ts | 10 ++++++++++ 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/changelog.d/garrytan-approvals-public-repo-privacy.md b/changelog.d/garrytan-approvals-public-repo-privacy.md index ca8cfd546..de7e5b9c6 100644 --- a/changelog.d/garrytan-approvals-public-repo-privacy.md +++ b/changelog.d/garrytan-approvals-public-repo-privacy.md @@ -1,6 +1,6 @@ **`gstack-approvals render` no longer puts the owner's exact words, autoplan run ids or plan hashes into PR bodies on public repositories (GST-1160).** On a public repo, /ship pasted the full `## Approvals` table into the PR description. That table carries the owner's verbatim quote, the run id and the plan sha256, and GitHub keeps every earlier version of a description in its edit history, which no API can delete. ### Fixed -- **`bin/gstack-approvals render`** asks gh for the repository's visibility. When the repository is private (or internal), it prints the full table as before. When the repository is public, or gh can't tell, it prints only `Approved: yes|no (record r-<12 hex>)`. The record id is a hash of the row ids, so the Captain can match it to the private record without anything identifying being published. A stderr line, `APPROVALS_RENDER: redacted (repository visibility …)`, says which mode was used. +- **`bin/gstack-approvals render`** asks gh for the visibility of the repository gh picks by default (`GH_REPO`, its default repo, an `upstream` remote) and of the `origin` repository the PR is opened against. When every one of them is private (or internal), it prints the full table as before. When any is public, or gh can't tell, it prints only `Approved: yes|no (record r-<12 hex>)`. The record id is a hash of the row ids, so the Captain can match it to the private record without anything identifying being published. A stderr line, `APPROVALS_RENDER: redacted (repository visibility …)`, says which mode was used. - **`--full`** prints the full table when gh can't read the visibility and you know the repository is private. It never prints the full table for a repository GitHub reports as public. `gstack-approvals list` still shows the full record. - **/ship Step 19** (`ship/sections/capy-pr.md`) says a public repository gets only the one line, and never to pass `--full` for a PR body. diff --git a/lib/approval-record.ts b/lib/approval-record.ts index 5d1618f8a..ec36a35ad 100644 --- a/lib/approval-record.ts +++ b/lib/approval-record.ts @@ -225,12 +225,22 @@ export const APPROVALS_FOOTER_END = '(display only; not an approval citation).'; export type RepoVisibility = 'public' | 'private' | 'unknown'; -/** The origin repository's visibility as GitHub reports it through gh; anything gh cannot answer is unknown. */ +/** + * The repository's visibility as GitHub reports it through gh, for the repository gh acts on by default (GH_REPO, its + * default repo, an upstream remote) and for the origin remote a PR is opened against: private only when every one + * asked is private, public when any is public, and unknown for anything gh cannot answer. + */ export function repoVisibility(cwd: string, env: Env = process.env): RepoVisibility { - const r = spawnSync('gh', ['repo', 'view', '--json', 'visibility', '-q', '.visibility'], { cwd, env, encoding: 'utf8', timeout: 15_000 }); - if (r.status !== 0) return 'unknown'; - const v = (r.stdout ?? '').trim().toLowerCase(); - return v === 'private' || v === 'internal' ? 'private' : v === 'public' ? 'public' : 'unknown'; + const ask = (repo: string[]): RepoVisibility => { + const r = spawnSync('gh', ['repo', 'view', ...repo, '--json', 'visibility', '-q', '.visibility'], { cwd, env, encoding: 'utf8', timeout: 15_000 }); + if (r.status !== 0) return 'unknown'; + const v = (r.stdout ?? '').trim().toLowerCase(); + return v === 'private' || v === 'internal' ? 'private' : v === 'public' ? 'public' : 'unknown'; + }; + const origin = spawnSync('git', ['remote', 'get-url', 'origin'], { cwd, env, encoding: 'utf8', timeout: 15_000 }); + const slug = origin.status === 0 ? /([^/:]+\/[^/]+?)(?:\.git)?\/*$/.exec(origin.stdout.trim())?.[1] : undefined; + const seen = [ask([]), ...(slug ? [ask([slug])] : [])]; + return seen.includes('public') ? 'public' : seen.every(v => v === 'private') ? 'private' : 'unknown'; } /** diff --git a/test/approval-record.test.ts b/test/approval-record.test.ts index 7286d8dc7..e318d766c 100644 --- a/test/approval-record.test.ts +++ b/test/approval-record.test.ts @@ -143,6 +143,16 @@ describe('public repositories get no identifying approval text (GST-1160)', () = expect(r.err).toContain('public'); }); + test('a private repository gh picks by default does not unlock the table for a public origin', () => { + const cwd = repo(); const state = tmp('appr-state-'); const d = tmp('appr-gh-'); + spawnSync('git', ['remote', 'add', 'origin', 'https://github.com/acme/public-widgets.git'], { cwd, timeout: 30_000 }); + fs.writeFileSync(path.join(d, 'gh'), '#!/bin/sh\ncase " $* " in *" acme/public-widgets "*) echo PUBLIC ;; *) echo PRIVATE ;; esac\n', { mode: 0o755 }); + expect(bin(QUOTE, { cwd, env: { GSTACK_STATE_ROOT: state } }).status).toBe(0); + const r = bin(['render'], { cwd, env: { GSTACK_STATE_ROOT: state, GH_REPO: 'acme/private-upstream', PATH: `${d}:${process.env.PATH}` } }); + expect(leaks(r.out)).toEqual([]); + expect(r.err).toContain('APPROVALS_RENDER: redacted (repository visibility public'); + }); + test('the redacted record id is stable for the same rows and changes when a row is added', () => { const cwd = repo(); const state = tmp('appr-state-'); const id = () => /record (r-[0-9a-f]{12})/.exec(bin(['render'], { cwd, env: { GSTACK_STATE_ROOT: state, ...fakeGh('PUBLIC') } }).out)![1];