Skip to content

fix: enforce scoped subkey context on middlewareKey routes#1947

Merged
riderx merged 2 commits into
mainfrom
codex/fix-ghsa-2h89-vcvx-5pvh
Apr 24, 2026
Merged

fix: enforce scoped subkey context on middlewareKey routes#1947
riderx merged 2 commits into
mainfrom
codex/fix-ghsa-2h89-vcvx-5pvh

Conversation

@riderx
Copy link
Copy Markdown
Member

@riderx riderx commented Apr 24, 2026

Summary (AI generated)

  • make middlewareKey expose the effective subkey as the request API key context when x-limited-key-id is used
  • preserve the original parent key separately for flows that intentionally need parent-key policy checks
  • add a regression test covering the advisory path on /organization and /organization/members

Motivation (AI generated)

middlewareKey authenticated the parent key correctly, but downstream handlers still read the parent key from context after a scoped subkey was attached. That let scoped subkey requests behave like the unrestricted parent key on middlewareKey-protected routes.

Business Impact (AI generated)

This closes a high-severity authorization bypass in the public API surface. Scoped keys used in CI/CD, partner integrations, and delegated automation now respect their intended org limits, which reduces tenant-isolation risk and protects customer trust.

Test Plan (AI generated)

  • Run bunx eslint supabase/functions/_backend/utils/hono.ts supabase/functions/_backend/utils/hono_middleware.ts supabase/functions/_backend/public/webhooks/index.ts tests/organization-api.test.ts
  • Run bun run supabase:db:reset
  • Run bun run supabase:with-env -- bunx vitest run tests/organization-api.test.ts tests/webhooks-apikey-policy.test.ts

Generated with AI

Summary by CodeRabbit

  • Bug Fixes

    • Improved API key subkey authentication mechanism to properly enforce organization-level access scoping and permission boundaries when using subkeys with specific organization limits.
  • Tests

    • Added comprehensive test coverage validating that organization-scoped API key subkeys correctly restrict access and enforce permission boundaries across multiple organizations.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 24, 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: 918ccda2-1ad1-4372-8ab4-11a27af87af0

📥 Commits

Reviewing files that changed from the base of the PR and between 633aeea and 0b2ee3d.

📒 Files selected for processing (5)
  • supabase/functions/_backend/public/webhooks/index.ts
  • supabase/functions/_backend/utils/hono.ts
  • supabase/functions/_backend/utils/hono_middleware.ts
  • tests/app-error-cases.test.ts
  • tests/organization-api.test.ts

📝 Walkthrough

Walkthrough

The changes implement parent API key and subkey context management in the Hono middleware layer. A new parentApikey field is added to the context type, the middleware now stores the parent key separately and writes the active subkey into the apikey context field, and the webhook handler uses this distinction for policy-based permission checks.

Changes

Cohort / File(s) Summary
Middleware Context Type
supabase/functions/_backend/utils/hono.ts
Added optional parentApikey field to Hono middleware context variables to track parent API key rows separately from active keys.
Middleware Auth Logic
supabase/functions/_backend/utils/hono_middleware.ts
Updated auth context setup to store parent API key in context, write active subkey into context's apikey field, and conditionally update capgkey with subkey-specific values for downstream policy checks.
Webhook Permission Handler
supabase/functions/_backend/public/webhooks/index.ts
Modified checkWebhookPermissionV2 to derive parentApikey from the new context field instead of the raw apikey.
Test Suite Updates
tests/app-error-cases.test.ts
Updated test setup to fetch and use authorization headers upfront for API-key lifecycle operations in beforeAll/afterAll hooks.
Middleware Scoping Tests
tests/organization-api.test.ts
Added comprehensive test suite validating parent/subkey scoping behavior, verifying that subkey-limited requests only access permitted organizations.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Middleware as Hono Middleware
    participant Auth as Auth Context
    participant WebhookCheck as checkWebhookPermissionV2
    participant Policy as RLS Policy

    Client->>Middleware: Request with apikey + x-limited-key-id header
    activate Middleware
    Middleware->>Auth: Authenticate parent API key
    Auth-->>Middleware: Parent key row
    Middleware->>Middleware: Store parent in c.parentApikey
    Middleware->>Auth: Lookup subkey by x-limited-key-id
    Auth-->>Middleware: Subkey row
    Middleware->>Middleware: Write subkey to c.apikey<br/>Update c.capgkey with subkey secret
    deactivate Middleware
    
    Client->>WebhookCheck: Request webhook permission check
    activate WebhookCheck
    WebhookCheck->>WebhookCheck: Read parentApikey from context
    WebhookCheck->>Policy: Check org access using parentApikey
    Policy-->>WebhookCheck: Permission result
    WebhookCheck-->>Client: Permission granted/denied
    deactivate WebhookCheck
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly Related PRs

Suggested Labels

codex

Poem

🐰 Hops through the middleware tunnel with glee,
Parent keys and subkeys in harmony!
Context holds secrets, scoped just right,
Each rabbit-powered key has its might! 🔐✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title concisely describes the main change: enforcing scoped subkey context on middlewareKey routes, which directly reflects the security fix for authorization bypass.
Description check ✅ Passed The description includes a summary section, motivation, business impact, and a detailed test plan with specific commands executed. However, the checklist section is incomplete—most items lack checkmarks.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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-ghsa-2h89-vcvx-5pvh

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

@codspeed-hq
Copy link
Copy Markdown
Contributor

codspeed-hq Bot commented Apr 24, 2026

Merging this PR will not alter performance

✅ 28 untouched benchmarks


Comparing codex/fix-ghsa-2h89-vcvx-5pvh (0b2ee3d) with main (633aeea)

Open in CodSpeed

@sonarqubecloud
Copy link
Copy Markdown

@riderx riderx marked this pull request as ready for review April 24, 2026 09:59
@riderx
Copy link
Copy Markdown
Member Author

riderx commented Apr 24, 2026

@CodeRabbit review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 24, 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.

@riderx riderx merged commit ee782f3 into main Apr 24, 2026
16 checks passed
@riderx riderx deleted the codex/fix-ghsa-2h89-vcvx-5pvh branch April 24, 2026 12:48
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