Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 96 additions & 0 deletions .github/workflows/reviewer-comment.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
name: Reviewer comment

# Trusted half of the advisory reviewer (see reviewer.yml).
#
# The `Reviewer` workflow runs against untrusted PR code with a read-only token
# and cannot post its own comment — on a fork PR GitHub downgrades
# GITHUB_TOKEN to read-only, so `pull-requests: write` there is silently
# dropped and the API call 403s. This workflow runs from the default branch in
# the base repo's context, where the token *is* writable, and posts the comment
# the other workflow staged as an artifact.
#
# Security properties that must be preserved:
# * It never checks out, builds, or executes PR code — it only reads two
# files out of an artifact. This is what makes holding a writable token
# here safe, unlike `pull_request_target` + checkout of PR head.
# * The PR number is filtered to digits on both sides before use.
# * The comment body is passed to the API as data (never through a shell or
# `eval`), so a crafted diff cannot escalate beyond comment text.
on:
workflow_run:
workflows: ["Reviewer"]
types: [completed]

permissions:
contents: read
actions: read # download the artifact from the triggering run
pull-requests: write # post/update the advisory review comment

jobs:
comment:
runs-on: ubuntu-latest
# Only for PR-triggered runs. Note we do not gate on the triggering run's
# conclusion: the reviewer job exits 0 by design (advisory), and we still
# want a comment when it produced a partial result.
if: github.event.workflow_run.event == 'pull_request'
steps:
- name: Download the staged review comment
id: dl
continue-on-error: true
uses: actions/download-artifact@v4
with:
name: signetry-reviewer-comment
path: staged
run-id: ${{ github.event.workflow_run.id }}
github-token: ${{ github.token }}

- name: Post / update the review comment
if: steps.dl.outcome == 'success'
uses: actions/github-script@v9
with:
script: |
const fs = require('fs');

// PR number: digits only. workflow_run.pull_requests is empty for
// forks, so the number is carried in the artifact instead.
let raw = '';
try { raw = fs.readFileSync('staged/pr-number', 'utf8'); } catch (e) {}
const issue_number = parseInt(String(raw).replace(/\D/g, ''), 10);
if (!Number.isInteger(issue_number) || issue_number <= 0) {
core.info('No usable PR number in the artifact; nothing to post.');
return;
}

let body = '';
try { body = fs.readFileSync('staged/comment.md', 'utf8'); } catch (e) {}
if (!body.trim()) body = 'Signetry Reviewer: no review was produced.';

const marker = '<!-- signetry-reviewer -->';
// GitHub rejects bodies over 65536 chars.
const LIMIT = 65000;
if (body.length > LIMIT) {
body = body.slice(0, LIMIT) + '\n\n_(truncated)_';
}
body = marker + '\n' + body;

const { owner, repo } = context.repo;
const comments = await github.paginate(
github.rest.issues.listComments,
{ owner, repo, issue_number, per_page: 100 },
);
// Only ever update a comment we authored, so a contributor cannot
// get the bot to overwrite someone else's comment by planting the
// marker in a PR description or comment.
const mine = comments.find(
c => c.body
&& c.body.includes(marker)
&& c.user
&& c.user.type === 'Bot',
);
if (mine) {
await github.rest.issues.updateComment({ owner, repo, comment_id: mine.id, body });
core.info(`Updated comment ${mine.id} on #${issue_number}.`);
} else {
await github.rest.issues.createComment({ owner, repo, issue_number, body });
core.info(`Created a comment on #${issue_number}.`);
}
65 changes: 44 additions & 21 deletions .github/workflows/reviewer.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,23 @@ name: Reviewer
# and posts one recommendation comment. Advisory only — it never merges and never
# fails the PR. Changes to security-sensitive surfaces (workflows, packaging) are
# escalated to a human. See https://github.com/Signetry/reviewer
#
# This job runs against untrusted PR code, so it deliberately holds NO write
# permission. On a fork PR, GitHub downgrades GITHUB_TOKEN to read-only
# regardless of the `permissions:` block, which is why requesting
# `pull-requests: write` here failed with "Resource not accessible by
# integration" (403) on every outside contribution. The rendered comment is
# handed to the trusted `Reviewer comment` workflow (reviewer-comment.yml) via
# an artifact and posted from there.
#
# We deliberately do NOT use `pull_request_target`: that runs with a writable
# token in the base repo's context, and checking out PR head code under it is a
# remote-code-execution footgun.
on:
pull_request:

permissions:
contents: read
pull-requests: write
checks: read

jobs:
Expand All @@ -24,7 +35,7 @@ jobs:
python-version: "3.12"
- name: Install signetry-reviewer
# source-available (All Rights Reserved); install from source, not PyPI.
run: pip install "signetry-reviewer @ git+https://github.com/Signetry/reviewer@v0.1.2"
run: pip install "signetry-reviewer @ git+https://github.com/Signetry/reviewer@v0.2.0"
- name: Compute the PR diff
env:
BASE_SHA: ${{ github.event.pull_request.base.sha }}
Expand All @@ -39,6 +50,9 @@ jobs:
env:
PR: ${{ github.event.pull_request.number }}
run: |
# GitHub runs `run:` steps as `bash -e`, so a non-zero exit from the
# reviewer aborted the step before the trailing `exit 0` could run.
set +e
set -uo pipefail
signetry-reviewer review \
--diff "$OUT/pr.diff" \
Expand All @@ -47,25 +61,34 @@ jobs:
--required-check unknown \
--protected ".github/workflows/*,.github/actions/**,action.yml,pyproject.toml" \
--comment-out "$OUT/comment.md" \
--json-out "$OUT/review.json"
--json-out "$OUT/review.json" || true
exit 0
- name: Post / update the review comment

# Hand the rendered comment to the trusted workflow. This job cannot post
# it itself: it has no write permission, and on fork PRs it could not be
# granted one. The PR number travels in the artifact because
# `workflow_run.pull_requests` is empty for forks.
- name: Stage the review comment for the trusted poster
if: always()
env:
PR: ${{ github.event.pull_request.number }}
run: |
set -uo pipefail
mkdir -p "${RUNNER_TEMP:-/tmp}/rev-out"
if [ -f "$OUT/comment.md" ]; then
cp "$OUT/comment.md" "${RUNNER_TEMP:-/tmp}/rev-out/comment.md"
else
printf 'Signetry Reviewer: no review was produced.\n' \
> "${RUNNER_TEMP:-/tmp}/rev-out/comment.md"
fi
# Digits only — this value is read back by the trusted workflow.
printf '%s' "$PR" | tr -cd '0-9' > "${RUNNER_TEMP:-/tmp}/rev-out/pr-number"

- name: Upload the review comment artifact
if: always()
uses: actions/github-script@v9
uses: actions/upload-artifact@v4
with:
script: |
const fs = require('fs'); const path = require('path');
const out = process.env.OUT || '';
let body = 'Signetry Reviewer: no review was produced.';
try { body = fs.readFileSync(path.join(out, 'comment.md'), 'utf8'); } catch (e) {}
const marker = '<!-- signetry-reviewer -->';
body = marker + '\n' + body;
const { owner, repo } = context.repo;
const issue_number = context.issue.number;
const comments = await github.rest.issues.listComments({ owner, repo, issue_number });
const mine = comments.data.find(c => c.body && c.body.includes(marker));
if (mine) {
await github.rest.issues.updateComment({ owner, repo, comment_id: mine.id, body });
} else {
await github.rest.issues.createComment({ owner, repo, issue_number, body });
}
name: signetry-reviewer-comment
path: ${{ runner.temp }}/rev-out
retention-days: 3
if-no-files-found: warn
Loading