Tenant isolation: fail-open on lookup error, and NULL-org rows visible to every tenant

forail/main/migrations/0211_rls_scoped_null_org.py

@@ -0,0 +1,53 @@
+"""Multi-Tenancy v2: stop treating every NULL organization as globally shared.
+
+Codex M6 — the policies emitted ``OR organization_id IS NULL`` for every table,
+so a row saved without an organization was readable by every tenant. Whether a
+NULL organization means "shared with the platform" is now decided per table by
+``RLS_GLOBAL_NULL_ORG_TABLES``; the rest are scoped strictly.
+
+This changes visibility for existing rows: on a tenant-scoped request, rows with
+no organization in a strictly-scoped table stop being returned. They are not
+deleted and remain visible to superusers and to any request without a tenant
+context. If a resource "disappears" for a tenant after this migration, the fix
+is to assign its organization -- not to add its table to the global list.
+
+Idempotent: each policy is dropped (IF EXISTS) then recreated, for both direct
+and indirect tables, so this converges on fresh and existing databases.
+"""
+
+from django.db import migrations
+
+from forail.main.tenancy.helpers import (
+    RLS_TABLES_DIRECT,
+    RLS_TABLES_INDIRECT,
+    build_rls_policy_sql,
+    build_rls_policy_sql_indirect,
+)
+
+
+def _rebuild_sql():
+    statements = []
+    for table, org_col in RLS_TABLES_DIRECT:
+        create, drop = build_rls_policy_sql(table, org_col)
+        statements.append(f'ALTER TABLE {table} ENABLE ROW LEVEL SECURITY;')
+        statements.append(f'ALTER TABLE {table} FORCE ROW LEVEL SECURITY;')
+        statements.append(drop)
+        statements.append(create)
+    for table, fk_col, parent_table, parent_org_col in RLS_TABLES_INDIRECT:
+        create, drop = build_rls_policy_sql_indirect(table, fk_col, parent_table, parent_org_col)
+        statements.append(f'ALTER TABLE {table} ENABLE ROW LEVEL SECURITY;')
+        statements.append(f'ALTER TABLE {table} FORCE ROW LEVEL SECURITY;')
+        statements.append(drop)
+        statements.append(create)
+    return '\n'.join(statements)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('main', '0210_rls_nullif_cast'),
+    ]
+
+    operations = [
+        migrations.RunSQL(sql=_rebuild_sql(), reverse_sql=migrations.RunSQL.noop),
+    ]

forail/main/observability/bootstrap.py

@@ -23,9 +23,22 @@
     'OTEL_TRACES_SAMPLER',
     'OTEL_TRACES_SAMPLER_ARG',
     'TENANCY_ENABLED',
+    # Codex M2: TENANCY_ENABLED synced but the controls that actually enforce
+    # isolation did not, so a deployment could set the one flag it was offered
+    # and get tenancy features with no row-level security behind them. Turning
+    # tenancy on through the environment now reaches the whole set.
+    'TENANCY_RLS_ENABLED',
+    'TENANCY_STRICT_ISOLATION_ENABLED',
+    'TENANCY_RATE_LIMITING_ENABLED',
 )
 
-_BOOL_KEYS = ('OTEL_ENABLED', 'TENANCY_ENABLED')
+_BOOL_KEYS = (
+    'OTEL_ENABLED',
+    'TENANCY_ENABLED',
+    'TENANCY_RLS_ENABLED',
+    'TENANCY_STRICT_ISOLATION_ENABLED',
+    'TENANCY_RATE_LIMITING_ENABLED',
+)
 
 
 def _coerce_env_value(key, raw):

forail/main/tenancy/helpers.py

@@ -232,12 +232,11 @@ def tenant_queue_name(org_id):
 # that references Organization.id — directly ('organization_id') for most
 # tables or via a subquery for indirect relationships.
 #
-# Tables with nullable organization_id are included; the RLS policy handles
-# NULLs by treating them as "visible to everyone" (no tenant scope). This is
-# intentional for AWX's genuinely global, org-less resources (a credential or
-# label created with organization=NULL is shared platform-wide). Do NOT add a
-# table here whose rows are always tenant-owned unless it forbids NULL org, or
-# those NULL rows would leak across every tenant (see needtofix M5).
+# Tables with nullable organization_id are included. Whether a NULL organization
+# means "shared with the whole platform" or "nobody set it" is decided per table
+# by RLS_GLOBAL_NULL_ORG_TABLES below -- it used to be assumed to mean the first
+# for every table, which is what made a single missed assignment enough to turn
+# a tenant-owned row into cross-tenant data (see needtofix M5, Codex M6).
 #
 # Coverage note (needtofix M4): several models that *look* org-scoped
 # (Project, WorkflowJobTemplate, Schedule, workflow nodes, job events) do not
@@ -287,12 +286,51 @@ def tenant_queue_name(org_id):
 ]
 
 
+# Tables where `organization_id IS NULL` is a deliberate sharing or ownership
+# mechanism inherited from AWX, and the row really is meant to be visible
+# outside any one tenant:
+#
+#   main_credential           NULL org = a user-owned (personal) credential
+#   main_oauth2application    NULL org = a user-owned application
+#   main_executionenvironment NULL org = a globally shared execution environment
+#   main_unifiedjobtemplate   system job templates carry no organization
+#   main_unifiedjob           ...and neither do their runs
+#
+# Everything else is scoped strictly: a NULL organization there is an unset
+# field, not a shared resource, and the row stays invisible to a tenant-scoped
+# request. Hidden is the safe direction for an ambiguous row; visible-to-all is
+# not. An unscoped request (superuser, or no tenant context) still sees it, so
+# nothing becomes unreachable -- and every Forail-authored table is in this
+# group, which is where an org is assigned by our own code rather than by AWX's
+# long-standing conventions.
+#
+# Adding a table here is a decision that its NULL rows may be read by every
+# tenant. Do not add one to silence a "resource disappeared" report; assign the
+# organization instead.
+RLS_GLOBAL_NULL_ORG_TABLES = frozenset(
+    {
+        'main_credential',
+        'main_oauth2application',
+        'main_executionenvironment',
+        'main_unifiedjobtemplate',
+        'main_unifiedjob',
+    }
+)
+
+
+def null_org_is_global(table):
+    """Whether rows of ``table`` with no organization are visible to every tenant."""
+    return table in RLS_GLOBAL_NULL_ORG_TABLES
+
+
 def build_rls_policy_sql(table, org_column='organization_id'):
     """Return (create_sql, drop_sql) for a permissive RLS policy.
 
     The policy allows the row when:
     1. ``organization_id`` matches ``forail.current_tenant_id``, OR
-    2. ``organization_id`` IS NULL (global/shared resources), OR
+    2. ``organization_id`` IS NULL **and** this table is listed in
+       ``RLS_GLOBAL_NULL_ORG_TABLES`` (a NULL org is a sharing mechanism there,
+       not an unset field), OR
     3. The session variable is empty / unset (no tenant context — backwards
        compatible for non-tenant requests and superusers).
     """
@@ -303,12 +341,13 @@ def build_rls_policy_sql(table, org_column='organization_id'):
     # before the cast is evaluated. NULLIF collapses '' to NULL, which casts
     # cleanly and is caught by the IS NULL branch (→ all rows visible).
     tenant = "NULLIF(current_setting('forail.current_tenant_id', true), '')"
+    null_branch = f'OR {org_column} IS NULL ' if null_org_is_global(table) else ''
     create = (
         f'CREATE POLICY {policy_name} ON {table} '
         f'AS PERMISSIVE FOR ALL '
         f'USING ('
         f'{org_column} = {tenant}::int '
-        f'OR {org_column} IS NULL '
+        f'{null_branch}'
         f'OR {tenant} IS NULL'
         f');'
     )
@@ -319,18 +358,25 @@ def build_rls_policy_sql(table, org_column='organization_id'):
 def build_rls_policy_sql_indirect(table, fk_column, parent_table, parent_org_column):
     """Return (create_sql, drop_sql) for an indirect RLS policy.
 
-    Uses a subquery to resolve the organization from a parent table.
+    Uses a subquery to resolve the organization from a parent table. Whether a
+    NULL organization on the *parent* is global is decided by the parent's entry
+    in ``RLS_GLOBAL_NULL_ORG_TABLES``, since that is the row that carries it.
+
+    ``{fk_column} IS NULL`` stays unconditionally: a row with no parent at all
+    has no organization to compare against, and hiding it would make orphans
+    unreachable rather than merely unscoped.
     """
     policy_name = f'tenant_isolation_{table}'
     tenant = "NULLIF(current_setting('forail.current_tenant_id', true), '')"
+    parent_null_branch = f'OR {parent_org_column} IS NULL' if null_org_is_global(parent_table) else ''
     create = (
         f'CREATE POLICY {policy_name} ON {table} '
         f'AS PERMISSIVE FOR ALL '
         f'USING ('
         f'{fk_column} IN ('
         f'SELECT id FROM {parent_table} WHERE '
         f'{parent_org_column} = {tenant}::int '
-        f'OR {parent_org_column} IS NULL'
+        f'{parent_null_branch}'
         f') '
         f'OR {fk_column} IS NULL '
         f'OR {tenant} IS NULL'

forail/main/tenancy/isolation.py

@@ -73,9 +73,25 @@ def __call__(self, request):
         try:
             tenant_org = self._resolve_tenant_org(request)
         except Exception:
-            # Resolving the user's tenant org failed. A user we cannot place
-            # into a tenant is treated as unscoped (superuser / non-tenant),
-            # which is the pre-existing contract for a None result.
+            # Fail CLOSED, for the same reason set_tenant_id does below.
+            #
+            # This used to swallow the error and continue with tenant_org=None,
+            # which __call__ cannot tell apart from "superuser / not a tenant
+            # user" -- so a transient database error during the org lookup let
+            # the request run with NO RLS scope, and RLS reads an unset tenant
+            # id as "all rows". A failed lookup is not evidence that the user
+            # has no tenant; it is evidence that we do not know.
+            #
+            # Only meaningful when tenancy and RLS are on: with either off,
+            # _resolve_tenant_org returns before it touches the database, so an
+            # exception here cannot come from the lookup and a single-tenant
+            # install is not turned into a 500.
+            if getattr(settings, 'TENANCY_ENABLED', False) and getattr(settings, 'TENANCY_RLS_ENABLED', False):
+                logger.exception('TenantIsolationMiddleware: tenant org resolution failed — failing closed')
+                return JsonResponse(
+                    {'detail': 'Tenant isolation could not be established.'},
+                    status=500,
+                )
             logger.debug('TenantIsolationMiddleware: tenant org resolution failed', exc_info=True)
             tenant_org = None
 
@@ -214,14 +230,14 @@ def _resolve_tenant_org(request):
         if getattr(user, 'is_superuser', False):
             return None
 
-        try:
-            orgs = list(
-                user.organizations.filter(is_tenant_root=True)
-                .only('pk', 'is_tenant_root', 'tenant_isolation_strict')[:1]
-            )
-        except Exception:
-            logger.debug('_resolve_tenant_org: org lookup failed', exc_info=True)
-            return None
+        # Deliberately not wrapped in try/except. A database error here must
+        # reach __call__, which fails the request closed; returning None would
+        # make "the lookup broke" indistinguishable from "this user has no
+        # tenant", and the second answer runs the request unscoped.
+        orgs = list(
+            user.organizations.filter(is_tenant_root=True)
+            .only('pk', 'is_tenant_root', 'tenant_isolation_strict')[:1]
+        )
 
         return orgs[0] if orgs else None
 

tests_standalone/test_observability.py

@@ -22,6 +22,7 @@ def _load(mod_name, rel_path):
 
 
 helpers = _load('obs_helpers', 'forail/main/observability/helpers.py')
+bootstrap = _load('obs_bootstrap', 'forail/main/observability/bootstrap.py')
 
 parse_resource_attributes = helpers.parse_resource_attributes
 parse_endpoint = helpers.parse_endpoint
@@ -152,3 +153,35 @@ def test_should_recheck_is_inverse(self):
 
 if __name__ == '__main__':
     unittest.main()
+
+
+class TestEnvSyncKeys(unittest.TestCase):
+    """Which settings a deployment can drive from the environment.
+
+    Codex M2: the chart offered one tenancy switch, and only that one was
+    mirrored into the Setting registry -- so an operator who turned tenancy on
+    got the tenancy features with no row-level security behind them, because
+    TENANCY_RLS_ENABLED stayed at its False default with no way to reach it.
+    """
+
+    TENANCY_KEYS = (
+        'TENANCY_ENABLED',
+        'TENANCY_RLS_ENABLED',
+        'TENANCY_STRICT_ISOLATION_ENABLED',
+        'TENANCY_RATE_LIMITING_ENABLED',
+    )
+
+    def test_every_tenancy_control_is_syncable(self):
+        for key in self.TENANCY_KEYS:
+            self.assertIn(key, bootstrap._ENV_SYNC_KEYS, f'{key} cannot be set from the environment')
+
+    def test_every_tenancy_control_is_parsed_as_a_boolean(self):
+        # Without this the Setting row stores the string "false", which is
+        # truthy everywhere it is read.
+        for key in self.TENANCY_KEYS:
+            self.assertIn(key, bootstrap._BOOL_KEYS)
+            self.assertIs(bootstrap._coerce_env_value(key, 'false'), False)
+            self.assertIs(bootstrap._coerce_env_value(key, 'true'), True)
+
+    def test_non_bool_keys_stay_strings(self):
+        self.assertEqual(bootstrap._coerce_env_value('OTEL_SERVICE_NAME', 'forail'), 'forail')

tests_standalone/test_tenancy.py

@@ -197,6 +197,8 @@ def test_non_dict(self):
 build_rls_policy_sql_indirect = helpers.build_rls_policy_sql_indirect
 RLS_TABLES_DIRECT = helpers.RLS_TABLES_DIRECT
 RLS_TABLES_INDIRECT = helpers.RLS_TABLES_INDIRECT
+RLS_GLOBAL_NULL_ORG_TABLES = helpers.RLS_GLOBAL_NULL_ORG_TABLES
+null_org_is_global = helpers.null_org_is_global
 
 
 class TestBuildRlsPolicySql(unittest.TestCase):
@@ -221,8 +223,46 @@ def test_create_contains_bypass_clauses(self):
         create, _ = build_rls_policy_sql('main_inventory', 'organization_id')
         # Bypass when session var is unset/empty — NULLIF('') collapses to NULL.
         self.assertIn("NULLIF(current_setting('forail.current_tenant_id', true), '') IS NULL", create)
-        # Bypass when org column is NULL (shared resources)
-        self.assertIn('organization_id IS NULL', create)
+
+    def test_null_org_bypass_only_for_globally_shared_tables(self):
+        # Codex M6: this branch used to be emitted for every table, so one row
+        # saved without an organization was readable by every tenant.
+        shared, _ = build_rls_policy_sql('main_credential', 'organization_id')
+        scoped, _ = build_rls_policy_sql('main_scanner', 'organization_id')
+        self.assertIn('OR organization_id IS NULL', shared)
+        self.assertNotIn('OR organization_id IS NULL', scoped)
+
+    def test_every_forail_table_is_scoped_strictly(self):
+        # The Forail-authored tables assign the organization in our own code,
+        # so a NULL there is an unset field rather than AWX's sharing mechanism.
+        forail_tables = [
+            'main_eventrule', 'main_outboundwebhook', 'main_eventlog',
+            'main_hostfactsnapshot', 'main_driftdetection', 'main_driftalertrule',
+            'main_driftalert', 'main_policy', 'main_policydecision', 'main_scanner',
+            'main_scanresult', 'main_servicecatalogitem', 'main_auditevent',
+        ]
+        for table in forail_tables:
+            self.assertFalse(null_org_is_global(table), f'{table} must not expose NULL-org rows')
+            create, _ = build_rls_policy_sql(table, 'organization_id')
+            self.assertNotIn('OR organization_id IS NULL', create)
+
+    def test_global_tables_are_a_closed_list(self):
+        # Every entry is a deliberate decision that NULL-org rows may be read by
+        # any tenant; the assertion is here so growing the list is not quiet.
+        self.assertEqual(
+            RLS_GLOBAL_NULL_ORG_TABLES,
+            frozenset({
+                'main_credential',
+                'main_oauth2application',
+                'main_executionenvironment',
+                'main_unifiedjobtemplate',
+                'main_unifiedjob',
+            }),
+        )
+
+    def test_every_global_table_is_actually_under_rls(self):
+        covered = {table for table, _ in RLS_TABLES_DIRECT}
+        self.assertTrue(RLS_GLOBAL_NULL_ORG_TABLES.issubset(covered))
 
     def test_create_is_permissive(self):
         create, _ = build_rls_policy_sql('main_inventory', 'organization_id')
@@ -265,6 +305,19 @@ def test_create_contains_bypass(self):
         # needtofix L6: NULLIF-guarded empty/unset sentinel (was `= ''`).
         self.assertIn("NULLIF(current_setting('forail.current_tenant_id', true), '') IS NULL", create)
 
+    def test_parent_null_org_follows_the_parent_table(self):
+        # A host is visible through its inventory, so whether a NULL org is
+        # global is the inventory's question, not the host's. main_inventory is
+        # not globally shared, so hosts of an org-less inventory stay hidden
+        # from a scoped tenant.
+        create, _ = build_rls_policy_sql_indirect(
+            'main_host', 'inventory_id', 'main_inventory', 'organization_id'
+        )
+        self.assertNotIn('OR organization_id IS NULL)', create)
+        # An orphan row keeps its bypass: no parent means no organization to
+        # compare, and hiding it would make it unreachable rather than unscoped.
+        self.assertIn('OR inventory_id IS NULL', create)
+
     def test_drop_contains_policy_name(self):
         _, drop = build_rls_policy_sql_indirect(
             'main_host', 'inventory_id', 'main_inventory', 'organization_id'