Skip to content

[Wave 3] Medium — Security-contract fixes: Other (error-path tests and correctness) #645

Open

Description

@cemililik

Wave 3 · 2 finding(s) in 2 fix package(s). Medium-severity findings that touch the security contract (exit codes, secrets, paths, provenance): do these before the rest of wave 3.

Findings

  • CR-A06-023 (code, Medium) — Hard-failure paths of the eval gates are untested: judge SSRF block, benchmark init/execution failure, --benchmark-only exit 3, generative-guard load failure
    severity Medium · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
    Failure scenario. A change makes a blocked judge endpoint, a crashed lm-eval run or a failed guard load report passed=True, or makes a failed benchmark-only run exit 0. CI cannot tell. This is the 'every non-zero exit path has a test' rule (STD-101) not being met.
    Where. forgelm/judge.py:650-656, forgelm/benchmark.py:177-191, forgelm/cli/_no_train_modes.py:123-125, forgelm/safety/_orchestrator.py:251-263
    Recommended fix. Add run-level tests: judge with a private api_base returns passed=False with the policy reason, lm_eval stubs that raise in HFLM and simple_evaluate, a failing benchmark-only SystemExit(3), and a generative-guard load failure through run_safety_evaluation.

  • CR-A10-027 (code, Medium) — Approve on a cross-device output dir can leave a partial final_model/ with no audit event and blocks the retry
    severity Medium · reachability documented-opt-in · intent contract-violation · confidence confirmed · independently confirmed · reproduced with stubs
    Failure scenario. staging on a different mount from final_model (documented EXDEV case), disk fills mid-copy: exit 2, final_model/ contains a subset of the weight shards, staging intact, audit chain says nothing. A downstream export/deploy/chat that defaults to final_model/ would load a truncated model.
    Where. forgelm/cli/subcommands/_approve.py:227-255 (_atomic_rename_or_move; shutil.move fallback at :254), forgelm/cli/subcommands/_approve.py:573-581 (comment promising the gate 'either succeeds with both promotion and audit, or fails with neither'), forgelm/cli/subcommands/_approve.py:565-570 (os.path.lexists(final_path) guard that blocks the retry)
    Recommended fix. On failure in the move branch, remove the partial destination (and record a failed-promotion audit event), or copy to final_model.partial.<run_id> and rename into place.

Done when

  • Every finding above is fixed or closed with a reason (tick it here).
  • Code fixes carry a test; doc fixes update the Turkish mirror in the same PR.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area: eval-gatesCode area: eval-gatesarea: gdpr-approvalCode area: gdpr-approvalbugSomething isn't workingseverity: mediumReal defect with limited reach or a workaroundsource: review-2026-10Found by the October 2026 code and documentation reviewwave: 3Roadmap wave 3: medium severity, module sweeps

    Type

    No type

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions