Skip to content

Commit ef6cfea

Browse files
martinrrmCopilot
andauthored
fix(owner): use scoped registry for user lookup (#9786)
## Summary Ensure `npm owner add` and `npm owner rm` resolve users from the same registry used for the target package. The user lookup now receives the package `spec`, allowing `npm-registry-fetch` to honor scoped registry configuration instead of falling back to the global registry. ## Testing Added regression coverage for split global/scoped registry configurations, including: - Preventing substitution of the user added as an owner. - Preventing removal of an unintended existing owner. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1e1231bc-474e-4a27-8b78-242366f48e19
1 parent 9e17dc0 commit ef6cfea

2 files changed

Lines changed: 107 additions & 1 deletion

File tree

lib/commands/owner.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,10 @@ class Owner extends BaseCommand {
160160
let u
161161

162162
try {
163-
u = await npmFetch.json(uri, this.npm.flatOptions)
163+
u = await npmFetch.json(uri, {
164+
...this.npm.flatOptions,
165+
spec,
166+
})
164167
} catch (err) {
165168
log.error('owner mutate', `Error getting user data for ${user}`)
166169
throw err

test/lib/commands/owner.js

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,58 @@ t.test('owner add <user> <pkg>', async t => {
189189
t.equal(joinedOutput(), `+ ${username} (${packageName})`)
190190
})
191191

192+
t.test('owner add resolves user from package scoped registry', async t => {
193+
const scopedRegistryUrl = 'https://scoped.registry.npmjs.org'
194+
const scopedAuth = '//scoped.registry.npmjs.org/:_authToken'
195+
const { npm, joinedOutput } = await loadMockNpm(t, {
196+
config: {
197+
...auth,
198+
'@npmcli:registry': scopedRegistryUrl,
199+
[scopedAuth]: 'scoped-auth-token',
200+
},
201+
})
202+
const username = 'requested-user'
203+
const globalRegistry = new MockRegistry({
204+
tap: t,
205+
registry: npm.config.get('registry'),
206+
authorization: 'test-auth-token',
207+
})
208+
const scopedRegistry = new MockRegistry({
209+
tap: t,
210+
registry: scopedRegistryUrl,
211+
authorization: 'scoped-auth-token',
212+
})
213+
214+
// If `owner add` mistakenly asked the global registry for user data
215+
// instead of the package's scoped registry, this substituted user would
216+
// be added as the owner instead of the one requested.
217+
globalRegistry.nock = globalRegistry.nock
218+
.get(`/-/user/org.couchdb.user:${encodeURIComponent(username)}`)
219+
.optionally()
220+
.reply(200, { name: 'substituted-user', email: 'substituted@example.com' })
221+
scopedRegistry.couchuser({ username, body: { name: username, email: 'requested@example.com' } })
222+
223+
const manifest = scopedRegistry.manifest({
224+
name: packageName,
225+
packuments: [{ maintainers, version: '1.0.0' }],
226+
})
227+
await scopedRegistry.package({ manifest })
228+
scopedRegistry.nock.put(`/${spec.escapedName}/-rev/${manifest._rev}`, body => {
229+
t.match(body, {
230+
_id: manifest._id,
231+
_rev: manifest._rev,
232+
maintainers: [
233+
...manifest.maintainers,
234+
{ name: username, email: 'requested@example.com' },
235+
],
236+
})
237+
return true
238+
}).reply(200, {})
239+
240+
await npm.exec('owner', ['add', username, packageName])
241+
t.equal(joinedOutput(), `+ ${username} (${packageName})`)
242+
})
243+
192244
t.test('owner add <user> cwd package', async t => {
193245
const { npm, joinedOutput } = await loadMockNpm(t, {
194246
prefixDir: {
@@ -364,6 +416,57 @@ t.test('owner rm <user> <pkg>', async t => {
364416
t.equal(joinedOutput(), `- ${username} (${packageName})`)
365417
})
366418

419+
t.test('owner rm resolves user from package scoped registry', async t => {
420+
const scopedRegistryUrl = 'https://scoped.registry.npmjs.org'
421+
const scopedAuth = '//scoped.registry.npmjs.org/:_authToken'
422+
const { npm, joinedOutput, logs } = await loadMockNpm(t, {
423+
config: {
424+
...auth,
425+
'@npmcli:registry': scopedRegistryUrl,
426+
[scopedAuth]: 'scoped-auth-token',
427+
},
428+
})
429+
const username = 'requested-user'
430+
const globalRegistry = new MockRegistry({
431+
tap: t,
432+
registry: npm.config.get('registry'),
433+
authorization: 'test-auth-token',
434+
})
435+
const scopedRegistry = new MockRegistry({
436+
tap: t,
437+
registry: scopedRegistryUrl,
438+
authorization: 'scoped-auth-token',
439+
})
440+
441+
// If `owner rm` mistakenly asked the global registry for user data instead
442+
// of the package's scoped registry, this substituted user would be
443+
// resolved and treated as an existing owner rather than the one requested.
444+
globalRegistry.nock = globalRegistry.nock
445+
.get(`/-/user/org.couchdb.user:${encodeURIComponent(username)}`)
446+
.optionally()
447+
.reply(200, maintainers[0])
448+
scopedRegistry.couchuser({ username, body: { name: username, email: 'requested@example.com' } })
449+
450+
const manifest = scopedRegistry.manifest({
451+
name: packageName,
452+
packuments: [{ maintainers, version: '1.0.0' }],
453+
})
454+
await scopedRegistry.package({ manifest })
455+
let update
456+
scopedRegistry.nock
457+
.put(`/${spec.escapedName}/-rev/${manifest._rev}`, body => {
458+
update = body
459+
return true
460+
})
461+
.optionally()
462+
.reply(200, {})
463+
464+
await npm.exec('owner', ['rm', username, packageName])
465+
t.equal(update, undefined, 'does not remove the substituted user')
466+
t.equal(joinedOutput(), '', 'does not report a removal')
467+
t.match(logs.info.byTitle('owner rm'), [`Not a package owner: ${username}`])
468+
})
469+
367470
t.test('owner rm <user> <pkg> not a current owner', async t => {
368471
const { npm, logs } = await loadMockNpm(t, {
369472
config: { ...auth },

0 commit comments

Comments
 (0)