From f5f025343bb6c6c2af2d8b3f13666ef070458d54 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Tue, 14 Jul 2026 12:20:28 +0100 Subject: [PATCH 1/2] feat(agents): consent approval sets registry handle from sanitized identity claim Derive the registry handle via _slugify from the already-sanitized identity claim in the approve path. Before minting the canonical identity, check get_by_handle(status='active'); if an active identity owns that handle, return 409 and leave the request PENDING so the approver can try a different variant. A SUSPENDED holder does not block reuse. After activating the new agent, update its handle via the store update setter. Tests: - TestHandleSetOnApprove: handle is set, agent passes the a2a no-handle gate - TestHandleCollisionActiveRejects: 409 + request stays pending when an active identity already has the handle - TestHandleCollisionSuspendedAllowsReuse: approval succeeds once the previous holder is suspended Docs-Reviewed: consent approve sets registry handle per lead-agent-identity-and-canvas-access.md --- tests/test_routes_agent_auth_requests.py | 189 ++++++++++++++++++++++ tinyagentos/routes/agent_auth_requests.py | 15 +- 2 files changed, 203 insertions(+), 1 deletion(-) diff --git a/tests/test_routes_agent_auth_requests.py b/tests/test_routes_agent_auth_requests.py index c4def4688..78fc1719a 100644 --- a/tests/test_routes_agent_auth_requests.py +++ b/tests/test_routes_agent_auth_requests.py @@ -531,3 +531,192 @@ async def test_get_unknown_id_returns_404(self, client, monkeypatch): monkeypatch.setattr(client._transport.app.state, "auth_requests", store) resp = await client.get("/api/agents/auth-requests/nonexistent123") assert resp.status_code == 404 + + +class TestHandleSetOnApprove: + """Slice 7: approve sets the registry handle from the sanitized identity + claim; the new agent then passes the a2a bus 'no handle' gate.""" + + @pytest.mark.asyncio + async def test_approve_sets_handle_on_registry(self, client, monkeypatch, tmp_path): + from tinyagentos.agent_registry_store import ( + AgentRegistryStore, + load_or_create_signing_keypair, + ) + from tinyagentos.auth_requests_store import AuthRequestsStore + from tinyagentos.agent_grants_store import AgentGrantsStore + + registry = AgentRegistryStore(tmp_path / "reg-handle.db") + await registry.init() + auth_store = AuthRequestsStore(tmp_path / "auth-handle.db") + await auth_store.init() + grants = AgentGrantsStore(tmp_path / "grants-handle.db") + await grants.init() + priv, pub = load_or_create_signing_keypair(tmp_path / "keys-handle") + + record = await auth_store.create( + identity_claim="@taOSmd-dev", + framework="openclaw", + requested_scopes=["memory_read"], + requested_skills=None, + reason="", + duration_secs=None, + project_id=None, + ) + + monkeypatch.setattr(client._transport.app.state, "agent_registry", registry) + monkeypatch.setattr(client._transport.app.state, "auth_requests", auth_store) + monkeypatch.setattr(client._transport.app.state, "agent_grants", grants) + monkeypatch.setattr( + client._transport.app.state, "agent_registry_keypair", (priv, pub) + ) + + resp = await client.post( + f"/api/agents/auth-requests/{record['id']}/approve", + json={"granted_scopes": ["memory_read"]}, + ) + assert resp.status_code == 200, resp.text + + agents = await registry.list_all() + assert len(agents) == 1 + assert agents[0]["handle"] == "taosmd-dev" + + handle = (agents[0].get("handle") or "").strip() + assert handle, "handle must be set so the a2a 'no handle' gate passes" + + await registry.close() + await auth_store.close() + await grants.close() + + +class TestHandleCollisionActiveRejects: + """Slice 7: a handle collision with an ACTIVE identity returns 409 and + leaves the auth request PENDING so the approver can pick another variant.""" + + @pytest.mark.asyncio + async def test_409_when_handle_collides_with_active( + self, client, monkeypatch, tmp_path + ): + from tinyagentos.agent_registry_store import ( + AgentRegistryStore, + load_or_create_signing_keypair, + ) + from tinyagentos.auth_requests_store import AuthRequestsStore + from tinyagentos.agent_grants_store import AgentGrantsStore + + registry = AgentRegistryStore(tmp_path / "reg-collide.db") + await registry.init() + auth_store = AuthRequestsStore(tmp_path / "auth-collide.db") + await auth_store.init() + grants = AgentGrantsStore(tmp_path / "grants-collide.db") + await grants.init() + priv, pub = load_or_create_signing_keypair(tmp_path / "keys-collide") + + existing = await registry.register( + framework="openclaw", + display_name="existing-agent", + user_id="user-existing", + origin="taos-deployed", + handle="taosmd-dev", + ) + + record = await auth_store.create( + identity_claim="@taOSmd-dev", + framework="openclaw", + requested_scopes=["memory_read"], + requested_skills=None, + reason="", + duration_secs=None, + project_id=None, + ) + + monkeypatch.setattr(client._transport.app.state, "agent_registry", registry) + monkeypatch.setattr(client._transport.app.state, "auth_requests", auth_store) + monkeypatch.setattr(client._transport.app.state, "agent_grants", grants) + monkeypatch.setattr( + client._transport.app.state, "agent_registry_keypair", (priv, pub) + ) + + resp = await client.post( + f"/api/agents/auth-requests/{record['id']}/approve", + json={"granted_scopes": ["memory_read"]}, + ) + assert resp.status_code == 409, resp.text + + pending = await auth_store.get(record["id"]) + assert pending is not None + assert pending["status"] == "pending" + + active_agents = await registry.list_all(status="active") + assert len(active_agents) == 1 + + await registry.close() + await auth_store.close() + await grants.close() + + +class TestHandleCollisionSuspendedAllowsReuse: + """Slice 7: if the previous holder of the handle is SUSPENDED, the handle + may be reused and approval succeeds.""" + + @pytest.mark.asyncio + async def test_approve_succeeds_after_handle_holder_suspended( + self, client, monkeypatch, tmp_path + ): + from tinyagentos.agent_registry_store import ( + AgentRegistryStore, + load_or_create_signing_keypair, + ) + from tinyagentos.auth_requests_store import AuthRequestsStore + from tinyagentos.agent_grants_store import AgentGrantsStore + + registry = AgentRegistryStore(tmp_path / "reg-suspended.db") + await registry.init() + auth_store = AuthRequestsStore(tmp_path / "auth-suspended.db") + await auth_store.init() + grants = AgentGrantsStore(tmp_path / "grants-suspended.db") + await grants.init() + priv, pub = load_or_create_signing_keypair(tmp_path / "keys-suspended") + + old_agent = await registry.register( + framework="openclaw", + display_name="old-agent", + user_id="user-old", + origin="taos-deployed", + handle="taosmd-dev", + ) + await registry.set_status( + old_agent["canonical_id"], "suspended", actor="user-old" + ) + + record = await auth_store.create( + identity_claim="@taOSmd-dev", + framework="openclaw", + requested_scopes=["memory_read"], + requested_skills=None, + reason="", + duration_secs=None, + project_id=None, + ) + + monkeypatch.setattr(client._transport.app.state, "agent_registry", registry) + monkeypatch.setattr(client._transport.app.state, "auth_requests", auth_store) + monkeypatch.setattr(client._transport.app.state, "agent_grants", grants) + monkeypatch.setattr( + client._transport.app.state, "agent_registry_keypair", (priv, pub) + ) + + resp = await client.post( + f"/api/agents/auth-requests/{record['id']}/approve", + json={"granted_scopes": ["memory_read"]}, + ) + assert resp.status_code == 200, resp.text + + new_agents = await registry.list_all() + new_active = [a for a in new_agents if a["status"] == "active"] + assert len(new_active) == 1 + assert new_active[0]["handle"] == "taosmd-dev" + + await registry.close() + await auth_store.close() + await grants.close() diff --git a/tinyagentos/routes/agent_auth_requests.py b/tinyagentos/routes/agent_auth_requests.py index 4d06b37a3..6b5f627be 100644 --- a/tinyagentos/routes/agent_auth_requests.py +++ b/tinyagentos/routes/agent_auth_requests.py @@ -28,7 +28,7 @@ from fastapi.responses import JSONResponse from pydantic import BaseModel -from tinyagentos.agent_registry_store import mint_registry_token +from tinyagentos.agent_registry_store import _slugify, mint_registry_token from tinyagentos.auth_context import CurrentUser, current_user logger = logging.getLogger(__name__) @@ -336,6 +336,18 @@ async def _do_approve(request: Request, request_id: str, body: ApproveBody, user # non-empty value. _claim = record["identity_claim"].strip().removeprefix("@").strip() display_name = _claim or record["framework"] + + handle = _slugify(_claim) + existing_active = await registry.get_by_handle(handle, status="active") + if existing_active is not None: + raise HTTPException( + status_code=409, + detail=( + f"handle '{handle}' is already in use by active agent " + f"{existing_active['canonical_id']}; pick a different identity_claim" + ), + ) + reg_record = await registry.register( framework=record["framework"], display_name=display_name, @@ -350,6 +362,7 @@ async def _do_approve(request: Request, request_id: str, body: ApproveBody, user # them to 'active' so they are NOT in the bus inactive/revocation feed and # @taOSmd's identity-AND-grant gate accepts them. await registry.set_status(canonical_id, "active", actor=user.user_id) + await registry.update(canonical_id, handle=handle) # Resolve effective project binding: admin override wins; fall back to the # project_id the agent requested (may be None for a global token). From de21830f4a3bc494bff1be51ee687948d86bd7cb Mon Sep 17 00:00:00 2001 From: jaylfc Date: Tue, 14 Jul 2026 20:54:47 +0100 Subject: [PATCH 2/2] fix(agents): close consent handle-set TOCTOU and active-without-handle orphan window Register the handle at birth in the approve path so an agent is never left ACTIVE with an empty handle, and add a partial unique index on agent_registry(handle) WHERE status = 'active' AND handle != '' so the DB atomically rejects a concurrent approval of the same identity_claim instead of granting two active agents the identical handle (a2a "from" spoofing). The concurrent loser now fails with 409 and its half-registered row is rolled back and removed. Docs-Reviewed: close consent handle-set TOCTOU + orphan window per lead-agent-identity-and-canvas-access.md --- tests/test_routes_agent_auth_requests.py | 156 ++++++++++++++++++++++ tinyagentos/agent_registry_store.py | 30 +++++ tinyagentos/routes/agent_auth_requests.py | 60 ++++++--- 3 files changed, 231 insertions(+), 15 deletions(-) diff --git a/tests/test_routes_agent_auth_requests.py b/tests/test_routes_agent_auth_requests.py index 78fc1719a..369d44fc2 100644 --- a/tests/test_routes_agent_auth_requests.py +++ b/tests/test_routes_agent_auth_requests.py @@ -1,3 +1,4 @@ +import asyncio import pytest @@ -720,3 +721,158 @@ async def test_approve_succeeds_after_handle_holder_suspended( await registry.close() await auth_store.close() await grants.close() + + +class TestPartialUniqueHandleIndex: + """The DB (not just the application pre-check) must reject a duplicate + active handle, so two concurrent approvals of the same identity_claim + cannot both become active with identical handles (a2a 'from' spoofing).""" + + @pytest.mark.asyncio + async def test_index_blocks_two_active_with_same_handle(self, tmp_path): + from tinyagentos.agent_registry_store import AgentRegistryStore + + registry = AgentRegistryStore(tmp_path / "reg-idx.db") + await registry.init() + + # Both born pending with the same handle (allowed: index only covers + # active + non-empty handle). + a = await registry.register( + framework="openclaw", display_name="a", origin="external-selfjoin", + handle="dup-handle", + ) + b = await registry.register( + framework="openclaw", display_name="b", origin="external-selfjoin", + handle="dup-handle", + ) + await registry.set_status(a["canonical_id"], "active") + + # The second activation must be rejected by the partial unique index. + with pytest.raises(Exception) as exc: + await registry.set_status(b["canonical_id"], "active") + assert "ux_agent_active_handle" in str(exc.value) or "UNIQUE" in str(exc.value) + + # And the loser must never be left active with the handle. + b_row = await registry.get(b["canonical_id"]) + assert b_row["status"] == "pending" + await registry.close() + + +class TestConcurrentApproveSameIdentity: + """Two approvals racing on the SAME identity_claim: exactly one active + agent with the handle set, the other returns 409 and leaves no extra + active (or pending) agent behind.""" + + @pytest.mark.asyncio + async def test_concurrent_approve_one_active_other_409( + self, client, monkeypatch, tmp_path + ): + from tinyagentos.agent_registry_store import ( + AgentRegistryStore, + load_or_create_signing_keypair, + ) + from tinyagentos.auth_requests_store import AuthRequestsStore + from tinyagentos.agent_grants_store import AgentGrantsStore + + registry = AgentRegistryStore(tmp_path / "reg-race.db") + await registry.init() + auth_store = AuthRequestsStore(tmp_path / "auth-race.db") + await auth_store.init() + grants = AgentGrantsStore(tmp_path / "grants-race.db") + await grants.init() + priv, pub = load_or_create_signing_keypair(tmp_path / "keys-race") + + # Two independent auth requests for the SAME identity_claim. The + # per-request lock does NOT protect this case (different request ids), + # so this exercises the unique-index race directly. + r1 = await auth_store.create( + identity_claim="@taOSmd-dev", framework="openclaw", + requested_scopes=["memory_read"], requested_skills=None, reason="", + duration_secs=None, project_id=None, + ) + r2 = await auth_store.create( + identity_claim="@taOSmd-dev", framework="openclaw", + requested_scopes=["memory_read"], requested_skills=None, reason="", + duration_secs=None, project_id=None, + ) + + monkeypatch.setattr(client._transport.app.state, "agent_registry", registry) + monkeypatch.setattr(client._transport.app.state, "auth_requests", auth_store) + monkeypatch.setattr(client._transport.app.state, "agent_grants", grants) + monkeypatch.setattr( + client._transport.app.state, "agent_registry_keypair", (priv, pub) + ) + + async def _approve(rid): + return await client.post( + f"/api/agents/auth-requests/{rid}/approve", + json={"granted_scopes": ["memory_read"]}, + ) + + # Drive both approvals concurrently so neither pre-check sees an active + # handle yet; the unique index decides the winner. + resps = await asyncio.gather(_approve(r1["id"]), _approve(r2["id"])) + + statuses = sorted(r.status_code for r in resps) + assert statuses == [200, 409], [r.status_code for r in resps] + + active = await registry.list_all(status="active") + assert len(active) == 1, active + # The winner's handle is set and non-empty. + assert active[0]["handle"] == "taosmd-dev" + assert active[0]["handle"] + + # The loser is not left behind as an active or pending agent. + assert len(await registry.list_all()) == 1 + + await registry.close() + await auth_store.close() + await grants.close() + + @pytest.mark.asyncio + async def test_approved_active_handle_is_nonempty( + self, client, monkeypatch, tmp_path + ): + from tinyagentos.agent_registry_store import ( + AgentRegistryStore, + load_or_create_signing_keypair, + ) + from tinyagentos.auth_requests_store import AuthRequestsStore + from tinyagentos.agent_grants_store import AgentGrantsStore + + registry = AgentRegistryStore(tmp_path / "reg-nonempty.db") + await registry.init() + auth_store = AuthRequestsStore(tmp_path / "auth-nonempty.db") + await auth_store.init() + grants = AgentGrantsStore(tmp_path / "grants-nonempty.db") + await grants.init() + priv, pub = load_or_create_signing_keypair(tmp_path / "keys-nonempty") + + record = await auth_store.create( + identity_claim="grok-bot", framework="grok", + requested_scopes=["memory_read"], requested_skills=None, reason="", + duration_secs=None, project_id=None, + ) + + monkeypatch.setattr(client._transport.app.state, "agent_registry", registry) + monkeypatch.setattr(client._transport.app.state, "auth_requests", auth_store) + monkeypatch.setattr(client._transport.app.state, "agent_grants", grants) + monkeypatch.setattr( + client._transport.app.state, "agent_registry_keypair", (priv, pub) + ) + + resp = await client.post( + f"/api/agents/auth-requests/{record['id']}/approve", + json={"granted_scopes": ["memory_read"]}, + ) + assert resp.status_code == 200, resp.text + + active = await registry.list_all(status="active") + assert len(active) == 1 + # Never left active with an empty handle (the orphan-window bug). + assert active[0]["handle"] + assert active[0]["handle"] == "grok-bot" + + await registry.close() + await auth_store.close() + await grants.close() diff --git a/tinyagentos/agent_registry_store.py b/tinyagentos/agent_registry_store.py index 48740bb88..7cbe43157 100644 --- a/tinyagentos/agent_registry_store.py +++ b/tinyagentos/agent_registry_store.py @@ -47,6 +47,16 @@ revoked_at TEXT, status TEXT NOT NULL DEFAULT 'active' ); + +-- Partial unique index: at most ONE active agent may own any given non-empty +-- handle. SQLite enforces this atomically, so two concurrent consent approvals +-- of the same identity_claim cannot both flip to 'active' with the same handle. +-- Pending / suspended / revoked rows and empty handles are excluded from the +-- index, so (a) a handle can be reused once its owner leaves 'active', and (b) +-- pre-existing empty-handle active agents never block the index's creation. +CREATE UNIQUE INDEX IF NOT EXISTS ux_agent_active_handle + ON agent_registry(handle) + WHERE status = 'active' AND handle != ''; """ # --------------------------------------------------------------------------- @@ -425,6 +435,26 @@ async def register( # Read # ------------------------------------------------------------------ + async def rollback(self) -> None: + """Roll back the current transaction, clearing any failed write so the + connection can accept new statements. No-op if the store is closed or + no transaction is open. + """ + if self._db is not None: + await self._db.rollback() + + async def delete(self, canonical_id: str) -> None: + """Permanently remove *canonical_id* (used to clean up a half-registered + row when a concurrent approve wins the active-handle race). Returns + silently if *canonical_id* does not exist. + """ + if self._db is None: + raise RuntimeError("AgentRegistryStore not initialised") + await self._db.execute( + "DELETE FROM agent_registry WHERE canonical_id = ?", (canonical_id,) + ) + await self._db.commit() + async def get(self, canonical_id: str) -> Optional[dict]: """Return the record for *canonical_id*, or ``None``.""" if self._db is None: diff --git a/tinyagentos/routes/agent_auth_requests.py b/tinyagentos/routes/agent_auth_requests.py index 6b5f627be..958a4ee87 100644 --- a/tinyagentos/routes/agent_auth_requests.py +++ b/tinyagentos/routes/agent_auth_requests.py @@ -28,6 +28,7 @@ from fastapi.responses import JSONResponse from pydantic import BaseModel +from aiosqlite import IntegrityError from tinyagentos.agent_registry_store import _slugify, mint_registry_token from tinyagentos.auth_context import CurrentUser, current_user @@ -348,21 +349,50 @@ async def _do_approve(request: Request, request_id: str, body: ApproveBody, user ), ) - reg_record = await registry.register( - framework=record["framework"], - display_name=display_name, - user_id=user.user_id, - origin="external-selfjoin", - handle="", - ) - canonical_id = reg_record["canonical_id"] - - # Consent approval IS the activation. external-selfjoin agents are born - # 'pending' (governance lifecycle); approving the auth-request transitions - # them to 'active' so they are NOT in the bus inactive/revocation feed and - # @taOSmd's identity-AND-grant gate accepts them. - await registry.set_status(canonical_id, "active", actor=user.user_id) - await registry.update(canonical_id, handle=handle) + # Register with the handle SET at birth. external-selfjoin agents are born + # 'pending' (governance lifecycle), so the partial unique index + # ux_agent_active_handle (active + non-empty handle) does not fire at INSERT + # time; it fires only when we flip the row to 'active' below. That removes + # the old window (register handle='' -> set active -> update handle) in which + # an agent could sit ACTIVE with an empty handle, and lets SQLite reject a + # duplicate active handle the instant a concurrent approve tries to take it. + canonical_id = None + try: + reg_record = await registry.register( + framework=record["framework"], + display_name=display_name, + user_id=user.user_id, + origin="external-selfjoin", + handle=handle, + ) + canonical_id = reg_record["canonical_id"] + + # Consent approval IS the activation. external-selfjoin agents are born + # 'pending' (governance lifecycle); approving the auth-request transitions + # them to 'active' so they are NOT in the bus inactive/revocation feed and + # @taOSmd's identity-AND-grant gate accepts them. + await registry.set_status(canonical_id, "active", actor=user.user_id) + except IntegrityError: + # A concurrent approve already took this active handle (the partial + # unique index fired). Roll back the failed write and remove the + # half-registered pending row so we never leave an active-without-handle + # agent or a stale pending row. Return the same friendly 409. + try: + await registry.rollback() + except Exception: # noqa: BLE001 - never mask the 409 below + pass + if canonical_id is not None: + try: + await registry.delete(canonical_id) + except Exception: # noqa: BLE001 - never mask the 409 below + pass + raise HTTPException( + status_code=409, + detail=( + f"handle '{handle}' is already in use by active agent; " + f"pick a different identity_claim" + ), + ) # Resolve effective project binding: admin override wins; fall back to the # project_id the agent requested (may be None for a global token).