ci: run CodeQL only on main - #832
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts the repository’s CodeQL GitHub Actions workflow so security analysis runs only after changes are merged to the default branch, reducing CI usage on pull requests.
Changes:
- Removed
pull_requestandworkflow_dispatchtriggers from the CodeQL workflow. - Removed the PR-specific
concurrencyconfiguration.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
HAYDEN-OAI
left a comment
HAYDEN-OAI
left a comment
There was a problem hiding this comment.
One substantive security issue: making the security CodeQL workflow main-only allows release creation and Maven publication to begin before the released commit has completed its first security scan. Details and a concrete observed release/scan timeline are in the inline comment.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
.github/workflows/codeql.yml:7
- Removing
concurrencymeans multiple pushes tomaincan run CodeQL in parallel, consuming extra CI minutes and generating redundantworkflow_runcompletions (even if downstream release creation later skips stale SHAs). Since this workflow now only runs onpushtomain, consider adding a simple concurrency group (e.g., by ref) withcancel-in-progress: trueso only the latestmainanalysis runs.
on:
push:
branches:
- main
.github/workflows/create-releases.yml:140
- With
--limit 1, this step will fail immediately if the CodeQL run forSOURCE_SHAis still queued/in_progress or not yet indexed (common right after a push). That can make the scheduled/manual paths flaky even though CodeQL will eventually succeed. Consider pollinggh run listuntil the run iscompleted/success(with a reasonable timeout) or skipping the schedule path and relying on theworkflow_runtrigger as the authoritative gate.
run_json="$(
gh run list \
--repo "$GITHUB_REPOSITORY" \
--workflow codeql.yml \
--commit "$SOURCE_SHA" \
c09243a to
45d93a0
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.github/workflows/create-releases.yml:45
workflow_runtriggers for completed runs of the CodeQL workflow regardless of how that CodeQL run was started (push, workflow_dispatch, re-run, etc.). The current job-levelifonly checksconclusion == 'success', so a manually-dispatched CodeQL run onmaincan unintentionally trigger the automatic release selection path. If releases should only be triggered by the push-to-main CodeQL analysis, also gate ongithub.event.workflow_run.event == 'push'.
if: >-
github.repository == 'openai/openai-java' &&
((github.event_name == 'workflow_run' &&
github.event.workflow_run.conclusion == 'success') ||
(github.event_name != 'workflow_run' && github.ref == 'refs/heads/main'))
|
Follow-up from the latest automated review: a successful manually dispatched CodeQL run on main could also satisfy the workflow_run trigger and start automatic release selection. Fixed in a6cf6f8 by requiring workflow_run.event == push. On-demand scans remain available, but only push-originated scans can initiate automatic releases. Actionlint, YAML parsing, diff checks, and thermo-nuclear review pass. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (3)
.github/workflows/create-releases.yml:144
- This CodeQL verification only inspects the most recent run for the commit (
--limit 1). If there are multiple push-triggered CodeQL runs for the same commit (e.g., reruns), a newer non-successful run can cause releases to be blocked even when an earlier successful analysis exists. It’s safer to look for any completed+successful run for the SHA.
--workflow codeql.yml \
--commit "$SOURCE_SHA" \
--event push \
--limit 1 \
--json conclusion,headSha,status,url
.github/workflows/codeql.yml:11
concurrency.groupis now identical forpushandworkflow_dispatchruns on the same branch (github.ref). A manually dispatched CodeQL scan onmaincan therefore cancel an in-progresspushscan formain, and theCreate releasesworkflow intentionally ignoresworkflow_dispatchCodeQL completions (it only reacts toworkflow_run.event == 'push'). This can unintentionally block automatic release selection until the next successful push scan completes.
concurrency:
group: codeql-${{ github.ref }}
cancel-in-progress: true
.github/workflows/create-releases.yml:133
Ensure automatic source is still currentruns after the CodeQL lookup. On scheduled runs, ifmainadvances after the workflow starts, the CodeQL lookup may fail (or find only canceled/incomplete runs) for the now-stalegithub.shaand abort the job before it can gracefully skip as stale. Consider moving the staleness check before the CodeQL verification, or making the CodeQL verification conditional on the source still matching currentmain.
This issue also appears on line 140 of the same file.
- name: Verify successful CodeQL analysis
if: github.event_name != 'workflow_run'
env:
GH_TOKEN: ${{ github.token }}
SOURCE_SHA: ${{ steps.retry.outputs.source_sha || github.sha }}
|
Addressed the three latest orchestration findings in 5479eb1: push and manual CodeQL runs now use separate concurrency groups; automatic source staleness is checked before scheduled CodeQL verification; and scheduled/retry verification accepts any completed successful push analysis for the exact source SHA rather than only the newest run. Actionlint, YAML parsing, diff checks, jq success-selection coverage, and thermo-nuclear review all pass. |
HAYDEN-OAI
left a comment
HAYDEN-OAI
left a comment
There was a problem hiding this comment.
Fresh follow-up at exact head 5479eb1. The push-event provenance guard and manual/push scan separation are fixed, but one P1 security gap and one P2 release-concurrency failure remain; details are inline.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.github/workflows/codeql.yml:11
- The CodeQL
concurrency.groupincludesgithub.event_name, so a manualworkflow_dispatchrun on the same ref will not cancel (or be canceled by) an in-flightpushrun for that ref. This contradicts the PR goal of canceling superseded CodeQL scans for the same ref and can lead to duplicate long-running analyses onmain.
concurrency:
group: codeql-${{ github.event_name }}-${{ github.ref }}
cancel-in-progress: true
5479eb1 to
9f69c3c
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f69c3cb28
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
HAYDEN-OAI
left a comment
HAYDEN-OAI
left a comment
There was a problem hiding this comment.
Re-reviewed exact head 9f69c3c and every prior review thread. The final diff only removes the repository CodeQL pull-request trigger and keeps ref-scoped cancellation; main-push and manual scans remain available. create-releases.yml is byte-for-byte identical to main, the managed PR code-quality scan remains active, and the previous exact-SHA and release-concurrency findings no longer apply after the explicitly confirmed decision that CodeQL is informational and must not gate releases. No substantive correctness, security, release-orchestration, or concurrency issues found.
Summary
Impact
The generated PR-time code-quality scan remains in place, while the repository's roughly 15–18 minute security-query scan moves off the pull-request critical path. Releases continue to trigger directly from pushes to main and do not wait for CodeQL results.
Validation