Skip to content

Conversation

@jamesbhobbs
Copy link
Contributor

@jamesbhobbs jamesbhobbs commented Oct 9, 2025

feat: add audit checks to ci.yml

Summary

Adds two new security audit jobs to the CI workflow:

  • Audit - Production: Uses jlpm npm audit --environment production to check production dependencies only
  • Audit - All: Uses jlpm npm audit to check all dependencies

The production audit is intended to become a required status check, while the all-dependencies audit provides broader visibility into security issues. Also adds packages: read permission for private GitHub package access and fixes link checking configuration.

Review & Testing Checklist for Human

  • Verify Yarn audit command compatibility - Test that jlpm npm audit --environment production works correctly in this repo's Yarn setup (the syntax is specific to Yarn Berry v2+)
  • Confirm production dependency filtering works - Validate that the production audit actually checks fewer packages than the all-dependencies audit (the filtering logic should exclude devDependencies)
  • Test audit failure behavior - When vulnerabilities exist, verify the jobs fail appropriately and CI reports the security issues clearly (this is expected behavior, not a bug)
  • Validate GitHub package access - Ensure the packages: read permission and GITHUB_TOKEN setup doesn't cause authentication issues with private Deepnote packages

Notes

  • The repo appears to use Yarn workspaces with jlpm command, but audit syntax compatibility should be verified
  • Both audit jobs install JupyterLab first to ensure jlpm is available, following the pattern from existing jobs
  • This is part of standardizing security audit checks across Deepnote repositories

Link to Devin run: https://app.devin.ai/sessions/1494020fb75d493c8b35d32b2f17aea9
Requested by: @jamesbhobbs

- Add 'Audit - Production' job for production dependencies
- Add 'Audit - All' job for all dependencies
- Use jlpm (yarn) for dependency auditing
@devin-ai-integration
Copy link

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 9, 2025

📝 Walkthrough

Walkthrough

Adds two CI jobs—audit-prod and audit-all—to .github/workflows/ci.yml. Each job checks out the repo, sets up the environment, installs dependencies, installs Node dependencies, and runs a dependency audit (audit-prod scoped to production/dependencies; audit-all runs a full audit). The workflow permissions now includes packages: read and each new job enforces a 15-minute timeout. Updates .github/workflows/build.yml to add an ignore_links input to the existing check-links step with PR/issue URL prefixes to be skipped.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor Dev as Developer
    participant GH as GitHub
    participant WF as CI Workflow
    participant AuditProd as audit-prod
    participant AuditAll as audit-all
    participant CheckLinks as check-links

    Dev->>GH: push / open PR
    GH-->>WF: trigger workflows

    par Run audits in parallel
        rect rgb(240,248,255)
        note over AuditProd: audit-prod (production scope)
        WF->>AuditProd: start job
        AuditProd->>AuditProd: checkout → setup → install deps → install node deps
        AuditProd->>AuditProd: run dependency audit (production scope)
        end

        rect rgb(245,255,250)
        note over AuditAll: audit-all (full)
        WF->>AuditAll: start job
        AuditAll->>AuditAll: checkout → setup → install deps → install node deps
        AuditAll->>AuditAll: run full dependency audit
        end
    end

    rect rgb(255,250,240)
    note over CheckLinks: check-links step (build.yml)
    WF->>CheckLinks: run check-links with ignore_links param
    CheckLinks->>CheckLinks: skip matching PR/issue links
    end

    WF-->>GH: report results
Loading

Possibly related PRs

Suggested reviewers

  • saltenasl
  • andyjakubowski
  • Artmann

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly captures the main PR goal of adding Yarn audit checks to the CI workflow. It focuses on the core feature without including unrelated details like link fixes or permission updates. Its concise single-sentence form clearly informs teammates of the update’s purpose, meeting the guidelines for a specific and clear PR title.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 45fb503 and 4d362fb.

