Summary
runs.run_dir_for builds the run directory as project / RUNS_DIR / run_id (runs.py:85-86) and runs.session_name builds the multiplexer session as f"bmad-loop-{run_id}" (runs.py:68-69), both straight from the caller-supplied id. --run-id is an argparse.SUPPRESS-ed flag on both run and sweep (cli.py:1600, :1625) — hidden, documented as "pre-assigned id (used by the TUI)", but still reachable from a shell.
Nothing validates it. --run-id ../../../tmp/x resolves the run dir outside the project; a dot or colon in the id produces a tmux session name tmux itself mangles or rejects; a Windows-illegal character (:, ?, *) makes the run dir uncreatable on the windows leg.
Scope note
Explicitly carved out of #102 / PR #103. That PR added platform_util.safe_ref_segment and applied it to unit_branch_name, so a dirty run_id no longer breaks git branch creation. The directory and session-name positions are untouched — a dirty id still reaches both raw. #103 hardened the ref position only because that is where the reported failure was.
Fix direction
Not simply safe_segment(run_id) at each use site: the run id is a lookup key, unlike the story key. bmad-loop resume <run_id> / status / kill recompose the same path from the same id, so a sanitizer is coherent (identity for clean ids, and every auto-generated id from new_run_id() is clean) — but only if every site that maps an id to a path or session name applies it, and runs.discover-style reverse mappings (name[len(_SESSION_PREFIX):] at runs.py:255, which recovers a run_id from a session name) would then recover the sanitized id, not the raw one. That asymmetry is the actual design question.
Validate-and-reject at the CLI boundary is probably the better shape for a hidden, machine-supplied flag: --run-id has exactly one legitimate producer (the TUI, which calls new_run_id()), so rejecting anything that is not already a clean segment costs nothing and keeps the id↔path↔session mapping bijective. platform_util.is_absolute_path + has_parent_ref already exist for the traversal half of the check.
Low severity — the flag is hidden and its only in-tree caller generates clean ids. Filed so the "id reaches a path unvalidated" class is tracked rather than assumed handled by #102.
Refs: #74, #98, #102, #103
Summary
runs.run_dir_forbuilds the run directory asproject / RUNS_DIR / run_id(runs.py:85-86) andruns.session_namebuilds the multiplexer session asf"bmad-loop-{run_id}"(runs.py:68-69), both straight from the caller-supplied id.--run-idis anargparse.SUPPRESS-ed flag on bothrunandsweep(cli.py:1600, :1625) — hidden, documented as "pre-assigned id (used by the TUI)", but still reachable from a shell.Nothing validates it.
--run-id ../../../tmp/xresolves the run dir outside the project; a dot or colon in the id produces a tmux session name tmux itself mangles or rejects; a Windows-illegal character (:,?,*) makes the run dir uncreatable on the windows leg.Scope note
Explicitly carved out of #102 / PR #103. That PR added
platform_util.safe_ref_segmentand applied it tounit_branch_name, so a dirtyrun_idno longer breaks git branch creation. The directory and session-name positions are untouched — a dirty id still reaches both raw. #103 hardened the ref position only because that is where the reported failure was.Fix direction
Not simply
safe_segment(run_id)at each use site: the run id is a lookup key, unlike the story key.bmad-loop resume <run_id>/status/killrecompose the same path from the same id, so a sanitizer is coherent (identity for clean ids, and every auto-generated id fromnew_run_id()is clean) — but only if every site that maps an id to a path or session name applies it, andruns.discover-style reverse mappings (name[len(_SESSION_PREFIX):]at runs.py:255, which recovers a run_id from a session name) would then recover the sanitized id, not the raw one. That asymmetry is the actual design question.Validate-and-reject at the CLI boundary is probably the better shape for a hidden, machine-supplied flag:
--run-idhas exactly one legitimate producer (the TUI, which callsnew_run_id()), so rejecting anything that is not already a clean segment costs nothing and keeps the id↔path↔session mapping bijective.platform_util.is_absolute_path+has_parent_refalready exist for the traversal half of the check.Low severity — the flag is hidden and its only in-tree caller generates clean ids. Filed so the "id reaches a path unvalidated" class is tracked rather than assumed handled by #102.
Refs: #74, #98, #102, #103