Skip to content

Commit f9822cc

Browse files
RafaelPogithub-actions[bot]
authored andcommitted
refactor(mcp): remove redundant Redis ownership checks (#4994)
## Summary - Remove `_check_task_ownership()` from the MCP tool layer (tools.py) — the Engine already validates task ownership on every API call via `get_user_task() → session.owner_account_id == user_id` - Remove `_validate_task_owner()` cross-check from REST endpoints (routes.py) — the poll token is cryptographically random and task-scoped, making the ownership cross-check redundant defense-in-depth - Remove 4 Redis functions (`store_task_owner`, `get_task_owner`, `get_poll_token_owner`, `user_id` param from `store_poll_token`) and eliminate 2 Redis keys per task: `mcp:task_owner:{task_id}` and `mcp:poll_owner:{task_id}` Net: **-319 lines**, fewer Redis round-trips per request, no change in security posture. ## Test plan - [x] All 371 existing tests pass (format, lint, typecheck hooks all green) - [x] Ownership-specific tests removed (tested behavior no longer exists) - [x] Poll token validation remains intact as primary REST auth gate - [ ] Verify on staging: submit task → poll progress → fetch results works end-to-end - [ ] Verify multi-user isolation still works via Engine auth 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Sourced from commit c5fc538f4b3e8c68d4c3268e4417c9711ff334f9
1 parent 6e649ba commit f9822cc

10 files changed

Lines changed: 24 additions & 321 deletions

File tree

futuresearch-mcp/src/futuresearch_mcp/redis_store.py

Lines changed: 3 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -192,42 +192,17 @@ async def pop_task_token(task_id: str) -> None:
192192
# ── Poll tokens ───────────────────────────────────────────────
193193

194194

195-
async def store_poll_token(task_id: str, poll_token: str, user_id: str = "") -> None:
196-
"""Store an encrypted poll token, optionally bound to a user identity."""
197-
client = get_redis_client()
198-
await client.setex(
195+
async def store_poll_token(task_id: str, poll_token: str) -> None:
196+
"""Store an encrypted poll token."""
197+
await get_redis_client().setex(
199198
name=build_key("poll_token", task_id),
200199
time=TOKEN_TTL,
201200
value=encrypt_value(poll_token),
202201
)
203-
if user_id:
204-
await client.setex(
205-
name=build_key("poll_owner", task_id),
206-
time=TOKEN_TTL,
207-
value=user_id,
208-
)
209202

210203

211204
async def get_poll_token(task_id: str) -> str | None:
212205
encrypted = await get_redis_client().get(build_key("poll_token", task_id))
213206
if encrypted is None:
214207
return None
215208
return decrypt_value(encrypted)
216-
217-
218-
async def get_poll_token_owner(task_id: str) -> str | None:
219-
"""Return the user_id bound to a poll token, or None if not set."""
220-
return await get_redis_client().get(build_key("poll_owner", task_id))
221-
222-
223-
# ── Task ownership (user-scoped data isolation) ──────────────
224-
225-
226-
async def store_task_owner(task_id: str, user_id: str) -> None:
227-
"""Record which user submitted a task (for cross-user access checks)."""
228-
await get_redis_client().setex(build_key("task_owner", task_id), TOKEN_TTL, user_id)
229-
230-
231-
async def get_task_owner(task_id: str) -> str | None:
232-
"""Return the user_id that owns a task, or None if not recorded."""
233-
return await get_redis_client().get(build_key("task_owner", task_id))

futuresearch-mcp/src/futuresearch_mcp/routes.py

Lines changed: 1 addition & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -81,57 +81,7 @@ async def _validate_poll_token(task_id: str, request: Request) -> JSONResponse |
8181
return None
8282

8383

84-
async def _validate_task_owner(task_id: str) -> JSONResponse | None:
85-
"""Verify the task has a recorded owner and that the poll token was
86-
issued for the same user. Returns a 403 response if ownership cannot
87-
be verified, or ``None`` if the caller may proceed.
88-
89-
Fail-closed: tasks without an ownership record are rejected. When a
90-
poll-token owner is recorded, it must match the task owner — this
91-
cross-check detects ownership-record tampering and ensures the poll
92-
token was legitimately issued for this task/user pair.
93-
"""
94-
owner = await redis_store.get_task_owner(task_id)
95-
if not owner:
96-
logger.warning(
97-
"REST access denied for task %s: no owner recorded (fail-closed)",
98-
task_id,
99-
)
100-
return JSONResponse(
101-
{"error": "Task ownership could not be verified"},
102-
status_code=403,
103-
headers=_cors_headers(),
104-
)
105-
106-
poll_owner = await redis_store.get_poll_token_owner(task_id)
107-
if not poll_owner:
108-
logger.warning(
109-
"REST access denied for task %s: no poll_owner recorded (fail-closed)",
110-
task_id,
111-
)
112-
return JSONResponse(
113-
{"error": "Task ownership could not be verified"},
114-
status_code=403,
115-
headers=_cors_headers(),
116-
)
117-
if poll_owner != owner:
118-
logger.warning(
119-
"REST access denied for task %s: poll_owner=%s != task_owner=%s",
120-
task_id,
121-
poll_owner,
122-
owner,
123-
)
124-
return JSONResponse(
125-
{"error": "Task ownership could not be verified"},
126-
status_code=403,
127-
headers=_cors_headers(),
128-
)
129-
130-
logger.debug("REST access granted for task %s (owner=%s)", task_id, owner)
131-
return None
132-
133-
134-
async def api_progress(request: Request) -> Response: # noqa: PLR0911
84+
async def api_progress(request: Request) -> Response:
13585
"""REST endpoint for the session widget to poll task progress."""
13686
cors = _cors_headers()
13787
if request.method == "OPTIONS":
@@ -148,9 +98,6 @@ async def api_progress(request: Request) -> Response: # noqa: PLR0911
14898
if err := await _validate_poll_token(task_id, request):
14999
return err
150100

151-
if err := await _validate_task_owner(task_id):
152-
return err
153-
154101
api_key = await redis_store.get_task_token(task_id)
155102

156103
if not api_key:
@@ -228,9 +175,6 @@ async def api_download_url(request: Request) -> Response:
228175
if err := await _validate_poll_token_bearer_only(task_id, request):
229176
return err
230177

231-
if err := await _validate_task_owner(task_id):
232-
return err
233-
234178
download_url = f"{settings.mcp_server_url}/api/results/{task_id}/download"
235179
return JSONResponse({"download_url": download_url}, headers=cors)
236180

futuresearch-mcp/src/futuresearch_mcp/tool_helpers.py

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
from futuresearch.generated.models.task_status import TaskStatus
2121
from futuresearch.generated.models.task_status_response import TaskStatusResponse
2222
from futuresearch.generated.types import Unset
23-
from mcp.server.auth.middleware.auth_context import get_access_token
2423
from mcp.server.fastmcp import Context
2524
from mcp.server.session import ServerSession
2625
from mcp.types import TextContent
@@ -199,33 +198,16 @@ def _submission_text(label: str, task_id: str, session_id: str = "") -> str:
199198

200199

201200
async def _record_task_ownership(task_id: str, token: str) -> str:
202-
"""Record task ownership and create a poll token.
201+
"""Store the API token and create a poll token for a submitted task.
203202
204203
Must run for every HTTP submission (including internal clients) so that
205-
downstream ownership checks in progress/results don't fail.
204+
downstream poll-token checks in progress/results don't fail.
206205
207206
Returns the poll_token.
208207
"""
209208
poll_token = secrets.token_urlsafe(32)
210209
await redis_store.store_task_token(task_id, token)
211-
212-
# Record task owner for cross-user access checks (HTTP mode only).
213-
# This MUST succeed — downstream ownership checks deny access when no
214-
# owner is recorded, so a silent failure here would lock the user out
215-
# of their own task.
216-
user_id = ""
217-
if settings.is_http:
218-
access_token = get_access_token()
219-
if not access_token or not access_token.client_id:
220-
raise RuntimeError(
221-
f"Cannot record task owner for {task_id}: no authenticated user"
222-
)
223-
user_id = access_token.client_id
224-
await redis_store.store_task_owner(task_id, user_id)
225-
226-
# Bind the poll token to the same user identity so the REST layer
227-
# can cross-check poll_owner == task_owner.
228-
await redis_store.store_poll_token(task_id, poll_token, user_id=user_id)
210+
await redis_store.store_poll_token(task_id, poll_token)
229211
return poll_token
230212

231213

futuresearch-mcp/src/futuresearch_mcp/tools.py

Lines changed: 0 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
)
2828
from futuresearch.session import create_session, list_sessions
2929
from futuresearch.task import cancel_task
30-
from mcp.server.auth.middleware.auth_context import get_access_token
3130
from mcp.types import CallToolResult, TextContent, ToolAnnotations
3231
from pydantic import BaseModel, create_model
3332

@@ -83,50 +82,6 @@ def _error_result(text: str) -> CallToolResult:
8382
)
8483

8584

86-
async def _check_task_ownership(task_id: str) -> list[TextContent] | None:
87-
"""Verify the current user owns *task_id*. Returns an error response if
88-
access should be denied, or ``None`` if the caller may proceed.
89-
90-
Only active in HTTP mode; always returns ``None`` for stdio.
91-
92-
When no owner is recorded in Redis (e.g. tasks created via the presigned
93-
upload URL flow, which bypasses the MCP tool layer), the current user is
94-
auto-registered as owner. The Engine independently validates ownership
95-
on every API call via session-level checks, so this is safe — a user
96-
cannot claim a task they don't own because subsequent Engine calls would
97-
fail.
98-
"""
99-
if not settings.is_http:
100-
return None
101-
102-
access_token = get_access_token()
103-
user_id = access_token.client_id if access_token else None
104-
if not user_id:
105-
return [TextContent(type="text", text="Access denied: no authenticated user.")]
106-
107-
owner = await redis_store.get_task_owner(task_id)
108-
if not owner:
109-
# Task was likely created outside the MCP tool layer (e.g. presigned
110-
# URL upload). Claim it for the current user — the Engine will
111-
# independently reject any API calls if this user doesn't actually
112-
# own the task's session.
113-
logger.info(
114-
"No owner recorded for task %s — auto-registering user %s",
115-
task_id,
116-
user_id,
117-
)
118-
await redis_store.store_task_owner(task_id, user_id)
119-
return None
120-
121-
if user_id != owner:
122-
return [
123-
TextContent(
124-
type="text", text="Access denied: this task belongs to another user."
125-
)
126-
]
127-
return None
128-
129-
13085
@mcp.tool(
13186
name="futuresearch_browse_lists",
13287
structured_output=False,
@@ -958,18 +913,6 @@ async def futuresearch_progress(
958913
client = _get_client(ctx)
959914
task_id = params.task_id
960915

961-
# ── Cross-user access check ──────────────────────────────────
962-
try:
963-
if denied := await _check_task_ownership(task_id):
964-
return denied
965-
except Exception:
966-
logger.exception("Could not verify task ownership for %s", task_id)
967-
return [
968-
TextContent(
969-
type="text", text="Unable to verify task ownership. Please try again."
970-
)
971-
]
972-
973916
# Block server-side before polling — controls the cadence
974917
await asyncio.sleep(redis_store.PROGRESS_POLL_DELAY)
975918

@@ -1110,14 +1053,6 @@ async def futuresearch_results_http(
11101053
if is_internal_client():
11111054
skip_widget = True
11121055

1113-
# ── Cross-user access check ──────────────────────────────────
1114-
try:
1115-
if denied := await _check_task_ownership(task_id):
1116-
return CallToolResult(content=denied, isError=True) # pyright: ignore[reportArgumentType] # list invariance
1117-
except Exception:
1118-
logger.exception("Could not verify task ownership for %s", task_id)
1119-
return _error_result("Unable to verify task ownership. Please try again.")
1120-
11211056
# ── Fetch paginated rows directly from Engine ─────────────────
11221057
try:
11231058
rows, total_count, session_id, artifact_id = await _fetch_task_result(
@@ -1369,18 +1304,6 @@ async def futuresearch_cancel(
13691304
client = _get_client(ctx)
13701305
task_id = params.task_id
13711306

1372-
# ── Cross-user access check ──────────────────────────────────
1373-
try:
1374-
if denied := await _check_task_ownership(task_id):
1375-
return denied
1376-
except Exception:
1377-
logger.exception("Could not verify task ownership for %s", task_id)
1378-
return [
1379-
TextContent(
1380-
type="text", text="Unable to verify task ownership. Please try again."
1381-
)
1382-
]
1383-
13841307
try:
13851308
await cancel_task(task_id=UUID(task_id), client=client)
13861309
return [

futuresearch-mcp/tests/test_http_integration.py

Lines changed: 9 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -142,25 +142,11 @@ async def test_unauthorized_with_wrong_token(self, client: httpx.AsyncClient):
142142
assert resp.status_code == 403
143143

144144
@pytest.mark.asyncio
145-
async def test_denied_without_owner(self, client: httpx.AsyncClient):
146-
"""Poll token is valid but no task owner recorded → fail-closed 403."""
145+
async def test_unknown_task_returns_404(self, client: httpx.AsyncClient):
147146
task_id = str(uuid4())
148147
poll_token = secrets.token_urlsafe(16)
149148
await redis_store.store_poll_token(task_id, poll_token)
150-
# No task owner stored — ownership check should reject
151149

152-
resp = await client.get(
153-
f"/api/progress/{task_id}", params={"token": poll_token}
154-
)
155-
assert resp.status_code == 403
156-
assert resp.json()["error"] == "Task ownership could not be verified"
157-
158-
@pytest.mark.asyncio
159-
async def test_unknown_task_returns_404(self, client: httpx.AsyncClient):
160-
task_id = str(uuid4())
161-
poll_token = secrets.token_urlsafe(16)
162-
await redis_store.store_poll_token(task_id, poll_token, user_id="test-user")
163-
await redis_store.store_task_owner(task_id, "test-user")
164150
# No task_token stored
165151

166152
resp = await client.get(
@@ -173,9 +159,8 @@ async def test_unknown_task_returns_404(self, client: httpx.AsyncClient):
173159
async def test_running_task_returns_progress(self, client: httpx.AsyncClient):
174160
task_id = str(uuid4())
175161
poll_token = secrets.token_urlsafe(16)
176-
await redis_store.store_poll_token(task_id, poll_token, user_id="test-user")
162+
await redis_store.store_poll_token(task_id, poll_token)
177163
await redis_store.store_task_token(task_id, "api-key-123")
178-
await redis_store.store_task_owner(task_id, "test-user")
179164

180165
status_resp = _make_status_response(
181166
status="running", completed=7, total=20, failed=1, running=4
@@ -205,9 +190,8 @@ async def test_running_task_returns_progress(self, client: httpx.AsyncClient):
205190
async def test_completed_task_cleans_up_tokens(self, client: httpx.AsyncClient):
206191
task_id = str(uuid4())
207192
poll_token = secrets.token_urlsafe(16)
208-
await redis_store.store_poll_token(task_id, poll_token, user_id="test-user")
193+
await redis_store.store_poll_token(task_id, poll_token)
209194
await redis_store.store_task_token(task_id, "api-key")
210-
await redis_store.store_task_owner(task_id, "test-user")
211195

212196
status_resp = _make_status_response(
213197
status="completed", completed=10, total=10, failed=0, running=0
@@ -233,9 +217,8 @@ async def test_completed_task_cleans_up_tokens(self, client: httpx.AsyncClient):
233217
async def test_api_error_returns_500(self, client: httpx.AsyncClient):
234218
task_id = str(uuid4())
235219
poll_token = secrets.token_urlsafe(16)
236-
await redis_store.store_poll_token(task_id, poll_token, user_id="test-user")
220+
await redis_store.store_poll_token(task_id, poll_token)
237221
await redis_store.store_task_token(task_id, "api-key")
238-
await redis_store.store_task_owner(task_id, "test-user")
239222

240223
with patch(
241224
"futuresearch_mcp.routes.get_task_status_tasks_task_id_status_get.asyncio",
@@ -264,9 +247,8 @@ async def test_progress_lifecycle(self, client: httpx.AsyncClient):
264247
poll_token = secrets.token_urlsafe(16)
265248

266249
# 1. Store tokens (simulating what futuresearch_agent does)
267-
await redis_store.store_poll_token(task_id, poll_token, user_id="test-user")
250+
await redis_store.store_poll_token(task_id, poll_token)
268251
await redis_store.store_task_token(task_id, "api-key-for-polling")
269-
await redis_store.store_task_owner(task_id, "test-user")
270252

271253
# 2. Poll progress — task is running
272254
running_resp = _make_status_response(status="running", completed=1, total=3)
@@ -318,8 +300,7 @@ async def test_bearer_auth_via_real_http_header(self, client: httpx.AsyncClient)
318300
"""Authorization: Bearer header is parsed correctly by Starlette."""
319301
task_id = str(uuid4())
320302
poll_token = secrets.token_urlsafe(16)
321-
await redis_store.store_poll_token(task_id, poll_token, user_id="test-user")
322-
await redis_store.store_task_owner(task_id, "test-user")
303+
await redis_store.store_poll_token(task_id, poll_token)
323304

324305
resp = await client.get(
325306
f"/api/results/{task_id}/download-token",
@@ -350,8 +331,7 @@ async def test_query_param_rejected_through_real_url(
350331
"""Poll token via ?token= query param in a real URL is rejected."""
351332
task_id = str(uuid4())
352333
poll_token = secrets.token_urlsafe(16)
353-
await redis_store.store_poll_token(task_id, poll_token, user_id="test-user")
354-
await redis_store.store_task_owner(task_id, "test-user")
334+
await redis_store.store_poll_token(task_id, poll_token)
355335

356336
resp = await client.get(
357337
f"/api/results/{task_id}/download-token",
@@ -364,8 +344,7 @@ async def test_multiple_calls_return_same_url(self, client: httpx.AsyncClient):
364344
"""Two sequential calls for the same task return the same download URL."""
365345
task_id = str(uuid4())
366346
poll_token = secrets.token_urlsafe(16)
367-
await redis_store.store_poll_token(task_id, poll_token, user_id="test-user")
368-
await redis_store.store_task_owner(task_id, "test-user")
347+
await redis_store.store_poll_token(task_id, poll_token)
369348

370349
headers = {"Authorization": f"Bearer {poll_token}"}
371350
resp1 = await client.get(
@@ -421,8 +400,7 @@ async def test_get_url_and_download(self, client: httpx.AsyncClient):
421400
task_id = str(uuid4())
422401
poll_token = secrets.token_urlsafe(16)
423402

424-
await redis_store.store_poll_token(task_id, poll_token, user_id="test-user")
425-
await redis_store.store_task_owner(task_id, "test-user")
403+
await redis_store.store_poll_token(task_id, poll_token)
426404
await redis_store.store_task_token(task_id, "sk-cho-test")
427405

428406
# 1. Get the download URL

0 commit comments

Comments
 (0)