ci: switch CodeQL to advanced setup with JS/TS, same-repo and fork PR scanning#99
Merged
Merged
Conversation
- Replaces default setup to also analyze javascript-typescript sources (currently only `actions` is scanned). - Uses pull_request_target with explicit PR head checkout and persist-credentials: false so fork PRs are scanned safely without exposing the base-repo token to fork code. - build-mode: none for both languages, so no fork code is executed. - Adds .github/codeql/codeql-config.yml to skip generated bundles (dist/), node_modules, lockfiles, and test fixtures to keep alerts signal-heavy. Default CodeQL setup must be disabled before this can upload SARIF; see PR description for the API call.
- Add `pull_request` trigger so PRs from branches within SAP/pull-request-semver-bumper are analyzed using their own workflow version (lets us iterate on the workflow inside a PR). - Keep `pull_request_target` trigger for fork PRs so SARIF can be uploaded under the base repo for the fork's head SHA. - Add a job-level `if` so each PR is analyzed exactly once: same-repo via pull_request, forks via pull_request_target. - Push and schedule triggers are unchanged.
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
Previously the workflow used `config-file: ./.github/codeql/codeql-config.yml`
which, on `pull_request_target` for fork PRs, resolved to the fork's
version of that file because actions/checkout had already overlaid the
fork's source onto the working directory.
A malicious fork could submit a PR carrying a config like:
disable-default-queries: true
paths-ignore:
- "**/*"
This would make CodeQL upload an empty SARIF, satisfy the `code_scanning`
ruleset rule, and let the fork merge unscanned code.
Switch to the `config:` (inline YAML) input on codeql-action/init, which
takes precedence over `config-file` and is read from the workflow file
itself \u2014 always the trusted version on main when triggered by
pull_request_target.
The CodeQL configuration is now provided inline in the workflow's `config:` input (see prior commit). Keeping the file would create two sources of truth and \u2014 worse \u2014 leave a fork-controlled file on disk that future contributors might wire up via `config-file:` again, reintroducing the gate-bypass vulnerability.
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
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.
Summary
Switch CodeQL from default setup to advanced setup so that:
actionsis scanned).code_scanningrule in ruleset10774671and unblocking PRs like fix: use commit-message input #98 that are currentlymergeable_state: BLOCKEDsolely because no analysis exists for the head SHA.Motivation
/code-scanning/default-setupis configured for["actions"]only. The repo ruleset onmainrequires:code_scanning(CodeQL, severity ≥ high, quality alerts ≥ errors)code_quality(severity errors)copilot_code_reviewFor a PR from a fork (e.g., #98 from
Herrtian/pull-request-semver-bumper), default setup never analyzes the PR head, so the rule cannot be satisfied and the PR stays blocked even with green CI and an approving review.Changes
Added
.github/workflows/codeql.yml— CodeQL Advanced workflow.pushto main,pull_requeston main,pull_request_targeton main, weekly schedule (26 1 * * 3).ifensures each PR is analyzed exactly once:pull_requestonly.pull_request_targetonly.actions(build-mode: none),javascript-typescript(build-mode: none).security-and-qualityso quality alerts feed thecode_qualityrule.actions/checkoutuses the PR head ref/repo for any pull_request[_target] event.config:input (see Security section).Why the dual
pull_request+pull_request_targettriggerif:SAP/pull-request-semver-bumperpull_requestHerrtian/...)pull_request_targetmain's version (fork can't tamper)Without splitting, a single
pull_request_target-only setup would force every workflow change to be merged before being testable; a singlepull_request-only setup would silently skip fork PRs (no SARIF write permission for fork tokens). The split gives both audiences the right behavior with no duplicate runs.Security model for fork-PR scanning
pull_request_targetruns in the base-repo context with a token that hassecurity-events: write. That power needs careful containment. The defenses in this workflow:pull_request_targetreads the workflow file frommain. The fork's.github/workflows/codeql.ymlis ignored.init'sconfig:input rather thanconfig-file:. Aconfig-file:path resolves against the working directory, which contains the fork's checkout, so a malicious fork could shipdisable-default-queries: true+ broadpaths-ignoreand neuter the gate. Inline config is read from the trusted workflow file frommain.actions/checkoutusespersist-credentials: false, so the base-repoGITHUB_TOKENis not written to.git/config. Fork code can'tgit pushback.build-mode: nonefor both languages. CodeQL extracts directly from source. Nonpm install, noactions/setup-node, norun:step that touches fork content.contents: read,security-events: write,pull-requests: read. Nocontents: write, noid-token: write.This follows GitHub's documented pattern for fork-PR CodeQL coverage.
Manual steps required after merge
CodeQL default setup must be disabled before the advanced workflow can upload SARIF, otherwise
analyzefails with "Code scanning is configured with default setup".After this PR merges, an admin must run once:
Or via UI: Settings → Code security → CodeQL analysis → switch off default setup.
The next push to
main(or any PR) will then run the advanced workflow.Compatibility & impact
Breaking changes
CI impact
build-mode: noneand small in scope, so wall-clock impact is minor (~1-2 min).Dependencies
actions/checkout@v4,github/codeql-action/init@v4,github/codeql-action/analyze@v4(current major versions).Verification
After merge + disabling default setup, verify both languages produce analyses:
Expected: entries for
/language:actionsand/language:javascript-typescript.For PRs (same-repo or fork), after the workflow runs:
Should return the head SHA, and the PR's
mergeable_stateshould leaveBLOCKED.Checklist
pull_request_targetpersist-credentials: falseto avoid leaking base-repo token to checked-out codepaths-ignore/disable-default-queriesto bypass the gategithub/codeql-action