Skip to content

baton-github: refersh appjwttoken once it expires #117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Bencheng21 merged 4 commits into from
Feb 3, 2026
+53 −17
Merged

baton-github: refersh appjwttoken once it expires #117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a7c5106
baton-github: refersh appjwttoken once it expires
Bencheng21 Jan 30, 2026
d81f551
update the comments
Bencheng21 Feb 2, 2026
7073cc7
update sleep time
Bencheng21 Feb 2, 2026
1e17759
update workflow
Bencheng21 Feb 3, 2026
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
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,6 @@ jobs:
uses: ConductorOne/github-workflows/actions/sync-test@v2
with:
connector: ./baton-github
baton-entitlement: 'repository:642588514:triage'
baton-entitlement: 'repository:642588514:admin'
Copy link
Contributor Author

@Bencheng21 Bencheng21 Feb 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

@laurenleach sleep time doesn't solve the problem, but this line fixes the problem.
The issue is that the user is a member of a team that has been granted access to this repository. Even if we remove the user’s direct access entitlement, they will still be able to access the repo through the team.
I really don't like grant the user with admin entitlement, but we can't do anything if we don't have admin pat token. ( updating these tests requires the resource id )

baton-principal: '166871869'
baton-principal-type: 'user'
68 changes: 52 additions & 16 deletions pkg/connector/connector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ import (

const githubDotCom = "https://github.com"

// JWT token expires in 10 minutes, so we set it to 9 minutes to leave some buffer.
const jwtExpiryTime = 9 * time.Minute

var (
ValidAssetDomains = []string{"avatars.githubusercontent.com"}
maxPageSize int = 100 // maximum page size github supported.
Expand Down Expand Up @@ -219,9 +222,9 @@ func (gh *GitHub) validateAppCredentials(ctx context.Context) (annotations.Annot
return nil, fmt.Errorf("github-connector: only one org is allowed when using github app")
}

_, resp, err := findInstallation(ctx, gh.appClient, orgLogins[0])
_, err := findInstallation(ctx, gh.appClient, orgLogins[0])
if err != nil {
return nil, wrapGitHubError(err, resp, "github-connector: failed to retrieve org installation")
return nil, err
}
Comment on lines +225 to 228
Copy link

@coderabbitai coderabbitai bot Feb 2, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Guard against empty org list before indexing.
Line 225 indexes orgLogins[0]; if the org list is empty (misconfig or future call path), this will panic. Return a clear error instead.

✅ Proposed fix 
 func (gh *GitHub) validateAppCredentials(ctx context.Context) (annotations.Annotations, error) {
 	orgLogins := gh.orgs
 	if len(orgLogins) > 1 {
 		return nil, fmt.Errorf("github-connector: only one org is allowed when using github app")
 	}
+	if len(orgLogins) == 0 {
+		return nil, fmt.Errorf("github-connector: org is required when using github app")
+	}
 
 	_, err := findInstallation(ctx, gh.appClient, orgLogins[0])
 	if err != nil {
 		return nil, err
 	}
 	return nil, nil
 }
🤖 Prompt for AI Agents 
In `@pkg/connector/connector.go` around lines 225 - 228, The code indexes
orgLogins[0] before ensuring the slice is non-empty which can panic; update the
caller in connector.go to check len(orgLogins) > 0 before calling
findInstallation and return a descriptive error (e.g., "no org logins provided")
when empty, then only call findInstallation(ctx, gh.appClient, orgLogins[0])
when the guard passes; reference orgLogins and findInstallation to locate the
change and ensure any returned error is propagated unchanged.

return nil, nil
}
Expand Down Expand Up @@ -273,9 +276,9 @@ func New(ctx context.Context, ghc *cfg.Github, appKey string) (*GitHub, error) {
if err != nil {
return nil, err
}
installation, resp, err := findInstallation(ctx, appClient, ghc.Orgs[0])
installation, err := findInstallation(ctx, appClient, ghc.Orgs[0])
if err != nil {
return nil, wrapGitHubError(err, resp, "github-connector: failed to find app installation")
return nil, err
}

token, err := getInstallationToken(ctx, appClient, installation.GetID())
Expand All @@ -292,7 +295,16 @@ func New(ctx context.Context, ghc *cfg.Github, appKey string) (*GitHub, error) {
ctx: ctx,
instanceURL: ghc.InstanceUrl,
installationID: installation.GetID(),
jwttoken: jwttoken,
jwtTokenSource: oauth2.ReuseTokenSource(
&oauth2.Token{
AccessToken: jwttoken,
Expiry: time.Now().Add(jwtExpiryTime),
Copy link
Contributor

@btipling btipling Jan 30, 2026

Choose a reason for hiding this comment

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

so we set an expiration time on the token that is different than on the jwt, because it will expire sooner and lead to a refresh before the token expires. That aligns with the description of recommended library in #115

Copy link
Contributor Author

@Bencheng21 Bencheng21 Jan 31, 2026

Choose a reason for hiding this comment

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

The JWT is valid for 10 minutes. It’s better to refresh it a bit earlier to avoid a 401 error interrupting the workflow.

},
&appJWTTokenRefresher{
appID: ghc.AppId,
privateKey: appKey,
},
),
},
)
}
Expand Down Expand Up @@ -377,28 +389,36 @@ func getClientToken(ghc *cfg.Github, privateKey string) (string, string, error)
return "", ghc.Token, nil
}

key, err := loadPrivateKeyFromString(privateKey)
token, err := getJWTToken(ghc.AppId, privateKey)
if err != nil {
return "", "", err
}
return token, "", nil
}

func getJWTToken(appID string, privateKey string) (string, error) {
key, err := loadPrivateKeyFromString(privateKey)
if err != nil {
return "", err
}
now := time.Now()
token, err := jwtv5.NewWithClaims(jwtv5.SigningMethodRS256, jwtv5.MapClaims{
"iat": now.Unix() - 60, // issued at
"exp": now.Add(time.Minute * 10).Unix(), // expires
"iss": ghc.AppId, // GitHub App ID
"iss": appID, // GitHub App ID
}).SignedString(key)
if err != nil {
return "", "", err
return "", err
}
return token, "", nil
return token, nil
}
Comment on lines +399 to 414
Copy link

@coderabbitai coderabbitai bot Jan 30, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Validate appID before signing.

An empty appID yields an invalid iss claim and opaque auth failures. Add an explicit check.

Suggested fix 
 func getJWTToken(appID string, privateKey string) (string, error) {
+	if strings.TrimSpace(appID) == "" {
+		return "", errors.New("github-connector: app ID is required")
+	}
 	key, err := loadPrivateKeyFromString(privateKey)
 	if err != nil {
 		return "", err
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func getJWTToken(appID string, privateKey string) (string, error) {
key, err := loadPrivateKeyFromString(privateKey)
if err != nil {
return "", err
}
now := time.Now()
token, err := jwtv5.NewWithClaims(jwtv5.SigningMethodRS256, jwtv5.MapClaims{
"iat": now.Unix() - 60, // issued at
"exp": now.Add(time.Minute * 10).Unix(), // expires
"iss": ghc.AppId, // GitHub App ID
"iss": appID, // GitHub App ID
}).SignedString(key)
if err != nil {
return "", "", err
return "", err
}
return token, "", nil
return token, nil
}
func getJWTToken(appID string, privateKey string) (string, error) {
if strings.TrimSpace(appID) == "" {
return "", errors.New("github-connector: app ID is required")
}
key, err := loadPrivateKeyFromString(privateKey)
if err != nil {
return "", err
}
now := time.Now()
token, err := jwtv5.NewWithClaims(jwtv5.SigningMethodRS256, jwtv5.MapClaims{
"iat": now.Unix() - 60, // issued at
"exp": now.Add(time.Minute * 10).Unix(), // expires
"iss": appID, // GitHub App ID
}).SignedString(key)
if err != nil {
return "", err
}
return token, nil
}
🤖 Prompt for AI Agents 
In `@pkg/connector/connector.go` around lines 399 - 414, The getJWTToken function
should validate that appID is non-empty before creating the JWT; add an explicit
check at the start of getJWTToken (before calling loadPrivateKeyFromString) that
returns a clear error if appID is empty, so the generated token never contains
an empty "iss" claim; update the function to return that error (e.g., from
getJWTToken) and ensure callers handle it.


func findInstallation(ctx context.Context, c *github.Client, orgName string) (*github.Installation, *github.Response, error) {
func findInstallation(ctx context.Context, c *github.Client, orgName string) (*github.Installation, error) {
installation, resp, err := c.Apps.FindOrganizationInstallation(ctx, orgName)
if err != nil {
return nil, nil, err
return nil, wrapGitHubError(err, resp, "github-connector: failed to find installation")
Copy link
Contributor

@btipling btipling Jan 30, 2026

Choose a reason for hiding this comment

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

this is an unrelated change just to wrap the GitHub error in one place yes?

Copy link
Contributor Author

@Bencheng21 Bencheng21 Jan 31, 2026

Choose a reason for hiding this comment

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

Yes

}
return installation, resp, nil
return installation, nil
}

func getInstallationToken(ctx context.Context, c *github.Client, id int64) (*github.InstallationToken, error) {
Expand All @@ -415,19 +435,35 @@ func getInstallationToken(ctx context.Context, c *github.Client, id int64) (*git
return token, nil
}

// appJWTTokenRefresher is used to refresh the app jwt token when it expires.
type appJWTTokenRefresher struct {
appID string
privateKey string
}

func (r *appJWTTokenRefresher) Token() (*oauth2.Token, error) {
token, err := getJWTToken(r.appID, r.privateKey)
if err != nil {
return nil, err
}

return &oauth2.Token{
AccessToken: token,
Expiry: time.Now().Add(jwtExpiryTime),
}, nil
}

type appTokenRefresher struct {
ctx context.Context
jwttoken string
jwtTokenSource oauth2.TokenSource
instanceURL string
installationID int64
}

func (r *appTokenRefresher) Token() (*oauth2.Token, error) {
appClient, err := newGitHubClient(r.ctx,
r.instanceURL,
oauth2.StaticTokenSource(
&oauth2.Token{AccessToken: r.jwttoken},
),
r.jwtTokenSource,
)
if err != nil {
return nil, err
Expand Down
Loading