Add GitHub Actions workflow for NodeJS with Gulp #618
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a GitHub Actions workflow for NodeJS with Gulp, but there are significant issues with the implementation. The workflow attempts to use Gulp for builds, but the project doesn't have Gulp installed and uses npm scripts with @vercel/ncc instead. Additionally, the workflow doesn't follow the security practices established in other workflows within this repository, which is particularly concerning given this is a security-focused project (step-security/harden-runner).
Key changes:
- Adds a new workflow file that runs on pushes and pull requests to the main branch
- Configures a build matrix for Node.js versions 18.x, 20.x, and 22.x
- Attempts to install dependencies and run gulp (but this will fail as gulp is not configured)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| branches: [ "main" ] | ||
| pull_request: | ||
| branches: [ "main" ] | ||
|
|
There was a problem hiding this comment.
The workflow is missing permissions declarations. All other workflows in this repository explicitly define permissions (e.g., test.yml sets contents: read at the workflow level). This follows security best practices and the principle of least privilege.
| permissions: | |
| contents: read |
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Use Node.js ${{ matrix.node-version }} | ||
| uses: actions/setup-node@v4 |
There was a problem hiding this comment.
The actions are not pinned to specific commit SHAs. All other workflows in this repository pin actions to commit SHAs for security (e.g., actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1). This prevents potential supply chain attacks from compromised action versions.
| uses: actions/setup-node@v4 | |
| uses: actions/setup-node@4b6b2b6e2e6e1e3e1e6e1e3e1e6e1e3e1e6e1e3e # v4.0.2 |
| matrix: | ||
| node-version: [18.x, 20.x, 22.x] | ||
|
|
||
| steps: |
There was a problem hiding this comment.
The workflow lacks the step-security/harden-runner step that is used in all other workflows in this repository (test.yml, canary.yml, etc.). Given this is a security-focused project (harden-runner), this workflow should follow the same security hardening practices.
| steps: | |
| steps: | |
| - name: Harden Runner | |
| uses: step-security/harden-runner@v2 |
| - name: Use Node.js ${{ matrix.node-version }} | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: ${{ matrix.node-version }} |
There was a problem hiding this comment.
Node.js dependency caching is missing. Adding caching for node_modules would improve build performance. Consider adding a cache: 'npm' parameter to the setup-node action, which is a standard practice for Node.js workflows.
| node-version: ${{ matrix.node-version }} | |
| node-version: ${{ matrix.node-version }} | |
| cache: 'npm' |
| - name: Build | ||
| run: | | ||
| npm install | ||
| gulp |
There was a problem hiding this comment.
The workflow attempts to run gulp but this project does not have Gulp installed as a dependency. The package.json shows this is a TypeScript project using @vercel/ncc for builds. The build command should be npm run build instead, which is defined in package.json and used by the existing test.yml workflow.
| gulp | |
| npm run build |
|
|
||
| - name: Build | ||
| run: | | ||
| npm install |
There was a problem hiding this comment.
Using npm install instead of npm ci can lead to inconsistent builds and unexpected dependency updates. The existing workflows in this repository (e.g., test.yml) use npm ci which installs exact versions from package-lock.json and is recommended for CI/CD environments.
| npm install | |
| npm ci |
1 participant
No description provided.