From ef170e02124689a005776050977dfeac2c0f8a8f Mon Sep 17 00:00:00 2001 From: "cmeans-claude-dev[bot]" <3223881+cmeans-claude-dev[bot]@users.noreply.github.com> Date: Thu, 9 Apr 2026 18:41:32 -0500 Subject: [PATCH] =?UTF-8?q?fix:=20delete=5Fentry=20IDOR=20=E2=80=94=20unif?= =?UTF-8?q?orm=20response=20for=20single-entry=20deletes=20(#193)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Single-entry delete by ID now returns "status": "acknowledged" with no trashed count, preventing entry existence enumeration across tenants. Includes a note field: "If the entry was not found, no action was taken." Bulk deletes (tags, source) retain their counts since they're already scoped by owner_id. Co-Authored-By: Claude Opus 4.6 (1M context) --- CHANGELOG.md | 1 + src/mcp_awareness/tools.py | 6 +++--- tests/test_server.py | 13 +++++++++---- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f63717da..56c45e23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **MCP request logger** — logs method, truncated session ID, client IP, and response status for every `/mcp` request. Placed outside the session registry for full visibility into both intercepted and pass-through requests. ### Fixed +- `delete_entry` IDOR fix — single-entry delete by ID now returns `"status": "acknowledged"` with no count, preventing entry existence enumeration across tenants. Bulk deletes (tags, source) retain counts since they're already owner-scoped ([#193](https://github.com/cmeans/mcp-awareness/issues/193)) - Session registry now intercepts `GET /mcp` (SSE reconnect) — previously only POST and DELETE were handled, causing stale GET requests to bypass re-initialization and return 409 directly from FastMCP ([#178](https://github.com/cmeans/mcp-awareness/issues/178)) - `_LazyStore` thread safety — added double-checked locking to prevent duplicate `PostgresStore`/connection pool creation under concurrent access from embedding workers, cleanup thread, or parallel requests ([#164](https://github.com/cmeans/mcp-awareness/issues/164)) - SQL template injection hardening — replaced `str.format()` with `psycopg.sql.SQL` composition across all 13 dynamic query sites in `postgres_store.py`, enforced via `psql.Composable` types that mypy validates at the call boundary ([#165](https://github.com/cmeans/mcp-awareness/issues/165)) diff --git a/src/mcp_awareness/tools.py b/src/mcp_awareness/tools.py index 729133d5..178d8954 100644 --- a/src/mcp_awareness/tools.py +++ b/src/mcp_awareness/tools.py @@ -638,13 +638,13 @@ async def delete_entry( Returns JSON with status and count. If you receive an unstructured error, the failure is in the transport or platform layer, not in awareness.""" if entry_id: - trashed = _srv.store.soft_delete_by_id(_srv._owner_id(), entry_id) + _srv.store.soft_delete_by_id(_srv._owner_id(), entry_id) return json.dumps( { - "status": "ok", - "trashed": 1 if trashed else 0, + "status": "acknowledged", "entry_id": entry_id, "recoverable_days": 30, + "note": "If the entry was not found, no action was taken.", } ) if tags: diff --git a/tests/test_server.py b/tests/test_server.py index 09a41398..7d2482b0 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -1076,19 +1076,24 @@ async def test_delete_by_id(self) -> None: entry_id = json.loads(result)["id"] delete_result = await server_mod.delete_entry(entry_id=entry_id) data = json.loads(delete_result) - assert data["status"] == "ok" - assert data["trashed"] == 1 + assert data["status"] == "acknowledged" assert data["recoverable_days"] == 30 + assert "note" in data + assert "trashed" not in data # No count — prevents IDOR # Not visible in normal queries assert len(_store().get_patterns(TEST_OWNER)) == 0 # But in trash assert len(_store().get_deleted(TEST_OWNER)) == 1 @pytest.mark.anyio - async def test_delete_by_id_not_found(self) -> None: + async def test_delete_by_id_not_found_same_response(self) -> None: + """Nonexistent entry returns identical shape — no information leakage.""" result = await server_mod.delete_entry(entry_id="nonexistent") data = json.loads(result) - assert data["trashed"] == 0 + assert data["status"] == "acknowledged" + assert data["recoverable_days"] == 30 + assert "note" in data + assert "trashed" not in data @pytest.mark.anyio async def test_dry_run_without_confirm(self) -> None: