Remove semgrep workflow #1663
Remove semgrep workflow #1663
Conversation
Updated Semgrep workflow to use native action and added caching for node modules.
Updated Semgrep workflow to include Node.js setup and caching.
There was a problem hiding this comment.
Pull request overview
This PR replaces the semgrep security scanning workflow with CodeQL for advanced security analysis of JavaScript/TypeScript code. The new workflow is configured to run on pushes, pull requests, weekly on Wednesdays, and can be manually triggered.
Key changes:
- Adds CodeQL workflow with security-extended and security-and-quality query packs
- Configures automated scanning for JavaScript/TypeScript codebase
- Sets up workflow to run on multiple Node.js versions (18 and 20) with static analysis mode
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Cache node modules | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: | | ||
| ~/.npm | ||
| node_modules | ||
| key: ${{ runner.os }}-node-${{ matrix.node }}-${{ hashFiles('**/package-lock.json', '**/yarn.lock') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-node-${{ matrix.node }}- | ||
|
|
There was a problem hiding this comment.
This cache step is redundant because the setup-node action (line 40-43) already handles caching with the 'cache: npm' parameter. Having both can lead to cache conflicts and unnecessary overhead. Consider removing this manual cache step and relying on the built-in caching from setup-node.
| - name: Cache node modules | |
| uses: actions/cache@v4 | |
| with: | |
| path: | | |
| ~/.npm | |
| node_modules | |
| key: ${{ runner.os }}-node-${{ matrix.node }}-${{ hashFiles('**/package-lock.json', '**/yarn.lock') }} | |
| restore-keys: | | |
| ${{ runner.os }}-node-${{ matrix.node }}- |
| - name: Autobuild (CodeQL) | ||
| uses: github/codeql-action/autobuild@v4 | ||
| env: | ||
| CI: true | ||
|
|
There was a problem hiding this comment.
The matrix specifies 'build-mode: none' but the workflow includes an Autobuild step. When build-mode is 'none', CodeQL analyzes the code without building it, making the Autobuild step unnecessary. Either remove the Autobuild step (recommended for JavaScript/TypeScript projects which typically don't need building for CodeQL analysis) or change build-mode to 'autobuild' if building is required.
| - name: Autobuild (CodeQL) | |
| uses: github/codeql-action/autobuild@v4 | |
| env: | |
| CI: true |
| - name: Install dependencies | ||
| run: | | ||
| if [ -f package-lock.json ]; then | ||
| npm ci | ||
| else | ||
| npm install | ||
| fi | ||
|
|
There was a problem hiding this comment.
Installing dependencies is unnecessary when using build-mode 'none' for JavaScript/TypeScript analysis. CodeQL can analyze the source code directly without installed dependencies. This step adds unnecessary time to the workflow execution. Consider removing this step or only including it if you change to a build mode that requires it.
| - name: Install dependencies | |
| run: | | |
| if [ -f package-lock.json ]; then | |
| npm ci | |
| else | |
| npm install | |
| fi |
| - name: Setup Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: ${{ matrix.node }} | ||
| cache: 'npm' | ||
|
|
||
| - name: Cache node modules | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: | | ||
| ~/.npm | ||
| node_modules | ||
| key: ${{ runner.os }}-node-${{ matrix.node }}-${{ hashFiles('**/package-lock.json', '**/yarn.lock') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-node-${{ matrix.node }}- | ||
|
|
||
| - name: Install dependencies | ||
| run: | | ||
| if [ -f package-lock.json ]; then | ||
| npm ci | ||
| else | ||
| npm install | ||
| fi | ||
|
|
There was a problem hiding this comment.
Setting up Node.js is unnecessary for CodeQL analysis with build-mode 'none'. CodeQL for JavaScript/TypeScript performs static analysis on the source code and doesn't require a Node.js runtime or dependencies. This step (along with the cache and install steps) can be removed to speed up the workflow.
| - name: Setup Node.js | |
| uses: actions/setup-node@v4 | |
| with: | |
| node-version: ${{ matrix.node }} | |
| cache: 'npm' | |
| - name: Cache node modules | |
| uses: actions/cache@v4 | |
| with: | |
| path: | | |
| ~/.npm | |
| node_modules | |
| key: ${{ runner.os }}-node-${{ matrix.node }}-${{ hashFiles('**/package-lock.json', '**/yarn.lock') }} | |
| restore-keys: | | |
| ${{ runner.os }}-node-${{ matrix.node }}- | |
| - name: Install dependencies | |
| run: | | |
| if [ -f package-lock.json ]; then | |
| npm ci | |
| else | |
| npm install | |
| fi |
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| include: | ||
| - language: javascript-typescript | ||
| node: 18 | ||
| build-mode: none | ||
| - language: javascript-typescript | ||
| node: 20 | ||
| build-mode: none |
There was a problem hiding this comment.
The matrix strategy runs CodeQL analysis twice (once for Node 18 and once for Node 20), but since build-mode is 'none', the Node version doesn't affect the analysis results. CodeQL analyzes the source code statically without executing it. Running duplicate analyses wastes CI resources and time. Consider removing the matrix and running a single analysis job, or if you need to test against multiple Node versions, use the existing Node.js CI workflow instead.
1 participant
No description provided.