Repository navigation
Map RLS permission errors to client responses without leaking row data #880
Copy link
Copy link
Open
Labels
app: backendTask implementation touches the backendTask implementation touches the backendapp: workerTask implementation touches the workerTask implementation touches the worker
Milestone
Description
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 assqlalchemy.exc.ProgrammingErrorwith.orig.pgcode == "42501". The only database exceptions the api catches today are specificIntegrityErrors. 42501covers two different failures, distinguishable only by the message:new row violates row-level security policy for table "<table>", plus the(USING expression)variants forON 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 ORMUPDATEmatches zero rows because the row is hidden, also reaches the middleware as a 500.send_slack_errorand the logging context both serialize the exception withstr(err).- In the worker,
worker/lib/managers/utils.pyclassifies a42501asUNKNOWN.
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.pywithhide_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
42501in 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, neverstr(err).
Tests
- Unit tests for the classifier, covering psycopg2 errors with each message form listed above, including the
ON CONFLICTvariant. - A router test on the production-topology fixture: a write that violates a
WITH CHECKpolicy returns 403, and the captured Slack payload contains no row values.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
app: backendTask implementation touches the backendTask implementation touches the backendapp: workerTask implementation touches the workerTask implementation touches the worker
Activity