PR13 — Fix ACTION_PERMISSION_MAP gap for workflow.execute/model.use

app/core/permissions.py

@@ -9,25 +9,50 @@
 reserves (app/core/permission_names.py there: workflow.execute,
 dataset.read, workflow.manage, etc) without inventing any new ones.
 
-Deliberately opt-in: enforced only when the caller actually populates
-`permissions` (a non-empty list). No role in the live system is granted
-these registry permissions yet (they're reserved-but-unassigned in
-omnibioai-auth), so every real request today still carries permissions=[]
-and this gate is a no-op there, falling back to the pre-existing
-role-based rbac.evaluate_rbac check -- exactly today's behavior, not a
-new production restriction. It only actively deters/tests once a caller
-(a fixture in these tests, or eventually a real user once
-omnibioai-auth grants these permissions to a role) supplies a non-empty
-permissions list. This is intentional scope containment for PR12 -- see
-this PR's report for why granting these permissions to real roles is out
-of scope (would require a DB/role change in omnibioai-auth).
+Deliberately opt-in: still a no-op for a caller who supplies
+permissions=[] (falls back to the pre-existing role-based
+rbac.evaluate_rbac check), so nothing here changes behavior for traffic
+that predates permission-awareness entirely.
+
+PR13 (Dynamic Permission Assignment & Enterprise RBAC Activation):
+omnibioai-auth now grants these registry permissions to real roles
+(scientist/viewer, and any org's custom roles), so `permissions` is
+populated for real traffic, not just this file's own test fixtures --
+this gate is now genuinely live, not aspirational. That PR also fixed a
+real gap in ACTION_PERMISSION_MAP below: workbench/tes/toolserver/
+model-registry (4 of the 5 gateway-routed services) had zero real
+enforcement through this gate regardless of what permissions a caller
+held, because the Gateway sends `action` as the literal permission string
+itself ("workflow.execute"/"model.use") for those services, which matched
+neither this map nor PREFIX_PERMISSION_MAP's "tes." prefix rule -- only
+"dataset.read" (rag) was ever actually enforced. See
+tests/test_permissions.py's workflow.execute/model.use regression tests.
 """
 
 from typing import List, Optional
 
 # action (exact match) -> required permission
+#
+# PR13: workflow.execute/model.use added. The Gateway's PolicyClient
+# (omnibioai-api-gateway app/services/policy_client.py) pre-resolves
+# `action` from SERVICE_PERMISSION_MAP for its 5 mapped services -- for
+# workbench/tes/toolserver it sends the literal string "workflow.execute",
+# for model-registry "model.use". Before this PR, only "dataset.read" (rag)
+# had a matching entry here; the other two matched neither this map nor
+# PREFIX_PERMISSION_MAP's "tes." prefix (the literal string "workflow.execute"
+# does not start with "tes." -- that prefix only ever matched this module's
+# own synthetic test fixtures, never real gateway traffic), so
+# required_permission() silently returned None and evaluate_permission
+# allowed unconditionally for 4 of 5 gateway-routed services regardless of
+# what the caller's permissions were. These two entries are identity
+# mappings by design, matching how "dataset.read" already (correctly)
+# worked -- the map's job for gateway-routed traffic is confirming "yes,
+# this action requires this permission," not translating one string to
+# another.
 ACTION_PERMISSION_MAP = {
     "dataset.read": "dataset.read",
+    "workflow.execute": "workflow.execute",
+    "model.use": "model.use",
 }
 
 # action prefix -> required permission

tests/test_engine_permission_tenancy.py

@@ -65,6 +65,39 @@ def test_permission_allow_falls_through_to_all_passed():
     assert decision.policy_source == "ALL_PASSED"
 
 
+def test_permission_deny_for_real_gateway_action_shape_workflow_execute():
+    """PR13 regression lock: the tests above all use action="tes.submit",
+    which is never what real gateway-routed traffic actually sends for
+    workbench/tes/toolserver (the Gateway's PolicyClient pre-resolves
+    `action` to the literal permission string itself, per
+    SERVICE_PERMISSION_MAP) -- this is that real shape, and would have
+    caught the ACTION_PERMISSION_MAP gap this PR fixes (previously
+    required_permission("workflow.execute", ...) returned None, so this
+    request was allowed unconditionally regardless of `permissions`)."""
+    engine, _ = make_engine()
+    req = basic_request(
+        roles=[], permissions=["dataset.read"],  # populated, but missing workflow.execute
+        action="workflow.execute", resource="workbench",
+    )
+
+    decision = engine._evaluate_core(req)
+
+    assert decision.allowed is False
+    assert decision.policy_source == "PERMISSION"
+
+
+def test_permission_allow_for_real_gateway_action_shape_model_use():
+    engine, _ = make_engine()
+    req = basic_request(
+        roles=[], permissions=["model.use"], action="model.use", resource="model-registry",
+    )
+
+    decision = engine._evaluate_core(req)
+
+    assert decision.allowed is True
+    assert decision.policy_source == "ALL_PASSED"
+
+
 def test_permission_check_is_noop_for_legacy_role_only_traffic():
     """No permissions supplied at all (today's real production shape) must
     behave exactly as it did before this PR."""

tests/test_permissions.py

@@ -27,6 +27,54 @@ def test_required_permission_for_tes_prefix():
     assert required_permission("tes.submit", "job") == "workflow.execute"
 
 
+# ---------------------------------------------------------------------------
+# PR13: regression lock for a real gap this map had -- the Gateway's
+# PolicyClient sends the literal string "workflow.execute" (for
+# workbench/tes/toolserver) and "model.use" (for model-registry) as
+# `action` for its 5 gateway-routed services, never a "tes.*"-prefixed
+# string. Before PR13, neither literal matched anything in this map (the
+# "tes." prefix rule only ever matched this test file's own synthetic
+# "tes.submit" fixtures, never real gateway traffic), so
+# required_permission() silently returned None for those two and
+# evaluate_permission allowed unconditionally regardless of the caller's
+# permissions -- 4 of 5 gateway-routed services had zero real enforcement
+# through this engine no matter what permissions a caller did or didn't
+# have. These are the exact action shapes real traffic sends, not just the
+# "tes.submit"-style ones every other test in this file uses.
+# ---------------------------------------------------------------------------
+
+
+def test_required_permission_for_workflow_execute_action():
+    assert required_permission("workflow.execute", "job") == "workflow.execute"
+
+
+def test_required_permission_for_model_use_action():
+    assert required_permission("model.use", "model") == "model.use"
+
+
+def test_scientist_denied_workflow_publish_via_gateway_shaped_action():
+    """Scientist's permission set (per this PR's role matrix) has
+    workflow.execute/dataset.read/model.use but not workflow.publish."""
+    allowed, reason = evaluate_permission(
+        [], ["workflow.execute", "dataset.read", "model.use"], "model.use", "model"
+    )
+    assert allowed is True
+
+    allowed, reason = evaluate_permission(
+        [], ["workflow.execute", "dataset.read", "model.use"], "workflow.execute", "job"
+    )
+    assert allowed is True
+
+
+def test_viewer_denied_workflow_execute_via_gateway_shaped_action():
+    """Viewer's permission set (dataset.read/workflow.read) does not
+    include workflow.execute -- the exact scenario a gateway-routed
+    workbench/tes/toolserver request from a Viewer must be denied."""
+    allowed, reason = evaluate_permission([], ["dataset.read", "workflow.read"], "workflow.execute", "job")
+    assert allowed is False
+    assert "workflow.execute" in reason
+
+
 def test_required_permission_for_dataset_read():
     assert required_permission("dataset.read", "human_genome") == "dataset.read"