fix(tools): fail closed when a bare session id resolves into another profile

[liuhao1024]

What does this PR do?

session_search's read shape (session_id alone) fails open across profiles: when the id misses in the caller's own state.db, _read_with_profile_fallback unconditionally scans every profile's state.db via _locate_session_db and returns the first hit's full transcript. No opt-in, no authorization check — an agent (or user) in profile A can read profile B's entire conversation given only a session id, and session ids are not secrets (logs, tool output, the databases themselves). The same hole fires even for an explicit profile= read that misses, silently serving a third profile's transcript instead of an error.

This PR keeps the cross-profile locate as a discovery pointer but fails closed on the read: on a miss, the tool now reports which profile owns the id and asks the caller to name it (re-run with profile='<owner>'). The transcript is only served when the caller explicitly passes that profile. This preserves the original safety net's recovery path (a model that dropped the profile prefix from an @session:<profile>/<id> link gets the owner named in the error and succeeds on the next call), while closing the leak.

Related Issue

Fixes #106761

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• 🔒 Security fix

Changes Made

• tools/session_search_tool.py — _read_with_profile_fallback no longer auto-reads the located profile's db on a miss; it closes it and returns a fail-closed error naming the owning profile. Explicit profile= reads, link-prefix autosplit, and same-profile reads are untouched.
• tests/tools/test_session_search.py — test_bare_id_locates_across_profiles now asserts the fail-closed contract: a bare id that lives in another profile returns success: false with profile='asdf' in the error and no messages; the named re-run (profile='asdf') still reads the session (authorized path, unchanged).

How to Test

1. Two profiles with separate homes (default + asdf), each with its own state.db; asdf owns session s_far.
2. From default, call session_search(session_id="s_far").
   • Observed result before: {"success": true, "profile": "asdf", "messages": [...full transcript of asdf...]}.
   • Observed result after: {"success": false, "error": "session_id 's_far' lives in profile 'asdf'. Reading another profile's session requires naming the profile: re-run with profile='asdf'."} — no messages key.
3. Re-run with session_search(session_id="s_far", profile="asdf"): {"success": true, "mode": "read", ...} — the authorized explicit-profile path is unchanged.
4. pytest tests/tools/test_session_search.py -q → 53 passed; pytest tests/agent/test_anthropic_mcp_prefix_strip.py tests/run_agent/test_token_persistence_non_cli.py -q → 18 passed; ruff check clean.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: macOS 15.4 (arm64)

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — or N/A (docstring updated in place)
• I've updated cli-config.yaml.example if I added/changed config keys — N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — N/A (no platform-specific code touched)
• I've updated tool descriptions/schemas if I changed tool behavior — or N/A (the read shape's contract for explicit ids is unchanged; only the miss path tightens, and the error message itself teaches the recovery step)

Screenshots / Logs

N/A — see How to Test for the before/after tool output.

[alt-glitch]

This was generated by AI during triage.

Related: #106768 and #106772 are competing fixes for the same #106761 hole filed within the same hour. #106768 removes the cross-profile scan outright; #106772 gates it behind a new config key; this PR keeps the locate as a pointer and fails closed naming the owner (still an ownership oracle for a bare id). Flagging the cluster so a maintainer can pick one mechanism.

[liuhao1024]

Thanks for flagging the cluster — agreed the mechanism choice is the maintainer's call, and #87779 / #61947 are useful context (the config opt-out there is a partial mitigation, so a default-safe fix still seems worth landing).

For the record on this PR's trade-off: relative to the current fail-open behavior (a bare id silently returns another profile's full transcript), failing closed while naming the owner is a strict tightening. The residual oracle only discloses that an already-possessed high-entropy session id exists and which profile owns it — the attacker must hold the id itself to learn anything. Keeping the locate as a pointer rather than removing the scan outright (as in #106768) also preserves the recovery path when an agent drops the profile prefix mid-task: the next hop can re-issue with the explicit profile instead of dead-ending.

Happy to rework or withdraw if the maintainer prefers one of the other mechanisms.

[egilewski]

suggesting changes

The patch prevents a bare ID from returning a foreign transcript, but its fail-closed error still reveals the owning profile. Remove that owner oracle before merge; bounded tests and named-profile controls pass.

• [P2] Bare-ID fallback still discloses another profile's session ownership (tools/session_search_tool.py:397)
  On a miss in the caller's database, _read_with_profile_fallback still scans every profile and formats the discovered owner into the error. A caller that supplies a valid ID from another isolated profile therefore learns that profile's name and that the ID exists even though transcript content is withheld. The patch closes content disclosure but not this cross-profile metadata oracle; use a generic failure or an authenticated explicit-link path.
  Remediation: Return a generic profile-required/not-found error, or gate owner disclosure behind an authenticated explicit-profile link.

Security evidence:

• trust boundary: session_search accepts model- or user-controlled session_id and optional profile in one active profile context. Profile state databases are intended to be isolated; an explicit profile link is the opt-in cross-profile path.
• source/sink/invariant: A bare ID must not cause transcript or cross-profile ownership metadata to cross the active profile boundary; only a named, validated profile may select another database. The fallback scans all profiles and sends the owner in an error.
• current-main reproduction: Against an archived current-main tree, a bare ID present only in another profile returned success with that session's message. The targeted session-search test file still passed 53 tests, so the old leak was not caught by the existing suite.
• PR-head or patch-replay validation: In the bound patch replay, the same bare-ID probe returned success false with no messages, while a named profile still returned its transcript. The bare error nevertheless included the owning profile name and a profile-specific re-run instruction.
• positive/negative cases: Positive: current main exposes a foreign transcript; the patch blocks the transcript but exposes its owner. Negative: an unknown ID returns a generic not-found error, same-profile reads succeed, and an explicitly named valid profile still reads its own session.
• residual bypass search: The bounded source review covered read, browse, discovery, scroll, embedded-link, and named-profile branches. No additional content path was introduced by this patch; the residual disclosure is the fallback owner string, while explicit profile reads remain intentional sharing.
• reviewer validation: Ran the 53-test session-search file on the archived current-main tree and on the patch replay, plus a direct two-profile probe. Both test runs passed; the probe observed the transcript leak on current main and the owner oracle on the patch replay.

Not checked:

• full repository suite
• live gateway authorization
• remote profile boundary
• CodeRabbit review

Signed: GPT-5.6-luna-max in Codex

[liuhao1024]

Pushed e2a12a70af taking the first remediation path: the cross-profile locate scan is removed outright, so a bare id now returns the same generic not-found error whether the id is unknown or owned by another profile — indistinguishable from the caller's side. Naming the profile (or an embedded @session:<profile>/<id> link) remains the only authorized cross-profile read path and is still covered by tests. The regression test now also asserts the error text does not disclose the owning profile name. Thanks for the thorough review — this is strictly better than the previous iteration.

[kvnloo]

Prior-art / overlap on tools/session_search_tool.py bare session_id profile isolation (live heads):

• fix(tools): fail closed when a bare session id resolves into another profile #106773 e2a12a7 (@liuhao1024): fail-closed when a bare session id resolves into another profile
• fix(session_search): do not scan other profiles for bare session_id #106768 5e7a5a2 (@KoNit-K): do not scan other profiles for bare session_id

Ask maintainers: which approach is canonical? The other should close as prior-art/duplicate rather than land both. No competing PR from me.

[liuhao1024]

Thanks for the overlap summary — happy for maintainers to pick the canonical one. For the record on the current head of this PR (e2a12a70af):

• egilewski's P2 from the earlier review (owner oracle) is addressed: a bare-id miss now returns a generic not-found error, indistinguishable between an unknown id and one owned by another profile.
• The head is current against main and merges cleanly (no rebase needed).
• The authorized read path is kept and asserted as a positive control: an explicit profile= argument still serves the transcript.

No objection to either landing; deferring to maintainer preference.

[alt-glitch]

This was generated by AI during triage.

Duplicate of #106768: after the review-driven push (e2a12a70) this PR removes _locate_session_db and fails closed with a generic miss, which is the same mechanism #106768 landed first. Marking this one duplicate so reviewers converge on a single PR; if a maintainer prefers this branch (e.g. for its test coverage), the label can be swapped.

[liuhao1024]

Acknowledged. As stated when the cluster was first flagged, I'm happy either way — maintainers pick the canonical branch and the other closes.

One factual note on the duplicate framing: the mechanism convergence is the direct outcome of the P2 owner-oracle review on this branch (drop the locate → generic miss on a bare-id miss), not a re-implementation of #106768's approach. On the current state of both heads: this branch has its review feedback fully addressed (e2a12a70) and merges cleanly against main, while #106768 still has an outstanding branch-refresh request before its merge evidence can be established. Whichever is picked as canonical, the tests from this branch should carry over easily.

[alt-glitch]

This was generated by AI during triage.

Re-triaged: relabeled from duplicate to related. #106768 and this PR are open competing fixes for #106761 that converged on the same mechanism via independent review; the maintainer picks the canonical branch. Cross-linked: #106768, #106772 (closed), #106761, #87779.

[liuhao1024]

Closing in favor of #108074 (merged; fixes #106761). Its _read_scoped lands exactly the mechanism this branch had converged on at e2a12a70 — _locate_session_db dropped, bare-id miss fails closed — plus a caller-facing not-found hint, so this PR is fully superseded. As noted earlier, whichever branch wasn't picked closes. Leaving the fork branch intact in case any test coverage here is worth salvaging.