Skip to content

ci: add security scanning (CodeQL, dependency review, secret scan)#110

Merged
rubenhensen merged 3 commits into
mainfrom
ci/security-scanning
Jul 1, 2026
Merged

ci: add security scanning (CodeQL, dependency review, secret scan)#110
rubenhensen merged 3 commits into
mainfrom
ci/security-scanning

Conversation

@rubenhensen

@rubenhensen rubenhensen commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Closes #103.

Adds automated security scanning to CI. Kept as standalone workflows so the existing ci.yml (build / test / release) stays focused. These workflows run on this PR, so their first results are visible in the Checks tab.

What's added

  • .github/workflows/codeql.yml — CodeQL SAST for JavaScript/TypeScript on PRs, pushes to main, and a weekly schedule (security-extended query pack). Findings surface in the Security tab.
  • .github/workflows/security.yml
    • Dependency review (PRs) — fails a PR that introduces a dependency with a high+ advisory. Delta-based, so it gates new risk without tripping on pre-existing debt. Active gate (the Dependency graph has been enabled on the repo).
    • Secret scan — TruffleHog over the diff. (Chosen over gitleaks-action, which requires a paid licence for organisation accounts.)
    • npm audit (advisory) — full-tree npm audit --audit-level=high, intentionally non-blocking (continue-on-error).

Why npm audit is non-blocking

npm audit currently reports 1 high advisory — a Windows-only dev-server issue in vite/esbuild, which are dev dependencies and not in the runtime image. Gating on the full tree would fail CI immediately on pre-existing, low-real-risk debt. The advisory job keeps it visible; cleaning it up (npm audit fix) is better handled as its own dependency bump. Flip continue-on-error off once the tree is clean.

Follow-ups (repo settings — can't be done in a PR)

  • Enable GitHub secret scanning + push protection (free for public repos).
  • Add the new checks to branch-protection required status checks once they've had a green run.
  • Clean up the pre-existing dev-only vite/esbuild advisory (npm audit fix) and flip the npm audit job to blocking.

@dobby-coder

dobby-coder Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

On it, @rubenhensen! I'm reviewing the security scanning workflows in this PR for correctness and rule-compliance, and will follow up with a consolidated review.

@github-advanced-security

Copy link
Copy Markdown

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:

  • The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
  • Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
  • You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

@dobby-coder dobby-coder Bot 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.

Dobby review — CI security-scanning PR.

Rule compliance: ran the full memory rule set (74 org rules + 3 postguard-business repo notes) against the diff — all clean, no violations. The PR title is conventional-commit compliant (ci:), it's ready-for-review from a human maintainer, and Closes #103 is present.

CI / tests: all PR checks are green on a25b53e (the earlier dependency-review red check is now fixed via continue-on-error). The npm audit --audit-level=high claim reproduces locally: exit 1 from the vite/esbuild dev-server advisory — dev-only and Windows-only, so correctly gated non-blocking.

No blocking problems. Two non-blocking notes inline (1 style, 1 nit) — neither should hold up merge.

fetch-depth: 0

- name: TruffleHog
uses: trufflesecurity/trufflehog@main

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.

style: trufflesecurity/trufflehog is pinned to the mutable @main ref. In a security-hardening PR this is itself a supply-chain risk — unreviewed upstream changes would execute in CI with checkout access to the repo. Pin to a release tag or commit SHA, consistent with every other action in these two workflows (checkout@v6, dependency-review-action@v4, setup-node@v6, codeql-action/init+analyze@v3).

Comment thread .github/workflows/security.yml Outdated

- name: Dependency review
uses: actions/dependency-review-action@v4
continue-on-error: true

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.

nit: making dependency-review continue-on-error with the in-code TODO correctly resolves the prior red-check bug (this Security workflow failed on 2266395, passes on a25b53e) — nicely documented in the surrounding comment. Minor follow-up gap: the PR description's Follow-ups list still omits enabling the repo Dependency graph (Settings → Code security), which is the prerequisite to flip continue-on-error off later. It's captured in the workflow comment (good) but not alongside secret-scanning + branch-protection in the description's tracking list.

@dobby-coder

dobby-coder Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Reviewed! All 74 org rules + repo-specific notes came back clean — no violations, CI is green, conventional commit title checks out, and Closes #103 is present. Left two non-blocking notes on .github/workflows/security.yml:

  • Style: trufflehog is pinned to @main (mutable), which is a small supply-chain risk for a security-focused PR — worth pinning to a tag or SHA like the other actions do.
  • Nit: The dependency-review continue-on-error fix is good, but the PR description's follow-ups list doesn't mention enabling the repo's Dependency graph (which that action needs to be fully effective).

Otherwise this looks ready for a human maintainer to merge. 🟢

@rubenhensen rubenhensen merged commit 0f83193 into main Jul 1, 2026
15 checks passed
@rubenhensen rubenhensen deleted the ci/security-scanning branch July 1, 2026 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI: add security scanning (CodeQL + dependency audit + secret scan)

2 participants