Redact bnk_config from the instance-wide global cluster list

backend/routes/k8s/_shared.py

@@ -26,6 +26,7 @@ def serialize_cluster(
     cluster: KubernetesCluster,
     include_project_id: bool = True,
     membership: "tuple[list[int], list[int]] | None" = None,
+    include_bnk_config: bool = True,
 ) -> dict:
     """
     Serialize a KubernetesCluster to dict.
@@ -34,6 +35,13 @@ def serialize_cluster(
     membership: pre-fetched (host_ids, dpu_ids) for BNK clusters in list
     contexts.  When provided, _serialize_bnk_config uses it directly instead
     of issuing per-cluster queries (eliminates the 2N pattern; ADR-424 finding C).
+
+    include_bnk_config: when False, bnk_config is redacted (None). The
+    ADR-424 bnk_config carries cross-project infrastructure membership
+    (host_ids, dpu_ids, control_plane_host_id, tmfifo_pool_cidr); the global
+    instance-wide list must not leak it to any viewer (#116). The
+    project-scoped list and the per-cluster detail keep it (their callers
+    are the surfaces that actually consume it).
     """
     platform_context = PlatformContextService.serialize_cluster_context(cluster)
 
@@ -68,7 +76,11 @@ def serialize_cluster(
         "deployable_release_id": cluster.deployable_release_id,
         "running_release_id": cluster.running_release_id,
         # BNK multi-host cluster configuration side-table (ADR-424)
-        "bnk_config": _serialize_bnk_config(cluster, membership=membership) if getattr(cluster, "bnk_config", None) else None,
+        "bnk_config": (
+            _serialize_bnk_config(cluster, membership=membership)
+            if include_bnk_config and getattr(cluster, "bnk_config", None)
+            else None
+        ),
     }
     if include_project_id:
         result["project_id"] = cluster.project_id

backend/services/bnk_cluster_service.py

