fix(bootstrap): gate npm registry config on corp gcloud account#321
fix(bootstrap): gate npm registry config on corp gcloud account#321evansenter wants to merge 1 commit into
Conversation
configure_npm_registry tried `gcloud artifacts print-settings npm` against the internal ah-3p-staging-npm repo for any active gcloud account. On a personal account (e.g. gmail.com) that call PERMISSION_DENIEDs, so every `./bootstrap.sh -p` printed "Failed to configure NPM credentials automatically" — a useless warning since public npm uses the explicit registry.npmjs.org and no ~/.npmrc entry is needed. Skip the step silently unless the active account ends in $CORP_NPM_DOMAIN (@anthropic.com). Adds two mutation-verified behavioral tests that load the real constant + function and assert print-settings is reached (and ~/.npmrc written) only for a corp account. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Prompt: claude-review.md
Code Review — Summary
Gates configure_npm_registry on the active gcloud account ending in @anthropic.com, silencing the spurious PERMISSION_DENIED warning on personal machines. The fix is minimal, correctly placed, and covered by two behavioral tests that load the real constant + function from bootstrap.sh (via sed/grep, same pattern as the is_gateway_host tests).
Notes
- Gate logic is correct: the leading @ in the constant anchors the suffix match, so lookalike domains (e.g. foo@evil-anthropic.com) cannot falsely match — only genuine @anthropic.com accounts proceed.
- Placement is right: the gate sits after the command-v / active-account checks and before the print-settings call, so personal accounts no-op silently as intended (no warning).
- Tests cover both branches; the corp path also verifies ~/.npmrc is written, and the env assignments are command-scoped (no PATH/HOME leakage). The sed extraction correctly relies on the function's closing brace at column 0.
Verdict: APPROVE — Clean, well-tested fix with no Critical/Important/Suggestion findings.
(Note: posted via gh pr review because the gh api inline-review path was blocked by the sandbox; this review has no inline comments, so no information is lost.)
Automated review by Claude Code
|
Closing as superseded by 09a288d, which rewrote Generated by Claude Code |
Problem
On
./bootstrap.sh -p,configure_npm_registryrangcloud artifacts print-settings npmagainst the internalah-3p-staging-npmArtifact Registry repo for any active gcloud account. On a personal account (e.g.gmail.com) that call returnsPERMISSION_DENIED, so the function fell into itselsebranch and printed:every single
-prun. It's harmless — public npm installs already pass--registry https://registry.npmjs.org/explicitly and no~/.npmrcentry is needed — but it's noise on personal machines.Fix
Add a
CORP_NPM_DOMAIN="@anthropic.com"constant and skipconfigure_npm_registrysilently unless the active gcloud account ends in that domain. Corp machines behave exactly as before; personal machines no-op cleanly.Tests
Two new behavioral tests in
tests/test-bootstrap.shthat load the real constant + function frombootstrap.sh(viased/grep, same pattern as the existingis_gateway_hosttests), stubgcloud/npmonPATH, and assert via a marker file:npm registry skips personal gcloud account—gmail.comaccount never reachesprint-settings, never writes~/.npmrcnpm registry runs for corp gcloud account—@anthropic.comaccount does bothMutation-verified: removing the gate flips the personal-account test to red.
make checkgreen (lint, 37 bootstrap tests, hooks).🤖 Generated with Claude Code