fix: harden record_build_time rpc access

[riderx]

Summary (AI generated)

• Revoke public exposure of public.record_build_time via Supabase migration so anonymous role cannot call it.
• Add a REST-level regression test that posts directly to /rest/v1/rpc/record_build_time with SUPABASE_ANON_KEY and asserts the call is rejected and no build_logs row is written.

Motivation (AI generated)

• A public record_build_time path allowed unauthenticated callers to inject build-time rows, enabling usage/quota poisoning and cross-tenant abuse.

Business Impact (AI generated)

• Closing this path reduces fraud risk and protects quota enforcement, billing signals, and storage/aggregation load from unauthenticated abuse.

Test Plan (AI generated)

• bunx eslint tests/build_time_tracking.test.ts
• Run the full backend test suite and targeted security regression checks in CI.

Generated with AI

[coderabbitai]

Warning

Rate limit exceeded

@riderx has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 4 minutes and 29 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2371aaaf-acf5-4f15-9476-5432c580d4de

📥 Commits

Reviewing files that changed from the base of the PR and between c4c01a4 and fbfd02d.

📒 Files selected for processing (2)

• supabase/migrations/20260308203309_cli_created_record_build_time_public_revoke.sql
• tests/build_time_tracking.test.ts

📝 Walkthrough

Walkthrough

A database migration revokes PUBLIC role access to the record_build_time SQL function, and a corresponding test verifies that unauthenticated calls to this RPC endpoint are properly rejected. These changes enforce stricter authorization controls.

Changes

Cohort / File(s)	Summary
Database Migration supabase/migrations/20260308203309_cli_created_record_build_time_public_revoke.sql	Revokes all privileges on the public.record_build_time function for the PUBLIC role, restricting execution to authenticated users only.
Security Test tests/build_time_tracking.test.ts	Adds a new test case that validates unauthenticated Supabase client calls to the record_build_time RPC are properly rejected with an error.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

• fix(build): prevent cross-app job access #1590: Updates server-side code paths that invoke recordBuildTime and implements tighter authorization checks alongside this migration-level access revocation.

Poem

🐰 A lock on the gate, so secure and tight,
No strangers shall pass in the dead of night,
The rabbit ensures what's private stays so—
With tests standing guard, our secrets won't go! 🔐

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: harden record_build_time rpc access' directly and clearly describes the main change: hardening access control for a specific RPC function, matching the changeset's focus on revoking public role privileges and adding security tests.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check	✅ Passed	The pull request description includes a summary, motivation, business impact, and test plan, mostly aligning with the template structure despite using AI-generated headers.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch riderx/fix-build-rpc

---

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

[coderabbitai]

🧹 Nitpick comments (1)

tests/build_time_tracking.test.ts (1)

138-158: Consider using it.concurrent() and strengthening the error assertion.

Two suggestions:

1. Per coding guidelines, tests should use it.concurrent() for parallel execution. While existing tests in this file also use it(), new tests should follow the guideline.

2. The assertion expect(error).toBeTruthy() is broad—any error (network, malformed input, etc.) would pass. Consider verifying the error indicates a permission denial to ensure the migration is working as intended.

Suggested improvements

-  it('should reject unauthenticated calls to record_build_time RPC', async () => {
+  it.concurrent('should reject unauthenticated calls to record_build_time RPC', async () => {
     const supabaseUrl = process.env.SUPABASE_URL
     const supabaseAnonKey = process.env.SUPABASE_ANON_KEY
     if (!supabaseUrl || !supabaseAnonKey)
       throw new Error('SUPABASE_URL and SUPABASE_ANON_KEY are required for this test')

     const publicSupabase = createClient(supabaseUrl, supabaseAnonKey, {
       auth: {
         persistSession: false,
       },
     })

     const { error } = await publicSupabase.rpc('record_build_time', {
       p_org_id: ORG_ID,
       p_user_id: USER_ID,
       p_build_id: randomUUID(),
       p_platform: 'ios',
       p_build_time_unit: 30,
     })
     expect(error).toBeTruthy()
+    expect(error?.message).toMatch(/permission denied|not authorized/i)
   })

As per coding guidelines: "Design all tests for parallel execution; use it.concurrent() instead of it() to maximize parallelism".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/build_time_tracking.test.ts` around lines 138 - 158, Change the test to
run in parallel by replacing it(...) with it.concurrent(...), and strengthen the
assertion so it verifies the failure is a permission denial from the
database/RPC rather than any error: after invoking
publicSupabase.rpc('record_build_time', {...}) assert that error is truthy and
additionally check a permission-specific indicator (for example assert
error.message or error.details includes 'permission' or 'permission denied', or
that error.code equals the Postgres permission error code such as '42501') so
the test specifically confirms the RPC was rejected due to insufficient
privileges.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/build_time_tracking.test.ts`:
- Around line 138-158: Change the test to run in parallel by replacing it(...)
with it.concurrent(...), and strengthen the assertion so it verifies the failure
is a permission denial from the database/RPC rather than any error: after
invoking publicSupabase.rpc('record_build_time', {...}) assert that error is
truthy and additionally check a permission-specific indicator (for example
assert error.message or error.details includes 'permission' or 'permission
denied', or that error.code equals the Postgres permission error code such as
'42501') so the test specifically confirms the RPC was rejected due to
insufficient privileges.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 307d15c9-b140-4b68-9cf8-813571c40325

📥 Commits

Reviewing files that changed from the base of the PR and between 2af3a08 and c4c01a4.

📒 Files selected for processing (2)

• supabase/migrations/20260308203309_cli_created_record_build_time_public_revoke.sql
• tests/build_time_tracking.test.ts

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud