Repository navigation
fix(codeql): resolve live base identity from authenticated branch refs - #2657
Open
seonghobae wants to merge 1 commit into
Open
seonghobae wants to merge 1 commit into
seonghobae wants to merge 1 commit into
Conversation
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 34 minutes. View limit details
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. Comment |
seonghobae
marked this pull request as ready for review
October 15, 2026 11:10
![cwl-noema-review[bot]](https://avatars.githubusercontent.com/u/295022177?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Noema LLM review
Approve. The change consistently replaces stale pull-request base snapshots with authenticated current branch tips, fails closed on lookup or identity mismatches, and preserves settlement safety when the base branch moves.
Reviewed changed lines
.github/workflows/codeql-pr.yml:356 (RIGHT): The coordinator resolves the named base branch through the Git refs API and requires an exact refs/heads identity and commit-shaped SHA before constructing dispatch evidence..github/workflows/codeql-pr.yml:730 (RIGHT): The receipt reader uses the same authenticated branch-tip resolution before deriving the expected status context, preventing stale PR metadata from accepting an obsolete receipt..github/workflows/codeql-scan-dispatch.yml:381 (RIGHT): Dispatch admission is bound to the live authenticated base branch and fails if the supplied base identity does not match it..github/workflows/codeql-scan-dispatch.yml:547 (RIGHT): The privileged pre-scan revalidation repeats the authenticated branch lookup, preserving the live-base check at the security boundary..github/workflows/codeql-scan-dispatch.yml:1047 (RIGHT): Settlement rejects wake-up when the current authenticated branch tip differs from the base identity bound to the scan.
Adversarial validation
.github/workflows/codeql-scan-dispatch.yml:381 (RIGHT)falsified: A stale pull-request base.sha could either reject a valid dispatch or admit a dispatch against the wrong base. — The new validation derives live_base_sha from the exact branch ref and compares the supplied identity against that value; the added stale-snapshot contract test exercises this case..github/workflows/codeql-pr.yml:356 (RIGHT)falsified: A slash-containing or otherwise encoded base branch could resolve the wrong ref or permit endpoint ambiguity. — The branch name is URI-encoded before the Git refs request and the response is required to match the exact refs/heads value; the coordinator contract test asserts the encoded path and resulting payload identity..github/workflows/codeql-scan-dispatch.yml:1047 (RIGHT)falsified: A base branch moved after scanning could still wake the required run because settlement only trusted the historical PR snapshot. — Settlement now fetches the current branch ref and exits before posting the wake-up when its SHA differs; the added changed-branch settlement test asserts that no post occurs..github/workflows/codeql-scan-dispatch.yml:381 (RIGHT)falsified: A ref transport failure could trigger fallback to the historical PR base and admit an unverified dispatch. — The new lookup is executed under fail-fast shell handling with no fallback assignment; the added transport-failure test requires a nonzero result and no dispatch.- Residual risk: No confirmed regression found. A narrow unavoidable TOCTOU window remains between ref lookup and later GitHub operations, but the dispatch handler and settlement boundary revalidate the live branch identity and fail closed.
Findings
- No blocking findings.
- Result: APPROVE
- Head SHA:
e04587abd3cdd228651bf8b64d71a5efd6f29efc - Reviewer credential:
noema-review-github-app-refresh - Actor:
cwl-noema-review[bot]
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head product diff. Coverage is a separate gate.
Changed files
.github/workflows/codeql-pr.yml— GitHub Actions review job.github/workflows/codeql-scan-dispatch.yml— GitHub Actions review jobtests/test_codeql_pr_workflow_contract.py— regression suitetests/test_codeql_scan_dispatch_workflow_contract.py— regression suite
Changed behavior
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: codeql-pr.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: codeql-pr.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Workflow: codeql-scan-dispatch.yml"]
S2 --> I2["GitHub Actions review job"]
I2 --> R2["Review risk: Workflow: codeql-scan-dispatch.yml"]
R2 --> V2["actionlint plus required checks"]
Evidence --> S3["Test: test_codeql_pr_workflow_contract.py (2 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test: test_codeql_pr_workflow_contract.py (2 files)"]
R3 --> V3["targeted test run"]
Findings
No source-backed product finding is synthesized from the coverage gate. A coverage miss belongs in the status comment.
- Head SHA:
e04587abd3cdd228651bf8b64d71a5efd6f29efc - Workflow run: 38725543518
- Workflow attempt: 1
- Coverage gate:
failure
Review outcome
Coverage is a gate, not the review. This body reviews the changed product files.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: codeql-pr.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: codeql-pr.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Workflow: codeql-scan-dispatch.yml"]
S2 --> I2["GitHub Actions review job"]
I2 --> R2["Review risk: Workflow: codeql-scan-dispatch.yml"]
R2 --> V2["actionlint plus required checks"]
Evidence --> S3["Test: test_codeql_pr_workflow_contract.py (2 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test: test_codeql_pr_workflow_contract.py (2 files)"]
R3 --> V3["targeted test run"]
Contributor
OpenCode Review Overview
Coverage evidence did not pass, so approval is blocked. The formal pull-request review is the source-backed diff review, not this status comment. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Observed failure
CodeQL dispatch run 38724148386 for #2642 head 8864077 rejects the producer merge because the PR object's base.sha is af81ef4, while the actual main ref and producer merge's first parent are 6a03ae9. A fresh substantive head reproduced the mismatch. The named branch ref, not an old PR base snapshot, must supply current base identity.
Repair
Read the authenticated exact branch ref in all five affected boundaries: receipt reader, coordinator, dispatch admission, pre-scan revalidation and required-run settlement. Encode branch names in the API path and require the exact returned ref, commit object type and canonical 40-hex SHA.
The existing exact merge-parent/head/source checks, actor/repository admission, required-run/language-job receipt identity, permissions, scanner gates and stale-run refusal remain. Missing, malformed, wrong or unavailable refs fail closed; there is no fallback to historical PR base.sha. No branch protection, token, ACL or runner registration is changed.
Verified scope
Integration boundary
Ready for normal independent review after source repair and local verification, not operational acceptance. Protected repository_dispatch executes the handler from main: an unmerged PR client cannot prove the new protected handler. Keep exact-head remote security/CodeQL/review checks and normal protected integration requirements; do not bypass them or treat predecessor or skipped checks as acceptance. After protected handler integration, affected consumers need fresh source/run-bound evidence.
Existing broad owners #2040 and #2051 remain untouched; this is a narrow live-reference correction on current main.