[security] fix(session): scope detail loads to active profile

[Hinotoi-agent]

Summary

This PR hardens the profile boundary for direct session-detail reads.

• Fixes a gap where /api/session?session_id=... loaded a session by id without checking whether the loaded session belongs to the request's active Hermes profile.
• Reuses the existing _profiles_match(...) semantics that already scope /api/sessions, including legacy/default-root alias behavior.
• Adds a regression test proving a foreign-profile session id returns 404 instead of a transcript payload.

Security issues covered

Issue	Impact	Severity
Direct session-detail reads were not profile-scoped	A user/client with a stale, leaked, or guessed session id could fetch another profile's transcript through /api/session even though /api/sessions hides that row by default.	Medium

Before this PR

• /api/sessions filtered rows to the active profile by default.
• /api/session then accepted a raw session_id, called get_session(...), and serialized the loaded session without applying the same active-profile check.
• A direct URL/API call could therefore bypass the sidebar/list scoping and return the other profile's session metadata/messages.

After this PR

• /api/session derives the loaded session's profile, resolves the active request profile, and returns 404 when they do not match.
• The check uses _profiles_match(...) so existing legacy/root-profile compatibility remains intact.
• The new regression test asserts that a foreign-profile session id does not produce a JSON session payload.

Why this matters

Hermes profiles separate config, state, skills, memory, cron, and API-related data. The session list already treated profile scoping as a boundary, but the detail endpoint could bypass that boundary when the caller knew or retained a session id. Returning 404 keeps direct session loads aligned with the list endpoint and avoids exposing another profile's transcript.

Attack flow

client has/guesses/stales a session_id from another profile
    -> calls GET /api/session?session_id=<foreign>&messages=1
        -> route loads the sidecar by id without active-profile authorization
            -> route serializes the session
                -> other profile's transcript/metadata is disclosed

Affected code

Issue	Files
Direct session detail bypasses profile scoping	api/routes.py, tests/test_issue1611_session_profile_filtering.py

Root cause

Issue: direct session-detail reads were not profile-scoped

• The /api/session detail path relied on get_session(sid, ...) and only later merged/redacted the loaded session.
• It did not enforce the active profile boundary that /api/sessions already applies before showing sessions to the client.

CVSS assessment

Issue	CVSS v3.1	Vector
Direct session-detail reads were not profile-scoped	6.5 Medium	CVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:N/A:N

Rationale:

• The attacker is an already-admitted WebUI client/user, but the endpoint can disclose another profile's conversation contents when a foreign session id is known or retained.

Safe reproduction steps

1. Create or mock a session tagged to a non-active profile, e.g. profile="other".
2. Set the request active profile to default.
3. Call GET /api/session?session_id=<foreign>&messages=1&resolve_model=0.
4. Pre-patch: the endpoint returns a JSON session payload for the foreign profile.
5. With this PR: the endpoint returns 404 and no session payload.

Expected vulnerable behavior

• A direct session-detail endpoint should not disclose sessions hidden by /api/sessions profile scoping.
• Pre-patch, the route returned 200 with the foreign-profile session payload.
• The regression test's safe proof signal is that the pre-patch route calls the JSON response path; after the fix it calls the 404 path.

Changes in this PR

• Adds an active-profile check immediately after /api/session loads the requested session.
• Returns 404 for mismatched profiles to avoid confirming cross-profile session existence.
• Adds regression coverage for the direct session-detail route.

Files changed

Category	Files	What changed
API guard	api/routes.py	Scope /api/session detail loads to the active profile before serializing session data.
Regression test	tests/test_issue1611_session_profile_filtering.py	Assert foreign-profile session ids return 404 and no JSON transcript payload.

Maintainer impact

• Narrow API-only behavior change for /api/session.
• No storage migration, frontend change, or CLI behavior change.
• Legacy/default profile aliasing remains supported through the existing _profiles_match(...) helper.

Fix rationale

• The session-detail endpoint is the correct boundary because it is the path that turns a raw session_id into transcript data.
• Reusing _profiles_match(...) avoids duplicating profile semantics and preserves compatibility with renamed root profiles and legacy rows.
• The regression test pins the bypass shape directly: foreign-profile session id plus active-profile mismatch must not serialize a session.

Type of change

• Security fix
• Tests
• Documentation update
• Refactor with no behavior change

Test plan

• New regression test failed before the fix and passes after the fix.
• Profile-filtering test module passes.
• Nearby /api/session payload tests pass.

Executed with:

• python -m pytest tests/test_issue1611_session_profile_filtering.py::test_get_session_rejects_session_from_inactive_profile -q
• python -m pytest tests/test_issue1611_session_profile_filtering.py::test_get_session_rejects_session_from_inactive_profile tests/test_issue1611_session_profile_filtering.py tests/test_session_tail_payload.py tests/test_session_metadata_cli_lookup.py -q

