ci(SDK-1375): harden npm publish/unpublish release workflows (#2898) · Gusto/embedded-react-sdk@a891cb6 · GitHub
Skip to content

Commit a891cb6

Browse files
ci(SDK-1375): harden npm publish/unpublish release workflows (#2898)
* ci(SDK-1375): harden npm publish/unpublish release workflows - Publish no longer runs package lifecycle scripts, since only the registry call itself needs the token. - Stable releases drop the manual trigger; only the automated, CI-gated path can publish. - RC/unpublish workflow_dispatch inputs move into env vars with allowlist validation before use, instead of being interpolated into shell scripts. - Release jobs always install and build fresh instead of restoring node_modules/dist from cache. - publish-rc/unpublish/unpublish-rc gain a dry_run option so they can be exercised end-to-end without touching the registry. - Adds a shared input validator with its own test, run in CI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(SDK-1375): update release docs for the removed manual publish trigger Publish to NPM no longer accepts workflow_dispatch, so the manual "re-run from Actions UI" escape hatch in CONTRIBUTING.md no longer exists. Points readers to Publish RC to NPM for on-demand builds and to re-running CI as the recovery path for a missed auto-publish. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(SDK-1375): label dry-run output as dry-run Dispatching publish-rc/unpublish-rc with dry_run: true still printed "Published"/"Successfully unpublished", which reads as confirmation that the registry was actually mutated. Found by running the real dry-run dispatches on this branch — 0.56.7-rc.1 correctly never appeared on the registry and 0.54.1-rc.1 correctly stayed published, but the messaging didn't reflect that. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ci(SDK-1375): isolate NPM_TOKEN from the SDK build and pin remaining release refs Splits publish.yaml and publish-rc.yaml into a secret-less build job (new reusable build-sdk.yaml: checkout, npm ci, i18n:generate, vite build, upload dist artifact) and a thin publish job that only downloads the artifact, tags/bumps version, and runs npm publish/version with NPM_TOKEN. No devDependency or build-tool code now ever executes in the same runner as the token. publish-rc.yaml's validate job resolves its branch/commit input to a concrete commit SHA before fanning out to build and publish — those run as separate jobs minutes apart, so re-resolving a mutable branch name independently in each could otherwise build dist/ from one commit and publish package.json/CHANGELOG.md from a later one if the branch moved in between. Also: pins publish.yaml's checkout to the triggering workflow_run's head_sha instead of main's tip; adds a validate-release-workflows.sh CI guard against actions/cache creeping into these jobs, untrusted input/event values bypassing env:, or a workflow dropping its validator call; documents the no-cache/--ignore-scripts/env-only rules inline; and tightens the rc-number input pattern to reject leading zeros. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(SDK-1375): close three detection gaps in validate-release-workflows.sh Code review found the new guard script could be bypassed three ways: single-line `run: <cmd>` steps (and the dash-attached `- run:` step form) were invisible to the injection check since it only scanned block-scalar `run: |`/`run: >` bodies; the regex required a non-dot character between the `${{` delimiter and `inputs.`, missing the common no-space form `${{inputs.x}}`; and the "validator still called" check was a bare substring grep satisfied by the filename appearing in a comment, not a real invocation. Reproduced all three against a scratch copy of the workflows before and after to confirm the fix actually closes them, and re-verified the real workflow files still pass clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(SDK-1375): make husky's prepare script tolerant of a missing binary The publish job in publish.yaml/publish-rc.yaml deliberately never runs npm ci, so devDependencies aren't installed there. npm publish's --ignore-scripts does not suppress the "prepare" lifecycle script (a long-standing npm bug fixed in npm 11, confirmed against CI's pinned npm 10.9.8 — see npm/cli#7211), so every real RC/stable publish was failing with "sh: husky: command not found", caught by re-running the dry-run verification against the restructured workflows. Considered moving the hook to postinstall instead, but postinstall (unlike prepare) also fires when the package is installed as a dependency by a consumer — confirmed empirically — which would break every integrator's npm install since husky is a devDependency never present in their tree. "prepare": "husky || true" keeps the hook exempt from running for consumers while making the script itself survive CI's missing-devDependencies case. Upgrading the publish job to npm 11+ would fix this at the root but is being tracked separately as a wider dependency decision. Also renames validate-release-workflows.sh to validate-release-workflows.test.sh to signal it's a test script, not something invoked from a release workflow directly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * refactor(SDK-1375): trim duplicated comment blocks in release workflows Each hardening-rationale comment (no-cache, no-npm-ci, env-only input handling) was repeated near-verbatim across 3-4 workflow files, and publish-rc.yaml explained the ref-pinning fix twice in the same file (once at the job outputs, once at the step that does it). Trims each to the one non-obvious fact plus a pointer to validate-release-workflows.test.sh, which already states the full mechanism once. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 7da6137 commit a891cb6

11 files changed

Lines changed: 487 additions & 132 deletions
Lines changed: 50 additions & 0 deletions
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
#!/usr/bin/env bash
2+
# Exercises validate-release-input.sh's allowlists directly, so the
3+
# publish/unpublish workflows' input validation can be checked locally
4+
# and in CI without dispatching a real workflow or touching the npm
5+
# registry. Run: bash .github/scripts/validate-release-input.test.sh
6+
set -euo pipefail
7+
8+
cd "$(dirname "${BASH_SOURCE[0]}")"
9+
SCRIPT=./validate-release-input.sh
10+
11+
pass=0
12+
fail=0
13+
14+
assert_valid() {
15+
local kind="$1" value="$2"
16+
if "$SCRIPT" "$kind" "$value" >/dev/null 2>&1; then
17+
pass=$((pass + 1))
18+
else
19+
fail=$((fail + 1))
20+
echo "FAIL: expected '$value' to be a valid $kind"
21+
fi
22+
}
23+
24+
assert_invalid() {
25+
local kind="$1" value="$2"
26+
if "$SCRIPT" "$kind" "$value" >/dev/null 2>&1; then
27+
fail=$((fail + 1))
28+
echo "FAIL: expected '$value' to be rejected as $kind"
29+
else
30+
pass=$((pass + 1))
31+
fi
32+
}
33+
34+
# semver
35+
assert_valid semver "0.56.7"
36+
assert_valid semver "0.56.7-rc.12"
37+
assert_invalid semver ""
38+
assert_invalid semver "0.56.7; curl evil.example"
39+
assert_invalid semver "\$(whoami)"
40+
assert_invalid semver $'0.56.7-rc.1\nNODE_OPTIONS=--require=/tmp/x'
41+
42+
# semver-base
43+
assert_valid semver-base "0.56.7"
44+
assert_invalid semver-base "0.56.7-rc.1"
45+
assert_invalid semver-base "latest"
46+
47+
# commit-sha
48+
assert_valid commit-sha "a1b2c3d"
49+
assert_valid commit-sha "a1b2c3d4e5f60718293a4b5c6d7e8f9a0b1c2d3"
50+
assert_invalid commit-sha "main\"; echo pwned; \""
51+
assert_invalid commit-sha "../../etc/passwd"
52+
assert_invalid commit-sha "abc"
53+
54+
# branch-name
55+
assert_valid branch-name "main"
56+
assert_valid branch-name "feat/marie/sdk-1375-harden-publish-workflows"
57+
assert_invalid branch-name "-rf /"
58+
assert_invalid branch-name "main\" && curl evil.example \""
59+
assert_invalid branch-name "\$(printenv NODE_AUTH_TOKEN)"
60+
61+
# rc-number
62+
assert_valid rc-number "1"
63+
assert_valid rc-number "42"
64+
assert_invalid rc-number "1; rm -rf /"
65+
assert_invalid rc-number "one"
66+
assert_invalid rc-number "01"
67+
assert_invalid rc-number "0"
68+
69+
# boolean
70+
assert_valid boolean "true"
71+
assert_valid boolean "false"
72+
assert_invalid boolean "TRUE"
73+
assert_invalid boolean "true; echo pwned"
74+
75+
# confirm
76+
assert_valid confirm "CONFIRM"
77+
assert_invalid confirm "confirm"
78+
assert_invalid confirm "CONFIRM\" || npm publish \""
79+
80+
# unpublish-type
81+
assert_valid unpublish-type "specific_version"
82+
assert_valid unpublish-type "latest_rc"
83+
assert_valid unpublish-type "all_rc_for_version"
84+
assert_invalid unpublish-type "specific_version; rm -rf /"
85+
assert_invalid unpublish-type "anything_else"
86+
87+
# unknown kind fails closed
88+
assert_invalid made-up-kind "whatever"
89+
90+
echo ""
91+
echo "$pass passed, $fail failed"
92+
[ "$fail" -eq 0 ]
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
#!/usr/bin/env bash
2+
# Guards the npm publish/unpublish release workflows against regressions
3+
# this hardening pass fixed: a cache action reintroduced into an
4+
# NPM_TOKEN-bearing job, an untrusted dispatch input/event value
5+
# interpolated directly into a shell script instead of passed through
6+
# `env:`, or a workflow that stops calling the input validator.
7+
set -euo pipefail
8+
9+
cd "$(dirname "${BASH_SOURCE[0]}")/.."
10+
11+
fail=0
12+
# Workflows that hold NPM_TOKEN at some point (directly, or via a job in
13+
# the same file) — the cache-poisoning risk this guards against.
14+
token_bearing_workflows=(workflows/publish.yaml workflows/publish-rc.yaml workflows/unpublish.yaml workflows/unpublish-rc.yaml)
15+
# Also includes build-sdk.yaml: it's secret-less, but it still consumes an
16+
# untrusted-ish `ref` input forwarded from publish-rc.yaml, so a future
17+
# edit that starts interpolating it directly into a run: script instead of
18+
# via env:/action inputs is still worth catching here.
19+
input_consuming_workflows=("${token_bearing_workflows[@]}" workflows/build-sdk.yaml)
20+
dispatch_workflows=(workflows/publish-rc.yaml workflows/unpublish.yaml workflows/unpublish-rc.yaml)
21+
22+
# Extracts the shell content of every `run:` step — both block-scalar
23+
# (`run: |`/`run: >`, body = lines indented further than the `run:` key)
24+
# and single-line (`run: <command>`) forms — so the checks below only look
25+
# at shell script content, not YAML mapping values like `env:` or `with:`.
26+
run_block_bodies() {
27+
awk '
28+
/^[[:space:]]*(-[[:space:]]+)?run:[[:space:]]*[|>]/ {
29+
match($0, /^[[:space:]]*/); indent = RLENGTH; in_run = 1; next
30+
}
31+
/^[[:space:]]*(-[[:space:]]+)?run:[[:space:]]*[^|>[:space:]]/ {
32+
sub(/^[[:space:]]*(-[[:space:]]+)?run:[[:space:]]*/, ""); print; next
33+
}
34+
in_run {
35+
if ($0 ~ /^[[:space:]]*$/) { next }
36+
match($0, /^[[:space:]]*/)
37+
if (RLENGTH <= indent) { in_run = 0 } else { print }
38+
}
39+
' "$1"
40+
}
41+
42+
for f in "${token_bearing_workflows[@]}"; do
43+
if grep -q 'uses:.*actions/cache' "$f"; then
44+
echo "FAIL: $f uses actions/cache in an NPM_TOKEN-bearing workflow"
45+
fail=1
46+
fi
47+
done
48+
49+
for f in "${input_consuming_workflows[@]}"; do
50+
while IFS= read -r line; do
51+
# Pull out each ${{ ... }} expression and check its *own* contents for
52+
# a github.event.*/inputs.* reference at a real token boundary (start
53+
# of the expression, or preceded by a non-identifier character) —
54+
# rather than matching against the raw line, which required a
55+
# boundary character between the `${{` delimiter itself and the
56+
# identifier and so missed the common no-space form `${{inputs.x}}`.
57+
while IFS= read -r expr_match; do
58+
expr="${expr_match#\$\{\{}"
59+
expr="${expr%\}\}}"
60+
if [[ "$expr" =~ (^|[^.[:alnum:]_])(github\.event\.|inputs\.) ]]; then
61+
echo "FAIL: $f interpolates an untrusted input/event value directly in a run: script (should go through env:): $line"
62+
fail=1
63+
fi
64+
done < <(grep -oE '\$\{\{[^}]*\}\}' <<< "$line")
65+
done < <(run_block_bodies "$f")
66+
done
67+
68+
for f in "${dispatch_workflows[@]}"; do
69+
if ! grep -v '^[[:space:]]*#' "$f" | grep -q 'validate-release-input\.sh'; then
70+
echo "FAIL: $f no longer calls validate-release-input.sh"
71+
fail=1
72+
fi
73+
done
74+
75+
if [ "$fail" -eq 0 ]; then
76+
echo "All release-workflow guard checks passed."
77+
fi
78+
exit $fail

‎.github/workflows/build-sdk.yaml‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# Reusable workflow: installs dependencies and builds the SDK for a given
2+
# ref, so stable and RC publish run the exact same build steps and that
3+
# logic lives in one place. Deliberately takes no secrets and requests
4+
# only read permissions — this job runs npm ci/i18n:generate/vite build,
5+
# which execute devDependency and plugin code, so NPM_TOKEN must never be
6+
# reachable here. The caller's separate publish job downloads this job's
7+
# `dist` artifact and never runs npm ci itself, keeping dependency/build
8+
# code execution out of the runner that holds the token.
9+
name: Build SDK
10+
on:
11+
workflow_call:
12+
inputs:
13+
ref:
14+
description: 'Git ref (branch, tag, or commit SHA) to build'
15+
required: true
16+
type: string
17+
jobs:
18+
build:
19+
runs-on: ubuntu-latest
20+
permissions:
21+
contents: read
22+
steps:
23+
- uses: actions/checkout@v6
24+
with:
25+
ref: ${{ inputs.ref }}
26+
27+
- uses: actions/setup-node@v6
28+
with:
29+
node-version-file: '.nvmrc'
30+
31+
- name: Install dependencies
32+
run: npm ci
33+
34+
- name: Generate i18n translations
35+
run: npm run i18n:generate
36+
37+
- name: Build
38+
run: npx vite build
39+
40+
- name: Upload dist
41+
uses: actions/upload-artifact@v6
42+
with:
43+
name: dist
44+
path: dist/
45+
retention-days: 1
46+
if-no-files-found: error

‎.github/workflows/ci.yaml‎

Lines changed: 17 additions & 0 deletions

0 commit comments

Comments
 (0)