Skip to content

fix(core): rule on a publish whichever client ran it - #128

Open
Cedric921 wants to merge 1 commit into
Memnox:mainfrom
Cedric921:fix/rule-on-every-publish
Open

Cedric921 wants to merge 1 commit into
Memnox:mainfrom
Cedric921:fix/rule-on-every-publish

Conversation

@Cedric921

Copy link
Copy Markdown
Contributor

Closes #35.

What this changes

Measured on main, every publish but npm's own reached the registry as an ordinary shell line:

npm publish          ->  npm.publish      write
pnpm publish         ->  shell.execute    normal
yarn npm publish     ->  shell.execute    normal
bun publish          ->  shell.execute    normal
cargo publish        ->  shell.execute    normal
helm uninstall api   ->  shell.execute    normal
pulumi destroy       ->  shell.execute    normal
npm unpublish pkg    ->  npm.unpublish    destructive
pnpm unpublish pkg   ->  shell.execute    normal

So a rule naming npm.publish covered the one spelling most projects do not use. All four clients now reach the same action — the registry cannot tell them apart, so neither should a rule — while because still names the client that ran it, so a ledger row says pnpm publish rather than claiming npm did.

Classes in the new tables, named as the contributing guide asks

cargo — destructive: yank. Write: publish (tagged production), install, owner. Read: search.

helm — destructive: uninstall, delete. Write: install and upgrade (tagged production), rollback. Read: list, status, get.

pulumi — destructive: destroy. Write: up (tagged production), config set (tagged secrets). Read: preview, stack ls.

Package installs: bun, uv, pipx, gem and brew now classify as installing code that then runs here, which only npm, pnpm, yarn and pip did. uv pip install is handled, since it puts the verb one word further along than every other manager.

Nothing in an existing table moved.

The one judgement call

Only the publishing verbs route onto npm's table — publish, unpublish, dist-tag. Routing all of pnpm there was the obvious reading of "alias to npm", and it is wrong: pnpm add x is package.install today, and npm's table would have made it npm.add, a write. That is a real class moving under a command people run fifty times a day. Three tests pin pnpm add, pnpm install and yarn add as package installs so the next person widening this notices.

How it was verified

Twenty cases in resolve.test.ts, all of which fail on main: the three workalike spellings resolve to whatever npm publish resolves to (compared against it rather than hard-coded, so the two cannot drift), every cargo, helm and pulumi verb above, the five new package managers, and the three no-regression rows.

Test Files  286 passed (286)
     Tests  8486 passed (8486)

pnpm format && pnpm typecheck && pnpm test && pnpm deadcode all clean, plus pnpm build, on Node 24.

npm-workalike.ts is its own module because resolve.ts went 11 lines over the five-hundred-line cap with it inline, and house-style.test.ts said so.

Checklist

  • pnpm format && pnpm typecheck && pnpm test && pnpm deadcode all pass
  • Behaviour change ships with a test
  • No any, no magic values, no console.* outside cli-output.ts
  • If this touches the decision path: still deterministic — tables and an argv rewrite, no model, network or randomness
  • If this changes a verb table: the classes are named above; three tables are added and none already there is touched
  • If this changes a command, flag or file it writes: n/a; scan now lists three more CLIs, which the changeset names

@Cedric921

Copy link
Copy Markdown
Contributor Author

The red Dependency audit here is not this change — all twelve test legs pass, and the audit fails the same way on a clean main.

It is GHSA-vfj7-8cjw-p6xm on braces, which the advisory says is patched in >=3.0.4. That version was never published: pnpm view braces dist-tags answers { latest: '3.0.3' }, and 3.0.3 is the vulnerable one. So there is nothing to override to, and pnpm audit --audit-level high cannot pass on any branch right now. braces is a devDependency only and reaches nothing the four packages publish.

I tried bumping the dependency that pulls it before reporting. @changesets/cli@3 does drop the chain, but knip reaches the same micromatch@4.0.8 on its own, so the advisory stays:

before:  braces <- micromatch <- fast-glob <- globby <- @manypkg/get-packages <- @changesets/cli
after:   braces <- micromatch <- fast-glob <- knip

I reverted that rather than push a major bump of the tool pnpm ship runs for a fix that is not one.

Written up with the evidence in #129, where the remaining choices are policy ones — an ignore entry, a qualified gate, or merging past it — which seemed yours to make rather than mine to guess at. This PR needs nothing.

This branch has not been deployed

No deployments
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.

intercept: publishing through pnpm, yarn, bun or cargo, and helm or pulumi destroy, bypass every rule

1 participant