From 1c7246cd57eda643ee93396e9f89f864be5d7bcf Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Tue, 28 Jul 2026 18:52:47 +0200 Subject: [PATCH 1/4] feat(tasks): add ownership guard to close_task Add claim-holder check to close_task() SQL (AND claimed_by IS NULL OR claimed_by = ?), mirroring the release guard pattern. The route handler passes force=True when the caller is the project lead/curator, letting leads close cards claimed by others. Fixes #2191 --- tests/projects/test_task_store.py | 50 ++++++++++++++++++++++++++++++ tinyagentos/projects/task_store.py | 23 ++++++++++---- tinyagentos/routes/projects.py | 5 ++- 3 files changed, 71 insertions(+), 7 deletions(-) diff --git a/tests/projects/test_task_store.py b/tests/projects/test_task_store.py index d08cdd6f4..6cbbd348d 100644 --- a/tests/projects/test_task_store.py +++ b/tests/projects/test_task_store.py @@ -371,3 +371,53 @@ async def test_get_task_context_project_falls_back_without_project_store(store): t = await store.create_task(project_id="p", title="T", created_by="u") ctx = await store.get_task_context(t["id"]) assert ctx["project"]["id"] == "p" + + +# ── close_task ownership guard ────────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_close_by_claimer_passes(store): + """Claim holder can close their own claimed card.""" + t = await store.create_task(project_id="p", title="A", created_by="u") + await store.claim_task(t["id"], claimer_id="agent-1") + ok = await store.close_task(t["id"], closed_by="agent-1", reason="done") + assert ok is True + again = await store.get_task(t["id"]) + assert again["status"] == "closed" + assert again["closed_by"] == "agent-1" + + +@pytest.mark.asyncio +async def test_close_by_stranger_rejected(store): + """A non-claimer cannot close a claimed card (ownership guard).""" + t = await store.create_task(project_id="p", title="A", created_by="u") + await store.claim_task(t["id"], claimer_id="agent-1") + ok = await store.close_task(t["id"], closed_by="agent-2", reason="intruder") + assert ok is False + again = await store.get_task(t["id"]) + assert again["status"] == "claimed" + assert again["claimed_by"] == "agent-1" + + +@pytest.mark.asyncio +async def test_close_by_lead_passes(store): + """Lead/curator can force-close a card claimed by someone else.""" + t = await store.create_task(project_id="p", title="A", created_by="u") + await store.claim_task(t["id"], claimer_id="agent-1") + ok = await store.close_task(t["id"], closed_by="lead", reason="escalation", force=True) + assert ok is True + again = await store.get_task(t["id"]) + assert again["status"] == "closed" + assert again["closed_by"] == "lead" + + +@pytest.mark.asyncio +async def test_close_unclaimed_unchanged(store): + """Unclaimed cards can still be closed by any authorised caller.""" + t = await store.create_task(project_id="p", title="A", created_by="u") + ok = await store.close_task(t["id"], closed_by="reviewer", reason="stale") + assert ok is True + again = await store.get_task(t["id"]) + assert again["status"] == "closed" + assert again["closed_by"] == "reviewer" diff --git a/tinyagentos/projects/task_store.py b/tinyagentos/projects/task_store.py index cc9c6c277..6b8b717f9 100644 --- a/tinyagentos/projects/task_store.py +++ b/tinyagentos/projects/task_store.py @@ -370,14 +370,25 @@ async def close_task( task_id: str, closed_by: str, reason: str | None = None, + *, + force: bool = False, ) -> bool: now = time.time() - cursor = await self._db.execute( - """UPDATE project_tasks - SET status = 'closed', closed_by = ?, closed_at = ?, close_reason = ?, updated_at = ? - WHERE id = ? AND status NOT IN ('closed', 'cancelled')""", - (closed_by, now, reason, now, task_id), - ) + if force: + cursor = await self._db.execute( + """UPDATE project_tasks + SET status = 'closed', closed_by = ?, closed_at = ?, close_reason = ?, updated_at = ? + WHERE id = ? AND status NOT IN ('closed', 'cancelled')""", + (closed_by, now, reason, now, task_id), + ) + else: + cursor = await self._db.execute( + """UPDATE project_tasks + SET status = 'closed', closed_by = ?, closed_at = ?, close_reason = ?, updated_at = ? + WHERE id = ? AND status NOT IN ('closed', 'cancelled') + AND (claimed_by IS NULL OR claimed_by = ?)""", + (closed_by, now, reason, now, task_id, closed_by), + ) await self._db.commit() changed = cursor.rowcount == 1 if changed: diff --git a/tinyagentos/routes/projects.py b/tinyagentos/routes/projects.py index 1fb79e134..97c2d2bad 100644 --- a/tinyagentos/routes/projects.py +++ b/tinyagentos/routes/projects.py @@ -994,8 +994,11 @@ async def close_task( existing = await store.get_task(task_id) if existing is None or existing["project_id"] != project_id: return JSONResponse({"error": "not found"}, status_code=404) - ok = await store.close_task(task_id, closed_by=closed_by, reason=payload.reason) + force = project.get("lead_member_id") == actor_id + ok = await store.close_task(task_id, closed_by=closed_by, reason=payload.reason, force=force) if not ok: + if existing.get("claimed_by") and existing["claimed_by"] != closed_by: + return JSONResponse({"error": "not claimed by you"}, status_code=409) return JSONResponse({"error": "cannot close"}, status_code=409) _beads_mark_dirty(request, project_id) await pstore.log_activity(project_id, closed_by, "task.closed", {"task_id": task_id}) From ed53caf3a172d3edc86db30198fa5b19590e1126 Mon Sep 17 00:00:00 2001 From: hognek <227774406+hognek@users.noreply.github.com> Date: Tue, 4 Aug 2026 13:59:59 +0200 Subject: [PATCH 2/4] docs: add changelog fragment for close_task ownership guard (#2287) --- changelog.d/2287-close-ownership-guard.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog.d/2287-close-ownership-guard.md diff --git a/changelog.d/2287-close-ownership-guard.md b/changelog.d/2287-close-ownership-guard.md new file mode 100644 index 000000000..24883f9dd --- /dev/null +++ b/changelog.d/2287-close-ownership-guard.md @@ -0,0 +1,3 @@ +### Fixed + +- Closing a claimed task is refused unless you are the claim holder or the project lead; a non-claimer now gets 409 instead of silently closing someone else's card (#2287). From 25a206fbefea6b373eebadb5ff60e1274fa1a116 Mon Sep 17 00:00:00 2001 From: hognek <227774406+hognek@users.noreply.github.com> Date: Thu, 13 Aug 2026 23:24:58 +0200 Subject: [PATCH 3/4] fix(tasks): widen close_task ownership bypass to owner + session admin (#2191) The force bypass previously covered only the lead_member_id (an AGENT registry id on the live board), so admin-session callers (USER ids) could never close an agent-claimed card. Widen force to lead OR project owner (user_id) OR session admin (request.state.is_admin). --- tests/test_routes_projects_agent_tasks.py | 62 ++++++++++++++++++++++- tinyagentos/routes/projects.py | 11 +++- 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/tests/test_routes_projects_agent_tasks.py b/tests/test_routes_projects_agent_tasks.py index bc130ddc1..729e2ab9c 100644 --- a/tests/test_routes_projects_agent_tasks.py +++ b/tests/test_routes_projects_agent_tasks.py @@ -64,7 +64,7 @@ async def _new_task(ctx, pid, title="T"): return resp.json()["id"] -async def _mint_agent(ctx, project_id, scopes=("project_tasks",)): +async def _mint_agent(ctx, project_id, scopes=("project_tasks",), handle="@grok"): registry = ctx.app.state.agent_registry grants = ctx.app.state.agent_grants priv, _pub = ctx.app.state.agent_registry_keypair @@ -72,7 +72,7 @@ async def _mint_agent(ctx, project_id, scopes=("project_tasks",)): framework="grok", display_name="Grok", origin="external-selfjoin", - handle="@grok", + handle=handle, ) cid = rec["canonical_id"] await registry.set_status(cid, "active") @@ -530,6 +530,64 @@ async def test_admin_claims_and_closes_as_any_actor(self, ctx): assert close.status_code == 200 assert close.json()["status"] == "closed" + async def test_admin_session_closes_agent_claimed_card_lead_is_agent(self, ctx): + """Issue #2191 repro: the board lead is an AGENT registry id, so a + session admin's actor id (a USER id) can never equal lead_member_id. + The ownership guard's force bypass must cover owner + session admin, + not just the lead, or every admin-session close of an agent-claimed + card returns 409.""" + pid = await _new_project(ctx, "alpha") + tid = await _new_task(ctx, pid) + lead_cid, _lead_token = await _mint_agent(ctx, pid, handle="@lead") + pstore = ctx.app.state.project_store + await pstore.add_member(pid, lead_cid, "native") + await pstore.set_lead(pid, lead_cid) + # A different lane agent claims the card. + lane_cid, lane_token = await _mint_agent(ctx, pid, handle="@lane") + async with _bare(ctx.app) as bare: + claim = await bare.post( + f"/api/projects/{pid}/tasks/{tid}/claim", + json={"claimer_id": lane_cid}, + headers=_hdr(lane_token), + ) + assert claim.status_code == 200, claim.text + assert claim.json()["claimed_by"] == lane_cid + # The admin session closes it, recording the closer as its own user id. + close = await ctx.client.post( + f"/api/projects/{pid}/tasks/{tid}/close", + json={"closed_by": ctx.uid}, + ) + assert close.status_code == 200, close.text + assert close.json()["status"] == "closed" + assert close.json()["closed_by"] == ctx.uid + + async def test_lead_agent_closes_other_agents_card(self, ctx): + """Control (merge-gate path): the lead-agent bypass is preserved — a + lead agent still force-closes a card claimed by a different lane + agent.""" + pid = await _new_project(ctx, "alpha") + tid = await _new_task(ctx, pid) + lead_cid, lead_token = await _mint_agent(ctx, pid, handle="@lead") + pstore = ctx.app.state.project_store + await pstore.add_member(pid, lead_cid, "native") + await pstore.set_lead(pid, lead_cid) + lane_cid, lane_token = await _mint_agent(ctx, pid, handle="@lane") + async with _bare(ctx.app) as bare: + claim = await bare.post( + f"/api/projects/{pid}/tasks/{tid}/claim", + json={"claimer_id": lane_cid}, + headers=_hdr(lane_token), + ) + assert claim.status_code == 200, claim.text + close = await bare.post( + f"/api/projects/{pid}/tasks/{tid}/close", + json={"closed_by": lead_cid}, + headers=_hdr(lead_token), + ) + assert close.status_code == 200, close.text + assert close.json()["status"] == "closed" + assert close.json()["closed_by"] == lead_cid + async def test_unauthenticated_still_401(self, ctx): pid = await _new_project(ctx, "alpha") async with _bare(ctx.app) as bare: diff --git a/tinyagentos/routes/projects.py b/tinyagentos/routes/projects.py index 97c2d2bad..9e59c13ca 100644 --- a/tinyagentos/routes/projects.py +++ b/tinyagentos/routes/projects.py @@ -994,7 +994,16 @@ async def close_task( existing = await store.get_task(task_id) if existing is None or existing["project_id"] != project_id: return JSONResponse({"error": "not found"}, status_code=404) - force = project.get("lead_member_id") == actor_id + # Ownership-guard bypass: a card claimed by one agent may still be closed + # by the project lead (lead_member_id), the project owner (user_id), or a + # session admin. The lead is typically an AGENT registry id, so an + # admin/owner session caller (a USER id) can never equal it — widen the + # bypass to cover all three (issue #2191). + force = ( + project.get("lead_member_id") == actor_id + or project.get("user_id") == actor_id + or bool(getattr(request.state, "is_admin", False)) + ) ok = await store.close_task(task_id, closed_by=closed_by, reason=payload.reason, force=force) if not ok: if existing.get("claimed_by") and existing["claimed_by"] != closed_by: From 8146cac8d85b283d2f133131dba76c88b80a7a07 Mon Sep 17 00:00:00 2001 From: hognek <227774406+hognek@users.noreply.github.com> Date: Mon, 17 Aug 2026 10:44:38 +0200 Subject: [PATCH 4/4] =?UTF-8?q?docs:=20doc-gate=20waiver=20=E2=80=94=20clo?= =?UTF-8?q?se=5Ftask=20ownership=20guard=20(security=20hardening)=20(#2287?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Docs-Reviewed: the close_task guard adds a claim-holder check to existing SQL (AND claimed_by IS NULL OR claimed_by = ?) with a force bypass for project lead/curator/admin. No new endpoint, no route shape change, no new agent-facing surface — agent-coordination.md does not document close_task's authorization internals, so there is no prose to correct.