Disclosure notes

• This PR keeps claims bounded to active-profile authorization on the session detail endpoint.
• No production systems were tested.
• No secrets or real session contents are included in the test or PR body.
• No unrelated files were changed.

[greptile-apps]

Greptile Summary

This security fix scopes /api/session detail loads to the active Hermes profile, closing a bypass that allowed a caller with a known or guessed session_id to retrieve another profile's transcript even though /api/sessions already filtered rows by profile.

• _session_visible_to_active_profile is introduced as a thin wrapper around _profiles_match; the guard runs immediately after get_session in the WebUI sidecar path and after _lookup_cli_session_metadata in the CLI fallback path, covering messages=1, messages=0, and cookie-less requests.
• get_active_profile_name is promoted from a deferred per-call import to a module-level alias (_get_active_profile_name), removing the previous fail-open except Exception fallback.
• Four new regression tests cover the full-transcript, metadata-only, cookie-less, and CLI-fallback bypass shapes; two existing tests receive a _get_active_profile_name monkeypatch to remain consistent with the now-enforced profile boundary.

Confidence Score: 4/5

Safe to merge with awareness of the handler is None bypass path; no production callers exist today but the guard is silently skippable for any future internal caller.

The fix correctly closes the disclosed bypass across both code paths (WebUI sidecar and CLI fallback), and the module-level import removes the previous fail-open exception handler. The handler is None short-circuit in _session_visible_to_active_profile is documented and has no current production call sites, but it leaves a latent open path that could quietly re-introduce the bypass if a background or internal caller ever invokes handle_get(None, ...) in the future.

The handler is None branch in _session_visible_to_active_profile (api/routes.py lines 210–211) warrants a second look before merge to confirm the design intent is acceptable long-term.

Important Files Changed

Filename	Overview
api/routes.py	Adds _session_visible_to_active_profile guard in both the WebUI sidecar path (after get_session) and the CLI fallback path (after _lookup_cli_session_metadata). Import of get_active_profile_name moved to module level. The handler is None short-circuit is documented as intentional for unit callers; no production call sites exist.
tests/test_issue1611_session_profile_filtering.py	Adds four new regression tests covering: full transcript, metadata-only, cookie-less, and CLI fallback paths. All patch api.routes._get_active_profile_name (the correct module-level target). Fakery wires bad and j into a captured dict to detect which response path fires.
tests/test_issue2762_state_sync_profile_kwarg.py	Adds a _get_active_profile_name monkeypatch so the new profile guard doesn't 404 a same-profile session in the existing metadata-only test.
tests/test_webui_state_db_reconciliation.py	Same one-line fix as test_issue2762: patches _get_active_profile_name to match the session's profile so the reconciliation test continues to exercise the intended code path.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Route as /api/session handler
    participant ProfileCheck as _session_visible_to_active_profile
    participant GetSession as get_session()
    participant CLIMeta as _lookup_cli_session_metadata()

    Client->>Route: "GET /api/session?session_id=X"

    alt WebUI sidecar path
        Route->>GetSession: get_session(sid)
        GetSession-->>Route: session object
        Route->>ProfileCheck: _session_visible_to_active_profile(session.profile, handler)
        ProfileCheck->>ProfileCheck: _get_active_profile_name()
        ProfileCheck->>ProfileCheck: _profiles_match(session_profile, active_profile)
        alt profiles match
            ProfileCheck-->>Route: True
            Route-->>Client: 200 session payload
        else profiles differ
            ProfileCheck-->>Route: False
            Route-->>Client: 404 Session not found
        end
    else KeyError — CLI fallback path
        GetSession-->>Route: KeyError
        Route->>Route: check SESSION_INDEX_FILE for webui origin
        alt was WebUI session (deleted sidecar)
            Route-->>Client: 404 Session not found
        else genuine CLI session
            Route->>CLIMeta: _lookup_cli_session_metadata(sid)
            CLIMeta-->>Route: cli_meta (may include profile field)
            Route->>ProfileCheck: _session_visible_to_active_profile(cli_meta.profile, handler)
            alt profiles match
                ProfileCheck-->>Route: True
                Route-->>Client: 200 CLI stub payload
            else profiles differ
                ProfileCheck-->>Route: False
                Route-->>Client: 404 Session not found
            end
        end
    end

Loading

_{Reviews (6): Last reviewed commit: "Merge branch 'master' into security/scop..." | Re-trigger Greptile}

[Hinotoi-agent]

Thanks for the review. Addressed in 3c1161f:

• moved active-profile lookup to the existing module-level profiles import path and removed the fail-open default fallback
• applied the same profile check to the CLI-session fallback before returning messages
• updated the sidecar test to patch the route-level helper and added CLI fallback regression coverage

Validation:

