Skip to content

Memory lifecycle hardening: wire-boundary attribution trust and undo semantics - #602

Open
jhrozek wants to merge 3 commits into
mainfrom
acc/memory-lifecycle-hardening
Open

Memory lifecycle hardening: wire-boundary attribution trust and undo semantics#602
jhrozek wants to merge 3 commits into
mainfrom
acc/memory-lifecycle-hardening

Conversation

@jhrozek

@jhrozek jhrozek commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Post-merge hardening follow-up to ADR 0107's operator-profile memory lifecycle,
driven by ADR 0226 and its
acceptance plan. Six scenarios,
four independent threads.

The main thread: attribution could switch off a security scan

tool.MemoryAttribution is documented as inert — "stores must never derive
authorization from context attribution"
— but one line in
ValidateMemoryContentWrite reads Writer/Origin to decide whether the
instruction-injection scan runs on a user/-scoped write. That gate is
deliberate and worth keeping: a human deliberately storing instruction-shaped
text is legitimate; a model autonomously persisting it is the attack shape
automatic learning produces from a poisoned tool result.

The problem was who fills the field in. Over the gRPC memory driver the request
supplied it and the server threaded it in verbatim, so a peer could set
Writer="user" — or send no attribution at all, which the legacy
RememberEntry handler already did — and the scan silently did not run.

Fix: a new MemoryAttribution.ForceInstructionScan bit that feeds exactly
one expression (the scan gate) and nothing else. Persisted Writer/Origin/
Source still report what the caller claimed — overwriting them would corrupt
the durable provenance of every legitimate wire write to fix a scan.

Applied to all three RPCs whose request carries a MemoryEntry the stores will
scan: RememberEntry, RememberVersioned, and RememberIfCurrent. The
third is easy to miss — it reads as a convergence CAS, not a lifecycle write —
but it runs the same ValidateMemoryEntryWrite as its siblings. Asserted as a
table over the write RPCs so a fourth cannot silently escape the gate.

withForcedMemoryWriteAttribution delegates to withProtoAttribution and flips
one bit rather than rebuilding the struct, so the two cannot drift on a future
MemorySource field.

Three smaller threads

  • Undo. UndoLatest filtered its target selection but read its restore
    source
    unfiltered — same intent, different predicate, so a redo was
    reachable. Both walks now share undoCandidate, and the contract (no redo;
    repeated undo walks strictly backward through revisions never yet undone) is
    stated on MemoryLifecycleStore rather than left implicit. The horizon guard
    stays keyed on target == 0 deliberately; the comment explains why keying it
    on the walk result is wrong.
  • Truncation. MemoryRecord.Truncated / wire history_truncated let
    Inspect distinguish a retained tail from complete history, instead of it
    surfacing later as a confusing undo failure. max_revisions sets the flag too.
  • Operator profile. childOperatorProfileSource was a deny-list that failed
    open — an unrecognised role got the operator's durable user facts.
    Inverted to an allow-list. Verified behaviour-preserving across all 19 role
    strings the codebase constructs, including the per-call model-override and
    agent-def roles. The six model-facing lifecycle tools were also missing from
    the caller-ownership classification table; added.

Deliberately not done

Recorded in the plan's Deferred decisions rather than left as gaps:

  • Consolidating the two injection phrase lists. Looks like free hygiene; it
    is not. DirectiveShapedUserMemory is also the operator-profile RENDER
    filter, applied unconditionally every turn, so widening it does not merely
    reject new writes — it silently drops already-stored facts from the prompt,
    with no error and no entry in the profile's own omitted count.
  • Raising Undo*'s permission floor. Remember* is equally floor-Allow, so
    gating Undo alone closes nothing. The property worth having needs write gating
    on tombstone state generally.
  • The render sinks disagree about the directive-shape filter
    (operatorprofile.go applies it, memoryindex.go and usermodel.go do not).
    Pre-existing, out of scope, newly written down — and the reason the write-time
    gate is load-bearing rather than redundant with the render layer.

The wire fix has an honest cost, stated in the ADR: a legitimate remote driver
deployment permanently loses the ability to write human-authored,
instruction-shaped preferences. Present-day reach is nil — nothing in-tree
registers MemoryStoreServiceServer in production.

Verification

task lint clean (0 issues × 7 modules), task api:check ok, docs/lint ok,
go run ./cmd/mecademo ok.

