Skip to content

Map RLS permission errors to client responses without leaking row data #880

Open

Description

@bencap

Summary

When RLS blocks a write, the api currently answers with a 500 error and sends the full failing SQL, including row values, to Slack and the logs. This issue maps those errors to proper 403 and 404 responses and keeps row data out of alerts.

Under FORCE ROW LEVEL SECURITY, an INSERT or UPDATE that fails a policy's WITH CHECK raises SQLSTATE 42501. Nothing catches it. CatchAllErrorMiddleware returns a 500 and sends str(err) to Slack and to the log context. The engine doesn't set hide_parameters, so that text includes the full SQL and the bound row values.

Problem

  • psycopg2 raises InsufficientPrivilege, which SQLAlchemy wraps as sqlalchemy.exc.ProgrammingError with .orig.pgcode == "42501". The only database exceptions the api catches today are specific IntegrityErrors.
  • 42501 covers two different failures, distinguishable only by the message:
    • new row violates row-level security policy for table "<table>", plus the (USING expression) variants for ON CONFLICT. Python permitted a write the policy rejects, so Python and the policy disagree.
    • permission denied for table <table>. A grant is missing, which is a deployment bug.
  • StaleDataError, raised when an ORM UPDATE matches zero rows because the row is hidden, also reaches the middleware as a 500.
  • send_slack_error and the logging context both serialize the exception with str(err).
  • In the worker, worker/lib/managers/utils.py classifies a 42501 as UNKNOWN.

Proposed behavior

  • Add a database error classifier used by CatchAllErrorMiddleware:
Condition Response Alert
42501, row-level security policy message 403 Slack, with SQLSTATE, table and route only
42501, permission denied for table 500 Slack, with SQLSTATE, table and route only
StaleDataError 404 none
  • Create the engine in db/session.py with hide_parameters=True, so bound values never appear in exception text.
  • Build Slack and log payloads for database errors from the SQLSTATE, table name, constraint name and route, never from str(err).
  • Classify 42501 in the worker as a non-retryable permission failure.
  • The worker's engine also sets hide_parameters=True, and its job failure logging builds messages from SQLSTATE, table and constraint name, never str(err).

Tests

  • Unit tests for the classifier, covering psycopg2 errors with each message form listed above, including the ON CONFLICT variant.
  • A router test on the production-topology fixture: a write that violates a WITH CHECK policy returns 403, and the captured Slack payload contains no row values.

Metadata

Metadata

Assignees

No one assigned

    Labels

    app: backendTask implementation touches the backendapp: workerTask implementation touches the worker

    Type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions