Add Azure Artifacts npm authentication refresh - #2339
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const result = spawnSync(invocation.command, invocation.args, { | ||
| stdio: "inherit", | ||
| }); |
There was a problem hiding this comment.
Pull request overview
Adds cross-platform Azure Artifacts npm authentication setup for Microsoft contributors.
Changes:
- Adds guarded credential refresh and scoped
.npmrcgeneration. - Adds comprehensive Vitest coverage.
- Documents setup and ignores generated configurations.
Show a summary per file
| File | Description |
|---|---|
scripts/npm-auth-refresh.mjs |
Implements configuration and authentication refresh. |
scripts/npm-auth-refresh.d.mts |
Declares script types. |
nodejs/test/npm-auth-refresh.test.ts |
Tests CLI and platform behavior. |
nodejs/package.json |
Adds the refresh command. |
CONTRIBUTING.md |
Documents contributor setup. |
.gitignore |
Ignores generated .npmrc files. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 5/6 changed files
- Comments generated: 2
- Review effort level: Balanced
| export function writeProjectNpmConfigs(npmrcPaths) { | ||
| const config = buildProjectNpmConfig(); | ||
| for (const npmrcPath of npmrcPaths) { | ||
| writeFileSync(npmrcPath, config, "utf8"); |
| }, | ||
| { | ||
| command: "artifacts-npm-credprovider", | ||
| args: ["-c", npmrcPath], |
SteveSandersonMS
left a comment
SteveSandersonMS
left a comment
There was a problem hiding this comment.
Line 32 — writeProjectNpmConfigs unconditionally overwrites .npmrc files, erasing any pre-existing unrelated settings. Should either merge the @github:registry key into existing content or reject if the file exists and wasn't generated by this script.
Line 63 — Non-Windows path missing -f/--force flag for artifacts-npm-credprovider. Without it, the documented 'rerun after Azure 401/403' recovery step won't force a fresh token refresh on mac/Linux.
Line 88 — CodeQL alert: shell command built from process.env.ComSpec. Either hardcode the interpreter or validate/sanitize the env value.
SteveSandersonMS
left a comment
SteveSandersonMS
left a comment
There was a problem hiding this comment.
Review of technical issues
| export function writeProjectNpmConfigs(npmrcPaths) { | ||
| const config = buildProjectNpmConfig(); | ||
| for (const npmrcPath of npmrcPaths) { | ||
| writeFileSync(npmrcPath, config, "utf8"); |
There was a problem hiding this comment.
writeProjectNpmConfigs unconditionally overwrites .npmrc files, erasing any pre-existing unrelated settings. Should either merge the @github:registry key into existing content or reject if the file exists and wasn't generated by this script.
| }, | ||
| { | ||
| command: "artifacts-npm-credprovider", | ||
| args: ["-c", npmrcPath], |
There was a problem hiding this comment.
Non-Windows path missing -f/--force flag for artifacts-npm-credprovider. Without it, the documented 'rerun after Azure 401/403' recovery step won't force a fresh token refresh on mac/Linux.
| const invocation = getCommandInvocation(platform, command, args, commandInterpreter); | ||
| const result = spawnSync(invocation.command, invocation.args, { | ||
| stdio: "inherit", | ||
| }); |
There was a problem hiding this comment.
CodeQL alert: shell command built from process.env.ComSpec. Either hardcode the interpreter or validate/sanitize the env value.
Summary
@githubwhile storing credentials at user levelValidation
npm test -- npm-auth-refresh.test.ts(15 passed)npm run lint(passed with 3 existing warnings)npm run typechecknpx tsc --noEmit --strict --skipLibCheck --module NodeNext --moduleResolution NodeNext --target ES2022 --esModuleInterop test\npm-auth-refresh.test.tsnpx prettier --config .prettierrc.json --check test\npm-auth-refresh.test.ts ..\scripts\npm-auth-refresh.mjs ..\scripts\npm-auth-refresh.d.mtsnode .\scripts\npm-auth-refresh.mjs --helpnpm run format:checkwas also run; on this Windows checkout it reports existing CRLF formatting drift in 104 untouched Node files. All new auth files pass the targeted Prettier check above.