Restrict public execution of API key oracle RPCs#1758
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughTightens RPC EXECUTE grants for API-key oracle functions, adds a SECURITY DEFINER helper for apps-bucket storage RLS, updates storage.objects RLS policies, and adds tests plus small test harness changes to authenticate as Changes
Sequence DiagramsequenceDiagram
participant Client
participant StorageRLS as Storage RLS Policy
participant Helper as can_access_apps_bucket_object()
participant Oracle as get_identity_apikey_only() / get_user_id()
participant Apps as public.apps
Client->>StorageRLS: request object operation (+ optional apikey header)
StorageRLS->>Helper: can_access_apps_bucket_object(keymode[], object_name)
alt authenticated (auth.uid() present)
Helper->>Helper: parse owner from object_name
Helper->>Client: compare owner == auth.uid() → return true/false
else unauthenticated
Helper->>Client: read apikey header via get_apikey_header()
Helper->>Oracle: get_identity_apikey_only(keymode)
Oracle-->>Helper: user_id or null
Helper->>Apps: lookup app by appid → owner_org
Helper->>Helper: call is_allowed_action(apikey, appid) → checks is_app_owner(owner_org)
Helper-->>StorageRLS: boolean allowed/denied
end
StorageRLS-->>Client: allow or deny operation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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 the current code and only fix it if needed.
Inline comments:
In `@supabase/migrations/20260308203222_restrict_apikey_oracle_public_access.sql`:
- Around line 9-11: The three GRANTs reopen parameterized oracle helpers to the
authenticated role; remove the GRANT EXECUTE statements for
"public"."get_user_id" (both overloads: ("apikey" "text") and ("apikey" "text",
"app_id" "text")) and for "public"."get_org_perm_for_apikey" ("apikey" "text",
"app_id" "text") so these functions remain service_role-only; if client access
is required, instead expose or GRANT only no-argument wrapper functions that
perform authentication checks (create/ensure wrappers that call the service_role
helpers internally and grant those wrappers to authenticated).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 673dacae-151b-486b-b317-44cd9693c3a5
📒 Files selected for processing (1)
supabase/migrations/20260308203222_restrict_apikey_oracle_public_access.sql
| GRANT EXECUTE ON FUNCTION "public"."get_user_id" ("apikey" "text") TO "authenticated"; | ||
| GRANT EXECUTE ON FUNCTION "public"."get_user_id" ("apikey" "text", "app_id" "text") TO "authenticated"; | ||
| GRANT EXECUTE ON FUNCTION "public"."get_org_perm_for_apikey" ("apikey" "text", "app_id" "text") TO "authenticated"; |
There was a problem hiding this comment.
Don't re-open these oracle helpers to all authenticated sessions.
Lines 9-11 still make the parameterized API-key lookup functions callable by any logged-in client. That preserves the oracle for authenticated users, which is the same class of exposure these low-level helpers usually need to avoid. Prefer keeping these functions service_role-only and exposing only a checked wrapper if client access is required.
🔒 Suggested change
-GRANT EXECUTE ON FUNCTION "public"."get_user_id" ("apikey" "text") TO "authenticated";
-GRANT EXECUTE ON FUNCTION "public"."get_user_id" ("apikey" "text", "app_id" "text") TO "authenticated";
-GRANT EXECUTE ON FUNCTION "public"."get_org_perm_for_apikey" ("apikey" "text", "app_id" "text") TO "authenticated";Based on learnings, "API key identities resolve through the authenticated role via JWT, not the anon role" and parameterized helper functions "should NOT be granted to anon/authenticated roles ... only the no-argument wrapper functions should be public as they perform authentication checks."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| GRANT EXECUTE ON FUNCTION "public"."get_user_id" ("apikey" "text") TO "authenticated"; | |
| GRANT EXECUTE ON FUNCTION "public"."get_user_id" ("apikey" "text", "app_id" "text") TO "authenticated"; | |
| GRANT EXECUTE ON FUNCTION "public"."get_org_perm_for_apikey" ("apikey" "text", "app_id" "text") TO "authenticated"; | |
| -- (previous lines in migration file) | |
| -- Remove the GRANT EXECUTE statements for parameterized helpers | |
| -- These functions should remain service_role-only to prevent oracle access |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/20260308203222_restrict_apikey_oracle_public_access.sql`
around lines 9 - 11, The three GRANTs reopen parameterized oracle helpers to the
authenticated role; remove the GRANT EXECUTE statements for
"public"."get_user_id" (both overloads: ("apikey" "text") and ("apikey" "text",
"app_id" "text")) and for "public"."get_org_perm_for_apikey" ("apikey" "text",
"app_id" "text") so these functions remain service_role-only; if client access
is required, instead expose or GRANT only no-argument wrapper functions that
perform authentication checks (create/ensure wrappers that call the service_role
helpers internally and grant those wrappers to authenticated).
There was a problem hiding this comment.
🧹 Nitpick comments (1)
supabase/tests/45_test_apikey_oracle_permissions.sql (1)
54-68: Inconsistent test coverage for service_role.The authenticated tests (lines 33-52) verify all three function signatures:
get_user_id(apikey),get_user_id(apikey, app_id), andget_org_perm_for_apikey(apikey, app_id). However, the service_role tests omitget_user_id(apikey, app_id).Consider adding the missing test for consistency:
🔧 Proposed fix
SELECT tests.authenticate_as_service_role(); SELECT is( get_user_id('ae6e7458-c46d-4c00-aa3b-153b0b8520ea'), '6aa76066-55ef-4238-ade6-0b32334a4097', 'service role should still call get_user_id' ); +SELECT + is( + get_user_id('ae6e7458-c46d-4c00-aa3b-153b0b8520ea', 'com.demo.app'), + '6aa76066-55ef-4238-ade6-0b32334a4097', + 'service role should still call get_user_id with app_id' + ); + SELECT is( get_org_perm_for_apikey('ae6e7458-c46d-4c00-aa3b-153b0b8520ea', 'com.demo.app'), 'perm_owner', 'service role should still call get_org_perm_for_apikey' );Update the plan count accordingly:
-SELECT plan(8); +SELECT plan(9);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/tests/45_test_apikey_oracle_permissions.sql` around lines 54 - 68, Add the missing service_role test for the two-argument variant of get_user_id: inside the service-role authenticated block (where tests.call authenticate_as_service_role() and other assertions run) add an is(...) assertion that calls get_user_id('ae6e7458-c46d-4c00-aa3b-153b0b8520ea', 'com.demo.app') and compares it to the expected user id (same expected value used for the single-arg test), and update any test plan/expect count if present to reflect the additional assertion; reference the existing get_user_id(apikey) and get_org_perm_for_apikey(apikey, app_id) tests to mirror formatting and expected value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@supabase/tests/45_test_apikey_oracle_permissions.sql`:
- Around line 54-68: Add the missing service_role test for the two-argument
variant of get_user_id: inside the service-role authenticated block (where
tests.call authenticate_as_service_role() and other assertions run) add an
is(...) assertion that calls get_user_id('ae6e7458-c46d-4c00-aa3b-153b0b8520ea',
'com.demo.app') and compares it to the expected user id (same expected value
used for the single-arg test), and update any test plan/expect count if present
to reflect the additional assertion; reference the existing get_user_id(apikey)
and get_org_perm_for_apikey(apikey, app_id) tests to mirror formatting and
expected value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fda628bf-343a-45c7-99ab-423c1702d598
📒 Files selected for processing (1)
supabase/tests/45_test_apikey_oracle_permissions.sql
|
CI check failed on March 16, 2026. The failing case is , which errors with . I did not apply a quick fix here because the same failure signature is also present on other Riderx PRs and looks like a shared/base-branch permission regression rather than something isolated to this branch. |
|
CI check |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/migrations/20260313104433_restrict-apikey-oracle-rls-fixes.sql`:
- Around line 24-51: After creating the function public.is_allowed_action(apikey
text, appid text) revoke default public privileges and explicitly grant only the
required role(s); add a REVOKE ALL ON FUNCTION
"public"."is_allowed_action"("apikey" "text","appid" "text") FROM PUBLIC
followed by GRANT EXECUTE ON FUNCTION "public"."is_allowed_action"("apikey"
"text","appid" "text") TO "service_role" (or other required roles such as
"authenticated") immediately after the function definition to prevent public
execution.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d46531c8-67bb-418d-b434-8f1a33951757
📒 Files selected for processing (5)
supabase/migrations/20260313104433_restrict-apikey-oracle-rls-fixes.sqlsupabase/tests/07_auth_functions.sqlsupabase/tests/10_utility_functions.sqlsupabase/tests/19_test_identity_functions.sqlsupabase/tests/45_test_apikey_oracle_permissions.sql
🚧 Files skipped from review as they are similar to previous changes (1)
- supabase/tests/45_test_apikey_oracle_permissions.sql
supabase/migrations/20260313104433_restrict-apikey-oracle-rls-fixes.sql
Outdated
Show resolved
Hide resolved
|
Investigated the failing |
|
CI run https://github.com/Cap-go/capgo/actions/runs/23171828925/job/67325191602 still fails in 51 CLI tests and the new permissions tests because now returns for anonymous/authenticated callers. That failure signature comes from the base branch lockdown in this PR, so I can’t safely revert or poke a one-line grant without undoing the recent permission hardening. It looks like the CLI/oracle auth path needs a broader redesign (keep the RPCs non-public while letting the CLI resolve API keys) before these tests can pass again. |
|
CI run https://github.com/Cap-go/capgo/actions/runs/23171828925/job/67325191602 still fails in 51 CLI tests and the new permissions tests because |
|
CI still fails because the CLI is calling |
|
Run tests fails across CLI/API-key paths with |
|
CI still fails on #1758 in Run tests. The log shows the same shared CLI/API-key auth breakage: |
|
PR #1758: the failures are concentrated in plan/auth tests and |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc193bc323
ℹ️ 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".
supabase/migrations/20260313104433_restrict-apikey-oracle-rls-fixes.sql
Outdated
Show resolved
Hide resolved
|
CI is still red on https://github.com/Cap-go/capgo/actions/runs/23273329108/job/67670737529. The failure is shared across the CLI/API-key auth and RPC suites, with |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3dd59d0b2e
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
supabase/migrations/20260313104433_restrict-apikey-oracle-rls-fixes.sql (1)
40-40: Use quoted identifier for consistency.Other schema-qualified references in this file use quoted identifiers (e.g.,
"storage"."foldername","public"."key_mode"). For consistency with the fully-qualified-names guideline:Proposed fix
- SELECT auth.uid() INTO _auth_uid; + SELECT "auth"."uid"() INTO _auth_uid;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/migrations/20260313104433_restrict-apikey-oracle-rls-fixes.sql` at line 40, Replace the unquoted schema reference auth.uid() with a quoted schema identifier to match the file's convention; update the SELECT to call "auth".uid() INTO _auth_uid so the schema name is quoted consistently (keep the target variable _auth_uid unchanged and update only the schema-qualified function call).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/migrations/20260313104433_restrict-apikey-oracle-rls-fixes.sql`:
- Around line 38-55: The function is incorrectly indexing PostgreSQL arrays at
0; update usages of _folder_parts in this block so they use 1-based indexing:
change the check that compares auth.uid() to _folder_parts[0] to compare
auth.uid() to _folder_parts[1], and likewise change the final comparison that
compares _api_key_user_id to _folder_parts[0] to use _folder_parts[1]; these
references appear near the calls to auth.uid(), public.get_apikey_header(),
public.is_allowed_capgkey(..., _folder_parts[1]), and
public.get_identity_apikey_only(keymode).
---
Nitpick comments:
In `@supabase/migrations/20260313104433_restrict-apikey-oracle-rls-fixes.sql`:
- Line 40: Replace the unquoted schema reference auth.uid() with a quoted schema
identifier to match the file's convention; update the SELECT to call
"auth".uid() INTO _auth_uid so the schema name is quoted consistently (keep the
target variable _auth_uid unchanged and update only the schema-qualified
function call).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7c8a4fe3-e3df-4831-9e67-c2ed98b10646
📒 Files selected for processing (2)
supabase/migrations/20260313104433_restrict-apikey-oracle-rls-fixes.sqlsupabase/tests/45_test_apikey_oracle_permissions.sql
🚧 Files skipped from review as they are similar to previous changes (1)
- supabase/tests/45_test_apikey_oracle_permissions.sql
supabase/migrations/20260313104433_restrict-apikey-oracle-rls-fixes.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e13ec6af5
ℹ️ 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".
| IF _auth_uid IS NOT NULL THEN | ||
| RETURN _auth_uid::text = _folder_parts[0]; |
There was a problem hiding this comment.
When a request carries both a session JWT and a capgkey, this early return skips the API-key path entirely. The previous apps-bucket policy allowed either auth.uid() or the API-key branch, and the repo already relies on mixed JWT+capgkey contexts (see tests/hashed-apikey-rls.test.ts:623). After this change, those calls are denied unless the JWT subject also matches the folder owner, so authenticated operators using a scoped API key lose access to the apps bucket.
Useful? React with 👍 / 👎.
| GRANT EXECUTE ON FUNCTION "public"."can_access_apps_bucket_object"("keymode" "public"."key_mode"[], "object_name" "text") TO "anon"; | ||
| GRANT EXECUTE ON FUNCTION "public"."can_access_apps_bucket_object"("keymode" "public"."key_mode"[], "object_name" "text") TO "authenticated"; |
There was a problem hiding this comment.
Granting anon/authenticated execute here turns this helper into another PostgREST oracle. The repo already treats public functions as /rpc/* entrypoints (tests/get-identity-apikey-only-rpc.test.ts checks that behavior), so anyone who knows an apps-bucket object_name can probe /rpc/can_access_apps_bucket_object with a candidate capgkey and learn whether that key matches the folder owner/app. That reintroduces the same class of key-validation leak this migration is trying to close.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/migrations/20260313104433_restrict-apikey-oracle-rls-fixes.sql`:
- Around line 17-20: Revoke of execute on function get_identity_apikey_only is
happening too early and breaks anon/authenticated storage flows; restore execute
for PUBLIC/anon/authenticated (remove or defer the REVOKE statements for
"public"."get_identity_apikey_only") and only restrict access after all other
callers are migrated (or implement a more targeted policy change for the apps
bucket). Update the migration so the REVOKE/GRANT changes to
get_identity_apikey_only are either removed here or moved to a later migration
(also adjust the related block referenced in lines ~103-150) and ensure only the
service_role grant is applied after migration completes.
- Around line 7-16: The migration revoked EXECUTE on get_user_id(apikey text),
get_user_id(apikey text, app_id text), and get_org_perm_for_apikey(apikey text,
app_id text) from "authenticated" and only granted them to "service_role",
removing the API-key identity path for JWT-authenticated users; restore the
API-key path by granting EXECUTE on those same functions back to "authenticated"
(the three function signatures listed) in this migration (or split into a
follow-up migration) so JWT-authenticated callers keep access, or alternatively
ensure and document that every caller has been moved behind a privileged service
wrapper before removing those grants.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 071078c4-342c-4799-8e73-497a9eede187
📒 Files selected for processing (1)
supabase/migrations/20260313104433_restrict-apikey-oracle-rls-fixes.sql
| REVOKE ALL ON FUNCTION "public"."get_user_id" ("apikey" "text") FROM "authenticated"; | ||
| REVOKE ALL ON FUNCTION "public"."get_user_id" ("apikey" "text", "app_id" "text") FROM "authenticated"; | ||
| REVOKE ALL ON FUNCTION "public"."get_org_perm_for_apikey" ("apikey" "text", "app_id" "text") FROM "authenticated"; | ||
| REVOKE ALL ON FUNCTION "public"."get_user_id" ("apikey" "text") FROM "public"; | ||
| REVOKE ALL ON FUNCTION "public"."get_user_id" ("apikey" "text", "app_id" "text") FROM "public"; | ||
| REVOKE ALL ON FUNCTION "public"."get_org_perm_for_apikey" ("apikey" "text", "app_id" "text") FROM "public"; | ||
|
|
||
| GRANT EXECUTE ON FUNCTION "public"."get_user_id" ("apikey" "text") TO "service_role"; | ||
| GRANT EXECUTE ON FUNCTION "public"."get_user_id" ("apikey" "text", "app_id" "text") TO "service_role"; | ||
| GRANT EXECUTE ON FUNCTION "public"."get_org_perm_for_apikey" ("apikey" "text", "app_id" "text") TO "service_role"; |
There was a problem hiding this comment.
authenticated loses the API-key identity path here.
This revokes get_user_id(...) / get_org_perm_for_apikey(...) from authenticated and never replaces that path in the same PR. That lines up with the current Invalid apikey / Cannot authenticate user with provided API key failures and turns this migration into a known auth outage unless every remaining caller is moved behind a privileged wrapper first. Based on learnings: API key identities resolve through the authenticated role via JWT, not the anon role.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/20260313104433_restrict-apikey-oracle-rls-fixes.sql`
around lines 7 - 16, The migration revoked EXECUTE on get_user_id(apikey text),
get_user_id(apikey text, app_id text), and get_org_perm_for_apikey(apikey text,
app_id text) from "authenticated" and only granted them to "service_role",
removing the API-key identity path for JWT-authenticated users; restore the
API-key path by granting EXECUTE on those same functions back to "authenticated"
(the three function signatures listed) in this migration (or split into a
follow-up migration) so JWT-authenticated callers keep access, or alternatively
ensure and document that every caller has been moved behind a privileged service
wrapper before removing those grants.
| REVOKE ALL ON FUNCTION "public"."get_identity_apikey_only" ("keymode" "public"."key_mode" []) FROM PUBLIC; | ||
| REVOKE ALL ON FUNCTION "public"."get_identity_apikey_only" ("keymode" "public"."key_mode" []) FROM anon; | ||
| REVOKE ALL ON FUNCTION "public"."get_identity_apikey_only" ("keymode" "public"."key_mode" []) FROM authenticated; | ||
| GRANT EXECUTE ON FUNCTION "public"."get_identity_apikey_only" ("keymode" "public"."key_mode" []) TO "service_role"; |
There was a problem hiding this comment.
get_identity_apikey_only is locked down before its other storage callers are migrated.
Only the apps bucket policies are redirected away from direct get_identity_apikey_only(...) calls here. The current CI already shows supabase/tests/45_test_shared_public_images.sql failing with permission denied for function get_identity_apikey_only, so this revoke still breaks existing anon/authenticated storage flows outside this bucket.
Also applies to: 103-150
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/20260313104433_restrict-apikey-oracle-rls-fixes.sql`
around lines 17 - 20, Revoke of execute on function get_identity_apikey_only is
happening too early and breaks anon/authenticated storage flows; restore execute
for PUBLIC/anon/authenticated (remove or defer the REVOKE statements for
"public"."get_identity_apikey_only") and only restrict access after all other
callers are migrated (or implement a more targeted policy change for the apps
bucket). Update the migration so the REVOKE/GRANT changes to
get_identity_apikey_only are either removed here or moved to a later migration
(also adjust the related block referenced in lines ~103-150) and ensure only the
service_role grant is applied after migration completes.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36bc575075
ℹ️ 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".
| IF NOT public.is_allowed_capgkey(_api_key_text, keymode, _folder_parts[1]) THEN | ||
| RETURN false; | ||
| END IF; | ||
|
|
||
| SELECT public.get_identity_apikey_only(keymode) INTO _api_key_user_id; |
There was a problem hiding this comment.
can_access_apps_bucket_object() now feeds _folder_parts[1] into is_allowed_capgkey(..., app_id) and then immediately reuses that same path element as the caller UUID in the return check. The previous apps-bucket policies compared different segments for those two concepts (foldername(name)[0] for the owner and [1] for the app in supabase/migrations/20250530233128_base.sql:6841-6855), so collapsing them here changes the authorization meaning. If any request still relies on the storage.objects apps bucket policies, valid reads/writes will start failing because the same folder part cannot be both the app id and the user's UUID.
Useful? React with 👍 / 👎.
|
CI is blocked by repo-wide lint failures in src/components/dashboard/InviteTeammateModal.vue:25 and src/pages/onboarding/organization.vue:613-614, which are unrelated to this branch. The unresolved review threads still cover capgkey fallback, public helper exposure, auth-path preservation, and apps-bucket path handling. #1758 |
|
CI is blocked by repo-wide lint failures in #1758. The failing run stops in () and (). I did not patch this branch because the failures are outside the PR diff and need a repo-wide lint fix or rebase. |
|
Correction: CI is blocked by repo-wide lint failures in #1758. The failing run stops in |
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53424c7e6d
ℹ️ 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".
| IF NOT public.is_allowed_capgkey(_api_key_text, keymode, _folder_parts[1]) THEN | ||
| RETURN false; | ||
| END IF; | ||
|
|
||
| SELECT public.get_identity_apikey_only(keymode) INTO _api_key_user_id; |
There was a problem hiding this comment.
Fresh evidence from supabase/functions/_backend/private/upload_link.ts:59-60 and supabase/functions/_backend/files/files.ts:447-469 shows the canonical object name is orgs/{owner_org}/apps/{app_id}/.... In that shape, _folder_parts[1] here is the org segment, not an app id or user id, so this helper passes an org id into is_allowed_capgkey(..., app_id) and then compares the returned user_id against that same org id. Any apps-bucket request that still relies on these RLS policies will therefore reject valid API-key uploads/downloads.
Useful? React with 👍 / 👎.
|
Status update for #1758: This PR still has 7 unresolved review threads. The current I did not attempt a partial patch because the failure pattern suggests a wider regression or rebase issue. |
|
Thanks for the contribution. This PR looks like a low-signal or template-style submission, which is hard to review and maintain. Please submit a focused PR with clear problem context, concrete diffs, and a short validation plan. |
1 participant
Summary (AI generated)
PUBLICand lock down directget_user_idandget_org_perm_for_apikeyexecute rights, preventing anonymous caller access to API-key validation oracles.authenticatedandservice_roleonly, preserving intended internal access patterns.Motivation (AI generated)
Business Impact (AI generated)
Test plan (AI generated)
bun lint:backendScreenshots (AI generated)
Checklist (AI generated)
bun run lint:backend && bun run lint.Summary by CodeRabbit
Bug Fixes
Tests