Skip to content

chore(loop): separate the write credential from the agent, and measure what each holds - #45

Merged
Alex1990 merged 9 commits into
mainfrom
chore/loop-write-scope
Sep 15, 2026
Merged

chore(loop): separate the write credential from the agent, and measure what each holds#45
Alex1990 merged 9 commits into
mainfrom
chore/loop-write-scope

Conversation

@Alex1990

@Alex1990 Alex1990 commented Sep 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Superseded in part by #46 (stacked on this branch).

This PR's facts stand and are still load-bearing: merging lives under Contents,
persist-credentials is needed, and a PR needs a head branch. Its conclusion —
that the loop's safety should be expressed by splitting credentials — does not, and
#46 replaces it. A PAT acts as its owner, who is this repository's only admin and
therefore the ruleset's bypass actor; and PRs authored by the owner cannot be
approved by the owner, so loop PRs would be un-approvable. #46 makes the Main branch ruleset the gate instead: every change through a reviewed PR, the owner the
only bypass actor and only for merging, and no PAT anywhere.

Read the two together; the sections below marked "L2c" describe the design that was
tried and are kept only so the reasoning is not re-derived. Merge them
back-to-back — this one first — so main never sits on the replaced version.

Records how to give the loop PR creation without merge, adds a tool that measures a credential instead of assuming it, and closes a hole the measurement exposed: the agent used to inherit whatever GH_TOKEN held.

1. Merging is not under "Pull requests"

Repository permissions for "Contents"
  PUT /repos/{owner}/{repo}/pulls/{pull_number}/merge   | write

Repository permissions for "Pull requests"
  GET /repos/{owner}/{repo}/pulls/{pull_number}/merge   | read     ← query only

Merging writes commits to the target branch, so GitHub files it under Contents. Two consequences:

  • Pull requests: write cannot merge. It covers POST /pulls, PATCH /pulls/{n}, reviews, review comments — and stops there.
  • "Can open a PR" and "can merge a PR" are the same permission — within one credential. A PR needs a head branch; creating one needs Contents: write; and that is the merge permission.

Contents: write also carries POST /releases (the gate the spec reserves for humans) and POST /dispatches.

2. So split the roles, not the scope list

Credential Held by Scopes Used for
GITHUB_TOKEN (job) the workflow's own steps contents: write creating and pushing the loop/<n>-* branch
LOOP_GH_TOKEN (PAT) the agent subprocess Issues: RW, Pull requests: RW — no Contents labels, comments, opening the PR

The agent then has no route to PUT /pulls/{n}/merge (403) and no route to push anything, while the loop still produces PRs.

Two details, both easy to miss:

  • actions/checkout must set persist-credentials: false. Its default is true and writes the job token into the repository's git config so scripts can run authenticated git commands — handing the agent exactly the Contents: write this removes.
  • Only the push moves to a host step. Committing is local and needs no network credential, so the agent still stages and commits; the host pushes and opens the PR from the agent's proposed metadata — the same "agent proposes, host executes" shape as D30/D33.

Side effect to plan for: a PR created with GITHUB_TOKEN yields a pull_request event whose runs start approval-required (closed/labeled/edited create no runs at all). The loop will not triage or review its own PR until a human approves — which removes recursive self-review, and matches D26's direction. A human merge still fires pull_request_target.closed, driving the metrics gate.

Rulesets cannot substitute here: solo repository, one collaborator with admin, so a required-approval rule blocks the owner's own PRs forever and an admin bypass_actor would let the loop through too.

3. The configured PAT is correct

gh-check.mjs runs before the agent can reach the credential, because GH_TOKEN is secrets.LOOP_GH_TOKEN || github.token — configuring a PAT silently changes every gh call in the job, widening scope or narrowing the reads sweep depends on.

Measured on Actions against the real PAT — all sound probes (2xx means the write actually executed; every probe restores the value it read):

  OK       repo metadata / issues list / pull requests list / releases list
  present  Issues: write        (HTTP 200)
  present  Pull requests: write (HTTP 200)
  ABSENT   Contents: write      (HTTP 403)
  ABSENT   Actions: write       (HTTP 403)
  ABSENT   Secrets access
host holds no Contents:write — merging, branch pushes and releases are impossible for it.

Reads all pass, both needed write scopes are present, and the three dangerous ones are absent. No change needed.

4. Two wrong methods, and what replaced them

Worth documenting because both looked plausible and both produced confident wrong answers.

Attempt 1 — 403/404 against an impossible id. Call a write endpoint on /issues/999999999; read 403 as "no scope", 404 as "scope present". Disproved by measurement: a github.token capped to read-only by permissions: replied 404, not 403. So 404 means only "has some scope for this resource" — read or write — and every write probe was a false positive. It reported the agent's read-only token as holding Contents: write.

Attempt 2 — .permissions from GET /repos/{repo}. Reported the configured PAT as holding Contents: write, and sent the owner to recreate a token that was already right. That object describes the authenticated user's role on the repository, so it answers push: true for any token belonging to an admin, however narrowly scoped. It is not a token scope report. (It is now printed with that label, so the next reader does not repeat it.)

What works: PATCH /git/refs/heads/{branch} with the SHA the ref already has. Setting a ref to its current value is a no-op, but only a real write scope returns 2xx. Verified locally — 200, ref unchanged — and on Actions, where the correct PAT answers 403.

403 survives only for scopes a credential lacks entirely, which is why the Actions and Secrets checks are sound.

5. The hole the measurement exposed

runPi spawned the agent with env: process.env, so the agent inherited GH_TOKEN. With a write-capable PAT that would have let it push to main or merge a PR, with only the prompt in the way — writeLevel gates the host, not the agent's own gh calls.

Now separated at the spawn boundary:

GH_TOKEN        host credential (PAT when configured)
GH_READ_TOKEN   github.token, capped at read by `permissions:`

runPi hands the child GH_READ_TOKEN whenever writeLevel !== 'auto' and logs which one it used. Verified on Actions:

can read the repo:   yes
acts as a user:      no — a GitHub App token, i.e. github.token
ABSENT   Issues: write   (HTTP 403)
ABSENT   Contents: write (HTTP 403)
OK: read-only by the workflow's `permissions:` block — the report boundary is a platform guarantee.

(The agent check cannot use identity: github.token is a GitHub App installation token and GET /user refuses it outright. It uses two observable signals instead — can it read the repo, and is a write refused.)

Verification

Still open (not this PR)

L2c implementation is in #46, stacked on this branch. Its design differs from
the credential split argued above — see the note at the top — and it supersedes the
L2c sections of this PR's diff. Merge this branch first so #46 retargets to main,
then merge #46 immediately: until it lands, main carries this PR's wording, which
#46 rewrites.

@Alex1990
chore(loop): record the write-credential scope decision (does PR writ…
20f3b7a 
…e include merge?)

Answers a question the host had no answer for, and pins the L2 scope before
anyone configures LOOP_GH_TOKEN. No behaviour change.

The finding that drives the decision: **merging a pull request is not covered by
"Pull requests: write"**. GitHub's endpoint-to-permission table puts
`PUT /pulls/{n}/merge` under **Contents**, because merging means writing commits
to the target branch. `Pull requests: write` covers create/update/review and
review comments, and nothing more.

Two consequences worth stating plainly:

- "Can open a PR" and "can merge a PR" are the same permission. A PR needs a
  head branch, creating a branch needs `Contents: write`, and that is the merge
  permission. They cannot be separated.
- `Contents: write` also carries `POST /releases` (the release gate) and
  `POST /dispatches`, so granting it for PR creation hands over more than the
  obvious.

Hence two sizes of L2, with different risk:

- L2a (recommended): `Issues: RW` + `Pull requests: RW` + `Contents: read`.
  Covers every write action the host actually implements (`planActions`:
  label / comment / close), and leaves merging, code and branches physically
  out of reach. `no self-merge` moves from a prompt constraint to a platform
  impossibility, matching the guarantee `git push` already has.
- L2b: adds `Contents: RW` for branch push and PR creation — and with it merge
  and release.

Server-side rulesets do not rescue L2b in this repository: it is a solo repo
(one collaborator with admin), so a required-approval rule would block the
owner's own PRs forever, and an admin bypass actor would let the loop's PAT
bypass it too. The existing `Main branch` ruleset is `enforcement: disabled`
and only covers `deletion` + `non_fast_forward` — it never restricted merging.

Also corrects three statements that had drifted out of date: the architecture
diagram now shows the `terminal` decision (D33) and the D34-conditional push,
the `auto` row notes that permission is not ability while the PAT is unset, and
the L2 checklist item points at the scope choice.
@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Coverage

