Skip to content

Guard private memory artifacts from delivery - #37003

Open
kelvmg wants to merge 1 commit into
NousResearch:mainfrom
kelvmg:fix/privacy-memory-artifact-guards
Open

Guard private memory artifacts from delivery#37003
kelvmg wants to merge 1 commit into
NousResearch:mainfrom
kelvmg:fix/privacy-memory-artifact-guards

Conversation

@kelvmg

@kelvmg kelvmg commented Jun 1, 2026

Copy link
Copy Markdown

Summary

  • keep context compaction handoff summaries out of assistant-role transcript turns
  • filter context compaction handoffs from session_search discovery/scroll output
  • deny media delivery of profile MEMORY.md, USER.md, and memories/ artifacts
  • add regression coverage for compression, session_search hygiene, and gateway media path validation

Test Plan

  • python3 -m pytest tests/gateway/test_platform_base.py tests/tools/test_session_search.py tests/run_agent/test_compression_boundary.py -q
  • python3 -m ruff check agent/context_compressor.py gateway/platforms/base.py tools/session_search_tool.py tests/gateway/test_platform_base.py tests/tools/test_session_search.py tests/run_agent/test_compression_boundary.py

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery tool/memory Memory tool and memory providers labels Jun 2, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the privacy-focused coverage. The memory-delivery and session-search directions still address gaps on current main, but the compaction hunk needs rework before salvage.

Problems

  • agent/context_compressor.py PR line 1998 forces every standalone handoff to role="user". Current main deliberately selects against both neighbors and merges on double collisions (agent/context_compressor.py:3127-3144). In the reverse case—user head, assistant tail—the PR creates adjacent user turns; current regression coverage requires an assistant summary there (tests/agent/test_context_compressor.py:1942-1973, 2188-2228).
  • Session-search filtering does not cover direct reads (tools/session_search_tool.py:227-250) or title-match discovery (tools/session_search_tool.py:462-489), both of which can return stored handoffs.

Suggested changes

  • Preserve current main's role-selection, forced-leading-user, and merge logic; apply transcript hygiene at retrieval/rendering boundaries instead.
  • Apply handoff filtering across every session_search shape and add direct-read/title-match coverage.
  • Test the production memory locations under memories/MEMORY.md and memories/USER.md (tools/memory_tool.py:55-57).

This is an automated hermes-sweeper review.

# serialize them as assistant-role turns: assistant messages are the
# same channel that gateways persist/deliver as user-visible output, so
# a compaction handoff there can leak into transcripts or messaging
# adapters. Prefer a user-role handoff with explicit reference-only

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Forcing user here loses the protected-head check. If the head ends in user and the tail begins with assistant, this inserts adjacent user turns. Current main handles that reverse double collision by merging into the tail; please retain the two-neighbor role selection rather than making every summary a user turn.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit area/memory Memory subsystem: store, providers, sync, background reviews labels Jul 13, 2026

@GottZ GottZ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This was generated by AI during triage.

Summary

Three PRs are associated with this complex, but only #37003 addresses the reported privacy exposure by changing compaction handling, session-search rendering, and media-delivery denylisting. #46938 and #46940 make the same test-comment cleanup and do not address the privacy cause.

Related pull requests

  • #37003 related — (+143/-23) — keep open for rework: The media denylist directly protects profile MEMORY.md, USER.md, and memories/ artifacts, and the session-search changes hide some compaction handoffs. As the contributor keep_open review notes, however, the compaction role change conflicts with current alternation/merge behavior, while direct reads and title-match discovery remain unfiltered, so the fix is not yet complete.
  • #46938 [closed] related — (+1/-1) — superseded duplicate: This closed PR only removes stale “THE BUG” wording from a compression-boundary test and does not change behavior or address the privacy cause; it remains relevant as the earlier version superseded by refreshed #46940.
  • #46940 related — (+1/-1) — mergeable cleanup: This accurately removes the same stale test-docstring wording from its current path, consistent with the contributor keep_open review, but it does not change runtime behavior or address #37003’s privacy gaps.

Duplicates

#46938 and #46940 are substantively identical one-line test-comment cleanups; #46940 is the refreshed current-path version, while #46938 is already closed.

Suggested consolidation

Merge #46940 as the current, accurate cleanup and retain #46938 as its closed superseded duplicate. Keep #37003 open but require rework before merge: preserve current compaction role-selection and merge invariants, move transcript hygiene to retrieval/rendering boundaries, and cover direct-read and title-match session_search paths; this follows rather than overrides the contributor keep_open review on #37003.

Cross-PR triage: Reviewed 3 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 14 kB of PR diffs, 1 kB of issue/PR text, 2 kB of discussion (3 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/memory Memory subsystem: store, providers, sync, background reviews comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state tool/memory Memory tool and memory providers type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants