Repository navigation
ci: scan pull requests from forks with CodeQL advanced setup - #3848
Conversation
Default setup excludes pull requests from forks, so findings on most pull requests here only surface after merge. This workflow runs the same languages, query suite and categories on pull_request, push to main and a weekly schedule. Default setup has to be switched off when it lands. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The workflow is narrowly scoped, correctly configured, and its expected transitional failure is documented and validated.
Review effort: Balanced
Findings: None
What changed in this PR
Adds advanced CodeQL scanning so fork pull requests are analyzed before merge.
Changes:
- Scans Actions and Python on pull requests, main pushes, and weekly.
- Applies least-privilege permissions, pinned actions, and concurrency controls.
- Preserves existing CodeQL alert categories.
| File | Description |
|---|---|
.github/workflows/codeql.yml |
Defines the advanced CodeQL workflow. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
I see errors: |
| # | ||
| # Default setup blocks uploads from this workflow while it is enabled. Switch | ||
| # it off (Settings > Code security > CodeQL analysis > Switch to advanced) when | ||
| # this file lands. |
There was a problem hiding this comment.
Remove this before merge, but do the recommended action after merging.
There was a problem hiding this comment.
Removed in ae9c9a9. The switch-to-advanced steps stay in the pull request description, and I will do them right after this merges.
| branches: | ||
| - main | ||
| schedule: | ||
| - cron: '17 4 * * 1' |
There was a problem hiding this comment.
Is this aligned with what happens currently? Seems like a random schedule
There was a problem hiding this comment.
Good catch, it was arbitrary. Default setup does not expose its cron, so I looked at its scheduled runs on main (runs that re-scanned a SHA that had already been scanned). They land on Thursdays at 08:10 UTC: 2025-12-18, 2026-02-19, 2026-03-26 and 2026-05-14 all at exactly 08:10, and 2026-06-18 at 08:19. ae9c9a9 switches the schedule to 10 8 * * 4, the same slot (next runs: Oct 8, 15, 22).
This is expected until settings are changed to advanced. When this happens the CodeQL scans will not run on upstream PRs so it only makes sense to do this after this is merged (or could do it immediately before, but probably not necessary to check this PR) |
…note Default setup's scheduled scan ran on Thursdays at 08:10 UTC; use the same slot. The note about switching default setup off belongs in the pull request, not in the workflow. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
|
setup changes - step runs now - |
Description
Problem: CodeQL never sees pull requests from forks
This repository uses CodeQL default setup (languages
actionsandpython, weekly schedule). GitHub documents that default setup scans pull requests "excluding pull requests from forks" (About setup types). Almost all of our pull requests come from forks, so findings only appear after merge, whenmainis scanned:dynamic/github-code-scanning/codeql) was for a fork pull request. All of them were formain, for branches in this repository, or for pull requests opened from such branches.actions/cache-poisoning/poisonable-step, high) and #17 (actions/missing-workflow-permissions, medium) were first raised onmainwithin a minute of merging fork pull requests ci: deny cache-service access to code the Bazel tests check out #3776 (alerts 15/16) and Feature/win arm64 support #3718 (alert 17). Neither pull request got a CodeQL result before it was merged.Change: CodeQL advanced setup
.github/workflows/codeql.ymlruns the same analysis as a regular workflow, so it also runs onpull_requestevents from forks:pull_requesttomain,pushtomain, and weekly on Thursdays at 08:10 UTC (10 8 * * 4), the slot default setup's scheduled scans used. No path filters: both languages build withbuild-mode: noneand finish in about 2 minutes (measured below), so filtering would save little and could miss workflow changes.actionsandpython, the default query suite, and the categories/language:actionsand/language:python. Because the categories match, the existing alerts keep their numbers and history when advanced setup takes over.contents: readat workflow level. Only the analyze job addssecurity-events: write.persist-credentials: falseon checkout. Actions are pinned by SHA, like the rest of.github/workflows. For fork pull requests GitHub downgrades the token to read-only. Code scanning still accepts their results: in oneTBB, which already uses advanced setup, the python job on a pull request fromArthur031221/oneTBBlogsSuccessfully uploaded results/Analysis upload status is complete.CodeQLcheck run that fails only on alerts the pull request introduces. Existing alerts onmaindo not fail it.Required step after merge: switch default setup to advanced (repository admin)
Default setup and advanced setup cannot both be active: while default setup is on, GitHub blocks CodeQL uploads from workflows. An admin has to switch it off right after this pull request is merged:
CodeQLworkflow onmain(or wait for the next push) and check that both/language:actionsand/language:pythonanalyses are uploaded under Security → Code scanning → Tool status.The same is possible from the CLI:
Observed on this pull request: default setup is still on, so both
Analyze (...)jobs run to the upload step and then fail withCode Scanning could not process the submitted SARIF file: CodeQL analyses from advanced configurations cannot be processed when the default setup is enabled(run 37218373184). That failure says nothing about the workflow itself, and it goes away once step 3 is done. It does confirm the cross-repository part: on a pull request from a fork, the workflow runs and reaches the upload step.Optional, to make it blocking:
maincurrently has no required status checks, so a failingCodeQLcheck shows on the pull request but does not prevent merging. To enforce it, add a branch ruleset formainwith Require code scanning results → toolCodeQL, security alertsHigh or higher, alertsErrors.Validation
codeql/actions-queriescode-scanning suite, onmainat43a87bf. It reports exactly the three open alerts:nightly-test.yml:227,nightly-test.yml:235,ci-win.yml:68. With this branch applied it reports the same three and nothing fromcodeql.yml.actionlint1.7.7: clean.zizmor1.30.1 (default persona): no findings.napetrov/oneDAL, which has default setup off, I pushed this workflow to a base branch, then opened a pull request that adds a deliberately vulnerable workflow (pull_request_target+ checkout ofgithub.event.pull_request.head.sha+run, withcontents: write):Analyze (actions)succeeded in 38 s and uploaded 3 results (the alerts above).Analyze (python)succeeded in 113 s and uploaded 0 results.CodeQLcheck run failed with "1 new alert including 1 critical severity security vulnerability". Its annotation was on the planted checkout line:actions/untrusted-checkout/critical, "Checkout of untrusted code in a privileged context". The three existing alerts did not count against the pull request.Not in this pull request
nightly-test.yml, flagged only for itsworkflow_dispatchtrigger) belongs in a separate change. The likely fix is to declarecache-mode: nonefor that workflow and then dismiss the two alerts, because the query does not modelcache-mode.Checklist:
Completeness and readability
Testing
Performance
🤖 Generated with Claude Code