task test exits 201 on 7 failures in cmd/mecatui (6, learning-settings) and
internal/adapter/skills (1). All 7 reproduce identically on a detached
origin/main worktree — they are the macOS /var-symlink $TMPDIR issue, not
this branch. Expected green on Linux CI.

🤖 Generated with Claude Code

jhrozek and others added 3 commits August 17, 2026 20:59
ADR 0226 and its acceptance plan: the smallest set of work that closes the
wire-boundary attribution bypass ADR 0107's memory lifecycle shipped with,
makes UndoLatest's contract explicit, surfaces history truncation, and clears
the correctness/hygiene items a multi-axis review found.

Six scenarios, each independently demoable. Three design questions are
answered by NOT acting on them, with the reasoning recorded so a later pass
does not treat them as oversights: Undo*'s permission floor stays Allow
(Remember* is equally ungated, so gating Undo alone delivers nothing),
RememberVersioned keeps last-write-wins on an empty expected version, and
consolidating the two instruction-injection phrase lists is out of scope
because engine/tool.DirectiveShapedUserMemory is also the operator-profile
RENDER filter — widening it silently drops already-stored facts from the
prompt rather than merely rejecting new writes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Scenarios 1-3 and 5 of the plan.

Wire boundary. MemoryAttribution is documented as carrying no authority, but
ValidateMemoryContentWrite's modelAuthored predicate reads Writer/Origin to
decide whether the instruction scan runs on a user/-scoped write, and the
driver server threaded the wire's claimed Attribution straight into it — so a
remote peer could set Writer="user" (or omit it) and suppress the scan. Every
RPC whose request carries a MemoryEntry now forces the scan classification:
RememberEntry, RememberVersioned, and RememberIfCurrent. The membership rule
is that criterion, not "looks like a lifecycle write" — RememberIfCurrent
reads as a convergence CAS but validates through the same
ValidateMemoryEntryWrite — so it is asserted as a table rather than three
hand-written cases. Classification is a separate bit (ForceInstructionScan),
never an overwrite of the attribution: persisted Writer/Origin/Source must
still report what the caller claimed. The forcing helper delegates to
withProtoAttribution so the two cannot drift on a future MemorySource field.
In-process callers are untouched, including the human-preference exemption.

Undo. MemoryLifecycleStore now states the contract: no redo, repeated undo
walks strictly backward through revisions never yet undone, and the restore
source uses the same filter as target selection. The horizon guard stays
keyed on target == 0 deliberately — when target > 0 the prior state is
history[target-1], which always exists, so no restore candidate there means
"everything older was already undone", a known-empty baseline. memmemory
prunes its undone ledger when retainLatest drops the revisions it refers to.

Truncation. MemoryRecord.Truncated and the wire's history_truncated report a
retained tail, so Inspect can distinguish it from complete history instead of
surfacing only as a later confusing undo failure. max_revisions bounds the
response and sets the same flag, because capping is truncation from the
caller's point of view.

Wire hygiene. Uniform UNIMPLEMENTED mapping across all four lifecycle client
methods, and buf.validate size bounds on every expected_version/version field.
The annotations are documentation plus future enforcement; the protovalidate
runtime is not wired.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Scenarios 4 and 6 of the plan.

childOperatorProfileSource was a deny-list: any role string it did not
recognise fell through to the default branch and received the operator
profile, so a future internal-purpose engine leaks operator facts into a
one-shot internal prompt until someone remembers to add it. Inverted to an
allow-list over the first-class role shapes — main engine (empty role),
Subagent children ("task"/"task:"), team members ("member:"), and Parallel
branches ("parallel"/"parallel:"). Behaviour-preserving for every role in use
today, including the per-call model-override roles for both Subagent and
Parallel; "parallel-judge" and the other judge variants stay excluded, since
a hyphen does not match the "parallel:" prefix. New roles now default to
excluded rather than included.

The six model-facing memory lifecycle tools (Inspect/Forget/Undo in both
project and user scope) were missing from modelToolAccessTable and
ModelToolBoundaries, so the caller-ownership audit did not cover them.
Added, with the shared rationale strings hoisted to constants.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jhrozek
jhrozek force-pushed the acc/memory-lifecycle-hardening branch from a9605e6 to 8348f00 Compare August 17, 2026 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants