Skip to content

docs(adr): design dispatch+poll architecture to restore central CodeQL - #1772

Merged
seonghobae merged 2 commits into
mainfrom
claude/codeql-required-workflow-dispatch
Sep 3, 2026
Merged

docs(adr): design dispatch+poll architecture to restore central CodeQL#1772
seonghobae merged 2 commits into
mainfrom
claude/codeql-required-workflow-dispatch

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • codeql-pr.yml's CodeQL analysis jobs cannot run inside a required workflow — GitHub categorically disallows github/codeql-action/init/analyze there (root-caused and fixed by removal from ruleset 18156473 in docs(doctoring): record codeql-pr.yml required-workflow platform restriction #1767 / docs/doctoring/codeql-pr-required-workflow-always-fails.md).
  • This ADR designs the follow-up to restore a real, centrally-owned, hard-gating CodeQL required check: mirror the dispatch+poll pattern already proven by strix.yml and opencode-review.yml — a required-workflow-safe entrypoint in the target repo dispatches the actual codeql-action work via repository_dispatch to a new native .github/workflows/codeql-scan-dispatch.yml, which reports back via commit status.
  • Covers: why native "code scanning default setup" isn't a substitute, the exact job/file split, the scope decision to drop analyze-merge (required nowhere per fix(ci): job-level runner-admission gate for required workflows #1766), security considerations (payload forgery/TOCTOU, cross-repo checkout trust boundary, status-publish credential scope, spoofable poll targets), alternatives rejected, and a 5-step implementation follow-up.
  • No behavior change in this PR — design document only. Implementation (extracting the SARIF gate script, the new dispatch handler, and the codeql-pr.yml rewrite) is tracked as separate follow-up work per the ADR's own "Follow-up" section.

Test plan

  • Docs-only change; no code paths affected.
  • Follow-up PRs will need: scripts/ci/codeql_sarif_gate.py + tests, new codeql-scan-dispatch.yml, codeql-pr.yml rewrite, a contract test asserting zero codeql-action references remain in codeql-pr.yml, then re-adding it to ruleset 18156473 and verifying a real PR passes.

🤖 Generated with Claude Code


Devin Review

codeql-pr.yml cannot run codeql-action inside a required workflow (GitHub
platform restriction, root-caused in docs/doctoring/codeql-pr-required-workflow-always-fails.md
and fixed there by removing it from ruleset 18156473). This ADR designs the
follow-up: mirror the strix.yml/opencode-review.yml dispatch+poll pattern so
codeql-pr.yml stays required-workflow-safe while the actual codeql-action
work runs natively in .github via repository_dispatch.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 39 minutes.

Check out review usage here.

View limit details

Limit 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.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 0a2992d8-7f34-4bda-b740-aca3c0ce046b

📥 Commits

Reviewing files that changed from the base of the PR and between ff332ea and ac2fdc3.

📒 Files selected for processing (1)
  • docs/adr/0025-codeql-required-workflow-dispatch-architecture.md

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

@seonghobae
seonghobae merged commit 211f167 into main Sep 3, 2026
7 of 18 checks passed
@seonghobae
seonghobae deleted the claude/codeql-required-workflow-dispatch branch September 3, 2026 07:44
seonghobae added a commit that referenced this pull request Sep 3, 2026
No conflicts; picked up main's CodeQL dispatch+poll work (#1772 ADR,
#1774 SARIF gate extraction). Full suite (2717 tests) passes.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
seonghobae added a commit that referenced this pull request Sep 3, 2026
…sitive_log (#1808)

Skip json.loads() for log lines that obviously can't be JSON (checked
via a first-character set), avoiding expensive JSONDecodeError exception
handling on large non-JSON CI log files. Scalar JSON values (numbers,
true/false/null) still parse successfully but fall through to the
unstructured redactor rather than round-tripping through json.dumps,
matching existing behavior for those lines.

Also fixes a pre-existing bug this change newly exercises:
_consume_sensitive_assignment's unquoted-value scanner didn't stop at
a bare quote character, so an assignment embedded inside an outer
JSON-string literal (e.g. "token=secret123456789") silently swallowed
the closing quote into the redacted value, corrupting line structure.

Clean rebuild of #1751 (branch bolt/optimize-json-decode-8663451122774961708):
that branch's own diff against current main is 91 files / 7805 deletions,
including deleting codeql-scan-dispatch.yml and gutting codeql-pr.yml back
toward a pre-#1772/#1774/#1776/#1778 state -- its merge-base with main
(0c9a5ab) is 12 commits stale and its own in-branch "Merge branch 'main'"
commit did not actually pick up main's content correctly. This branch
carries forward only #1751's real, intended change (verified against its
isolated file diff) rebuilt on current main, plus the one pre-existing
bug that change exposed.
seonghobae added a commit that referenced this pull request Sep 3, 2026
dependabot#1517 and #1518 proposed this bump against codeql-pr.yml's
inline init/analyze steps, but those steps moved to
codeql-scan-dispatch.yml during the CodeQL dispatch+poll rebuild
(#1772/#1774/#1776/#1778) after the PRs were opened -- both are now
DIRTY (git can't find the lines they target). Applying the same
verified v4.37.9 SHA (cdf488f595d80d6e07e03d4674febd5ab45fa938,
confirmed against github/codeql-action's own tag ref) at all 12 live
call sites instead: codeql-scan-dispatch.yml, scheduled-security-scan.yml,
scorecard-analysis.yml, python-security.yml, sast-semgrep.yml,
secret-scan.yml, security-scan.yml (x3).

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant