fix(codeql): name a status credential that is not configured - #1991
fix(codeql): name a status credential that is not configured#1991seonghobae wants to merge 1 commit into
Conversation
The four-rung credential ladder in "Publish CodeQL dispatch status" returns
before printing when a token is empty, so an unconfigured rung and one that was
never reached look identical in a run log: the label simply does not appear.
That hid half the ladder in production. In run 34017996201 -- one of the runs
created during the ~69-second window when the dispatch allowlist was open, so it
is one of the few that reached this step at all -- the log carries:
##[notice] ... using target-app-token did not succeed: HTTP 403
##[notice] ... using github-token did not succeed: HTTP 403
##[error] Could not publish ... after all configured credentials failed
Nothing for `pr-review-merge-token` or `opencode-approve-token`. The only way to
tell those two were empty rather than skipped was to count label occurrences in
the raw log: two for each configured rung (source echo plus runtime notice) and
one for each empty rung. That is not a diagnosis anyone should have to
reconstruct while an outage is live.
This changes no credential, permission or ordering -- it adds one line so an
absent rung says it is absent.
Worth recording alongside it, because the fix does not address it: the two rungs
that ARE configured both fail with `Resource not accessible by integration
(HTTP 403)` on a cross-repository status write. So the dispatch allowlist
(.github#1929) is necessary but not sufficient -- opening it lets runs reach this
step and stop here instead. Naming the empty rungs is what makes that second
break legible the next time a run gets through.
The test executes the published block under bash with a stub `gh` that refuses
every request, two rungs configured and two empty, and requires the four labels
to be distinguishable by outcome.
Gates: 2976 passed (2975 + 1, as predicted), coverage 100%, interrogate 100%.
Negative control: deleting the notice line fails exactly this test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 46 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
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 |
![cwl-noema-review[bot]](https://avatars.githubusercontent.com/u/295022177?s=60&v=4){kind=small}
There was a problem hiding this comment.
Noema LLM review
The PR improves observability in the CodeQL dispatch workflow by explicitly logging when a status credential is not configured, rather than letting it fail silently within the credential ladder. The logic change is safe and maintainable, and it is verified by a comprehensive test case that distinguishes between unconfigured and failed credentials.
Reviewed changed lines
.github/workflows/codeql-scan-dispatch.yml:440 (RIGHT): The addition of the notice for unconfigured credentials improves observability. The return code 1 correctly notifies the calling loop to move to the next rung. Hypothesis: the return 1 might terminate the job. Falsified: the calling code forpost_statusin this workflow is designed to handle failures of individual rungs by continuing the ladder until one succeeds or all fail.tests/test_codeql_scan_dispatch_workflow_contract.py:525 (RIGHT): The test provides high-fidelity validation of the fix. Probe 1: Verifies that labels with provided tokens result in failure messages from the stub. Probe 2: Verifies that labels with empty tokens result in the new 'credential is not configured' notice. The test correctly simulates the environment and captures stdout/stderr.
Adversarial validation
.github/workflows/codeql-scan-dispatch.yml:440 (RIGHT)falsified: Returning 1 from the helper function will cause the entire GitHub Actions job to exit prematurely. — Thepost_statusfunction is called within a loop intended to try multiple credentials (a ladder). The return code is used by the loop to determine if it should move to the next candidate token, not to terminate the job.tests/test_codeql_scan_dispatch_workflow_contract.py:525 (RIGHT)falsified: The test may pass even if the workflow logs are ambiguous because it doesn't strictly check for the absence of the failure message on empty tokens. — The test explicitly loops through('target-app-token', 'github-token')to verify failure messages and separately loops through('pr-review-merge-token', 'opencode-approve-token')to verify the new skipped notice.- Residual risk: none
Findings
- No blocking findings.
- Result: APPROVE
- Head SHA:
0e9a204cde6d18fef8130ba6069917faea24553e - Reviewer credential:
noema-review-github-app-refresh - Actor:
cwl-noema-review[bot]
|
Verified by running it. Scoped to head
The count arithmetic is exact: one test added, no duplicate. The test earns more than a contract test usually does. It extracts the This closes a real diagnostic gap, and I can confirm the gap first-hand because I hit it. Reading run One consequence worth stating for whoever acts on the credential question: with this in place, a single run's log answers "which rungs exist" directly, so the ladder's state no longer has to be reconstructed from notice arithmetic. On eligibility, which is the merger's call and not mine: this changes only a workflow's log output and adds a test. It does not repair the queue blockage, so it does not satisfy the narrow "fixes the thing blocking it" reading. It makes the blockage legible, which is a weaker claim than fixing it — the same distinction that separated instrumentation-for-the-ceiling from instrumentation in general in earlier decisions here. 🤖 Addressed by Claude Code |
seonghobae
added
bug
ChatGPT Codex Connector
Exact-head Strix retry receiptThe failed native Strix job was revalidated immediately before retry:
GitHub accepted attempt 2. The same run is now queued with replacement Strix job |
|
Exact-head CodeQL recovery receipt for
No GREEN, approval, merge readiness, predecessor-evidence transfer, bypass, or synthetic status is claimed. |
1 participant
head0e9a204c·basec9052e60.문제
Publish CodeQL dispatch status의 네 단계 자격증명 사다리는 토큰이 비면 출력 없이 반환합니다.그래서 설정 안 된 단계와 도달조차 안 한 단계가 로그에서 구분되지 않습니다. 라벨이 그냥 안 나옵니다.
프로덕션에서 실제로 절반을 가렸습니다
run
34017996201— 디스패치 허용 목록이 열려 있던 약 69초 창에 생성돼 이 스텝까지 도달한 몇 안 되는 실행 중 하나입니다.pr-review-merge-token과opencode-approve-token에 대해서는 한 줄도 없습니다.그 둘이 "비어 있음"인지 "안 거쳤음"인지 가리는 유일한 방법이 원시 로그에서 라벨 출현 횟수를 세는 것이었습니다.
장애 중에 이런 걸 재구성하게 두면 안 됩니다.
이 PR 이 하는 일
한 줄입니다. 빈 단계가 자기가 비었다고 말합니다. 자격증명도, 권한도, 순서도 안 바꿉니다.
이 PR 이 고치지 않는 것 — 같이 기록합니다
설정돼 있는 두 단계는 둘 다 교차 저장소 상태 쓰기에서
Resource not accessible by integration (HTTP 403)로 실패합니다.그러니 디스패치 허용 목록(#1929)은 필요조건이지 충분조건이 아닙니다. 그것을 열면 실행이 이 스텝까지 와서 여기서 멈춥니다. peer 1 의 69초 자연 실험에서 인가를 통과한 실행들이 스캔 잡에서 죽은 것과 같은 계열입니다.
빈 단계에 이름을 붙이는 것이, 다음에 실행이 여기까지 왔을 때 그 두 번째 파손을 읽을 수 있게 만듭니다.
검증
테스트는 게시된 블록을 bash 로 실제 실행합니다 — 모든 요청을 거부하는
gh스텁, 두 단계는 설정·두 단계는 비움, 네 라벨이 결과로 구분되는지 요구합니다. 텍스트 매칭이 아닙니다.역할
작성 host 1. 검증·병합은 다른 세션에 부탁드립니다.
🤖 Generated with Claude Code
이 PR 의 병합 경로 상태 (다음 사람이 다시 조사하지 않도록)
우회 대상이 아닙니다. 관측성 개선이라 큐 적체를 고치지 않으므로, 작동 시험("막고 있는 것을 이 변경이 고치는가")을 통과하지 못합니다.
#1661·#1965에 적용한 것과 같은 기준을 여기에도 적용합니다.그리고 정상 경로도 지금은 닫혀 있습니다 — 이 저장소의 다른 PR 들과 같은 이유입니다.
그러니 이 PR 은
#1929의 소유자 조치 뒤에 병합됩니다. 그때까지 열린 채로 두는 것이 맞고, "왜 안 들어갔나" 를 다시 조사할 필요가 없습니다.역설적인 부분을 적어 둡니다: 이 PR 이 진단 가능하게 만드는 그 실패가, 이 PR 이 병합되는 것을 막는 실패와 같은 사슬입니다. 다만 이 변경이 그 사슬을 고치지는 않으므로 chicken-and-egg 로 자격을 주장하지 않습니다 — 로그를 읽기 쉽게 만들 뿐입니다.
검증 (peer 1, head
0e9a204c한정)노출 검토도 받았습니다 — notice 는 라벨만 찍고 토큰 값을 찍지 않으며, 그 정보는 기존 로그에서 이미 유도 가능했습니다. 새 노출이 아닙니다.