Skip to content

ci(release): inherit secrets so the npm environment token reaches the publisher - #60

Open
javiertoledo wants to merge 1 commit into
mainfrom
ci/inherit-npm-environment-secret
Open

ci(release): inherit secrets so the npm environment token reaches the publisher#60
javiertoledo wants to merge 1 commit into
mainfrom
ci/inherit-npm-environment-secret

Conversation

@javiertoledo

Copy link
Copy Markdown
Member

The 0.3.1 run got further than the last one — package-release passed, so #59's fix works — and then split:

Result
GHCR ✅ six images promoted at 0.3.1 and sha-73f88778a5a3
npm ❌ nothing — @theagilemonkeys/facility is absent from the registry
tag ❌ none; record-release needs both publishers

publish-npm / publish failed at Bootstrap the package once with a granular token with NPM_BOOTSTRAP_TOKEN is required for the first package publication, and NODE_AUTH_TOKEN printed blank rather than *** — an empty value, not a masked one.

Why

The token is fine. NPM_BOOTSTRAP_TOKEN exists in the npm environment (created 2026-07-31), the environment allows only main, and the publish job correctly declares environment: npm.

The gap is the reusable-workflow boundary: ci.yml calls release.yml with no secrets: key. A called workflow does not receive environment-scoped secrets by declaring the environment alone — without secrets: inherit on the caller, every environment-scoped secret resolves to an empty string rather than erroring. GitHub's docs omit this; it is tracked in actions/runner#1490 and github/docs#44458.

So the job authenticated with nothing and failed the way it would if the secret had never been created.

Why inherit is not a widening here

  • The environment still gates the token: it materializes only in the job naming environment: npm, which keeps the branch policy and approval as the real control.
  • gh api repos/theam/facility/actions/secrets and the org-secrets endpoint both return empty — there is nothing else for inherit to forward.
  • Explicitly mapping NPM_BOOTSTRAP_TOKEN: from the caller cannot work: the calling job does not declare the environment, so it would forward an empty string too.

publish-images needs no change — it uses GITHUB_TOKEN, which called workflows always receive.

Verification

node --test scripts/*.test.mjs → 88/88. The new test asserts secrets: inherit on the caller and environment: npm on the callee; removing either makes it fail, checked both ways. Both workflows parse, and secrets is accepted on the publish-npm job.

I cannot prove this end to end from a PR — publish-npm only runs on a push to main, so the release run is again the first real exercise.

Before merging

The six GHCR images already carry 0.3.1 from 73f8877. scripts/images.mjs:160 refuses to move an existing tag to a different digest, and .dockerignore does not exclude .github, so rebuilding after this commit may produce different digests and wedge promote. Delete the six 0.3.1 package versions first, so this merge can publish a coherent 0.3.1 to both registries.

Follow-up worth considering, not included here

docs/releasing.md tells maintainers to retry the failed jobs in the same run. That cannot work when the failure is in the workflow itself, since a re-run uses the workflow files from the run's own commit — true for both 0.3.1 attempts. The runbook deserves that caveat.

… publisher

The publish job names `environment: npm` and reads NPM_BOOTSTRAP_TOKEN from it,
but ci.yml calls that workflow without passing secrets. A called workflow does
not fail on an environment-scoped secret it was never passed — it resolves the
secret to an empty string. So the job authenticated with nothing and stopped at
`NPM_BOOTSTRAP_TOKEN is required for the first package publication`.

That is what split the first 0.3.1 run: six images promoted to GHCR, nothing on
npm, and no tag, because record-release needs both publishers.

`inherit` does not widen the blast radius here. The environment still gates the
token — it materializes only in the job that names the environment — and the
repository and organization have no Actions secrets for `inherit` to forward.

The test asserts both halves, since either alone silently reproduces the split.
Verified by removing each in turn: both make it fail.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7f6cf4a97b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/release.test.mjs
Comment on lines +201 to +204
assert.match(
publishNpm,
/\n secrets: inherit\b/,
"publish-npm must pass secrets to the reusable workflow, or NPM_BOOTSTRAP_TOKEN arrives empty",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Add integration coverage for inherited release secrets

This test only regex-matches the workflow text, so it passes without exercising the GitHub Actions behavior this change depends on: whether NPM_BOOTSTRAP_TOKEN actually materializes across the reusable-workflow boundary. Because this modifies a secrets-critical publication path that has already failed during a real release, add a deterministic integration test or local runner/fake that verifies the token reaches the protected bootstrap job on the successful path and remains unavailable on relevant denial paths; the repository explicitly requires both unit and integration coverage for secrets changes.

AGENTS.md reference: AGENTS.md:L5-L7

Useful? React with 👍 / 👎.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant