Skip to content

fix(dependabot): refuse a check rollup the token could not fully read - #134

Merged
twistedmelonman merged 3 commits into
mainfrom
claude/digest-error-path-dee4868f
Sep 12, 2026
Merged

fix(dependabot): refuse a check rollup the token could not fully read#134
twistedmelonman merged 3 commits into
mainfrom
claude/digest-error-path-dee4868f

Conversation

@twistedmelonman

Copy link
Copy Markdown
Member

A fine-grained token cannot read check results, and GitHub does not say so

Fine-grained PATs have no Checks: read permission at all
(community#129512). The
API does not refuse the query for it. It returns statusCheckRollup with
HTTP 200 and the correct totalCount, then nulls every CheckRun the token
may not see.

Measured on kebab-tax-netlify#280, digest token vs a full login:

contexts readable
full login 12 12
digest token 12 1 (the Netlify StatusContext)

Mapped as written, that PR's 7 failing builds and 1 green required check
became "1 failing check, 0 required checks".

Why this was worse than a broken run

Reproduced against the classifier: a PR whose only checks are nulled CheckRuns
classified as ready-to-merge and was issued a paste-ready
merge-lock authorize line.

bucket=ready-to-merge blocking=0 advisory=0
merge-lock authorize #9 "ok"

Most of the fleet has no Netlify status to survive the nulling, so that was the
common path, not an edge case. A digest that recommends merging PRs with
failing builds is worse than no digest — it is precisely the failure the
classifier was written to prevent, arriving through the one door nobody was
watching.

Three layers, in depth order

  • collect.sh refuses any response containing a nameless context, naming
    the count and the discussion. An unreadable check is not an absent one.
  • classify.sh adds a checks-unreadable bucket that outranks every other
    signal — nothing else can be trusted once the check data is known partial.
    Second line of defence, in the component that decides what is safe to merge.
  • render.sh renders that bucket first and says plainly that nothing in it
    is green.

Verified both directions against the live token: the digest token now exits 1
with a clear reason; a fully-scoped login still completes with rc=0.

Also

  • GraphQL errors are grouped rather than printed once per context. One refused
    permission produced eleven identical lines; it now reads
    FORBIDDEN at …contexts.nodes.0 (and 10 more): ….
  • The private-repo probe read repo metadata, which answers under Metadata
    alone — so it passed for a token that could not read a single commit. It now
    reads commits, matching what the survey needs.
  • The error formatter now keeps GraphQL's path, which is what identified the
    failing field. The test that extracts it reads a multi-line jq program; a
    line-based grep found nothing after the reformat and failed for the wrong
    reason.

The regression test was validated against the unguarded classifier, where it
fails with "a PR whose checks are all null classified as ready-to-merge". It
first passed vacuously against an undefined path variable — that is now caught
explicitly.

What this does not resolve

The digest still cannot read check results for private repos under a
fine-grained token, and no permission grant fixes that. It now fails loudly
instead of lying. The options — a classic PAT, a GitHub App, or accepting
public-repo-only coverage — are a decision, not a bug, and are left for that
decision rather than pre-empted here.

Advances #120.

https://claude.ai/code/session_01ESsw699T54JHARkQXrdL3o

Claude Code Bot added 3 commits September 11, 2026 18:20
…ling field

Two defects found while diagnosing a FORBIDDEN that took four runs to localize.

The private-repo probe tested the wrong thing. It read `repos/<owner>/<name>`,
which answers under the Metadata permission alone — so it passed for a token
that could not read a single commit, while every PR detail read failed. The
survey reads each PR's checks through `pullRequest.commits`, so the probe now
reads `commits` on the probe repo. A probe that cannot fail the way the real
work fails is not a probe.

The error formatter dropped the GraphQL `path`. The message alone —
"Resource not accessible by personal access token" — names no field, so the CI
log could not say which read was refused. The path
(`repository.pullRequest.commits.nodes.0`) is what finally identified it, and
it was available in the response body the whole time.

Both are covered by assertions validated against the known-bad form: removing
the path fails with "the error formatter dropped the path", and the probe
assertion now matches the commits-based message.

Advances #120.

Claude-Session: https://claude.ai/code/session_01ESsw699T54JHARkQXrdL3o
A fine-grained token cannot be granted Checks: read at all
(github.com/orgs/community/discussions/129512). GitHub does not refuse the
query for it. It returns statusCheckRollup with HTTP 200 and the correct
totalCount, then nulls every CheckRun the token may not see.

Measured on kebab-tax-netlify#280: 12 contexts returned, 11 of them null, the
one survivor a Netlify StatusContext. Mapped as written, that PR's seven
failing builds and one green required check became "1 failing check, 0 required
checks".

The consequence, reproduced against the classifier: a PR whose only checks are
nulled CheckRuns classified as ready-to-merge and was issued a paste-ready
`merge-lock authorize` line. Most of the fleet has no Netlify status to survive
the nulling, so that was the common path, not the edge case. A digest that
recommends merging PRs with failing builds is worse than no digest — it is the
exact failure the classifier was written to prevent.

Three changes, in depth order:

- collect.sh refuses any response containing a nameless context, naming the
  count and the discussion. An unreadable check is not an absent one.
- classify.sh adds a `checks-unreadable` bucket that outranks every other
  signal, because nothing else can be trusted once the check data is known to
  be partial. This is the second line of defence, in the component that
  decides what is safe to merge.
- render.sh renders that bucket first, and says plainly that nothing in it is
  green.

Also: GraphQL errors are now grouped rather than printed once per context (one
refused permission produced eleven identical lines), and the test that extracts
the error formatter now reads a multi-line jq program — a line-based grep found
nothing after the reformat and failed for the wrong reason.

The regression test was validated by running it against the unguarded
classifier, where it fails with "a PR whose checks are all null classified as
ready-to-merge". It initially passed vacuously against an undefined path
variable; that is now caught explicitly.

Advances #120.

Claude-Session: https://claude.ai/code/session_01ESsw699T54JHARkQXrdL3o
@claude

claude Bot commented Sep 12, 2026

Copy link
Copy Markdown

Checks for blocking issues completed. No bugs, regressions, security vulnerabilities, missing error handling, or data loss risks identified.

The PR adds defensive measures to prevent silent failures when fine-grained GitHub tokens lack read access to check data. The multi-layered approach (collect.sh validation, classify.sh detection, render.sh warnings) correctly prevents unreadable checks from being treated as passing. Test coverage validates the regression prevention.

VERDICT: PASS

@twistedmelonman
twistedmelonman merged commit 27b077d into main Sep 12, 2026
3 checks passed
@twistedmelonman
twistedmelonman deleted the claude/digest-error-path-dee4868f branch September 12, 2026 02:38
twistedmelonman pushed a commit that referenced this pull request Sep 12, 2026
The three fine-grained tokens cannot do the job and no scoping fixes it: a
fine-grained PAT has no Checks permission at all
(github.com/orgs/community/discussions/129512), so it cannot read GitHub
Actions results on a private repository. Apps do have one, and a single app
installs on all three owners — which also removes the reason there are three
credentials.

The plan records the measurement behind the decision, the human steps (app
creation, private key, three installs, two secrets), the agent steps (workflow
minting via create-github-app-token, pinned to its v3.2.0 SHA), and the
cleanup, gated behind a verified run rather than listed before it.

Three things are called out as NOT fine-grained-token artifacts, because
"remove what the experiment added" would otherwise read as including them:

  - the three-layer nulled-checks guard (#134) — it enforces "unreadable is
    not absent", which holds for any credential, and a published digest is
    itself the proof the migration worked
  - verify_private_visibility and DIGEST_PROBE_* — an installation scoped to
    selected repositories degrades the same silent public-only way
  - run-digest.sh's per-owner DIGEST_TOKEN_<OWNER> indirection — it reads the
    token from the environment and does not care how it was obtained, which is
    what makes this migration workflow-only

collect.sh needs one edit, not a revert: its refusal message names fine-grained
tokens specifically and would misdirect if an app installation ever hit the
same guard.

STATUS.md now states the real blocker. It still said the digest was waiting on
tokens that have since been installed.

Advances #120.

Claude-Session: https://claude.ai/code/session_01ESsw699T54JHARkQXrdL3o
twistedmelonman pushed a commit that referenced this pull request Sep 12, 2026
#133 and #134 landed while this plan was in review, so three statements in it
were stale on arrival: the prerequisite line, step 4's "branched from main
after #134 merges", and an open item recommending #133 be closed unmerged.

Step 8 now rewrites a runbook that is accurate rather than one that is wrong —
it condenses the fine-grained material to a paragraph instead of correcting it.

Verified against origin/main rather than assumed: the nulled-checks guard is
present in collect.sh and the checks-unreadable bucket in classify.sh.

Claude-Session: https://claude.ai/code/session_01ESsw699T54JHARkQXrdL3o
twistedmelonman added a commit that referenced this pull request Sep 12, 2026
#135)

* docs(dependabot): plan the move from fine-grained PATs to a GitHub App

The three fine-grained tokens cannot do the job and no scoping fixes it: a
fine-grained PAT has no Checks permission at all
(github.com/orgs/community/discussions/129512), so it cannot read GitHub
Actions results on a private repository. Apps do have one, and a single app
installs on all three owners — which also removes the reason there are three
credentials.

The plan records the measurement behind the decision, the human steps (app
creation, private key, three installs, two secrets), the agent steps (workflow
minting via create-github-app-token, pinned to its v3.2.0 SHA), and the
cleanup, gated behind a verified run rather than listed before it.

Three things are called out as NOT fine-grained-token artifacts, because
"remove what the experiment added" would otherwise read as including them:

  - the three-layer nulled-checks guard (#134) — it enforces "unreadable is
    not absent", which holds for any credential, and a published digest is
    itself the proof the migration worked
  - verify_private_visibility and DIGEST_PROBE_* — an installation scoped to
    selected repositories degrades the same silent public-only way
  - run-digest.sh's per-owner DIGEST_TOKEN_<OWNER> indirection — it reads the
    token from the environment and does not care how it was obtained, which is
    what makes this migration workflow-only

collect.sh needs one edit, not a revert: its refusal message names fine-grained
tokens specifically and would misdirect if an app installation ever hit the
same guard.

STATUS.md now states the real blocker. It still said the digest was waiting on
tokens that have since been installed.

Advances #120.

Claude-Session: https://claude.ai/code/session_01ESsw699T54JHARkQXrdL3o

* docs(dependabot): the plan's prerequisites are merged, not pending

#133 and #134 landed while this plan was in review, so three statements in it
were stale on arrival: the prerequisite line, step 4's "branched from main
after #134 merges", and an open item recommending #133 be closed unmerged.

Step 8 now rewrites a runbook that is accurate rather than one that is wrong —
it condenses the fine-grained material to a paragraph instead of correcting it.

Verified against origin/main rather than assumed: the nulled-checks guard is
present in collect.sh and the checks-unreadable bucket in classify.sh.

Claude-Session: https://claude.ai/code/session_01ESsw699T54JHARkQXrdL3o

---------

Co-authored-by: Claude Code Bot <claude-code@smartwatermelon.github>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant