From 8c7af4592688709a1ab4968b724d0dc8fbd89d4c Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 29 Oct 2026 01:00:11 -0400 Subject: [PATCH 1/2] docs(security-advisory): bring the fix to v2/main through an opaque public PR The skill and AGENTS.md said an advisory's draft card converts to an issue only after publication, so the public PR that actually brings the fix to v2/main had no issue to close and no card to track it. Describe the flow as it is run: convert the card to an issue that keeps its opaque title when the fix is ready to merge, push the fork's branch under the same opaque name, open a public PR that closes that issue with no Copilot loop, merge, close the fork's PR, and give the issue its public title only after publication. Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- .claude/skills/board-ops/SKILL.md | 6 +- .claude/skills/release/SKILL.md | 4 +- .claude/skills/security-advisory/SKILL.md | 181 +++++++++++++++------- AGENTS.md | 50 +++--- docs/ai-software-factory.md | 5 +- scripts/release-notes.mjs | 11 +- 6 files changed, 175 insertions(+), 82 deletions(-) diff --git a/.claude/skills/board-ops/SKILL.md b/.claude/skills/board-ops/SKILL.md index 02808f21fa..bb75edece9 100644 --- a/.claude/skills/board-ops/SKILL.md +++ b/.claude/skills/board-ops/SKILL.md @@ -299,8 +299,10 @@ fi # Then the two chained item-edit calls from "Add a card and set its fields". ``` -**Convert** it to an issue only after the advisory is **published**. The web UI -has "Convert to issue" on the card; the API equivalent is: +**Convert** it to an issue when its fix is ready to merge, keeping its opaque +title; a follow-up card converts only after the advisory is **published**. When +each happens, and what the issue gets, is the `/security-advisory` flow. The web +UI has "Convert to issue" on the card; the API equivalent is: ```sh REPO_ID=$(gh api repos/modelcontextprotocol/servers --jq .node_id) || REPO_ID= diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index 4f3fe0bc45..4138280cf4 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -539,8 +539,8 @@ added to the draft by hand: - **Every advisory this release fixes**, as `--advisory GHSA-xxxx-yyyy-zzzz`, once each: the `[GHSA-…]` cards on the board for the milestone's work - (`security-advisory`). Its fix shipped from a private fork with no public - issue, so its reporters are no issue's author. A draft advisory works. + (`security-advisory`). Its issue is converted from the card by a + maintainer, so its reporters are no issue's author. A draft advisory works. - **Every outside contributor credited on one of the milestone's issues**, as `--thank "@login (#PR), , for #ISSUE"`, once per line of the prototype paragraph. The preview lists each candidate (below). diff --git a/.claude/skills/security-advisory/SKILL.md b/.claude/skills/security-advisory/SKILL.md index f63f22317d..d847d0670a 100644 --- a/.claude/skills/security-advisory/SKILL.md +++ b/.claude/skills/security-advisory/SKILL.md @@ -26,15 +26,18 @@ things safe to say in public about the backlog are **aggregate counts** (how many are in each state). A branch named after the bug, or a commit message that describes it, discloses it as surely as an issue would. -Related: `/board-ops` holds the board IDs and the draft-card recipes, and -`/issue-create` gives public tracking its labels and milestone **after -publication**, never merely after the release (step 6). - -⚠️ **The PR flow does not apply to the fix itself.** It needs a public issue -and a public PR against `v2/main`, which is the disclosure this flow exists to -delay. The fix is reviewed inside the private fork (step 4), and `AGENTS.md` -records that fork PR as an exception to its PR rules (no public issue link, no -labels, tracked by the draft card). +Related: `/board-ops` holds the board IDs and the draft-card recipes, +`/issue-create` gives the converted issue its labels and milestone (step 5), +and `/pr-flow` opens the public PR that brings the fix to `v2/main` (step 5). + +⚠️ **The fix is written and reviewed in the private fork, then merged through +an opaque public PR.** The fork's PR (step 4) links no public issue, carries no +labels and is tracked by the draft card; it is never merged. When the fix is +ready to merge, the card converts to an issue that keeps its opaque title, and +a public PR from the same opaque branch closes it (step 5). That PR follows +`/pr-flow` except that **nothing in it describes the vulnerability** and it +gets **no Copilot review**. `AGENTS.md` records both PRs as an exception to +its PR rules. ## The flow @@ -44,8 +47,8 @@ labels, tracked by the draft card). | 2 | **Verify the claim**: which server, and does the code belong to it or to an SDK? | Judgment | | 3 | Valid and ours → **accept** (`triage` → `draft`); otherwise close with a reason | **Human only** | | 4 | Create the **private fork**, fix and review there | Mechanical | -| 5 | Merge to `v2/main`, milestone merge, **release**, close the fork's PR, then **publish** | **Human only** (closing the fork's PR is mechanical) | -| 6 | **After publication**, convert the draft card into a public issue | Mechanical | +| 5 | Close to the release: **convert the card** to an opaque issue, open the opaque **public PR**, merge it, close the fork's PR; then milestone merge, **release**, **publish** | **Human only** for merging, releasing and publishing (converting, the PR and closing the fork's PR are mechanical) | +| 6 | **After publication**, give the issue its public title and the advisory link | Mechanical | There is **one release line**. Every fix lands on `v2/main` and ships through the next milestone merge to `main` and a release, as any other change does @@ -197,7 +200,8 @@ moves the advisory `triage` → `draft`. The API shows it as `state` and normally assigning a milestone, and a draft card cannot carry one, so for an advisory draft **accepting the advisory is the approval**. `AGENTS.md` records this as the advisory exemption to its `Incoming` ⇔ milestone rule. The milestone -arrives with the public issue in step 6, where the ordinary rule resumes. +arrives when the card converts to an issue in step 5, where the ordinary rule +resumes. **Invalid, out of scope, a duplicate, or an SDK's → close** with a comment saying which, and why; for an SDK's, say where it was routed. A reporter who is @@ -249,8 +253,10 @@ any other change: a regression test that fails before the fix, and the gate under **Before pushing**. Name the branch `v2/fix/` (lowercase, **no descriptive slug**, since a branch name outlives the fork once merged), and keep commit messages equally opaque until -publication. **The fork's PR targets `v2/main`**, never `main`: check its base -before anyone merges it. +publication. **The fork's PR targets `v2/main`**, never `main`, so the diff +reviewed there is the diff that lands. It is where the review happens; it is +**not merged**. The same branch is pushed to this repository and merged +through a public PR (step 5), and the fork's PR is then closed. ⚠️ **Move the card as the work moves.** `In Progress` when the fix is started, `In Review` when the fork's PR is open. The fork is invisible to everyone not on @@ -290,8 +296,10 @@ body nobody will read again. already accepted, and asking for the work approves it. Otherwise it goes in `Incoming`. Score its Priority with step 1's table, using the severity of the residual exposure rather than the advisory's. -- **Say in the fix PR's body what the fix leaves open**, and link the - follow-up. The advisory's text must say it too when it is published (step 5). +- **Say in the fork PR's body what the fix leaves open**, and name the + follow-up card. The public PR (step 5) says nothing about it, since + describing the gap describes the vulnerability. The advisory's text must say + it too when it is published (step 5). Create it with `/board-ops`'s draft-card recipe, using the follow-up title in place of `[$GHSA]`. @@ -308,35 +316,111 @@ of three cases: - It is non-security work outside the advisory's scope, which gets a public issue **after publication**, like any other finding. -### 5. Merge, release, publish (human only) +### 5. Bring the fix to `v2/main`, release, publish Publish **after** the fix has shipped in a release, never before: publishing discloses the vulnerability, and doing it while users have no upgrade hands out a working exploit. **Shipped** means the fixed version of that server is on npm or PyPI. -⚠️ **The patch stops being secret at MERGE, not at publish.** Merging the fork -puts an ordinary public commit on `v2/main`, readable by anyone, and it then -travels through the milestone merge to `main` before `release.yml` publishes it. -The time between merge and publish is not a period of secrecy; it is a period -of **exposure to anyone reading commits**, so keep it short: merge close to the -release, and publish as soon as the release is out. +⚠️ **The patch stops being secret when its branch reaches this repository, not +at publish.** Pushing the fork's branch here (below) makes its commits readable +by anyone, the public PR shows the diff, and the merge then travels through the +milestone merge to `main` before `release.yml` publishes it. The time between +that push and the publish is not a period of secrecy; it is a period of +**exposure to anyone reading commits**, so keep it short: start this step close +to the release, merge as soon as CI passes, and publish as soon as the release +is out. The release itself is a maintainer action (`release.yml` runs only when a maintainer publishes a GitHub Release, behind the `release` environment's approval; see [`RELEASING.md`](../../../RELEASING.md)). Say which server needs a release and stop there. -⚠️ **Publishing is irreversible and human-only.** It makes the advisory public, -with no undo. Same rule as accepting: recommend, never perform. +⚠️ **Merging, releasing and publishing are human-only.** Publishing is also +irreversible: it makes the advisory public, with no undo. Same rule as +accepting: recommend, never perform. Converting the card, opening the public +PR and closing the fork's PR are mechanical. + +#### Convert the card to an opaque issue + +When the fix's review in the fork is done and the release is near, convert the +draft card to an issue, so the public PR has an issue to close and a card that +tracks it. + +⚠️ **Convert the draft; do not file a new issue.** "Convert to issue" creates a +**new** issue from the draft card, and there is no way to point an existing card +at an issue filed separately, so filing by hand and then converting produces two +issues and two cards. + +1. **Keep the opaque title**, `[GHSA-xxxx-yyyy-zzzz]`, and the + link-and-triage body (step 1). The issue takes the card's title and body, + and the advisory is still private, so no summary may appear yet (step 6 + adds it). The advisory link in the body opens only for people on the + advisory. +2. **Convert the draft card to an issue** in `modelcontextprotocol/servers` + (the card keeps its place and its field values). The recipe is in + `/board-ops`. +3. Apply the labels and milestone from `/issue-create` (steps 1 and 2): `v2`, + one type label (normally `bug`), the server's scope label, and the milestone + of the release the fix will ship in. **Skip its board step**: the converted + card is already on #43. + +A **follow-up card stays a draft** here: its write-up describes the gap, so it +converts only after publication (step 6). + +#### Open the public PR + +Push the fork's branch to this repository under the **same opaque name**, +`v2/fix/`, and open the PR with `/pr-flow` steps 4 and 6. The fork has +no CI, so run the gate on exactly what you push: + +```sh +GHSA= +BRANCH="v2/fix/$(tr '[:upper:]' '[:lower:]' <<<"$GHSA")" +FORK=$(gh api repos/modelcontextprotocol/servers/security-advisories/$GHSA \ + --jq '.private_fork.full_name // empty') || FORK= +[ -n "$FORK" ] || echo "no private fork for $GHSA (or the lookup failed)" >&2 +WT="../$(basename "$PWD")-${BRANCH#v2/fix/}" +git fetch origin v2/main +# The push runs only after the gate exits 0, from the worktree holding the fork's head. +[ -n "$FORK" ] && git fetch "https://github.com/$FORK.git" "$BRANCH" \ + && git worktree add --detach "$WT" FETCH_HEAD \ + && cd "$WT" && npm install && npm run local:gate \ + && git push origin "HEAD:refs/heads/$BRANCH" +``` + +If the branch no longer merges cleanly onto `origin/v2/main`, resolve that in +the fork, where the review is, before pushing. + +- **Title**: `fix(): GHSA-xxxx-yyyy-zzzz`, and nothing more. +- **Body**: `Closes #` (the converted issue) on the first line, then + `/pr-flow`'s sections, **describing no part of the vulnerability**: what it + is ("the fix for GHSA-xxxx-yyyy-zzzz, reviewed in the advisory's private + fork; details stay with the advisory until it is published"), the gate's + result, and each checklist item answered. Client evidence stays with the + advisory; say so under **How Has This Been Tested?** rather than recording + scenarios that describe the attack. +- Label it `v2`, link it with `addCloseIssueReferences` and read the link back + (`/pr-flow` step 6), and move the card to **In Review** if it is not there + already. +- ⚠️ **No Copilot review: skip `/pr-flow` steps 7 to 9.** Review comments on a + public PR are public, and a reviewer's findings would describe the + vulnerability. The review happened in the fork. Do not request one. + +#### Merge it, then close the issue + +A maintainer merges it once CI passes. Then close the issue by hand and move +its card to **Done** (`/pr-flow` step 10), as for any PR into `v2/main`: +`Done` means the work shipped, and it has. #### Close the fork's PR once its fix is on `v2/main` **GitHub will not publish an advisory while its private fork has an open pull request.** The advisory page says "you must merge or close outstanding pull requests" and **Publish advisory** stays disabled. The fix reaches `v2/main` -through a PR in this repository, so the fork's own PR is left open with -nothing still to merge, and blocks publication until someone closes it. +through the public PR above, so the fork's own PR is left open with nothing +still to merge, and blocks publication until someone closes it. Closing it is mechanical and reversible (a PR can be reopened), and it neither closes nor publishes the advisory. Close a PR **only once its head is on @@ -381,9 +465,10 @@ fork, so check each of these first: - **Nothing that matters lives only in the fork.** Publishing deletes it. Review findings worth keeping, a residual-exposure note, a follow-up's write-up: copy each onto the advisory (if it belongs in what is published) or - onto the draft card that tracks it (a follow-up's write-up goes in **its - follow-up card's** `## Write-up` section, where step 6 reads it), within step - 4's confidentiality limits, before the button is pressed. + onto the follow-up draft card that tracks it (a follow-up's write-up goes in + **its follow-up card's** `## Write-up` section, where step 6 reads it), within + step 4's confidentiality limits, before the button is pressed. Never onto the + converted issue, which is public. - **The affected product**: ecosystem (`npm` or `pip`) and package name, as published (`@modelcontextprotocol/server-filesystem`, `mcp-server-git`). @@ -402,33 +487,25 @@ fork, so check each of these first: platforms or cases are still exposed, so a user who upgrades does not believe they are fully covered. -### 6. After publication, convert the card into public tracking +### 6. After publication, give the issue its public title -**The trigger is publication, not the release.** The release ships the fix -while the advisory can still be private, and a public issue opened in that gap +**The trigger is publication, not the merge or the release.** Both happen while +the advisory can still be private, and a summary added to the issue in that gap describes a vulnerability the advisory has not disclosed yet. -⚠️ **Convert the draft; do not file a new issue.** "Convert to issue" creates a -**new** issue from the draft card, and there is no way to point an existing card -at an issue filed separately, so filing by hand and then converting produces two -issues and two cards. +1. **Retitle the issue** `[GHSA-xxxx-yyyy-zzzz] - `. Only + now, with the advisory public, may the summary appear. +2. **Replace the body's triage lines** with a link to the now-public advisory. -1. **Give the card its public title first**, `[GHSA-xxxx-yyyy-zzzz] - ` (edit the draft's title on the card), since the issue takes the - card's title. Only now, with the advisory public, may the summary appear. -2. **Convert the draft card to an issue** in `modelcontextprotocol/servers` - (the card keeps its place and its field values). The recipe is in - `/board-ops`. -3. Apply the labels and milestone from `/issue-create` (steps 1 and 2): `v2`, - one type label (normally `bug`), the server's scope label, and the milestone - of the release the fix shipped in. **Skip its board step**: the converted - card is already on #43. -4. Replace the body's triage lines with a link to the now-public advisory, and - **close** the issue: the work shipped before the issue existed. -5. Move the card to **`Done`**, which is right here because the fix shipped. +```sh +gh issue edit --repo modelcontextprotocol/servers \ + --title "[GHSA-xxxx-yyyy-zzzz] - " --body-file +``` + +The issue is already closed and its card in `Done` (step 5); neither changes. -**A follow-up card converts at the same time, but stays open**, because its -work has not shipped: +**A follow-up card converts now, and stays open**, because its work has not +shipped: 1. Retitle it `[GHSA-xxxx-yyyy-zzzz follow-up] - `. 2. Convert it. diff --git a/AGENTS.md b/AGENTS.md index 8fe94f01a7..a823456c1c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -359,15 +359,22 @@ every commit a pull request adds; one unsigned commit fails it. The repair work is invisible to the board. If there is no issue yet, create it first. This holds for a maintainer's own one-line fix as much as for a feature. -- **Exception: a security advisory's fix, in its private fork.** A PR in the - advisory's temporary private fork (`servers-ghsa-xxxx-yyyy-zzzz`) links no - public issue, carries no labels, and is tracked by the advisory's `[GHSA-…]` draft - card rather than an issue's card, because each of those would disclose the - vulnerability before a fix ships. For the same reason the work-start rules - on assigning an issue and naming the branch `v2//-` - do not apply: its branch is `v2/fix/`, with **no descriptive - slug**. It still targets `v2/main`, and it is reviewed inside the fork. The - flow is the `security-advisory` skill. +- **Exception: a security advisory's fix.** It is written and reviewed in a + PR in the advisory's temporary private fork (`servers-ghsa-xxxx-yyyy-zzzz`), + which links no public issue, carries no labels, and is tracked by the + advisory's `[GHSA-…]` draft card rather than an issue's card, because each + of those would disclose the vulnerability before a fix ships. For the same + reason the work-start rules on assigning an issue and naming the branch + `v2//-` do not apply: its branch is + `v2/fix/`, with **no descriptive slug**. It targets `v2/main`, and + it is **not merged**. When the fix is ready to merge, close to the release, + the draft card converts to an issue that keeps its opaque title, and that + branch is pushed here under the same name and merged through a **public PR** + that `Closes` that issue and follows every rule above, except that its + title and body describe no part of the vulnerability and it gets **no + Copilot review loop**: review comments on a public PR are public, and the + review already happened in the fork. The fork's PR is then closed. The flow + is the `security-advisory` skill. - **Exception: a milestone's merge PR.** It is the one PR that targets `main`. It is a pure merge of `v2/main` (its branch is `origin/v2/main` with no @@ -480,13 +487,16 @@ here. card** titled with the bare id, `[GHSA-xxxx-yyyy-zzzz]`, because a real issue would disclose the vulnerability before a fix exists. The title stays that opaque until publication: board access is wider than advisory access, - and a summary can name the server or the attack. It becomes a public - issue by conversion **once the advisory is published**, never merely once - the fix ships. Work the fix leaves open is tracked by a second draft card, - `[GHSA-xxxx-yyyy-zzzz follow-up]`, under the same rules. It stays open - after conversion, because its work has not shipped. The `[GHSA-` prefix is - load-bearing: **any other draft card is still a defect to delete**. The flow - is the `security-advisory` skill. + and a summary can name the server or the attack. It converts to an issue + **when its fix is ready to merge**, keeping that opaque title, so the + public PR that brings the fix to `v2/main` has an issue to close; the + summary joins the title only **once the advisory is published**, never + merely once the fix ships. Work the fix leaves open is tracked by a second + draft card, `[GHSA-xxxx-yyyy-zzzz follow-up]`, under the same rules, except + that it converts only once the advisory is published, since its write-up + describes the gap. It stays open after conversion, because its work has + not shipped. The `[GHSA-` prefix is load-bearing: **any other draft card + is still a defect to delete**. The flow is the `security-advisory` skill. **Accepting, closing and publishing an advisory, and replying to its reporter, are human-only; never automate or bulk-apply any of them.** - **Only issues go on the board, never PRs.** A PR is tracked through its linked @@ -511,8 +521,8 @@ here. approval. A `[GHSA-` advisory draft is exempt: a draft card cannot carry a milestone, so its approval act is **accepting the advisory**, which moves it - `Incoming` → `Todo`. Its milestone arrives with the public issue after - publication. + `Incoming` → `Todo`. Its milestone arrives when the card converts to an + issue, before the fix's public PR opens. - **`Done` means the work shipped**: its PR merged, or it is a parent whose last sub-issue closed. A duplicate, won't-fix, not-planned or superseded issue shipped nothing, so its card is **deleted** rather than moved to Done. @@ -573,7 +583,9 @@ here. so **link it explicitly** with the `addCloseIssueReferences` GraphQL mutation right after opening it. **On merge, close the issue by hand and move its card to Done.** Keep the `Closes` line anyway. -- **After opening a PR, run a Copilot review loop to convergence, unprompted.** +- **After opening a PR, run a Copilot review loop to convergence, unprompted** + (except a security advisory's public fix PR; see + [Contributing](#contributing)). Request a review, wait for the round to post or for Copilot's session to end without one, answer every comment, and request again only when you pushed a fix or the last round ended without a review. Stop on the **first** clean diff --git a/docs/ai-software-factory.md b/docs/ai-software-factory.md index e0d5356ead..85e46296ab 100644 --- a/docs/ai-software-factory.md +++ b/docs/ai-software-factory.md @@ -212,8 +212,9 @@ A privately reported vulnerability is tracked by a draft board card titled with the bare advisory id, `[GHSA-xxxx-yyyy-zzzz]`, because a public issue would disclose it before a fix exists. The `security-advisory` skill covers checking whether the code is this repository's or the SDK's underneath, the -private fork the fix is built in, and turning the card into public tracking -once the advisory is published. **Accepting, closing and publishing an +private fork the fix is built in, converting the card to an issue with the +same opaque title so an opaque public PR can bring the fix to `v2/main`, and +giving that issue its public title once the advisory is published. **Accepting, closing and publishing an advisory, and replying to its reporter, are human-only acts**, never automated or bulk-applied. diff --git a/scripts/release-notes.mjs b/scripts/release-notes.mjs index fe5eb39362..046473de83 100644 --- a/scripts/release-notes.mjs +++ b/scripts/release-notes.mjs @@ -38,11 +38,12 @@ // Two kinds of credit never come from an issue's author, and were once pasted // in by hand, which is how v2.0.0 published without its Thanks (#5250): // -// * An advisory's fix ships from its private fork with no public issue, so -// each `--advisory GHSA-…` is read for its credits (a draft works: notes are -// drafted before advisories publish). Declined credits, bots and -// maintainers are dropped; a credit still pending acceptance is listed and -// flagged, since the reporter may yet decline it. +// * An advisory's issue is converted from its board card by a maintainer, +// never filed by its reporter, so each `--advisory GHSA-…` is read for its +// credits (a draft works: notes are drafted before advisories publish). +// Declined credits, bots and maintainers are dropped; a credit still +// pending acceptance is listed and flagged, since the reporter may yet +// decline it. // * An outside PR is closed and its author credited on an issue (Harvest), so // each `--thank "@login (#ref), "` is a line of the prototype // paragraph. Which issues carry such a credit is a reading, not a lookup, From 11f64f981679fa073c65becef1d83d43d4082653 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 29 Oct 2026 01:18:24 -0400 Subject: [PATCH 2/2] docs(security-advisory): pass --head when opening the public PR from a detached HEAD Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- .claude/skills/security-advisory/SKILL.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.claude/skills/security-advisory/SKILL.md b/.claude/skills/security-advisory/SKILL.md index d847d0670a..a591c51c4a 100644 --- a/.claude/skills/security-advisory/SKILL.md +++ b/.claude/skills/security-advisory/SKILL.md @@ -393,6 +393,9 @@ git fetch origin v2/main If the branch no longer merges cleanly onto `origin/v2/main`, resolve that in the fork, where the review is, before pushing. +The worktree is on a detached `HEAD`, so `gh pr create` cannot infer the +branch: add `--head "$BRANCH"` to `/pr-flow`'s create command. + - **Title**: `fix(): GHSA-xxxx-yyyy-zzzz`, and nothing more. - **Body**: `Closes #` (the converted issue) on the first line, then `/pr-flow`'s sections, **describing no part of the vulnerability**: what it