From 364d7524d412885e215a79270bf50dc643479082 Mon Sep 17 00:00:00 2001 From: debuggingfuture Date: Sun, 23 Aug 2026 00:01:23 +0800 Subject: [PATCH] fix(github-app): the manifest grants the writes the runs actually do MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both manifest copies declared `contents: read` and no `issues` at all, so a fresh install of this App could not open a pull request or file an issue — which is to say it could not produce the one artifact every proposing run exists to produce. `pull_requests: write` only covers opening a PR once a branch exists. Creating the branch and committing the files is `contents`, and at `read` every `openDraftPullRequest` returns 403. The live `fractalboxdev` install was raised to `contents: write` + `issues: write` on 2026-08-16 after `org-spec-audit` produced four real questions and could not file them; this brings the manifest that provisions every OTHER install into line with what the code does. Two things made this survive review for so long, and both are worth stating because neither is visible in a diff: `contents: read` reads as a virtuous least-privilege default. It is the shape of a correct decision. Nothing about it announces that the repo's entire Act path is downstream of it. And the failure could not turn anything red. A failed PR write is deliberately never fatal — a notice or proposal that cannot be delivered must not fail a run that already did its work — so the run completed green with its output discarded. A write the App can never perform, on a path that never reports failure, produces silence indistinguishable from a quiet week. That is the same shape as an unset config key resolving an empty estate, and it is the third time this repo has been bitten by a read or write that degrades instead of failing. `members: read` is deliberately NOT added. The live App carries it, but the only membership check in this codebase — `deploy-authz` — reads its groups from Cloudflare Access's GitHub IdP via `get-identity`, never from an App token. Propagating it would make every new install request a permission nothing uses. It should probably come off the live App too, but revoking a grant on a live install is its own change. Worth knowing for next time: the parity test asserts the two committed copies match EACH OTHER and never compares either to the live App, so App-vs-manifest drift is silent in both directions. That is how the live install and the manifest disagreed about two permissions with every check green. --- apps/dispatcher/src/routes/github.ts | 15 ++++++++++++++- infra/github-app-manifest.json | 10 ++++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/apps/dispatcher/src/routes/github.ts b/apps/dispatcher/src/routes/github.ts index 58da9da..6047367 100644 --- a/apps/dispatcher/src/routes/github.ts +++ b/apps/dispatcher/src/routes/github.ts @@ -85,8 +85,21 @@ export const MANIFEST_TEMPLATE = { public: false, default_permissions: { checks: "write", - contents: "read", + // `write`, not `read`: every run whose output is a pull request has to + // create a branch and commit files, and that is `contents:write` — the + // `pull_requests` grant below only covers opening the PR once a branch + // exists. Held at `read` this reads as a sane least-privilege default and + // is in fact the difference between a run that proposes and a run that + // cannot: `openDraftPullRequest` 403s, and because a failed notice or PR is + // never fatal, the run still reports green. That combination — a write the + // App can never do, on a path that never turns red — is why this went + // eight days without anyone noticing on the live install. + contents: "write", deployments: "read", + // `org-spec-audit` files one issue per open question, and the triage desk's + // issue half needs the label writes. `issues` was absent entirely, so a + // fresh install could not file a question at all. + issues: "write", metadata: "read", // `write`, not `read`: the check-run callback (workflow.ts) creates/updates // check-runs AND the `pr-review` run posts a PR review comment — both need diff --git a/infra/github-app-manifest.json b/infra/github-app-manifest.json index e8ae788..de77c83 100644 --- a/infra/github-app-manifest.json +++ b/infra/github-app-manifest.json @@ -9,10 +9,16 @@ "public": false, "default_permissions": { "checks": "write", - "contents": "read", + "contents": "write", "deployments": "read", + "issues": "write", "metadata": "read", "pull_requests": "write" }, - "default_events": ["check_run", "check_suite", "deployment_status", "pull_request"] + "default_events": [ + "check_run", + "check_suite", + "deployment_status", + "pull_request" + ] }