fix(tasks): add ownership guard to close_task

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).

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"

tests/test_routes_projects_agent_tasks.py

@@ -64,15 +64,15 @@ 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
     rec = await registry.register(
         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:

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:

tinyagentos/routes/projects.py

@@ -994,8 +994,20 @@ 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)
+    # 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:
+            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})