@@ -81,7 +81,8 @@ def bulk_cluster_membership(
     ) -> dict[int, tuple[list[int], list[int]]]:
         """Return {cluster_id: (host_ids, dpu_ids)} for multiple clusters in 2 queries.
 
-        Eliminates the 2N query pattern in list_all_clusters / list_project_clusters:
+        Eliminates the 2N query pattern in list_project_clusters (list_all_clusters
+        no longer serializes bnk_config, so it does not call this -- #116):
         instead of 2 queries per BNK cluster, run one grouped host query and one
         grouped DPU query, then bucket by cluster_id.  Single-sources the query
         logic used by cluster_membership (ADR-424 finding C).

backend/services/cluster_management_service.py

@@ -347,30 +347,43 @@ def create_cluster(self, project_id: int, cluster_data) -> dict[str, Any]:
         }
 
     def list_all_clusters(self) -> dict[str, Any]:
-        """List all Kubernetes clusters (global)."""
+        """List all Kubernetes clusters (global).
+
+        This endpoint is instance-wide (require_viewer, not project-scoped), so
+        it must not expose the ADR-424 bnk_config -- host/DPU membership,
+        control-plane host, tmfifo pool CIDR -- cross-project to any viewer
+        (#116). bnk_config is redacted here; the project-scoped list and the
+        per-cluster detail keep it. No frontend consumer of this global list
+        reads bnk_config (only the project-scoped K8sClusterList does), so this
+        removes the disclosure without losing a feature -- and it also drops the
+        now-unnecessary membership bulk-fetch those fields required.
+        """
         from sqlalchemy.orm import selectinload
 
         from routes.k8s._shared import serialize_cluster
-        from services.bnk_cluster_service import BnkClusterService
 
         clusters = (
             self.db.query(KubernetesCluster)
             .options(selectinload(KubernetesCluster.bnk_config))
             .all()
         )
-        # Bulk-fetch membership for all BNK clusters in 2 queries (not 2N).
-        # selectinload(bnk_config) already avoids the config N+1; this bulk
-        # call eliminates the host+DPU membership N+1 in _serialize_bnk_config.
-        bnk_ids = [c.id for c in clusters if getattr(c, "bnk_config", None)]
-        membership_map = BnkClusterService(self.db).bulk_cluster_membership(bnk_ids)
-        result = [
-            serialize_cluster(c, membership=membership_map.get(c.id))
-            for c in clusters
-        ]
+        result = [serialize_cluster(c, include_bnk_config=False) for c in clusters]
         return {"clusters": result, "count": len(result)}
 
     def list_project_clusters(self, project_id: int) -> dict[str, Any]:
-        """List all Kubernetes clusters for a project."""
+        """List all Kubernetes clusters for a project.
+
+        NOTE (#116): this list still renders bnk_config, and its route is
+        require_viewer (any authenticated user) with NO membership/ownership
+        check -- project_id is a path param anyone may supply. Combined with the
+        global list (which still returns each cluster's project_id), any viewer
+        can read any project's bnk_config in two requests. Redacting bnk_config
+        on the global list (this change) is a strict improvement but does NOT
+        fully close #116: the project list must enforce per-project membership
+        first, and that is a pre-existing tenancy-model decision (require_viewer
+        is role-based across the app) larger than this change. Until that lands,
+        bnk_config here is readable by any authenticated user.
+        """
         from sqlalchemy.orm import selectinload
 
         from routes.k8s._shared import serialize_cluster
@@ -392,7 +405,17 @@ def list_project_clusters(self, project_id: int) -> dict[str, Any]:
         return {"clusters": result, "count": len(result)}
 
     def get_cluster_details(self, cluster_id: int) -> dict[str, Any]:
-        """Get cluster details."""
+        """Get cluster details.
+
+        NOTE (#116): GET /k8s/clusters/{cluster_id} is require_viewer with NO
+        project scope (unlike the PUT/DELETE on the same path, which use
+        require_cluster_owner). This handler hand-builds its dict and must NOT
+        gain a bnk_config key -- reusing serialize_cluster here (or adding
+        bnk_config by hand) would reintroduce the cross-project disclosure #116
+        closes, one request further along, and the id needed comes straight from
+        the global list. If bnk_config is ever needed on detail, scope this route
+        to the project first.
+        """
         cluster = self._get_cluster(cluster_id)
         context = PlatformContextService.serialize_cluster_context(cluster)
         return {

backend/tests/component/test_bnk_cluster_member_assignment.py

@@ -628,11 +628,14 @@ def test_hosts_and_dpus_bucketed_by_cluster(self, db):
     def test_serialize_cluster_with_bnk_config_and_members(
         self, client, db, admin_headers, sample_user, make_project, make_k8s_cluster
     ):
-        """GET /api/k8s/clusters renders bnk_config.host_ids / dpu_ids correctly
-        for a cluster that has a BnkClusterConfig with members assigned.
+        """The project-scoped cluster list renders bnk_config.host_ids / dpu_ids
+        correctly; the instance-wide global list redacts bnk_config (#116).
 
-        Verifies _serialize_bnk_config branch (bnk_config present) and that
-        host IDs do not leak into the dpu_ids bucket (B6).
+        Verifies _serialize_bnk_config branch (bnk_config present on the
+        project-scoped list) and that host IDs do not leak into the dpu_ids
+        bucket (B6). The subject moved from the global list to the project list
+        because #116 redacts bnk_config on the global path -- so this also pins
+        the redaction: present when project-scoped, absent when global.
 
         Two hosts and two DPUs ensure that IDs are distinct across the tables
         (SQLite auto-increments per-table from 1, so IDs can coincide with a
@@ -660,16 +663,19 @@ def test_serialize_cluster_with_bnk_config_and_members(
         )
         assert r.status_code == 200, r.text
 
-        # Fetch the cluster list — exercises bulk_cluster_membership + serialize_cluster.
-        resp = client.get("/api/k8s/clusters", headers=admin_headers)
+        # Project-scoped list — exercises bulk_cluster_membership + serialize_cluster,
+        # and is the surface that still renders bnk_config after #116.
+        resp = client.get(
+            f"/api/projects/{project.id}/k8s/clusters", headers=admin_headers
+        )
         assert resp.status_code == 200, resp.text
 
         clusters = resp.json()["clusters"]
         target = next((c for c in clusters if c["id"] == cluster.id), None)
         assert target is not None, f"cluster {cluster.id} not in response"
 
         bnk = target.get("bnk_config")
-        assert bnk is not None, "bnk_config must be present for a BNK cluster"
+        assert bnk is not None, "bnk_config must be present on the project-scoped list"
 
         assert sorted(bnk["host_ids"]) == sorted([host1.id, host2.id]), (
             f"host_ids must contain the two assigned hosts, got {bnk['host_ids']}"
@@ -685,6 +691,18 @@ def test_serialize_cluster_with_bnk_config_and_members(
             f"host_ids must have exactly 2 entries (no DPU leakage), got {bnk['host_ids']}"
         )
 
+        # #116: the instance-wide global list must NOT leak bnk_config to any
+        # viewer, even though the same cluster carries it on the project list.
+        global_resp = client.get("/api/k8s/clusters", headers=admin_headers)
+        assert global_resp.status_code == 200, global_resp.text
+        global_target = next(
+            (c for c in global_resp.json()["clusters"] if c["id"] == cluster.id), None
+        )
+        assert global_target is not None
+        assert global_target.get("bnk_config") is None, (
+            "global list leaked bnk_config cross-project (#116)"
+        )
+
 
 # ---------------------------------------------------------------------------
 # B6 — _require_cp_member=True BadRequestError path

backend/tests/component/test_cluster_management_service.py

@@ -235,6 +235,33 @@ def test_list_project_clusters_filters(self, db, make_project, make_k8s_cluster)
         assert result["count"] == 1
         assert result["clusters"][0]["name"] == "c1"
 
+    def test_global_list_redacts_bnk_config_but_project_list_keeps_it(
+        self, db, make_project, make_k8s_cluster
+    ):
+        """#116: the instance-wide global list must not leak ADR-424 bnk_config
+        (host/DPU membership, control-plane host, tmfifo pool CIDR) cross-project
+        to any viewer. The project-scoped list -- whose caller actually renders
+        it -- must still include it."""
+        from models.kubernetes import BnkClusterConfig
+
+        p = make_project()
+        cluster = make_k8s_cluster(project=p, name="bnk-cluster")
+        db.add(BnkClusterConfig(cluster_id=cluster.id, tmfifo_pool_cidr="192.168.100.0/22"))
+        db.commit()
+
+        svc = ClusterManagementService(db)
+
+        global_row = next(
+            c for c in svc.list_all_clusters()["clusters"] if c["name"] == "bnk-cluster"
+        )
+        assert global_row["bnk_config"] is None, "global list leaked bnk_config (#116)"
+
+        project_row = next(
+            c for c in svc.list_project_clusters(p.id)["clusters"] if c["name"] == "bnk-cluster"
+        )
+        assert project_row["bnk_config"] is not None, "project-scoped list must keep bnk_config"
+        assert project_row["bnk_config"]["tmfifo_pool_cidr"] == "192.168.100.0/22"
+
     def test_list_project_clusters_nonexistent_project(self, db):
         svc = ClusterManagementService(db)
         with pytest.raises(NotFoundError):