Repository navigation
Description
Wave 3 · 15 finding(s) in 12 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-A02-070 (code, Medium) — Library ForgeTrainer silently ignores a config's pipeline: block: trains only the root sections, skips every per-stage gate, state file and manifest, and returns a success TrainResult
severity Medium · reachabilitydefault· intentcontract-violation· confidence confirmed · independently confirmed · reproduced end to end
Failure scenario. An Airflow operator follows library_api.md and loads a production SFT->DPO pipeline YAML whose sft stage requires human approval. ForgeTrainer.train() trains one SFT run from the root defaults into the root output_dir and promotes it to final_model with no approval staging. The DPO stage never runs. It returns TrainResult(success=True) with no warning, and its audit log and model card read as a completed pipeline.
Where.forgelm/trainer.py:339-357,forgelm/trainer.py:1661-1705,forgelm/config.py:1643-1649,forgelm/config.py:1985-1989
Recommended fix. Fail loudly. ForgeTrainer.init (or train()) should raise ConfigError when config.pipeline is not None, pointing to the CLI or to a public orchestrator, or at least log a WARNING and write an audit event. Alternatively expose a public pipeline entry point. State in library_api(.md/-tr), the reference and pipeline.md that pipelines are CLI-only. -
CR-A03-013 (code, Medium) — Rows with a documented 'text' prompt and 'output' response silently lose every response: the text branch wins and the output column is dropped
severity Medium · reachabilitydefault· intentcontract-violation· confidence confirmed · independently confirmed · reproduced end to end
Where.forgelm/data.py:483-488,forgelm/data.py:340-351,forgelm/data.py:669-683,docs/reference/data_preparation.md:26-28
Recommended fix. Take the text branch only when no response column (Assistant/output/response) is present. Otherwise treat 'text' as the prompt, or raise. Make the docs and the dispatch agree. -
CR-A03-020 (code, Medium) — fit-check silently substitutes 7B-class architecture defaults when the config cannot be read or nests text_config
severity Medium · reachabilitydefault· intentcontract-violation· confidence confirmed · independently confirmed · reproduced end to end
Failure scenario. Operator fit-checks a gated 70B or an MoE/VLM model whose AutoConfig fails (no auth, offline cache miss); the verdict is computed from a 7B profile and reads FITS/OOM as if measured.
Where.forgelm/fit_check.py:108-133(name-hint / 4096,32,11008,32000,32 defaults),forgelm/fit_check.py:361-377(only note emitted is the dtype caveat, and only for non-quantized configs),forgelm/cli/_dispatch.py:277,forgelm/fit_check.py:92-103
Recommended fix. Record in result.notes and breakdown which fields were fallbacks (e.g. 'architecture_source': 'autoconfig'|'size_hint'|'default'), log at WARNING, and set verdict to UNKNOWN or flag low confidence when the AutoConfig probe failed. Call the HF auth setup before the probe when model.offline is false. -
CR-A03-036 (code, Medium) — A directory dataset bypasses the extension whitelist: .txt/.tsv/.zip/.xml/.hdf5/... are loaded, and files in a minority format are silently dropped
severity Medium · reachabilitydefault· intentcontract-violation· confidence confirmed · independently confirmed · reproduced end to end
Failure scenario. An operator follows the 'directory of JSONL files' contract and putschat.jsonlnext toextra.parquetandextra2.parquetin data/. Training runs only on the parquet rows and logs nothing about the JSONL. Another operator points at a folder of raw .txt notes, which the single-file path refuses with a clear message. Here the run silently trains SFT on individual lines.
Where.forgelm/data.py:90-104,forgelm/data.py:312-322,forgelm/data.py:337,forgelm/config.py:807-809
Recommended fix. For a directory, list the files yourself (non-hidden, whitelisted extensions, a deliberate recursion choice). Reject the directory when it holds files with other extensions, or holds more than one builder family. Callload_dataset(builder, data_files=...)with an explicit mapping. Make the extension check case-insensitive for directories too, matching the file branch. -
CR-A03-037 (code, Medium) — Filename split inference silently diverts or drops corpus files and can leave no train split
severity Medium · reachabilitydefault· intentcontract-violation· confidence confirmed · independently confirmed · reproduced end to end
Failure scenario. A corpus directory holdstrain.jsonlandtrain_v2_additions.jsonl; both are kept, because 'train' matches. A colleague addsnew_examples.jsonl: it is silently ignored. A file namedtest_prompts.jsonlin an otherwise unsplit corpus makes the whole directory load as validation only. The run crashes after the model download with KeyError 'train'.
Where.forgelm/data.py:337,forgelm/data.py:563,forgelm/data.py:584-615,forgelm/data.py:712
Recommended fix. Resolve splits explicitly: train.jsonl / validation.jsonl / test.jsonl, plus documented aliases shared with the audit. Fail loudly on any whitelisted file the mapping does not use, and raise an actionable error before the model load when no train split exists. Document the directory layout in configuration.md and its TR mirror. -
CR-A03-038 (code, Medium) — extra_datasets with directory sources: the primary's test split is discarded once extras are set, extras' non-train splits are ignored, and an extra with no train split crashes with a bare KeyError
severity Medium · reachabilitydefault· intentcontract-violation· confidence confirmed · independently confirmed · reproduced end to end
Failure scenario. An operator keeps a curated held-out test.jsonl in the primary directory and later adds a second corpus through extra_datasets. Their curated eval set disappears, and early stopping, baseline loss and eval metrics now run on a random 10% slice that includes rows from the extra corpus. Nothing in the logs or the manifest says the eval set changed.
Where.forgelm/data.py:512-546,forgelm/data.py:533-536,forgelm/data.py:556-563,forgelm/data.py:698-710
Recommended fix. Run _ensure_validation_split, or at least the test-to-validation promotion, on the primary before merging. Give a clear error when an extra has no train split, and log when an extra's non-train splits are ignored. -
CR-A03-044 (code, Medium) — Unmatched target_modules entries are silently dropped. On Phi-3 (a wizard-curated model) the documented Q/K/V/O list trains only o_proj, while the model card and compliance report record Q/K/V/O
severity Medium · reachabilitydefault· intentintended-but-harmful· confidence confirmed · independently confirmed · reproduced (plumbing only)
Failure scenario. An operator picks Phi-3-mini in the wizard plus the 'extended' or 'full' preset. Training succeeds with a fraction of the intended adapter capacity: no LoRA on Q/K/V, or none on gate/up. The shipped model card and Annex IV technical documentation claim Q/K/V/O (or all linear layers).
Where.forgelm/model.py:535,forgelm/model.py:609,forgelm/wizard/_state.py:141-145,forgelm/wizard/_orchestrator.py:363-382
Recommended fix. After get_peft_model, compute the set of requested names that matched no module. Warn, or with a strict flag fail with ConfigError, and record the applied module names (from model.named_modules()) in the manifest and model card. Offer architecture-aware presets (qkv_proj / gate_up_proj) or the PEFT 'all-linear' string in the wizard. -
CR-A07-034 (code, Medium) — A missing audit genesis manifest is a silent downgrade: a write failure is one WARNING and never retried, and verify-audit passes with only a DEBUG note although the docs promise a warning
severity Medium · reachabilitydefault· intentcontract-violation· confidence confirmed · independently confirmed · reproduced end to end · already tracked: KNOWN-OPEN TRUST-01 — docs/roadmap/phase-16-trust-surface-hardening.md:575-576,883,989 (S9 open; unit id inferred from exit ordering)
Where.forgelm/compliance.py:509-514,forgelm/compliance.py:607-608,forgelm/compliance.py:2449-2462,forgelm/compliance.py:505-514(confirmed)
Recommended fix. Re-attempt the manifest write on later events while it is absent and a first entry exists (recompute the hash from line 1), or raise. At minimum emit a catalogued audit event or persistent marker so 'manifest never written' is distinguishable from 'never tried'. -
↳ same root cause: DOC-D1B04-023 (docs, Medium) — docs/reference/audit_event_catalog.md: misleading — The genesis manifest sidecar defends against whole-log truncation back to 'genesis', and is always on (written once on the first event).
severity Medium
Where.docs/reference/audit_event_catalog.md:141(Turkish mirror:docs/reference/audit_event_catalog-tr.md:141) · type: misleading
The document says. The genesis manifest sidecar defends against whole-log truncation back to 'genesis', and is always on (written once on the first event).
What the code does. verify-audit treats a missing sidecar as a pass: it logs at DEBUG and returns no failure. The sidecar is unsigned JSON, and writing it is best-effort (a failed write is only a warning). A log rewritten from genesis with its sidecar deleted verifies with exit 0.
Code references.forgelm/compliance.py:2457-2462,forgelm/compliance.py:509-510
Proposed correction. Doc: 'The sidecar detects truncation only while it exists. verify-audit does not fail when it is missing, and it is not signed; protect the log and the sidecar together.' Code: in keyed mode, require the sidecar and authenticate it.
Turkish mirror. the Turkish page has the same error; fix both in the same PR -
↳ same root cause: DOC-D1B04-041 (docs, Medium) — docs/reference/verify_audit.md: misleading — The genesis sidecar is auto-detected and cross-checked when present; corrupting it must not be a quieter way to disarm the truncation guard than deleting…
severity Medium
Where.docs/reference/verify_audit.md:16,126 (Turkish mirror:docs/reference/verify_audit-tr.md:16) · type: misleading
The document says. The genesis sidecar is auto-detected and cross-checked when present; corrupting it must not be a quieter way to disarm the truncation guard than deleting it.
What the code does. For a non-empty log, a deleted sidecar is silently accepted (DEBUG log only). Deleting it is therefore the quiet way to disarm the guard: rewriting the log from genesis and removing the sidecar verifies with exit 0.
Code references.forgelm/compliance.py:2457-2462
Proposed correction. 'A missing sidecar is not a failure for a non-empty log; protect the sidecar with the same storage controls as the log.'
Turkish mirror. the Turkish page has no such statement -
CR-A10-029 (code, Medium) — Purge --row-id silently skips unparseable corpus lines, so an erasure reports success while malformed lines holding the identifier survive
severity Medium · reachabilitydefault· intentintended-but-harmful· confidence confirmed · independently confirmed · reproduced end to end · already tracked: KNOWN-OPEN TRUST-13 — docs/roadmap/phase-16-trust-surface-hardening.md:573-574 (S9 open; unit id inferred)
Failure scenario. The operator relays a DPO erasure request, purge reports success, and the data subject's personal data is still in the training corpus inside a malformed line.
Where.forgelm/cli/subcommands/_purge.py:272-295(_find_matching_rows: continue on JSONDecodeError at 286-288 and on non-dict at 289-290),forgelm/cli/subcommands/_purge.py:298-368(_atomic_rewrite_dropping_lines copies every non-dropped line verbatim, 318-322),forgelm/cli/subcommands/_purge.py:270-293(confirmed),forgelm/cli/subcommands/_purge.py:471
Recommended fix. Count unparseable lines and emit them in the envelope warnings and the erasure audit event. Optionally add a raw-substring check for the id on unparseable lines, or refuse (exit 1) unless --allow-unparseable is given. -
CR-A13-010 (code, Medium) — Export's model_integrity.json update is a non-atomic in-place rewrite whose failures are logged only at DEBUG
severity Medium · reachabilitydefault· intentintended-but-harmful· confidence confirmed · independently confirmed · not reproduced (code trace)
Failure scenario. Provenance record silently incomplete.
Where.forgelm/export.py:232-239(confirmed),forgelm/export.py:207-239,forgelm/export.py:493-494(confirmed),forgelm/trainer.py:2157-2163
Recommended fix. Log at WARNING and surface in the result/JSON envelope. -
CR-A14-003 (code, Medium) — --wizard-start-from: _select_strategy falls back to QLoRA when the loaded (lora.method, load_in_4bit) pair has no exact menu match, so a bare Enter flips full-precision configs to 4-bit
severity Medium · reachabilitydefault· intentcontract-violation· confidence confirmed · independently confirmed · reproduced end to end · already tracked: KNOWN-OPEN CLI-09 — docs/roadmap/phase-16-trust-surface-hardening.md:502-503,512-513 (S7 open; unit id inferred)
Failure scenario. Operator iterating on a bf16 LoRA config taps Enter through the wizard and overwrites the file; the next run asks for 4-bit quantization (bitsandbytes) or, on non-CUDA hosts, runs unquantized while the compliance artefacts record load_in_4bit true.
Where.forgelm/wizard/_collectors.py:1097-1136,forgelm/wizard/_orchestrator.py:335-358,406,forgelm/wizard/_collectors.py:1112-1134,forgelm/wizard/_orchestrator.py:408-425
Recommended fix. When existing_method is None treat it as 'lora' (schema default) and match on load_in_4bit alone; add a test for a config without lora.method. -
↳ same root cause: DOC-D2B05-031 (docs, Low) — docs/guides/quickstart.md: false claim — Choice prompts (Strategy, ...) detect the loaded value and shift their default index so Enter still preserves it
severity Low
Where.docs/guides/quickstart.md:80· type: false claim
The document says. Choice prompts (Strategy, ...) detect the loaded value and shift their default index so Enter still preserves it
What the code does. Strategy default shifts only when lora.method is present in the YAML; with no lora.method the default is QLoRA and Enter flips load_in_4bit false -> true.
Code references.forgelm/wizard/_collectors.py:1118-1127
Proposed correction. Fix the code (F3); no doc change needed afterwards.
Turkish mirror. the Turkish page has the same error; fix both in the same PR -
CR-A14-020 (code, Medium) — Zero-chunk wizard ingest returns None but leaves (and may have overwritten a previous good file with) an empty JSONL at out_path, silently
severity Medium · reachabilitydefault· intentintended-but-harmful· confidence confirmed · independently confirmed · reproduced end to end
Failure scenario. Operator re-runs the wizard on the same directory with default output path after the corpus became unreadable; the earlier JSONL (possibly hand-reviewed or already audited) is replaced by an empty file and the wizard just re-prompts.
Where.forgelm/wizard/_byod.py:91-94,forgelm/wizard/_byod.py:129-134,forgelm/ingestion.py:2515,forgelm/ingestion.py:2623-2630
Recommended fix. Make ingest_path not promote when chunk_count is 0 (preferred, fixes M-A12-004 for every caller), or have the wizard delete/skip the empty file and confirm before overwriting an existing output.
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.
Metadata
Metadata
Assignees
Labels
Type
Projects
- StatusShow more project fieldsTodo
Activity