Skip to content

fix(db): secure exist_app_versions RPC#2061

Merged
riderx merged 4 commits intomainfrom
codex/fix-exist-app-versions-rpc
May 7, 2026
Merged

fix(db): secure exist_app_versions RPC#2061
riderx merged 4 commits intomainfrom
codex/fix-exist-app-versions-rpc

Conversation

@riderx
Copy link
Copy Markdown
Member

@riderx riderx commented May 7, 2026

Summary (AI generated)

  • Secures both exist_app_versions RPC overloads with service-role bypass plus RBAC/API-key authorization before returning bundle existence.
  • Fixes check_min_rights so direct apps RLS respects explicit API-key RBAC bindings instead of falling back to broader owner-user permissions.
  • Removes the unused CLI getAccessibleAppsForApiKey helper and drops get_accessible_apps_for_apikey_v2; the CLI now relies on the corrected apps RLS path.
  • Adds pgTAP regression coverage for anonymous/version checks and restricted API-key app-list RLS.

Motivation (AI generated)

exist_app_versions was an unauthenticated oracle for release-history enumeration. While reviewing the CLI app-list path, the API-key RBAC restriction belonged in check_min_rights itself so direct table RLS stays correct and no app-list wrapper RPC is needed.

Business Impact (AI generated)

This protects customer release timelines and security-fix history from public enumeration, and keeps restricted API keys restricted consistently across CLI and direct table access.

Test Plan (AI generated)

  • bun lint
  • bun typecheck
  • bun run cli:check
  • bun scripts/supabase-worktree.ts test db supabase/tests/00-supabase_test_helpers.sql supabase/tests/05_app_functions.sql supabase/tests/33_test_rbac_phase1.sql
  • Local REST check: anon request without capgkey returns false
  • Local REST check: request with valid capgkey returns true

Generated with AI

Summary by CodeRabbit

  • Chores

    • Test fixture version bumped to 6.45.10.
    • Removed a legacy RPC and corresponding client helpers/types.
  • Bug Fixes

    • Tightened DB-side authorization and RBAC checks for app version access.
    • Normalized API‑key handling and strengthened permission enforcement.
  • Tests

    • Expanded authorization and RBAC test coverage, including API‑key and anonymous scenarios.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 7, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

Adds RBAC-gated authorization for app-version checks via new SECURITY DEFINER functions, removes a deprecated API-key RPC and its CLI/type usages, updates DB tests to cover API-key scoping, and pins test fixture package version to 6.45.10.

Changes

exist_app_versions RBAC + cleanup

Layer / File(s) Summary
DB Authorization Primitives
supabase/migrations/20260507091347_secure_exist_app_versions_rpc.sql
Adds public.check_min_rights(min_right, user_id, org_id, app_id, channel_id) SECURITY DEFINER enforcing org ownership, optional org-level 2FA/password checks, and routing to legacy or RBAC paths.
exist_app_versions Implementation
supabase/migrations/20260507091347_secure_exist_app_versions_rpc.sql
Adds two overloads: 2-arg wrapper that forwards API-key header and 3-arg implementation that resolves owning org, bypasses internal roles, normalizes user/API-key context, enforces rbac_check_permission_direct(public.rbac_perm_app_read_bundles(), ...), and checks public.app_versions existence.
RPC Permission Management
supabase/migrations/20260507091347_secure_exist_app_versions_rpc.sql
Sets ownership to postgres, revokes PUBLIC execute, grants EXECUTE to anon/authenticated/service_role (per-overload granularity), and drops public.get_accessible_apps_for_apikey_v2(apikey text) if present.
Type Removals
cli/src/types/supabase.types.ts, src/types/supabase.types.ts, supabase/functions/_backend/utils/supabase.types.ts
Removes get_accessible_apps_for_apikey_v2 RPC type declarations. Note: src/types/supabase.types.ts also shows stray numeric tokens inside find_fit_plan_v3 return type (likely generation/merge artifact) requiring verification.
CLI Utility Removal
cli/src/utils.ts
Removes getAccessibleAppsForApiKey helper that called the removed RPC.
Authorization Tests
supabase/tests/05_app_functions.sql
Increases TAP plan (12→16) and adds exist_app_versions authorization assertions simulating empty headers, header capgkey, valid API-key arg, and invalid API-key arg.
RBAC Scope Isolation Tests
supabase/tests/33_test_rbac_phase1.sql
Adds restricted apikey rbac-test-restricted-key-phase1 and a role_bindings entry binding that apikey to the org with org_member; adds assertions verifying restricted apikey sees 0 rows while original key sees 1.
Test Fixture Version Pinning
cli/test/fixtures/setup-test-projects.sh
Updates PACKAGE_VERSION from 6.25.5 to 6.45.10 and uses it when generating fixture package.json files; updates version-mismatch trap comment to reflect installed-fixture vs claimed-version scenario.
sequenceDiagram
  participant Client as Caller
  participant Rpc as exist_app_versions()
  participant RBAC as rbac_check_permission_direct
  participant Apps as public.apps / public.app_versions
  Client->>Rpc: call exist_app_versions(appid, name_version[, apikey])
  Rpc->>Apps: resolve owning org (apps)
  alt not found
    Rpc-->>Client: return false
  else found
    Rpc->>RBAC: check permission (bundle-read) using user or apikey context
    alt allowed
      Rpc->>Apps: query app_versions for (org, appid, name_version)
      Rpc-->>Client: return exists? (true/false)
    else denied
      Rpc-->>Client: return false
    end
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • Cap-go/capgo#1974: Touches related authorization helpers and API-key header flows used by the new check_min_rights/get_apikey_header patterns.
  • Cap-go/capgo#1967: Also modifies check_min_rights and SECURITY DEFINER patterns; closely related to the RPC authorization changes.
  • Cap-go/capgo#1983: Involves the get_accessible_apps_for_apikey_v2 RPC and CLI usage that this PR removes, making it directly relevant.

Poem

🐰 I nibbled through headers, hops so spry,
RBAC fences raised up high,
Old RPCs are put to rest,
Fixtures pinned — the garden's dressed,
Hooray — secure apples for every eye!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: securing the exist_app_versions RPC function.
Description check ✅ Passed The PR description includes AI-generated Summary, Motivation, and Business Impact sections, plus a detailed Test Plan with passing checkmarks, exceeding the template requirements for backend changes.
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-exist-app-versions-rpc

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 SQLFluff (4.1.0)
supabase/tests/05_app_functions.sql

User Error: No dialect was specified. You must configure a dialect or specify one on the command line using --dialect after the command. Available dialects:
ansi, athena, bigquery, clickhouse, databricks, db2, doris, duckdb, exasol, flink, greenplum, hive, impala, mariadb, materialize, mysql, oracle, postgres, redshift, snowflake, soql, sparksql, sqlite, starrocks, teradata, trino, tsql, vertica

supabase/migrations/20260507091347_secure_exist_app_versions_rpc.sql

User Error: No dialect was specified. You must configure a dialect or specify one on the command line using --dialect after the command. Available dialects:
ansi, athena, bigquery, clickhouse, databricks, db2, doris, duckdb, exasol, flink, greenplum, hive, impala, mariadb, materialize, mysql, oracle, postgres, redshift, snowflake, soql, sparksql, sqlite, starrocks, teradata, trino, tsql, vertica

supabase/tests/33_test_rbac_phase1.sql

User Error: No dialect was specified. You must configure a dialect or specify one on the command line using --dialect after the command. Available dialects:
ansi, athena, bigquery, clickhouse, databricks, db2, doris, duckdb, exasol, flink, greenplum, hive, impala, mariadb, materialize, mysql, oracle, postgres, redshift, snowflake, soql, sparksql, sqlite, starrocks, teradata, trino, tsql, vertica


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 7, 2026

Merging this PR will not alter performance

✅ 28 untouched benchmarks


Comparing codex/fix-exist-app-versions-rpc (5ec1de6) with main (b44a7a2)

Open in CodSpeed

@riderx riderx marked this pull request as ready for review May 7, 2026 10:29
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0114d48268

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@riderx riderx force-pushed the codex/fix-exist-app-versions-rpc branch from 0114d48 to 5ec1de6 Compare May 7, 2026 11:51
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@cli/test/fixtures/setup-test-projects.sh`:
- Line 10: The PACKAGE_VERSION variable in setup-test-projects.sh is set to a
non-existent npm version ("6.45.10") which will cause installs to fail under set
-e; update the PACKAGE_VERSION value to an existing published version (for
example "6.43.5") or coordinate/release 6.45.10 before merging so that the
install commands that reference PACKAGE_VERSION succeed; locate and edit the
PACKAGE_VERSION="6.45.10" assignment in setup-test-projects.sh to the chosen
valid version.
🪄 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

Run ID: e13d7fd8-7583-457b-b354-b3d8b1cfbb62

📥 Commits

Reviewing files that changed from the base of the PR and between 0114d48 and 5ec1de6.

📒 Files selected for processing (8)
  • cli/src/types/supabase.types.ts
  • cli/src/utils.ts
  • cli/test/fixtures/setup-test-projects.sh
  • src/types/supabase.types.ts
  • supabase/functions/_backend/utils/supabase.types.ts
  • supabase/migrations/20260507091347_secure_exist_app_versions_rpc.sql
  • supabase/tests/05_app_functions.sql
  • supabase/tests/33_test_rbac_phase1.sql
💤 Files with no reviewable changes (4)
  • cli/src/utils.ts
  • cli/src/types/supabase.types.ts
  • src/types/supabase.types.ts
  • supabase/functions/_backend/utils/supabase.types.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • supabase/migrations/20260507091347_secure_exist_app_versions_rpc.sql

Comment thread cli/test/fixtures/setup-test-projects.sh
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 7, 2026

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