From b9f915eb858b9b0614d3c0c9309d869a7d708738 Mon Sep 17 00:00:00 2001 From: Krzysztof Macewicz Date: Sun, 27 Sep 2026 20:06:28 +0200 Subject: [PATCH] wip: the npm registry check outlasts the registry's five-minute cache; npm trust needs --allow-publish and real 2FA Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/workflows/release.yml | 13 ++++++++----- docs/publishing.md | 24 ++++++++++++++++++++++-- python/tests/test_release.py | 15 +++++++++++++++ 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 5080aef..d4683b9 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -228,16 +228,19 @@ jobs: DIST_TAG: ${{ needs.build-npm.outputs.dist_tag }} run: | set -euo pipefail - # Bounded: the registry's metadata can trail the publish by seconds, and a check that - # waits for ever is not a check. - for attempt in 1 2 3 4 5 6 7 8 9 10; do + # Bounded, and longer than the registry's cache. The package document is served with + # `cache-control: public, max-age=300`, and the build job has just fetched it, so for up + # to five minutes an edge can answer with the list from before this publish. Measured on + # the hand-made 0.1.0-dev.0: published at 17:57:51Z, visible at 18:02:58Z. Forty tries + # ten seconds apart outlast that, and a check that waits for ever is not a check. + for attempt in $(seq 1 40); do published=$(npm view "@smart-data-engines/sde@$VERSION" version 2>/dev/null || true) tagged=$(npm view "@smart-data-engines/sde" "dist-tags.$DIST_TAG" 2>/dev/null || true) if [ "$published" = "$VERSION" ] && [ "$tagged" = "$VERSION" ]; then echo "@smart-data-engines/sde@$published is on the registry as $DIST_TAG" exit 0 fi - sleep 6 + sleep 10 done - echo "::error::after 10 attempts the registry shows version '$published' and $DIST_TAG '$tagged', not $VERSION" + echo "::error::after 40 attempts the registry shows version '$published' and $DIST_TAG '$tagged', not $VERSION" exit 1 diff --git a/docs/publishing.md b/docs/publishing.md index 60091b3..703df9c 100644 --- a/docs/publishing.md +++ b/docs/publishing.md @@ -626,9 +626,26 @@ package, which is the shape of good news worth distrusting. ```bash npx npm@11.15.0 trust github @smart-data-engines/sde \ - --repo Smart-Data-Engines/smart-data-engine-sdk --file release.yml --env npm + --repo Smart-Data-Engines/smart-data-engine-sdk --file release.yml --env npm \ + --allow-publish --yes ``` + **Two things measured on 27 September.** + - **The command needs a permission flag.** Without `--allow-publish`, npm 11.15.0 stops with "At + least one permission flag is required (--allow-publish, --allow-stage-publish)". + - **It needs a session with real two-factor authentication.** A granular token that bypasses 2FA + can publish the bootstrap, and the registry refuses it for this: `E403` "Granular access + tokens that bypass two-factor authentication may not perform this action". A token of that kind + can do the publish; the trust configuration then belongs on the package page (Settings → + Trusted Publisher → GitHub Actions) or in an `npm login` session. The same run printed "npm + tokens that bypass 2FA are being restricted for account changes and direct publishing" + (), so for any later hand-made publish prefer + `npm login`. + + **A new package takes minutes to appear.** The package document is served with + `cache-control: public, max-age=300`. The bootstrap was published at 17:57:51Z and answered 404 + until 18:02:58Z. + **Do not put a token in an Actions secret to avoid this.** It would buy one attestation and leave behind a credential that publishes under our scope for as long as nobody remembers it is there. @@ -683,8 +700,11 @@ one needs the one before it. npm login npm publish --access public --tag latest npx npm@11.15.0 trust github @smart-data-engines/sde \ - --repo Smart-Data-Engines/smart-data-engine-sdk --file release.yml --env npm + --repo Smart-Data-Engines/smart-data-engine-sdk --file release.yml --env npm \ + --allow-publish --yes ``` + + `npm trust` needs the permission flag and an interactive session with 2FA; §5.3 says why. 3. **GitHub.** Require two-factor authentication for the organisation ([`github-security.md`](github-security.md)). It is off today. 4. **The two tags**, both on the bump commit: diff --git a/python/tests/test_release.py b/python/tests/test_release.py index 75c393c..a2047bf 100644 --- a/python/tests/test_release.py +++ b/python/tests/test_release.py @@ -471,3 +471,18 @@ def test_the_publish_job_takes_the_tag_the_build_job_chose_from_the_registry() - assert publish.count("DIST_TAG: ${{ needs.build-npm.outputs.dist_tag }}") == 2 assert 'npm publish "$tarball" --access public --tag "$DIST_TAG"' in publish assert '"dist-tags.$DIST_TAG"' in publish + + +def test_the_registry_check_waits_longer_than_the_registry_caches() -> None: + """The package document is cached for 300 seconds, and the build job fetched it just before. + + Measured on the hand-made `0.1.0-dev.0`: published at 17:57:51Z and visible at 18:02:58Z. A + check that gave up after a minute would have reported a successful publish as a failure. + """ + workflow = (ROOT / ".github" / "workflows" / "release.yml").read_text(encoding="utf-8") + publish = workflow.split(" publish-npm:", 1)[1] + loop = re.search(r"for attempt in \$\(seq 1 (\d+)\); do(.*?)done", publish, re.DOTALL) + assert loop is not None, "the registry check is no longer a bounded loop" + pause = re.search(r"sleep (\d+)", loop.group(2)) + assert pause is not None + assert int(loop.group(1)) * int(pause.group(1)) > 300 + 60