Repository navigation
fix(publish): harden gh calls against arg injection; lock down temp .npmrc - #584
Merged
Merged
Conversation
…p .npmrc Add a leading-dash guard (assertNotOption in @releasekit/core, mirroring git's barrier) on tags passed to gh release view/create/edit, so a tag starting with '-' can never be parsed as a flag (gh exposes -R/--repo, -F/--notes-file). The publish pipeline creates the tag via git first (already guarded), but the notes-backfill path feeds gh tags enumerated from existing repo refs, so an out-of-band refs/tags/-… reached gh unguarded. Also write the token-bearing temp .npmrc with mode 0600, for parity with config's saveAuth. Closes #561 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BT6fVBZrmExXZj7vQ29881
Contributor
|
Standing release PR: #582 · v0.41.1 queued (4 packages) · open 25m · ✅ ready to merge Release Preview — v0.41.0 → v0.41.1
These changes will be added to the release PR (#582) when merged: Changelogmonorepo 0.41.0 → 0.41.1
After merge — predicted release
Updated automatically by ReleaseKit |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two small, self-contained publish-path hardening fixes from the security review (defense-in-depth):
-guard onghcalls. AddsassertNotOptionto@releasekit/core(mirrors git's existing barrier) and applies it to thetagpositional before it reaches theghargv inpackages/publish/src/stages/github-release.ts(bothrelease viewandrelease create) andpackages/release/src/backfill/github-release.ts(getReleaseBody/editReleaseBody/getReleaseInfo).gh(cobra) would otherwise parse a--prefixed tag as a flag (-R/--repo,-F/--notes-file). Reachability is low in the publish pipeline (git creates the tag first and rejects leading--), but the backfill path feedsghtags enumerated from existing repo refs, so an out-of-bandrefs/tags/-…reachedghunguarded. Not RCE —ghhas no shell/exec flag. In the publish stage the guard records a failed per-tag result and continues (preserving the stage's per-tag non-critical error strategy); in backfill it throws (consistent with existing handling).0600mode on the temp.npmrc. The token-bearing temp.npmrcis now writtenmode: 0o600(packages/publish/src/utils/npm-env.ts), for parity with config'ssaveAuth.The guard lives in
@releasekit/core(bothpackages/publishandpackages/releasealready depend on it) since the reachable backfill call site is inpackages/release.Test plan
should …): dash-guard rejection beforeghis invoked (core helper, the three backfill functions, and the publish stage), and the temp.npmrc0600mode.biome check .exit 0; forcedturbo run typecheck test --force→ 24/24, 0 cached.Closes #561
🤖 Generated with Claude Code
https://claude.ai/code/session_01BT6fVBZrmExXZj7vQ29881
Greptile Summary
This PR hardens the publish and backfill paths against argument injection into
ghCLI calls and tightens the file-permission posture of the token-bearing temp.npmrc. Both fixes are narrowly scoped and backed by new unit tests.assertNotOptionguard in@releasekit/core: rejects any tag value starting with-before it reaches aghargv; applied torunGithubReleaseStage(publish, non-critical per-tag strategy) and all three backfill functionsgetReleaseBody/editReleaseBody/getReleaseInfo(throws, consistent with existing error handling there).0o600mode on temp.npmrc: the token-bearing file is now written owner-only, matching the permission thatconfig'ssaveAuthalready enforces and adding a second layer of protection beyond the0700mkdtemp directory.Confidence Score: 5/5
Safe to merge — both changes are narrowly scoped, well-tested, and do not alter any existing behaviour for valid inputs.
The guard correctly uses
startsWith('-')which catches both short (-x) and long (--flag) forms. The per-tag try/catch in the publish stage preserves the existing non-critical error strategy. The backfill functions placeassertNotOptionbefore the try/catch so a bad tag throws rather than being absorbed. The0o600file-mode test masksmode & 0o777correctly. Env-var cleanup in the npm-env test suite usesafterEachto deleteNPM_TOKEN/NODE_AUTH_TOKEN, so no cross-test leakage. No issues found across all nine changed files.No files require special attention.
Important Files Changed
assertNotOptionguard that rejects any value starting with-before it can reach a CLI argv; well-documented and correctly scoped.assertNotOptionhelper from the core public API.assertNotOptionin a per-tag try/catch consistent with the existing non-critical error strategy; records failure and continues rather than aborting the stage.mode: 0o600to the token-bearing temp.npmrcwrite, matching the owner-only permission thatconfig'ssaveAuthalready enforces.ghcall sites (getReleaseBody,editReleaseBody,getReleaseInfo) withassertNotOption; the guard throws synchronously beforeexecFileSyncis reached.assertNotOptiontests cover dash-prefix rejection (single and double dash), kind inclusion in error message, and clean acceptance of normal values.execCommandorexecCommandSafeis invoked, and that a failure result is recorded in the output.fs.statSync(...).mode & 0o777to confirm the written.npmrchas exactly0600permissions;afterEachproperly cleans up env vars and paths.gh argument-injection guardsuite tests all three guarded functions reject dash-prefixed tags without spawningexecFileSync, plus a happy-path check for normal tags.Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Caller participant assertNotOption participant gh Note over Caller,gh: publish stage (non-critical, per-tag) Caller->>assertNotOption: assertNotOption(tag, 'tag') alt tag starts with '-' assertNotOption-->>Caller: throws Error Caller->>Caller: "push {success:false, reason} → continue" else tag is safe assertNotOption-->>Caller: returns void Caller->>gh: execCommandSafe / execCommand gh-->>Caller: result end Note over Caller,gh: backfill path (throws to surface errors) Caller->>assertNotOption: assertNotOption(tag, 'tag') alt tag starts with '-' assertNotOption-->>Caller: throws Error (propagates up) else tag is safe assertNotOption-->>Caller: returns void Caller->>gh: execFileSync(gh, [...]) gh-->>Caller: stdout end%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Caller participant assertNotOption participant gh Note over Caller,gh: publish stage (non-critical, per-tag) Caller->>assertNotOption: assertNotOption(tag, 'tag') alt tag starts with '-' assertNotOption-->>Caller: throws Error Caller->>Caller: "push {success:false, reason} → continue" else tag is safe assertNotOption-->>Caller: returns void Caller->>gh: execCommandSafe / execCommand gh-->>Caller: result end Note over Caller,gh: backfill path (throws to surface errors) Caller->>assertNotOption: assertNotOption(tag, 'tag') alt tag starts with '-' assertNotOption-->>Caller: throws Error (propagates up) else tag is safe assertNotOption-->>Caller: returns void Caller->>gh: execFileSync(gh, [...]) gh-->>Caller: stdout endReviews (1): Last reviewed commit: "fix(publish): harden gh calls against ar..." | Re-trigger Greptile