diff --git a/changelog.d/tsk-iup5rd-project-notes-scope-binding.md b/changelog.d/tsk-iup5rd-project-notes-scope-binding.md new file mode 100644 index 000000000..6e7648923 --- /dev/null +++ b/changelog.d/tsk-iup5rd-project-notes-scope-binding.md @@ -0,0 +1,2 @@ +### Fixed +- project_notes scope now requires project_id binding when granting via auth request approve, rejecting the unbound approvals that previously minted inert grants (approval looked successful while the agent silently had no notes access) diff --git a/docs/agent-coordination.md b/docs/agent-coordination.md index bfc92bdc1..b41ef5111 100644 --- a/docs/agent-coordination.md +++ b/docs/agent-coordination.md @@ -515,9 +515,24 @@ lifecycle routes (approve/reject/suspend/reactivate) still 403 non-admins before any lookup, which discloses nothing. Requested scopes are validated against the same closed `VALID_SCOPES` vocabulary -as the consent flow. `project_tasks` and the canvas scopes still require an -explicit `project_id`; `decisions_read` / `decisions_write` (and the other global -scopes) may be granted globally (`project_id=None`) or per-project. Creation +as the consent flow. The project-bound scopes -- `project_tasks`, +`project_tasks_create`, `project_tasks_update`, `project_lists`, `project_notes`, +the canvas scopes (`canvas_read`, `canvas_write`) and the files scopes +(`files_read`, `files_write`) -- all require an explicit, operator-validated +`project_id` on approval (see `_PROJECT_SCOPES` in +`tinyagentos/routes/agent_auth_requests.py`). Omitting the project picker for +one of these scopes is rejected with 400; the only way to mint a project-bound +grant unbound (`project_id=None`) is the explicit `defer_binding` opt-in, and +such a grant is inert until bound: `check_agent_scope_for_project` only +authorizes a grant whose `project_id` equals the requested project, and the +project-bound routes take their `project_id` from the URL, so an unbound grant +matches nothing and authorizes nothing until assign-agent later binds it. +`project_notes` joined this set in the beta.47 promote (#2320): it was +previously grantable without a `project_id`, which minted an inert note grant +the operator believed was usable; it now follows the same rule as +`project_tasks`. `decisions_read` / +`decisions_write` (and the other global scopes) may be granted globally +(`project_id=None`) or per-project. Creation surfaces a bell notification (`source: agent_scope_requests`) to the owner/admin, retired when the request is decided. diff --git a/tests/test_agent_scope_requests.py b/tests/test_agent_scope_requests.py index 36a3c30c9..a84fcee59 100644 --- a/tests/test_agent_scope_requests.py +++ b/tests/test_agent_scope_requests.py @@ -571,6 +571,7 @@ def test_project_scope_set_is_a_single_definition(): "project_tasks_create", "project_tasks_update", "project_lists", + "project_notes", "canvas_read", "canvas_write", "files_read", diff --git a/tests/test_project_notes_bug.py b/tests/test_project_notes_bug.py new file mode 100644 index 000000000..f3a939d78 --- /dev/null +++ b/tests/test_project_notes_bug.py @@ -0,0 +1,100 @@ +"""Test that project_notes scope requires project_id binding.""" + +import pytest + + +class TestProjectNotesScopeBinding: + """Verify project_notes scope requires project_id binding (CR-Critical finding #2320).""" + + @pytest.mark.asyncio + async def test_approve_project_notes_without_project_id_is_400( + self, client, monkeypatch, tmp_path + ): + """project_notes granted without a project_id must be rejected (400), + since project_notes now requires project binding like project_tasks. + + This was the CR-Critical finding (#2320): project_notes was not in + _PROJECT_SCOPES, so it could be granted unbound (no project_id). That did + NOT make the grant usable cross-project: check_agent_scope_for_project + (agent_token_auth.py:205) only authorizes a grant whose project_id EQUALS + the requested project, and the four project_notes routes are all + project-bound (the project_id comes from the URL), so an unbound grant + matched nothing and authorized nothing. The real defect was that the + operator saw a successful approval that minted an INERT grant -- the + agent silently had no access at all, not access to every project. After the + fix, project_notes is in _PROJECT_SCOPES and approving it without a + project_id is rejected with 400 instead of minting an inert grant. + """ + from tinyagentos.routes.agent_auth_requests import _PROJECT_SCOPES, VALID_SCOPES + + assert "project_notes" in VALID_SCOPES + + from tinyagentos.auth_requests_store import AuthRequestsStore + from tinyagentos.agent_grants_store import AgentGrantsStore + from tinyagentos.agent_registry_store import AgentRegistryStore, load_or_create_signing_keypair + + registry = AgentRegistryStore(tmp_path / "reg-test.db") + await registry.init() + auth_store = AuthRequestsStore(tmp_path / "auth-test.db") + await auth_store.init() + grants = AgentGrantsStore(tmp_path / "grants-test.db") + await grants.init() + priv, pub = load_or_create_signing_keypair(tmp_path / "keys-test") + + # Register agent with a unique handle + reg = await registry.register( + framework="openclaw", display_name="test-bot-diff", user_id="u", + origin="external-selfjoin", handle="test-bot-diff", + ) + await registry.set_status(reg["canonical_id"], "active") + + # Create auth request with project_notes but NO project_id + record = await auth_store.create( + identity_claim="@test-bot-unique", framework="openclaw", + requested_scopes=["project_notes"], + requested_skills=None, + reason="", + duration_secs=None, + project_id=None, + ) + + # Monkeypatch stores onto client app state + 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) + ) + + # Try to approve without project_id + # After fix: this MUST 400 because project_notes requires project_id + resp = await client.post( + f"/api/agents/auth-requests/{record['id']}/approve", + json={"granted_scopes": ["project_notes"]}, + ) + + assert resp.status_code == 400, ( + f"project_notes approved without project_id must be rejected with 400, " + f"got {resp.status_code}. Response: {resp.text}" + ) + + # The rejection must also leave no side effects: the whole point of the + # fix is that no inert grant reaches the store. + assert await grants.list_grants(reg["canonical_id"]) == [], ( + "a rejected approval must not mint any grant" + ) + + await registry.close() + await auth_store.close() + await grants.close() + + @pytest.mark.asyncio + async def test_project_notes_is_in_project_scopes( + self, client, monkeypatch, tmp_path + ): + """Verify project_notes is now in _PROJECT_SCOPES after the fix.""" + from tinyagentos.routes.agent_auth_requests import _PROJECT_SCOPES + + assert "project_notes" in _PROJECT_SCOPES, ( + "project_notes should be in _PROJECT_SCOPES after the fix" + ) diff --git a/tinyagentos/routes/agent_auth_requests.py b/tinyagentos/routes/agent_auth_requests.py index b3d067f18..82f7f966c 100644 --- a/tinyagentos/routes/agent_auth_requests.py +++ b/tinyagentos/routes/agent_auth_requests.py @@ -216,7 +216,7 @@ async def _retire_scope_request_notification(request: Request, request_id: str) # project_tasks_create globally. One definition, referenced everywhere. _CANVAS_SCOPES = {"canvas_read", "canvas_write"} _FILES_SCOPES = {"files_read", "files_write"} -_PROJECT_SCOPES = {"project_tasks", "project_tasks_create", "project_tasks_update", "project_lists"} | _CANVAS_SCOPES | _FILES_SCOPES +_PROJECT_SCOPES = {"project_tasks", "project_tasks_create", "project_tasks_update", "project_lists", "project_notes"} | _CANVAS_SCOPES | _FILES_SCOPES def _get_approve_lock(request: Request, request_id: str) -> asyncio.Lock: