Approve derives final_path from the raw staging_path and never bounds it, so a forged staging_path renames a directory outside output_dir (CR-A10-007) #384

Wave 1 · theme D1-3: Revert, approval and GDPR erasure safety · highest severity High · 1 finding(s) · effort small

Summary

Shared parent /runs. runB is pending approval with /runs/runB/final_model.staging.RB/. A writer of /runs/runA appends {"event":"human_approval.required","run_id":"RA","staging_path":"/runs/runA/../runB/final_model.staging.RB/../../runA/payload"} and creates /runs/runA/payload/. The approver runs forgelm approve RA --output-dir /runs/runA, which exits 0. /runs/runA/payload is renamed to /runs/runB/final_model. runB's log shows no decision, yet its canonical final_model/ now holds attacker content.

Primary finding

CR-A10-007 — Approve derives final_path from the raw staging_path and never bounds it, so a forged staging_path renames a directory outside output_dir

severity High · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end

What happens. _run_approve_cmd checks only that realpath(staging_path) lies inside output_dir (_approve.py:545-550), then computes final_path = staging_path[:staging_path.rfind('.staging')] on the raw string (:555-556) and os.rename's into it (:589) with no normalisation or boundary check. A '..'-laden path through an existing .staging dir, or an output_dir whose own path contains '.staging', makes final_path land outside output_dir (e.g. another run's final_model/).

Where.

• forgelm/cli/subcommands/_approve.py:589
• forgelm/cli/subcommands/_approve.py:541-556
• forgelm/cli/subcommands/_approve.py:562-575
• forgelm/cli/subcommands/_approve.py:556
• forgelm/cli/subcommands/_approve.py:570
• forgelm/cli/subcommands/_approve.py:99-116 (_staging_path_inside_output_dir checks the source only)
• forgelm/cli/subcommands/_audit_log_reader.py:195-224 (find_latest_event_for_run, no chain/HMAC verification)
• docs/reference/approve_subcommand.md:36
• forgelm/trainer.py:1611
• forgelm/cli/subcommands/_approve.py:570-589

Evidence.

• _approve.py:555-556 _idx = staging_path.rfind(_STAGING_SUFFIX); final_path = staging_path[:_idx] if _idx != -1 else staging_path runs on the raw string after the realpath-only check at :545. Approve JSON output in the repro: "final_model_path": ".../runs/runA/../runB/final_model", "promote_strategy": "rename".
• approve_check.py case D: output_dir=/D/proj.staging_area/out with forged staging_path=/model_dir. Approve exits 0 with final_model_path=/D/proj, model_dir is gone from output_dir, and /D now contains 'proj'.

Recommended fix. Normalise first: staging_path = os.path.realpath(staging_path). Derive final_path from the normalised value, or better, rebuild the expected staging and final paths from output_dir, final_model_dir and run_id, and require staging_path to equal the expected staging path. Also check that realpath(final_path) is inside output_dir. Consider requiring verify_audit_log (chain + HMAC when the secret is set) to pass before promoting.

Acceptance criteria

• A regression test reproduces the failure scenario and asserts the corrected behaviour.
• Documentation listed above is corrected in English and in its Turkish mirror in the same PR.
• Every finding in this issue is resolved or explicitly closed with a reason.

_{Found by the October 2026 ForgeLM code and documentation review against commit cab2947. Code links point at that commit; line numbers may have moved since. Finding IDs (CR-…, DOC-…, TECH-…) are stable references for this backlog.}