Repository navigation
feat: add the shared PR governance action - #49
Conversation
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (2)
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67f5b206ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let review = risk.tier === 'R0' | ||
| ? { provider: policy.reviewer.provider, state: 'success', reason: 'review_not_required', comment_id: null, | ||
| format: null, commit_sha: null, coverage: null, unresolved: null } | ||
| : reviewerEvidence(snapshot, policy, now); |
There was a problem hiding this comment.
For an R0-only change, such as a plain-text edit to README.md, this branch publishes successful ai-review and risk-gate statuses without examining reviewer evidence. It also ignores an existing receipt reporting unresolved findings unless an unresolved inline thread exists. This bypasses the current-head Codex receipt requirement in AGENTS.md and contradicts the README's R0 requirements. Evaluate reviewer evidence for R0 changes rather than synthesizing a successful review.
AGENTS.md reference: AGENTS.md:L92-L92
Useful? React with 👍 / 👎.
| const branch = await defaultBranch(github, context); | ||
| const { policy, policyCommit, policySha } = await loadPolicy(github, context, branch); | ||
| return run({ github, context, core, policy, policySha, policyCommit, defaultBranch: branch }); |
There was a problem hiding this comment.
If policy retrieval, parsing, validation, or owner configuration fails after a head has received successful statuses, runGate throws before calling the runner. Consequently, the runner's publishUnavailable path is unreachable from this entry point, leaving the previous pr-template, ai-review, and risk-gate successes intact. In the supplied risk-gate.yml template, branch protection consumes these contexts rather than the failed Evaluate PR governance job, so a subsequent invalidating event can leave the PR mergeable. Catch policy-loading failures and invoke the runner's unavailable-policy handling to replace those stale successes with blocking statuses.
Useful? React with 👍 / 👎.
| findings = scan(binary, 'git', root, Path(directory) / 'range.json', | ||
| config=config, log_opts=f'{base}..{head}') |
There was a problem hiding this comment.
When a credential is introduced only while resolving a merge conflict, the PR scan misses it: this explicit log_opts value replaces scan()'s merge-aware default with a bare revision range. Gitleaks documents that it scans git log -p patches, and the Git manual states that without an explicit merge-diff option, “merge commits will not show a diff.” The history scan handles this case, but the Gitleaks required check wired through templates/gitleaks.yml uses this range path and can pass before the credential is merged. Include --diff-merges=first-parent alongside the range and extend the existing merge-resolution regression test to cover range_scan.
Useful? React with 👍 / 👎.
| if (context.eventName === 'pull_request_target' && context.payload?.action === 'labeled') { | ||
| return recordLabel({ github, context, core }); |
There was a problem hiding this comment.
When a clearing label is added, both record_label and its dependent evaluate job in pr-governance/templates/risk-gate.yml invoke this same entry point with mode: gate and the same labeled payload. This early return therefore makes both jobs record the label without evaluating governance or updating risk-gate; invoking the entry point twice confirms that it writes only two clearing-label statuses. A reviewed R2 PR remains blocked after agent-approved or owner-approved is added until another qualifying event or the six-hour sweep occurs, contrary to the label-triggered re-evaluation required by AGENTS.md. Give recording and evaluation distinct dispatch paths so the dependent job actually runs the evaluator.
AGENTS.md reference: AGENTS.md:L92-L92
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Here are some automated security review suggestions for this pull request.
Reviewed commit: 67f5b206ed
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
| history = before.statuses; | ||
| if (before.pr.head.sha !== head) throw new Error('Head changed during collection'); | ||
| const dependabot = gate.dependabotEvidence(before, policy); | ||
| if (before.pr.base.sha !== policyCommit) { |
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Only in consumers that install this gate without strict/up-to-date branch protection, verify that the reviewed head actually contains the live base. Line 285 compares pull_request.base.sha with the policy commit but never checks Git ancestry, while the receipt is bound only to the unchanged head. After the base advances, an unexpired receipt (and any R2 label) can therefore produce fresh ai-review/risk-gate successes for a head reviewed against the old base, letting an external PR author plus a merge-capable actor land an unreviewed combined change. Query the merge base and fail unless the live base is an ancestor. Shared-workflow build/document integrity is in scope.
SECURITY.md reference: SECURITY.md:L19-L20
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Pigbibi
added
the
agent-approved
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Here are some automated security review suggestions for this pull request.
Reviewed commit: 7263610194
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
| const lines = patch.split('\n'); | ||
| assert(lines.filter(line => line.startsWith('+')).length === file.additions | ||
| && lines.filter(line => line.startsWith('-')).length === file.deletions, 'Incomplete patch'); | ||
| assert(patch || (file.additions === 0 && file.deletions === 0 && file.status === 'renamed'), 'Missing patch'); |
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
In consumers where renamed binary assets affect a build or publication, verify blob identity before this exception. GitHub can represent a content-changing binary rename with no patch and zero textual additions/deletions, so the gate accepts it as complete; the reproduced R1 snapshot publishes all three successes once a clean current-head receipt exists. A PR author can therefore hide changed executable or asset bytes behind a rename without an R2 label. Compare the old and new blob OIDs, or reject patchless renames.
SECURITY.md reference: SECURITY.md:L19-L20
Useful? React with 👍 / 👎.
AI assistance
Codex and the Qoder agent wrote this change. The reviewer is Codex; any coding agent may submit a pull request.
What changed
Adds the organization-wide
pr-governanceaction and its caller templates.RISK_GATE_OWNER_ID/RISK_GATE_OWNER; committed policy files carry no account identity, and a missing configuration fails the check.PR review signalworkflow and evaluated by a trustedworkflow_run.secret_scan.pyno longer decodes git patch output as UTF-8, so repositories with non-UTF-8 history scan cleanly.Actortype exposes, so evaluations no longer fail withEvaluation failed.How verified
node --testsix suites: 159 tests pass.test_secret_scan.py: 40 tests pass against Gitleaks 8.30.1.actionlintreports no errors on the templates or the test workflow.Risk
R2: this is the trust anchor for consumer repositories. Consumer changes follow separately.