Skip to content

feat: add npm trusted publishing detection#1161

Merged
woodruffw merged 8 commits into
zizmorcore:mainfrom
KristianGrafana:feat/npm-trusted-publishing
Sep 18, 2025
Merged

feat: add npm trusted publishing detection#1161
woodruffw merged 8 commits into
zizmorcore:mainfrom
KristianGrafana:feat/npm-trusted-publishing

Conversation

@KristianGrafana

@KristianGrafana KristianGrafana commented Sep 17, 2025

Copy link
Copy Markdown
Contributor

This PR adds comprehensive npm trusted publishing detection to zizmor.

Changes Made

  • Add npmjs registry detection: Detects when actions/setup-node uses npmjs registry with manual authentication
  • Add npm command patterns: Detects npm publish, yarn run publish, pnpm publish, etc.
  • Improve hybrid scenario detection: Flags workflows that have id-token: write but still use manual tokens (incomplete migration)
  • Add comprehensive tests: 137 lines of test cases covering various npm publishing scenarios

This addresses the gap in npm package publishing security by recommending trusted publishing over manual token usage.

- Add npmjs registry constants and action coordinates
- Detect npm/yarn/pnpm publish commands that should use trusted publishing
- Improve detection logic to catch hybrid scenarios (id-token present but manual auth used)
- Add comprehensive test cases for npm publishing workflows
- Detect yarn run publish, npm run publish, pnpm run publish patterns

Resolves npm package publishing security issues by recommending trusted publishing over manual NODE_AUTH_TOKEN usage.
@kusari-inspector

kusari-inspector Bot commented Sep 17, 2025

Copy link
Copy Markdown

Kusari Inspector

Kusari Analysis Results:

Proceed with these changes

✅ No Flagged Issues Detected
All values appear to be within acceptable risk parameters.

No pinned version dependency changes, code issues or exposed secrets detected!

Note

View full detailed analysis result for more information on the output and the checks that were run.


@kusari-inspector rerun - Trigger a re-analysis of this PR
@kusari-inspector feedback [your message] - Send feedback to our AI and team
See Kusari's documentation for setup and configuration.
Commit: e01dd91, performed at: 2025-09-17T08:47:09Z

Found this helpful? Give it a 👍 or 👎 reaction!

@woodruffw

Copy link
Copy Markdown
Member

Awesome, thanks @KristianGrafana! I'll try and review this tonight.

@woodruffw woodruffw added the enhancement New feature or request label Sep 17, 2025
Signed-off-by: William Woodruff <william@yossarian.net>
@kusari-inspector

Copy link
Copy Markdown

Kusari PR Analysis rerun based on - 3eab607 performed at: 2025-09-18T02:31:55Z - link to updated analysis

Signed-off-by: William Woodruff <william@yossarian.net>
@kusari-inspector

Copy link
Copy Markdown

Kusari PR Analysis rerun based on - eae6c03 performed at: 2025-09-18T11:59:56Z - link to updated analysis

Comment on lines +364 to +367
} else {
// Medium confidence when no id-token - could be intentional for non-TP registries
Confidence::Medium
};

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@KristianGrafana Thoughts on this being low-confidence instead? I suspect this will snare a lot of third-party index users (especially in ecosystems like Python where 3p indices are common).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(But I also don't feel super confident about that either, so I'm curious what you think.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@woodruffw Good point, agree. id-token is mandatory for TP so Low makes more sense. Thanks! Added in d8794bb

- Change confidence from Medium to Low for publishing commands when no id-token permission
- High confidence: has id-token but uses manual tokens (hybrid/incomplete migration)
- Low confidence: no id-token, uses manual tokens (suggestions, could be intentional)
- Reduces noise and provides more nuanced confidence scoring
- Update test snapshots to reflect improved confidence levels
@kusari-inspector

Copy link
Copy Markdown

Kusari PR Analysis rerun based on - d8794bb performed at: 2025-09-18T14:57:23Z - link to updated analysis

@woodruffw woodruffw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @KristianGrafana, this looks great to me!

(The online snapshots need to be bumped too, but I can do that later today unless you feel like it. There are steps in the development docs 🙂)

Signed-off-by: William Woodruff <william@yossarian.net>
@kusari-inspector

Copy link
Copy Markdown

Kusari PR Analysis rerun based on - 10b1ece performed at: 2025-09-18T23:20:29Z - link to updated analysis

@kusari-inspector

Copy link
Copy Markdown

Kusari PR Analysis rerun based on - 29f9df9 performed at: 2025-09-18T23:20:55Z - link to updated analysis

Signed-off-by: William Woodruff <william@yossarian.net>
@kusari-inspector

Copy link
Copy Markdown

Kusari PR Analysis rerun based on - e01dd91 performed at: 2025-09-18T23:22:15Z - link to updated analysis

@woodruffw
woodruffw merged commit 8581a84 into zizmorcore:main Sep 18, 2025
9 checks passed
@woodruffw

Copy link
Copy Markdown
Member

Thanks again @KristianGrafana!

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants