From 94ce9d48e28e8b698d58c0f4d964d1191cadacda Mon Sep 17 00:00:00 2001 From: Logan Cusano Date: Sun, 16 Aug 2026 21:31:09 -0400 Subject: [PATCH] Commit the Firestore security rules that were never in source control SAAS_PLAN.md's review found the actual finding underneath "no multi-tenancy": drb-frontend reads Firestore directly from the browser (every hook in lib/ does onSnapshot(collection(db, ...))), so drb-c2-core/app/internal/auth.py is never in that read path at all. Whatever rules were protecting calls, incidents, and nodes had been hand-set in the Firebase console - unversioned, unreviewed, and invisible to anyone reading this repo. Added infra/firestore/firestore.rules: deny-by-default, with every tenant-scoped collection (nodes, systems, calls, incidents, alert_events, alert_rules) gated on resource.data.org_id == request.auth.token.org_id, an org_id claim that doesn't exist yet - the next commits add it. All client writes stay denied; c2-core's admin SDK bypasses rules and remains the sole writer, which was already the architecture. Secret-bearing collections (node_keys, the new enrollment_tokens) are denied to clients outright rather than org-scoped, since nothing should ever hand a raw credential to the browser. trips/trip_events keep their current "signed-in users can read" shape rather than being pulled into org scoping - that feature isn't tenant-scoped in this pass (see B7), just hidden from non-founding-org users in the UI. Added infra/firestore/firestore.indexes.json for the composite indexes the org_id-scoped queries will need once the frontend hooks add the equality filter alongside their existing orderBy/range/array-contains clauses - without these, those queries fail at runtime with a FAILED_PRECONDITION "index required" error rather than at review time. Also extended internal/firestore.py's collection_where() with optional order_by/limit_to/start_after params (SAAS_PLAN.md item 1, a stated prerequisite for B2: scoped queries need to stay ordered and bounded, and the existing helper could only do unordered full-collection scans). array_contains needed no new code - it was already a pass-through op string to FieldFilter. None of this is live yet. Deploying rules/indexes is a manual step (firebase deploy --only firestore:rules,firestore:indexes --project , from infra/firestore/) - nothing in CI does this. Until it runs, the console-configured rules are still what's actually enforced, and these rules reference an org_id claim no token carries yet. Deploy this alongside (not before) the org_id-stamping commits that follow, or every read breaks for the current single-org deployment. Co-Authored-By: Claude Opus 5 --- drb-c2-core/app/internal/firestore.py | 27 +++- infra/firestore/firebase.json | 7 ++ infra/firestore/firestore.indexes.json | 47 +++++++ infra/firestore/firestore.rules | 166 +++++++++++++++++++++++++ 4 files changed, 246 insertions(+), 1 deletion(-) create mode 100644 infra/firestore/firebase.json create mode 100644 infra/firestore/firestore.indexes.json create mode 100644 infra/firestore/firestore.rules diff --git a/drb-c2-core/app/internal/firestore.py b/drb-c2-core/app/internal/firestore.py index 809bccb..c3ab551 100644 --- a/drb-c2-core/app/internal/firestore.py +++ b/drb-c2-core/app/internal/firestore.py @@ -68,16 +68,41 @@ async def collection_list(collection: str, **filters) -> list[dict]: async def collection_where( collection: str, conditions: list[tuple[str, str, Any]], + order_by: Optional[list[tuple[str, str]]] = None, + limit_to: Optional[int] = None, + start_after: Optional[dict] = None, ) -> list[dict]: """ Query a collection with arbitrary where-clauses. conditions: list of (field, op, value) — e.g. [("ended_at", ">=", cutoff_dt)] - Supports any Firestore operator: "==", "!=", "<", "<=", ">", ">=". + Supports any Firestore operator, including "array_contains" — it's just + forwarded straight to FieldFilter, so a condition like + ("incident_ids", "array_contains", incident_id) already worked before this + function grew explicit order_by/limit/cursor params below. + + order_by: list of (field, direction) — direction is "ASCENDING" or + "DESCENDING" (Firestore's own constants; passed straight through as + strings so this module doesn't need a google.cloud.firestore_v1.Query + import). Applied in list order, so multi-field sorts work. + limit_to: cap the number of documents returned. + start_after: cursor — a dict of the same field values as the *last* + document from a previous page's order_by fields (Firestore's + `Query.start_after()` takes a field-value mapping, not a document + snapshot, when you're not holding one). + + Added for org_id-scoped queries that also need to be ordered/paginated — + unscoped equality-only lookups can keep using collection_list(). """ def _query(): ref = db.collection(collection) for field, op, value in conditions: ref = ref.where(filter=FieldFilter(field, op, value)) + for field, direction in (order_by or []): + ref = ref.order_by(field, direction=direction) + if start_after is not None: + ref = ref.start_after(start_after) + if limit_to is not None: + ref = ref.limit(limit_to) return [doc.to_dict() for doc in ref.stream()] return await asyncio.to_thread(_query) diff --git a/infra/firestore/firebase.json b/infra/firestore/firebase.json new file mode 100644 index 0000000..639d24d --- /dev/null +++ b/infra/firestore/firebase.json @@ -0,0 +1,7 @@ +{ + "//": "Deploy target for firestore.rules / firestore.indexes.json only — this is not a Firebase Hosting project config. Run from this directory: firebase deploy --only firestore:rules,firestore:indexes --project . No project id is pinned here deliberately (see [[self-hosted-infra-pointers]] for where that value lives) — pass --project explicitly or run `firebase use ` once first.", + "firestore": { + "rules": "firestore.rules", + "indexes": "firestore.indexes.json" + } +} diff --git a/infra/firestore/firestore.indexes.json b/infra/firestore/firestore.indexes.json new file mode 100644 index 0000000..4c44a38 --- /dev/null +++ b/infra/firestore/firestore.indexes.json @@ -0,0 +1,47 @@ +{ + "//": "Composite indexes required once drb-frontend's lib/use*.ts hooks add a where(\"org_id\",\"==\",orgId) equality filter alongside an existing range/orderBy/array-contains clause. Firestore auto-indexes single-field lookups and equality-only compound queries, but org_id==X combined with an inequality, orderBy on a different field, or array-contains needs an explicit composite index or the query fails at runtime with a FAILED_PRECONDITION 'index required' error (see SAAS_PLAN.md 2.8/B2 and the URL Firestore prints in that error, which is the fastest way to double-check this list against the live query shapes). Deploy with: firebase deploy --only firestore:indexes --project ", + "indexes": [ + { + "collectionGroup": "calls", + "queryScope": "COLLECTION", + "fields": [ + { "fieldPath": "org_id", "order": "ASCENDING" }, + { "fieldPath": "started_at", "order": "ASCENDING" } + ] + }, + { + "collectionGroup": "calls", + "queryScope": "COLLECTION", + "fields": [ + { "fieldPath": "org_id", "order": "ASCENDING" }, + { "fieldPath": "incident_ids", "arrayConfig": "CONTAINS" } + ] + }, + { + "collectionGroup": "incidents", + "queryScope": "COLLECTION", + "fields": [ + { "fieldPath": "org_id", "order": "ASCENDING" }, + { "fieldPath": "started_at", "order": "ASCENDING" } + ] + }, + { + "collectionGroup": "alert_events", + "queryScope": "COLLECTION", + "fields": [ + { "fieldPath": "org_id", "order": "ASCENDING" }, + { "fieldPath": "triggered_at", "order": "ASCENDING" } + ] + }, + { + "collectionGroup": "alert_events", + "queryScope": "COLLECTION", + "fields": [ + { "fieldPath": "org_id", "order": "ASCENDING" }, + { "fieldPath": "acknowledged", "order": "ASCENDING" }, + { "fieldPath": "triggered_at", "order": "ASCENDING" } + ] + } + ], + "fieldOverrides": [] +} diff --git a/infra/firestore/firestore.rules b/infra/firestore/firestore.rules new file mode 100644 index 0000000..9510c9a --- /dev/null +++ b/infra/firestore/firestore.rules @@ -0,0 +1,166 @@ +// Firestore security rules — the actual tenant boundary for DRB. +// +// WHY THIS FILE EXISTS: drb-frontend reads Firestore directly from the +// browser (see lib/use*.ts — onSnapshot(collection(db, ...))), so c2-core's +// app/internal/auth.py is NOT in that read path at all. These rules are the +// only thing standing between a signed-in stranger and every org's radio +// traffic. Before this file existed, whatever rules were live had been +// hand-set in the Firebase console: unversioned, unreviewed, unknown. See +// SAAS_PLAN.md B1. +// +// DEPLOY IS A MANUAL, OUT-OF-BAND STEP — nothing in CI or this codebase +// pushes these rules to Firebase: +// firebase deploy --only firestore:rules --project +// (from this directory, or point --config at infra/firestore/firebase.json +// from the repo root). Do this before or immediately after the code that +// starts stamping org_id ships — until these rules are live, the +// console-configured rules are still what's actually enforced. +// +// MODEL: c2-core (firebase-admin SDK, server-side) bypasses these rules +// entirely and is the sole writer for every collection below — that was +// already the architecture (see CLAUDE.md "Auth — three distinct +// mechanisms"). These rules therefore only need to gate READS for the +// browser client, and can safely deny ALL client writes. +// +// Deny-by-default: the catch-all match at the bottom denies anything not +// explicitly listed above it, including collections added later that +// someone forgets to add a rule for. + +rules_version = '2'; + +service cloud.firestore { + match /databases/{database}/documents { + + function signedIn() { + return request.auth != null; + } + + // Platform-level role (admin/operator/viewer) — set by drb-c2-core + // routers/users.py custom claims. Distinct from org_role (owner/member), + // which is per-organization. A platform admin can read across every org + // (support/debugging), mirroring internal/auth.py's require_org() + // ?org_id= override for the same role. + function isPlatformAdmin() { + return signedIn() && + (request.auth.token.role == 'admin' || request.auth.token.admin == true); + } + + // The org_id claim is set by POST /auth/signup (or /admin/users) at + // account-provisioning time. No claim => no access, by construction — + // this is what backs AuthProvider's no-claim guard (SAAS_PLAN.md B3): + // a user with no org_id claim can hold a valid Firebase session and + // still read nothing here. + function myOrgId() { + return request.auth.token.org_id; + } + + function inOrg(orgId) { + return signedIn() && (myOrgId() == orgId || isPlatformAdmin()); + } + + function docInMyOrg() { + return inOrg(resource.data.org_id); + } + + // ── Org identity ────────────────────────────────────────────────────── + match /organizations/{orgId} { + allow read: if inOrg(orgId); + allow write: if false; // c2-core only (POST /auth/signup, routers/org.py) + } + + match /org_members/{uid} { + allow read: if signedIn() && (request.auth.uid == uid || isPlatformAdmin() || + inOrg(resource.data.org_id)); + allow write: if false; // c2-core only + } + + // ── Tenant-scoped radio data — the whole point of this file ─────────── + match /nodes/{nodeId} { + allow read: if docInMyOrg(); + allow write: if false; + } + + match /systems/{systemId} { + allow read: if docInMyOrg(); + allow write: if false; + } + + match /calls/{callId} { + allow read: if docInMyOrg(); + allow write: if false; + } + + match /incidents/{incidentId} { + allow read: if docInMyOrg(); + allow write: if false; + } + + match /alert_events/{alertId} { + allow read: if docInMyOrg(); + allow write: if false; + } + + match /alert_rules/{ruleId} { + allow read: if docInMyOrg(); + allow write: if false; + } + + // ── Never client-readable, org-scoped or not ─────────────────────────── + // Secrets / credential material. Reads for these go through c2-core + // REST routes (which apply their own auth), never straight to Firestore. + match /node_keys/{nodeId} { + allow read, write: if false; + } + + match /enrollment_tokens/{tokenHash} { + allow read, write: if false; // routers/org.py mints/lists/revokes server-side + } + + match /org_api_keys/{keyId} { + allow read, write: if false; // not implemented server-side yet (DEFERRED.md) — deny regardless + } + + // ── Platform-admin-only collections ──────────────────────────────────── + // Listed here mainly so the deny-default catch-all's intent is explicit; + // these are already read/written exclusively through c2-core admin + // routes (require_admin_token), never straight from the browser. + match /audit_log/{entryId} { + allow read, write: if false; + } + + match /config/{docId} { + allow read, write: if false; + } + + match /bot_tokens/{tokenId} { + allow read, write: if false; + } + + match /waitlist/{entryId} { + allow read, write: if false; // POST /waitlist writes via the admin SDK + } + + // ── Trips (internal utility feature, not org-scoped — see + // [[trips-feature-intentional]] and SAAS_PLAN.md B7. Frontend hides + // /trips outside the founding org; these rules keep the existing + // "public unless flagged private" trip model working for whichever + // users the UI still exposes it to) ──────────────────────────────────── + match /trips/{tripId} { + allow read: if signedIn(); + allow write: if false; + } + + match /trip_events/{eventId} { + allow read: if signedIn(); + allow write: if false; + } + + // ── Deny-by-default catch-all ────────────────────────────────────────── + // Anything not explicitly matched above — including collections added + // later without a corresponding rule — is denied. This is the guard + // rail: a missing rule fails closed, not open. + match /{document=**} { + allow read, write: if false; + } + } +}