python -m pytest tests/test_issue1611_session_profile_filtering.py tests/test_session_tail_payload.py tests/test_session_metadata_cli_lookup.py -q
# 19 passed, 1 warning

[nesquena-hermes]

Summary

I pulled the branch and read both hunks in api/routes.py plus the new test against origin/master. The direction is right — /api/session should honor the same _profiles_match() boundary that /api/sessions already enforces (routes.py:6311), and reusing the canonical helper from api/profiles.py:253 is the correct call. But there are three gating inconsistencies between the two new checks (and against the existing list endpoint) that I'd resolve before merge, because as written they create asymmetric behavior that could either over-block or under-block.

Code reference

Main-path check (routes.py:5807 on the branch):

s = get_session(sid, metadata_only=(not load_messages))
_session_profile = getattr(s, 'profile', None) or None
_active_profile = _get_active_profile_name()
if (
    load_messages
    and isinstance(_session_profile, str)
    and _session_profile.strip()
    and not _profiles_match(_session_profile, _active_profile)
):
    return bad(handler, "Session not found", 404)

CLI-fallback check (routes.py:6112 on the branch):

cli_meta = _lookup_cli_session_metadata(sid)
_session_profile = (cli_meta or {}).get("profile") or None
_active_profile = _get_active_profile_name()
if not _profiles_match(_session_profile, _active_profile):
    return bad(handler, "Session not found", 404)

Three concerns

1. The two checks gate differently. The main path only 404s when load_messages is true and the profile is a non-empty string. The CLI-fallback path 404s unconditionally (no load_messages guard) and treats None via _profiles_match (which maps None->'default'). So a CLI session with no profile is blocked from a named profile even on a metadata-only load, while a native session with no profile is not. Same logical boundary, two different behaviors depending on which code path the id resolves through. I'd extract one helper and call it from both sites.

2. The metadata-only exemption interacts with the "Show from other profiles" flow. loadSession() in static/sessions.js:898 always does a metadata-first fetch (messages=0), then a second messages=1 fetch. Because the main-path check is gated on load_messages, the metadata fetch succeeds cross-profile and only the transcript fetch 404s. That's fine as a security outcome, but note that the sidebar ?all_profiles=1 toggle (sessions.js:4887, "Show N from other profiles") deliberately exposes cross-profile rows to the same caller. After this PR, a user who opts into that view can see the row and load its metadata but gets a 404 on the transcript — a half-open state. Either that's intended (then a short comment saying "all_profiles browse is metadata-only by design" would help), or the detail endpoint should grow an ?all_profiles=1 opt-in mirroring the list endpoint for parity.

3. The isinstance(..., str) guard leaves a gap the list endpoint doesn't have. A session with profile=None (legacy/root rows) is hidden from a named profile's list, because the list does _profiles_match(s.get("profile"), active_profile) and _profiles_match(None, 'named') is False (routes.py:6311 + profiles.py:269, None->'default'). But the main-path check here skips None/empty profiles entirely (isinstance(_session_profile, str) and _session_profile.strip()), so those same rows stay transcript-loadable from a named profile. That's the exact disclosure the PR is closing, just for the None-profile case. If the intent is to match list semantics, drop the isinstance/strip guard and let _profiles_match handle None the way it does in the list path.

Test note

The two added tests (test_get_session_rejects_session_from_inactive_profile, ...rejects_cli_session...) cover the positive block well, but both pin messages=1. Given concern #1/#2, I'd add: (a) a metadata-only (messages=0) cross-profile request asserting current behavior is intentional, and (b) a profile=None row under a named active profile asserting whichever behavior you settle on. That locks the gating decision into the test rather than leaving it implicit.

Verdict

Sound and worth landing once the two checks are unified and the None-profile + metadata-only behaviors are made explicit (either blocked for parity with the list, or documented as intentionally metadata-open). Nothing here is a blocker on the core fix — it's about making the boundary consistent across both code paths and against the existing list scoping.

[Hinotoi-agent]

Thanks — addressed in 7fb0bab. _session_visible_to_active_profile no longer treats default-profile/no-cookie requests as an implicit bypass; real request handlers now always compare the loaded session profile against _get_active_profile_name(), with only handler-less direct unit callers retaining the legacy shortcut. I also added a cookieless regression for /api/session?messages=0 and updated the existing profile-routing tests to set the intended active profile explicitly.

Validation:

• python -m pytest tests/test_issue1611_session_profile_filtering.py tests/test_issue1436_context_indicator_load_path.py tests/test_issue2762_state_sync_profile_kwarg.py::test_api_session_metadata_only_passes_session_profile_to_summary tests/test_webui_state_db_reconciliation.py::test_deferred_session_model_resolution_uses_profile_provider -q → 29 passed
• python -m pytest tests/test_session_tail_payload.py tests/test_session_metadata_cli_lookup.py tests/test_issue1611_session_profile_filtering.py -q → 21 passed