diff --git a/plugins/magpie-security/hooks/public-surface-scrub.test.ts b/plugins/magpie-security/hooks/public-surface-scrub.test.ts index d01db8433..4bde063fc 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.test.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.test.ts @@ -2443,4 +2443,52 @@ upstream_repo: apache/kafka }); }); + describe('a blocked gh api write names its vetted route', () => { + const projectConfig = { trackerRepo: 'airflow-s/airflow-s', upstreamRepo: 'apache/airflow' }; + const evaluate = (cmd: string) => + evaluatePublicSurfaceCommand(cmd, { projectConfig, readFile: async () => 'Fixed as CVE-2026-1234.' }); + + test('an advisory comment post is still blocked and points at advisory-comment', async () => { + const res = await evaluate( + 'gh api -X POST repos/apache/airflow/security-advisories/GHSA-abcd-efgh-ijkl/comments -F body=@/tmp/b.md', + ); + assertStrictEqual(res.blocked, true); + assert(res.reason?.includes('advisory-comment GHSA-abcd-efgh-ijkl '), res.reason); + assert(res.reason?.includes('[callers]'), res.reason); + }); + + test('an advisory field edit is still blocked and says no vetted operation exists', async () => { + const res = await evaluate('gh api --method PATCH repos/apache/airflow/security-advisories/GHSA-abcd-efgh-ijkl -f cve_id=CVE-2026-1234'); + assertStrictEqual(res.blocked, true); + assert(res.reason?.includes('No vetted operation edits advisory record fields'), res.reason); + }); + + test('an advisory comment edit says to use the web UI', async () => { + const res = await evaluate('gh api -X PATCH repos/apache/airflow/security-advisories/GHSA-abcd-efgh-ijkl/comments/12 -F body=@/tmp/b.md'); + assertStrictEqual(res.blocked, true); + assert(res.reason?.includes('No vetted operation edits or deletes an advisory comment'), res.reason); + }); + + test('a comment edit and a comment post point at comment-update and issue-comment', async () => { + const edit = await evaluate('gh api -X PATCH repos/apache/airflow/issues/comments/987 -F body=@/tmp/b.md'); + assertStrictEqual(edit.blocked, true); + assert(edit.reason?.includes('comment-update 987 '), edit.reason); + + const post = await evaluate('gh api repos/apache/airflow/issues/42/comments -F body=@/tmp/b.md'); + assertStrictEqual(post.blocked, true); + assert(post.reason?.includes('issue-comment 42 '), post.reason); + }); + + test('other gh api writes get no vetted-route hint', async () => { + const res = await evaluate('gh api -X POST repos/apache/airflow/pulls/5/comments -f body="Fixes CVE-2026-1234"'); + assertStrictEqual(res.blocked, true); + assert(!res.reason?.includes('vetted-op'), res.reason); + }); + + test('the tracker exemption is unchanged', async () => { + const res = await evaluate('gh api -X PATCH repos/airflow-s/airflow-s/issues/comments/987 -F body=@/tmp/b.md'); + assertStrictEqual(res.blocked, false, JSON.stringify(res)); + }); + }); + }); diff --git a/plugins/magpie-security/hooks/public-surface-scrub.ts b/plugins/magpie-security/hooks/public-surface-scrub.ts index 387574f54..02cf05d92 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.ts @@ -677,6 +677,10 @@ export interface ParsedSubcommand { hasDynamicInput: boolean; isGraphqlMutation?: boolean; isLiteralUnknownEndpoint?: boolean; + /** For `gh api`: the REST endpoint path, with any host, `v3` prefix and query string stripped. */ + apiEndpoint?: string; + /** For `gh api`: the HTTP method given with `-X` / `--method`, upper-cased. */ + apiMethod?: string; hasGitDirOrWorkTree?: boolean; /** The command runs with environment set for it: a leading `NAME=value` or an `env` wrapper, after any prefix. */ setsOwnEnvironment?: boolean; @@ -1749,6 +1753,7 @@ export function parseSubcommand(subcommand: string): ParsedSubcommand { let endpoint = token.replace(/^(?:https?:\/\/[^/]+)?\/+/, ''); // Strip optional v3 or api/v3 prefix endpoint = endpoint.replace(/^(?:api\/)?v3\/+/, ''); + result.apiEndpoint = endpoint.replace(/[?#].*$/, ''); if (endpoint.startsWith('repos/')) { const parts = endpoint.slice('repos/'.length).split('/'); @@ -1794,6 +1799,17 @@ export function parseSubcommand(subcommand: string): ParsedSubcommand { break; } + for (let i = cmdIdx + 2; i < tokens.length; i++) { + const token = tokens[i]; + if ((token === '-X' || token === '--method') && i + 1 < tokens.length) { + result.apiMethod = tokens[i + 1].toUpperCase(); + } else if (token.startsWith('--method=')) { + result.apiMethod = token.slice('--method='.length).toUpperCase(); + } else if (/^-X[A-Za-z]+$/.test(token)) { + result.apiMethod = token.slice(2).toUpperCase(); + } + } + for (let i = cmdIdx + 2; i < tokens.length; i++) { const token = tokens[i]; @@ -2056,6 +2072,66 @@ export function generateBlockingReason(commandType: string, terms: string[]): st ].join('\n'); } +/** + * For a blocked `gh api` write that has a vetted route, names that route. + * + * Advisory discussions and tracker comments are private surfaces whose text legitimately carries + * the terms this guard scans for. The guard's decision does not change; the hint only names the + * vetted-ops operation that performs the same write, and says the operation has to be granted to + * the caller in the adopter's vetted-ops config, which is a maintainer's change, not the agent's. + * Returns undefined when the endpoint has no vetted route. + */ +export function vettedRouteHint(parsed: ParsedSubcommand): string | undefined { + if (parsed.commandType !== 'gh api' || !parsed.apiEndpoint) { + return undefined; + } + const endpoint = parsed.apiEndpoint; + const sendsFields = parsed.extractedTexts.length > 0 || parsed.referencedFiles.length > 0; + const method = parsed.apiMethod ?? (sendsFields ? 'POST' : 'GET'); + const grant = + 'The calling skill needs that operation in its `[callers]` list in ' + + '`.apache-magpie-overrides/tools/vetted-ops/config.toml`, and adding it is a maintainer\'s change. ' + + 'The body file goes under `~/.cache/apache-magpie/vetted-ops-bodies/` (see tools/vetted-ops/README.md).'; + + const advisoryComments = endpoint.match( + /^repos\/[^/]+\/[^/]+\/security-advisories\/(GHSA-[0-9a-z-]+)\/comments(\/\d+)?\/?$/i, + ); + if (advisoryComments) { + if (advisoryComments[2] || method === 'PATCH' || method === 'DELETE') { + return 'No vetted operation edits or deletes an advisory comment; do that in the advisory web UI.'; + } + return [ + 'A security advisory discussion is private to its collaborators. Post the comment with the vetted operation instead:', + ` vetted-op --caller advisory-comment ${advisoryComments[1]} `, + grant, + ].join('\n'); + } + + if (/^repos\/[^/]+\/[^/]+\/security-advisories\/GHSA-[0-9a-z-]+\/?$/i.test(endpoint)) { + return 'No vetted operation edits advisory record fields (`cve_id`, `credits`, `severity`, ...); edit them in the advisory web UI.'; + } + + const commentEdit = endpoint.match(/^repos\/[^/]+\/[^/]+\/issues\/comments\/(\d+)\/?$/); + if (commentEdit && method === 'PATCH') { + return [ + 'If this comment is on the private tracker, edit it with the vetted operation instead:', + ` vetted-op --caller comment-update ${commentEdit[1]} `, + grant, + ].join('\n'); + } + + const issueComment = endpoint.match(/^repos\/[^/]+\/[^/]+\/issues\/(\d+)\/comments\/?$/); + if (issueComment && method === 'POST') { + return [ + 'If this issue is on the private tracker, post the comment with the vetted operation instead:', + ` vetted-op --caller issue-comment ${issueComment[1]} `, + grant, + ].join('\n'); + } + + return undefined; +} + /** * Commands known to be purely read-only or harmless environment manipulations. * If any preceding command on a composite shell line is NOT in this set, @@ -2499,6 +2575,10 @@ export async function evaluatePublicSurfaceCommand( if (matched.length > 0) { const uniqueTerms = Array.from(new Set(matched)); let reason = generateBlockingReason(parsed.commandType, uniqueTerms); + const vettedHint = vettedRouteHint(parsed); + if (vettedHint) { + reason += `\n\n${vettedHint}`; + } if (trackerFallbackHint) { // The Bash tool call carries no cwd, so a command run in the tracker checkout cannot be // told apart from one run after an earlier `cd ../`; an explicit target can. diff --git a/plugins/magpie-security/skills/issue-sync/github-advisory.md b/plugins/magpie-security/skills/issue-sync/github-advisory.md index 66a119085..bb60e04ab 100644 --- a/plugins/magpie-security/skills/issue-sync/github-advisory.md +++ b/plugins/magpie-security/skills/issue-sync/github-advisory.md @@ -115,6 +115,8 @@ It returns: The patch is one Step 4 proposal item per advisory; apply it with `gh api --method PATCH repos//security-advisories/ --input `. +Where the public-surface guard runs, it blocks that `PATCH` whenever the patch names a CVE (a `cve_id` always does), and no vetted op edits advisory fields: +hand the operator the advisory `html_url` and the field values to set in the web UI, and record the change only after re-reading the advisory. ### Two-way comment sync — ` ghsa-mirror` @@ -137,6 +139,8 @@ Both are proposals: the tracker copies are one Step 4 item, and **each** advisor since it reaches the reporter at once and cannot be deleted through the API. Apply the copies with `gh issue comment --repo --body-file ` or the `comment-update` vetted op, and the advisory posts with the `advisory-comment` vetted op (or `gh api --method POST …/comments -F body=@`; an edit is `--method PATCH …/comments/`). +The public-surface guard blocks the plain `gh api` forms when the text names a CVE or the advisory, so where it runs the vetted op is the only route; +it needs `advisory-comment` in the caller's `[callers]` list of the adopter's vetted-ops config, and when that grant is missing the operator adds it — the agent does not. ### Read the advisory discussion