fix(toolbox): a run id names a run, not a path -- all seven writer doors

GREEN. `_checked_run_id` rejects a run id that carries a path -- a separator (`/`, `\`,
`os.sep`, `os.altsep`), a bare `.` or `..`, an absolute prefix, an embedded NUL, or the
empty string -- before any door touches the filesystem. A refusal, exit 3 with the reason
named, and no directory left behind: the call parsed, so 2 would be the wrong code.

Applied as a class. The composition is identical in all seven writers, so fixing the door
the escape was measured on would have left six the same shape.

One check, not two, and that is a measurement. The first draft also required the composed
path to resolve inside the resolved output directory; mutating that check away left all 36
arms green, because after the string rule there is no composition that can leave the
directory. An unreachable check is not defence in depth, it is dead code that reads like
defence, so it is gone -- and the property it claimed is asserted where it IS reachable,
in the probe's accepting arm.

Mutants, each run against the door probes with the tree restored from scratch in between:
guard dropped from one door -> 4 arms fall; whole guard disabled -> all 28 escape arms
fall; `--stop-reason` required -> default="" -> the new stop-reason arm falls, where it
previously survived the entire suite. The one that survived (the containment half) became
the finding above rather than a green tick.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

src/portfolio_optimiser/toolbox.py

@@ -31,6 +31,7 @@ from __future__ import annotations
 
 import argparse
 import json
+import os
 import sys
 from collections.abc import Mapping, Sequence
 from dataclasses import dataclass
@@ -256,6 +257,40 @@ def capture_verdict_command(args: argparse.Namespace) -> Mapping[str, Any]:
     return verdict_to_dict(verdict)
 
 
+def _checked_run_id(run_id: str) -> str:
+    """A run id NAMES a run; it never carries a path — and the doors enforce that here.
+
+    Every writer composes ``<out_dir>/<run_id>-<artefact>.json``. The caller owns the directory
+    (its own rule), but the run id went into that composition untouched, so
+    ``--out-dir <d>/inni --run-id ../../X`` wrote two levels ABOVE the directory the caller named
+    and answered 0. Measured on ``write-prepass`` 20.09; it was never one door's bug — the
+    composition is the same in all seven, so the guard belongs to the class.
+
+    ONE check, and that is a measurement rather than a preference. The first draft had two — this
+    string rule, plus a second that required the composed path to resolve INSIDE the resolved
+    output directory. Mutating the second one away left all 36 arms green: after a run id with no
+    separator, no ``..`` and no absolute prefix, there is no composition that can leave the
+    directory, so the containment check was unreachable and only looked like defence. It is gone,
+    and the property it claimed is asserted where it IS reachable — the accepting arm of the probe
+    checks that what the door wrote resolves inside the directory the caller named.
+
+    A refusal (exit 3), never a usage error: the call parsed, and the answer is no."""
+    separators = {"/", "\\", os.sep} | ({os.altsep} if os.altsep else set())
+    carries_path = (
+        not run_id
+        or run_id in {".", ".."}
+        or "\x00" in run_id
+        or any(sep in run_id for sep in separators)
+        or Path(run_id).is_absolute()
+    )
+    if carries_path:
+        raise ValueError(
+            f"--run-id {run_id!r}: a run id names a run, not a path — no path separator, "
+            "no '..', no absolute path. Name the directory with the output flag instead."
+        )
+    return run_id
+
+
 def _payload_file(path: str, expected: type) -> Any:
     """A JSON argument, read as the shape the core writer already takes.
 
@@ -277,17 +312,18 @@ def write_run_config_command(args: argparse.Namespace) -> Mapping[str, Any]:
     protocol reads. ``resolved_models`` comes in as data because the run path resolves it before
     this step too (``resolve_model`` is held out of the toolbox for exactly that reason — it
     looks up the chat client's deployment)."""
+    run_id = _checked_run_id(args.run_id)
     models = _payload_file(args.resolved_models, dict)
     path = outbox.write_run_config(
         args.out_dir,
-        args.run_id,
+        run_id,
         profile=args.profile,
         resolved_models={str(role): str(model) for role, model in models.items()},
         max_rounds=args.max_rounds,
         max_tokens=args.max_tokens,
         top_k=args.top_k,
     )
-    return {"path": str(path), "run_id": args.run_id}
+    return {"path": str(path), "run_id": run_id}
 
 
 def write_coverage_command(args: argparse.Namespace) -> Mapping[str, Any]:
@@ -297,11 +333,10 @@ def write_coverage_command(args: argparse.Namespace) -> Mapping[str, Any]:
     requires it for a measured reason: "the run finished" and "we never found out" must not be
     the same value. A door that supplied the empty string on the caller's behalf would turn
     every unfinished run into a finished one."""
+    run_id = _checked_run_id(args.run_id)
     rows = _payload_file(args.rows, list)
-    path = outbox.write_coverage(
+    path = outbox.write_coverage(args.outbox_dir, run_id, rows=rows, stop_reason=args.stop_reason)
-        args.outbox_dir, args.run_id, rows=rows, stop_reason=args.stop_reason
+    return {"path": str(path), "run_id": run_id, "rows": len(rows)}
-    )
-    return {"path": str(path), "run_id": args.run_id, "rows": len(rows)}
 
 
 def write_outbox_command(args: argparse.Namespace) -> Mapping[str, Any] | Refused:
@@ -320,6 +355,7 @@ def write_outbox_command(args: argparse.Namespace) -> Mapping[str, Any] | Refuse
     A blocked proposal is ``REFUSED``, and the artefacts are still written — they are where the
     rejection is recorded. The exit code answers the caller's question, not the writer's: an
     agent that only reads rc must never take a blocked proposal for a cleared one."""
+    run_id = _checked_run_id(args.run_id)
     proposal = _proposal_from(args.proposal)
     stamp = ProvenanceStamp.model_validate_json(Path(args.provenance).read_text(encoding="utf-8"))
     baseline = (
@@ -328,7 +364,7 @@ def write_outbox_command(args: argparse.Namespace) -> Mapping[str, Any] | Refuse
     outcome = validate_proposal(proposal, baseline=baseline)
     proposal_path, outcome_path = outbox.write_outbox(
         args.outbox_dir,
-        args.run_id,
+        run_id,
         outcome=outcome,
         provenance=stamp,
         checker_verdict=args.checker_verdict,
@@ -340,7 +376,7 @@ def write_outbox_command(args: argparse.Namespace) -> Mapping[str, Any] | Refuse
         "decision": "validated" if validated else "rejected",
         "proposal_path": str(proposal_path),
         "outcome_path": str(outcome_path),
-        "run_id": args.run_id,
+        "run_id": run_id,
         "verdict_id": args.verdict_id,
     }
     return payload if validated else Refused(payload)
@@ -351,9 +387,10 @@ def write_prepass_command(args: argparse.Namespace) -> Mapping[str, Any]:
 
     Its PRESENCE is what tells "withdrawn by design" apart from the S2c regression, so the door
     writes whenever it is called, exactly as the run path writes whenever a payload was given."""
+    run_id = _checked_run_id(args.run_id)
     declaration = _payload_file(args.declaration, dict)
-    path = outbox.write_prepass(args.outbox_dir, args.run_id, declaration=declaration)
+    path = outbox.write_prepass(args.outbox_dir, run_id, declaration=declaration)
-    return {"path": str(path), "run_id": args.run_id}
+    return {"path": str(path), "run_id": run_id}
 
 
 def write_parse_failures_command(args: argparse.Namespace) -> Mapping[str, Any]:
@@ -361,13 +398,14 @@ def write_parse_failures_command(args: argparse.Namespace) -> Mapping[str, Any]:
 
     Plain mappings in, exactly as the run path flattens ``generate.ParseFailure`` before this
     step: the RAW output layer stays free of the module that imports MAF, and so does this door."""
+    run_id = _checked_run_id(args.run_id)
     failures = _payload_file(args.failures, list)
     path = outbox.write_parse_failures(
         args.outbox_dir,
-        args.run_id,
+        run_id,
         failures=[{str(k): str(v) for k, v in f.items()} for f in failures],
     )
-    return {"path": str(path), "run_id": args.run_id, "failures": len(failures)}
+    return {"path": str(path), "run_id": run_id, "failures": len(failures)}
 
 
 def write_proposal_reviews_command(args: argparse.Namespace) -> Mapping[str, Any]:
@@ -376,9 +414,10 @@ def write_proposal_reviews_command(args: argparse.Namespace) -> Mapping[str, Any
     Already-rendered payload in, for the run path's reason: the renderer lives in the module that
     owns the type. The write rule — iff a reviewer was given, INCLUDING an empty list — is the
     caller's here, because calling this door IS giving one."""
+    run_id = _checked_run_id(args.run_id)
     payload = _payload_file(args.payload, dict)
-    path = outbox.write_proposal_reviews(args.outbox_dir, args.run_id, payload=payload)
+    path = outbox.write_proposal_reviews(args.outbox_dir, run_id, payload=payload)
-    return {"path": str(path), "run_id": args.run_id}
+    return {"path": str(path), "run_id": run_id}
 
 
 def write_debate_tools_command(args: argparse.Namespace) -> Mapping[str, Any]:
@@ -388,14 +427,15 @@ def write_debate_tools_command(args: argparse.Namespace) -> Mapping[str, Any]:
     debate declared nothing" is an honest positive statement, and an empty trace is the S2c
     regression itself — it has to be readable off the artefact, never inferred from a file that
     is not there."""
+    run_id = _checked_run_id(args.run_id)
     tool_calls = _payload_file(args.tool_calls, list)
     requirements = [] if args.requirements is None else _payload_file(args.requirements, list)
     path = outbox.write_debate_tools(
-        args.outbox_dir, args.run_id, tool_calls=tool_calls, requirements=requirements
+        args.outbox_dir, run_id, tool_calls=tool_calls, requirements=requirements
     )
     return {
         "path": str(path),
-        "run_id": args.run_id,
+        "run_id": run_id,
         "tool_calls": len(tool_calls),
         "requirements": len(requirements),
     }