diff --git a/.github/workflows/pr-risk.yml b/.github/workflows/pr-risk.yml index 4aa81d9..b0cf113 100644 --- a/.github/workflows/pr-risk.yml +++ b/.github/workflows/pr-risk.yml @@ -258,8 +258,13 @@ on: bot_logins: description: >- Extra logins treated as bots, on top of any `[bot]`-suffixed login - (comma-separated). A bot with no runbook registry entry grades as - human — identity alone never buys trust. + and any author GitHub itself types as a `Bot` (comma-separated). A + real GitHub App needs no entry here; a machine USER account does. A + bot with no runbook registry entry grades as human — identity alone + never buys trust. LOAD-BEARING, not a hint: a listed login skips the + first-time-contributor test, moving a non-fork NONE PR from `external` + (R3) to `human` (R1), and nothing validates that the login really is a + machine account. List only accounts you control; prune retired ones. type: string required: false default: github-actions,dependabot,renovate,coderabbitai,cursor,comfy-pr-bot,web-flow diff --git a/docs/callers/pr-risk.md b/docs/callers/pr-risk.md index 7d6e9e5..6ac9046 100644 --- a/docs/callers/pr-risk.md +++ b/docs/callers/pr-risk.md @@ -78,8 +78,8 @@ statuses: read | Input | Default | Notes | |---|---|---| | `workflows_ref` | — (**required**) | Pin to the SAME full commit SHA as `uses:`. No default on purpose: a floating default let a caller SHA-pin `uses:` and still load the grader from HEAD of main. | -| `fleet_logins` | `mattmillerai` | Logins whose PRs grade provenance `agent-supervised` alongside `agent-coded`. | -| `bot_logins` | `github-actions,dependabot,renovate,coderabbitai,cursor,comfy-pr-bot,web-flow` | Extra logins treated as bots. A bot with no runbook entry still grades as human — identity alone buys no trust. | +| `fleet_logins` | `mattmillerai` | Logins whose PRs grade provenance `agent-supervised` alongside `agent-coded`. Both are read for **human** authors only: an author GitHub types as a `Bot` is a runbook candidate regardless, so listing a bot here (or labelling its PR) buys it nothing — only a registry entry that asserts can promote it. | +| `bot_logins` | `github-actions,dependabot,renovate,coderabbitai,cursor,comfy-pr-bot,web-flow` | Extra logins treated as bots. Needed only for **machine USER accounts** — a real GitHub App is recognized from GitHub's own actor type, no list entry required. A bot with no runbook entry still grades as human — identity alone buys no trust. **This list is load-bearing, not a hint:** a listed login skips the first-time-contributor test, so it moves a non-fork `NONE`/`FIRST_TIME_CONTRIBUTOR` PR from `external` (R3) to `human` (R1). Nothing validates that a listed login is really a machine account, so add one only for an account you control, and remove it when it is retired. | | `label_map` | `''` | Rename the five grader-owned labels as `tier=label` pairs. Tier keys are fixed; only the label text is yours. | | `wait_for_checks_minutes` | `10` | How long to wait for the rest of the check rollup to settle before labeling (clamped to 25 — what a 30-minute job can spend waiting). `0` labels immediately, expect R2 floors from still-pending checks. | | `repo_map_path` | `.github/risk.json` | Consumer risk-map override, read from the PR **base ref**. | diff --git a/scripts/pr-risk/README.md b/scripts/pr-risk/README.md index 9ce5413..28a2b0e 100644 --- a/scripts/pr-risk/README.md +++ b/scripts/pr-risk/README.md @@ -34,9 +34,16 @@ record. 2. **Provenance** — what PROCESS produced the diff: `runbook` (a registered producer in [`runbook-registry.v0.json`](runbook-registry.v0.json) whose identity AND diff shape both assert), `agent-supervised`, `human`, or - `external` (fork / first-time contributor — R3, no exceptions, even when a - runbook shape matches). Identity is the server-attributed author login, - never the forgeable commit author string. + `external` (any fork, or a first-time **human** contributor — R3, no + exceptions, even when a runbook shape matches). Identity is the + server-attributed author login, never the forgeable commit author string. + A **non-fork bot is never `external`**, even though every GitHub App authors + with `author_association: NONE` — it is a runbook candidate, judged by the + registry's shape assertion, and falls back to `human` when no entry asserts. + "Is this a bot?" is answered from GitHub's own actor type (`Bot`) plus the + `bot_logins` list, so an App needs no list entry and a machine *user* account + does. The fork half is unconditional, so a bot opening a PR *from a fork* is + still `external` R3. 3. **Reversibility** — mutates persistent state or deletes data → R3; **removes** a file under a sensitive class → R3 (a delete, or a rename out of that class); no green check rollup → R2; green but no test file touched → R1; green with @@ -177,7 +184,14 @@ them by committing: - `.github/risk.json` — the repo's own path→tier map (same schema as [`risk-map.v0.json`](risk-map.v0.json)) - `.github/risk-runbooks.json` — the repo's own producer registry (same schema - as [`runbook-registry.v0.json`](runbook-registry.v0.json)) + as [`runbook-registry.v0.json`](runbook-registry.v0.json)). **Name a GitHub App + by its suffixed login** — `"my-bot[bot]"`, the form REST and the web UI show + you. The grader reads the PR over GraphQL, which reports a Bot login + **unsuffixed**, and restores the suffix before matching — one way only, and + only for an author GitHub's own actor type calls a `Bot`. A bare slug + (`"my-bot"`) still matches literally, because a machine **user** account has no + suffix and needs it; but a bare slug is equally matchable by a same-named + *human*, who would then inherit that runbook — so never name an App with one. Both are read from the PR's **base ref**, so a PR cannot edit the rules that judge it (editing them — or the grader — at all is R3 by the map's own first diff --git a/scripts/pr-risk/grade-pr-risk.sh b/scripts/pr-risk/grade-pr-risk.sh index 9797da3..951c22f 100755 --- a/scripts/pr-risk/grade-pr-risk.sh +++ b/scripts/pr-risk/grade-pr-risk.sh @@ -30,7 +30,9 @@ # is the WORST tier over every rule any changed path matches. # AXIS 2 — PROVENANCE. runbook / agent-supervised / human / external. A PR whose identity # matches a runbook but whose DIFF SHAPE does not is not a runbook, and falls back to its -# underlying class. `external` (fork / first-time contributor) is R3 on provenance alone. +# underlying class. `external` (any fork, or a first-time HUMAN contributor) is R3 on +# provenance alone; a non-fork bot is a runbook candidate, not an outsider, because every +# GitHub App authors with `author_association: NONE`. # AXIS 3 — REVERSIBILITY. Single clean revert? Mutates persistent state or deletes data? # Did tests covering the touched lines actually run? Answered from the changed-path list # + change types + the PR's own check rollup. @@ -126,8 +128,8 @@ read_map() { # -> JSON on stdout, rc 0; rc 1 + reaso then "provenance_tiers carries a tier outside \(known)" # EVERY provenance class must be MAPPED, not just well-typed. Checking only the values let # a map that OMITS `external` pass, and the lookup then fell back to a tier of its own - # choosing — silently retiring the "external (fork / first-time contributor) is R3, no - # exceptions" invariant that both the default map comment and the README promise. A class + # choosing — silently retiring the "external (a fork, or a first-time human contributor) is + # R3, no exceptions" invariant that the default map comment and the README promise. A class # nobody mapped is a routing decision nobody made, so it is refused at load time. # (No apostrophes in here: the whole shape program is a single-quoted shell string.) elif ((["runbook","agent-supervised","human","external"] - [.provenance_tiers | keys[]]) | length) > 0 @@ -252,6 +254,32 @@ cat <<'JQ' # runbook — provenance alone is never sufficient. | ($r.author // null) as $author | ($author | classify_login($fleetl; $botl)) as $cls + # IS THE AUTHOR A BOT? The shared resolver answers from the login STRING — a `[bot]` suffix or + # the caller's `bot_logins` list — and on the live path that string is not enough on its own. + # GraphQL reports a Bot actor's login WITHOUT the `[bot]` suffix (`cloud-code-bot`, where REST + # and the web UI both show `cloud-code-bot[bot]`), so the suffix test can only ever fire on a + # REST-shaped or offline-corpus record. `author.__typename == "Bot"` is GitHub's OWN actor + # type for the same account and is not forgeable by the PR, so it is collected alongside the + # login and ORed in here. It is additive: a record without the field (every pre-schema-4 + # corpus row) falls back to the resolver's answer exactly as before, and `bot_logins` still + # works for machine USER accounts, which are typed `User` and carry no suffix at all. + # Deliberately kept OUT of classify_login: that jq is shared verbatim with agent-work's + # grade-collect.sh, and a field only this collector emits would fork the shared definition. + # + # `== true`, NOT jq truthiness. This is the one field that can switch the `external` guard + # off, and in jq the string "false", 0 and "" are all truthy — so a foreign collector, or a + # corpus row someone hand-edited, could turn a stringly-typed "false" into a confident + # not-an-outsider. Only the literal boolean counts; anything else falls back to the login. + # + # NO STATUS TWIN HERE, deliberately, and it is the only provenance input without one. The + # twins exist where the un-collected default would ANSWER — reading an absent `is_fork` as + # "not a fork" retires `external => R3`. This default runs the other way: absent means "not + # known to be a bot", which sends the author BACK through the association test and grades it + # the stricter way, exactly as a pre-schema-4 record graded before. Nothing is retired, so + # there is nothing to refuse to answer. A schema-3 corpus row therefore still grades an App + # PR `external` where a live schema-4 grade now says `human`: that is a versioned behaviour + # change, which is what `schema_version` and `map_version` are on the record to express. + | ((($r.author_is_bot // false) == true) or $cls == "bot") as $is_bot | (($r.labels // []) | index("agent-coded") != null) as $agent_coded # The label list has a STATUS TWIN for the same reason the file list does: `agent-coded` is # read from it, and a TRUNCATED label list answers "is this agent-coded?" with a confident no @@ -264,11 +292,47 @@ cat <<'JQ' # class never routed unattended — silently unreachable. Unread is `unknown`, and `unknown` # refuses to grade the axis. | ($r.provenance_status // (if ($r | has("is_fork")) then "ok" else "absent" end)) as $pvst + # ORDER IS THE RULE HERE, AND IT IS NOT REARRANGEABLE. The fork test runs FIRST and stays + # unconditional: a fork PR is `external` no matter who authored it, so a bot on a fork can + # never escape R3 by presenting a bot login. Only AFTER that does the author-association half + # get narrowed to authors the shared resolver does NOT read as a bot. + # + # WHY THE NARROWING. A GitHub App is never an org member, so EVERY PR opened by a repo-owned + # App arrives with `author_association: NONE` — and testing that string before the login + # classification pinned every such PR `external` => R3 regardless of what it changed (measured + # on one consumer: 14 of 19 R3 grades in a 24-PR sample came from this, not from the diff). + # It was also unfixable from the consumer side, because both levers sit further down: the + # `bot_logins` input feeds `classify_login`, and `.github/risk-runbooks.json` is read by the + # shape assertion below. A non-fork bot now falls through to `runbook-candidate`, where the + # registry and its shape assertion decide — and a `runbook-candidate` with no asserting entry + # still lands on `human` below, so identity alone continues to buy no trust. + # The first-time-contributor guard is UNCHANGED for humans: `$cls` is "human" (or "unknown") + # for anyone the resolver does not classify as a bot, so a human's NONE still reads external. + # + # AND THE BOT TEST RUNS BEFORE THE LABEL/FLEET TEST, for the same reason `classify_login` + # answers "bot" before it answers "fleet": a bot is a bot no matter what else is true of it. + # Both halves of that branch are reachable for a bot and neither is evidence about a bot: + # * `agent-coded` is a LABEL — anyone with write access applies it, and what it claims is + # that a human supervised an agent. That is a claim about a human author, not about an App. + # * `$cls == "fleet"` is reachable for a bot at all only because `$is_bot` no longer comes + # from the login string alone: a Bot actor whose GraphQL login arrives unsuffixed + # (`cloud-code-bot`) and also appears in `fleet_logins` classifies "fleet" while + # `author_is_bot` says Bot. Leaving `fleet` first lets this use site contradict the + # resolver's own documented bot-beats-fleet ordering. + # What this does NOT change: a REGISTERED producer was never at risk here, because the shape + # assertion below resolves `$rbk != null` to `runbook` ahead of `$base_class` either way. + # What it fixes is the UNREGISTERED bot, which either half would classify `agent-supervised` + # — contradicting this axis's stated promise (and the map's `_why`) that a non-fork bot with + # no asserting entry falls back to `human`. The default map tiers both R1, so nothing moves + # there; the two classes exist so a CONSUMER map can tier them apart, and a consumer that + # trusts its own supervised agents at R0 would otherwise hand R0 to any bot PR someone + # labelled `agent-coded`. Trust in this axis is earned by the shape assertion, never by a label. | (if $pvst != "ok" or $lbst != "ok" then "unknown" - elif ($r.is_fork // false) or (($r.author_association // "") | IN("FIRST_TIME_CONTRIBUTOR","FIRST_TIMER","NONE")) + elif ($r.is_fork // false) then "external" + elif ($is_bot | not) and (($r.author_association // "") | IN("FIRST_TIME_CONTRIBUTOR","FIRST_TIMER","NONE")) then "external" + elif $is_bot then "runbook-candidate" elif $agent_coded or $cls == "fleet" then "agent-supervised" - elif $cls == "bot" then "runbook-candidate" elif $cls == "human" then "human" else "unknown" end) as $base_class # The shape assertion: identity match AND every changed path inside permitted_paths AND the @@ -281,8 +345,23 @@ cat <<'JQ' # login test is therefore REQUIRED, and head_ref_patterns are an ADDITIONAL condition where # the producer declares them. The parentheses are load-bearing — do not let this collapse # back into a bare or/and chain. + # + # AND THE APP'S SUFFIXED LOGIN IS RESTORED BEFORE MATCHING, in ONE direction only. GraphQL + # hands us `cloud-code-bot` where REST and the web UI say `cloud-code-bot[bot]`, so without + # this an entry listing the form a human can actually SEE never asserts on a live grade, and + # the miss is silent — `shape_failures` only records entries whose identity DID match. The + # direction matters and is the whole safety argument: the suffixed form is SYNTHESIZED, never + # stripped, and only when `author_is_bot` — GitHub's own actor type, not the caller's + # `bot_logins` — says the author is a Bot. So an App reaches an `"x[bot]"` entry, and a USER + # account named `x` still cannot: it can only ever present `x`, and nothing here turns + # `"x[bot]"` into `"x"`. That asymmetry is why this is safe where normalizing both sides is + # not. Consequence for registries: an App should be listed by its SUFFIXED login alone. A + # bare slug still matches literally — machine USER accounts have no suffix and need it — but + # a bare slug is also matchable by a same-named human, so never use one to name an App. + | (if $author != null and (($r.author_is_bot // false) == true) and ($author | endswith("[bot]") | not) + then [$author, ($author + "[bot]")] else [$author] end) as $author_forms | ([$RB.runbooks[]? | . as $bk - | select((($bk.identity.logins // []) | any(. as $l | ($author // "") | ascii_downcase == ($l | ascii_downcase))) + | select((($bk.identity.logins // []) | any(. as $l | $author_forms | any(. // "" | ascii_downcase == ($l | ascii_downcase)))) and ((($bk.identity.head_ref_patterns // []) | length) == 0 or (($r.head_ref // "") | matches_any($bk.identity.head_ref_patterns // [])))) | {id: $bk.id, lane: $bk.lane, daily_cap: $bk.daily_cap, @@ -467,7 +546,7 @@ fetch_pr_record() { # -> record JSON on stdout, rc 1 on an unreadab q='query($owner:String!,$name:String!,$num:Int!){ repository(owner:$owner,name:$name){ pullRequest(number:$num){ number title state isDraft createdAt updatedAt closedAt mergedAt - author{ login } authorAssociation baseRefName headRefName isCrossRepository + author{ login __typename } authorAssociation baseRefName headRefName isCrossRepository additions deletions changedFiles labels(first:100){ pageInfo{ hasNextPage } nodes{ name } } commits(last:1){ nodes{ commit{ oid statusCheckRollup{ state @@ -613,7 +692,14 @@ fetch_pr_record() { # -> record JSON on stdout, rc 1 on an unreadab then {list: null, status: "unknown", reason: "changed-file list is short (\($files | length) of \(.changedFiles)) — GitHub caps the files API at 3000 files"} else {list: $files, status: "ok", reason: null} end) as $fread - | {schema_version:3, repo:$repo, pr:.number, title:.title, author:(.author.login // null), + | {schema_version:4, repo:$repo, pr:.number, title:.title, author:(.author.login // null), + # The actor type GitHub itself assigns the author, recorded because the LOGIN cannot answer + # it here: GraphQL reports a Bot login UNSUFFIXED, so `cloud-code-bot[bot]` arrives as + # `cloud-code-bot` and the `[bot]` test in the shared resolver never fires on this path. + # Additive (schema 3 -> 4): the grader reads it as `// false`, so a schema-3 corpus record + # still grades exactly as it did. + # (No apostrophes in here: this whole jq program is a single-quoted shell string.) + author_is_bot:((.author.__typename // "") == "Bot"), author_association:.authorAssociation, is_fork:(.isCrossRepository // false), labels:$labels, agent_coded:($labels | index("agent-coded") != null), labels_status:(if $labels_trunc then "unknown" else "ok" end), diff --git a/scripts/pr-risk/risk-map.v0.json b/scripts/pr-risk/risk-map.v0.json index 5f05718..991a5f5 100644 --- a/scripts/pr-risk/risk-map.v0.json +++ b/scripts/pr-risk/risk-map.v0.json @@ -68,7 +68,7 @@ "agent-supervised": "R1", "human": "R1", "external": "R3", - "_why": "external (a fork / first-time contributor) is R3 on provenance alone — an unreviewed outside diff is the one provenance class never routed unattended. runbook is R0 ONLY when the shape assertion also holds (see the runbook registry); a claimed-but-shape-failed runbook falls back to its underlying class." + "_why": "external (ANY fork, or a first-time HUMAN contributor) is R3 on provenance alone — an unreviewed outside diff is the one provenance class never routed unattended. A non-fork bot is NOT external even though every GitHub App authors with author_association NONE: it is a runbook candidate, graded by the registry's shape assertion, and falls back to human when no entry asserts. runbook is R0 ONLY when the shape assertion also holds (see the runbook registry); a claimed-but-shape-failed runbook falls back to its underlying class." }, "reversibility": { diff --git a/scripts/pr-risk/runbook-registry.v0.json b/scripts/pr-risk/runbook-registry.v0.json index 74dc25d..ac21b2d 100644 --- a/scripts/pr-risk/runbook-registry.v0.json +++ b/scripts/pr-risk/runbook-registry.v0.json @@ -13,6 +13,13 @@ "IDENTITY is the server-attributed PR author login (and, where the producer is a workflow,", "the head-ref shape that workflow pushes) — never the forgeable commit author string.", "", + "NAME A GITHUB APP BY ITS SUFFIXED LOGIN — \"x[bot]\", the form REST and the web UI show you.", + "The grader reads the PR over GraphQL, which reports a Bot login UNSUFFIXED, and it restores", + "the suffix before matching — but only ONE WAY, and only for an author GitHub's own actor", + "type calls a Bot. A bare slug \"x\" still matches literally, because a machine USER account", + "has no suffix and needs it; but a bare slug is equally matchable by a same-named HUMAN, who", + "would then inherit that runbook. So never name an App with one.", + "", "This default registers only ecosystem-universal producers. A consumer repo registers its", "own producers (release bots, spec-sync pushes, caller bumpers) by committing", ".github/risk-runbooks.json; the workflow reads it from the PR's BASE ref, so a PR cannot", @@ -26,8 +33,9 @@ { "id": "dependabot", "why": "Dependabot — the highest-volume automated producer, and the one with the best-known diff shape", - "identity": { "logins": ["dependabot[bot]", "dependabot", "dependabot-preview[bot]"], + "identity": { "logins": ["dependabot[bot]", "dependabot"], "head_ref_patterns": ["dependabot/**"] }, + "_logins_note": "The bare `dependabot` is the ONE sanctioned exception to \"never name an App with a bare slug\", and it is here for the OFFLINE corpus, not the live path: pre-schema-4 rows carry the unsuffixed GraphQL login with no `author_is_bot`, so the grader cannot restore the suffix for them and only a literal entry matches. It costs little — the head_ref_patterns above additionally require a `dependabot/**` branch, so a same-named human would have to push one. Do NOT copy the pattern into a consumer registry.", "_permitted_paths_note": "MUST stay a superset of the `deps` class in risk-map.v0.json. Two manifest lists that must agree are one list too many: when this one is SHORT, a Ruby or PHP dependabot PR fails paths_ok, falls back to `human`, and records a shape_failure that says dependabot failed to look like dependabot. The overall tier does not move — the path floor pins those manifests R3 anyway — but the noise trains readers to skip the one field designed to catch real impersonation.", "permitted_paths": ["go.mod", "go.sum", "**/go.mod", "**/go.sum", "**/go.work*", "**/package.json", "**/package-lock.json", "**/npm-shrinkwrap.json", diff --git a/scripts/pr-risk/tests/test_grade_pr_risk.sh b/scripts/pr-risk/tests/test_grade_pr_risk.sh index c56b72d..cf4bf73 100755 --- a/scripts/pr-risk/tests/test_grade_pr_risk.sh +++ b/scripts/pr-risk/tests/test_grade_pr_risk.sh @@ -8,6 +8,9 @@ # * PROVENANCE ALONE IS NEVER SUFFICIENT: a runbook IDENTITY whose diff SHAPE does not # assert is not a runbook, and the failure is recorded. # * EXTERNAL IS NEVER OVERRIDDEN: a fork imitating a runbook's shape is still external R3. +# * EXTERNAL IS ABOUT FORKS AND FIRST-TIME HUMANS (new here): a repo-owned App authors with +# `author_association: NONE`, so it is a runbook candidate rather than an outsider — while a +# bot on a FORK, and a first-time human, both stay external R3. # * THE UNKNOWN CONTRACT: an unreadable input is tier null + status unknown + exit 1, # never a confident tier; a structurally empty map is refused outright (exit 2). # * THE GRADE CARRIES ITS MAP VERSION so a map revision can be replayed later. @@ -400,6 +403,207 @@ out="$(paged_pr --self-run-id 999)" eq "a PR with no checks is untouched by the drain" ok "$(jq -r '.checks_status // "ok"' <<<"$out")" eq "and still grades" ok "$(jq -r '.risk.status' <<<"$out")" +echo "— phase 21: a repo-owned App is a runbook candidate, not an outsider —" +# A GitHub App is never an org member, so EVERY PR a repo-owned App opens arrives with +# `author_association: NONE`. Testing that string BEFORE the login classification graded every +# such PR `external` => R3 regardless of its diff, and no consumer lever could reach it: the +# `bot_logins` input and .github/risk-runbooks.json are both read further down. The fix narrows +# the association half to non-bots; the FORK half stays unconditional, which is what keeps a bot +# on a fork from presenting a bot login to escape R3. +cat > "$SANDBOX/app-runbooks.json" <<'FIX' +{"registry_version":"v0-test","runbooks":[ + {"id":"data-snapshot-refresh", + "why":"a repo-owned App's cron that refreshes one committed data snapshot", + "identity":{"logins":["cloud-code-bot[bot]"]}, + "permitted_paths":["data/*.json"], + "shape":{"max_changed_files":1,"max_additions":5000,"max_deletions":5000}, + "daily_cap":4,"lane":"data-refresh"}]} +FIX +app_grade() { bash "$GRADER" --stdin --runbooks "$SANDBOX/app-runbooks.json" 2>/dev/null; } +SNAP='[{"path":"data/skills.json","additions":40,"deletions":12,"change_type":"MODIFIED"}]' + +# (1) non-fork App PR, author_association NONE, registry entry asserts -> runbook, R0 on the axis. +out="$(rec 22 'cloud-code-bot[bot]' 'chore: refresh skills snapshot' "$SNAP" ok SUCCESS bot/refresh NONE false | app_grade)" +eq "a non-fork App PR is not external" runbook "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" +eq "and names the runbook that asserted" data-snapshot-refresh "$(jq -r '.risk.axes.provenance.runbook' <<<"$out")" +eq "provenance proposes R0" R0 "$(jq -r '.risk.axes.provenance.tier' <<<"$out")" +# The other axes still decide: an unmapped data path floors R0 and green-but-no-test is R1, so +# the grade now RESPONDS to the diff instead of being pinned R3 by the author's association. +eq "the diff, not the association, decides the grade" R1 "$(jq -r '.risk.tier' <<<"$out")" + +# (2) the SAME App PR from a FORK is still external R3. The fork test runs first and is +# unconditional — reordering the two would open exactly this hole. +out="$(rec 23 'cloud-code-bot[bot]' 'chore: refresh skills snapshot' "$SNAP" ok SUCCESS bot/refresh NONE true | app_grade)" +eq "a bot on a fork is still external" external "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" +eq "and still grades R3" R3 "$(jq -r '.risk.tier' <<<"$out")" + +# (3) a non-fork HUMAN with author_association NONE is untouched: the first-time-contributor +# guard is narrowed to non-bots, not removed. +out="$(rec 24 'drive-by-human' 'feat: my first patch' "$SNAP" ok SUCCESS feature NONE false | app_grade)" +eq "a first-time human contributor is still external" external "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" +eq "and still grades R3" R3 "$(jq -r '.risk.tier' <<<"$out")" + +# (4) identity alone buys NO trust: a bot with no asserting registry entry falls back to human. +out="$(rec 25 'unregistered-bot[bot]' 'chore: something' "$SNAP" ok SUCCESS bot/x NONE false | app_grade)" +eq "an unregistered bot grades human, never runbook" human "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" +eq "human provenance proposes R1, not R0" R1 "$(jq -r '.risk.axes.provenance.tier' <<<"$out")" +# ...and so does a REGISTERED bot whose diff shape does not assert (paths outside its set). +out="$(rec 26 'cloud-code-bot[bot]' 'chore: refresh' '[{"path":"src/evil.go","additions":9,"deletions":0,"change_type":"MODIFIED"}]' ok SUCCESS bot/refresh NONE false | app_grade)" +eq "a registered App failing its shape is human, not runbook" human "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" +sf="$(jq -r '.risk.axes.provenance.shape_failures | length' <<<"$out")" +if [ "$sf" -ge 1 ]; then ok "and the shape failure is recorded"; else bad "and the shape failure is recorded" "$sf"; fi + +# (5) --bot-logins reaches the classification too: a bot login WITHOUT the `[bot]` suffix (a +# machine user, or an App whose login the API reports unsuffixed) is only a bot if the caller +# says so, and it must get the same non-external treatment when it is. +out="$(rec 27 'snapshot-machine' 'chore: refresh skills snapshot' "$SNAP" ok SUCCESS bot/refresh NONE false \ + | bash "$GRADER" --stdin --runbooks "$SANDBOX/app-runbooks.json" --bot-logins snapshot-machine 2>/dev/null)" +eq "a --bot-logins machine user is not external" human "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" +out="$(rec 28 'snapshot-machine' 'chore: refresh skills snapshot' "$SNAP" ok SUCCESS bot/refresh NONE false | app_grade)" +eq "and without --bot-logins it reads as a first-time human" external "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" + +# (6) A LABEL CANNOT STAND IN FOR THE SHAPE ASSERTION. `agent-coded` and `fleet_logins` both +# sat ABOVE the bot test, and both are reachable for a bot only now that the association half +# has stopped pinning every App `external` first — so they are pinned here, alongside that fix. +# A REGISTERED producer was never at risk (the shape assertion resolves to `runbook` ahead of +# the base class either way) — this pins that, then pins the case that WAS wrong: the +# UNREGISTERED bot, which either half classified `agent-supervised`, contradicting the promise +# that a bot with no asserting entry falls back to `human`. The default map tiers both R1, so +# no grade moves here; the classes exist so a CONSUMER map can tier them apart, and a consumer +# that trusts its supervised agents at R0 would otherwise hand R0 to any `agent-coded` bot PR. +out="$(rec 29 'cloud-code-bot[bot]' 'chore: refresh skills snapshot' "$SNAP" ok SUCCESS bot/refresh NONE false \ + | jq -c '.labels = ["agent-coded"]' | app_grade)" +eq "a registered App is runbook with or without the label" runbook "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" +eq "and still earns R0 from the shape assertion" R0 "$(jq -r '.risk.axes.provenance.tier' <<<"$out")" +out="$(rec 30 'unregistered-bot[bot]' 'chore: something' "$SNAP" ok SUCCESS bot/x NONE false \ + | jq -c '.labels = ["agent-coded"]' | app_grade)" +eq "an agent-coded unregistered bot is human, not agent-supervised" human "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" +eq "and human is R1, exactly what agent-supervised was" R1 "$(jq -r '.risk.axes.provenance.tier' <<<"$out")" +# The same for `fleet_logins`. That collision exists only because `author_is_bot` no longer +# comes from the login string: a Bot actor's GraphQL login arrives UNSUFFIXED, so an operator +# who lists it in --fleet-logins makes `classify_login` say "fleet" while GitHub says Bot. The +# resolver's own rule is bot-beats-fleet; this use site must not contradict it. +out="$(rec 31 'cloud-code-bot' 'chore: refresh skills snapshot' "$SNAP" ok SUCCESS bot/refresh NONE false \ + | jq -c '.author_is_bot = true' \ + | bash "$GRADER" --stdin --fleet-logins cloud-code-bot 2>/dev/null)" +eq "a Bot actor in fleet_logins is still read as a bot" human "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" +# ...and with a registry entry that lists the unsuffixed form, the same record reaches `runbook`. +jq '.runbooks[0].identity.logins = ["cloud-code-bot[bot]","cloud-code-bot"]' \ + "$SANDBOX/app-runbooks.json" > "$SANDBOX/app-runbooks-bothforms.json" +out="$(rec 32 'cloud-code-bot' 'chore: refresh skills snapshot' "$SNAP" ok SUCCESS bot/refresh NONE false \ + | jq -c '.author_is_bot = true' \ + | bash "$GRADER" --stdin --runbooks "$SANDBOX/app-runbooks-bothforms.json" --fleet-logins cloud-code-bot 2>/dev/null)" +eq "and the registry, not the fleet list, is what promotes it" runbook "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" +# A HUMAN is untouched by the reorder — the label still classifies a human author. +out="$(rec 33 'a-teammate' 'feat: something' "$SNAP" ok SUCCESS feature MEMBER false \ + | jq -c '.labels = ["agent-coded"]' | app_grade)" +eq "an agent-coded human is still agent-supervised" agent-supervised "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" + +# (7) `author_is_bot` is tested `== true`, NOT for jq truthiness. It is the one field that can +# switch the `external` guard off, and in jq the STRING "false" — what a foreign collector +# writing JSON by hand emits — is truthy. Read loosely, that grades a first-time outsider +# `human` R1 on a field nobody set to true. +for junk in '"false"' '0' '""' 'null'; do + out="$(rec 34 'drive-by-human' 'feat: my first patch' "$SNAP" ok SUCCESS feature NONE false \ + | jq -c ".author_is_bot = $junk" | app_grade)" + eq "author_is_bot=$junk is not a bot — still external" external "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" +done +out="$(rec 35 'drive-by-human' 'feat: my first patch' "$SNAP" ok SUCCESS feature NONE false \ + | jq -c '.author_is_bot = true' | app_grade)" +eq "and only the literal boolean flips it" human "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" + +echo "— phase 22: on the LIVE path the login alone cannot tell you it is a bot —" +# The case above is the corpus/REST record shape. The live path does not see it: GraphQL reports +# a Bot actor login WITHOUT the `[bot]` suffix, so `cloud-code-bot[bot]` arrives as +# `cloud-code-bot` and the resolver's suffix test never fires. Measured on the real PR in the +# ticket: `author.login` is `cloud-code-bot`, `author.__typename` is `Bot`. That typename is +# GitHub's own actor resolution and is what the record carries, so the fix must survive it. +mkdir -p "$SANDBOX/binbot" +jq '.data.repository.pullRequest.author = {"login":"cloud-code-bot","__typename":"Bot"} + | .data.repository.pullRequest.authorAssociation = "NONE" + # phase 12 left changedFiles at 5 on the shared fixture; this stub serves ONE file, and a + # short read is `unknown` by design — so restate it rather than inherit a truncated record. + | .data.repository.pullRequest.changedFiles = 1 + | .data.repository.pullRequest.title = "data: refresh bundled skills snapshot (auto)" + | .data.repository.pullRequest.headRefName = "bot/refresh-skills" + # ...and the ROLLUP STATE with it. Without --self-run-id the grader reads `.state` directly, + # and phase 9 left it PENDING — which floored reversibility R2 and meant the SUCCESS CheckRun + # below was never actually consulted, so the fixture implied coverage it did not have. + | .data.repository.pullRequest.commits.nodes[0].commit.statusCheckRollup.state = "SUCCESS" + | .data.repository.pullRequest.commits.nodes[0].commit.statusCheckRollup.contexts.nodes = + [{"__typename":"CheckRun","name":"unit tests","status":"COMPLETED","conclusion":"SUCCESS", + "checkSuite":{"workflowRun":{"databaseId":1000,"workflow":{"name":"CI"}}}}]' \ + "$SANDBOX/fixture.json" > "$SANDBOX/botfixture.json" +cat > "$SANDBOX/botfiles.json" <<'FIX' +[{"filename":"data/skills.json","additions":40,"deletions":12,"status":"modified"}] +FIX +cat > "$SANDBOX/binbot/gh" <<'STUB' +#!/usr/bin/env bash +fixture=""; filter="" +for ((i=1; i<=$#; i++)); do + a="${!i}" + case "$a" in + graphql) fixture="$FIXTURE_DIR/botfixture.json" ;; + *pulls/*/files*) fixture="$FIXTURE_DIR/botfiles.json" ;; + --jq) n=$((i+1)); filter="${!n}" ;; + esac +done +[ -n "$fixture" ] || { echo "gh stub: unhandled args: $*" >&2; exit 1; } +if [ -n "$filter" ]; then jq -c "$filter" "$fixture"; else cat "$fixture"; fi +STUB +chmod +x "$SANDBOX/binbot/gh" +bot_pr() { PATH="$SANDBOX/binbot:$PATH" bash "$GRADER" --repo test/repo --pr 42 "$@" 2>/dev/null; } + +out="$(bot_pr)" +eq "the live record carries GitHub's own actor type" true "$(jq -r '.author_is_bot' <<<"$out")" +eq "and the login really does arrive unsuffixed" cloud-code-bot "$(jq -r '.author' <<<"$out")" +eq "an App PR is no longer external on the live path" human "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" +# ...and with the consumer's registry entry it reaches `runbook` — the ticket's end state. The +# entry names the App the way a human can SEE it (`cloud-code-bot[bot]`, what REST and the web +# UI show); the grader restores the suffix GraphQL dropped, so no consumer has to know this. +out="$(bot_pr --runbooks "$SANDBOX/app-runbooks.json" --bot-logins '')" +eq "an entry listing only the SUFFIXED form asserts against an unsuffixed Bot login" \ + runbook "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" +eq "and the tier now responds to the diff" R0 "$(jq -r '.risk.axes.provenance.tier' <<<"$out")" +# The rollup really is read on this path: green, but no test file in the diff, so reversibility +# proposes R1 and that — not the author's account type — is what decides the grade. +eq "the green rollup is actually consulted" R1 "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" +eq "and the overall grade is R1, decided by the diff" R1 "$(jq -r '.risk.tier' <<<"$out")" +# THE SYNTHESIS IS ONE-WAY AND GATED ON GITHUB'S ACTOR TYPE, which is the whole safety argument: +# the suffix is only ever ADDED, and only for an author GitHub types `Bot`. A USER account that +# happens to be named `cloud-code-bot` presents `cloud-code-bot` and nothing turns the entry's +# `cloud-code-bot[bot]` back into it — so it cannot inherit the App's runbook. +jq '.data.repository.pullRequest.author = {"login":"cloud-code-bot","__typename":"User"} + | .data.repository.pullRequest.authorAssociation = "MEMBER"' \ + "$SANDBOX/botfixture.json" > "$SANDBOX/bfuser.json" && cp "$SANDBOX/bfuser.json" "$SANDBOX/botfixture.json" +out="$(bot_pr --runbooks "$SANDBOX/app-runbooks.json" --bot-logins '')" +eq "a same-named USER cannot inherit the App's runbook" human "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" +# A BARE-SLUG entry still matches literally — that is what a machine USER account needs — and +# that is exactly why a bare slug must never be used to name an App. +jq '.runbooks[0].identity.logins = ["cloud-code-bot"]' \ + "$SANDBOX/app-runbooks.json" > "$SANDBOX/app-runbooks-bare.json" +out="$(bot_pr --runbooks "$SANDBOX/app-runbooks-bare.json" --bot-logins '')" +eq "a bare-slug entry matches a same-named USER too" runbook "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" +# Restore the Bot author for the rest of the phase. +jq '.data.repository.pullRequest.author = {"login":"cloud-code-bot","__typename":"Bot"} + | .data.repository.pullRequest.authorAssociation = "NONE"' \ + "$SANDBOX/botfixture.json" > "$SANDBOX/bfbot.json" && cp "$SANDBOX/bfbot.json" "$SANDBOX/botfixture.json" +jq '.runbooks[0].identity.logins = ["cloud-code-bot[bot]","cloud-code-bot"]' \ + "$SANDBOX/app-runbooks.json" > "$SANDBOX/app-runbooks-both.json" +# The fork half is unconditional on the live path too — the API fork flag, not the actor. +jq '.data.repository.pullRequest.isCrossRepository = true' \ + "$SANDBOX/botfixture.json" > "$SANDBOX/bf2.json" && cp "$SANDBOX/bf2.json" "$SANDBOX/botfixture.json" +out="$(bot_pr --runbooks "$SANDBOX/app-runbooks-both.json")" +eq "a Bot author on a fork is still external R3" external "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" +eq "and the grade is R3" R3 "$(jq -r '.risk.tier' <<<"$out")" +# A HUMAN author on the same live path keeps the first-time-contributor guard. +jq '.data.repository.pullRequest.isCrossRepository = false + | .data.repository.pullRequest.author = {"login":"drive-by","__typename":"User"}' \ + "$SANDBOX/botfixture.json" > "$SANDBOX/bf2.json" && cp "$SANDBOX/bf2.json" "$SANDBOX/botfixture.json" +out="$(bot_pr --runbooks "$SANDBOX/app-runbooks-both.json")" +eq "a live first-time human is still external" external "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" +eq "and still grades R3" R3 "$(jq -r '.risk.tier' <<<"$out")" + echo echo "passed $PASS, failed $FAIL" [ "$FAIL" -eq 0 ]