Skip to content

[codex] Restore app-scoped statistics access#1871

Merged
riderx merged 3 commits into
mainfrom
codex/fix-app-stats-rbac
Mar 27, 2026
Merged

[codex] Restore app-scoped statistics access#1871
riderx merged 3 commits into
mainfrom
codex/fix-app-stats-rbac

Conversation

@riderx
Copy link
Copy Markdown
Member

@riderx riderx commented Mar 27, 2026

Summary (AI generated)

  • add an app-scoped public.get_app_metrics(org_id, app_id, start_date, end_date) overload that preserves the March org-level hardening while allowing app-level readers to fetch their own metrics
  • route /statistics/app/:app_id to the new app-scoped RPC instead of the org-scoped RPC that now requires broader access
  • strengthen the statistics regression tests so a 200 response with an empty array no longer passes as success

Motivation (AI generated)

The March 13, 2026 security hardening correctly restricted the org-scoped metrics RPC, but the app statistics endpoint still called that org-scoped RPC after only checking app.read. For app-scoped users and subkeys, the endpoint therefore returned 200 with empty metrics even though chart data existed.

Business Impact (AI generated)

This restores MAU and bandwidth charts for app-scoped customers without weakening the org-level RBAC fix. It removes a silent failure mode on a core dashboard path and reduces support/debug time for missing statistics.

Test Plan (AI generated)

  • bun lint supabase/functions/_backend/public/statistics/index.ts tests/statistics.test.ts
  • bun run supabase:db:reset
  • SUPABASE_URL=http://127.0.0.1:62001 SUPABASE_DB_URL=postgresql://postgres:postgres@127.0.0.1:62002/postgres SUPABASE_ANON_KEY=sb_publishable_ACJWlzQHlZjBrEguHvfOxg_3BJgxAaH bun test tests/statistics.test.ts

Generated with AI

Summary by CodeRabbit

  • New Features

    • Enhanced app-scoped metrics queries with improved organization and application-level access control.
    • Added caching layer for metrics retrieval with automatic refresh when data is stale.
  • Bug Fixes

    • Restored correct app-scoped chart access with proper security validation.
  • Tests

    • Strengthened statistics validation tests with additional assertions for metrics data integrity.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 27, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR implements app-scoped and org-scoped Role-Based Access Control (RBAC) for the metrics system by introducing secure database functions with JWT validation, conditional parameter handling, and caching logic. The TypeScript backend is updated to branch requests based on whether an app ID is provided, and comprehensive tests validate both authenticated and unauthenticated access patterns.

Changes

Cohort / File(s) Summary
Backend Statistics Function
supabase/functions/_backend/public/statistics/index.ts
Added AppMetricRow interface and updated getNormalStats to conditionally invoke get_app_metrics with parameterized names (p_org_id, p_app_id, p_start_date, p_end_date) when app ID is present, or original names when absent. Modified response handling to coerce nullable RPC results and adjusted route logic to pass owner_org to the statistics function.
Database Metrics RBAC Migration
supabase/migrations/20260327210500_app_scoped_metrics_rbac.sql
Created/replaced four function overloads for get_app_metrics and get_global_metrics with SECURITY DEFINER enforcement. Implemented app-scoped access control via JWT role extraction and permission validation, integrated caching with 5-minute staleness checks, and ensured metrics are filtered by app/org ownership before returning.
Statistics Tests
tests/statistics.test.ts
Added hasSeededStats helper to validate metrics contain non-zero values. Strengthened existing assertions for app and org statistics endpoints. Added two new test cases for GET /statistics/org/... with breakdown=true&noAccumulate=true covering both authenticated and non-authenticated request scenarios.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Backend as TypeScript Backend
    participant Supabase as Supabase RPC
    participant JWT as JWT Validator
    participant Cache as Metrics Cache
    participant DB as Database

    Client->>Backend: GET /statistics/app/:app_id
    Backend->>Backend: Extract appId & ownerOrg
    Backend->>Supabase: get_app_metrics(p_org_id, p_app_id, p_start_date, p_end_date)
    
    Supabase->>JWT: Extract caller role from JWT
    Supabase->>Supabase: get_identity_org_appid(caller)
    Supabase->>Supabase: check_min_rights(caller, app)
    
    Supabase->>DB: Verify app exists for org
    Supabase->>Cache: Check cache entry validity
    
    alt Cache missing or stale
        Supabase->>DB: seed_get_app_metrics_caches()
        DB-->>Cache: Populate cache
    end
    
    Cache-->>Supabase: Return cached metrics JSON
    Supabase->>Supabase: jsonb_to_recordset & filter by app_id
    Supabase-->>Backend: Metrics rows
    
    Backend->>Backend: Coerce to AppMetricRow[] via ?? []
    Backend-->>Client: Statistics response
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

codex

Poem

🐰 Metrics locked tight with RBAC's might,
JWT checks guard every sight,
Cached data flows through org-scoped rows,
App-to-app access: only what each one knows! 📊✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and concisely summarizes the main change: restoring app-scoped statistics access after org-level hardening.
Description check ✅ Passed The PR description covers Summary, Motivation, and Business Impact with detail, but the Test Plan section is auto-generated and lacks explicit manual testing steps as required by the template.

✏️ 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-app-stats-rbac

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.0.4)
supabase/migrations/20260327210500_app_scoped_metrics_rbac.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 Mar 27, 2026

Merging this PR will not alter performance

✅ 28 untouched benchmarks


Comparing codex/fix-app-stats-rbac (c9ab2bb) with main (7bd5daf)

Open in CodSpeed

@riderx riderx marked this pull request as ready for review March 27, 2026 22:25
@riderx riderx merged commit 936afd0 into main Mar 27, 2026
13 of 15 checks passed
@riderx riderx deleted the codex/fix-app-stats-rbac branch March 27, 2026 22:30
@sonarqubecloud
Copy link
Copy Markdown

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