Skip to content

[codex] fix webhook API key scope#2016

Open
riderx wants to merge 1 commit intomainfrom
codex/fix-webhook-api-key-scope
Open

[codex] fix webhook API key scope#2016
riderx wants to merge 1 commit intomainfrom
codex/fix-webhook-api-key-scope

Conversation

@riderx
Copy link
Copy Markdown
Member

@riderx riderx commented May 2, 2026

Summary (AI generated)

  • Enforce app-scope rejection before org-scoped webhook authorization.
  • Validate both parent API keys and x-limited-key-id subkeys on webhook org actions.
  • Add regression coverage for webhook listing, test delivery, and delivery retry.

Motivation (AI generated)

GHSA-qvr7-f6j6-64wp showed that app-scoped API keys could trigger org-scoped webhook test deliveries because limited_to_apps was not treated as incompatible with organization webhook actions.

Business Impact (AI generated)

This restores API key scope isolation for organization webhook operations and prevents lower-scope credentials from triggering signed outbound webhook traffic for broader org resources.

Test Plan (AI generated)

  • Run bun run lint:backend
  • Run bunx eslint tests/webhooks.test.ts
  • Run bun typecheck
  • Run git diff --check
  • Run bun run supabase:with-env -- bunx vitest run tests/webhooks.test.ts in CI because local Docker is unavailable here

Generated with AI

Summary by CodeRabbit

  • Bug Fixes

    • Improved webhook permission validation to properly enforce API key scoping restrictions across all endpoints
    • App-scoped API keys are now correctly rejected when used in organization-level contexts
  • Tests

    • Added comprehensive test coverage for webhook permission scenarios with organization-scoped API keys

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 2, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3b0393a9-88fc-4511-8dc9-c8e888bf6d46

📥 Commits

Reviewing files that changed from the base of the PR and between 97ee8f0 and a396836.

📒 Files selected for processing (3)
  • supabase/functions/_backend/public/webhooks/index.ts
  • supabase/functions/_backend/utils/supabase.ts
  • tests/webhooks.test.ts

📝 Walkthrough

Walkthrough

Webhook permission validation is refactored to use reusable helpers that build and validate an API-key chain from parent and current keys, ensuring each key passes org webhook scope and org policy checks. A guard added to apikeyHasOrgRight rejects app-scoped keys. Tests verify that app-scoped keys paired with org-scoped subkeys are rejected across webhook endpoints.

Changes

API-Key Chain Permission Validation

Layer / File(s) Summary
Permission Guard Logic
supabase/functions/_backend/utils/supabase.ts
apikeyHasOrgRight returns false early when key is limited to apps, before org-level policy checks.
Core Refactoring
supabase/functions/_backend/public/webhooks/index.ts
Three helper functions added: uniqueApiKeys deduplicates by ID; getWebhookApiKeyChain and getWebhookAuthApiKeyChain construct key chains from parent and current keys; assertWebhookApiKeyChain validates each key in chain for org webhook scope and org policy. checkWebhookPermission and checkWebhookPermissionV2 replaced inline validation with shared chain-based validation calls.
Test Coverage
tests/webhooks.test.ts
Test setup creates org-scoped API subkey and cleans up after tests. Three new tests added to verify app-scoped keys with org-scoped subkeys are rejected with no_permission errors across GET /webhooks, POST /webhooks/test, and POST /webhooks/deliveries/retry endpoints.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 Hop along through permission chains,
Each key validated, none in vain,
App-scoped keys shall not mix and mingle,
With org-scoped subkeys, ever single!
The refactored guards now stand so tall, 🔐✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The PR description includes a summary and test plan, though it lacks the required checklist items and deviates from the template structure with AI-generated sections instead of following the standard format. Follow the provided description template more closely: use the standard checklist format, remove 'AI generated' markers, and clearly document which checklist items are completed with [x] marks.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title '[codex] fix webhook API key scope' directly summarizes the main change: fixing webhook API key scope validation to reject app-scoped keys.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-webhook-api-key-scope

Review rate limit: 0/5 reviews remaining, refill in 59 minutes and 17 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@codspeed-hq
Copy link
Copy Markdown
Contributor

codspeed-hq Bot commented May 2, 2026

Merging this PR will not alter performance

✅ 28 untouched benchmarks


Comparing codex/fix-webhook-api-key-scope (a396836) with main (17d36c6)1

Open in CodSpeed

Footnotes

  1. No successful run was found on main (97ee8f0) during the generation of this report, so 17d36c6 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@riderx riderx force-pushed the codex/fix-webhook-api-key-scope branch from 1a739ad to a396836 Compare May 2, 2026 14:07
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 2, 2026

@riderx riderx marked this pull request as ready for review May 2, 2026 14:20
@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@riderx
Copy link
Copy Markdown
Member Author

riderx commented May 2, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 2, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant