fix(api): bind /recall to the authenticated agent the way /search does

[zznate]

Summary

Bind POST /recall to the authenticated agent the way POST /search is bound: an agent credential may only filter to itself, and an omitted filter_agent_id recalls as the authenticated agent, not as the tenant. A tenant or user credential is unchanged.

Related Issue

Closes #994

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that changes existing behavior)
• Documentation update
• Refactor / internal cleanup
• Other:

How Has This Been Tested?

Four tests in tests/test_route_authz_gaps.py, next to the /search ones from #801 and using the same as_auth fixture:

• a peer in filter_agent_id is refused with 403 and the same message /search gives;
• filtering to the agent's own id is allowed;
• an omitted filter reaches search_memories with caller_agent_id equal to the authenticated agent and filter_agent_id None (the search call is patched at the module seam to capture its arguments);
• a tenant credential with no bound agent may still filter by any agent.

core-api/.venv/bin/ruff check core-api/src/core_api/routes/memories.py tests/test_route_authz_gaps.py   # All checks passed
core-api/.venv/bin/ruff format --check core-api/src/core_api/routes/memories.py tests/test_route_authz_gaps.py
core-api/.venv/bin/pytest tests/                                                                        # 5304 passed

Verified live on a 2.30.0 image with this change cherry-picked, real bge-m3 embeddings, and MEMCLAW_API_KEY set so X-Agent-ID binds. The reproduction from the issue now gives the /search answers on /recall: a trust-1 peer with no filter is forced to its own fleet and does not see another fleet's scope_team row, and a filter naming the owner returns 403. An external conformance check that runs an owner-side control read beside the peer read passes on both paths.

Checklist

• I have read CONTRIBUTING.md
• I have added tests that cover my changes (or explained why none are needed)
• ruff check and ruff format --check pass
• mypy passes
• pytest passes locally
• I have updated relevant documentation (README, docs, etc.)
• I have updated CHANGELOG.md under the Unreleased section (if user-facing)

Additional Notes

• The change is the /search block moved to /recall, with the comment saying why, so the two read paths now read the same. The trust<2 fleet forcing and enforce_fleet_read run against the effective identity, and search_memories gets it as caller_agent_id.
• The MCP twin, caura_recall, already passes the authenticated agent_id as caller_agent_id and is unchanged.
• No documentation change was needed: the public API contract for /recall already describes the /search semantics; this makes the route match it.
• CHANGELOG.md had no Unreleased section on main; this adds one above 2.32.0. If feat(common): allow a self-hosted base URL for the chat LLM path #993 lands first, the two entries merge under the same heading.

[erni-a]

Thanks for this — and for including the tenant-credential case in the tests. That's the one people usually leave out, and it's what makes the change reviewable rather than just plausible.

I've verified the reasoning against main rather than taking it on faith: /search already carries this binding (its own comments describe it as closing "the search side of the BOLA chain"), and the MCP caura_recall path resolves identity from the authenticated agent, so the REST /recall handler was the outlier. Your change lines it up with both, and the eff_agent_id fallback keeps tenant/user credentials behaving as they do today.

Two process notes:

1. Security-affecting changes. For anything with a security dimension, we'd rather you go through private vulnerability reporting, or security@caura.ai as a fallback — see SECURITY.md. It lets us prepare the patch and a public advisory together, and you get credited in the advisory. No issue at all this time, and I'm happy to credit you in one for this if you'd like it — just letting you know the door exists.

2. Before this can land:

• Rebase onto main. The PR is conflicting right now. Most relevantly, fix(api): /recall and /ingest/commit 500 under headers_enabled=True (D14 follow-up) #984 added a response: Response parameter to recall_endpoint (slowapi needs it to inject rate-limit headers, and its absence was 500ing the route) in the same region you're editing. A fair amount else has landed since you opened this.
• Drop the CHANGELOG.md edit. That file is generated by release-please from Conventional Commit subjects — see changelog-path in release-please-config.json — so a hand-written entry will duplicate or conflict with the generated one. Your commit subject already carries it.

I'm approving the workflows so CI runs, and I'll take it through review once it's rebased and green.

One behavioural note worth capturing in the release notes rather than the code: an agent credential that currently omits filter_agent_id and relies on tenant-wide recall will start getting fleet-scoped results after this. That's the fix doing its job, but it will look like a change to anyone who built against the old behaviour.

[erni-a]

CI has run now, and the one red check is not your change — it's the rebase staleness, so don't go chasing it.

The failing gate is Broker OpenAPI has no breaking change vs main. It regenerates the broker spec from the branch's code and diffs it against the copy committed on main. The last commit to touch that spec is #1005, merged a few hours ago, which your branch predates — so the diff reports properties on GET /api/v1/health (version, platform_init_errors, unhealthy_dependencies) as "removed" when in fact your branch simply doesn't have the code that produces them yet. A rebase clears it.

