fix(webhooks): resolve GitHub installation_id for OAuth tenant link - #606
fix(webhooks): resolve GitHub installation_id for OAuth tenant link#606yuvrxj-afk wants to merge 1 commit into
Conversation
GitHub sends installation_id in neither the token body nor the callback query, so tenant-link resolution returned null and GitHub webhooks never routed to a tenant. Merge OAuth callback query params under the token body (token wins) and fall back to GET /user/installations in the GitHub resolver.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
core
Plugin PR scorecard —
|
| Check | Status | Notes |
|---|---|---|
| R1 — Scope: plugin files only | ❌ | Out of scope: packages/corsair/oauth/index.ts, packages/corsair/tests/oauth-callback-params.test.ts, packages/corsair/tunnel/index.ts |
| R2 — Tests with assertions | ✅ | |
| R3 — Description | ❌ | Description section is empty or placeholder |
| R3 — Linked issue / claim | No "Fixes #…" or claim link — add one if this PR has a claim or issue | |
| R4 — Demo video / recording | ❌ | Required in "Screenshots / Demos" before a maintainer reviews |
Rules: PLUGIN_PR_RULES.md · re-runs on every push
📝 WalkthroughWalkthroughOAuth callback parameters now pass through the tunnel and OAuth processing flow. Token data takes precedence over callback parameters. GitHub tenant-link resolution now falls back to the GitHub installations API when no installation ID is present. ChangesOAuth provider data handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant OAuthCallbackTunnel
participant processOAuthCallback
participant mergeOAuthProviderData
participant resolveGithubOAuthWebhookTenantLink
participant GitHubAPI
OAuthCallbackTunnel->>processOAuthCallback: callbackParams
processOAuthCallback->>mergeOAuthProviderData: tokens and callbackParams
mergeOAuthProviderData->>resolveGithubOAuthWebhookTenantLink: merged provider data
resolveGithubOAuthWebhookTenantLink->>GitHubAPI: GET /user/installations when needed
GitHubAPI-->>resolveGithubOAuthWebhookTenantLink: installation data or error
resolveGithubOAuthWebhookTenantLink-->>processOAuthCallback: tenant link or null
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR propagates OAuth callback query parameters into webhook tenant-link resolution and adds a GitHub API fallback when
Confidence Score: 4/5The PR should not merge until GitHub installation selection is unambiguous and callback parameters are propagated through every supported OAuth completion path. Selecting the first accessible GitHub installation can persist the wrong routing identity, while browser-delivery and management callbacks still discard the query-only installation ID that this change is intended to preserve. Files Needing Attention: packages/github/webhooks/oauth-tenant-link.ts and packages/corsair/oauth/index.ts Important Files Changed
Sequence DiagramsequenceDiagram
participant Provider as GitHub
participant Hub
participant SDK as Corsair SDK
participant API as GitHub API
participant DB
Provider->>Hub: OAuth callback (code, query params)
Hub->>SDK: Signed oauth.callback payload
SDK->>Provider: Exchange code for access token
alt installation_id present
SDK->>SDK: Merge callback params with token data
else installation_id absent
SDK->>API: GET /user/installations
API-->>SDK: Accessible installations
SDK->>SDK: Select first installation
end
SDK->>DB: Persist installation_id → tenant link
Reviews (1): Last reviewed commit: "fix(webhooks): resolve GitHub installati..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const body = (await res.json()) as { | ||
| installations?: Array<{ id?: unknown }>; | ||
| }; | ||
| return toExternalId(body.installations?.[0]?.id) ?? null; |
There was a problem hiding this comment.
If an OAuth token can access multiple GitHub App installations and no installation_id was forwarded, this fallback persists installations[0] without identifying which installation belongs to the connection, causing webhooks from the intended installation to miss the tenant routing link.
Knowledge Base Used: OAuth Helpers, Dev Tunnel, and Webhook Tenant Routing
| plugin?: string; | ||
| tenantId?: string; | ||
| // Provider callback params (e.g. GitHub's installation_id) absent from the token body. | ||
| callbackParams?: Record<string, string>; |
There was a problem hiding this comment.
When GitHub OAuth completes through the existing browser-delivery or management entry point, those callers do not supply the new callbackParams, so a query-only installation_id is lost and the tenant remains dependent on the ambiguous first-installation fallback for webhook routing.
Knowledge Base Used: OAuth Helpers, Dev Tunnel, and Webhook Tenant Routing
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/github/webhooks/oauth-tenant-link.ts`:
- Around line 22-28: Update the fetch call in the OAuth tenant-link flow to use
an AbortController with a bounded timeout, passing its signal to the GitHub API
request. Preserve the existing null fallback on failure and clear the timeout
timer in a finally block.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: df5094ea-7113-4881-8714-410ad2c200bb
📒 Files selected for processing (5)
packages/corsair/oauth/index.tspackages/corsair/tests/oauth-callback-params.test.tspackages/corsair/tunnel/index.tspackages/github/webhooks/oauth-tenant-link.test.tspackages/github/webhooks/oauth-tenant-link.ts
| const res = await fetch('https://api.github.com/user/installations', { | ||
| headers: { | ||
| Authorization: `Bearer ${accessToken}`, | ||
| Accept: 'application/vnd.github+json', | ||
| 'X-GitHub-Api-Version': '2022-11-28', | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file="packages/github/webhooks/oauth-tenant-link.ts"
printf '%s\n' '--- file map ---'
ast-grep outline "$file" --view expanded || true
printf '%s\n' '--- target file ---'
cat -n "$file"
printf '%s\n' '--- related symbols ---'
rg -n -C 5 'resolveGithubOAuthWebhookTenantLink|processOAuthCallback|fetchInstallationId' packages .githubRepository: corsairdev/corsair
Length of output: 23598
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- OAuth callback implementation ---'
sed -n '230,380p' packages/corsair/oauth/index.ts
printf '%s\n' '--- GitHub resolver tests ---'
cat -n packages/github/webhooks/oauth-tenant-link.test.ts
printf '%s\n' '--- timeout and fetch controls ---'
rg -n -C 4 'AbortController|AbortSignal|setTimeout|request timeout|timeout|fetch\(' packages/corsair packages/github | head -n 300Repository: corsairdev/corsair
Length of output: 29730
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- remaining processOAuthCallback flow ---'
sed -n '360,470p' packages/corsair/oauth/index.ts
printf '%s\n' '--- webhook resolver dispatch ---'
rg -n -C 8 'function resolveOAuthWebhookTenantLink|resolveOAuthWebhookTenantLink\(' packages/corsair
printf '%s\n' '--- request entry points and response timeout configuration ---'
rg -n -C 6 'handle.*Request|processCorsair|setTimeout|AbortSignal|timeoutMs|requestTimeout|REQUEST_TIMEOUT' packages/corsair/tunnel packages/corsair/hub packages/corsair/oauth packages/corsair/core
printf '%s\n' '--- standalone source invariant probe ---'
python3 - <<'PY'
from pathlib import Path
p = Path("packages/github/webhooks/oauth-tenant-link.ts")
s = p.read_text()
fetch_start = s.index("const res = await fetch(")
fetch_end = s.index("\n\t\t});", fetch_start) + len("\n\t\t});")
fetch_block = s[fetch_start:fetch_end]
print("fetch_has_signal:", "signal:" in fetch_block)
print("fetch_is_awaited:", "const res = await fetch(" in fetch_block)
print("resolver_awaits_fetch_helper:", "fromToken ?? (await fetchInstallationId" in s)
print("helper_catches_fetch_failure:", "catch {" in s and "return null;" in s[s.index("catch {"):])
PYRepository: corsairdev/corsair
Length of output: 36864
Bound the GitHub API request.
Add an abort signal with a bounded timeout. Preserve the null fallback and clear the timer in finally.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/github/webhooks/oauth-tenant-link.ts` around lines 22 - 28, Update
the fetch call in the OAuth tenant-link flow to use an AbortController with a
bounded timeout, passing its signal to the GitHub API request. Preserve the
existing null fallback on failure and clear the timeout timer in a finally
block.
1 participant
Problem
GitHub returns
installation_idin neither the OAuth token body nor the callback query — thelogin/oauth/authorizecallback is just?code=&state=. Webhook tenant-link resolution had nothing to key on, returned null, and inbound GitHub webhooks never routed to a tenant. Slack and Linear work because their identity is in the token body.Change
mergeOAuthProviderData(token body wins on collision), threaded through the tunnel payload, so query-only identity likeinstallation_idcan route.GET /user/installationswith the access token wheninstallation_idis absent, using the first installation.Tests
packages/corsair/tests/oauth-callback-params.test.ts— 3 passingpackages/github/webhooks/oauth-tenant-link.test.ts— 4 passingPairs with corsairdev/hub
fix/webhook-tenant-link(hub forwards the callback params and resolvesinstallation_idfor managed connections). Both are additive; the GitHub webhook path needs both.Summary by CodeRabbit
New Features
Bug Fixes