feat: three-actor RLS + adversarial attack integration tests (Alice/Bob/Mallory)#1095
Merged
pyramation merged 10 commits intomainfrom May 10, 2026
Merged
feat: three-actor RLS + adversarial attack integration tests (Alice/Bob/Mallory)#1095pyramation merged 10 commits intomainfrom
pyramation merged 10 commits intomainfrom
Conversation
- Extend seed setup.sql with database_settings and api_settings tables - Add Bob's storage schema with RLS policies (anonymous sees public-bucket files only) - Seed Bob's tenant data: database, APIs, storage_module, buckets, settings - Add feature flag tests: presigned uploads enabled/disabled via api_settings cascade - Add tenant isolation tests: Alice and Bob cannot see each other's files - Add RLS enforcement tests: anonymous role restricted to public-bucket files
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
This comment has been minimized.
This comment has been minimized.
…-seed private file for RLS test
…sive RLS/attack scenarios - Add Mallory as adversarial third tenant with strictest RLS (SELECT-only for anonymous) - Add RLS on Bob's app_buckets table (anonymous can only see public buckets) - Add bucket enumeration attack tests - Add Supabase-style file mutation attack tests (bucket_id tampering, is_public flipping, deletion) - Add bucket mutation attack tests (create, update, delete) - Add cross-tenant header manipulation attack tests (mismatched database_id/API name) - Add X-Schemata cross-tenant leakage tests - Add feature flag gating test for Mallory - Pre-seed files in both Bob and Mallory schemas for mutation/RLS testing - Single beforeAll setup — one server, one pool, all three schemas (stays fast)
This comment has been minimized.
This comment has been minimized.
…uccess, superuser verification, DRY SQL
…onnection isolation concern)
refactor: clean up upload integration test — expectRlsDenied, expectSuccess, DRY SQL
Contributor
|
Found 10 test failures on Blacksmith runners: Failures
|
…erns - Update mutations: patch → appFilePatch/appBucketPatch (PostGraphile v5 input types) - Add 'No values were' pattern to expectRlsDenied (RLS USING-clause denials on delete/update)
3 tasks
…n-variables fix: correct mutation variable shapes and expand expectRlsDenied patterns
- Add INTERNAL_SERVER_ERROR code check (PostGraphile masks PG errors in production)
- Add explicit GRAPHQL_VALIDATION_FAILED rejection to catch test bugs
- Handle nested null data pattern (e.g. { appFile: null }) from RLS USING clause
- Remove 'Supabase-style' from test describe name
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extends
upload.integration.test.tsfrom a two-actor (Alice/Bob) test into a comprehensive three-actor adversarial security test. Adds Mallory as a third tenant with the strictest RLS policies, and exercises attack vectors for metadata tampering, cross-tenant header manipulation, and bucket/file mutation abuse.Three-actor model
app_files(anonymous: SELECT public-bucket files + INSERT only) andapp_buckets(anonymous: SELECT public only). No UPDATE/DELETE for anonymous.Test suites (8
describeblocks)database_settings/api_settingscascade for Alice, Bob (2 APIs), and Mallory viaX-Api-Nameheaderbucket_idupdate,is_publicflip, delete, direct file injection, plus persistence check that seeded data survives all attacksX-Database-Id/X-Api-Namecombos (5 scenarios) +X-Schematacross-schema leakage (2 scenarios)expectRlsDeniedhelperThe helper handles the three ways PostGraphile surfaces an RLS denial:
permission denied,new row violates row-level security,insufficient_privilege, orNo values werecode: 'INTERNAL_SERVER_ERROR'; the raw message is only logged server-sidenullor an object with all-null fields (e.g.{ appFile: null })GraphQL validation errors (
GRAPHQL_VALIDATION_FAILED) are explicitly rejected — if a mutation fails due to a wrong field name or missing type, the test fails rather than silently passing.Seed data changes
schema.sql: Refactored into a_test_create_storage_schema()helper function that creates buckets + files tables for any schema name. Adds Bob's storage schema with RLS on bothapp_bucketsandapp_files; adds Mallory's schema with strictest RLS (anonymous SELECT-only on both tables).test-data.sql: Seeds Mallory's database, API (mallory-app),storage_module, two buckets (public/private), two pre-seeded files (one per bucket),database_settings, and all required metaschema rows. Also adds a pre-seeded public file in Bob's schema for mutation attack testing.setup.sql: Addsdatabase_settingsandapi_settingsDDL (mirrors production tables from constructive-db PR feat(node-type-registry): add DataLimitCounter, DataFeatureFlag, AuthzAppMembership #1060).Performance
All tests share a single
beforeAllwith onegetConnections()server instance — nobeforeEach, no per-describe teardown. All three schemas are loaded once and reused across all 8 test suites.Updates since last revision
patchbut PostGraphile v5 expects table-specific names:appFilePatchforUpdateAppFileInputandappBucketPatchforUpdateAppBucketInput. Previously these tests were failing for schema validation, not RLS — they never actually reached the RLS layer.expectRlsDenied— In CI,NODE_ENVis notdevelopment, so PostGraphile masks all PG errors. The client receives"An unexpected error occurred"withcode: INTERNAL_SERVER_ERRORinstead of the raw PG message. Added the error code check alongside the raw message patterns.{ updateAppFile: { appFile: null } }rather than a top-levelnull. The helper now accepts objects where all values are null.GRAPHQL_VALIDATION_FAILEDrejection — Prevents tests from silently passing when a mutation fails for the wrong reason (e.g. typo in a field name).mas a prefix (e.g.m1m1m1m1-...), butmis not a valid hexadecimal digit. Replaced withfa-prefix UUIDs.Review & Testing Checklist for Human
INTERNAL_SERVER_ERRORacceptance isn't too broad — In production mode,expectRlsDeniedaccepts any masked internal error as an RLS denial. If PostGraphile hits a real bug (unrelated to RLS), the test would falsely pass. TheGRAPHQL_VALIDATION_FAILEDrejection catches the most common false-pass case, but a non-RLS database error would still be accepted. Consider whether to setNODE_ENV=developmentin the CI test runner to get unmasked PG errors.X-Schematatests aren't vacuously true — The twoX-Schematacross-schema tests useif (res.status === 200 && res.body.data)before asserting. If the server returns a non-200 status for a different reason (e.g., schema not found), the assertions are silently skipped and the test passes. Worth verifying these actually exercise the intended 200 path.test-data.sqland the test file. A mismatch between seedINSERTUUIDs and test constants would cause silent false-passes in mutation/isolation tests.database_settings/api_settingsDDL insetup.sqlmatches production — These table definitions are duplicated from constructive-db PR feat(node-type-registry): add DataLimitCounter, DataFeatureFlag, AuthzAppMembership #1060. If those schemas change, these test seeds must be updated to match.Notes
integration-tests (graphql/server-test)job that runs these tests against real PostgreSQL + MinIO.is_public = false(nottrue) to match the server'sisPublic: falseconfiguration. The original upload tests useX-Schematawhich bypasses API lookup entirely, so this doesn't affect them.Link to Devin session: https://app.devin.ai/sessions/94a2728a9c414500bead29cbbc829c15
Requested by: @pyramation