Skip to content

Commit ecd8eea

Browse files
committed
add support for github app
1 parent 8287407 commit ecd8eea

File tree

2 files changed

+445
-4
lines changed

2 files changed

+445
-4
lines changed

pkg/github/client.go

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -822,6 +822,12 @@ type request struct {
822822
org string
823823
requestBody interface{}
824824
exitCodes []int
825+
// allowInDryRun allows this request even in dry-run mode.
826+
// WARNING: This should ONLY be used for read-only operations that enable other reads,
827+
// such as GitHub App installation token acquisition. NEVER use this for actual mutations
828+
// (creating/updating/deleting org members, teams, repos, etc.) as it would defeat the
829+
// purpose of dry-run mode. Currently only used for: /app/installations/{id}/access_tokens
830+
allowInDryRun bool
825831
}
826832

827833
type requestError struct {
@@ -923,12 +929,35 @@ func (c *client) requestWithContext(ctx context.Context, r *request, ret interfa
923929

924930
// requestRaw makes a request with retries and returns the response body.
925931
// Returns an error if the exit code is not one of the provided codes.
932+
// isDryRunAllowed returns true if this request should be allowed in dry-run mode.
933+
// This enforces an allowlist of read-only operations that enable other reads.
934+
// Currently only allows GitHub App installation token acquisition.
935+
func isDryRunAllowed(r *request) bool {
936+
if !r.allowInDryRun {
937+
return false
938+
}
939+
940+
// Hardcoded allowlist: ONLY allow GitHub App token acquisition
941+
// Pattern: POST /app/installations/{installation_id}/access_tokens
942+
if r.method == http.MethodPost &&
943+
strings.Contains(r.path, "/app/installations/") &&
944+
strings.HasSuffix(r.path, "/access_tokens") {
945+
return true
946+
}
947+
948+
// If allowInDryRun is set but doesn't match the allowlist, this is a bug
949+
// Log an error to catch misuse during development
950+
logrus.Errorf("SECURITY: allowInDryRun=true set for non-allowed endpoint: %s %s. This is a bug - allowInDryRun should ONLY be used for GitHub App token acquisition.", r.method, r.path)
951+
return false
952+
}
953+
926954
func (c *client) requestRaw(r *request) (int, []byte, error) {
927955
return c.requestRawWithContext(context.Background(), r)
928956
}
929957

930958
func (c *client) requestRawWithContext(ctx context.Context, r *request) (int, []byte, error) {
931-
if c.fake || (c.dry && r.method != http.MethodGet) {
959+
// In dry-run mode, block all non-GET requests except for explicitly allowed read-only operations
960+
if c.fake || (c.dry && r.method != http.MethodGet && !isDryRunAllowed(r)) {
932961
return r.exitCodes[0], nil, nil
933962
}
934963
resp, err := c.requestRetryWithContext(ctx, r.method, r.path, r.accept, r.org, r.requestBody)
@@ -5036,15 +5065,21 @@ func (c *client) getAppInstallationToken(installationId int64) (*AppInstallation
50365065
durationLogger := c.log("AppInstallationToken")
50375066
defer durationLogger()
50385067

5039-
if c.dry {
5040-
return nil, fmt.Errorf("not requesting GitHub App access_token in dry-run mode")
5041-
}
5068+
// Note: We allow token fetching even in dry-run mode because:
5069+
// 1. Fetching a token is effectively a read-only operation - it has no side effects on the org/repos
5070+
// 2. The token is required to make any subsequent API calls (even GET requests)
5071+
// 3. All actual mutations (POST/PUT/PATCH/DELETE to org/repo resources) are still blocked by dry-run mode
5072+
// 4. This allows tools to run in dry-run mode with GitHub Apps
50425073

50435074
var token AppInstallationToken
50445075
if _, err := c.request(&request{
50455076
method: http.MethodPost,
50465077
path: fmt.Sprintf("/app/installations/%d/access_tokens", installationId),
50475078
exitCodes: []int{201},
5079+
// allowInDryRun: This is the ONLY place this flag should be set to true.
5080+
// Token acquisition is read-only and enables subsequent reads. Do not use
5081+
// this flag for actual mutations to org/repo resources.
5082+
allowInDryRun: true,
50485083
}, &token); err != nil {
50495084
return nil, err
50505085
}

0 commit comments

Comments
 (0)