Warn if a private registry configuration uses a PAT, but has no username #3422
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR adds detection of PAT misuse in private registry credentials and enhances the artifact scanner to recognize fine-grained GitHub tokens, with corresponding test coverage and support utilities.
Changes:
- Extend the artifact scanner with a
TokenTypeenum, reusable token patterns (including fine-grained PATs), and an exportedisAuthTokenhelper used both in scanning and elsewhere. - Update
getCredentialsinstart-proxy.tsto warn when a GitHub PAT (classic or fine-grained) is used without a username, and add tests to verify logging behavior. - Introduce
makeTestTokenand expand artifact-scanner tests to systematically validate token detection and logging for all supported token types.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/testing-utils.ts | Adds makeTestToken helper for generating deterministic token-like strings used in new tests. |
| src/start-proxy.ts | Imports the artifact scanner, introduces isPAT, and logs a warning when credentials use a PAT without a username. |
| src/start-proxy.test.ts | Adds a test that verifies getCredentials still accepts PAT-based credentials but logs the expected warning when no username is provided. |
| src/artifact-scanner.ts | Refactors token pattern handling into typed TokenType/TokenPattern, adds explicit PAT patterns (classic & fine-grained), exposes isAuthToken, and adapts scanning to use the typed patterns. |
| src/artifact-scanner.test.ts | Reworks tests to cover makeTestToken, isAuthToken, and all token types in scanArtifactsForTokens, checking both thrown error messages and logged summaries. |
| lib/* (multiple files) | Generated JavaScript outputs corresponding to the updated TypeScript sources; not manually reviewed per repository guidelines. |
| export function isAuthToken( | ||
| value: string, | ||
| patterns: TokenPattern[] = GITHUB_TOKEN_PATTERNS, | ||
| ) { | ||
| for (const { type, pattern } of patterns) { | ||
| if (pattern.test(value)) { | ||
| return type; | ||
| } | ||
| } | ||
| return undefined; |
There was a problem hiding this comment.
isAuthToken iterates over TokenPattern values whose regexes all use the /g flag and calls pattern.test(value) directly. Because RegExp.prototype.test with a global regex mutates and uses lastIndex, repeated calls with the same value can alternate between true and false, so isAuthToken is not reliably idempotent and may fail to detect tokens on subsequent calls. Consider either resetting pattern.lastIndex before each test or using non-global regexes for this helper (while keeping the /g-flagged patterns for the scanning code that needs them).
| var core7 = __toESM(require_core()); | ||
|
|
||
| // src/artifact-scanner.ts | ||
| var exec = __toESM(require_exec()); |
There was a problem hiding this comment.
Unused variable exec.
To authenticate using a PAT, the corresponding username is required. Using a PAT on its own, without the corresponding username, does not work.
This PR modifies the
start-proxyaction to check whether a configuration seems to use a GitHub PAT without a username and issue a warning in that case.In the process, I also taught the artifact scanner about fine-grained tokens (start with
github_pat_and are longer than classic PATs) and extended the tests there.Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
Workflow types:
dynamicworkflows (Default Setup, CCR, ...).Products:
analysis-kinds: code-scanning.analysis-kinds: code-quality.Environments:
github.comand/or GitHub Enterprise Cloud with Data Residency.How did/will you validate this change?
.test.tsfiles).pr-checks).If something goes wrong after this change is released, what are the mitigation and rollback strategies?
How will you know if something goes wrong after this change is released?
Are there any special considerations for merging or releasing this change?
Merge / deployment checklist