Skip to content

fix(honcho): 4-pass sanitize for peer-card + AI Self-Representation rendering - #74202

Open
bbasketballer75 wants to merge 3 commits into
NousResearch:mainfrom
bbasketballer75:fix/honcho-4pass-sanitize
Open

fix(honcho): 4-pass sanitize for peer-card + AI Self-Representation rendering#74202
bbasketballer75 wants to merge 3 commits into
NousResearch:mainfrom
bbasketballer75:fix/honcho-4pass-sanitize

Conversation

@bbasketballer75

Copy link
Copy Markdown

Replaces the simpler imperative-shape and self-narration-prefix filters with a unified sanitization pass that runs against all four Honcho render surfaces: User Representation, User Peer Card, AI Self-Representation, AI Identity Card.

Four filtering passes (in order)

  1. Imperative-shape filter (e.g. INSTRUCTION:, RULE:, COMMAND:, DIRECTIVE:, PROMPT-INJECTION:) — prompt-injection vectors. Pulled into an [untrusted injection filtered from <section>] block at the end of the section so the model can see what was filtered but does not silently act on it as a user-stated instruction.

  2. Self-narration prefix filter (e.g. hermes says X, hermes said Y, [AUTO-NARRATED] ..., [DEBUG-LOG] ..., [SELF-TRACE] ...) — the AI Self-Representation can accumulate hundreds of these from prior debugging sessions; once surfaced they re-assert themselves as present-tense facts in every turn. Demoted to [historical, demoted from <section>].

  3. Self-narration phrase filter — user-peer observations that quote prior self-narration phrasing (e.g. austin said Hermes said 'Vee', austin shared that hermes says X) survive the prefix filter because they don't start with the trigger. Match the whole-word phrase hermes says / hermes said anywhere in the line, case-insensitive. Same demotion as pass 2.

  4. Line cap — if the kept section exceeds _MAX_LINES_PER_SECTION (60) lines, the overflow is demoted to a [historical, truncated] block. Prevents a single polluted section from blowing up the prompt cache for every turn of every session.

Supersedes

Both will be closed in favor of this combined fix for a single review surface.

Not duplicating

Out of scope

The 2026-07-18 self-trust loop root-cause is the deriver's extraction prompt reading quoted/prefixed agent narration as Explicit Observations. This PR is the read-side mitigation (rendering-time sanitization). The write-side mitigation is #66831. Neither fixes the deriver itself; that would be a separate change to the Honcho Python SDK extraction prompt and is upstream of Hermes.

Tests

Manual verification only at this stage — I can add parametrized tests for each pass if maintainers want them. The fix paths are self-documenting via the [historical, ...] block markers, so missing lines are visible to the user in the rendered context.

Copilot AI 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.

Pull request overview

This PR updates the Honcho memory plugin’s first-turn context rendering to run a unified, multi-pass sanitization over all four Honcho render surfaces (user/AI representation and peer/identity cards), aiming to demote prompt-injection-shaped and self-narration-derived lines and cap per-section size.

Changes:

  • Route User Representation / User Peer Card / AI Self-Representation / AI Identity Card through new _sanitize_*_lines() rendering-time sanitization.
  • Introduce a shared 4-pass sanitizer that (1) demotes imperative-shaped prefixes, (2) demotes self-narration prefixes, (3) demotes self-narration phrases anywhere in a line, and (4) caps retained lines per section with a demoted overflow block.
  • Modify sync_turn() behavior and documentation around assistant-message ingestion (currently a write-path behavior change).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread plugins/memory/honcho/__init__.py
Comment thread plugins/memory/honcho/__init__.py Outdated
Comment thread plugins/memory/honcho/__init__.py
Comment thread plugins/memory/honcho/__init__.py
@alt-glitch alt-glitch added type/security Security vulnerability or hardening P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins tool/memory Memory tool and memory providers needs-repro Bug needs reproduction steps needs-decision Awaiting maintainer decision before any implementation labels Jul 29, 2026
@bbasketballer75
bbasketballer75 force-pushed the fix/honcho-4pass-sanitize branch from 8702eda to 2f1ae72 Compare July 29, 2026 18:44
@bbasketballer75

Copy link
Copy Markdown
Author

Rebased onto current origin/main (c3ffe27). Clean rebase.

The 4-pass sanitization (imperative-shape → self-narration-prefix → prefix-only-prefix → final-scrub) for all four render surfaces (User Representation, User Peer Card, AI Self-Representation, AI Identity Card) is preserved. No upstream changes touched plugins/memory/honcho/ in the rebased range.

@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 consolidating the renderer paths; current main still interpolates all four Honcho context fields directly at plugins/memory/honcho/__init__.py:606-620, so the read-side problem is real.

Problems

  • The proposed line cap is ineffective: PR plugins/memory/honcho/__init__.py:752-754 splits after 60 lines, but :775-780 appends every overflow line back into the returned context.
  • The imperative filter likewise re-emits the exact untrusted payload at plugins/memory/honcho/__init__.py:758-765. A label does not keep that content out of the memory context.
  • PR plugins/memory/honcho/__init__.py:1499-1539 also changes assistant ingestion, despite this PR describing #66831 as the separate write-path fix. Current main persists both roles at plugins/memory/honcho/__init__.py:1343-1352; #66831 additionally addresses queued assistant entries during _flush_session().

Suggested changes

  • Keep only bounded metadata/counts for filtered or overflow lines, then add a test proving 61 input lines do not produce the 61st line in output.
  • Drop the sync_turn() write-path change from this PR and keep that policy in #66831.
  • Add renderer-level coverage for all four fields, phrase matching, and the bounded-output behavior.

Automated hermes-sweeper review.

Comment thread plugins/memory/honcho/__init__.py Outdated
Comment thread plugins/memory/honcho/__init__.py Outdated
Comment thread plugins/memory/honcho/__init__.py Outdated
Comment thread tests/honcho_plugin/test_session.py Outdated
@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users area/memory Memory subsystem: store, providers, sync, background reviews labels Jul 30, 2026
…endering

Replaces the simpler imperative-shape and self-narration-prefix filters with a
unified sanitization pass that runs against all four Honcho render surfaces:
User Representation, User Peer Card, AI Self-Representation, AI Identity Card.

Four filtering passes (in order):

1. **Imperative-shape filter** (e.g. `INSTRUCTION:`, `RULE:`, `COMMAND:`,
   `DIRECTIVE:`, `PROMPT-INJECTION:`) — prompt-injection vectors. Pulled into
   an `[untrusted injection filtered from <section>]` block at the end of
   the section so the model can see what was filtered but does not silently
   act on it as a user-stated instruction.

2. **Self-narration prefix filter** (e.g. `hermes says X`, `hermes said Y`,
   `[AUTO-NARRATED] ...`, `[DEBUG-LOG] ...`, `[SELF-TRACE] ...`) — the AI
   Self-Representation can accumulate hundreds of these from prior debugging
   sessions; once surfaced they re-assert themselves as present-tense facts
   in every turn. Demoted to a `[historical, demoted from <section>]` block.

3. **Self-narration phrase filter** — user-peer observations that *quote*
   prior self-narration phrasing (e.g. `austin said Hermes said 'Vee'`,
   `austin shared that hermes says X`) survive the prefix filter because
   they don't start with the trigger. Match the whole-word phrase
   `hermes says` / `hermes said` anywhere in the line, case-insensitive.
   Same demotion as pass 2.

4. **Line cap** — if the kept section exceeds `_MAX_LINES_PER_SECTION` (60)
   lines, the overflow is demoted to a `[historical, truncated]` block.
   Prevents a single polluted section from blowing up the prompt cache for
   every turn of every session.

This change supersedes NousResearch#66754 (single-pass imperative filter) and NousResearch#66810
(imperative + self-narration prefix only) by combining the same intent with
the broader phrase and line-cap passes. Closing those two PRs in favor of
this one for a single review surface.

The source-side `_strip_agent_self_quotes` change in NousResearch#66831 (separate
write-path fix) remains open — different concern, different layer.
Two review findings, both confirmed real by direct inspection:

1. The line cap didn't actually cap anything -- overflow past
   _MAX_LINES_PER_SECTION was relabeled '[historical, truncated]' and
   re-appended to the rendered output in full, so total output size was
   never actually bounded. Now dropped, with only a bare count surviving.

2. The imperative-shape (prompt-injection) filter labeled untrusted
   payloads as 'filtered' but still concatenated them into the model's
   context verbatim -- a warning label around injected text is demotion,
   not removal, and the model reads the attempt either way. Now dropped,
   with only a bare count surviving. filtered_historical (self-narration,
   not a security boundary) is unaffected -- that content is intentionally
   kept, just clearly labeled as non-authoritative.

