Skip to content

ci(codeql): add minimal-scope CodeQL workflow (security-extended, weekly + push-to-main) - #74396

Open
bbasketballer75 wants to merge 2 commits into
NousResearch:mainfrom
bbasketballer75:ci/codeql-minimal
Open

ci(codeql): add minimal-scope CodeQL workflow (security-extended, weekly + push-to-main)#74396
bbasketballer75 wants to merge 2 commits into
NousResearch:mainfrom
bbasketballer75:ci/codeql-minimal

Conversation

@bbasketballer75

Copy link
Copy Markdown

Summary

Adds a minimal-scope GitHub CodeQL workflow for CVE-class findings on push-to-main and on a weekly Monday 06:00 UTC schedule.

Scope (deliberately minimal)

  • queries: security-extended only (NOT security-and-quality — too FP-heavy on a single-maintainer Python codebase)
  • languages: python + javascript-typescript (the two source ecosystems on this side; workflow files are pinned to commit SHAs and already covered by supply-chain-audit.yml, so excluded here)
  • triggers: push-to-main + weekly cron + workflow_dispatch
  • action pins: full 40-char SHAs per repo convention (actions/checkout@de0fac2e4…, github/codeql-action@3b0bd1d11…)

Why not also enable the other GitHub security toggles?

  • dependabot.yml already exists and is scoped to github-actions only with an explicit pinning-vs-auto-bump policy in its header comment. Enabling general Dependabot version updates would conflict with that.
  • osv-scanner.yml and supply-chain-audit.yml already exist in .github/workflows/ — adding CodeQL on top gives incremental value (source-level taint analysis) without duplicating them.

Cost estimate

  • Per-push to main: ~3–6 min CI
  • Weekly schedule: ~3–6 min CI
  • Total new overhead: ~6–12 min/week on top of existing CI matrix

Test plan

  • Confirm the workflow file is valid YAML and passes actionlint in CI
  • Trigger workflow_dispatch from this PR branch and confirm both matrix legs (python, javascript-typescript) complete successfully against the current main
  • Wait for first weekly schedule (Monday 06:00 UTC) and confirm no false-positive spike beyond security-extended's expected baseline
  • After 1 week, review Security tab → Code scanning alerts and triage

Copilot AI review requested due to automatic review settings July 29, 2026 21:23

Copilot AI 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.

Pull request overview

Adds a new GitHub Actions workflow to run CodeQL code scanning on main pushes and on a weekly schedule, focusing on higher-signal security queries to detect CVE-class issues in the primary code ecosystems (Python and JS/TS).

Changes:

  • Introduces a minimal CodeQL workflow running security-extended queries only.
  • Scans python and javascript-typescript via a matrix job, with actions pinned to full SHAs.
  • Triggers on push-to-main, weekly cron (Mon 06:00 UTC), and workflow_dispatch.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/codeql.yml Outdated
@alt-glitch alt-glitch added type/security Security vulnerability or hardening P3 Low — cosmetic, nice to have needs-decision Awaiting maintainer decision before any implementation sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation labels Jul 29, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #10344 already adds .github/workflows/codeql.yml. These patches differ on PR triggering, query suite, concurrency, and action-pin policy, so this is a maintainer CI-policy decision rather than a duplicate.

@teknium1 teknium1 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.

Thanks for the narrowly scoped security workflow proposal. The current main checkout has no CodeQL workflow, so the premise remains valid, but this version cannot run as submitted.

Problems

  • .github/workflows/codeql.yml:53 and :65 use github/codeql-action@3b0bd1d116c0bde30213346b22d4f634d96a2fb0. That SHA does not resolve in the action repository; both analysis steps will fail before scanning.
  • .github/workflows/codeql.yml:59 adds security-extended to CodeQL's default query set; it does not make that suite exclusive. The stated minimal query policy therefore is not implemented.

Suggested changes

  • Replace both CodeQL action pins with a verified full SHA for the intended release, then retain the repository's SHA-pin convention.
  • If security-extended-only is intended, disable default queries and declare that suite through CodeQL configuration; otherwise update the scope description.
  • At .github/workflows/codeql.yml:24, remove unsupported [codeql skip]; GitHub documents [skip ci] but not that token.

This is an automated hermes-sweeper review.

Comment thread .github/workflows/codeql.yml
Comment thread .github/workflows/codeql.yml
Comment thread .github/workflows/codeql.yml Outdated
@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 30, 2026
… skip token

The github/codeql-action SHA (3b0bd1d1...) pinned by the original commit
does not resolve in the action repository -- verified via the GitHub API
(422 'No commit found for SHA'), so both matrix legs would fail before
scanning anything. Replaced with the real, API-verified current v3 SHA
for both the init and analyze steps.

Also fixes two review findings: the header comment claimed
security-extended was the exclusive query set ('CVE-class findings
only'), but queries: security-extended augments CodeQL's default suite,
it doesn't replace it -- reworded to state that accurately. Dropped the
non-functional '[codeql skip]' commit-message token from the docs;
GitHub only recognizes '[skip ci]' for this.
@bbasketballer75

Copy link
Copy Markdown
Author

Addressed the hermes-sweeper review:

  1. Broken SHA3b0bd1d116c0bde30213346b22d4f634d96a2fb0 doesn't resolve (confirmed via API, 422). Replaced with the current, API-verified v3 SHA (a2983b8bed1923f44751c5c43237f479442827b3) on both the init and analyze steps.
  2. Query-suite claim — reworded the header comment: queries: security-extended augments CodeQL's default suite, it doesn't replace it. The stated policy boundary is really "no security-and-quality" (FP-heavy), not "only security-extended runs" — comment now says that accurately instead of implying exclusivity.
  3. [codeql skip] — removed; GitHub only recognizes [skip ci].

Re: the #10344 collision on the same new file path — leaving that for a maintainer to reconcile since it's a different PR, not something I can resolve from this side.

@egilewski

Copy link
Copy Markdown
Contributor

suggesting changes

The workflow is syntactically valid and uses SHA-pinned actions, least-privilege permissions, and non-persisted checkout credentials. However, it only triggers on pushes to main, a weekly schedule, or manual dispatch. It therefore cannot provide the claimed PR-critical CodeQL gate: a vulnerable pull request can merge without a CodeQL result, and the first automatic scan occurs after merge. Add a fork-safe pull_request path or invoke the same analysis from the existing PR CI before treating this as mergeable.

  • [P3] CodeQL does not run for pull requests (.github/workflows/codeql.yml:30)
    The workflow trigger block declares push for main, schedule, and workflow_dispatch, but no pull_request event. Consequently a pull request has no CodeQL check to gate or inform its merge; the first automatic analysis is only after the change has landed on main. This contradicts the file's PR-critical description and leaves the new security control unable to prevent or warn on a CodeQL finding before merge.
    Remediation: Add a pull_request trigger (with fork-safe permissions and no secret-dependent build steps), or call a reusable CodeQL job from the existing pull-request CI and include its result in the required gate. If post-merge-only scanning is intentional, remove the PR-critical claim and document that it is advisory after merge.

Security evidence:

  • trust boundary: The workflow checks out the selected repository revision and invokes two full-commit-SHA-pinned actions. Its job permissions are limited to actions: read, contents: read, and security-events: write; checkout sets persist-credentials to false. There are no shell run steps, dynamic action references, pull-request payload interpolations, or secret inputs beyond the standard GitHub token passed to CodeQL initialization.
  • source/sink/invariant: The event declarations at lines 30-35 are the only automatic source of execution, and the fixed matrix sends Python and javascript-typescript to CodeQL init and analyze for SARIF upload. Pinning and permission invariants hold, but the claimed invariant that each pull request receives a pre-merge CodeQL result does not hold because pull_request is absent.
  • current-main reproduction: The current-main tree has no codeql workflow. In the replayed file, a pull_request event has no matching trigger; only push to main, schedule, and workflow_dispatch are declared, so PR-time coverage is absent until merge.
  • PR-head or patch-replay validation: The run-owned replay file matched the bound PR-head codeql blob, and the two-commit PR history was inspected on top of the current-main checkout. The final YAML parsed successfully with a YAML 1.2 parser and its trigger, matrix, permission, credential, and action-pin invariants passed.
  • positive/negative cases: Positive cases: both matrix languages are present, every uses reference is a 40-hex commit SHA, the permission map is least-privilege for SARIF upload, and checkout credentials are not persisted. Negative case: the parsed trigger map contains no pull_request key, so a PR event cannot enqueue this workflow; no dynamic ref or shell execution path was found.
  • residual bypass search: The workflow directory, CI orchestrator, and CodeQL references were searched for an alternate pull-request trigger or reusable invocation; none was found. The remaining gap is the absent PR trigger rather than an action-pin or credential-persistence bypass.
  • reviewer validation: Static source review, YAML 1.2 parsing, structural invariant checks, and the targeted repository pytest completed successfully. GitHub-hosted execution, SARIF upload, remote action-commit reachability, and branch-protection enforcement were not exercised.

Review setup: I reviewed a run-owned local rebase or patch replay against current GitHub main because the submitted branch is stale or conflicted; this does not mean the submitted branch itself merges cleanly.

Not checked:

  • GitHub-hosted CodeQL execution
  • Remote action SHA reachability
  • Branch-protection required-check configuration

Signed: GPT-5.6-luna-max in Codex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants