Skip to content

remove package-lock.json and add Snyk SCA GHA to CI#5669

Merged
jonnalley merged 6 commits into
mainfrom
fix/renovate-bun-lockfile-support
May 28, 2026
Merged

remove package-lock.json and add Snyk SCA GHA to CI#5669
jonnalley merged 6 commits into
mainfrom
fix/renovate-bun-lockfile-support

Conversation

@jonnalley
Copy link
Copy Markdown
Contributor

@jonnalley jonnalley commented May 14, 2026

Summary

  • Removes frontend/package-lock.json from git tracking and adds it to .gitignore
  • Adds a CI-driven Snyk SCA workflow to restore security scanning without a committed lockfile
  • This unblocks all Renovate frontend dependency PRs that are currently failing CI

Problem

Every Renovate PR that updates frontend/package.json fails with:

error: lockfile had changes, but lockfile is frozen
note: try re-running without --frozen-lockfile and commit the updated lockfile

The renovate/artifacts status shows FAILURE on all of them (PRs #5624, #5643, #5645, #5649, and earlier #5625).

Root cause: When both package-lock.json (npm) and bun.lock (bun) exist in the same directory, Renovate uses npm to update lockfiles. It updates package-lock.json but not bun.lock. Since CI runs bun install --frozen-lockfile, the stale bun.lock causes every frontend job to fail.

Snyk SCA Replacement

Since package-lock.json was added in PR #5604 to support Snyk SCA scanning (which doesn't natively support bun.lock), removing it breaks Snyk's SCM integration. This PR replaces that with a GitHub Actions workflow (.github/workflows/security_snyk.yml) that:

  1. Frontend: Generates package-lock.json ephemerally via npm install --package-lock-only, then runs snyk test and snyk monitor
  2. Backend API: Scans backend/ops_api/Pipfile with snyk/actions/python
  3. Data Tools: Scans backend/data_tools/Pipfile with snyk/actions/python

Results upload as SARIF to GitHub's Security tab (Code Scanning), alongside existing CodeQL and Semgrep findings.

Observability safeguards:

  • Validates SNYK_TOKEN exists before scanning (fail-fast on missing secret)
  • Distinguishes "vulnerabilities found" (exit code 1 with SARIF) from "scan infrastructure failure" (exit code 1 without SARIF)
  • Emits ::warning:: annotations if snyk monitor fails to register the project

Prerequisite: A SNYK_TOKEN repository secret must be added in Settings > Secrets and variables > Actions before the workflow will function. Also will need to modify the Snyk settings to not be concerned with the absence of the nom lockfile?

What this fixes

Once merged, Renovate will detect only bun.lock and use bun to regenerate it when dependencies change. The existing open Renovate PRs should automatically rebase and succeed after this lands.

Test plan

  • Add SNYK_TOKEN secret and verify the Snyk SCA workflow produces SARIF findings in the Security tab
  • Confirm the token validation step correctly fails the job when SNYK_TOKEN is missing
  • Verify CI passes on this PR (no frontend behavior changes — only the lockfile is removed)
  • After merge, verify that Renovate rebases an open PR (e.g., chore(deps): update dependency glob to v13 #5624) and renovate/artifacts passes

Renovate fails to update bun.lock when package-lock.json is also present,
causing all frontend dependency PRs to fail CI with "lockfile had changes,
but lockfile is frozen". Removing the npm lockfile lets Renovate detect bun
as the package manager and regenerate bun.lock correctly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jonnalley jonnalley added the security-privacy-compliance Work needed around Security, Privacy, or Compliance label May 14, 2026
@jonnalley jonnalley self-assigned this May 14, 2026
Adds a reusable GitHub Actions workflow that runs Snyk SCA against all
three project components (frontend, backend API, data tools), replacing
the non-functional SCM integration that required a committed npm lockfile.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jonnalley jonnalley marked this pull request as ready for review May 14, 2026 23:20
@jonnalley jonnalley changed the title fix: remove package-lock.json to unblock Renovate bun lockfile updates remove package-lock.json and add Snyk SCA GHA to CI May 14, 2026
Copy link
Copy Markdown
Contributor

@johndeange johndeange left a comment

Choose a reason for hiding this comment

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

Code Review

Overview

This PR solves a real pain point: all Renovate frontend dependency PRs are failing because npm's package-lock.json and bun's bun.lock coexist, causing lockfile conflicts. The fix removes frontend/package-lock.json, gitignores it, and replaces the Snyk SCM integration with a CI-driven workflow that generates the lockfile ephemerally.

Strengths

  • Well-written PR description — clearly explains the problem, root cause, and tradeoffs
  • Good observability — distinguishes "vulnerability found" vs "infrastructure failure" via SARIF file existence check
  • Pin-by-SHA for all third-party actions — solid supply chain practice
  • Minimal permissions scoped per job
  • continue-on-error: true on snyk test is correct — vulnerabilities shouldn't block CI, but infra failures should

Suggestions (non-blocking)

  1. SNYK_TOKEN chicken-and-egg: All 3 Snyk jobs currently fail because the secret isn't configured yet. Consider making the token validation a soft failure (warning + skip) so this PR can merge cleanly and the token configured afterward — or ensure the secret is added before merge.

  2. Reuse composite action: The project has .github/actions/setup-python/action.yml that handles Python/pipenv setup. The snyk-backend-api job could use it to reduce duplication. (The data-tools job can't directly since the composite action hardcodes working-directory: ./backend/ops_api.)

  3. Add timeout-minutes: These jobs depend on the external Snyk API. Adding timeout-minutes: 10 would prevent hangs during Snyk outages (default is 6 hours).

  4. External Snyk check: security/snyk (Flexion-OPRE-OPS) also fails — the legacy SCM integration will likely need to be reconfigured/disabled as a follow-up.

Security

  • No secrets leaked in logs — SNYK_TOKEN passed via env block
  • SHA-pinned actions prevent supply chain attacks
  • Removing the lockfile from git does NOT reduce security — Snyk still scans the ephemeral lockfile, and bun.lock provides reproducible builds

LGTM — architecture is sound and unblocks Renovate. 👍

Copy link
Copy Markdown
Contributor

@josbell josbell left a comment

Choose a reason for hiding this comment

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

Review: Request Changes

Thanks for this PR! The investigation confirms this correctly solves the Renovate lockfile conflict. However, I identified one critical issue that should be addressed before merge.

❗ Required: Add Timeout Protection

Issue: All three Snyk jobs lack timeout-minutes configuration. If Snyk API experiences an outage, jobs could hang for 6 hours (GitHub Actions default).

Fix: Add to each job (lines 16, 78, 140):

```yaml
snyk-frontend:
name: Snyk SCA for Frontend
runs-on: ubuntu-latest
timeout-minutes: 10 # Add this
permissions:
contents: read
security-events: write
```

Repeat for snyk-backend-api and snyk-backend-data-tools jobs.

Rationale: 10 minutes is sufficient for dependency scanning; longer runtime indicates API failure. Only 1 of 12+ workflows in our codebase has timeouts, making this even more important.


💡 Optional Suggestion: Use Composite Action for backend-api

Lines 84-97: Consider using the existing composite action instead of manual Python setup:

```yaml

Replace lines 84-97 with:

  • name: Setup Python
    uses: ./.github/actions/setup-python
    ```

Benefits:

  • Automatic pipenv caching (faster CI)
  • Consistent with other workflows
  • Reduces maintenance burden

Note: Can't use for data_tools job due to hardcoded path in composite action (separate issue to address later).


📋 Follow-up (Out of Scope)

Python 3.14 + setup-python@v4.3.0: Systemic issue affecting 4+ workflows. The v4.3.0 action predates Python 3.14 release, likely causing slower CI. Consider upgrading to v5.x in a separate PR.


Once the timeout is added, this is good to merge! 🚀

@josbell
Copy link
Copy Markdown
Contributor

josbell commented May 19, 2026

📍 Specific Line Comments

File: .github/workflows/security_snyk.yml

@josbell
Copy link
Copy Markdown
Contributor

josbell commented May 19, 2026

Line 16-24 (snyk-frontend job): Missing timeout-minutes: 10 - add after runs-on: ubuntu-latest

@josbell
Copy link
Copy Markdown
Contributor

josbell commented May 19, 2026

Line 78-86 (snyk-backend-api job): Missing timeout-minutes: 10 - add after runs-on: ubuntu-latest

@josbell
Copy link
Copy Markdown
Contributor

josbell commented May 19, 2026

Line 140-148 (snyk-backend-data-tools job): Missing timeout-minutes: 10 - add after runs-on: ubuntu-latest

@josbell
Copy link
Copy Markdown
Contributor

josbell commented May 19, 2026

Lines 84-97 (optional): Consider replacing manual Python setup with composite action uses: ./.github/actions/setup-python to get automatic pipenv caching

- Add timeout-minutes: 10 to all three Snyk jobs to prevent hung jobs
  on Snyk API outages (default is 6 hours)
- Replace manual Python setup in snyk-backend-api with the existing
  setup-python composite action for consistency and pipenv caching

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jonnalley
Copy link
Copy Markdown
Contributor Author

jonnalley commented May 19, 2026

@josbell those were great suggestions. I implemetend the "required" and "optional" changes. Please do re-review when you can, thanks!

@jonnalley jonnalley requested a review from josbell May 19, 2026 20:40
react-currency-format@1.1.0 declares a peer dep on React <=17, which
conflicts with our React 19. npm strict mode refuses to generate a
lockfile. Adding --legacy-peer-deps allows npm to produce the lockfile
for Snyk scanning despite the peer dep mismatch.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@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.

jonnalley and others added 2 commits May 28, 2026 02:05
The snyk/actions/python action runs inside a Docker container that does
not have pipenv installed. Pointing at Pipfile.lock instead of Pipfile
lets Snyk parse the resolved dependency tree directly without needing
pipenv. This also removes the now-unnecessary Python/pipenv setup steps.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The snyk/actions/python Docker action lacks pipenv inside its container,
causing "Failed to run pipenv" errors. Switch to snyk/actions/setup
(installs CLI on the runner) + shell commands so Snyk can access the
runner's pipenv installation and properly resolve the Pipfile.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jonnalley jonnalley merged commit 2f20080 into main May 28, 2026
65 checks passed
@jonnalley jonnalley deleted the fix/renovate-bun-lockfile-support branch May 28, 2026 06:27
@github-actions
Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 1.402.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Labels

released security-privacy-compliance Work needed around Security, Privacy, or Compliance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants