CI and OIDC publish workflow for @urbankitstudio/atlas - #2
Merged
Merged
Conversation
Ported from urbankitstudio/mcp-atlas's pair, which is not a design on paper --
it published 0.2.4 and 0.2.5 over OIDC with provenance, the second under a
GitHub Environment restricted to main. The MCP-specific steps (server.json
agreement, registry schema caps, stdio smoke, bin-in-tarball) are dropped and
replaced with the checks that matter for an SDK whose data IS the product.
Lands separately from the 0.6.3 -> 0.6.5 content sync on purpose. `publish.yml`
fires on a version CHANGE, and that sync moved the version to one npm already
serves; together they would have triggered a publish run that could only fail.
This commit leaves the version untouched, so the workflow's own "did this push
change the version?" gate answers no and every publish step skips.
**ci.yml** gates install, typecheck, test and build, then asserts two things
npm does not:
- every entry point `package.json` promises (`main`, `module`, `types`)
exists. A build that silently emits nothing publishes an installable
package resolving to no code, which surfaces only in a consumer's project.
- the TARBALL carries `data/index.json` with a plausible county count, and
the README and npm description state the number the bundled index actually
reports. Asserted against the tarball rather than the working tree, because
`files` and .npmignore decide what ships. The count is a FLOOR, not an
exact match: this repo is downstream of a registry that grows, so pinning
the number would fail on every real update. The floor exists to catch an
empty or truncated sync, which is the failure that would otherwise publish
quietly. The paired copy check is the one the monorepo already runs against
`packages/atlas` -- both surfaces stated 155 counties for two releases
after the atlas reached 170.
**publish.yml** keeps the parts of the mcp-atlas workflow that were each
written after something went wrong, and the comments say which:
- `environment: npm-publish`, because GitHub mints the `environment` OIDC
claim only when the JOB declares one. Setting the field on npm before the
workflow declares it breaks publishing in between.
- the version gate reads `github.event.before`, not `HEAD~1`, so a push
landing several commits cannot skip a release in silence.
- `dry_run` defaults to TRUE and anything that is not the exact string
"false" is a rehearsal, so a renamed or removed input costs a wasted run
rather than an unintended release.
- the publish step is idempotent, and gated on `github.ref == refs/heads/main`
so a dispatch on a feature branch cannot ship unmerged work.
- a 15-minute timeout, because a hung job holds a mintable npm publish
identity for GitHub's default six hours.
- values reach the `always()` report step through `env:`, never through
`${{ }}` inside `run:`.
Both run on ubuntu-latest. Neither targets the self-hosted runner, and
`pull_request` must never be pointed at it.
🔴 NOT LIVE YET, and the order matters. npm's Trusted Publisher entry for
@urbankitstudio/atlas still names urbankitstudio/urbankitstudio +
publish-atlas-package.yml. Until it is repointed at this repo + publish.yml,
this workflow cannot publish -- and after the repoint, that one cannot. Exactly
one of the two may be live. The monorepo's workflow stays until the repoint is
proven by a real release.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmquGJ5cp392nHnMum61Wi
A codex-runner pass over the first version found four HIGH issues. Three are
fixed here; the fourth is mitigated at its mechanism and stated in the file
rather than papered over. Every factual claim below was re-derived by running
it, not taken on the reviewer's word.
**The strong check could not block a publish, and the weak one could.** ci.yml
and publish.yml both trigger on a push to main with no `needs:` between them, so
ci.yml going red has no power to stop a release. ci.yml checked the county
floor AND the file count AND the README copy; publish.yml checked the floor
alone. The only gate that could actually stop a publish was the weaker one. Both
now call `scripts/verify-package.mjs`. One implementation, two callers, so they
cannot drift again.
**The floor failed OPEN on malformed metadata.** Verified by running it:
`node -p "require('./data/index.json').totals.counties"` on an index whose
`totals` exists without `counties` prints the string `undefined` and exits 0 --
it does not throw. `[ "undefined" -lt 100 ]` then errors with "integer
expected", and inside `if A || B` that error is simply a false clause. The check
passed. publish.yml had no second clause at all, so a schema drift that dropped
one field defeated its only data gate outright. Everything is asserted in JS
now, with `Number.isInteger` before any comparison.
**Neither file cross-checked the data against itself.** A floor only reads what
the index SAYS about itself, so a stale-but-large `totals.counties` beside a
truncated set of per-state files cleared both. Three numbers must now agree: the
declared total, the sum of the per-state counts the index lists, and the
counties actually present in the packaged files.
**`id-token: write` is a JOB permission, so the ref check was in the wrong
place.** The token-minting environment variables are live for every step, and
`npm ci` and `npm pack` run package lifecycle scripts. Only the final Publish
step checked `github.ref`, so a dispatch on any branch -- even at the default
`dry_run: true` -- executed those scripts with a mintable publish identity in
scope. The ref check moved into the `release` gate that every heavy step is
conditioned on. The cost is that a feature branch can no longer be rehearsed
here; that is the intended trade, and ci.yml already gates feature branches.
**The semver check could be defeated by a newline, forging a step output.**
`grep -Eq '^...$'` anchors per line and short-circuits on the first match.
Verified: `NEW=$'1.2.3\nrelease=true'` satisfies it, and
`echo "version=$NEW" >> $GITHUB_OUTPUT` then writes a second line that forges
`release=true`. The file's own comment claimed the check prevented exactly that.
It is now a JS regex -- no /m flag, so it anchors the whole string -- the value
is passed as argv rather than through the shell, and it runs BEFORE anything is
written to `$GITHUB_OUTPUT`.
**Neither file set a shell, so neither had pipefail.** Confirmed against
GitHub's workflow-syntax reference: the default for `run:` on Linux is
`bash -e {0}`; naming `shell: bash` gives `bash --noprofile --norc -eo pipefail
{0}`. Without it `TARBALL=$(npm pack --silent | tail -1)` swallows a pack
failure and hands the next step an empty filename. Both files now set
`defaults.run.shell: bash`.
**The fourth HIGH is mitigated, not fixed, and the file says so.** `npm pack`
and `npm publish` build SEPARATE archives and `prepublishOnly` runs only on
publish, so the bytes verified are not literally the bytes that ship. Publishing
the packed tarball would close it -- but npm's own docs do not state whether
provenance survives a pre-packed tarball publish, and provenance is the entire
reason this package publishes from a public repo. Trading it away to fix a
lower-tier risk that needs monorepo-write access is a bad bargain. So
verify-package.mjs pins the publish lifecycle scripts to an allow-list, which
blocks the mechanism instead of the symptom.
**Proven to fire, not read over.** `fixture-test-verifier.mjs` runs the verifier
against nine disposable copies -- including a control that must pass -- and each
failing case names in advance the exact error it should produce, so a red for
the wrong reason scores as a miss. All nine behaved as named: missing
`counties`, a string instead of a number, a truncated state file, a state file
absent from the package, README stating an unsupported count, a rogue
`prepublishOnly`, an absolute entry-point path, and a missing build output.
**One finding the review cleared, worth recording:** no path was found for a
fork PR to obtain the OIDC identity, secrets or a write token. ci.yml holds
`contents: read` and no `pull_request_target`; publish.yml has no PR trigger;
and a fork's own run cannot satisfy npm's exact-repo match.
The review also confirmed by API that the `npm-publish` environment does NOT yet
exist on this repo and that main is unprotected. GitHub creates an environment
on first reference with no rules, so a merely-referenced environment is not a
control -- the header now says that, and the ref check above no longer depends
on it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmquGJ5cp392nHnMum61Wi
LEOyrh
added a commit
to urbankitstudio/mcp-atlas
that referenced
this pull request
Sep 22, 2026
…t ships Porting the findings of the adversarial review of urbankitstudio/atlas#2 to this repo, which carried the same publish workflow structure. Looking for them turned up a larger problem that was nobody's finding. **THE REAL DRIFT: CI tested 171 counties while the package advertised 227.** package.json, README.md and server.json all claim 227 counties / 241 layers. package-lock.json pinned @urbankitstudio/atlas 0.6.2, which holds 171 counties and 174 endpoints -- measured, not inferred, by reading both published tarballs. So every typecheck, build and stdio smoke run in this repo exercised a dataset a third smaller than the one users get. Users were never affected, and it is worth being precise about why: no lockfile ships in the tarball (`files` lists dist, README, LICENSE, SECURITY-NOTES, and npm excludes it anyway -- verified by unpacking 0.2.5 from the registry), so `^0.6.2` resolves to 0.6.5 on install. The claim was true for them and false for our own CI. README.md already said "(atlas 0.6.5)", so the manifest was the half that was stale. The range is now `^0.6.5`, which is the version that actually backs the advertised coverage. A range whose floor cannot support the README is a claim nothing enforces. The suite passes unchanged against the larger data: 20 smoke assertions before and after, including the two that matter most -- that Oakland County MI is still REFUSED rather than handed a dud query, and the SQL-quote escaping in build_owner_query. **server.json's description was wrong, not just unchecked.** It read "227 verified US county parcel ArcGIS endpoints". 227 is the COUNTY count; the endpoint count is 241. Now "227 verified US counties (241 ArcGIS layers)", which is accurate and fits the registry's 100-character cap at 92. **The strong checks could not block a publish, and the weak ones could.** ci.yml and publish.yml both trigger on a push to main with no `needs:` between them, so ci.yml going red cannot stop a release, and the two files carried diverging copies of the package checks. Both now call `scripts/verify-package.mjs` -- one implementation, two callers, run against the EXTRACTED TARBALL rather than the working tree, because `files` decides what ships. It checks entry points (present, relative, inside the package), publish-time lifecycle scripts against an allow-list, `mcpName` read from the PACKAGED package.json rather than the source tree since the registry reads what shipped, and the coverage parity above across all three claim sites. **The anti-vacuity half is the important half.** Each claim site must yield at least one readable count. A regex that matches nothing passes every comparison it never makes, so a claim reworded past the pattern fails loudly instead of quietly retiring the check. **Neither file set a shell, so neither had pipefail.** GitHub's default for `run:` on Linux is `bash -e {0}`; naming `shell: bash` gives `bash --noprofile --norc -eo pipefail {0}`. Without it `TARBALL=$(npm pack --silent | tail -1)` swallows a pack failure and hands the next step an empty filename. Both files now set `defaults.run.shell: bash`. **The semver check was forgeable, and misplaced besides.** `grep -Eq '^...$'` anchors PER LINE and short-circuits, so `NEW=$'1.2.3\nrelease=true'` satisfied it and the following `echo "version=$NEW" >> "$GITHUB_OUTPUT"` wrote a second line forging a step output. Worse, it ran AFTER `changed=` and `release=` were already written, so even a working check there was too late to protect them. It is now a JS regex (no /m, so it anchors the whole string), the value passes as argv rather than through a shell, and it runs before anything reaches $GITHUB_OUTPUT. **The ref check moved into the release gate -- as DEPTH here, not a fix.** `id-token: write` is a job permission, so the token-minting variables are live for every step and `npm ci` runs lifecycle scripts; only the final Publish step checked `github.ref`. On atlas that was a live hole. Here it is not, and I measured rather than assumed it: dispatching this workflow on this branch failed the job in 3s with ZERO steps executed -- Branch "claude/publish-hardening" is not allowed to deploy to npm-publish due to environment protection rules. GitHub's documentation describes deployment branch policies without saying whether the job still runs, which is why it was worth testing. The environment restricts to `main` and is a genuine job-level gate. But it is a repository setting an admin can change with no review and no diff, so the guarantee is restated in the file where it is reviewable. Nothing observable changes while the environment stands; this is what holds if it falls. **Proven to fire, not read over.** `fixture-test-mcp-verifier.mjs` runs the verifier against thirteen disposable copies, each naming in advance the exact error it must produce, so a red for the wrong reason scores as a miss. The repo side is synthesised rather than copied so the atlas totals can be mutated without duplicating node_modules. All thirteen behaved as named: the control that must pass, a README overclaiming counties, a description overclaiming layers, a server.json reworded past the pattern (the anti-vacuity case), missing and string-typed totals, a truncated atlas with every claim lowered to match so only the floor could fire, a rogue postinstall, an altered prepublishOnly, an absolute bin path, a missing bin target, a mismatched mcpName, and a README naming a different atlas version than the range floor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AmquGJ5cp392nHnMum61Wi
LEOyrh
added a commit
to urbankitstudio/mcp-atlas
that referenced
this pull request
Sep 22, 2026
…t ships (#9) Porting the findings of the adversarial review of urbankitstudio/atlas#2 to this repo, which carried the same publish workflow structure. Looking for them turned up a larger problem that was nobody's finding. **THE REAL DRIFT: CI tested 171 counties while the package advertised 227.** package.json, README.md and server.json all claim 227 counties / 241 layers. package-lock.json pinned @urbankitstudio/atlas 0.6.2, which holds 171 counties and 174 endpoints -- measured, not inferred, by reading both published tarballs. So every typecheck, build and stdio smoke run in this repo exercised a dataset a third smaller than the one users get. Users were never affected, and it is worth being precise about why: no lockfile ships in the tarball (`files` lists dist, README, LICENSE, SECURITY-NOTES, and npm excludes it anyway -- verified by unpacking 0.2.5 from the registry), so `^0.6.2` resolves to 0.6.5 on install. The claim was true for them and false for our own CI. README.md already said "(atlas 0.6.5)", so the manifest was the half that was stale. The range is now `^0.6.5`, which is the version that actually backs the advertised coverage. A range whose floor cannot support the README is a claim nothing enforces. The suite passes unchanged against the larger data: 20 smoke assertions before and after, including the two that matter most -- that Oakland County MI is still REFUSED rather than handed a dud query, and the SQL-quote escaping in build_owner_query. **server.json's description was wrong, not just unchecked.** It read "227 verified US county parcel ArcGIS endpoints". 227 is the COUNTY count; the endpoint count is 241. Now "227 verified US counties (241 ArcGIS layers)", which is accurate and fits the registry's 100-character cap at 92. **The strong checks could not block a publish, and the weak ones could.** ci.yml and publish.yml both trigger on a push to main with no `needs:` between them, so ci.yml going red cannot stop a release, and the two files carried diverging copies of the package checks. Both now call `scripts/verify-package.mjs` -- one implementation, two callers, run against the EXTRACTED TARBALL rather than the working tree, because `files` decides what ships. It checks entry points (present, relative, inside the package), publish-time lifecycle scripts against an allow-list, `mcpName` read from the PACKAGED package.json rather than the source tree since the registry reads what shipped, and the coverage parity above across all three claim sites. **The anti-vacuity half is the important half.** Each claim site must yield at least one readable count. A regex that matches nothing passes every comparison it never makes, so a claim reworded past the pattern fails loudly instead of quietly retiring the check. **Neither file set a shell, so neither had pipefail.** GitHub's default for `run:` on Linux is `bash -e {0}`; naming `shell: bash` gives `bash --noprofile --norc -eo pipefail {0}`. Without it `TARBALL=$(npm pack --silent | tail -1)` swallows a pack failure and hands the next step an empty filename. Both files now set `defaults.run.shell: bash`. **The semver check was forgeable, and misplaced besides.** `grep -Eq '^...$'` anchors PER LINE and short-circuits, so `NEW=$'1.2.3\nrelease=true'` satisfied it and the following `echo "version=$NEW" >> "$GITHUB_OUTPUT"` wrote a second line forging a step output. Worse, it ran AFTER `changed=` and `release=` were already written, so even a working check there was too late to protect them. It is now a JS regex (no /m, so it anchors the whole string), the value passes as argv rather than through a shell, and it runs before anything reaches $GITHUB_OUTPUT. **The ref check moved into the release gate -- as DEPTH here, not a fix.** `id-token: write` is a job permission, so the token-minting variables are live for every step and `npm ci` runs lifecycle scripts; only the final Publish step checked `github.ref`. On atlas that was a live hole. Here it is not, and I measured rather than assumed it: dispatching this workflow on this branch failed the job in 3s with ZERO steps executed -- Branch "claude/publish-hardening" is not allowed to deploy to npm-publish due to environment protection rules. GitHub's documentation describes deployment branch policies without saying whether the job still runs, which is why it was worth testing. The environment restricts to `main` and is a genuine job-level gate. But it is a repository setting an admin can change with no review and no diff, so the guarantee is restated in the file where it is reviewable. Nothing observable changes while the environment stands; this is what holds if it falls. **Proven to fire, not read over.** `fixture-test-mcp-verifier.mjs` runs the verifier against thirteen disposable copies, each naming in advance the exact error it must produce, so a red for the wrong reason scores as a miss. The repo side is synthesised rather than copied so the atlas totals can be mutated without duplicating node_modules. All thirteen behaved as named: the control that must pass, a README overclaiming counties, a description overclaiming layers, a server.json reworded past the pattern (the anti-vacuity case), missing and string-typed totals, a truncated atlas with every claim lowered to match so only the floor could fire, a rogue postinstall, an altered prepublishOnly, an absolute bin path, a missing bin target, a mismatched mcpName, and a README naming a different atlas version than the range floor. Claude-Session: https://claude.ai/code/session_01AmquGJ5cp392nHnMum61Wi Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
Ported from
urbankitstudio/mcp-atlas's pair — which is not a design on paper: it published 0.2.4 and 0.2.5 over OIDC with provenance, the second under a GitHub Environment restricted tomain. The MCP-specific steps (server.json agreement, registry schema caps, stdio smoke, bin-in-tarball) are dropped and replaced with checks that matter for an SDK whose data is the product.Lands separately from the 0.6.3 → 0.6.5 content sync (#1) on purpose:
publish.ymlfires on a version change, and that sync moved the version to one npm already serves. This PR leaves the version untouched, so the workflow's own gate answers "no" and every publish step skips. That is what a green run here means.ci.yml— two things npm does not checkpackage.jsonpromises exists. A build that silently emits nothing publishes an installable package resolving to no code, which surfaces only in a consumer's project.filesand.npmignoredecide what ships. The county count is a floor, not an exact match — this repo is downstream of a registry that grows, so pinning it would fail on every real update. The floor catches an empty or truncated sync. The paired README/description check is the one the monorepo already runs: both surfaces stated 155 counties for two releases after the atlas reached 170.publish.yml— the parts that were each written after something went wrongenvironment: npm-publishenvironmentOIDC claim only when the job declares one. Setting the field on npm first breaks publishing in between — this is how mcp-atlas 0.2.5 came to be released.github.event.beforeHEAD~1woulddry_rundefaults true; only the exact string"false"publishesgithub.ref == refs/heads/mainalways()report takes values viaenv:, never${{ }}inrun:Both run on
ubuntu-latest. Neither targets the self-hosted runner, andpull_requestmust never be pointed at it.🔴 Not live yet, and the order matters
npm's Trusted Publisher entry for
@urbankitstudio/atlasstill namesurbankitstudio/urbankitstudio+publish-atlas-package.yml. Until it is repointed at this repo +publish.yml, this workflow cannot publish — and after the repoint, that one cannot. Exactly one of the two may be live. The monorepo's workflow stays until the repoint is proven by a real release.🤖 Generated with Claude Code
https://claude.ai/code/session_01AmquGJ5cp392nHnMum61Wi