For what it's worth, I validated your change against current main locally rather than waiting on that:

• Applies cleanly onto today's main (the "conflicting" flag was computed before the recent merges).
• It correctly preserves fix(api): /recall and /ingest/commit 500 under headers_enabled=True (D14 follow-up) #984's response: Response parameter on recall_endpoint, and the binding lands inside recall_endpoint rather than being absorbed into /search's existing eff_agent_id block — both things I specifically checked, since that region has moved a lot today.
• Ruff (CI's exact scopes), the legacy-name ratchet and the do-not-touch sentinel all clean; 67 tests green across the authz, recall, rate-limit and MCP suites; full-suite delta against main is zero.
• Reverting only memories.py and keeping your tests fails exactly test_recall_cannot_borrow_a_peer_identity_via_filter_agent_id and test_recall_binds_an_omitted_filter_to_the_authenticated_agent, while both no-regression tests still pass. That's the right two failing for the right reason — the tests earn their keep.

So it's just the rebase and dropping the CHANGELOG.md hunk. If you'd rather not, say the word — maintainerCanModify is on, so we can do the rebase on your branch and keep your authorship rather than let a fix like this sit.

[zznate]

Rebased onto main at d82eebd (head 4bb33b8) and dropped the CHANGELOG.md hunk; the commit subject carries the note. #984's response: Response parameter on recall_endpoint is kept, and the binding sits inside recall_endpoint as before.

Local run against this head: tests/test_route_authz_gaps.py 28 passed; full suite 5363 passed, 16 failed, the same 16 you see on main (test_atomic_fact_unembedded_persist, test_crystallization_must_not_wedge, test_embed_off_hot_path and friends), none in the authz, recall, rate-limit, or MCP suites.

For the release notes, in one sentence: an agent credential that omits filter_agent_id and relied on tenant-wide recall now gets fleet-scoped results, the same as /search.

Noted on the security process; the next report of this kind goes through private vulnerability reporting first. Credit in an advisory is welcome, thank you.

[zznate]

Rebased onto main at 38649e1 (head 9733014, the same single commit, no conflicts). Local run from a clean worktree: tests/test_route_authz_gaps.py 28 passed; full suite 5444 passed, 16 failed, the same 16 that fail on main at 38649e1 (test_integration_search, test_p3_2_relation_weights, test_a7_classifier_recall, pipeline/test_search_pipeline, test_ph6_entity_linking_storage), none in the authz, recall, rate-limit, or MCP suites.

[Eldad-Caura]

Thank you for this, and I'm sorry it sat for ten days while we shipped our own fix for the same bug without referencing yours. That's on us, not on you.

Your diagnosis was correct in full. #994 is one of the better bug reports this repo has received: exact source line references on a named commit, a working reproduction, the right root cause, and the observation that the MCP twin caura_recall was already bound and therefore unaffected. You also identified both consequences separately — the unchecked filter and the omitted-filter case passing caller_agent_id=None — and it's the second one that turned out to matter most, for reasons below.

Why I'm closing this rather than merging it. #1268 landed a shared _resolve_read_identity used by both /search and /recall, which covers everything this PR does plus one vector it doesn't: /recall parses SearchRequest, so caller_agent_id is also a caller-settable identity field. Guarding only filter_agent_id leaves the same escalation reachable by a different spelling. That's the sole substantive difference — your handling of the two consequences you reported is equivalent to what shipped, and you matched /search's error message exactly.

The shared-resolver shape mattered for a second reason: two routes parsing the same request model had drifted on what its fields mean, and that drift is the bug class. One implementation is what stops it recurring, which is why we went that way rather than adding a parallel check in /recall.

Your test is being merged, in #1276. Our five tests all named an identity explicitly, so the default path — no filter, no caller id, the ordinary recall — went unpinned. That's the case that needs no crafted request, and it's the consequence you flagged second. The behaviour is correct on main; nothing was holding it there. Your test now does, with attribution to you and to this PR.

Two things you got right that we got wrong, and I'd rather say so than quietly fix them:

The patch seam. You patched search_memories on memory_service. I first patched it on the route module — the binding /search uses — not realising /recall re-imports the function inside the handler and so resolves it at call time from the service module. My version silently no-op'd: the real search ran and the assertions read whatever that call left behind. It passed for the wrong reason until a removal probe caught it. #1276 carries a comment explaining the seam so nobody repeats it.

Asserting on arguments rather than results. Your test captures what reaches search_memories. Ours asserted result-set parity, which cannot distinguish "bound to the caller" from "happened to return the same rows" — those coincide whenever the caller has nothing hidden from it. Yours is the stronger assertion for an identity contract.

And your verification method was better than ours. You cherry-picked onto a 2.30.0 image and reproduced against real bge-m3 embeddings end to end. We verified with unit tests against a fake provider.

#994 is closed by #1268. If you hit anything else in the read paths, please do open it — this was a real high-severity finding and you found it from the outside.