Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions lib/entry-points.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 15 additions & 6 deletions src/artifact-scanner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ test("makeTestToken", (t) => {
t.is(makeTestToken(255).length, 255);
});

const NEW_FORMAT_GHS_TOKEN =
"ghs_abc123.def456.ghi789_abc123.def456.ghi789";

test("isAuthToken", (t) => {
// Undefined for strings that aren't tokens
t.is(isAuthToken("some string"), undefined);
Expand All @@ -32,9 +35,10 @@ test("isAuthToken", (t) => {
// Token types for strings that are tokens.
t.is(isAuthToken(`ghp_${makeTestToken()}`), TokenType.PersonalAccessClassic);
t.is(isAuthToken(`ghp_${makeTestToken()}`), TokenType.PersonalAccessClassic);
t.is(isAuthToken(NEW_FORMAT_GHS_TOKEN), TokenType.ServerToServer);
t.is(
isAuthToken(`ghs_${makeTestToken(255)}`),
TokenType.AppInstallationAccess,
TokenType.ServerToServer,
);
t.is(
isAuthToken(`github_pat_${makeTestToken(22)}_${makeTestToken(59)}`),
Expand All @@ -52,6 +56,15 @@ test("isAuthToken", (t) => {
]),
undefined,
);
t.is(
isAuthToken(NEW_FORMAT_GHS_TOKEN, [
{
type: TokenType.AppInstallationAccess,
pattern: /ghs_[A-Za-z0-9._-]{36,}/g,
},
]),
TokenType.AppInstallationAccess,
);
});

const testTokens = [
Expand All @@ -76,16 +89,12 @@ const testTokens = [
},
{
type: TokenType.ServerToServer,
value: `ghs_${makeTestToken()}`,
value: NEW_FORMAT_GHS_TOKEN,
},
Comment thread
hagould marked this conversation as resolved.
{
type: TokenType.Refresh,
value: `ghr_${makeTestToken()}`,
},
{
type: TokenType.AppInstallationAccess,
value: `ghs_${makeTestToken(255)}`,
},
];

for (const { type, value, checkPattern } of testTokens) {
Expand Down
25 changes: 18 additions & 7 deletions src/artifact-scanner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,15 @@ const GITHUB_TOKEN_PATTERNS: TokenPattern[] = [
},
{
type: TokenType.ServerToServer,
pattern: /\bghs_[a-zA-Z0-9]{36}\b/g,
pattern: /ghs_[A-Za-z0-9._-]{36,}/g,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot is correct. For our use case(s), we would not want this to be unconstrained.

},
{
type: TokenType.Refresh,
pattern: /\bghr_[a-zA-Z0-9]{36}\b/g,
},
{
type: TokenType.AppInstallationAccess,
pattern: /\bghs_[a-zA-Z0-9]{255}\b/g,
pattern: /ghs_[A-Za-z0-9._-]{36,}/g,
Comment thread
hagould marked this conversation as resolved.
Outdated
},
Comment thread
hagould marked this conversation as resolved.
Outdated
];

Expand Down Expand Up @@ -109,16 +109,27 @@ function scanFileForTokens(
logger: Logger,
): TokenFinding[] {
const findings: TokenFinding[] = [];
const seenMatches = new Set<number>();
Comment thread
hagould marked this conversation as resolved.
Outdated
Comment thread
hagould marked this conversation as resolved.
Outdated
try {
const content = fs.readFileSync(filePath, "utf8");

for (const { type, pattern } of GITHUB_TOKEN_PATTERNS) {
const matches = content.match(pattern);
if (matches) {
for (let i = 0; i < matches.length; i++) {
findings.push({ tokenType: type, filePath: relativePath });
const regex = new RegExp(pattern.source, pattern.flags);
let matchCount = 0;

for (const match of content.matchAll(regex)) {
const index = match.index;
if (index === undefined || seenMatches.has(index)) {
continue;
}
logger.debug(`Found ${matches.length} ${type}(s) in ${relativePath}`);

seenMatches.add(index);
findings.push({ tokenType: type, filePath: relativePath });
matchCount++;
Comment thread
hagould marked this conversation as resolved.
Outdated
}
Comment thread
hagould marked this conversation as resolved.
Outdated

if (matchCount > 0) {
logger.debug(`Found ${matchCount} ${type}(s) in ${relativePath}`);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain what the goal of these changes is in relation to the objectives of the PR?

Copy link
Copy Markdown
Author

@hagould hagould May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot is correct. For our use case(s), we would not want this to be unconstrained.

Updated to \bghs_[A-Za-z0-9._-]{36,516}(?![A-Za-z0-9._-]). The upper bound of 516 comes from babeld's MAX_PASSWORD_SIZE of 520 bytes, which is the hard ceiling.

Could you explain what the goal of these changes is in relation to the objectives of the PR?

They were added to handle overlapping patterns between ServerToServer and AppInstallationAccess, but since those are now collapsed into a single pattern, they're no longer needed.

}
}

Expand Down
Loading