Skip to content

fix(ci): advisory reviewer could never comment on a fork PR (403) - #92

Merged
bkd-dotcom merged 2 commits into
mainfrom
fix/reviewer-fork-pr-comment
Aug 18, 2026
Merged

fix(ci): advisory reviewer could never comment on a fork PR (403)#92
bkd-dotcom merged 2 commits into
mainfrom
fix/reviewer-fork-pr-comment

Conversation

@bkd-dotcom

@bkd-dotcom bkd-dotcom commented Aug 18, 2026

Copy link
Copy Markdown
Member

The bug

reviewer.yml runs on pull_request and requests pull-requests: write to post its advisory comment. On a PR from a fork, GitHub downgrades GITHUB_TOKEN to read-only regardless of the permissions: block — so the write grant is silently dropped and issues.createComment returns:

Resource not accessible by integration  (403)

Every outside contribution has shown a red review check and never received the advisory comment. Confirmed on #89 and #91 (both fork PRs, both 403 in the `Post / update the review comment" step).

The fix

The standard trusted/untrusted split:

trigger token runs PR code?
reviewer.yml pull_request no write perms yes
reviewer-comment.yml workflow_run pull-requests: write never

The untrusted job stages comment.md + the PR number as an artifact; the trusted job downloads it and posts. Because the trusted job never checks out, builds, or executes PR code, holding a writable token there is safe.

Why not pull_request_target

It's the one-line fix and it's the wrong one: it grants a writable token in the base repo's context, and checking out PR head code under it is a well-known RCE footgun. Not a trade a change-control product should make.

Hardening details

  • PR number travels in the artifact (workflow_run.pull_requests is empty for forks) and is filtered to digits on both write and read.
  • Comment body is passed to the API as data, never through a shell or eval, so a crafted diff cannot escalate past comment text.
  • Body truncated at 65 000 chars (API limit is 65 536).
  • The poster only updates a marker-bearing comment authored by a Bot, so a contributor cannot get the bot to overwrite a human's comment by planting the marker in a PR description.

Verification

  • actionlint clean on both files.
  • YAML parses; permissions verified as contents/checks: read (untrusted) and pull-requests: write (trusted).
  • Full chain can only be exercised once reviewer-comment.yml is on the default branch (workflow_run always loads from default) — I'll verify with a test PR immediately after merge and report back here.

Same bug exists in 9 other Signetry repos (plugins, codex, cursor, precommit, claude-code, eval, signetry, github-app, action); rolling this out there once it's proven here.

@bkd-dotcom
fix(ci): advisory reviewer could never comment on a fork PR (403)
6f8c4f9 
The Reviewer workflow runs on `pull_request` and requested
`pull-requests: write` to post its advisory comment. On a PR from a fork
GitHub downgrades GITHUB_TOKEN to read-only regardless of the
`permissions:` block, so the write grant was silently dropped and
`issues.createComment` failed with "Resource not accessible by
integration" (403). Every outside contribution showed a red `review`
check and never received the advisory comment it was supposed to get.

Split into the standard trusted/untrusted pair:

  * reviewer.yml (untrusted, runs PR code) now holds no write permission
    at all. It stages the rendered comment plus the PR number and uploads
    them as an artifact.
  * reviewer-comment.yml (trusted) runs on `workflow_run` from the
    default branch, where the token is writable, and posts the comment.
    It never checks out, builds, or executes PR code.

Deliberately not `pull_request_target`: that grants a writable token in
the base repo's context, and checking out PR head code under it is an
RCE footgun — not a trade this repo should make.

The PR number travels inside the artifact because
`workflow_run.pull_requests` is empty for fork PRs, and is filtered to
digits on both write and read. The poster only updates a comment whose
marker it finds on a Bot-authored comment, so a contributor cannot get
the bot to overwrite a human's comment by planting the marker.
@bkd-dotcom
fix(ci): advisory reviewer step could fail the PR despite 'exit 0'
3d77983 
GitHub runs `run:` steps as `bash -e`, so `set -uo pipefail` never
disabled errexit. A non-zero exit from signetry-reviewer (any Block
verdict) aborted the step immediately and the trailing
`exit 0   # advisory: never fail the PR from this job` was unreachable.