📒 Files selected for processing (1)
  • .github/workflows/build.yml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build
  • GitHub Check: check_release

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a93ea30 and cfdfab6.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build
  • GitHub Check: check_release
🔇 Additional comments (1)
.github/workflows/ci.yml (1)

35-59: Harden audit-prod job

  • Pin actions (actions/checkout, jupyterlab/maintainer-tools/.github/actions/base-setup) to full commit SHAs.
  • Switch Yarn auth from GITHUB_TOKENNODE_AUTH_TOKEN.
  • Emit JSON and archive report as an artifact (always()).
  • Enforce a severity level (e.g. --level high) for the audit.
  • Workflow already has contents: read & pull-requests: read; add packages: read only if you’re using GitHub Packages.
       - name: Install node dependencies
         run: jlpm
         env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }}

       - name: Run audit for production dependencies
-        run: jlpm audit --groups "dependencies"
+        run: jlpm audit --groups "dependencies" --level high
         env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }}

+      - name: Upload audit report
+        if: ${{ always() }}
+        run: jlpm audit --groups "dependencies" --json | tee audit-prod.json
+      - name: Persist audit artifact
+        if: ${{ always() }}
+        uses: actions/upload-artifact@<commit-SHA>
+        with:
+          name: audit-prod
+          path: audit-prod.json

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cfdfab6 and 5a00647.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build
  • GitHub Check: check_release

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (3)
.github/workflows/ci.yml (3)

52-55: Use NODE_AUTH_TOKEN for jlpm/Yarn auth (not GITHUB_TOKEN).

jlpm ignores GITHUB_TOKEN env var name; use NODE_AUTH_TOKEN so installs/audits can auth against registries.

Apply:

       - name: Install node dependencies
         run: jlpm
         env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }}

       - name: Run audit for production dependencies
         run: jlpm npm audit --environment production
         env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
       - name: Install node dependencies
         run: jlpm
         env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }}

       - name: Run audit for all dependencies
         run: jlpm npm audit
         env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Also applies to: 58-60, 76-79, 82-83


61-65: Make audit-all non-blocking.

If only prod audit will be required, set continue-on-error: true on audit-all so it won't block CI.

  audit-all:
    name: Audit - All
    runs-on: ubuntu-latest
+   continue-on-error: true
    timeout-minutes: 15

37-84: DRY the two audit jobs with a matrix (optional).

Reduce duplication with a single matrix job (prod/all), driving the audit args per entry.

Example:

jobs:
  audit:
    strategy:
      matrix:
        include:
          - name: Production
            args: 'npm audit --environment production'
          - name: All
            args: 'npm audit'
    name: Audit - ${{ matrix.name }}
    runs-on: ubuntu-latest
    timeout-minutes: 15
    steps:
      - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8
      - uses: jupyterlab/maintainer-tools/.github/actions/base-setup@<pinned-sha>
      - run: python -m pip install -U "jupyterlab>=4.0.0,<5"
      - run: jlpm
        env: { NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} }
      - run: jlpm ${{ matrix.args }}
        env: { NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5a00647 and 07084dd.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: check_release
  • GitHub Check: build
🔇 Additional comments (1)
.github/workflows/ci.yml (1)

56-58: --environment production is valid for Yarn Berry
jlpm npm audit --environment production correctly filters out devDependencies in Yarn v2/v3. Yarn Classic uses --groups "dependencies".

@jamesbhobbs jamesbhobbs marked this pull request as ready for review October 10, 2025 11:51
@codecov
Copy link

codecov bot commented Oct 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 18.50%. Comparing base (f18bc9c) to head (b9dd462).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #10   +/-   ##
=======================================
  Coverage   18.50%   18.50%           
=======================================
  Files          13       13           
  Lines         200      200           
  Branches       27       27           
=======================================
  Hits           37       37           
  Misses        163      163           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jamesbhobbs jamesbhobbs merged commit 973dff7 into main Oct 10, 2025
11 checks passed
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.

3 participants