feat: managed webhook suite for Graph plugins - #575
Open
yuvrxj-afk wants to merge 3 commits into
Open
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Contributor
|
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: one plugin per PR | ❌ | This PR touches: asana, dropbox, gitlab, hubspot, linear, notion, onedrive, outlook, sharepoint, slack, spotify, teams, zohomail |
| 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
📝 WalkthroughWalkthroughManaged authentication is added to Asana, Dropbox, GitLab, HubSpot, Linear, Notion, OneDrive, Outlook, SharePoint, Slack, Spotify, Teams, and Zoho Mail. Corsair renewal and subscription reporting now support managed authentication. Managed webhook verification fails closed when required state is missing. ChangesManaged authentication across plugins
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PluginKeyBuilder
participant CorsairHub
participant ManagedRefreshAuth
participant ManagedOAuthDelivery
PluginKeyBuilder->>CorsairHub: request managed access token
CorsairHub-->>PluginKeyBuilder: return access token
PluginKeyBuilder->>ManagedRefreshAuth: attach refresh authentication
ManagedOAuthDelivery->>CorsairHub: report or renew subscription
CorsairHub-->>ManagedOAuthDelivery: return subscription result
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
Contributor
Greptile SummaryThe PR extends managed OAuth support and managed Graph webhook handling.
Confidence Score: 4/5The SharePoint batched-notification verification failure needs to be fixed before merging. A batch is accepted after checking only its first notification, allowing subsequent entries with missing or mismatched client state to reach the webhook handler. Files Needing Attention: packages/sharepoint/webhooks/list-changes.ts and packages/sharepoint/webhooks/types.ts
|
| Filename | Overview |
|---|---|
| packages/corsair/hub/managed-oauth.ts | Starts subscription reporting after managed credentials are persisted while preserving successful OAuth delivery if subscription setup fails. |
| packages/corsair/oauth/renewal.ts | Primes renewal using each plugin's configured authentication type and Hub context. |
| packages/sharepoint/index.ts | Loads the static or persisted webhook client state and rejects requests when no verification value exists. |
| packages/sharepoint/webhooks/list-changes.ts | Passes the bound webhook key into verification, but batched requests accept later notifications without validating their client state. |
| packages/sharepoint/webhooks/types.ts | Fails closed when client state is absent but validates only the first notification in a batch. |
Sequence Diagram
sequenceDiagram
participant Hub
participant Corsair
participant Plugin
participant Graph
Hub->>Corsair: Deliver managed OAuth token
Corsair->>Corsair: Persist connection credentials
Corsair->>Plugin: subscribeAndReport
Plugin->>Hub: Resolve managed access token
Plugin->>Graph: Create subscription with clientState
Graph-->>Corsair: Webhook notification
Corsair->>Corsair: Load persisted clientState as ctx.key
Corsair->>Plugin: Handle and verify notification
Reviews (2): Last reviewed commit: "Merge branch 'main' into managed-webhook..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
yuvrxj-afk
force-pushed
the
managed-webhooks
branch
from
August 2, 2026 23:20
24caa4d to
608d4a6
Compare
yuvrxj-afk
force-pushed
the
managed-webhooks
branch
from
August 2, 2026 23:26
608d4a6 to
20ef459
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (10)
packages/dropbox/index.ts (1)
385-401: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSame duplicated managed branch as the other plugins. See the consolidated comment for the shared helper suggestion.
🤖 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/dropbox/index.ts` around lines 385 - 401, Replace the duplicated managed-auth handling in the Dropbox flow with the shared helper proposed for the other plugins. Update the branch around getManagedAccessToken and attachManagedRefreshAuth to delegate context validation, token retrieval, refresh attachment, and access-token return to that helper while preserving the existing Dropbox managed-auth behavior.packages/gitlab/index.ts (1)
922-939: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSame duplicated managed branch as the other plugins. See the consolidated comment.
🤖 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/gitlab/index.ts` around lines 922 - 939, The managed-auth branch in the GitLab flow duplicates the shared implementation used by other plugins. Replace the inline managedContext construction, getManagedAccessToken call, refresh attachment, and access-token return in the surrounding GitLab handler with the consolidated shared managed-auth path, preserving the existing hub validation and GitLab-specific context values.packages/hubspot/index.ts (1)
689-706: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePlace the managed branch inside the
source === 'endpoint'block.Lines 669-687 gate
api_keyandoauth_2onsource === 'endpoint'. The managed branch is not gated. The behavior is the same today, because the webhook branch at Lines 653-667 always returns or throws first. Moving the managed branch into the endpoint block keeps the reader from having to prove that. This block is also duplicated across the other plugins; see the consolidated comment.🤖 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/hubspot/index.ts` around lines 689 - 706, The managed authentication branch around getManagedAccessToken and attachManagedRefreshAuth should be nested within the source === 'endpoint' conditional alongside the api_key and oauth_2 branches. Preserve its existing hub validation, token refresh setup, and access-token return behavior while making the endpoint-source gate explicit.packages/zohomail/index.ts (1)
439-456: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSame duplicated managed branch as the other plugins. See the consolidated comment.
🤖 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/zohomail/index.ts` around lines 439 - 456, Replace the duplicated managed-auth branch in the zohomail flow with the shared managed-auth helper used by the other plugins, preserving the existing hub validation, managed context values, refresh-auth attachment, and access-token return behavior.packages/slack/index.ts (1)
796-813: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSame duplicated managed branch as the other plugins. See the consolidated comment.
🤖 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/slack/index.ts` around lines 796 - 813, Replace the duplicated managed-auth branch in the Slack flow with the shared managed-auth handling used by the other plugins. Reuse the existing helper or consolidated abstraction for validating ctx.hub, building managedContext, attaching refresh auth, and returning the access token, while preserving Slack’s plugin and tenant context.packages/spotify/index.ts (1)
613-630: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSame duplicated managed branch as the other plugins. See the consolidated comment.
🤖 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/spotify/index.ts` around lines 613 - 630, Consolidate the duplicated managed-auth branch in the Spotify integration by reusing the shared managed authentication helper used by the other plugins. Update the flow around getManagedAccessToken and attachManagedRefreshAuth while preserving hub validation, managedContext values, refresh-auth attachment, and access-token return behavior.packages/asana/index.ts (1)
1104-1121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated managed key branch into one
corsair/hubhelper.Eight plugins repeat the same managed branch: check
ctx.hub, build{ keys, hub, plugin, tenantId }, callgetManagedAccessToken, callattachManagedRefreshAuth, return the token. The only differences are the plugin id and the error text. The root cause is thatcorsair/hubexports the two primitives but no combined helper. Add one exported helper, for exampleresolveManagedKey(ctx, pluginId): Promise<string>, and call it from each plugin. The Hub-missing error text then stays consistent across plugins.
packages/asana/index.ts#L1104-L1121: replace the branch withreturn resolveManagedKey(ctx, 'asana');.packages/dropbox/index.ts#L385-L401: replace the branch withreturn resolveManagedKey(ctx, 'dropbox');.packages/gitlab/index.ts#L922-L939: replace the branch withreturn resolveManagedKey(ctx, 'gitlab');.packages/hubspot/index.ts#L689-L706: replace the branch withreturn resolveManagedKey(ctx, 'hubspot');, and move it inside thesource === 'endpoint'block.packages/notion/index.ts#L404-L421: replace the branch withreturn resolveManagedKey(ctx, 'notion');.packages/slack/index.ts#L796-L813: replace the branch withreturn resolveManagedKey(ctx, 'slack');.packages/spotify/index.ts#L613-L630: replace the branch withreturn resolveManagedKey(ctx, 'spotify');.packages/zohomail/index.ts#L439-L456: replace the branch withreturn resolveManagedKey(ctx, 'zohomail');.
packages/linear/index.tsat Lines 525-541 keeps its own return, because it prefixes the token withBearer. It can call the helper and add the prefix.🤖 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/asana/index.ts` around lines 1104 - 1121, Extract the shared managed-auth flow into an exported corsair/hub helper such as resolveManagedKey(ctx, pluginId), consolidating hub validation, managed context construction, getManagedAccessToken, attachManagedRefreshAuth, and token return with consistent errors. Replace the managed branches in packages/asana/index.ts (1104-1121), packages/dropbox/index.ts (385-401), packages/gitlab/index.ts (922-939), packages/notion/index.ts (404-421), packages/slack/index.ts (796-813), packages/spotify/index.ts (613-630), and packages/zohomail/index.ts (439-456) with the corresponding helper call; move the packages/hubspot/index.ts (689-706) call inside its source === 'endpoint' block. Also update packages/linear/index.ts (525-541) to use the helper while preserving its Bearer prefix.packages/notion/index.ts (1)
404-421: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSame duplicated managed branch as the other plugins. See the consolidated comment.
🤖 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/notion/index.ts` around lines 404 - 421, The managed-auth branch in the Notion integration duplicates the shared implementation used by other plugins. Replace the inline logic in the managed branch around getManagedAccessToken and attachManagedRefreshAuth with the established shared helper or consolidated flow, preserving hub validation and returning the managed access token.packages/onedrive/managed-webhook.test.ts (1)
3-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winManaged webhook test suites do not cover the endpoint/managed token-retrieval path. Both files test only the webhook key-builder branch; neither tests the
source === 'endpoint',authType === 'managed'branch that callsgetManagedAccessTokenandattachManagedRefreshAuth.
packages/onedrive/managed-webhook.test.ts#L3-L28: add a test that mockscorsair/hub'sgetManagedAccessToken/attachManagedRefreshAuthand asserts the OneDrivekeyBuilderreturns the resolved access token and throws whenctx.hubis missing, exercisingpackages/onedrive/index.tsLines 912-930.packages/outlook/managed-webhook.test.ts#L3-L28: add the equivalent test for the OutlookkeyBuilder, exercisingpackages/outlook/index.tsLines 679-697.🤖 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/onedrive/managed-webhook.test.ts` around lines 3 - 28, The managed webhook tests cover only stored clientState and omit endpoint managed-token retrieval. In packages/onedrive/managed-webhook.test.ts lines 3-28, add tests mocking corsair/hub getManagedAccessToken and attachManagedRefreshAuth to verify the OneDrive keyBuilder returns the resolved token and rejects when ctx.hub is missing; add the equivalent coverage in packages/outlook/managed-webhook.test.ts lines 3-28 for the Outlook keyBuilder.packages/onedrive/index.ts (1)
912-930: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winManaged-auth key-builder wiring is duplicated across three Graph plugins. Each plugin repeats the same Hub-config check,
managedContextconstruction,getManagedAccessTokencall, andattachManagedRefreshAuthattachment, differing only by thepluginliteral (and, for SharePoint, an extrasource === 'endpoint'guard). Extract this into a shared helper incorsair/hub, consistent with howgetManagedAccessTokenandattachManagedRefreshAuthare already centralized there.
packages/onedrive/index.ts#L912-L930: replace this block with a call to the shared helper, passing'onedrive'as the plugin id.packages/outlook/index.ts#L679-L697: replace this block with a call to the shared helper, passing'outlook'as the plugin id.packages/sharepoint/index.ts#L1472-L1490: replace this block with a call to the shared helper, passing'sharepoint'as the plugin id, preserving thesource === 'endpoint'guard around the call site.♻️ Proposed shared helper (e.g. in corsair/hub)
export async function resolveManagedKeyBuilderToken( pluginId: string, ctx: { hub?: unknown; keys: unknown; tenantId?: unknown }, ): Promise<string> { if (!ctx.hub) { throw new Error( `[auth-missing:${pluginId}:managed]: Hub config is required for managed auth. Pass hub: { ... } to createCorsair().`, ); } const managedContext = { keys: ctx.keys, hub: ctx.hub, plugin: pluginId, tenantId: ctx.tenantId, }; const result = await getManagedAccessToken(managedContext); await attachManagedRefreshAuth(ctx as Record<string, unknown>, managedContext); return result.accessToken; }🤖 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/onedrive/index.ts` around lines 912 - 930, The managed-auth flow is duplicated across the Graph plugins. Add a shared helper in corsair/hub that validates hub configuration, builds the managed context, resolves the token, attaches refresh auth, and returns the access token; then replace the blocks in packages/onedrive/index.ts lines 912-930 and packages/outlook/index.ts lines 679-697 with calls using their respective plugin IDs, and replace packages/sharepoint/index.ts lines 1472-1490 with the helper call using “sharepoint” while preserving the existing source === 'endpoint' guard.
🤖 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/dropbox/index.ts`:
- Around line 304-306: Update the webhook authentication check around authType
in the Dropbox integration to report the active authentication type when
throwing AuthMissingError, including managed instead of always naming oauth_2.
Preserve the existing error path for missing webhookSecret and align its message
behavior with the corresponding Zoho Mail implementation.
In `@packages/linear/index.ts`:
- Around line 525-541: Update the Linear request flow around
makeLinearRequest(ctx.key) to add a 401 retry path that invokes
ctx._refreshAuth, obtains the refreshed token, and retries the request using
Bearer ${freshToken}. Ensure both managed and OAuth2 authentication paths use
this wrapper instead of calling makeLinearRequest directly, while preserving the
existing initial-token behavior.
In `@packages/sharepoint/index.ts`:
- Around line 1468-1471: Update the comment above the source check near the
managed endpoint calls to state that webhook handling is already terminal and
the source condition is retained only as defensive belt-and-suspenders
protection. Remove the outdated explanation about webhook fall-through, while
preserving the existing source check and endpoint behavior.
In `@packages/zohomail/index.ts`:
- Around line 346-350: Update the managed branch in the webhook authentication
flow to allow managed connections to resolve webhook_signature, removing or
bypassing the error thrown for ctx.authType === 'managed'. Preserve webhook key
resolution for the handshake and subsequent managed deliveries, including the
persisted x-hook-secret from set_webhook_signature.
---
Nitpick comments:
In `@packages/asana/index.ts`:
- Around line 1104-1121: Extract the shared managed-auth flow into an exported
corsair/hub helper such as resolveManagedKey(ctx, pluginId), consolidating hub
validation, managed context construction, getManagedAccessToken,
attachManagedRefreshAuth, and token return with consistent errors. Replace the
managed branches in packages/asana/index.ts (1104-1121),
packages/dropbox/index.ts (385-401), packages/gitlab/index.ts (922-939),
packages/notion/index.ts (404-421), packages/slack/index.ts (796-813),
packages/spotify/index.ts (613-630), and packages/zohomail/index.ts (439-456)
with the corresponding helper call; move the packages/hubspot/index.ts (689-706)
call inside its source === 'endpoint' block. Also update
packages/linear/index.ts (525-541) to use the helper while preserving its Bearer
prefix.
In `@packages/dropbox/index.ts`:
- Around line 385-401: Replace the duplicated managed-auth handling in the
Dropbox flow with the shared helper proposed for the other plugins. Update the
branch around getManagedAccessToken and attachManagedRefreshAuth to delegate
context validation, token retrieval, refresh attachment, and access-token return
to that helper while preserving the existing Dropbox managed-auth behavior.
In `@packages/gitlab/index.ts`:
- Around line 922-939: The managed-auth branch in the GitLab flow duplicates the
shared implementation used by other plugins. Replace the inline managedContext
construction, getManagedAccessToken call, refresh attachment, and access-token
return in the surrounding GitLab handler with the consolidated shared
managed-auth path, preserving the existing hub validation and GitLab-specific
context values.
In `@packages/hubspot/index.ts`:
- Around line 689-706: The managed authentication branch around
getManagedAccessToken and attachManagedRefreshAuth should be nested within the
source === 'endpoint' conditional alongside the api_key and oauth_2 branches.
Preserve its existing hub validation, token refresh setup, and access-token
return behavior while making the endpoint-source gate explicit.
In `@packages/notion/index.ts`:
- Around line 404-421: The managed-auth branch in the Notion integration
duplicates the shared implementation used by other plugins. Replace the inline
logic in the managed branch around getManagedAccessToken and
attachManagedRefreshAuth with the established shared helper or consolidated
flow, preserving hub validation and returning the managed access token.
In `@packages/onedrive/index.ts`:
- Around line 912-930: The managed-auth flow is duplicated across the Graph
plugins. Add a shared helper in corsair/hub that validates hub configuration,
builds the managed context, resolves the token, attaches refresh auth, and
returns the access token; then replace the blocks in packages/onedrive/index.ts
lines 912-930 and packages/outlook/index.ts lines 679-697 with calls using their
respective plugin IDs, and replace packages/sharepoint/index.ts lines 1472-1490
with the helper call using “sharepoint” while preserving the existing source ===
'endpoint' guard.
In `@packages/onedrive/managed-webhook.test.ts`:
- Around line 3-28: The managed webhook tests cover only stored clientState and
omit endpoint managed-token retrieval. In
packages/onedrive/managed-webhook.test.ts lines 3-28, add tests mocking
corsair/hub getManagedAccessToken and attachManagedRefreshAuth to verify the
OneDrive keyBuilder returns the resolved token and rejects when ctx.hub is
missing; add the equivalent coverage in packages/outlook/managed-webhook.test.ts
lines 3-28 for the Outlook keyBuilder.
In `@packages/slack/index.ts`:
- Around line 796-813: Replace the duplicated managed-auth branch in the Slack
flow with the shared managed-auth handling used by the other plugins. Reuse the
existing helper or consolidated abstraction for validating ctx.hub, building
managedContext, attaching refresh auth, and returning the access token, while
preserving Slack’s plugin and tenant context.
In `@packages/spotify/index.ts`:
- Around line 613-630: Consolidate the duplicated managed-auth branch in the
Spotify integration by reusing the shared managed authentication helper used by
the other plugins. Update the flow around getManagedAccessToken and
attachManagedRefreshAuth while preserving hub validation, managedContext values,
refresh-auth attachment, and access-token return behavior.
In `@packages/zohomail/index.ts`:
- Around line 439-456: Replace the duplicated managed-auth branch in the
zohomail flow with the shared managed-auth helper used by the other plugins,
preserving the existing hub validation, managed context values, refresh-auth
attachment, and access-token return behavior.
🪄 Autofix (Beta)
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: 223a32d6-b17a-4784-9229-4bc46627ded9
📒 Files selected for processing (27)
packages/asana/index.tspackages/corsair/hub/managed-oauth.tspackages/corsair/oauth/renewal.tspackages/corsair/tests/managed-oauth-report.test.tspackages/corsair/tests/subscription-renewal.test.tspackages/dropbox/index.tspackages/gitlab/index.tspackages/hubspot/index.tspackages/linear/index.tspackages/notion/index.tspackages/onedrive/index.tspackages/onedrive/jest.config.cjspackages/onedrive/managed-webhook.test.tspackages/outlook/index.tspackages/outlook/jest.config.cjspackages/outlook/managed-webhook.test.tspackages/sharepoint/index.tspackages/sharepoint/jest.config.cjspackages/sharepoint/managed-webhook.test.tspackages/sharepoint/webhooks/list-changes.tspackages/sharepoint/webhooks/types.tspackages/slack/index.tspackages/spotify/index.tspackages/teams/index.tspackages/teams/jest.config.cjspackages/teams/managed-webhook.test.tspackages/zohomail/index.ts
Comment on lines
+525
to
+541
| if (ctx.authType === 'managed') { | ||
| if (!ctx.hub) { | ||
| throw new Error( | ||
| '[auth-missing:linear:managed]: Hub config is required for managed auth. Pass hub: { ... } to createCorsair().', | ||
| ); | ||
| } | ||
|
|
||
| const managedContext = { | ||
| keys: ctx.keys, | ||
| hub: ctx.hub, | ||
| plugin: 'linear', | ||
| tenantId: ctx.tenantId, | ||
| }; | ||
|
|
||
| const result = await getManagedAccessToken(managedContext); | ||
| await attachManagedRefreshAuth(ctx, managedContext); | ||
| return `Bearer ${result.accessToken}`; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the consumer of _refreshAuth and check how the returned value is applied.
rg -nP --type=ts -C6 '_refreshAuth' packages/corsairRepository: corsairdev/corsair
Length of output: 1373
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- all _refreshAuth references ---'
rg -n -C8 --glob '*.{ts,tsx,js,jsx}' '_refreshAuth' .
printf '%s\n' '--- managed auth implementation ---'
cat -n packages/corsair/hub/managed-auth.ts | sed -n '1,180p'
printf '%s\n' '--- Linear keyBuilder context and related auth paths ---'
cat -n packages/linear/index.ts | sed -n '470,555p'
printf '%s\n' '--- auth retry and request-header code candidates ---'
rg -n -C8 --glob '*.{ts,tsx,js,jsx}' '401|retry|Authorization|keyBuilder|authType' packages/corsair packages/linearRepository: corsairdev/corsair
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- exact _refreshAuth source lines ---'
python3 - <<'PY'
from pathlib import Path
for path in Path('.').rglob('*'):
if not path.is_file() or any(part in {'node_modules', '.git', 'dist', 'build'} for part in path.parts):
continue
try:
text = path.read_text()
except (UnicodeDecodeError, OSError):
continue
for number, line in enumerate(text.splitlines(), 1):
if '_refreshAuth' in line:
print(f'{path}:{number}:{line}')
PY
printf '%s\n' '--- endpoint binding retry flow ---'
cat -n packages/corsair/core/endpoints/bind.ts | sed -n '150,275p'
printf '%s\n' '--- key builder type declarations ---'
rg -n -C10 'CorsairKeyBuilder|KeyBuilder' packages/corsair packages/linear --glob '*.ts' | head -n 300
printf '%s\n' '--- Linear auth code ---'
cat -n packages/linear/index.ts | sed -n '490,550p'
printf '%s\n' '--- managed auth code ---'
cat -n packages/corsair/hub/managed-auth.ts | sed -n '105,145p'Repository: corsairdev/corsair
Length of output: 35740
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Linear refresh consumer ---'
rg -n -C12 '_refreshAuth|Authorization|Bearer|fetch' packages/linear/client.ts packages/linear --glob '*.ts' | head -n 400
printf '%s\n' '--- representative refresh consumers ---'
for file in packages/googlecalendar/client.ts packages/dropbox/client.ts packages/googlesheets/client.ts packages/github/client.ts; do
if [ -f "$file" ]; then
echo "--- $file ---"
rg -n -C8 '_refreshAuth|Authorization|Bearer' "$file"
fi
done
printf '%s\n' '--- context passed to endpoint calls ---'
cat -n packages/corsair/core/endpoints/bind.ts | sed -n '255,345p'Repository: corsairdev/corsair
Length of output: 21214
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- all Linear client call sites ---'
rg -n -C6 'makeLinearRequest|makeAuthenticated|ctx\.key|LinearAuthContext' packages/linear --glob '*.ts'
printf '%s\n' '--- complete Linear client request helpers ---'
cat -n packages/linear/client.ts | sed -n '100,180p'
printf '%s\n' '--- Linear endpoint request usage ---'
rg -n -C10 'makeLinear|ctx\.key|client' packages/linear/endpoints packages/linear --glob '*.ts' | head -n 500Repository: corsairdev/corsair
Length of output: 50374
Wire Linear requests through a 401 refresh path.
Linear endpoints call makeLinearRequest(ctx.key) directly, so no code reads ctx._refreshAuth. Managed and OAuth2 requests do not retry after a 401. Add a retry wrapper and pass the refreshed token as Bearer ${freshToken}.
🤖 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/linear/index.ts` around lines 525 - 541, Update the Linear request
flow around makeLinearRequest(ctx.key) to add a 401 retry path that invokes
ctx._refreshAuth, obtains the refreshed token, and retries the request using
Bearer ${freshToken}. Ensure both managed and OAuth2 authentication paths use
this wrapper instead of calling makeLinearRequest directly, while preserving the
existing initial-token behavior.
Comment on lines
+346
to
+350
| if (ctx.authType === 'managed') { | ||
| throw new Error( | ||
| '[auth-missing:zohomail:managed]: webhook signature is not available in managed mode', | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether webhook_signature is written for managed connections.
rg -nP --type=ts -C5 'set_webhook_signature|x-hook-secret' packagesRepository: corsairdev/corsair
Length of output: 30245
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- relevant zohomail implementation ---'
sed -n '300,380p' packages/zohomail/index.ts
printf '%s\n' '--- handshake handler ---'
cat -n packages/zohomail/webhooks/challenge.ts
printf '%s\n' '--- authType and key-manager handling ---'
rg -n -C5 --type=ts 'authType|managed|webhook_signature|get_webhook_signature|set_webhook_signature' packages/zohomail packages/corsair/core packages/corsairRepository: corsairdev/corsair
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- managed Zoho webhook coverage ---'
rg -n -C4 --type=ts 'zohomail|authType: .managed.|webhookSecret|keyBuilder|source === .webhook.' packages/corsair packages/zohomail --glob '*test.ts' --glob '*.ts' | head -n 500
printf '%s\n' '--- webhook dispatch and keyBuilder call sites ---'
rg -n -C6 --type=ts 'keyBuilder\\(|source: .webhook.|source === .webhook|build.*Key|plugin\\.keyBuilder' packages/corsair/core packages/corsair/hub
printf '%s\n' '--- managed delivery paths ---'
rg -n -C6 --type=ts 'processManagedOAuthDelivery|managed.*webhook|webhook.*managed|deliveryUrl|authType === .managed.' packages/corsairRepository: corsairdev/corsair
Length of output: 31346
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- keyBuilder call sites ---'
rg -n -C5 --type=ts 'keyBuilder|source.*webhook|source.*endpoint' packages/corsair/core packages/corsair/hub | head -n 600
printf '%s\n' '--- webhook request processing ---'
rg -n -C5 --type=ts 'pluginWebhookMatcher|pluginTenantWebhookMatcher|webhook.*key|key.*webhook|processWebhook|dispatchWebhook' packages/corsair/core packages/corsair/hub packages/corsair | head -n 800
printf '%s\n' '--- managed hub delivery implementation ---'
sed -n '1,230p' packages/corsair/hub/managed-oauth.tsRepository: corsairdev/corsair
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
index = Path("packages/zohomail/index.ts").read_text()
challenge = Path("packages/zohomail/webhooks/challenge.ts").read_text()
auth_types = Path("packages/corsair/core/auth/types.ts").read_text()
bind = Path("packages/corsair/core/webhooks/bind.ts").read_text()
managed = re.search(r"managed:\s*\{\s*integration:\s*\[\],\s*account:\s*\[(.*?)\]\s*as const", auth_types, re.S)
assert managed, "managed auth fields not found"
managed_fields = managed.group(1)
assert "webhook_signature" in managed_fields
assert "await ctx.keys.set_webhook_signature(hookSecret)" in challenge
assert re.search(
r"if \(source === 'webhook'\) \{\s*"
r"if \(ctx\.authType === 'managed'\) \{\s*"
r"throw new Error\(\s*"
r"\'[^\']*webhook signature is not available in managed mode",
index,
re.S,
)
assert "keyBuilder(ctx, 'webhook')" in bind
print("managed account fields include webhook_signature: yes")
print("Zoho handshake persists x-hook-secret through set_webhook_signature: yes")
print("webhook binding invokes keyBuilder with source='webhook': yes")
print("Zoho keyBuilder throws for authType='managed' before get_webhook_signature: yes")
print("Conclusion: a managed handshake can store webhook_signature, but subsequent bound webhook handling rejects managed auth before reading it.")
PYRepository: corsairdev/corsair
Length of output: 276
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
index = Path("packages/zohomail/index.ts").read_text()
challenge = Path("packages/zohomail/webhooks/challenge.ts").read_text()
auth_types = Path("packages/corsair/core/auth/types.ts").read_text()
bind = Path("packages/corsair/core/webhooks/bind.ts").read_text()
start = auth_types.index("\tmanaged:")
end = auth_types.index("\n\t},", start)
managed_block = auth_types[start:end]
assert "webhook_signature" in managed_block, managed_block
assert "await ctx.keys.set_webhook_signature(hookSecret)" in challenge
assert "if (ctx.authType === 'managed')" in index
guard_start = index.index("if (ctx.authType === 'managed')")
guard_end = index.index("\n\t\t\t\t}", guard_start) + len("\n\t\t\t\t}")
guard = index[guard_start:guard_end]
assert "webhook signature is not available in managed mode" in guard
assert "keyBuilder(ctx, 'webhook')" in bind
print("managed auth block:")
print(managed_block)
print("managed account fields include webhook_signature: yes")
print("Zoho handshake persists x-hook-secret through set_webhook_signature: yes")
print("webhook binding invokes keyBuilder with source='webhook': yes")
print("Zoho keyBuilder throws for authType='managed' before get_webhook_signature: yes")
print("Conclusion: the guard rejects a managed webhook after the handshake stores the signature.")
PYRepository: corsairdev/corsair
Length of output: 694
Allow managed connections to resolve webhook_signature. The managed account fields include webhook_signature, and the handshake persists x-hook-secret through set_webhook_signature. Webhook binding calls keyBuilder before the handler, so this guard rejects the handshake and all later managed deliveries. Remove the guard or bypass key resolution for the handshake.
🤖 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/zohomail/index.ts` around lines 346 - 350, Update the managed branch
in the webhook authentication flow to allow managed connections to resolve
webhook_signature, removing or bypassing the error thrown for ctx.authType ===
'managed'. Preserve webhook key resolution for the handshake and subsequent
managed deliveries, including the persisted x-hook-secret from
set_webhook_signature.
Arm the provider subscription at managed connect + renewal, and verify inbound events via the stored clientState — outlook, onedrive, teams, sharepoint. Reuses the #469 BYO engine; managed just resolves the token from the Hub.
yuvrxj-afk
force-pushed
the
managed-webhooks
branch
from
August 3, 2026 11:05
20ef459 to
a973a31
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/dropbox/index.ts (1)
390-408: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the managed key-generation block into a shared helper. Both files re-implement the same sequence: check
ctx.hub, buildmanagedContext, callgetManagedAccessToken, callattachManagedRefreshAuth, and return the access token. Move this into a shared helper incorsair/hub(for exampleresolveManagedAccessToken(ctx, plugin)) that plugins call with their plugin name and any plugin-specific pre-checks (such as GitLab's host restriction) kept local.
packages/dropbox/index.ts#L390-L408: replace the inline hub-check/managedContext/getManagedAccessToken/attachManagedRefreshAuth sequence with a call to the shared helper, passing'dropbox'as the plugin name.packages/gitlab/index.ts#L927-L952: keep the gitlab.com host restriction as a local pre-check, then replace the remaining hub-check/managedContext/getManagedAccessToken/attachManagedRefreshAuth sequence with a call to the same shared helper, passing'gitlab'as the plugin name.♻️ Proposed shared helper sketch
// packages/corsair/hub/managed-auth.ts export async function resolveManagedAccessToken( ctx: { hub?: unknown; keys: unknown; tenantId?: string } & Record<string, unknown>, plugin: string, ): Promise<string> { if (!ctx.hub) { throw new Error( `[auth-missing:${plugin}:managed]: Hub config is required for managed auth. Pass hub: { ... } to createCorsair().`, ); } const managedContext = { keys: ctx.keys, hub: ctx.hub, plugin, tenantId: ctx.tenantId, }; const result = await getManagedAccessToken(managedContext); await attachManagedRefreshAuth(ctx, managedContext); return result.accessToken; }🤖 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/dropbox/index.ts` around lines 390 - 408, Extract the shared managed-auth sequence into a helper such as resolveManagedAccessToken in packages/corsair/hub, preserving the hub validation, managed context construction, token retrieval, refresh attachment, and access-token return. In packages/dropbox/index.ts lines 390-408, replace the inline sequence with the helper using 'dropbox'; in packages/gitlab/index.ts lines 927-952, retain the gitlab.com host pre-check locally and replace the remaining sequence with the helper using 'gitlab'.
🤖 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.
Nitpick comments:
In `@packages/dropbox/index.ts`:
- Around line 390-408: Extract the shared managed-auth sequence into a helper
such as resolveManagedAccessToken in packages/corsair/hub, preserving the hub
validation, managed context construction, token retrieval, refresh attachment,
and access-token return. In packages/dropbox/index.ts lines 390-408, replace the
inline sequence with the helper using 'dropbox'; in packages/gitlab/index.ts
lines 927-952, retain the gitlab.com host pre-check locally and replace the
remaining sequence with the helper using 'gitlab'.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b53dbd96-24b5-4255-bc1a-5d3feb1828e6
📒 Files selected for processing (36)
packages/asana/index.tspackages/asana/jest.config.cjspackages/corsair/hub/managed-oauth.tspackages/corsair/oauth/renewal.tspackages/corsair/tests/managed-oauth-report.test.tspackages/corsair/tests/subscription-renewal.test.tspackages/dropbox/index.tspackages/dropbox/jest.config.cjspackages/gitlab/index.tspackages/gitlab/jest.config.cjspackages/hubspot/index.tspackages/hubspot/jest.config.cjspackages/linear/index.tspackages/linear/jest.config.cjspackages/notion/index.tspackages/notion/jest.config.cjspackages/onedrive/index.tspackages/onedrive/jest.config.cjspackages/onedrive/managed-webhook.test.tspackages/outlook/index.tspackages/outlook/jest.config.cjspackages/outlook/managed-webhook.test.tspackages/sharepoint/index.tspackages/sharepoint/jest.config.cjspackages/sharepoint/managed-webhook.test.tspackages/sharepoint/webhooks/list-changes.tspackages/sharepoint/webhooks/types.tspackages/slack/index.tspackages/slack/jest.config.cjspackages/spotify/index.tspackages/spotify/jest.config.cjspackages/teams/index.tspackages/teams/jest.config.cjspackages/teams/managed-webhook.test.tspackages/zohomail/index.tspackages/zohomail/jest.config.cjs
🚧 Files skipped from review as they are similar to previous changes (25)
- packages/teams/jest.config.cjs
- packages/sharepoint/managed-webhook.test.ts
- packages/spotify/index.ts
- packages/notion/index.ts
- packages/teams/managed-webhook.test.ts
- packages/corsair/oauth/renewal.ts
- packages/sharepoint/index.ts
- packages/linear/index.ts
- packages/sharepoint/webhooks/list-changes.ts
- packages/outlook/managed-webhook.test.ts
- packages/asana/index.ts
- packages/corsair/hub/managed-oauth.ts
- packages/sharepoint/jest.config.cjs
- packages/hubspot/index.ts
- packages/onedrive/jest.config.cjs
- packages/sharepoint/webhooks/types.ts
- packages/slack/index.ts
- packages/outlook/jest.config.cjs
- packages/teams/index.ts
- packages/outlook/index.ts
- packages/corsair/tests/subscription-renewal.test.ts
- packages/onedrive/managed-webhook.test.ts
- packages/corsair/tests/managed-oauth-report.test.ts
- packages/onedrive/index.ts
- packages/zohomail/index.ts
Collaborator
Author
| } | ||
|
|
||
| const clientState = ctx.options?.webhookClientState; | ||
| const clientState = ctx.key; |
Contributor
There was a problem hiding this comment.
When SharePoint sends multiple notifications in one request, this call accepts the entire batch after the verifier checks only value[0].clientState, so notifications later in the batch with missing or mismatched client state reach the handler as authenticated events.
How this was verified: verifySharepointWebhookSignature compares only the first notification against ctx.key and does not validate the remaining entries.
|
Hey @yuvrxj-afk, thanks for the contribution! 🏴☠️ Before a maintainer reviews, please fix the items below — the review re-runs automatically on your next push. Must fix
How this was verified: PR requirements (rules)
If anything remains after your next push, a bot commit will clean it up; a maintainer always does the final review and merge. |
devjain32
approved these changes
Aug 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extends #469's BYO webhook machine to managed connections on the Graph plugins (outlook, onedrive, teams, sharepoint). For managed the token lives in the Hub, so the subscribe engines never ran; this arms the subscription at connect and renewal by resolving the token from the Hub, then verifies inbound events via the stored clientState.
processManagedOAuthDeliveryfiressubscribeAndReportafter storing the delivered token (best-effort — a subscribe failure never breaks the connection).renewAccountsprimes each row by its plugin authType and passeshub, so the managed keyBuilder branch refreshes the token from the Hub before re-subscribe.webhook_signaturelike oauth_2 (the managed guards are removed); sharepoint reads the stored clientState, fail-closed.Also adds the
corsair/hubjest moduleNameMapper the four plugin configs were missing since #571 (any test loading theirindex.tsfailed to resolvecorsair/hubwithout it).Stacks on #571 (managed authType declaration). Rebases to a clean diff once that merges.
Tests: renewal (managed prime threads authType + hub, expiry forced), connect subscribe (fires after token store + failure swallowed), per-plugin webhook verify + fail-closed. Typecheck clean across corsair + the four plugins.
Summary by CodeRabbit