The workflow header promises the job 'never merges and does not block the
PR (the self-admission + test checks remain the gates)', but a Block
verdict turned the check red. Add `set +e` and an explicit `|| true` so
the verdict lands in the advisory comment, not in the check status.

Found while testing the fork-PR comment fix: this PR touches
.github/workflows/**, the reviewer returned Block, and the step died at
the reviewer invocation rather than reaching its own exit 0.
@bkd-dotcom

bkd-dotcom commented Aug 18, 2026

Copy link
Copy Markdown
Member Author

Found a second, independent bug while testing this — pushed a fix in 3d77983.

The exit 0 in the advisory review step has never run.

GitHub runs run: steps as bash -e (visible in the job log: shell: /usr/bin/bash -e {0}). set -uo pipefail does not clear errexit, so a non-zero exit from signetry-reviewer aborts the step immediately and this line is unreachable:

exit 0   # advisory: never fail the PR from this job

So the workflow header's promise — "Advisory only — it never merges and does not block the PR (the self-admission + test checks remain the gates)" — did not hold: any Block verdict turned the review check red.

This PR is how it surfaced. It touches .github/workflows/**, which is in --protected, so the reviewer returned 🔴 Block and the step died at the reviewer invocation, never reaching its own exit 0. Note the two failures are unrelated: #91 failed at the posting step (403), this one failed at the review step (errexit).

Fixed with set +e + explicit || true, so the verdict lands in the advisory comment rather than the check status.

Also worth logging: the reviewer's pull_request_target rule has a false positive.

It blocked this PR with two findings pointing at:

  • .github/workflows/reviewer.yml:17 — # We deliberately do NOT use \pull_request_target`: that runs with a writable`
  • .github/workflows/reviewer-comment.yml:15 — # here safe, unlike \pull_request_target` + checkout of PR head.`

Both are comments, in prose explaining why the trigger is deliberately avoided. The rule matches the bare string anywhere in the file rather than an actual on: trigger key, so documenting the risk trips the rule that exists to catch it.

Not fixable here (the reviewer is pinned at v0.1.2); I'll fix the rule in Signetry/reviewer to match the parsed trigger rather than raw text.

This was referenced Aug 18, 2026
Merged
Closed
@bkd-dotcom
bkd-dotcom merged commit e560d03 into main Aug 18, 2026
8 checks passed
@bkd-dotcom
bkd-dotcom deleted the fix/reviewer-fork-pr-comment branch August 18, 2026 19:13
bkd-dotcom added a commit to Signetry/reviewer that referenced this pull request Aug 18, 2026
@bkd-dotcom
fix(checks): CI rules fired on YAML comments; tighten pull_request_ta…
e632794 
…rget (#18)

Every rule in scan_ci_permissions matched raw added text, so a workflow
that *documented* a risk tripped the rule that exists to catch it.

Found in the field: Signetry/core#92 was returned Block with two findings
pointing at these lines —

  # We deliberately do NOT use `pull_request_target`: that runs with a writable
  #     here safe, unlike `pull_request_target` + checkout of PR head.

— i.e. prose explaining why the trigger is deliberately avoided. The same
hole applied to permissions: write-all, id-token: write and curl|sh.

  * Skip whole-line YAML comments. Inline trailing comments are still
    scanned on purpose: `#` is legal inside a quoted scalar, so stripping
    it by regex could hide real configuration.
  * ci.pull_request_target now matches the trigger rather than the string,
    in every form YAML allows: mapping key, `on:` scalar, inline sequence,
    and block sequence item. The pre-existing test only covered the
    `on: pull_request_target` scalar; running it caught that a key-only
    match would have regressed the other shapes.

Two new tests: all four trigger forms, and the five comment shapes that
previously produced false findings.

Co-authored-by: Binay <bkd-dotcom@users.noreply.github.com>
This was referenced Aug 18, 2026
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
bkd-dotcom added a commit to Signetry/plugins that referenced this pull request Aug 18, 2026
@bkd-dotcom
fix(ci): advisory reviewer could never comment on a fork PR (403) (#14)
0cfcc4d 
Rolls out Signetry/core#92, verified there end to end before propagating.

reviewer.yml runs on `pull_request` and requested `pull-requests: write`.
On a PR from a fork GitHub downgrades GITHUB_TOKEN to read-only
regardless of the `permissions:` block, so the grant was silently dropped
and issues.createComment failed with "Resource not accessible by
integration" (403). Every outside contribution showed a red `review`
check and never got the advisory comment.

Split into the standard trusted/untrusted pair:
  * reviewer.yml (untrusted, runs PR code) now holds no write permission
    and uploads the rendered comment + PR number as an artifact.
  * reviewer-comment.yml (trusted) runs on `workflow_run` from the default
    branch, where the token is writable, and posts it. It never checks
    out, builds, or executes PR code.

Deliberately not `pull_request_target`: that grants a writable token in
the base repo's context, and checking out PR head code under it is an RCE
footgun.

Also fixes a second bug found while testing the first: the advisory step's
trailing `exit 0` had never run, because GitHub invokes `run:` steps as
`bash -e`, so any non-zero reviewer exit (a Block verdict) aborted the
step first and turned the check red — contradicting the workflow's own
"never blocks the PR" contract.

Bumps the signetry-reviewer pin to v0.2.0, which carries the fix for CI
rules firing on YAML comments (a workflow that documented a risk tripped
the rule meant to catch it).

Co-authored-by: Binay <bkd-dotcom@users.noreply.github.com>
bkd-dotcom added a commit to Signetry/codex that referenced this pull request Aug 18, 2026
@bkd-dotcom
fix(ci): advisory reviewer could never comment on a fork PR (403) (#15)
7651b72 
Rolls out Signetry/core#92, verified there end to end before propagating.

reviewer.yml runs on `pull_request` and requested `pull-requests: write`.
On a PR from a fork GitHub downgrades GITHUB_TOKEN to read-only
regardless of the `permissions:` block, so the grant was silently dropped
and issues.createComment failed with "Resource not accessible by
integration" (403). Every outside contribution showed a red `review`
check and never got the advisory comment.

Split into the standard trusted/untrusted pair:
  * reviewer.yml (untrusted, runs PR code) now holds no write permission
    and uploads the rendered comment + PR number as an artifact.
  * reviewer-comment.yml (trusted) runs on `workflow_run` from the default
    branch, where the token is writable, and posts it. It never checks
    out, builds, or executes PR code.

Deliberately not `pull_request_target`: that grants a writable token in
the base repo's context, and checking out PR head code under it is an RCE
footgun.

Also fixes a second bug found while testing the first: the advisory step's
trailing `exit 0` had never run, because GitHub invokes `run:` steps as
`bash -e`, so any non-zero reviewer exit (a Block verdict) aborted the
step first and turned the check red — contradicting the workflow's own
"never blocks the PR" contract.

Bumps the signetry-reviewer pin to v0.2.0, which carries the fix for CI
rules firing on YAML comments (a workflow that documented a risk tripped
the rule meant to catch it).

Co-authored-by: Binay <bkd-dotcom@users.noreply.github.com>
bkd-dotcom added a commit to Signetry/cursor that referenced this pull request Aug 18, 2026
@bkd-dotcom
fix(ci): advisory reviewer could never comment on a fork PR (403) (#13)
31c2b2a 
Rolls out Signetry/core#92, verified there end to end before propagating.

reviewer.yml runs on `pull_request` and requested `pull-requests: write`.
On a PR from a fork GitHub downgrades GITHUB_TOKEN to read-only
regardless of the `permissions:` block, so the grant was silently dropped
and issues.createComment failed with "Resource not accessible by
integration" (403). Every outside contribution showed a red `review`
check and never got the advisory comment.

Split into the standard trusted/untrusted pair:
  * reviewer.yml (untrusted, runs PR code) now holds no write permission
    and uploads the rendered comment + PR number as an artifact.
  * reviewer-comment.yml (trusted) runs on `workflow_run` from the default
    branch, where the token is writable, and posts it. It never checks
    out, builds, or executes PR code.

Deliberately not `pull_request_target`: that grants a writable token in
the base repo's context, and checking out PR head code under it is an RCE
footgun.

Also fixes a second bug found while testing the first: the advisory step's
trailing `exit 0` had never run, because GitHub invokes `run:` steps as
`bash -e`, so any non-zero reviewer exit (a Block verdict) aborted the
step first and turned the check red — contradicting the workflow's own
"never blocks the PR" contract.

Bumps the signetry-reviewer pin to v0.2.0, which carries the fix for CI
rules firing on YAML comments (a workflow that documented a risk tripped
the rule meant to catch it).

Co-authored-by: Binay <bkd-dotcom@users.noreply.github.com>
bkd-dotcom added a commit to Signetry/precommit that referenced this pull request Aug 18, 2026
@bkd-dotcom
fix(ci): advisory reviewer could never comment on a fork PR (403) (#14)
9b8dd5e 
Rolls out Signetry/core#92, verified there end to end before propagating.

reviewer.yml runs on `pull_request` and requested `pull-requests: write`.
On a PR from a fork GitHub downgrades GITHUB_TOKEN to read-only
regardless of the `permissions:` block, so the grant was silently dropped
and issues.createComment failed with "Resource not accessible by
integration" (403). Every outside contribution showed a red `review`
check and never got the advisory comment.

Split into the standard trusted/untrusted pair:
  * reviewer.yml (untrusted, runs PR code) now holds no write permission
    and uploads the rendered comment + PR number as an artifact.
  * reviewer-comment.yml (trusted) runs on `workflow_run` from the default
    branch, where the token is writable, and posts it. It never checks
    out, builds, or executes PR code.

Deliberately not `pull_request_target`: that grants a writable token in
the base repo's context, and checking out PR head code under it is an RCE
footgun.

Also fixes a second bug found while testing the first: the advisory step's
trailing `exit 0` had never run, because GitHub invokes `run:` steps as
`bash -e`, so any non-zero reviewer exit (a Block verdict) aborted the
step first and turned the check red — contradicting the workflow's own
"never blocks the PR" contract.

Bumps the signetry-reviewer pin to v0.2.0, which carries the fix for CI
rules firing on YAML comments (a workflow that documented a risk tripped
the rule meant to catch it).

Co-authored-by: Binay <bkd-dotcom@users.noreply.github.com>
bkd-dotcom added a commit to Signetry/claude-code that referenced this pull request Aug 18, 2026
@bkd-dotcom
fix(ci): advisory reviewer could never comment on a fork PR (403) (#14)
006add8 
Rolls out Signetry/core#92, verified there end to end before propagating.

reviewer.yml runs on `pull_request` and requested `pull-requests: write`.
On a PR from a fork GitHub downgrades GITHUB_TOKEN to read-only
regardless of the `permissions:` block, so the grant was silently dropped
and issues.createComment failed with "Resource not accessible by
integration" (403). Every outside contribution showed a red `review`
check and never got the advisory comment.

Split into the standard trusted/untrusted pair:
  * reviewer.yml (untrusted, runs PR code) now holds no write permission
    and uploads the rendered comment + PR number as an artifact.
  * reviewer-comment.yml (trusted) runs on `workflow_run` from the default
    branch, where the token is writable, and posts it. It never checks
    out, builds, or executes PR code.

Deliberately not `pull_request_target`: that grants a writable token in
the base repo's context, and checking out PR head code under it is an RCE
footgun.

Also fixes a second bug found while testing the first: the advisory step's
trailing `exit 0` had never run, because GitHub invokes `run:` steps as
`bash -e`, so any non-zero reviewer exit (a Block verdict) aborted the
step first and turned the check red — contradicting the workflow's own
"never blocks the PR" contract.

Bumps the signetry-reviewer pin to v0.2.0, which carries the fix for CI
rules firing on YAML comments (a workflow that documented a risk tripped
the rule meant to catch it).

Co-authored-by: Binay <bkd-dotcom@users.noreply.github.com>
bkd-dotcom added a commit to Signetry/signetry that referenced this pull request Aug 18, 2026
@bkd-dotcom
fix(ci): advisory reviewer could never comment on a fork PR (403) (#27)
f5d4b9a 
Rolls out Signetry/core#92, verified there end to end before propagating.

reviewer.yml runs on `pull_request` and requested `pull-requests: write`.
On a PR from a fork GitHub downgrades GITHUB_TOKEN to read-only
regardless of the `permissions:` block, so the grant was silently dropped
and issues.createComment failed with "Resource not accessible by
integration" (403). Every outside contribution showed a red `review`
check and never got the advisory comment.

Split into the standard trusted/untrusted pair:
  * reviewer.yml (untrusted, runs PR code) now holds no write permission
    and uploads the rendered comment + PR number as an artifact.
  * reviewer-comment.yml (trusted) runs on `workflow_run` from the default
    branch, where the token is writable, and posts it. It never checks
    out, builds, or executes PR code.

Deliberately not `pull_request_target`: that grants a writable token in
the base repo's context, and checking out PR head code under it is an RCE
footgun.

Also fixes a second bug found while testing the first: the advisory step's
trailing `exit 0` had never run, because GitHub invokes `run:` steps as
`bash -e`, so any non-zero reviewer exit (a Block verdict) aborted the
step first and turned the check red — contradicting the workflow's own
"never blocks the PR" contract.

Bumps the signetry-reviewer pin to v0.2.0, which carries the fix for CI
rules firing on YAML comments (a workflow that documented a risk tripped
the rule meant to catch it).

Co-authored-by: Binay <bkd-dotcom@users.noreply.github.com>
bkd-dotcom added a commit to Signetry/github-app that referenced this pull request Aug 18, 2026
@bkd-dotcom
fix(ci): advisory reviewer could never comment on a fork PR (403) (#11)
6c22fac 
Rolls out Signetry/core#92, verified there end to end before propagating.

reviewer.yml runs on `pull_request` and requested `pull-requests: write`.
On a PR from a fork GitHub downgrades GITHUB_TOKEN to read-only
regardless of the `permissions:` block, so the grant was silently dropped
and issues.createComment failed with "Resource not accessible by
integration" (403). Every outside contribution showed a red `review`
check and never got the advisory comment.

Split into the standard trusted/untrusted pair:
  * reviewer.yml (untrusted, runs PR code) now holds no write permission
    and uploads the rendered comment + PR number as an artifact.
  * reviewer-comment.yml (trusted) runs on `workflow_run` from the default
    branch, where the token is writable, and posts it. It never checks
    out, builds, or executes PR code.

Deliberately not `pull_request_target`: that grants a writable token in
the base repo's context, and checking out PR head code under it is an RCE
footgun.

Also fixes a second bug found while testing the first: the advisory step's
trailing `exit 0` had never run, because GitHub invokes `run:` steps as
`bash -e`, so any non-zero reviewer exit (a Block verdict) aborted the
step first and turned the check red — contradicting the workflow's own
"never blocks the PR" contract.

Bumps the signetry-reviewer pin to v0.2.0, which carries the fix for CI
rules firing on YAML comments (a workflow that documented a risk tripped
the rule meant to catch it).

Co-authored-by: Binay <bkd-dotcom@users.noreply.github.com>
bkd-dotcom added a commit to Signetry/action that referenced this pull request Aug 18, 2026
@bkd-dotcom
fix(ci): advisory reviewer could never comment on a fork PR (403) (#29)
f54d4bc 
Rolls out Signetry/core#92, verified there end to end before propagating.

reviewer.yml runs on `pull_request` and requested `pull-requests: write`.
On a PR from a fork GitHub downgrades GITHUB_TOKEN to read-only
regardless of the `permissions:` block, so the grant was silently dropped
and issues.createComment failed with "Resource not accessible by
integration" (403). Every outside contribution showed a red `review`
check and never got the advisory comment.

Split into the standard trusted/untrusted pair:
  * reviewer.yml (untrusted, runs PR code) now holds no write permission
    and uploads the rendered comment + PR number as an artifact.
  * reviewer-comment.yml (trusted) runs on `workflow_run` from the default
    branch, where the token is writable, and posts it. It never checks
    out, builds, or executes PR code.

Deliberately not `pull_request_target`: that grants a writable token in
the base repo's context, and checking out PR head code under it is an RCE
footgun.

Also fixes a second bug found while testing the first: the advisory step's
trailing `exit 0` had never run, because GitHub invokes `run:` steps as
`bash -e`, so any non-zero reviewer exit (a Block verdict) aborted the
step first and turned the check red — contradicting the workflow's own
"never blocks the PR" contract.

Bumps the signetry-reviewer pin to v0.2.0, which carries the fix for CI
rules firing on YAML comments (a workflow that documented a risk tripped
the rule meant to catch it).

Co-authored-by: Binay <bkd-dotcom@users.noreply.github.com>
bkd-dotcom added a commit that referenced this pull request Aug 18, 2026
@bkd-dotcom
chore(ci): bump signetry-reviewer pin to v0.2.0 (#101)
6b2e262 
Picks up the fix for CI rules firing on YAML comments — this repo's own
#92 was returned a Block verdict for two comment lines explaining why it
deliberately avoids pull_request_target, which is what surfaced that bug.

Also brings in the new supply.dependency_skew check (lockfile changed
without its manifest), advisory and MEDIUM so it withholds auto-merge
without rejecting Dependabot refreshes.

Co-authored-by: Binay <bkd-dotcom@users.noreply.github.com>
bkd-dotcom added a commit to Signetry/eval that referenced this pull request Aug 18, 2026
@bkd-dotcom
fix(ci): advisory reviewer could never comment on a fork PR (403) (#32)
053900d 
Rolls out Signetry/core#92, verified there end to end before propagating.

reviewer.yml runs on `pull_request` and requested `pull-requests: write`.
On a PR from a fork GitHub downgrades GITHUB_TOKEN to read-only
regardless of the `permissions:` block, so the grant was silently dropped
and issues.createComment failed with "Resource not accessible by
integration" (403). Every outside contribution showed a red `review`
check and never got the advisory comment.

Split into the standard trusted/untrusted pair:
  * reviewer.yml (untrusted, runs PR code) now holds no write permission
    and uploads the rendered comment + PR number as an artifact.
  * reviewer-comment.yml (trusted) runs on `workflow_run` from the default
    branch, where the token is writable, and posts it. It never checks
    out, builds, or executes PR code.

Deliberately not `pull_request_target`: that grants a writable token in
the base repo's context, and checking out PR head code under it is an RCE
footgun.

Also fixes a second bug found while testing the first: the advisory step's
trailing `exit 0` had never run, because GitHub invokes `run:` steps as
`bash -e`, so any non-zero reviewer exit (a Block verdict) aborted the
step first and turned the check red — contradicting the workflow's own
"never blocks the PR" contract.

Bumps the signetry-reviewer pin to v0.2.0, which carries the fix for CI
rules firing on YAML comments (a workflow that documented a risk tripped
the rule meant to catch it).

Co-authored-by: Binay <bkd-dotcom@users.noreply.github.com>
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

@bkd-dotcom