bug: Memory commit accepts malformed revisions and search can return text that entries() rejects

[knqiufan]

Describe the bug

RFC 0014 and the Memory protocol name the backend as the final trust boundary: commit() must validate and atomically persist one complete Revision in the same transaction, and search must filter every candidate through the authoritative head manifest. The current implementation does not honor that contract.

This is not a default HTTP/MCP remember, revise, retire, or flush incident. Those paths go through MemoryService, which currently fills predecessor, hash, and projections itself. The gap is the public SPI: MemoryCommit, MemoryWritePlan, and MemoryBackend allow a writer to bypass the service and call backend.commit() directly. Tests and a later writer can already take that seam.

What the contract says, and what the code does

1. The protocol requires MemoryUnitOfWork.commit() to validate and atomically commit one complete Revision. _validate_commit() only checks family, the whole-document content_hash, revision +1, base identity, and equality of the active/projection version-id sets.
2. The RFC sequence diagram requires the backend to validate identity, predecessor, manifest, hash, and vectors. The implementation does not recompute the entry body hash, validate the direct predecessor, or require projections to match the authoritative entry.
3. The RFC schema draws a self-referencing foreign key on previous_version_id. pc_memory_entry_versions.previous_version_id is a plain string column.
4. The RFC requires search to revalidate active membership, exact version, and canonical bytes against the authoritative manifest. The implementation only confirms that the target is still the current head, so the index can act as a second content authority.
5. The RFC describes projections and indexes as rebuildable, non-authoritative copies. Hit text can come from an unrevalidated index/body and no longer line up with the manifest hash.

What has already been reproduced

1. A revision-1 entry with previous_version_id="does-not-exist" commits and reads back.
2. A body replaced with tampered searchable body. while keeping the hash of User prefers black tea. also commits.
3. In that corrupted state, search() returns the tampered body, while entries() raises InvalidMemoryCitationError for the same revision.
4. entries() also omits the version.entry_content_hash comparison that _validate_anchor() already performs.

Closing only the write path or only the read path leaves the loop open. Both sides should be fixed in one PR.

What this gap can cause later

Official product paths almost never construct a bad envelope today, so normal use usually does not show the damage. If the seam is taken, or if the service or a second writer later emits a bad commit, the likely consequences are:

1. Illegal history becomes durable authority. A hash is then a label written with the envelope, not a commitment.
2. A fake predecessor has almost no current read consumer, but later lineage, offline validation, or citation code that walks previous_version_id will not be able to rebuild history reliably.
3. After a body / declared / manifest hash mismatch, remember, revise, forget, and organize fail with hash-mismatch. The Memory can still be recalled but can no longer be governed.
4. If a projection or searchable_text diverges from the canonical row, search and automatic POST /v1/context/prepare injection can hand a different body to the model.
5. Context Pack and Handoff can consume text the authoritative manifest never committed.
6. A new adapter, migration script, or homemade MemoryWritePlan will copy the current thin checks as the contract, and the gap will spread.
7. Fixing only one side still lets dirty data in, or still lets search return it after it is in.

Steps to reproduce

1. Use a PowerContext checkout with its dependencies installed.
2. Save the following as repro_memory_integrity.py.
3. Run:

python repro_memory_integrity.py

#!/usr/bin/env python3
"""Reproduce Memory commit/search integrity gap."""

from __future__ import annotations

import asyncio
import json
from tempfile import TemporaryDirectory

from powercontext.builtin.artifacts.memory import (
    Memory,
    MemoryCommit,
    MemoryContent,
    MemoryEntryVersion,
    MemoryManifest,
    MemoryManifestEntry,
    MemoryProjection,
    MemoryService,
)
from powercontext.builtin.artifacts.memory.canonical import entry_content_hash, memory_content_hash
from powercontext.builtin.artifacts.memory.errors import InvalidMemoryCitationError
from powercontext.builtin.artifacts.search import analyze_text
from powercontext.builtin.persistence.memory import RelationalMemoryBackend
from powercontext.builtin.persistence.sqlite import SQLiteConfig
from powercontext.builtin.runtime import BuiltinConfig, open_builtin_contexts

ORIGINAL = "User prefers black tea."
TAMPERED = "tampered searchable body."


def _version(*, text: str, content_hash: str, previous_version_id: str | None) -> MemoryEntryVersion:
    return MemoryEntryVersion(
        memory_artifact_id="memory",
        entry_id="pref-tea",
        entry_version_id="pref-tea-v1",
        version=1,
        previous_version_id=previous_version_id,
        kind="fact",
        text=text,
        entry_content_hash=content_hash,
        created_in_revision=1,
    )


