Repository navigation
publish: --tolerate-republish exits 0 on the upload's 403 "You cannot publish over the previously published versions" - #45503
Conversation
… publish over the previously published versions" With the flag, `bun publish` read only the manifest that it fetched before the upload. When that manifest did not list the version (it was stale, or a second job published at the same time), the upload went out, and the registry's refusal was reported as any other failure. Both refused uploads (the first PUT and the PUT that carries the one-time password) now return through one function, `upload_refused`. With the flag set and a status of 403, it reads the `error` string of the JSON body. If that string starts with npm's sentence, it prints the existing warning and returns `false`, as the check before the upload does. Every other answer goes to `Npm::response_error` as before, including every 409.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 22 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Updated 8:06 AM PT - Oct 16th, 2026
✅ @robobun, your commit 7376c3a64e2e1a14c8b9e3e25b75d72a29c94bfd passed in 🧪 To try this PR locally: bunx bun-pr 45503That installs a local version of the PR into your bun-45503 --bun |
|
Status Reproduced on main (5866b5f) with a loopback registry:
The tests in this PR send the same requests. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline findings, I also checked the refactor of the two Npm::response_error call sites into upload_refused — since response_error returns Infallible, the non-OTP path still exits before the OTP prompt and the OTP-retry path still exits the same way, so notice printing and exit behavior are unchanged when the flag is off or the body does not match. A 403 for a different version than the one being published was also looked at: the warning names ctx.package_version, and the registry only answers for the PUT's own version, so that is not reachable.
Extended reasoning...
The change touches only src/runtime/cli/publish_command.rs (a new 403-body match behind --tolerate-republish, replacing two direct response_error calls), its docs, and bun-publish.test.ts; it reads a registry's error body but adds no auth, crypto or path-handling surface. The two confirmed inline findings (replayed PUT and unpublished-version 403 both mapped to the "already knows" skip) are the reason a human should weigh the contract, so this is a findings-present ruled-out note rather than an approval.
Findings marked 🟡 are optional suggestions and need no follow-up push.
| if manager.options.publish_config.tolerate_republish | ||
| && res.status_code() == 403 | ||
| && Self::says_version_taken(response_buf.list.as_slice())? | ||
| { | ||
| Self::warn_already_published(dependency::without_build_tag(&ctx.package_version)); | ||
| return Ok(false); |
There was a problem hiding this comment.
🔴 CI jobs that publish a version number that was unpublished (or whose package was deleted) now exit 0 with "Registry already knows about version X; skipping." although nothing is installable; the base exits 1 with the 403. The 403 sentence matched at src/runtime/cli/publish_command.rs:758-763 is not specific to listed versions: the repo's own registry model sends the same 403 for unpublished.has(key) and removed?.versions.has(key) (test/packages/registry/src/packages.ts:322-326), versions no manifest lists. Fix: exit 0 on this 403 only after confirming the registry serves the version (re-read the manifest or GET name/version and require it listed), else keep the error, and drop the docs claim that the 403 only comes for a version the registry lists.
Why this was flagged
A user runs bun publish --tolerate-republish for pkg@ 1.0.0 after that version was unpublished, or after the whole package was deleted, and reuses the number. check_package_version_exists at src/runtime/cli/publish_command.rs:782 finds the version absent (or gets a 404), so the PUT goes out. The registry answers 403 with error "You cannot publish over the previously published versions: 1.0.0."; the repo's npm model does exactly this for existing?.unpublished.has(key) || removed?.versions.has(key) at test/packages/registry/src/packages.ts:322-326. upload_refused at src/runtime/cli/publish_command.rs:758-763 sees flag + 403 + the prefix, prints the warning and returns Ok(false), so the command exits 0 and a -r run continues. On the base branch Npm::response_error prints the 403 and exits 1, so the operator learns the number is burned and bumps it. The docs at docs/pm/cli/publish.mdx:150 say the 403 is sent for a version the registry lists, and the only caveat given is the scan window, not a permanently burned version.
Verification: upload_refused at src/runtime/cli/publish_command.rs:758-763 returns Ok(false) whenever tolerate_republish && status == 403 && says_version_taken(body); nothing distinguishes a listed version from an unpublished one. test/packages/registry/src/packages.ts:321-327 throws the same 403 sentence for existing?.unpublished.has(key) || removed?.versions.has(key). On the base commit the sequence exits 1 with the 403.
There was a problem hiding this comment.
On registry.npmjs.org this case does not reach the new branch. npm refuses a version number that was unpublished with a different answer: 400 Bad Request and Cannot publish over previously published version "1.0.0". (npm/cli#3076, where a member of npm calls it intentional). That answer stays an error with the flag, and the row "400 for a version number that was unpublished" in bun-publish.test.ts pins it. A package that was removed less than 24 hours ago gets the 403 "cannot be republished until 24 hours have passed". That also stays an error and also has a row.
The 403 for unpublished.has(key) and removed?.versions.has(key) is the choice of the test registry. The commit that added it lists the three refusal texts as "the texts of npm as I recall them. I did not probe them" (#44055). Against a registry that does send npm's sentence for a version that it does not list, the flag now skips the version. The PR notes say so.
I did not add the confirming read. A second read of the same manifest is the read that was stale in the first place: npmjs serves it from its cache with cache-control: public, max-age=300, also to a request that carries a token. So that read turns the stale case back into exit 1. A read of ?write=true, which npmjs serves from the origin, does not have that problem. It gives the same result on npmjs and costs one request on the skip path. lerna, Nx, changesets and packages/bun-release/scripts/upload-npm.ts accept this sentence without a second read. The stricter rule is decision 4 in the PR notes, for a maintainer to choose.
Problem
bun publish --tolerate-republishexits 1 with403 Forbiddenand- You cannot publish over the previously published versions: 1.0.0.. The docs promise exit 0 "if the package version already exists".src/runtime/cli/publish_command.rs:631). A stale manifest or a second job lets the upload go out, andNpm::response_errorexits.Fix
upload_refused. With the flag and status 403, it reads the JSON body'serrorstring. If that starts with npm's sentence, it prints the existing warning and returnsfalse.Npm::response_erroras before, including every 409.test/cli/install/bun-publish.test.ts(7 new tests fail on main), and the proxy, registry CLI and npmrc suites.Background
packages/bun-release/scripts/upload-npm.tsaccept the same sentence.Npm::response_errorto parse the body once: four commands share that function.Downsides
.text, +4,096 B file size. An accepted publish runs no new code.Notes
The rule
--tolerate-republish, status 403, and a JSONerrorstring that starts withYou cannot publish over the previously published versions. The match is on the parsederror, not on the raw body. Nothing else is accepted: not another status with the sentence, not the sentence in another field, after other words, after a space or in lowercase, and not a sentence that is one letter shorter. With the constant cut toYou cannot publish, the row "a sentence that is one letter shorter" fails.30f3870 gave this reason for the manifest read: "More reliable: direct registry check instead of parsing error messages". The review comment before it asked about one of the eight substrings,
You cannot publish: "this one seems like it could be used in other situations". One status with one full sentence at the start oferroris the kind of rule thatNpm::response_erroralready has for the rejected one-time password.Decisions for a maintainer
-rand--filterno longer say that a second run "skips what was published", because that skip reads the manifest only. Without the flag these runs are unchanged: a refused upload ends the run. Native workspace release management:bun pm change,bun pm version --recursive,bun publish --recursive#41290 is the open issue next to this.?write=true, which npmjs serves from the origin and not from the cache. On npmjs the result is the same, because npm refuses an unpublished version number with a 400. The read only guards a registry that sends npm's sentence for a version that it does not list.Not changed on purpose
src/install/npm.rs. A split ofresponse_errorinto a parse step and a report step would remove the second parse. It touchesbun pm whoami,bun pm view,bun pm diffand the web-login poll, so it is a change for its own PR.Cannot publish over previously published version "X".(Once package@1.0.0 was published, then unpublish, it cannot be published again with the same version npm/cli#3076). That stays an error, and a test names it. The in-tree test registry sends the 403 sentence for that case (test/packages/registry/src/packages.ts), and the commit that added it says that it did not probe npm's texts (test: replace verdaccio with an npm registry on Bun.serve #44055). Against a registry that does that, the flag now skips the version.cache-control: public, max-age=300. A fresher read could shrink the stale case for the flag and for-rwithout it.publish(). A typed result instead ofboolrewrites every exit ofpublish()to change two.Known limit
The HTTP client sends a PUT again when a pooled connection closes before the response. If the registry stored the first upload, the job gets the 403 for its own upload. On main that job exits 1. With this PR and the flag it exits 0 with the warning, and it does not print
+ name@versionor runpublishandpostpublish. The replay is a separate bug.Measurements (main 5866b5f against this branch)
ParsedJson::parse_jsonandjson::parse_utf8, debug builds)exit(1): 7, 14 and 13 on both (no flag + the sentence, no flag + another 403, flag + a 409). With the flag, another 403 goes from 14 to 29, and a 403 that is not JSON from 12 to 24. (gdb, main thread,__rust_alloc+__rust_alloc_zeroed+__rust_realloc)upload_refusedon an accepted upload (with and without the flag),--dry-run, and the skip before the upload: 0 in all 4 rows.-rwith the flag). After that point-rwith the flag goes on to the next packages..text81,071,045 B to 81,072,581 B, stripped file 106,501,896 B to 106,505,992 B, which is one page (llvm-size -A,stat). The refusal code (npm::response_error,publish,upload_refused,says_version_taken): 9,359 B to 10,905 B (llvm-nm -S). bloaty, perf and valgrind are not installed here.Tests
-rwith the flag.no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/cli/install/bun-publish.test.ts