Enforce access checks on mutating routes + add coverage guard

[ruizhang0519]

Tracking: #2234 (step 0 of the RBAC enablement sequence).

RBAC is opt-in per endpoint: a mutation is only authorized if its handler reaches AccessChecker.check(). With the permissive default, an endpoint that never calls check() fails open silently — nothing turns red today, but it becomes a hole once a namespace is governed. Several mutating endpoints were uncovered (remove_complex_dimension_link built an access request and never checked it), and nothing guarded against the next one.

This PR makes the mutating-route surface covered and keeps it that way, with two guards that answer different questions:

• Is a check wired? tests/test_route_coverage.py enumerates every mutating route and asserts each reaches check() or is excluded with a reason. 67 routes reach a check; 30 are excluded across 7 reason buckets.
• Does the check bite? tests/api/write_enforcement_test.py asserts every mutating route has a denial test, is excluded, or is in a backlog with a reason. 19 cases execute (16 deny-WRITE, 3 deny-DELETE), covering 26 routes; 41 remain backlogged by reason.

The second guard exists because the first cannot tell a correct check from a wrong one. Review found exactly that case: upsert_materialization reached a READ check on a cube's metrics while writing to the node itself, so the coverage guard was green while transforms, dimensions and Druid cubes had no WRITE gate at all. A caller denied WRITE could create or overwrite a materialization config and reschedule its job. Fixed, with a denial test.

Checks added, each WRITE on the affected resource: dimension-link removal (node), namespace create (parent boundary), materialization writes including the upsert gap above (node), cube writes (cube node), and preaggregation writes (the node the pre-agg is based on).

Intentionally excluded: POST /preaggs/{id}/availability is a query-service callback, not a user action; governing it with user WRITE would break callbacks, so it awaits a service-identity model.

Also from review: create_or_reactivate_namespace moved to internal/namespaces.py next to the create_namespace it delegates to, and returns a NamespaceWriteResult instead of a JSONResponse so HTTP shaping stays in the endpoint. Status codes and bodies are unchanged. The already-exists case returns a status rather than raising, because register_table / register_view discard the result and only need the namespace to exist.

---

Why this won't take effect now: DJ's default access policy is permissive, so these checks allow requests unless an explicit restrictive grant or policy denies them.

When this will start taking effect: once a deployment enables restrictive RBAC and governs a namespace. Until then this is a no-op.

Out of scope (step-0 follow-ups): action/resource correctness, authorization on non-HTTP (deployment and background) paths, the availability service-identity model, and the 41 backlogged denial tests.

---

Verification:

1. Both guards, plus every executing denial case.

uv run pytest tests/api/write_enforcement_test.py tests/test_route_coverage.py
23 passed

2. Denial tests that need real fixtures, next to those fixtures.

uv run pytest tests/api/preaggregations_test.py
83 passed

3. The upsert_materialization gap, before and after.

# before the fix: the handler loaded the node and never authorized
POST /nodes/{node}/materialization -> 404
# after
POST /nodes/{node}/materialization -> 403 Access denied

4. Full server suite with the 100% coverage gate.

make test  ->  green, --cov-fail-under=100 satisfied

[netlify]

✅ Deploy Preview for thriving-cassata-78ae72 canceled.

Name	Link
🔨 Latest commit	6d35ddd
🔍 Latest deploy log	https://app.netlify.com/projects/thriving-cassata-78ae72/deploys/6a667f917b96ad0008a170ff

[shangyian]

Can this be moved into an internal/namespaces.py?

[ruizhang0519]

Moved it to internal/namespaces.py, next to create_namespace. Also dropped the JSONResponse return so HTTP shaping stays in the endpoint — it now returns a NamespaceWriteResult (created / reactivated / already_exists). Status codes and bodies unchanged.

Already-exists returns a status rather than raising, since register_* discard the result and just need the namespace to exist. Added a test to pin that.

[philipfweiss]

OK, so after some stress testing found an issue. upsert_materialization never checks WRITE on node_name.

create_new_materialization sends transforms and dimensions through build_non_cube_materialization_config, which is not passed an access_checker, and the DRUID_CUBE job through build_cube_materialization, which also does not check. The only branch that reaches .check() is the non-DRUID cube job, and that check is READ on the cube's metrics and dimensions, not WRITE on the node being materialized. So there is no WRITE gate on this endpoint for any node type.

Quick Example:
Let's say Restrictive RBAC governs finance; Alice has no grant on it.

POST /nodes/finance.orders/materializations/{name}/backfill returns 403

POST /nodes/finance.orders/materialization with a valid transform body returns 201: DJ writes the materialization, records history, and schedules the workflow. Because upsert updates in place, Alice can also overwrite the owner's existing config and reschedule it.

Suggested fix:
Add access_checker.add_request_by_node_name(node_name, ResourceAction.WRITE) and await access_checker.check(on_denied=AccessDenialMode.RAISE) at the top of the handler, matching deactivate and backfill.

[ruizhang0519]

Good catch — confirmed and fixed: added the WRITE check at the top of upsert_materialization, matching deactivate and backfill.

I also added a completeness test: every mutating route now needs a denial test or an explicit backlog entry. The denial suite is up to 21 cases (including a deny-DELETE variant), with 42 routes backlogged with reasons.