def _commit(version: MemoryEntryVersion) -> MemoryCommit:
    content = MemoryContent(
        manifest=MemoryManifest(
            entries=(
                MemoryManifestEntry(
                    entry_id=version.entry_id,
                    entry_version_id=version.entry_version_id,
                    entry_content_hash=version.entry_content_hash,
                    state="active",
                ),
            )
        )
    )
    memory = Memory(artifact_id="memory", revision=1, content=content)
    return MemoryCommit(
        base=None,
        memory=memory,
        content_hash=memory_content_hash(content),
        entry_versions=(version,),
        projections=(
            MemoryProjection(
                entry_version=version,
                searchable_text=analyze_text(version.text),
            ),
        ),
    )


def _backend(contexts: object, scope_id: str) -> RelationalMemoryBackend:
    return RelationalMemoryBackend(
        database=contexts.database,
        scope_id=scope_id,
        artifacts=contexts.repositories.artifacts,
        index=contexts.index,
    )


async def main() -> None:
    original_hash = entry_content_hash(
        kind="fact",
        text=ORIGINAL,
        source_refs=(),
        artifact_refs=(),
    )
    with TemporaryDirectory() as directory:
        config = BuiltinConfig(
            database=SQLiteConfig(url=f"sqlite+aiosqlite:///{directory}/repro.db")
        )
        async with open_builtin_contexts(config) as contexts:
            fake_backend = _backend(contexts, "fake-predecessor")
            fake_version = _version(
                text=ORIGINAL,
                content_hash=original_hash,
                previous_version_id="does-not-exist",
            )
            async with fake_backend.begin() as unit_of_work:
                committed = await unit_of_work.commit(_commit(fake_version))
            stored = (await fake_backend.entries(committed.as_ref()))[0]

            mismatch_backend = _backend(contexts, "hash-mismatch")
            mismatch_version = _version(
                text=TAMPERED,
                content_hash=original_hash,
                previous_version_id=None,
            )
            async with mismatch_backend.begin() as unit_of_work:
                mismatch_memory = await unit_of_work.commit(_commit(mismatch_version))
            service = MemoryService(backend=mismatch_backend)
            search = await service.search(
                "tampered searchable",
                memories=(mismatch_memory,),
                mode="fts",
            )
            entries_error = None
            try:
                await service.entries(mismatch_memory)
            except InvalidMemoryCitationError as error:
                entries_error = type(error).__name__

            observed = {
                "entries_error": entries_error,
                "fake_predecessor": stored.previous_version_id,
                "search_hits": [hit.text for hit in search.hits],
            }
            print(json.dumps(observed, sort_keys=True))
            expected = {
                "entries_error": "InvalidMemoryCitationError",
                "fake_predecessor": "does-not-exist",
                "search_hits": [TAMPERED],
            }
            if observed != expected:
                raise SystemExit(1)
            print("0")


if __name__ == "__main__":
    asyncio.run(main())

4. Observe that the process prints the JSON below and exits 0. That means it observed the known buggy state.

Do not use HTTP remember/flush as the minimal reproduction. That path currently fills the envelope correctly and does not prove the contract gap.

Expected behavior

The backend is the final trust boundary. commit() must reject an illegal Revision in the same transaction, including:

1. Nonexistent, cross-entry, cross-Memory, or skipped predecessors, and a first version with a non-null predecessor.
2. Body / declared-hash / manifest-hash mismatches.
3. Duplicate, missing, or extra entry versions, and projections that do not match the active manifest entry or canonical searchable_text.
4. A failed validation must roll back the whole transaction.

Search must treat the index as a rebuildable projection, not a second authority:

1. Before returning, each hit must be revalidated in the same read transaction against the exact revision, manifest, and entry: active membership, identity, version, and canonical bytes.
2. For a given corrupted state, search(), entries(), and expand() must agree: they either succeed together and return the same authoritative body, or fail together with an integrity error.
3. They must never return the suspect body.

The RFC-legal degradation “authoritative rows present, vector absent” must remain accepted. Existing stale-head concurrency semantics should stay unchanged.

Actual behavior

The reproducer consistently prints:

{"entries_error": "InvalidMemoryCitationError", "fake_predecessor": "does-not-exist", "search_hits": ["tampered searchable body."]}
0

That matches the reproduced items above:

1. The fake predecessor was stored as an authoritative row.
2. The body/hash mismatch was accepted.
3. search() returned the tampered body while entries() rejected the same revision.

The final 0 and process exit code 0 mean the reproducer observed the known buggy state.

Environment

• PowerContext version: current mainline
• Commit: 7bddf6abde482b6081b9984d1ca0dbae183e3946
• Storage backend: local SQLite
• External services: none

Related but not a duplicate: #1249 fixed a concurrent stale-head FTS miss and did not add authoritative candidate validation. PR #1293 added seekDB and did not change these validators.

Are you willing to submit a PR to fix this bug?

• Yes, I would like to submit a PR.