Skip to content

fix: add workflow permissions and npm provenance #16

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
evansantos merged 4 commits into from
Feb 19, 2026
Merged

fix: add workflow permissions and npm provenance #16

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
be372f9
fix: add workflow permissions and npm provenance for publish
evansantos Feb 19, 2026
f7b6043
fix: make npm audit non-fatal in CI
evansantos Feb 19, 2026
316b686
fix: address GitSniff review warnings
evansantos Feb 19, 2026
7e42f54
fix: implement GitSniff suggestions properly
evansantos Feb 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ on:
pull_request:
branches: [ main, develop ]
Comment on lines 6 to 7
Copy link
Copy Markdown

@gitsniff gitsniff bot Feb 19, 2026

Choose a reason for hiding this comment

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

🟡 Warning

The current CI workflow for ci.yml is triggered only on pull_request events for main and develop branches. This means that direct pushes to these branches, or any other branch, will not trigger the CI workflow. This can lead to situations where code is pushed directly to the main or develop branch without being fully tested by the CI pipeline, potentially introducing regressions or untested code.

Fix: Add push events as a trigger for the CI workflow to ensure that all pushes to the specified branches (main, develop) also run the CI checks. This provides more comprehensive testing and prevents untested code from being merged or deployed. It also provides immediate feedback on the health of the branch after a direct push.

🤖 Prompt for AI Agents 
In .github/workflows/ci.yml around line 6:

Issue: The current CI workflow for `ci.yml` is triggered only on `pull_request` events for `main` and `develop` branches. This means that direct pushes to these branches, or any other branch, will not trigger the CI workflow. This can lead to situations where code is pushed directly to the main or develop branch without being fully tested by the CI pipeline, potentially introducing regressions or untested code.

Add `push` events as a trigger for the CI workflow to ensure that all pushes to the specified branches (main, develop) also run the CI checks. This provides more comprehensive testing and prevents untested code from being merged or deployed. It also provides immediate feedback on the health of the branch after a direct push.

Replace with:
on:
  push:
    branches: [ main, develop ]
  pull_request:
    branches: [ main, develop ]


Copy link
Copy Markdown

@gitsniff gitsniff bot Feb 19, 2026

Choose a reason for hiding this comment

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

⚠️ Security

The ci.yml workflow specifies permissions for pull_request events, but not for push events. While contents: read is a sensible default, it's good practice to explicitly define permissions for all trigger events if they differ or if default permissions are insufficient for certain jobs. This ensures consistent and explicit permission management.

Fix: Add a permissions: block directly under the on:push: event or at the top-level on: block to cover all events where permissions might be required. In this case, contents: read is likely sufficient for the push event as well.

permissions:
contents: read

jobs:
test:
runs-on: ubuntu-latest
Expand Down Expand Up @@ -60,10 +63,10 @@ jobs:
steps:
- uses: actions/checkout@v4

- name: Use Node.js 20.x
- name: Use Node.js 22.x
uses: actions/setup-node@v4
with:
node-version: '20.x'
node-version: '22.x'
Copy link
Copy Markdown

@gitsniff gitsniff bot Feb 19, 2026

Choose a reason for hiding this comment

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

ℹ️ Info

The build-check job uses Node.js 22.x, but the lint, typecheck, and security jobs explicitly set node-version: ${{ env.NODE_VERSION }}, which is defined as 22 at the workflow level. For consistency and to avoid potential discrepancies, all jobs should reference the NODE_VERSION environment variable. This ensures a single source of truth for the Node.js version across the CI.

Fix: Modify the build-check job to use node-version: ${{ env.NODE_VERSION }} instead of a hardcoded '22.x'. This makes the Node.js version consistent across all jobs that use it and simplifies future updates.

Suggested change
- name: Use Node.js 22.x
uses: actions/setup-node@v4
with:
node-version: '20.x'
node-version: '22.x'
- name: Use Node.js ${{ env.NODE_VERSION }}
uses: actions/setup-node@v4
with:
node-version: ${{ env.NODE_VERSION }}

cache: 'npm'

- name: Install dependencies
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/publish.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ jobs:
runs-on: ubuntu-latest
permissions:
contents: write
id-token: write

steps:
- uses: actions/checkout@v4
Expand All @@ -48,7 +49,7 @@ jobs:

- name: Publish to npm
if: ${{ github.event.inputs.dry_run != 'true' }}
run: npm publish --access public
run: npm publish --access public --provenance
env:
NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}

Expand Down
Loading