Skip to content

Added support for new GitHub App tokens#213

Merged
michaelweber merged 1 commit intomainfrom
new-github-app-token-format
Apr 28, 2026
Merged

Added support for new GitHub App tokens#213
michaelweber merged 1 commit intomainfrom
new-github-app-token-format

Conversation

@willmccardell
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@github-actions
Copy link
Copy Markdown

Gemini Review

The PR successfully adds support for the new GitHub App JWT token format, but contains a critical regex bug that will cause false negatives and a likely compilation error in the tests.

Critical Issues

  • The trailing \b in the regex pattern will cause false negatives if the token's base64url signature ends with a hyphen (-). Since - is a non-word character, \b will fail to match when the token is followed by a space or EOF. Replace the trailing \b with a negative lookahead like (?![A-Za-z0-9_-]). (pkg/rule/rules/github.yml)
  • The test uses hasPrefix(...) which appears to be a typo for strings.HasPrefix(...). Unless hasPrefix is defined elsewhere in the test package, this will cause a compilation error. (pkg/validator/github_app_test.go)

Security

No security concerns flagged.

Suggestions

  • Consider adding an example token that ends with a hyphen (-) to the examples list to ensure the regex correctly handles base64url boundary edge cases. (pkg/rule/rules/github.yml)

Reviewed by Gemini (gemini-3.1-pro-preview) · 1 non-code file(s) skipped

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 28, 2026

Walkthrough

This pull request adds detection for GitHub App installation tokens in JWT format. A new detection rule (np.github.8) is introduced to identify ghs_ tokens with a numeric app ID followed by three dot-separated base64url-encoded JWT segments. The default ruleset is updated to include this rule. The GitHub App token validator is extended to support the new rule with corresponding test coverage, and test data is added to validate the new token format.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch new-github-app-token-format

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
pkg/validator/github_app.go (1)

13-40: Update the validator doc comment for the new rule.

The implementation now supports both np.github.3 and np.github.8, but the type comment still says this validator handles np.github.3 only. That wording is stale and will trip up the next person reading the file.

♻️ Suggested doc update
-// GitHubAppTokenValidator validates GitHub App tokens (np.github.3).
+// GitHubAppTokenValidator validates GitHub App tokens for np.github.3 and np.github.8.
 //
-// np.github.3 matches both ghu_ (user-to-server) and ghs_ (server-to-server)
-// tokens. These require different validation endpoints:
+// np.github.3 covers the legacy ghu_/ghs_ token formats, while np.github.8
+// covers the new JWT-format ghs_ installation tokens. These require different
+// validation endpoints:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/validator/github_app.go` around lines 13 - 40, Update the type doc
comment for GitHubAppTokenValidator to reflect that it validates both
np.github.3 and np.github.8 (not just np.github.3), and briefly describe the
dual behavior for ghu_ (user-to-server) and ghs_ (server-to-server/installation)
tokens and the different validation endpoints used (GET /user for ghu_ and GET
/installation/repositories for ghs_); reference the GitHubAppTokenValidator type
and its CanValidate method to ensure the comment aligns with the runtime support
for both rule IDs.
pkg/validator/github_app_test.go (1)

27-51: Add one end-to-end Validate test for the JWT-format token.

The new np.github.8 coverage currently checks extraction and the no-token path, but it never drives Validate(...) with a real JWT-format ghs_... sample. A small HTTP-mocked test would close that gap.

♻️ Suggested test skeleton
+func TestGitHubAppTokenValidator_GHS_JWT_Validate(t *testing.T) {
+	jwtToken := "ghs_123456_eyJhbGciOiJSUzI1NiJ9.eyJpc3MiOiJhcHBfaWQifQ.signature_here"
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		assert.Equal(t, "/installation/repositories", r.URL.Path)
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer server.Close()
+
+	v := NewGitHubAppTokenValidatorWithClient(server.Client())
+	match := &types.Match{
+		RuleID: "np.github.8",
+		NamedGroups: map[string][]byte{
+			"token": []byte(jwtToken),
+		},
+	}
+
+	result, err := v.Validate(t.Context(), match)
+	require.NoError(t, err)
+	assert.Equal(t, types.StatusValid, result.Status)
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/validator/github_app_test.go` around lines 27 - 51, Add an end-to-end
test that exercises GitHubAppTokenValidator.Validate with a JWT-format token:
create a test similar to TestGitHubAppTokenValidator_GHS_JWT_ExtractToken that
constructs a match with RuleID "np.github.8" and NamedGroups["token"] set to a
"ghs_..." JWT-style string, start an httptest.Server that mocks the GitHub
installation/token endpoint and returns a successful JSON response, configure
the validator to use that server (so NewGitHubAppTokenValidator / the
validator's HTTP client points to the mock), call v.Validate(t.Context(), match)
and assert no error and that result.Status equals types.StatusAllowed; also
assert the mock server received the expected request path/headers to ensure the
JWT route was used (use extractToken and NewGitHubAppTokenValidator to locate
behavior to test).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pkg/validator/github_app_test.go`:
- Around line 27-51: Add an end-to-end test that exercises
GitHubAppTokenValidator.Validate with a JWT-format token: create a test similar
to TestGitHubAppTokenValidator_GHS_JWT_ExtractToken that constructs a match with
RuleID "np.github.8" and NamedGroups["token"] set to a "ghs_..." JWT-style
string, start an httptest.Server that mocks the GitHub installation/token
endpoint and returns a successful JSON response, configure the validator to use
that server (so NewGitHubAppTokenValidator / the validator's HTTP client points
to the mock), call v.Validate(t.Context(), match) and assert no error and that
result.Status equals types.StatusAllowed; also assert the mock server received
the expected request path/headers to ensure the JWT route was used (use
extractToken and NewGitHubAppTokenValidator to locate behavior to test).

In `@pkg/validator/github_app.go`:
- Around line 13-40: Update the type doc comment for GitHubAppTokenValidator to
reflect that it validates both np.github.3 and np.github.8 (not just
np.github.3), and briefly describe the dual behavior for ghu_ (user-to-server)
and ghs_ (server-to-server/installation) tokens and the different validation
endpoints used (GET /user for ghu_ and GET /installation/repositories for ghs_);
reference the GitHubAppTokenValidator type and its CanValidate method to ensure
the comment aligns with the runtime support for both rule IDs.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ebbc84c6-4a17-43a9-adce-ae993606a3f0

📥 Commits

Reviewing files that changed from the base of the PR and between e58f294 and bc3be98.

📒 Files selected for processing (5)
  • pkg/rule/rules/github.yml
  • pkg/rule/rulesets/default.yml
  • pkg/validator/github_app.go
  • pkg/validator/github_app_test.go
  • testdata/secrets/github-tokens.txt

Copy link
Copy Markdown
Collaborator

@michaelweber michaelweber left a comment

Choose a reason for hiding this comment

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

This looks reasonable.

@michaelweber michaelweber merged commit c0e3390 into main Apr 28, 2026
25 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants