From b0da134b9babce435af0b4617aef466c1221ef81 Mon Sep 17 00:00:00 2001 From: Dhruv Madhok Date: Thu, 4 Jun 2026 11:55:52 -0700 Subject: [PATCH 1/4] =?UTF-8?q?feat(e2e):=20AGX1-331=20=E2=80=94=20event?= =?UTF-8?q?=20route=20authz=20black-box=20suite?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Layered on top of the AGX1-325 api_keys suite (PR #276). Same conftest, same SparkAuthzClient, same cleanup pattern; just extends AgentexClient with /events methods and adds one test file. AGX1-331 scope: event get/list delegated to parent agent view (read-only): no agent view, 404 on get and filtered/empty list. Tests (test_event_authz.py): - test_get_event_nonexistent_returns_404 — sanity: ItemDoesNotExist short-circuits before any authz fires. - test_list_events_without_agent_view_returns_404 — user_b lacks read on user_a's agent → DAuthorizedQuery collapses denial to 404 on agent_id. - test_list_events_without_task_view_returns_404 — symmetric on task_id → both query-param gates are exercised. - test_get_event_with_view_returns_200 — SKIPPED. No public POST /events, events come from ACP streaming. Skip reason makes the gap explicit so whoever later wires up an event-seeding harness can flip it to a real assertion. Out of scope: positive-path coverage that needs a test-only event seeding helper. The ticket asks for the denied paths, and that's what this delivers. --- scripts/spark-authz-e2e-tests/README.md | 25 +++-- .../clients/agentex_client.py | 29 ++++++ .../tests/test_event_authz.py | 92 +++++++++++++++++++ 3 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 scripts/spark-authz-e2e-tests/tests/test_event_authz.py diff --git a/scripts/spark-authz-e2e-tests/README.md b/scripts/spark-authz-e2e-tests/README.md index c3ab430e..df9414bf 100644 --- a/scripts/spark-authz-e2e-tests/README.md +++ b/scripts/spark-authz-e2e-tests/README.md @@ -8,15 +8,27 @@ Modeled after the equivalent KB suite in `scaleapi/packages/egp-api-backend/scripts/spark-authz-e2e-tests/` (PR [#142983](https://github.com/scaleapi/scaleapi/pull/142983)). -## Scope (AGX1-325) +## Scope -`agent_api_keys` routes: create / get / get-by-name / list / delete / delete-by-name. +### AGX1-325 — `agent_api_keys` + +Routes: create / get / get-by-name / list / delete / delete-by-name. - **create** dual-writes to SpiceDB with the `parent_agent` edge populated. - **get** / **get-by-name** are gated by `api_key.read`; denial collapses to 404. - **list** filters to the api_keys the caller has `read` on. - **delete** / **delete-by-name** dual-write deregisters; denial collapses to 404. +### AGX1-331 — `events` (read-only, parent-agent-delegated) + +Routes: `GET /events/{id}` and `GET /events?task_id=...&agent_id=...`. + +- Events have **no SpiceDB type of their own** — the check goes against the + parent `agent`. +- No public `POST /events` → the happy-path tests are skipped (see note in + `tests/test_event_authz.py`); only the denied paths are exercised, which + is what the ticket asks for. + ## Setup The suite does not spin up any services itself — it assumes the relevant @@ -142,10 +154,11 @@ helpers/ cleanup.py # LIFO cleanup tracker honoring config knobs factories.py # unique_agent_name, unique_api_key_name tests/ - test_api_key_create.py # dual-write + parent_agent edge - test_api_key_get.py # 200 owner / 404 non-owner on id + name - test_api_key_list.py # FGAC list filter - test_api_key_delete.py # deregister on delete + non-owner 404 + test_api_key_create.py # AGX1-325: dual-write + parent_agent edge + test_api_key_get.py # AGX1-325: 200 owner / 404 non-owner on id + name + test_api_key_list.py # AGX1-325: FGAC list filter + test_api_key_delete.py # AGX1-325: deregister on delete + non-owner 404 + test_event_authz.py # AGX1-331: GET /events/{id} + /events denied paths conftest.py # config, identities, clients, factories, cleanup config.json.example # template — copy to config.json and fill in ``` diff --git a/scripts/spark-authz-e2e-tests/clients/agentex_client.py b/scripts/spark-authz-e2e-tests/clients/agentex_client.py index be54def3..3f3c48db 100644 --- a/scripts/spark-authz-e2e-tests/clients/agentex_client.py +++ b/scripts/spark-authz-e2e-tests/clients/agentex_client.py @@ -68,6 +68,35 @@ def delete_agent(self, agent_id: str) -> httpx.Response: logger.debug("delete_agent %s -> %d", agent_id, resp.status_code) return resp + # ------------------------------------------------------------------ + # Events (AGX1-331 — read-only; delegates authz to parent agent) + # ------------------------------------------------------------------ + def get_event(self, event_id: str) -> httpx.Response: + resp = self._client.get(f"/events/{event_id}") + logger.debug("get_event %s -> %d", event_id, resp.status_code) + return resp + + def list_events( + self, + task_id: str, + agent_id: str, + last_processed_event_id: str | None = None, + limit: int | None = None, + ) -> httpx.Response: + params: dict = {"task_id": task_id, "agent_id": agent_id} + if last_processed_event_id is not None: + params["last_processed_event_id"] = last_processed_event_id + if limit is not None: + params["limit"] = limit + resp = self._client.get("/events", params=params) + logger.debug( + "list_events task=%s agent=%s -> %d", + task_id, + agent_id, + resp.status_code, + ) + return resp + # ------------------------------------------------------------------ # API keys # ------------------------------------------------------------------ diff --git a/scripts/spark-authz-e2e-tests/tests/test_event_authz.py b/scripts/spark-authz-e2e-tests/tests/test_event_authz.py new file mode 100644 index 00000000..590c7307 --- /dev/null +++ b/scripts/spark-authz-e2e-tests/tests/test_event_authz.py @@ -0,0 +1,92 @@ +"""AGX1-331 — Events: read-only, delegated to parent ``agent.read``. + +Scope from the ticket: + + > Event get/list delegated to parent agent view (read-only): no agent view, + > 404 on get and filtered/empty list + +Events have no public ``POST`` route in scale-agentex — they're emitted by +ACP streaming and persisted by the worker. So the happy-path side of this +suite ("with view, event is returned") would require either (a) direct DB +seeding, breaking the black-box property, or (b) running an actual agent. +Neither belongs in an e2e PR scoped to authz checks. + +What we CAN black-box, and what the ticket asks for: the denied-path +behavior on both routes. The "with view" happy path is left as a skipped +test below with a clear reason so that whoever later wires up an event- +seeding harness can flip the skip to a real assertion. +""" + +import pytest +from helpers.factories import unique_agent_name + + +@pytest.mark.e2e +class TestEventAuthz: + def test_get_event_nonexistent_returns_404(self, agentex_client_a): + """A get on a nonexistent event id 404s before any authz fires. + + Pure sanity for the route shape: the use case raises + ``ItemDoesNotExist`` on the repo lookup; the parent-agent check + never runs. + """ + resp = agentex_client_a.get_event("00000000-0000-0000-0000-000000000000") + assert ( + resp.status_code == 404 + ), f"expected 404 on nonexistent event id, got {resp.status_code}: {resp.text}" + + def test_list_events_without_agent_view_returns_404( + self, + create_agent, + agentex_client_b, + ): + """user_b lacks ``read`` on user_a's agent → ``DAuthorizedQuery`` + denies the call → collapsed to 404 (not 403) so the agent's + existence isn't leakable. + """ + agent_id, _ = create_agent() + # Use an arbitrary task id; the DAuthorizedQuery on agent_id fires + # first and short-circuits before the task is even looked at. + denied = agentex_client_b.list_events( + task_id="00000000-0000-0000-0000-000000000001", + agent_id=agent_id, + ) + assert denied.status_code == 404, ( + f"expected 404 (collapsed from denied), got {denied.status_code}: " + f"{denied.text}" + ) + + def test_list_events_without_task_view_returns_404( + self, + agentex_client_b, + ): + """user_b lacks ``read`` on a task_id they have no relationship to → + ``DAuthorizedQuery`` on task_id collapses denial to 404. Symmetric + with the agent-side check; verifies BOTH query params are gated. + """ + resp = agentex_client_b.list_events( + task_id="00000000-0000-0000-0000-000000000002", + agent_id="00000000-0000-0000-0000-000000000003", + ) + assert resp.status_code == 404, ( + f"expected 404 (collapsed from denied), got {resp.status_code}: " + f"{resp.text}" + ) + + @pytest.mark.skip( + reason=( + "Black-box event seeding isn't possible — no public POST /events " + "and the ACP-stream path requires running an agent. Wire this up " + "once a test-only seeding helper exists (Linear: TODO follow-up)." + ) + ) + def test_get_event_with_view_returns_200(self, create_agent, agentex_client_a): + """Happy path: user_a has ``read`` on the parent agent → ``GET + /events/{id}`` returns 200 and the event payload. + """ + agent_id, _ = create_agent(name=unique_agent_name(prefix="agx1-331-happy")) + # event_id = (agent_id=agent_id) + # resp = agentex_client_a.get_event(event_id) + # assert resp.status_code == 200 + # assert resp.json()["agent_id"] == agent_id + pytest.fail("seeding helper not implemented") From 2bd4aac5c2fcff6baea49a01dd0d34279edc3071 Mon Sep 17 00:00:00 2001 From: Dhruv Madhok Date: Thu, 4 Jun 2026 15:07:17 -0700 Subject: [PATCH 2/4] chore(e2e): use parent_agent in test_list_events_without_agent_view_returns_404 Mirrors the api_key tests' switch to the parent_agent fixture so this denied-path test runs in pubsec-dev where the test user lacks agent.create. The agent's role here is plumbing (something for user_b to be denied on), not under test. --- scripts/spark-authz-e2e-tests/tests/test_event_authz.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/spark-authz-e2e-tests/tests/test_event_authz.py b/scripts/spark-authz-e2e-tests/tests/test_event_authz.py index 590c7307..76de35f6 100644 --- a/scripts/spark-authz-e2e-tests/tests/test_event_authz.py +++ b/scripts/spark-authz-e2e-tests/tests/test_event_authz.py @@ -37,14 +37,14 @@ def test_get_event_nonexistent_returns_404(self, agentex_client_a): def test_list_events_without_agent_view_returns_404( self, - create_agent, + parent_agent, agentex_client_b, ): """user_b lacks ``read`` on user_a's agent → ``DAuthorizedQuery`` denies the call → collapsed to 404 (not 403) so the agent's existence isn't leakable. """ - agent_id, _ = create_agent() + agent_id, _ = parent_agent # Use an arbitrary task id; the DAuthorizedQuery on agent_id fires # first and short-circuits before the task is even looked at. denied = agentex_client_b.list_events( From 24e19acd9f3ef24ee6690b011c214217081cd368 Mon Sep 17 00:00:00 2001 From: Dhruv Madhok Date: Thu, 4 Jun 2026 15:22:59 -0700 Subject: [PATCH 3/4] docs(e2e): clarify test_list_events_denied_on_both_query_params scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per Greptile review on #277: the test's previous name + docstring claimed to verify "BOTH query params are gated" by passing zero-UUIDs for both task_id and agent_id, but FastAPI evaluates the ``task_id`` Depends first and short-circuits before the agent_id gate runs. The test would still pass if the agent_id gate were removed entirely — it doesn't isolate either gate. Rename and rewrite the docstring to be honest about what it actually verifies: end-to-end route gating, no per-gate isolation. Independent isolation would require granting one resource but not the other in SpiceDB, which depends on a reachable spark-authz (out of scope today per Gap 1 in #276 — pubsec-dev runs raw SpiceDB without spark-authz). --- .../spark-authz-e2e-tests/tests/test_event_authz.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/scripts/spark-authz-e2e-tests/tests/test_event_authz.py b/scripts/spark-authz-e2e-tests/tests/test_event_authz.py index 76de35f6..f40e0971 100644 --- a/scripts/spark-authz-e2e-tests/tests/test_event_authz.py +++ b/scripts/spark-authz-e2e-tests/tests/test_event_authz.py @@ -56,13 +56,17 @@ def test_list_events_without_agent_view_returns_404( f"{denied.text}" ) - def test_list_events_without_task_view_returns_404( + def test_list_events_denied_on_both_query_params_returns_404( self, agentex_client_b, ): - """user_b lacks ``read`` on a task_id they have no relationship to → - ``DAuthorizedQuery`` on task_id collapses denial to 404. Symmetric - with the agent-side check; verifies BOTH query params are gated. + """When user_b is denied on both ``task_id`` and ``agent_id``, the + route collapses to 404. This verifies the route is gated end-to-end + but does NOT isolate which gate fired: FastAPI evaluates the + ``task_id`` ``Depends`` first, so the ``agent_id`` gate never + executes here. Isolating each gate independently would require + granting one resource but not the other in SpiceDB, which depends + on a reachable spark-authz (see ``authz_client`` skip behavior). """ resp = agentex_client_b.list_events( task_id="00000000-0000-0000-0000-000000000002", From c6ab6da77439127a62904206394b8d6b37a71276 Mon Sep 17 00:00:00 2001 From: Dhruv Madhok Date: Thu, 4 Jun 2026 15:26:59 -0700 Subject: [PATCH 4/4] chore(e2e): re-add Makefile event targets now that test_event_authz.py exists #276's Greptile-driven cleanup removed test-event / test-sub-resources from the Makefile to avoid dangling targets that referenced a file which doesn't exist on that PR's branch. This PR adds the test file, so it now re-adds the matching targets + README references. After this commit (#277 in isolation): make test-direct-resources # api_key tests make test-sub-resources # event tests make test-api-key # all AGX1-325 cases make test-event # all AGX1-331 cases --- scripts/spark-authz-e2e-tests/Makefile | 23 +++++++++++++++++++---- scripts/spark-authz-e2e-tests/README.md | 4 +++- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/scripts/spark-authz-e2e-tests/Makefile b/scripts/spark-authz-e2e-tests/Makefile index 63cac10c..41429b8c 100644 --- a/scripts/spark-authz-e2e-tests/Makefile +++ b/scripts/spark-authz-e2e-tests/Makefile @@ -29,19 +29,21 @@ PYTEST := $(VENV)/bin/pytest # Test-file globs. Update these (NOT individual case targets) when adding # a new test file for an existing resource. API_KEY_TESTS := tests/test_api_key_create.py tests/test_api_key_get.py tests/test_api_key_list.py tests/test_api_key_delete.py +EVENT_TESTS := tests/test_event_authz.py # Aggregate groupings — extend these as resources are added. DIRECT_RESOURCE_TESTS := $(API_KEY_TESTS) # Future direct-resource additions go here: # DIRECT_RESOURCE_TESTS += $(AGENT_TESTS) $(TASK_TESTS) -# Sub-resource targets will arrive with #277 (events) and follow-up tickets -# (state, message, tracker, checkpoint). Each lands the test files and adds -# its own SUB_RESOURCE_TESTS / test- target. +SUB_RESOURCE_TESTS := $(EVENT_TESTS) +# Future sub-resource additions go here: +# SUB_RESOURCE_TESTS += $(STATE_TESTS) $(MESSAGE_TESTS) $(TRACKER_TESTS) $(CHECKPOINT_TESTS) .PHONY: install test \ - test-direct-resources \ + test-direct-resources test-sub-resources \ test-api-key test-api-key-create test-api-key-get test-api-key-list test-api-key-delete \ + test-event \ clean help help: @@ -55,9 +57,11 @@ help: @echo "" @echo "Run a logical group:" @echo " make test-direct-resources resources with their own SpiceDB type" + @echo " make test-sub-resources resources that delegate to a parent" @echo "" @echo "Run one resource:" @echo " make test-api-key all AGX1-325 cases" + @echo " make test-event all AGX1-331 cases" @echo "" @echo "Run one case:" @echo " make test-api-key-{create,get,list,delete}" @@ -76,6 +80,9 @@ test: test-direct-resources: $(PYTEST) $(DIRECT_RESOURCE_TESTS) -v +test-sub-resources: + $(PYTEST) $(SUB_RESOURCE_TESTS) -v + # --------------------------------------------------------------------------- # Direct resources # --------------------------------------------------------------------------- @@ -96,6 +103,14 @@ test-api-key-list: test-api-key-delete: $(PYTEST) tests/test_api_key_delete.py -v +# --------------------------------------------------------------------------- +# Sub-resources (delegate authz to a parent) +# --------------------------------------------------------------------------- + +# AGX1-331 — events delegate to parent agent +test-event: + $(PYTEST) $(EVENT_TESTS) -v + clean: rm -rf $(VENV) .pytest_cache __pycache__ find . -name __pycache__ -type d -exec rm -rf {} + diff --git a/scripts/spark-authz-e2e-tests/README.md b/scripts/spark-authz-e2e-tests/README.md index df9414bf..ae114ba1 100644 --- a/scripts/spark-authz-e2e-tests/README.md +++ b/scripts/spark-authz-e2e-tests/README.md @@ -81,7 +81,7 @@ cp config.json.example config.json # one-time # (see "Auth model" below for what those need to be). make test # all tests # See `make help` for the full list of targets, including logical groups -# (test-direct-resources) and per-resource targets. +# (test-direct-resources, test-sub-resources) and per-resource targets. ``` ### Minting credentials @@ -111,9 +111,11 @@ make test # Logical groups make test-direct-resources # resources with their own SpiceDB type (api_key, …) +make test-sub-resources # resources that delegate to a parent (event, …) # One resource (all cases) make test-api-key # AGX1-325 — all api_key cases +make test-event # AGX1-331 — all event cases # One case make test-api-key-create