Skip to content

fix: namespace defense-in-depth — AppContext setter + architectural guard + message constant#503

Merged
memtomem merged 2 commits intomainfrom
fix/issue-500-namespace-defense-in-depth
Apr 26, 2026
Merged

fix: namespace defense-in-depth — AppContext setter + architectural guard + message constant#503
memtomem merged 2 commits intomainfrom
fix/issue-500-namespace-defense-in-depth

Conversation

@memtomem
Copy link
Copy Markdown
Owner

Follow-up to #501 (merged in cb78142). Originally opened as #502 stacked on the #501 branch; auto-closed by GitHub when #501's branch was deleted on squash-merge — this is the same fresh content rebased onto main.

Summary

Three targeted reinforcements that close the regression class the multi-agent namespace-gate series (#491#494#496#498#499#500/#501) kept hitting one PR at a time — every PR closed one more public surface that had silently shipped without validate_namespace. The series itself is the evidence; this PR makes the next iteration trip a test before review instead of in production.

A. AppContext.current_namespace property setter

Convert the dataclass field to a backing-store + property pair so every write — mem_ns_set, a future tool we forget to gate, a Python adapter, or test code doing app.current_namespace = X — runs through validate_namespace before the value lands in app state.

The bypass this closes is the Python-level direct-mutation path: mem_ns_set is gated, but anyone with an AppContext reference can also do app.current_namespace = "agent-runtime:foo:bar" without going through the tool. mem_session_start(agent_id="default")'s priority chain (step 3 fallback, server/tools/session.py:109-110) reads current_namespace back as the sessions-row namespace, so the same #496 / #500 bypass shape round-trips through this back door.

The forward-shield contract from constants.py:96-100 is preserved: re-validation only happens on caller-supplied writes that reach app state. Internal callers reading the value back never re-validate; internally-derived namespaces (f"{AGENT_NAMESPACE_PREFIX}{agent_id}" after validate_agent_id succeeded, "default", SHARED_NAMESPACE) skip the gate, same as before.

B. Architectural guard test

tests/test_validate_namespace_architectural_guard.py AST-scans server/tools/*.py for any function with a namespace= / target= / old= / new= / old_namespace= parameter and forces every match to be classified:

A new tool with a namespace-shaped param that lands in neither set fails test_no_unclassified_ns_surfaces and forces the author to make the validation decision at PR time — gate it (move to VALIDATED) or explicitly defer (move to DEFERRED with rationale).

E. Constantize "invalid namespace" message prefix

Per #501 review: the test suite's reliance on out.startswith("Error: invalid namespace") made the validator's error formatting an implicit public API. Centralised as INVALID_NAMESPACE_MESSAGE_PREFIX in constants.py, referenced from both the validator's f-strings and the test assertions. No behaviour change.

Out of scope (deliberate)

Test plan

  • New TestAppContextCurrentNamespaceSetter class in test_validate_namespace_ns_tools.py — pins the property setter contract at the Python attribute level (5 tests + parametrised hostile sweep).
  • New test_validate_namespace_architectural_guard.py — 11 tests: 9 parametrised + test_no_unclassified_ns_surfaces + test_validated_and_deferred_are_disjoint.
  • Existing assertions in test_validate_namespace_ns_tools.py switched to _ERROR_PREFIX (sourced from the new constant).
  • uv run ruff check && ruff format --check — clean.
  • uv run mypy packages/memtomem/src/memtomem/server/context.py packages/memtomem/src/memtomem/constants.py — clean.
  • uv run pytest -m "not ollama" — 2887 passed.

🤖 Generated with Claude Code

pandas-studio and others added 2 commits April 26, 2026 18:04
…uard + message constant

Builds on PR #501 (issue #500) with three targeted reinforcements that
close the *regression class* the multi-agent namespace-gate series
(#491#494#496#498#499#500/#501) kept hitting one PR at
a time — a public surface gets added or refactored that takes a
namespace-shaped argument, but the new code path forgets to import /
call ``validate_namespace``. The series itself is the evidence: every
PR closed one more surface that had been silently ungated.

Three changes, cheapest-to-most-invasive:

──────────────────────────────────────────────────────────────────────
A. ``AppContext.current_namespace`` property setter
──────────────────────────────────────────────────────────────────────

Convert the dataclass field to a backing-store + property pair so
every write — ``mem_ns_set``, a future tool we forget to gate, a
Python adapter, or test code doing ``app.current_namespace = X`` —
runs through ``validate_namespace`` before the value lands in app
state. The forward-shield contract on the public input surfaces
(``mem_ns_set``, ``mem_session_start(namespace=)``,
``mem_agent_share(target=)``, …) still holds — this is defense-in-
depth, not a replacement.

The bypass this closes is the Python-level direct-mutation path:
``mem_ns_set`` is gated, but anyone with an ``AppContext`` reference
can also do ``app.current_namespace = "agent-runtime:foo:bar"``
without going through the tool. ``mem_session_start(agent_id=
"default")``'s priority chain (step 3 fallback, ``server/tools/
session.py:109-110``) reads ``current_namespace`` back as the
``sessions``-row namespace, so the same #496 / #500 bypass shape
round-trips through this back door.

The internally-derived contract from
``constants.py:96-100`` is preserved: re-validation only happens on
caller-supplied writes that reach app state. Reads never re-validate;
internally-derived namespaces (``f"{AGENT_NAMESPACE_PREFIX}{
agent_id}"`` after ``validate_agent_id`` succeeded, ``"default"``,
``SHARED_NAMESPACE``) skip the gate, same as before.

──────────────────────────────────────────────────────────────────────
B. Architectural guard test
──────────────────────────────────────────────────────────────────────

``tests/test_validate_namespace_architectural_guard.py`` AST-scans
``server/tools/*.py`` for any function with a
``namespace=``/``target=``/``old=``/``new=``/``old_namespace=``
parameter and forces every match to be classified into one of two
sets:

* ``VALIDATED_NS_SURFACES`` — the 9 ``(file, fn, param)`` triples
  PR #499 + PR #501 gate today. Test asserts each function body
  actually calls ``validate_namespace(<param>)``; a regression that
  drops the call (refactor, accidental deletion, copy-paste from a
  deferred surface) trips the test.
* ``DEFERRED_NS_SURFACES`` — the 22 triples the project has
  *explicitly* left ungated for now (issue #500's "broader UX call
  covered separately if pursued" line). Each entry carries inline
  rationale so future readers can tell deferral from oversight.

A new tool with a namespace-shaped param that lands in *neither* set
fails ``test_no_unclassified_ns_surfaces`` and forces the author to
make the validation decision at PR time — gate it (move to
VALIDATED) or explicitly defer it (move to DEFERRED with rationale).
Mirrors the ``feedback_drift_close_must_derive`` and
``feedback_stub_gap_check`` discipline: every literal that lists "the
gated surfaces" eventually drifts unless a guard forces re-
classification when the code adds new ones.

──────────────────────────────────────────────────────────────────────
E. Constantize ``"invalid namespace"`` message prefix
──────────────────────────────────────────────────────────────────────

Per PR #501 review: the test suite's reliance on ``out.startswith(
"Error: invalid namespace")`` made the validator's error formatting
an *implicit* public API. Centralise it as
``INVALID_NAMESPACE_MESSAGE_PREFIX`` in ``constants.py`` and
reference it from both the validator's f-strings and the test
assertions, so a future tweak (capitalisation, wording, etc.) stays
one change point. No behaviour change — the literal value is
unchanged.

──────────────────────────────────────────────────────────────────────
Out of scope (deliberate, per the original #500 contract)
──────────────────────────────────────────────────────────────────────

* **Storage-layer charset alignment.** Tightening
  ``sqlite_namespace._NS_NAME_RE`` (``[\w\-.:@ ]{1,255}``) to the
  strict gate would change behaviour for legacy data — out of scope
  for the same forward-only reasoning #496 / #499 / #500 used.
* **Final-resolution helper inside ``mem_session_start``.** Same
  pattern as the input gate at the boundary; marginal value over
  what (A) already delivers.
* **Validating ``mem_add`` / ``mem_search`` / ``mem_recall``
  ``namespace=``.** Tracked as deferred via the architectural
  guard's ``DEFERRED_NS_SURFACES``; promoting these is a UX call
  (do we reject ``mem_add(namespace="foo bar")`` loudly, or accept
  the legacy charset?) and belongs in a follow-up.

Tests:

* New ``TestAppContextCurrentNamespaceSetter`` class in
  ``test_validate_namespace_ns_tools.py`` — pins the property setter
  contract at the Python attribute level.
* New ``test_validate_namespace_architectural_guard.py`` — 11 tests:
  9 parametrised ``test_declared_validated_surfaces_call_validate_
  namespace`` + ``test_no_unclassified_ns_surfaces`` +
  ``test_validated_and_deferred_are_disjoint``.
* Existing assertions in ``test_validate_namespace_ns_tools.py``
  switched to ``_ERROR_PREFIX`` (sourced from the new constant).
* Full ``pytest -m "not ollama"`` green (2887 passed, was 2850
  before the +37 new tests).

Co-Authored-By: Claude <[email protected]>
…UTF-8

PR #503 review observations (Minor 1 + 2):

* AST matcher docstring was inaccurate. The original comment claimed
  aliased imports were matched ("Match bare ``validate_namespace(...)``
  and aliased imports..."), but the matcher actually checks the
  callee's ``Name.id`` / ``Attribute.attr`` against the literal string
  ``"validate_namespace"`` — an aliased import (``from ... import
  validate_namespace as vn``) rebinds the Name's id and silently fails
  to match. Reviewer flagged this as a future-contributor trap: a
  rejected aliased call would look like a guard regression rather than
  a deliberate non-match.

  The docstring now states the convention plainly: ``validate_namespace
  (<param>)`` written exactly that way. The three intentionally-unsupported
  forms (kwarg, renamed-local, aliased-import) are listed explicitly so
  a future contributor sees the boundary instead of guessing at it. The
  *supported* forms (conditional gate, ``mod.validate_namespace(...)``
  attribute access) are also enumerated so the matcher's actual reach is
  visible without reading the implementation.

* ``path.read_text()`` defaulted to the locale encoding under py312.
  Python 3.15 makes UTF-8 the default, but the guard runs on py312 CI
  today, so a non-UTF-8 runner could parse the same source file
  differently than a UTF-8 runner. Tool files are ASCII so no live
  divergence, but pinning ``encoding="utf-8"`` removes the
  environment-dependence forward.

No behaviour change: the matcher still matches exactly the same call
sites; the guard test list and pass/fail outcomes are unaffected. This
is purely descriptive accuracy + locale forward-shielding.

Co-Authored-By: Claude <[email protected]>
@memtomem memtomem merged commit 457d2e9 into main Apr 26, 2026
7 checks passed
@memtomem memtomem deleted the fix/issue-500-namespace-defense-in-depth branch April 26, 2026 09:11
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 26, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants