Repository navigation
Conversation
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ 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. |
![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: 6b62ff313d
ℹ️ 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".
| const data = await github.graphql(`query($owner:String!,$repo:String!,$number:Int!,$cursor:String) { | ||
| repository(owner:$owner,name:$repo) { pullRequest(number:$number) { | ||
| reviewThreads(first:100,after:$cursor) { | ||
| nodes { id isResolved isOutdated comments(first:1) { nodes { author { login databaseId } } } } |
There was a problem hiding this comment.
Every gate evaluation that reaches collect() sends this query, but author has the GraphQL Actor interface, whose documented fields do not include databaseId. GitHub rejects the query even when the PR has no review threads, so collection fails and the gate publishes errors instead of evaluating any PR. Request the numeric ID inside appropriate concrete-type fragments, or omit it and use the existing login fallback.
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 templates/risk-gate.yml invoke this same entry point with mode: gate and the same event payload. Consequently, both calls return from this branch after recording the label; neither evaluates the PR or updates risk-gate. An otherwise mergeable high-risk PR remains blocked until another event, manual dispatch, or the six-hour sweep. Give recording a separate operation so the dependent evaluation actually runs.
AGENTS.md reference: AGENTS.md:L92-L92
Useful? React with 👍 / 👎.
| if (context.eventName === 'push' || context.eventName === 'schedule') { | ||
| return (await openPullRequests()).map(pull => pull.number); | ||
| } | ||
| if (context.eventName === 'pull_request_target' || context.eventName === 'pull_request') { |
There was a problem hiding this comment.
The inspected templates/risk-gate.yml subscribes to both pull_request_review and pull_request_review_comment, but neither event is accepted here, so each reaches UNSUPPORTED_EVENT before evaluation or blocking-status publication. If Codex adds an unresolved review thread after a successful verdict, the old successful ai-review and risk-gate statuses remain until another supported event or the scheduled sweep, despite unresolvedReviewerThreads() being intended to block that PR. Route both events to their payload's PR number.
AGENTS.md reference: AGENTS.md:L92-L92
Useful? React with 👍 / 👎.
| 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 the policy API read fails or the trusted policy becomes invalid after this head previously received successful statuses, loadPolicy() throws before runner.run() is entered. This bypasses the runner's existing publishUnavailable() path and leaves the old successful required contexts untouched. The README's branch-protection setup requires the three published contexts, not the Evaluate PR governance job, so failing that job alone does not invalidate the previous verdict. Catch policy-loading failures and invoke the unavailable-policy handling so affected heads receive blocking statuses.
Useful? React with 👍 / 👎.
| verify_range(root, base, head) | ||
| with tempfile.TemporaryDirectory(prefix='gitleaks-', dir=os.environ.get('RUNNER_TEMP')) as directory: | ||
| findings = scan(binary, 'git', root, Path(directory) / 'range.json', | ||
| config=config, log_opts=f'{base}..{head}') |
There was a problem hiding this comment.
If a credential is introduced only while resolving a merge conflict, the range scan used by templates/gitleaks.yml misses it: this explicit log_opts replaces scan()'s merge-aware default with only the revision range. The pinned Gitleaks implementation appends those options to git log -p -U0, and the Git documentation states that “merge commits will not show a diff” without an explicit merge-diff option. A local conflicting-merge reproduction confirmed that the synthetic credential appears only after adding --diff-merges=first-parent. Preserve that option alongside the range so the required scan cannot pass a merge-only credential that the weekly history scan would later detect.
Useful? React with 👍 / 👎.
|
用同内容的单提交 PR #49 取代：旧分支的首个提交主题超过 commitlint 的 72 字符上限，且内容仍是修复前的门禁实现。 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ac348ed11
ℹ️ 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 prose-only change, this branch publishes successful ai-review and risk-gate statuses without inspecting the reviewer receipt—even if the newest receipt reports unresolved findings. Evaluating the supplied README fixture with no comments also returns allowed: true. This bypasses the current-head Codex review requirement in AGENTS.md and the README's stated requirement that R0 changes have a reviewer receipt. Evaluate reviewer evidence for R0 changes as well, rather than synthesizing a successful review.
AGENTS.md reference: AGENTS.md:L92-L92
Useful? React with 👍 / 👎.
| 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.
When the PR API reports the current target-branch tip in base.sha, this equality passes even if the PR head diverged before that tip and has not incorporated it. The later readback repeats the same comparison, so an existing head-only receipt can still produce successful governance statuses after the base advances. The canonical .github/workflows/risk-gate.yml explicitly checks this through headContainsBase() (lines 251–255), using the compare API's merge base; a repository-wide search finds no equivalent ancestry check in the new shared action. Restore that check against policyCommit before accepting reviewer evidence. Strict branch protection may independently block merging, but it does not make these published review verdicts accurate.
Useful? React with 👍 / 👎.
| if mode == 'git': | ||
| # Plain `git log -p` hides merge diffs, so a credential added only while | ||
| # resolving a conflict would never appear in the history scan. | ||
| command.append('--log-opts=' + (log_opts or '--all --diff-merges=first-parent')) |
There was a problem hiding this comment.
A repository can contain credentials.properties -diff in .gitattributes, causing both range and history scans to miss credentials in that file despite the trusted scanner configuration. Git documents that this produces “Binary files differ” instead of a textual patch, and the pinned Gitleaks implementation skips non-archive binary files. A local Git reproduction confirmed that the credential disappears from the exact patch command used here and reappears with --text; the existing history verification also succeeds because the suppressed diff is valid Git output. Prevent source-controlled attributes from hiding scan input, for example by forcing textual patches in both modes or scanning the underlying blobs.
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.