feat(#5880): accept GitHub OAuth2 for GET /v1/status - #6174
feat(#5880): accept GitHub OAuth2 for GET /v1/status#6174fullsend-ai-coder[bot] wants to merge 2 commits into
Conversation
Add multi-mode authentication for the /v1/status endpoint via the STATUS_AUTH environment variable (CSV of enabled modes, default: oidc). When "github" mode is enabled, callers can authenticate with a GitHub user token (e.g., from gh CLI or GH_TOKEN). The server validates the token by calling GET /user, then checks org/team membership against the configured STATUS_GITHUB_GROUP (ORG/TEAM format). Access is granted only to active team members. New configuration: - STATUS_AUTH: CSV of enabled modes (oidc, github). Default: oidc. - STATUS_GITHUB_GROUP: ORG/TEAM slug for membership gating. - STATUS_GITHUB_CLIENT_ID: OAuth App client ID (for client discovery). - STATUS_GITHUB_CLIENT_SECRET: OAuth App client secret. When both modes are enabled, OIDC is tried first. If it fails, the handler falls through to GitHub user token validation. The "access" mode is accepted without error for forward compatibility (#5881). Config fields are added to WorkerConfig and NewHandlerFromConfig for CF Worker and explicit-config deployments. Embed files synced. Note: pre-commit could not run (sandbox network policy blocked git fetch for hook setup). The post-code script runs it authoritatively. Closes #5880
|
🤖 Finished Review · ✅ Success · Started 10:15 AM UTC · Completed 10:30 AM UTC Commit: |
ReviewFindingsLow
Previous runReviewFindingsMedium
Low
Labels: PR modifies mint authentication in internal/mintcore/ and internal/dispatch/gcf/, matching the component/mint label. |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
requires-manual-review
|
/fs-fix the CI issues and AI review comments |
|
🤖 Finished Fix · ❌ Failure · Started 10:53 AM UTC · Completed 11:07 AM UTC Commit: |
|
The fix agent completed, but the post-fix script failed before finishing. Workflow run: https://github.com/fullsend-ai/.fullsend/actions/runs/31693001902 Details: Please check the workflow logs for full details and retry with |
|
/fs-fix the CI issues and AI review comments, ans do not neglect to sync the mint embed files |
|
🤖 Finished Fix · ✅ Success · Started 4:59 AM UTC · Completed 5:11 AM UTC Commit: |
- Fix TestBundleEmbeddedMintSource: update expected count from 18 to 19 and add assert for status_auth.go in the embed file list - Add post-loop validation in ValidateStatusAuthConfig ensuring at least one implemented mode (oidc or github) is present, preventing STATUS_AUTH=access from silently 401-ing all requests - Add url.PathEscape for org, team, and username in CheckTeamMembership URL construction (defense-in-depth) - Document STATUS_AUTH, STATUS_GITHUB_GROUP, STATUS_GITHUB_CLIENT_ID, STATUS_GITHUB_CLIENT_SECRET in standalone-mint.md env var table - Update infrastructure-reference.md status endpoint docs to mention GitHub user token auth mode - Sync status_auth.go.embed with source changes Addresses review feedback on #6174
🔧 Fix agent — iteration 1 (human-triggered)Fixed CI failure (TestBundleEmbeddedMintSource count), addressed 5 of 8 review findings with code/doc changes, and disagreed with 3 low-severity cosmetic findings. Synced all mint embed files. Fixed (6):
Disagreed (3):
Tests: passedNext steps:
|
|
🤖 Review · ❌ Terminated · Started 5:12 AM UTC · Ended 5:27 AM UTC Commit: |
Site previewPreview: https://50e6a73e-site.fullsend-ai.workers.dev Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| if group == "" { | ||
| return fmt.Errorf("STATUS_GITHUB_GROUP is required when github status auth mode is enabled") | ||
| } | ||
| if !strings.Contains(group, "/") { |
There was a problem hiding this comment.
[low] edge-case
ValidateStatusAuthConfig checks that STATUS_GITHUB_GROUP contains a / but does not validate that both the org and team parts are non-empty. A value like /team or org/ would pass validation, producing a malformed GitHub API URL and an empty org in the status response.
Suggested fix: Split on / and verify both parts are non-empty.
| // NewHandlerFromConfig and ParseWorkerConfig. | ||
| type StatusAuthConfig struct { | ||
| StatusAuth string | ||
| StatusGithubGroup string |
There was a problem hiding this comment.
[low] naming-coherence
StatusGithubClientID and StatusGithubClientSecret are stored in the handler but not used for server-side token validation. The comments state 'Stored for client discovery; not used by the server for token validation' but no client discovery endpoint exists in this PR and no issue reference explains the planned work.
Suggested fix: Add a comment with the issue reference for client discovery (e.g. #5879 or a child issue).
| if clientSecret == "" { | ||
| return fmt.Errorf("STATUS_GITHUB_CLIENT_SECRET is required when github status auth mode is enabled") | ||
| } | ||
| case "access": |
There was a problem hiding this comment.
[low] intent-documentation
ValidateStatusAuthConfig accepts an access mode for forward compatibility but no ADR, issue, or design document explains what access mode represents.
Suggested fix: Add a comment referencing #5881 (cited in the PR body as the tracking issue for access mode).
| return fmt.Errorf("STATUS_GITHUB_CLIENT_SECRET is required when github status auth mode is enabled") | ||
| } | ||
| case "access": | ||
| // Future mode — accepted without error for forward compatibility. |
There was a problem hiding this comment.
[low] edge-case
The access mode is accepted by ValidateStatusAuthConfig for forward compatibility but authenticateStatus has no handler for it. When combined with an implemented mode (e.g. STATUS_AUTH=acce... the access entry is silently ignored at dispatch time. This is intentional per the code comment.
|
|
||
| org, team := parseGitHubGroup(h.statusGithubGroup) | ||
|
|
||
| isMember, err := CheckTeamMembership(ctx, h.httpClient, h.githubBaseURL, token, org, team, username) |
There was a problem hiding this comment.
[low] token-scope-mismatch
The team membership check uses the caller's own user token, which requires the read:org scope. If the token lacks this scope, the API returns 404 (indistinguishable from non-membership), causing a legitimate team member to be denied with 'not a member of the required group.'
| } | ||
|
|
||
| // ParseStatusAuthModes parses the STATUS_AUTH CSV into a list of enabled modes. | ||
| // Default: ["oidc"]. |
There was a problem hiding this comment.
[low] logic-error
ParseStatusAuthModes does not deduplicate modes. STATUS_AUTH=gith... yields [github, github]. No runtime impact since statusAuthModeEnabled short-circuits on first match.
| // statusAuthError carries an HTTP status code and message for status auth failures. | ||
| type statusAuthError struct { | ||
| status int | ||
| message string |
There was a problem hiding this comment.
[low] naming-convention
statusAuthError uses a message field while the existing mintError in handler.go uses msg for a semantically similar purpose.
Suggested fix: Rename message to msg for consistency with the existing type.
fullsend-ai-review
Bot
added
ready-for-merge
|
🤖 Finished Review · ✅ Success · Started 5:12 AM UTC · Completed 5:27 AM UTC Commit: |
1 participant
Summary
GET /v1/statusviaSTATUS_AUTHenv var (CSV, default:oidc)githubmode is enabled, validate GitHub user tokens viaGET /userand check org/team membership againstSTATUS_GITHUB_GROUP(ORG/TEAM format)STATUS_AUTH,STATUS_GITHUB_GROUP,STATUS_GITHUB_CLIENT_ID,STATUS_GITHUB_CLIENT_SECRET— added to bothNewHandler(env vars) andNewHandlerFromConfig/WorkerConfig(explicit config)Details
When both
oidcandgithubmodes are enabled, OIDC is tried first; on failure the handler falls through to GitHub user token validation. Theaccessmode is accepted for forward compatibility (#5881) but not yet implemented.The
handleStatusfunction is refactored to accept an org string instead of full OIDC claims, enabling auth-mode-agnostic status responses. The OIDC authorization logic is preserved identically inauthenticateStatusOIDC.All mintcore embed files are synced, and
status_auth.gois registered in the GCF provisioner's embed list.Test plan
Closes #5880
Post-script verification
agent/5880-status-github-oauth)fafb2df30a66eb2dfa0ae9e4dff6641bfebe41a4..HEAD)