Metric % Covered / Total
Lines 90.19% 1003/1112
Statements 89.06% 1083/1216
Functions 91.03% 193/212
Branches 76.43% 519/679

@Alex1990
chore(loop): record L2c — the loop opens PRs and still cannot merge
1037acc 
Owner's requirement: the loop should open pull requests but must never be able
to merge one. The previous revision of this section stopped at "a single
credential cannot express that", which is true but not the whole answer — it
cannot express it *within one credential*, and the split can be made across
roles.

- `GITHUB_TOKEN` (the workflow's own steps): `contents: write`, used only to
  create and push the `loop/<n>-*` branch.
- `LOOP_GH_TOKEN` (the agent subprocess): `Issues: RW` + `Pull requests: RW`,
  no `Contents` — enough for labels, comments and opening the PR, and 403 for
  `PUT /pulls/{n}/merge`.

Two required details, both easy to miss:

- `actions/checkout` must set `persist-credentials: false`. Its default is
  `true` and writes the job token into the repository git config, which would
  hand the agent the very `Contents: write` this design removes.
- Only the push has to move to a host step. Committing is local and needs no
  network credential, so the agent keeps staging and committing in the
  workspace; the host pushes and opens the PR from the agent's proposed
  metadata — the existing "agent proposes, host executes" division.

Side effect worth stating: a PR created with `GITHUB_TOKEN` yields a
`pull_request` event whose runs start in approval-required state, so the loop
will not triage or review its own PR and CI will not run until a human approves
the runs. For this loop that is desirable — it removes recursive self-review on
the loop's own output, which is the same direction D26 points.

Also scopes two earlier claims to "within a single credential" so they no
longer contradict L2c, and points the L2 decision item at this target.
@Alex1990 Alex1990 changed the title chore(loop): record the write-credential scope decision (does PR write include merge?) chore(loop): record how the loop opens PRs but can never merge Sep 11, 2026
@Alex1990
chore(loop): measure the GitHub credential instead of assuming it
d02b8ce 
`GH_TOKEN` is `secrets.LOOP_GH_TOKEN || github.token`, so configuring a PAT
silently changes every `gh` call in the job — it can narrow the read-only ones
sweep depends on, or widen what the agent can write. Neither is visible from the
workflow file, so it has to be measured. Adds `gh-check.mjs` and runs it before
the agent can reach the credential.

Permission probing without side effects: each write endpoint is called against
an impossible id, so `403` means the scope is absent and `404`/`422` means the
request passed the permission check and only failed on the missing resource.
Nothing is mutated — every probe targets 999999999, far beyond this repository's
range.

Checks both directions, because they fail differently:

- reads the host depends on (repo metadata, issues, PRs, `gh release list`) —
  a PAT narrower than the job token breaks sweep silently;
- writes it should and should not hold — `Issues`, `Pull requests`, and
  `Contents`, where the last one is reported as a warning because it carries
  merging, branch pushes and `POST /releases` together.

The token value is never printed, only the login it authenticates as.
@Alex1990
chore(loop): widen the credential probe to the dangerous scopes too
a9a97e1 
Contents is the one that matters most, but it is not the only way to reach code
or CI without a pull request. Splits the probes into scopes the loop needs and
scopes it must not hold, and adds the two other high-value ones:

- Actions: write — dispatching a workflow is indirect code execution
- Secrets — listing secret names requires the permission, and it can overwrite
  the CI credentials. The listing is never printed, only whether it is present.

Local check against an admin token reports all three held, confirming the probes
discriminate rather than always returning 'present'.
@Alex1990
chore(loop): keep a writable credential out of the agent's hands
bcc22c4 
`runPi` spawned the agent with `env: process.env`, so once `LOOP_GH_TOKEN` was
configured the agent inherited the PAT — and the report boundary would have been
enforced by the prompt alone. Measured against the real PAT, that mattered: it
holds `Contents: write`, which is merging, branch pushes and `POST /releases` in
one scope.

The host still needs the real token (future `applyActions`), but the agent does
not, so the two are now separated at the spawn boundary:

  GH_TOKEN        the host's credential (PAT when configured)
  GH_READ_TOKEN   `github.token`, capped at read by `permissions:`

`runPi` hands the child `GH_READ_TOKEN` whenever `writeLevel !== 'auto'`, and
logs which one it used. The agent therefore reaches GitHub read-only even if it
ignores its prompt, and `auto` is unchanged — the agent gets the write token,
which is what that mode is for.

`gh-check.mjs` now also measures this: it probes the credential the *agent* will
hold and says outright whether the report boundary is a platform guarantee or
back to being a prompt instruction. That makes the property observable in a
smoke run, with no agent tokens spent.

Verified locally with a stub engine: under `report` the child sees
`READ-ONLY-JOB-TOKEN`; under `auto` it sees the write token.
@Alex1990
chore(loop): cross-check the credential probes against GitHub's own p…
af8cc9f 
…ermissions

The 403-vs-404 reading assumes GitHub checks permissions before resource
lookup. The first Actions run reported the agent's github.token as holding
Issues and Contents write, which contradicts the workflow's read-only
`permissions:` block — so either the assumption is wrong or the token really
does hold write. Both credentials now also report the `permissions` object
GitHub returns for them, which needs no inference.
@Alex1990
chore(loop): replace the unsound permission probe with idempotent writes
b311f76 
The 403-vs-404 method was wrong, and the first real measurement proved it: a
`github.token` that the workflow caps to read-only replied `404` to a write,
not `403`. So `404` means only "has *some* scope for this resource" — read or
write — and every write probe was reporting a false positive. The agent's token
was consequently reported as holding `Contents: write` when it holds nothing of
the sort.

Two sound methods now:

- GitHub's own `permissions` object for the repository, which needs no
  inference. `push: true` is exactly `Contents: write` — the scope carrying
  merge, branch pushes and `POST /releases` together.
- **Idempotent writes** for the scopes that object does not describe. The probe
  reads the current title and writes it straight back, so nothing changes, but
  only a real write scope returns `2xx`. Against an admin token all three answer
  `200`, confirming the probes discriminate.

`403` is still meaningful for a scope the credential lacks entirely — Actions
and Secrets keep using it, and they correctly report ABSENT for the PAT.

The agent check now decides on *identity* instead of probing scopes:
`github-actions[bot]` means `github.token`, whose reach the workflow's
`permissions:` block sets, so read-only is guaranteed by construction rather
than measured. That is the fact worth asserting, and it cannot be faked by a
probe artifact.
@Alex1990
chore(loop): check the agent credential on signals that actually exist
6160db1 
Identity was the wrong signal. `github.token` is a GitHub App installation
token and `GET /user` refuses it outright, so the check reported "(failed)"
rather than distinguishing a job token from a PAT.

Uses two signals that are observable instead: whether the credential can read
the repository at all, and whether an idempotent write is refused. A PAT shows
up as "acts as a user"; a read-only job token reads fine but cannot write. That
combination is what the report boundary depends on, and it is now asserted
directly rather than inferred from an identity lookup that cannot work.
@Alex1990 Alex1990 changed the title chore(loop): record how the loop opens PRs but can never merge chore(loop): separate the write credential from the agent, and measure what each holds Sep 11, 2026
@Alex1990
chore(loop): measure Contents with a real write, not the repo permiss…
86dbb07 
…ions object

The Contents check read `.permissions.push` from `GET /repos/{owner}/{repo}` and
treated it as a token scope report. It is not: that object describes the
*authenticated user's* role on the repository, so it answers `push: true` for
any token belonging to an admin — however narrowly that token was scoped. The
result was a false WARNING claiming a Contents Read-only PAT could merge and
push, plus an instruction to recreate a token that was already correct.
Reported by the owner, who had set Contents to Read-only.

Contents is now probed like the other scopes: read the default branch's SHA and
`PATCH /git/refs/heads/{branch}` with that same SHA. Setting a ref to the value
it already holds is a no-op, but only a real write scope returns 2xx — verified
locally (the ref came back unchanged after a 200) and on Actions, where the PAT
now correctly reports `ABSENT ... (HTTP 403)`.

The `.permissions` object is still printed, labelled as the user's role, so the
next reader does not repeat the mistake.

The agent check now probes Contents too: it is exactly the scope that carries
merge and push, so it is the one worth asserting about the credential the agent
holds. Both report ABSENT (HTTP 403) there.
@Alex1990
Alex1990 force-pushed the chore/loop-write-scope branch from c57c2eb to 86dbb07 Compare September 11, 2026 15:50
@Alex1990 Alex1990 mentioned this pull request Sep 15, 2026
Merged
@Alex1990
Alex1990 merged commit a11cc0a into main Sep 15, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Alex1990