fix(memory): return HTTP 403 for cross-agent scope mismatch#737
fix(memory): return HTTP 403 for cross-agent scope mismatch#737jdjioe5-cpu wants to merge 1 commit into
Conversation
The session-scope guard in every agent-scoped endpoint mapped
cross-agent access (session.agent_id != URL agent_id) to a generic
Exception, which the global error mapper then converted to HTTP 500.
That is semantically wrong: an authorization failure must be 403, not 500.
Replace all 12 occurrences of
raise map_error_to_http_exception(
Exception(
f"Session is for agent '{session.agent_id}', cannot access '{agent_id}'"
)
)
with
raise HTTPException(
status_code=403,
detail=f"Session is for agent '{session.agent_id}', cannot access '{agent_id}'",
)
Endpoints affected: remember, batch_remember, upload_file, recall,
answer, generate_daily_summary, generate_conflict_report, list_conflicts,
resolve_conflict, recall_as_of, recall_changed_since, recall_recent.
This fixes the CodeRabbit review item flagged on PR moorcheh-ai#732's forget-endpoint
follow-up (PR moorcheh-ai#733) and the parallel review item on PR moorcheh-ai#633's edit-endpoint
follow-up (PR moorcheh-ai#736). No new tests in this commit; the test coverage lives
in those follow-up PRs so the test/fix split stays visible.
📝 WalkthroughWalkthroughIn ChangesSession Scope 403 Enforcement
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
memanto/app/routes/memory.py (1)
150-152: ⚡ Quick winConsider extracting agent-id scope validation into a helper.
The same 4-line agent_id mismatch check is repeated across 12 endpoints. Extracting to a reusable helper would reduce duplication and centralize the error message format.
♻️ Example helper extraction
Add to
memanto/app/routes/auth_deps.pyor a newmemanto/app/routes/scope_validation.py:def validate_session_scope(session: Session, agent_id: str) -> None: """Enforce that session.agent_id matches the requested agent_id.""" if session.agent_id != agent_id: raise HTTPException( status_code=403, detail=f"Session is for agent '{session.agent_id}', cannot access '{agent_id}'", )Then replace each check with:
- if session.agent_id != agent_id: - raise HTTPException( - status_code=403, - detail=f"Session is for agent '{session.agent_id}', cannot access '{agent_id}'", - ) + validate_session_scope(session, agent_id)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@memanto/app/routes/memory.py` around lines 150 - 152, Extract the repeated agent_id scope validation check (comparing session.agent_id against the requested agent_id and raising HTTPException with status_code 403) into a reusable helper function called validate_session_scope that accepts a Session object and agent_id string parameter. Create this helper in memanto/app/routes/auth_deps.py or a new memanto/app/routes/scope_validation.py file, then replace all 12 occurrences of the 4-line validation block across the endpoints (including the one at the specified location) with a single call to this helper function. This centralizes the error message format and eliminates code duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@memanto/app/routes/memory.py`:
- Around line 150-152: Extract the repeated agent_id scope validation check
(comparing session.agent_id against the requested agent_id and raising
HTTPException with status_code 403) into a reusable helper function called
validate_session_scope that accepts a Session object and agent_id string
parameter. Create this helper in memanto/app/routes/auth_deps.py or a new
memanto/app/routes/scope_validation.py file, then replace all 12 occurrences of
the 4-line validation block across the endpoints (including the one at the
specified location) with a single call to this helper function. This centralizes
the error message format and eliminates code duplication.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7d040b40-aee0-4da6-ab5e-d3cbc91e897b
📒 Files selected for processing (1)
memanto/app/routes/memory.py
Summary
The session-scope guard in every agent-scoped endpoint mapped cross-agent
access (
session.agent_id != URL agent_id) to a genericException,which the global error mapper then converted to HTTP 500. That is
semantically wrong: an authorization failure must be 403, not 500.
Replace all 12 occurrences with a direct
HTTPException(status_code=403, ...).Why now
DELETE /memories/{id}test (cross-agent rejection test was asserting 403 but the current
code returns 500 → test fails against current
main).fix is the cleanest place to land the change.
Endpoints touched
remember,batch_remember,upload_file,recall,answer,generate_daily_summary,generate_conflict_report,list_conflicts,resolve_conflict,recall_as_of,recall_changed_since,recall_recent.Diff
beyond the status code.
Validation
python -m ruff check memanto/app/routes/memory.py→ cleanstatus_code == 500for an unrelatedcode path (
test_create_agent_fails_when_server_key_missing) are notaffected (different route, different failure mode).
500for any cross-agent check (confirmed bygrep over
tests/).Related
feat: add memory forget command)403for cross-agent DELETE; currently fails againstmainbecause the underlying route returns 500.feat: add memory edit command)mainand thecross-agent tests will pass.
Refs PR #733, PR #736
Summary by CodeRabbit