Skip to content

ci(security): add dependency review scanning - #1246

Merged
kovtcharov-amd merged 3 commits into
mainfrom
feat/codeql-884
May 29, 2026
Merged

kovtcharov-amd merged 3 commits into
mainfrom
feat/codeql-884

Conversation

@kovtcharov-amd

@kovtcharov-amd kovtcharov-amd commented May 29, 2026 •

Copy link
Copy Markdown
Collaborator

The repo has GitHub's default CodeQL setup enabled for Python SAST, but nothing catches known-vulnerable or restrictively-licensed dependencies introduced by PRs. This adds actions/dependency-review-action to flag high-severity CVEs and GPL-3.0/AGPL-3.0 licenses before merge, closing the SCA gap identified in #884.

Test plan

  • Workflow runs on this PR (check the Checks tab for "Dependency Review")
  • PR comment summary appears from the action
  • YAML follows repo conventions (compare with lint.yml / test_security.yml)

Closes #884

@github-actions github-actions Bot added the devops DevOps/infrastructure changes label May 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Clean, minimal addition that closes a real gap. The workflow is structurally sound and ready to ship — one minor efficiency note below.

Summary

This adds a CodeQL SAST workflow that covers injection, path-traversal, and crypto-misuse classes that the existing test_security.yml (runtime checks) and linting don't catch statically. The design choices — no path filter so the scanner always runs, weekly scheduled sweep, draft-skip guard, cancel-in-progress — are all correct for a security workflow. Permissions are minimal and consistent with the rest of the repo.

Issues Found

🟢 Minor — Autobuild step is a no-op for Python (.github/workflows/codeql.yml:52–54)

github/codeql-action/autobuild is designed for compiled languages (C/C++, Java, Go, Swift). For Python, CodeQL's extractor doesn't need a build step — init and analyze are sufficient, and the GitHub-recommended starter workflow for Python omits it entirely. Including it isn't wrong, but it adds roughly 1–2 minutes of wall time per scan for no benefit. Simple fix:

      - name: Run CodeQL analysis
        uses: github/codeql-action/analyze@v3
        with:
          category: "/language:python"

(Remove the Autobuild step block entirely, keeping Initialize CodeQL → Run CodeQL analysis.)

Strengths

  • Consistent with repo conventions. actions/checkout@v6, the concurrency group pattern, the cancel-in-progress: true setting, and the draft-PR if condition all match lint.yml and test_security.yml exactly.
  • Correct minimal permissions. contents: read + security-events: write is exactly what CodeQL needs to check out code and upload SARIF results to the Security tab — nothing broader.
  • Weekly schedule at an off-peak time. 17 4 * * 0 (Sunday 04:17 UTC) avoids GitHub Actions resource contention while ensuring the codebase is scanned even when no PRs land that week.

Verdict

Approve. The autobuild note is a nit with no correctness impact — safe to merge as-is or apply the one-line removal. Either way, this is ready.

@kovtcharov-amd kovtcharov-amd changed the title ci(security): add CodeQL SAST scanning ci(security): add dependency review scanning May 29, 2026
@github-actions

github-actions Bot commented May 29, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ Deprecation Warning: The deny-licenses option is deprecated for possible removal in the next major release. For more information, see issue 997.

Dependency Review

The following issues were found:
  • ✅ 0 vulnerable package(s)
  • ✅ 0 package(s) with incompatible licenses
  • ✅ 0 package(s) with invalid SPDX license definitions
  • ⚠️ 2 package(s) with unknown licenses.
See the Details below.

License Issues

.github/workflows/dependency-review.yml

PackageVersionLicenseIssue Type
actions/checkout6.*.*NullUnknown License
actions/dependency-review-action4.*.*NullUnknown License
Denied Licenses: GPL-3.0, AGPL-3.0

OpenSSF Scorecard

PackageVersionScoreDetails
actions/actions/checkout 6.*.* 🟢 5.7
Details
CheckScoreReason
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Binary-Artifacts🟢 10no binaries found in the repo
Code-Review🟢 10all changesets reviewed
Maintained⚠️ 01 commit(s) and 0 issue activity found in the last 90 days -- score normalized to 0
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Packaging⚠️ -1packaging workflow not detected
Fuzzing⚠️ 0project is not fuzzed
License🟢 10license file detected
Pinned-Dependencies🟢 3dependency not pinned by hash detected -- score normalized to 3
Signed-Releases⚠️ -1no releases found
Security-Policy🟢 9security policy file detected
Branch-Protection🟢 5branch protection is not maximal on development and all release branches
SAST🟢 8SAST tool detected but not run on all commits
actions/actions/dependency-review-action 4.*.* 🟢 7.7
Details
CheckScoreReason
Code-Review🟢 10all changesets reviewed
Security-Policy🟢 9security policy file detected
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Maintained🟢 1030 commit(s) and 0 issue activity found in the last 90 days -- score normalized to 10
Packaging⚠️ -1packaging workflow not detected
Token-Permissions🟢 9detected GitHub workflow tokens with excessive permissions
Binary-Artifacts🟢 10no binaries found in the repo
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Pinned-Dependencies⚠️ 1dependency not pinned by hash detected -- score normalized to 1
License🟢 10license file detected
Fuzzing⚠️ 0project is not fuzzed
Signed-Releases⚠️ -1no releases found
Branch-Protection🟢 6branch protection is not maximal on development and all release branches
SAST🟢 9SAST tool detected but not run on all commits

Scanned Files

  • .github/workflows/dependency-review.yml

@github-actions

Copy link
Copy Markdown
Contributor

Clean, minimal CI addition that closes the SCA gap left after the recent CodeQL SAST work. The workflow follows all repo conventions exactly and is safe to merge.

Summary

This adds the standard actions/dependency-review-action to flag high-severity CVEs and restrictively-licensed packages before merge. The YAML is structurally consistent with the rest of the repo — same checkout version, same concurrency group pattern, same draft-skip if: guard, and a correct permissions block with only what's needed. Only one minor nit worth noting.

Issues Found

🟢 comment-summary-in-pr: always posts on every PR, including ones with no dependency changes (dependency-review.yml:47)

The action will add a comment to every single PR even when it found nothing and scanned no manifests (e.g., a docs-only PR). on-failure would limit comments to when issues are actually detected, which is quieter and easier to notice when something real does fail.

          comment-summary-in-pr: on-failure

Strengths

  • Pattern-perfect: concurrency group, cancel-in-progress, actions/checkout@v6, and the draft-skip if: condition all match the established repo conventions (lint.yml, test_security.yml) exactly — zero friction for future maintainers.
  • Permissions minimal: contents: read + pull-requests: write is exactly the minimum the action requires. No over-provisioned security-events: write or id-token: write that other tools sometimes request unnecessarily.
  • Dependabot already covers it: the repo's github-actions Dependabot group will keep dependency-review-action@v4 pinned to the latest patch automatically — no manual maintenance burden added.

Verdict

Approve — one minor nit about comment noise, but this is ready to merge as-is.

@kovtcharov-amd kovtcharov-amd self-assigned this May 29, 2026
@kovtcharov-amd kovtcharov-amd added the p1 medium priority label May 29, 2026
itomek
itomek previously approved these changes May 29, 2026

@itomek itomek left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Workflow is convention-clean and the Dependency Review check already passed on this PR. One forward-looking note inline (deny-licenses deprecation that the bot didn't surface). The merge_group trigger will typically no-op for dependency-review since it needs a base...head PR comparison — harmless, just confirm it's intended rather than expected to gate the queue. Approving.


Generated by Claude Code

Comment thread .github/workflows/dependency-review.yml Resolved
Ovtcharov added 2 commits May 29, 2026 09:33
The deep code review (#884) found that while path-traversal and
shell-injection tests exist, there was no automated SAST/SCA scanning.
This adds a CodeQL workflow that runs on push, PR, merge queue, and a
weekly schedule to catch injection, crypto-misuse, and similar classes
of vulnerability that unit tests miss.
The repo already has GitHub's default CodeQL setup enabled for Python
SAST, so the custom workflow conflicted.  Replace it with
actions/dependency-review-action which fills the actual remaining gap:
flagging known-vulnerable or restrictively-licensed dependencies
introduced by PRs.
`always` spams every PR (including docs-only) with a "0 issues" comment;
`on-failure` keeps the signal-to-noise ratio high.
@kovtcharov-amd
kovtcharov-amd added this pull request to the merge queue May 29, 2026
Merged via the queue into main with commit b083db4 May 29, 2026
22 checks passed
@kovtcharov-amd
kovtcharov-amd deleted the feat/codeql-884 branch May 29, 2026 18:12
@kovtcharov-amd kovtcharov-amd mentioned this pull request Jun 1, 2026
5 of 6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

devops DevOps/infrastructure changes p1 medium priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security + cross-platform install smoke + OpenAPI contract testing

3 participants