Skip to content

Security audit: tenant isolation crossover report #217

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
RafaelPo merged 1 commit into from
Feb 25, 2026
Merged

Security audit: tenant isolation crossover report #217

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions everyrow-mcp/src/everyrow_mcp/redis_store.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,12 +250,20 @@ async def pop_task_token(task_id: str) -> None:
# ── Poll tokens ───────────────────────────────────────────────


async def store_poll_token(task_id: str, poll_token: str) -> None:
await get_redis_client().setex(
async def store_poll_token(task_id: str, poll_token: str, user_id: str = "") -> None:
"""Store an encrypted poll token, optionally bound to a user identity."""
client = get_redis_client()
await client.setex(
name=build_key("poll_token", task_id),
time=TOKEN_TTL,
value=encrypt_value(poll_token),
)
if user_id:
await client.setex(
name=build_key("poll_owner", task_id),
time=TOKEN_TTL,
value=user_id,
)


async def get_poll_token(task_id: str) -> str | None:
Expand All @@ -265,6 +273,11 @@ async def get_poll_token(task_id: str) -> str | None:
return decrypt_value(encrypted)


async def get_poll_token_owner(task_id: str) -> str | None:
"""Return the user_id bound to a poll token, or None if not set."""
return await get_redis_client().get(build_key("poll_owner", task_id))


# ── Task ownership (user-scoped data isolation) ──────────────


Expand Down
58 changes: 57 additions & 1 deletion everyrow-mcp/src/everyrow_mcp/routes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,57 @@ async def _validate_poll_token(task_id: str, request: Request) -> JSONResponse |
return None


async def api_progress(request: Request) -> Response:
async def _validate_task_owner(task_id: str) -> JSONResponse | None:
"""Verify the task has a recorded owner and that the poll token was
issued for the same user. Returns a 403 response if ownership cannot
be verified, or ``None`` if the caller may proceed.

Fail-closed: tasks without an ownership record are rejected. When a
poll-token owner is recorded, it must match the task owner — this
cross-check detects ownership-record tampering and ensures the poll
token was legitimately issued for this task/user pair.
"""
owner = await redis_store.get_task_owner(task_id)
Comment thread
RafaelPo marked this conversation as resolved.
if not owner:
logger.warning(
"REST access denied for task %s: no owner recorded (fail-closed)",
task_id,
)
return JSONResponse(
{"error": "Task ownership could not be verified"},
status_code=403,
headers=_cors_headers(),
)

poll_owner = await redis_store.get_poll_token_owner(task_id)
if not poll_owner:
logger.warning(
"REST access denied for task %s: no poll_owner recorded (fail-closed)",
task_id,
)
return JSONResponse(
{"error": "Task ownership could not be verified"},
status_code=403,
headers=_cors_headers(),
)
if poll_owner != owner:
logger.warning(
"REST access denied for task %s: poll_owner=%s != task_owner=%s",
task_id,
poll_owner,
owner,
)
return JSONResponse(
{"error": "Task ownership could not be verified"},
status_code=403,
headers=_cors_headers(),
)

logger.debug("REST access granted for task %s (owner=%s)", task_id, owner)
return None


async def api_progress(request: Request) -> Response: # noqa: PLR0911
"""REST endpoint for the session widget to poll task progress."""
cors = _cors_headers()
if request.method == "OPTIONS":
Expand All @@ -90,6 +140,9 @@ async def api_progress(request: Request) -> Response:
if err := await _validate_poll_token(task_id, request):
return err

if err := await _validate_task_owner(task_id):
return err

api_key = await redis_store.get_task_token(task_id)

if not api_key:
Expand Down Expand Up @@ -143,6 +196,9 @@ async def api_download(request: Request) -> Response:
if err := await _validate_poll_token(task_id, request):
return err

if err := await _validate_task_owner(task_id):
return err

csv_text = await redis_store.get_result_csv(task_id)
if csv_text is None:
return JSONResponse(
Expand Down
9 changes: 7 additions & 2 deletions everyrow-mcp/src/everyrow_mcp/tool_helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,19 +91,24 @@ async def _submission_ui_json(
"""Build JSON for the session MCP App widget, and store the token for polling."""
poll_token = secrets.token_urlsafe(32)
await redis_store.store_task_token(task_id, token)
await redis_store.store_poll_token(task_id, poll_token)

# Record task owner for cross-user access checks (HTTP mode only).
# This MUST succeed — downstream ownership checks deny access when no
# owner is recorded, so a silent failure here would lock the user out
# of their own task.
user_id = ""
if settings.is_http:
access_token = get_access_token()
if not access_token or not access_token.client_id:
raise RuntimeError(
f"Cannot record task owner for {task_id}: no authenticated user"
)
Comment on lines 100 to 105

@sentry sentry Bot Feb 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: In no-auth HTTP mode, task submission will always fail with a RuntimeError because _submission_ui_json unconditionally tries to get an access token which doesn't exist.
Severity: HIGH

Suggested Fix

Modify _submission_ui_json to handle the no-auth HTTP case. Either bypass the ownership check entirely when no access token is present in no-auth mode, or assign a default system/synthetic user as the task owner for these submissions. This will prevent the RuntimeError and allow tasks to be submitted successfully.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: everyrow-mcp/src/everyrow_mcp/tool_helpers.py#L100-L105

Potential issue: In the `_submission_ui_json` function, when running in HTTP mode
(`settings.is_http` is true), the code attempts to retrieve an `access_token`. In the
supported no-auth HTTP mode, `get_access_token()` correctly returns `None`. This
triggers a conditional check that raises a `RuntimeError`, halting task submission. This
occurs because the implementation does not distinguish between authenticated and
unauthenticated HTTP modes, enforcing an ownership check that cannot be satisfied
without a user. As a result, any tool attempting to submit a task in no-auth mode will
fail, rendering this mode unusable.

await redis_store.store_task_owner(task_id, access_token.client_id)
user_id = access_token.client_id
await redis_store.store_task_owner(task_id, user_id)

# Bind the poll token to the same user identity so the REST layer
# can cross-check poll_owner == task_owner.
await redis_store.store_poll_token(task_id, poll_token, user_id=user_id)
data: dict[str, Any] = {
"session_url": session_url,
"task_id": task_id,
Expand Down
29 changes: 24 additions & 5 deletions everyrow-mcp/tests/test_http_integration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,25 @@ async def test_unauthorized_with_wrong_token(self, client: httpx.AsyncClient):
assert resp.status_code == 403

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

resp = await client.get(
f"/api/progress/{task_id}", params={"token": poll_token}
)
assert resp.status_code == 403
assert resp.json()["error"] == "Task ownership could not be verified"

@pytest.mark.asyncio
async def test_unknown_task_returns_404(self, client: httpx.AsyncClient):
task_id = str(uuid4())
poll_token = secrets.token_urlsafe(16)
await redis_store.store_poll_token(task_id, poll_token, user_id="test-user")
await redis_store.store_task_owner(task_id, "test-user")
# No task_token stored

resp = await client.get(
Expand All @@ -148,8 +163,9 @@ async def test_unknown_task_returns_404(self, client: httpx.AsyncClient):
async def test_running_task_returns_progress(self, client: httpx.AsyncClient):
task_id = str(uuid4())
poll_token = secrets.token_urlsafe(16)
await redis_store.store_poll_token(task_id, poll_token)
await redis_store.store_poll_token(task_id, poll_token, user_id="test-user")
await redis_store.store_task_token(task_id, "api-key-123")
await redis_store.store_task_owner(task_id, "test-user")

status_resp = _make_status_response(
status="running", completed=7, total=20, failed=1, running=4
Expand Down Expand Up @@ -180,8 +196,9 @@ async def test_running_task_returns_progress(self, client: httpx.AsyncClient):
async def test_completed_task_cleans_up_tokens(self, client: httpx.AsyncClient):
task_id = str(uuid4())
poll_token = secrets.token_urlsafe(16)
await redis_store.store_poll_token(task_id, poll_token)
await redis_store.store_poll_token(task_id, poll_token, user_id="test-user")
await redis_store.store_task_token(task_id, "api-key")
await redis_store.store_task_owner(task_id, "test-user")

status_resp = _make_status_response(
status="completed", completed=10, total=10, failed=0, running=0
Expand All @@ -207,8 +224,9 @@ async def test_completed_task_cleans_up_tokens(self, client: httpx.AsyncClient):
async def test_api_error_returns_500(self, client: httpx.AsyncClient):
task_id = str(uuid4())
poll_token = secrets.token_urlsafe(16)
await redis_store.store_poll_token(task_id, poll_token)
await redis_store.store_poll_token(task_id, poll_token, user_id="test-user")
await redis_store.store_task_token(task_id, "api-key")
await redis_store.store_task_owner(task_id, "test-user")

with patch(
"everyrow_mcp.routes.get_task_status_tasks_task_id_status_get.asyncio",
Expand Down Expand Up @@ -237,8 +255,9 @@ async def test_progress_lifecycle(self, client: httpx.AsyncClient):
poll_token = secrets.token_urlsafe(16)

# 1. Store tokens (simulating what everyrow_agent does)
await redis_store.store_poll_token(task_id, poll_token)
await redis_store.store_poll_token(task_id, poll_token, user_id="test-user")
await redis_store.store_task_token(task_id, "api-key-for-polling")
await redis_store.store_task_owner(task_id, "test-user")

# 2. Poll progress — task is running
running_resp = _make_status_response(status="running", completed=1, total=3)
Expand Down
22 changes: 20 additions & 2 deletions everyrow-mcp/tests/test_result_store.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,9 @@ async def test_valid_download(self, client: httpx.AsyncClient):
poll_token = secrets.token_urlsafe(16)
csv_text = "name,score\nAlice,95\nBob,87\n"

await redis_store.store_poll_token(task_id, poll_token)
await redis_store.store_poll_token(task_id, poll_token, user_id="test-user")
await redis_store.store_result_csv(task_id, csv_text)
await redis_store.store_task_owner(task_id, "test-user")

resp = await client.get(
f"/api/results/{task_id}/download", params={"token": poll_token}
Expand All @@ -407,11 +408,28 @@ async def test_bad_token_returns_403(self, client: httpx.AsyncClient):
assert resp.status_code == 403

@pytest.mark.asyncio
async def test_missing_csv_returns_404(self, client: httpx.AsyncClient):
async def test_denied_without_owner(self, client: httpx.AsyncClient):
"""Valid poll token but no task owner → fail-closed 403."""
task_id = str(uuid4())
poll_token = secrets.token_urlsafe(16)

await redis_store.store_poll_token(task_id, poll_token)
await redis_store.store_result_csv(task_id, "data")
# No task owner stored

resp = await client.get(
f"/api/results/{task_id}/download", params={"token": poll_token}
)
assert resp.status_code == 403
assert resp.json()["error"] == "Task ownership could not be verified"

@pytest.mark.asyncio
async def test_missing_csv_returns_404(self, client: httpx.AsyncClient):
task_id = str(uuid4())
poll_token = secrets.token_urlsafe(16)

await redis_store.store_poll_token(task_id, poll_token, user_id="test-user")
await redis_store.store_task_owner(task_id, "test-user")
# No CSV stored

resp = await client.get(
Expand Down
66 changes: 61 additions & 5 deletions everyrow-mcp/tests/test_routes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,62 @@ async def test_missing_poll_token_returns_403(self):
assert resp.status_code == 403

@pytest.mark.asyncio
async def test_missing_task_token_returns_404(self):
async def test_denied_without_owner(self):
"""Valid poll token but no task owner → fail-closed 403."""
task_id = str(uuid4())
poll_token = secrets.token_urlsafe(16)
await redis_store.store_poll_token(task_id, poll_token)
# No task owner stored

req = FakeRequest(
path_params={"task_id": task_id},
headers={"authorization": f"Bearer {poll_token}"},
)
resp = await api_progress(req) # pyright: ignore[reportArgumentType]
assert resp.status_code == 403
body = json.loads(bytes(resp.body).decode())
assert body["error"] == "Task ownership could not be verified"

@pytest.mark.asyncio
async def test_denied_without_poll_owner(self):
"""Task owner exists but poll token has no bound user → fail-closed 403."""
task_id = str(uuid4())
poll_token = secrets.token_urlsafe(16)
await redis_store.store_poll_token(task_id, poll_token) # no user_id
await redis_store.store_task_owner(task_id, "test-user")

req = FakeRequest(
path_params={"task_id": task_id},
headers={"authorization": f"Bearer {poll_token}"},
)
resp = await api_progress(req) # pyright: ignore[reportArgumentType]
assert resp.status_code == 403
body = json.loads(bytes(resp.body).decode())
assert body["error"] == "Task ownership could not be verified"

@pytest.mark.asyncio
async def test_denied_on_owner_mismatch(self):
"""Poll token bound to user-A but task_owner tampered to user-B → 403."""
task_id = str(uuid4())
poll_token = secrets.token_urlsafe(16)
await redis_store.store_poll_token(task_id, poll_token, user_id="user-A")
await redis_store.store_task_owner(task_id, "user-B")

req = FakeRequest(
path_params={"task_id": task_id},
headers={"authorization": f"Bearer {poll_token}"},
)
resp = await api_progress(req) # pyright: ignore[reportArgumentType]
assert resp.status_code == 403
body = json.loads(bytes(resp.body).decode())
assert body["error"] == "Task ownership could not be verified"

@pytest.mark.asyncio
async def test_missing_task_token_returns_404(self):
task_id = str(uuid4())
poll_token = secrets.token_urlsafe(16)
await redis_store.store_poll_token(task_id, poll_token, user_id="test-user")
await redis_store.store_task_owner(task_id, "test-user")
# No task token stored

req = FakeRequest(
Expand All @@ -131,8 +183,9 @@ async def test_valid_progress_via_auth_header(self):
"""Poll token sent via Authorization: Bearer header works."""
task_id = str(uuid4())
poll_token = secrets.token_urlsafe(16)
await redis_store.store_poll_token(task_id, poll_token)
await redis_store.store_poll_token(task_id, poll_token, user_id="test-user")
await redis_store.store_task_token(task_id, "api-key-123")
await redis_store.store_task_owner(task_id, "test-user")

status_resp = _make_status_response(
status="running", completed=3, total=10, failed=1, running=2
Expand Down Expand Up @@ -166,8 +219,9 @@ async def test_backward_compat_query_param_for_download(self):
"""Poll token via ?token= query param still works (for download links)."""
task_id = str(uuid4())
poll_token = secrets.token_urlsafe(16)
await redis_store.store_poll_token(task_id, poll_token)
await redis_store.store_poll_token(task_id, poll_token, user_id="test-user")
await redis_store.store_task_token(task_id, "api-key-123")
await redis_store.store_task_owner(task_id, "test-user")

status_resp = _make_status_response(
status="running", completed=3, total=10, failed=1, running=2
Expand Down Expand Up @@ -200,8 +254,9 @@ async def test_backward_compat_query_param_for_download(self):
async def test_completed_task_pops_tokens(self):
task_id = str(uuid4())
poll_token = secrets.token_urlsafe(16)
await redis_store.store_poll_token(task_id, poll_token)
await redis_store.store_poll_token(task_id, poll_token, user_id="test-user")
await redis_store.store_task_token(task_id, "api-key")
await redis_store.store_task_owner(task_id, "test-user")

status_resp = _make_status_response(status="completed", completed=10, total=10)

Expand Down Expand Up @@ -229,8 +284,9 @@ async def test_completed_task_pops_tokens(self):
async def test_api_error_returns_500(self):
task_id = str(uuid4())
poll_token = secrets.token_urlsafe(16)
await redis_store.store_poll_token(task_id, poll_token)
await redis_store.store_poll_token(task_id, poll_token, user_id="test-user")
await redis_store.store_task_token(task_id, "api-key")
await redis_store.store_task_owner(task_id, "test-user")

req = FakeRequest(
path_params={"task_id": task_id},
Expand Down