fix(release): fail fast on invalid web components credentials - #36566
fix(release): fail fast on invalid web components credentials#36566Jane Chu (janechu) wants to merge 1 commit into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8069819a-0e6e-450d-9f10-59a2df1b15a0
📊 Bundle size report✅ No changes found |
|
Pull request demo site: URL |
|
Jane Chu (@janechu) - I have this in place #36564, invalid npm token has no destructive consequence unlike invalid PAT, I still like to have this check as well but I suggest to incoporate into the flow once mine lands so every pipeline has same safegurads. wdyt ? |
|
Please resolve merge conflicts |
| - script: | | ||
| node -r ./scripts/ts-node/src/register ./scripts/beachball/src/validateReleaseCredentials.ts | ||
| env: | ||
| GITHUB_PAT: $(githubPAT) | ||
| NPM_TOKEN: $(npmToken) | ||
| displayName: Validate release credentials | ||
| condition: not(${{ parameters.dryRun }}) | ||
|
|
There was a problem hiding this comment.
The new step reads $(githubPAT) as GITHUB_PAT, but #36634 replaced that PAT with a short-lived GitHub App token generated inside .devops/templates/run-with-token.yml. Publishing now uses BEACHBALL_GIT_TOKEN and BEACHBALL_NPM_TOKEN.
After resolving the current merge conflict, retaining this step in its present position would cause every non-dry-run release to fail before the GitHub App token exists. The PR should be rebased and validation integrated into the token lifecycle, while preserving guaranteed token revocation.
| async function validateNpmToken(token: string, fetchImpl: FetchLike): Promise<void> { | ||
| const response = await requestCredentialValidation( | ||
| { | ||
| credentialName: 'NPM_TOKEN', | ||
| url: npmWhoAmIEndpoint, | ||
| token, | ||
| headers: { | ||
| Accept: 'application/json', | ||
| }, | ||
| accessDeniedStatuses: [401, 403], | ||
| }, | ||
| fetchImpl, | ||
| ); | ||
|
|
||
| if (!hasNonEmptyStringProperty(response, 'username')) { | ||
| throw new Error('Unable to validate NPM_TOKEN: endpoint returned an unexpected response.'); | ||
| } | ||
| } |
There was a problem hiding this comment.
/-/whoami only confirms that the token authenticates as an npm user. A valid read-only granular token would pass, allow the expensive build to run, and then fail during beachball publish.
The preflight should confirm publish-level access to the relevant scope or packages, with a negative test for an authenticated token lacking publish permission.
| const repositoryResponse = await requestCredentialValidation( | ||
| { | ||
| credentialName: 'GITHUB_PAT', | ||
| url: githubRepositoryEndpoint, | ||
| token, | ||
| headers, | ||
| accessDeniedStatuses: [401, 403, 404], | ||
| }, | ||
| fetchImpl, | ||
| ); | ||
|
|
||
| if (!hasStringPropertyValue(repositoryResponse, 'full_name', 'microsoft/fluentui')) { | ||
| throw new Error('Unable to validate GITHUB_PAT: endpoint returned an unexpected response.'); | ||
| } | ||
| } |
There was a problem hiding this comment.
A successful repository GET and matching full_name prove only repository visibility. Beachball also needs permission to push release commits and tags.
The validator should require repository write capability—such as contents: write for the installation token—and include a test where the repository is readable but not writable.
3 participants
Previous Behavior
The web components release could complete build and test work before missing or invalid npm/GitHub credentials failed during publishing.
New Behavior
Non-dry-run releases validate npm identity plus GitHub identity and repository access immediately after dependency installation, before build and publish steps. Validation failures are sanitized so credential values are not logged; dry runs remain unchanged.
Testing
yarn nx run-many -t lint type-check test -p scripts-beachball --nxBail --skip-nx-cacheyarn nx format:check --files=azure-pipelines.release.web-components.yml,scripts/beachball/src/validateReleaseCredentials.ts,scripts/beachball/src/validateReleaseCredentials.test.tsNo beachball change file is included because no published package changed.
Related Issue(s)
None.