Conversation
VerdaccioRegistry.createTestDir() deleted the htpasswd file and forgot the users the file had generated. Verdaccio checks every authenticated request against that file, so a call from one test logged out every other test that was running at the same time. The registry already starts with a fresh htpasswd in start() and removes it in stop(), and no test file creates the same user twice, so the per-call reset is not needed.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 5 included reviews per hour; 0 remain after this review. WalkthroughThe test harness now clears tracked registry users during ChangesRegistry test harness reset
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Status: ready for review. This is the prerequisite for #40198, which is rebased on it. Verified with the debug build: |
There was a problem hiding this comment.
LGTM — test-harness-only change that stops createTestDir() from wiping the shared htpasswd mid-file.
What was reviewed:
- Verified every
generateUser/authBunfigcaller (bun-publish, bun-install-registry, npmrc, config-precedence) uses a unique username within its file, so the duplicate guard ingenerateUser()still cannot fire now thatthis.usersaccumulates. start()still deletes htpasswd and now also clearsthis.users, so per-file state remains fresh;stop()unchanged.bun-install-registry.test.ts's ownbeforeEachrm targetsimport.meta.dir/htpasswd(a different path), but that file only creates one user (whoami) so it is unaffected either way.
Extended reasoning...
Overview
This PR moves two lines inside VerdaccioRegistry in test/harness.ts: the htpasswd delete and this.users = {} reset move out of createTestDir() (called per-test) and the map reset is added to start() (called once per file). Net effect: authenticated users created earlier in a test file are no longer invalidated when a later test in the same file calls createTestDir(). No production code is touched.
Security risks
None. This is test infrastructure controlling a local verdaccio registry's user file. No auth, crypto, or runtime code paths are involved.
Level of scrutiny
Low. Three-line net change to a test helper, with clear rationale (unblocking concurrent auth-bearing verdaccio tests). The only correctness question is whether any test file relied on the per-test reset — i.e., whether any file creates the same username twice.
Other factors
I grepped every generateUser/authBunfig call site. All usernames are unique within each file (bun-publish.test.ts, bun-install-registry.test.ts, npmrc.test.ts, config-precedence.test.ts), so the if (this.users[username]) throw guard cannot newly fire. The pre-existing cross-file htpasswd sharing (which config-precedence.test.ts already works around with its own config dir) is untouched by this change — start()/stop() still delete the file. The PR description's claim about bun-install-registry.test.ts's own beforeEach deleting htpasswd is slightly off (it targets test/cli/install/htpasswd, not test/cli/install/registry/htpasswd), but that file only creates a single whoami user so the reset was never load-bearing there anyway. The author reports the affected test files pass; the change is strictly less destructive than before.
Problem
VerdaccioRegistry.createTestDir()(test/harness.ts) deleted the registry's htpasswd file and reset the harness's user map on every call. Verdaccio checks each authenticated request against that file, so acreateTestDir()call in one test logged out every other test that ran at the same time.createTestDir(). test(install): run bun-install-registry.test.ts concurrently and tighten its assertions #38504 works around it with a serialwhoamiblock.Fix
createTestDir()no longer touches htpasswd or the user map.start()still begins each file with a fresh htpasswd (and now an empty user map), andstop()still removes the file.generateUser()never fires. Users now accumulate in htpasswd for the life of one registry process, which is one test file.bun bd test test/cli/install/npmrc.test.ts test/cli/install/config-precedence.test.ts(91 pass) andbun bd test test/cli/install/bun-install-registry.test.ts(246 pass, 5 skip). These are the files that create verdaccio users. test: run bun-publish.test.ts concurrently and assert the exact requests and output #40198 is rebased on this change.Background
VerdaccioRegistryforks one verdaccio process per test file. Its users live intest/cli/install/registry/htpasswd.generateUser()creates a user through the registry's API and returns a token.authBunfig()wraps that token in abunfig.toml.no test proof · iteration 0 · no src or test change; test-proof not applicable