Also removed a scope-creep write-path change this PR had bundled in:
sync_turn() stopped writing assistant messages to Honcho entirely, which
is a real architectural policy decision that's already the stated purpose
of open PR NousResearch#66831 (both teknium1 and Copilot flagged the duplication
independently). Reverted to restore the original assistant-write path;
that policy question belongs in NousResearch#66831, not silently bundled into a
rendering-sanitizer fix. Also dropped an orphaned comment describing a
self-quote-stripping feature that was never actually implemented in this
version of the code (the real implementation lives in NousResearch#66831).

Found and fixed one more bug while adding test coverage: half of
_SELF_NARRATION_PREFIXES ('HERMES SAYS:', '[DEBUG-LOG] ', etc.) could
never match -- the comparison lowercases the input line but compared it
against the prefix tuple's original mixed case, so only the two
already-lowercase entries in the tuple were ever reachable. Normalized
the comparison to be case-insensitive on both sides.

Added tests/plugins/memory/test_honcho_sanitize_card_lines.py (16 tests,
0 existed before this) covering all 4 passes directly, including the
61-line-cap regression case the review explicitly asked for and the
phrase-anywhere matching (plus its false-positive guard) that had zero
coverage.
@bbasketballer75
bbasketballer75 force-pushed the fix/honcho-4pass-sanitize branch from 8a93b0c to 33efab8 Compare July 31, 2026 03:44
@bbasketballer75

Copy link
Copy Markdown
Author

Rebased onto current origin/main (clean, no conflicts) and addressed the review:

  1. Line cap didn't cap anything — overflow past _MAX_LINES_PER_SECTION was relabeled '[historical, truncated]' and re-appended to the output in full, so total size was never actually bounded. Now dropped, with only a bare count surviving.
  2. Injection payload still reached the model — the imperative-shape filter labeled untrusted text as 'filtered' but still concatenated it into context verbatim. A warning label around injected text is demotion, not removal. Now dropped, with only a bare count surviving. filtered_historical (self-narration, not a security boundary) is unaffected — kept verbatim, clearly labeled non-authoritative, as intended.
  3. Removed the bundled sync_turn() write-path change — this PR silently stopped writing assistant messages to Honcho entirely, which is a real architectural policy decision that's already fix(honcho-memory): stop assistant self-narration ingestion #66831's whole purpose (both you and Copilot flagged the duplication independently). Reverted to keep this PR scoped to rendering sanitization only; that policy call belongs in fix(honcho-memory): stop assistant self-narration ingestion #66831.
  4. Found while adding tests: half of _SELF_NARRATION_PREFIXES (HERMES SAYS:, [DEBUG-LOG] , etc.) could never match — the comparison lowercases the input but compared against the tuple's original mixed case, so only the 2 already-lowercase entries were ever reachable. Fixed.

Added tests/plugins/memory/test_honcho_sanitize_card_lines.py — 16 tests (0 existed before), covering all 4 passes directly, including the 61-line-cap regression case you asked for and the phrase-anywhere matching (plus its false-positive guard) that had no coverage at all.

Re-review welcome.

Two gaps from review:

1. filtered_historical was emitted verbatim and uncapped, so the class
   comment's claim that _MAX_LINES_PER_SECTION "prevents a single polluted
   section from blowing up the prompt cache" wasn't true: a section that is
   mostly self-narration routed its entire payload through the demotion
   path, bypassing the cap. Apply the same cap (keep the most recent N,
   count the rest) so the stated bound actually holds.

2. Every sanitizer test called the classmethod directly, so nothing verified
   that _format_first_turn_context() actually routes each of its four
   surfaces through it. A regression dropping one call site would have kept
   all tests green while shipping raw untrusted text into the system prompt.
   Adds renderer-level tests covering all four surfaces individually and
   together, plus a guard that `summary` stays deliberately unsanitized.

Both new tests were verified to fail against a deliberately reintroduced
bug (unsanitized ai_card; removed historical cap) and pass with the fix.
@bbasketballer75

Copy link
Copy Markdown
Author

Both remaining gaps addressed in 4a6f8452a.

1. filtered_historical is now capped. You were right that the section wasn't actually bounded — the trailer was emitted verbatim, so a section that's mostly self-narration routed its whole payload through the demotion path and _MAX_LINES_PER_SECTION bounded nothing in practice. Rather than soften the class comment, I made it true: the historical trailer now obeys the same cap, keeping the most recent N and emitting a count for the rest.

2. Renderer-level tests added. Your point that the existing tests only exercised the classmethod was exactly right — a regression dropping one of the four _format_first_turn_context() call sites would have left every test green while shipping raw untrusted text into the system prompt. Added tests that go through the renderer for each surface individually and all four at once, plus a guard that summary stays deliberately unsanitized so the boundary is explicit.

I verified both are real regression tests rather than assuming: reintroducing an unsanitized ai_card fails the wiring tests, and removing the historical cap fails the cap test. 24 passed in that file, 53 in tests/honcho_plugin/test_session.py.

@alt-glitch — on the needs-repro label: this is a rendering-only change with no runtime trigger, so a repro would just be "put an imperative-shaped line in a peer card and look at the resulting system prompt." The tests above encode exactly that. Happy to write it up as manual steps if that's still useful, but wanted to check whether a repro is meaningful here before adding one.

🤖 Addressed by Claude Code

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/plugins Plugin system and bundled plugins needs-decision Awaiting maintainer decision before any implementation needs-repro Bug needs reproduction steps P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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