feat: implement issue #1776 — Authorization preflight: fail loudly when a persona credential authenticates but cannot write (split from #1734 AC #5)

[don-petry]

Closes #1776

Implemented by dev-lead agent. Please review.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[qodo-code-review]

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

[coderabbitai]

Warning

Review limit reached

Next included review available in 28 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: be74f4d9-fbd9-471a-85b9-a93df98f4f6a

📥 Commits

Reviewing files that changed from the base of the PR and between 371c5e4 and 12bd8ed.

📒 Files selected for processing (5)

• .github/workflows/lint.yml
• .github/workflows/persona-runner-reusable.yml
• scripts/lib/persona-authz-preflight.sh
• scripts/persona-authz-preflight.sh
• tests/test_persona_authz_preflight.sh

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[don-petry]

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

[gemini-code-assist]

Code Review

This pull request introduces a non-destructive authorization preflight check for the persona runner, consisting of a pure decision library, a wrapper script that queries GitHub repository permissions, and comprehensive BATS unit tests. The preflight ensures that a credential has write access before proceeding, failing closed on unauthorized tokens and failing open on indeterminate API errors. The review feedback suggests adding error handling to the mktemp call in the wrapper script and asserting specific non-zero exit codes in the BATS tests instead of using generic non-zero checks to prevent false positives.

[don-petry]

Verified by execution — all six ACs met

bats tests/test_persona_authz_preflight.sh → 23/23 pass. shellcheck -S warning on both new
scripts → clean. Run on dev-lead/issue-1776-20260920-2030 in a fresh worktree.

AC	verdict	evidence
#1 detects authenticates-but-cannot-write, names account + credential + capability	✅	tests 15–16; #16 asserts it does not masquerade as a missing secret
#2 non-destructive probe	✅	single GET /repos/{owner}/{repo} read of .permissions; nothing written
#3 placement decided and justified	✅	option (a) — before the engine call, so paid work is never produced then lost
#4 cannot become a false gate	✅	tests 9–11, 14, 22–23
#5 pure decision logic unit-tested	✅	18 of 23 tests are the pure layer
#6 derived from manifests, not a list	✅	credential comes from the identity step; no persona enumerated

AC #4 is the one that matters and it is right. An unreadable surface is INDETERMINATE and fails
open, even when a stale capability argument says no — test 9 pins exactly that. A read failure is
never a definite negative. The header states the rule plainly: "a preflight that blocks advisories on a
hiccup is worse than the bug it prevents."

The detection-scope block is the best thing in this PR. Documenting what .permissions cannot
see — fine-grained "Issues: read but not write", org/branch restrictions — and routing those to
INDETERMINATE rather than a false negative is exactly right, and it resists the temptation to
overclaim what a cheap proxy proves.

Three non-blocking observations

1. One factual claim looks wrong and will mislead later readers. Both the lib header and the runner
comment say this class "silently dropped a pr-review approval on PR #1788." PR #1788 never carried an
approval — gh api pulls/1788/reviews returns 7 COMMENTED and zero APPROVED, and it merged via
mergedBy=don-petry, i.e. a human. The documented reason no verdict landed there was
review-one-pr.sh skipping with reason: ci-failing / ci-pending (#1795, now closed) — a CI-gate
problem, not an authorization one. If there was also an authz failure on #1788, it would be worth
citing the run; if not, please drop that sentence. #1734 alone carries the motivation, and it is a real,
verified instance (run 34183239687). A wrong "why this exists" is worse than no comment.

2. tests/test_persona_authz_preflight.sh is a bats file with a .sh extension. It runs correctly
in CI — lint.yml passes it explicitly to bats, which ignores the extension — so this is not a broken
gate. But bash tests/test_persona_authz_preflight.sh fails with @test: command not found, and any
tests/*.bats glob skips it. The repo has 80 .bats files to 6 .sh, and AC #5 named bats and
ADR-0004. Renaming to .bats (and updating the one lint.yml line) costs nothing and removes a trap
for the next person who tries to run it locally.

3. The credential→token if is now duplicated. The preflight repeats the post step's
[ "$POSTING_CREDENTIAL" = "DON_PETRY_BOT_GH_PAT" ] selection verbatim. The comment's claim — "the
token that WILL post is the token we probe" — is true today and silently stops being true the moment
either copy changes. That is the same derive-don't-duplicate rule AC #6 invokes for the persona list,
applied one level down. A tiny shared helper (or a single step output consumed by both) would make the
invariant structural instead of a comment.

None of these block the merge. Item 1 is the only one I would not leave as-is, since it is a factual
claim about a specific PR rather than a style preference.

[donpetry-bot]

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-09-20T21:43:06Z.

[don-petry]

Dev-Lead — fix-reviews (partial)

A commit was pushed, but not every requested change was applied. Per requested item:

• scripts/persona-authz-preflight.sh:56 — applied
• tests/test_persona_authz_preflight.sh:104 — applied
• tests/test_persona_authz_preflight.sh:109 — applied
• .github/workflows/persona-runner-reusable.yml:219 — applied
• scripts/lib/persona-authz-preflight.sh:70 — not applied

The unaddressed items above still need work.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[donpetry-bot]

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-09-20T21:50:02Z.

[donpetry-bot]

pr-review approved on PARTIAL advisory evidence: 4/6 required advisory bots reported before the gate's head-age-timeout fallback proceeded. Recorded for the miss-rate metric (#1596).

[don-petry]

No description provided.

[don-petry]

Dev-Lead — waiting on PR blockers (intent: fix-reviews)

PR: #1858
No changes were committed, but the PR still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews). The retry cron will re-attempt automatically. Next attempt after: 2026-09-20T22:57:52Z