fixup(c1a): defense-in-depth validate_name in render_seed_bytes + follow-up notes

pandas-studio · claude · pandas-studio · commit 0aca0d470f44 · 2026-05-01T08:23:37.000+09:00
PR #628 self-review: ``render_seed_bytes`` is in ``__all__`` and constructs ``store.root / asset_type / name / ...`` paths from the ``name`` parameter. ``seed_override`` (the usual caller) already validates, but a direct call with ``name = "../../etc/passwd"`` would otherwise traverse out of the wiki root. PR-D C1a is the first PR where the agents/commands path-construction code is live (PR-D-prep #627 added it under the ``_PR_C_ACTIVE_TYPES`` gate), so this is the right ship to defend at the function boundary rather than relying on caller discipline. - ``render_seed_bytes`` calls ``validate_name`` on entry. Idempotent with ``seed_override``'s validate (same kind string, same name). - New test ``test_render_seed_bytes_rejects_traversal_name`` covers all 3 asset types with traversal-shaped names. - Inline comment near the ``_dropped`` discard sites flags C1b as the follow-up that will surface dropped vendor fields via stderr WARNING. Co-Authored-By: Claude <noreply@anthropic.com>
diff --git a/packages/memtomem/src/memtomem/wiki/override.py b/packages/memtomem/src/memtomem/wiki/override.py
@@ -42,7 +42,14 @@ def render_seed_bytes(
     Skills → canonical ``SKILL.md``.
     Agents / commands → ``parse_canonical_*`` + vendor renderer.
     ``("commands", "codex")`` → :class:`NotImplementedError`.
+
+    ``name`` is validated here even though :func:`seed_override` (the usual
+    caller) already validates — the function is in ``__all__`` so direct
+    callers should not have to remember to pre-validate. Defense in depth
+    for the ``store.root / asset_type / name / ...`` path joins below.
     """
+    validate_name(name, kind=f"{asset_type.removesuffix('s')} name")
+
     if asset_type == "skills":
         src = store.root / "skills" / name / "SKILL.md"
         if not src.is_file():
@@ -55,6 +62,12 @@ def render_seed_bytes(
             f"wiki has no {asset_type}/{name}/{asset_type[:-1]}.md to seed from at {canonical}"
         )
 
+    # ``gen.render()`` returns ``(text, dropped_field_names)`` — fields the
+    # vendor format can't represent (e.g., gemini drops ``tools`` /
+    # ``skills`` / ``model``). C1a silently discards ``_dropped``; C1b CLI
+    # will surface them via stderr WARNING so users editing the override
+    # know which fields the runtime won't see.
+    #
     # Function-body imports dodge a wiki ↔ context import cycle:
     # ``context.install`` already imports ``wiki.store``; widening to
     # module-top imports here would close the loop.
diff --git a/packages/memtomem/tests/test_wiki_override.py b/packages/memtomem/tests/test_wiki_override.py
@@ -13,6 +13,7 @@
 
 import pytest
 
+from memtomem.context._names import InvalidNameError
 from memtomem.wiki.override import render_seed_bytes
 from memtomem.wiki.store import WikiStore
 
@@ -98,3 +99,22 @@ def test_render_seed_bytes_codex_commands_raises_not_implemented(
 
     with pytest.raises(NotImplementedError, match="commands not yet supported"):
         render_seed_bytes(store, "commands", "baz", "codex")
+
+
+def test_render_seed_bytes_rejects_traversal_name(wiki_root: Path) -> None:
+    """Defense-in-depth: ``render_seed_bytes`` validates ``name`` itself.
+
+    ``seed_override`` (the usual caller) already validates, but
+    ``render_seed_bytes`` is in ``__all__`` so direct callers should not
+    have to remember to pre-validate. A traversal-shaped name in the
+    ``store.root / asset_type / name / ...`` path would otherwise escape
+    the wiki root.
+    """
+    store = _initialized_wiki()
+
+    with pytest.raises(InvalidNameError):
+        render_seed_bytes(store, "agents", "../etc/passwd", "claude")
+    with pytest.raises(InvalidNameError):
+        render_seed_bytes(store, "commands", "../../escape", "claude")
+    with pytest.raises(InvalidNameError):
+        render_seed_bytes(store, "skills", "../../escape", "claude")
