Repository navigation
fix(supply-chain): ignore 10 no-fix CVEs that broke nightly 2026-05-21 - #59
Merged
Merged
Conversation
The 2026-05-21 nightly (run 26210539455) failed on `pip-audit` because the OSV / GHSA databases published 9 new torch advisories and 1 markdown advisory overnight, none of which have an upstream fix. Without an ignore, `tools/check_pip_audit.py` fails closed on UNKNOWN severity (pip-audit's JSON does not serialise OSV severity) and the nightly stays red, masking real future breakage. Triage (issue #58): - torch PYSEC-2025-191..197, PYSEC-2025-210, PYSEC-2026-139: all require a LOCAL attacker passing malformed inputs to specific torch APIs (`jit.script`, `lstm_cell`, `cuda.memory.*`, `pt2` loader, etc.). ForgeLM is a local-CLI tool; an attacker with that access is already inside the trust boundary. None of the affected APIs are called with attacker-controlled arguments in `forgelm/`. - markdown PYSEC-2026-89: OSV affected-range misclassification — the advisory description states the fix shipped in markdown==3.8.1 but the range record has no `fixed` event, so every version is flagged. Installed 3.10.2 is post-fix. Each ignore is documented inline in `.github/workflows/nightly.yml` with the surface, the threat-model carve-out, and the condition for re-evaluating (per `docs/reference/supply_chain_security.md`). Also drops the stale "Issue #37 tracks the active set" reference — #37 was a closed nightly-failure issue, not a tracker. Refs: #58 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
Reviewer's GuideExtends the nightly supply-chain audit step to explicitly ignore 10 newly published, no-fix CVEs (9 in torch, 1 in markdown), with detailed inline justification and reevaluation guidance, and cleans up an outdated tracking comment in the nightly workflow. Flow diagram for updated nightly pip-audit ignore handlingflowchart TD
NightlyJob[Nightly workflow: supply_chain job]
PipAudit[pip-audit --ignore-vuln
CVE-2026-1839
PYSEC-2025-191..197
PYSEC-2025-210
PYSEC-2026-139
PYSEC-2026-89]
JsonOut[/pip-audit JSON output
/tmp/pip-audit.json/]
CheckScript[tools/check_pip_audit.py
applies severity policy]
Policy[Fail on any non-ignored
UNKNOWN-severity finding]
NightlyJob --> PipAudit
PipAudit --> JsonOut
JsonOut --> CheckScript
CheckScript --> Policy
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider moving the growing
--ignore-vulnlist into a separate config (e.g., a checked-in JSON/YAML consumed bytools/check_pip_audit.py) so new CVE suppressions don’t require editing the workflow and can be managed/validated in one place.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider moving the growing `--ignore-vuln` list into a separate config (e.g., a checked-in JSON/YAML consumed by `tools/check_pip_audit.py`) so new CVE suppressions don’t require editing the workflow and can be managed/validated in one place.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 37 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Address PR #59 review comment: extract the growing --ignore-vuln list from .github/workflows/nightly.yml into tools/pip_audit_ignores.yaml, consumed by tools/check_pip_audit.py via a new opt-in --ignores flag. New CVE suppressions no longer require editing the workflow. Design — opt-in, not opt-out ============================ docs/reference/supply_chain_security.md explicitly tells deployers that ForgeLM does NOT ship a default project-level ignore list. The new flag preserves that contract: without --ignores PATH, check_pip_audit.py applies no suppressions, so a deployer running pip install forgelm[security] python3 tools/check_pip_audit.py /tmp/pip-audit.json still sees the full unfiltered severity gate. The project's own nightly opts in explicitly: python3 tools/check_pip_audit.py /tmp/pip-audit.json \ --ignores tools/pip_audit_ignores.yaml Schema enforcement ================== Every entry in pip_audit_ignores.yaml must carry six required fields (id, package, reason, threat_model, verified_at, reevaluate_after); missing any one fails the gate with an ::error:: that names the gap. This blocks the "stick a bare id: in and forget" pattern — every suppression now carries a written justification + re-evaluate trigger as required by docs/reference/supply_chain_security.md. Matching uses {id} ∪ aliases on both sides, so an ignore listing the CVE alias still matches a pip-audit finding emitted under its PYSEC primary id (and vice versa). Each match is logged as a ::notice:: annotation so the run summary surfaces the audit trail; suppressions don't disappear into the workflow log silently. Migrations ========== All 11 ignores in nightly.yml (CVE-2026-1839 transformers + 9 torch PYSEC-2025-191..197/210 + PYSEC-2026-139 + 1 markdown PYSEC-2026-89) moved verbatim into the YAML file with the justifications expanded into structured fields. The workflow's pip-audit step shrinks from ~95 lines of inline comments + per-CVE --ignore-vuln args to a ~12-line block pointing at the YAML. Tests ===== Extends tests/test_check_pip_audit.py with 9 new cases covering: - suppression by primary id and by alias - no false-positive match on unrelated CVEs - schema validation (each required field individually named on failure) - missing / invalid YAML files fail closed - default (no --ignores) is unchanged — deployer-safe - the checked-in tools/pip_audit_ignores.yaml itself passes schema validation (regression guard so the workflow never breaks on its own ignore file) Docs ==== Updates docs/reference/supply_chain_security.md and the TR mirror plus docs/usermanuals/{en,tr}/operations/supply-chain.md with the new deployer workflow (write your own ignores.yaml, pass via --ignores). Bilingual parity verified by tools/check_bilingual_parity.py --strict. Refs: #58, #59 (review comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
cemililik
added a commit
that referenced
this pull request
May 22, 2026
…view) Address review feedback on the pip_audit_ignores.yaml loader: - Validate `aliases` is a list of strings before unpacking it into the id index. Previously `aliases: CVE-2025-2953` (a bare string) would be iterated character-by-character, polluting the index with single-char "ids" and silently breaking matching; a non-iterable would crash. Now fails closed with a clear ::error::. - Validate value shape of required fields, not just presence: id, package, reason, threat_model, reevaluate_after must be non-empty strings, and verified_at must be a 'YYYY-MM-DD' string (quote it in YAML so it is not parsed as a datetime.date). An empty reason was a documented-justification gap the old presence-only check let through. Note: reevaluate_after is deliberately NOT date-validated — it is a free-text retirement condition ("Each release cycle, or when …"), so the review's suggested date-regex on it would have rejected every real entry. Date validation applies to verified_at only. - Hoist `ignore_ids = set(ignores)` once in _bucket_findings instead of intersecting against `ignores.keys()` per finding. - Doc grammar: "fails the gate closed" -> "causes the gate to fail closed" in both EN mirrors, and document the new malformed-value rejection in the reference + user-manual (EN + TR). Adds 7 tests: aliases-as-string, aliases-with-non-string-element, null-aliases-accepted, empty-required-string, malformed verified_at, unquoted-YAML-date rejection, and free-text reevaluate_after accepted. 23 tests pass. Refs: #59 (review) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cemililik
added a commit
that referenced
this pull request
May 24, 2026
…5-24) The nightly on main is green through the new --ignores mechanism (the run log shows CVE-2026-1839 correctly suppressed), but a new no-fix advisory dropped: transformers PYSEC-2025-217 (CVE-2025-14929), X-CLIP checkpoint-conversion deserialization RCE. CVSS is AV:L/UI:R (local + user must open a malicious checkpoint), affected through 5.0.0rc0 with no fix in the pinned 4.x line. Same threat-model carve-out as the existing transformers/torch deserialization ignores: operators are documented as required to load only trusted checkpoints, and ForgeLM does not invoke X-CLIP checkpoint conversion. With the Phase-refactored ignore file this is a one-line data change, no workflow edit — exactly the maintainability win the PR #59 review asked for. Refs: #58 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cemililik
added a commit
that referenced
this pull request
Jun 14, 2026
* chore: migrate repository references from cemililik to HodeTech org The project moved to the HodeTech GitHub organization (github.com/HodeTech/ForgeLM). Update every reference project-wide: - Repo URLs: github.com/cemililik/ForgeLM -> github.com/HodeTech/ForgeLM (README badges, pyproject [project.urls], docs, user manuals EN+TR, notebooks, site/, CHANGELOG, CONTRIBUTING, issue-template config, model_card.py, standards, roadmap). - GHCR image: ghcr.io/cemililik/forgelm -> ghcr.io/hodetech/forgelm (lowercase namespace, per GHCR owner naming). - Contact email (CODE_OF_CONDUCT.md): ai-team@cemililik.com -> forgelm@hodetech.io. - SonarCloud (sonar-project.properties): organization=hodetech, projectKey=HodeTech_ForgeLM. 271 references across 99 files, 1:1 replacements (no other edits). Local git remote origin re-pointed to the HodeTech URL. Doc/site guards re-run green: anchor resolution, bilingual parity, user-manual self-contained, CLI help consistency, site-version, site-claims. model_card tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(supply-chain): ignore transformers PYSEC-2025-217 (nightly 2026-05-24) The nightly on main is green through the new --ignores mechanism (the run log shows CVE-2026-1839 correctly suppressed), but a new no-fix advisory dropped: transformers PYSEC-2025-217 (CVE-2025-14929), X-CLIP checkpoint-conversion deserialization RCE. CVSS is AV:L/UI:R (local + user must open a malicious checkpoint), affected through 5.0.0rc0 with no fix in the pinned 4.x line. Same threat-model carve-out as the existing transformers/torch deserialization ignores: operators are documented as required to load only trusted checkpoints, and ForgeLM does not invoke X-CLIP checkpoint conversion. With the Phase-refactored ignore file this is a one-line data change, no workflow edit — exactly the maintainability win the PR #59 review asked for. Refs: #58 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs+supply-chain: address review findings (chat safety/compare, privacy pin, PYSEC-2025-217) All four findings verified against current code; all still valid. chat.md (EN+TR) — safety routing (L66-68): Verified forgelm/cli/_parser.py: `forgelm chat` has no --safety flag and loads no config; run_chat/ChatSession take no safety arg. The old copy claimed a YAML `safety:` binding into the chat session — false, and the key path was wrong. Rewritten to state the current release does NOT screen per-turn in the REPL, and that `evaluation.safety.enabled: true` is consumed only by the training auto-revert gate (forgelm/trainer.py) and `forgelm safety-eval`. Collapsed the inconsistent `--safety` / `--safety on` forms to a single `--safety`. chat.md (EN+TR) — chat-compare note (L110): Verified forgelm/cli/_no_train_modes.py: --benchmark-only runs lm-evaluation-harness on a SINGLE checkpoint, no side-by-side, no judge. Removed the implied parity; now says operators must run --benchmark-only per checkpoint, run the judge separately, and aggregate themselves until the planned chat-compare UX ships. site/js/translations.js — privacy.body (6 locales): Dropped the stale hard-coded `pip install forgelm==0.5.5` pin (real release is 0.7.0; this string is outside update_site_version.py's scope so it drifted). Now `pip install forgelm` — no version to drift, consistent with the site's no-build-step constraint. tools/pip_audit_ignores.yaml — PYSEC-2025-217: Reworded to advisory language: full CVSS vector (AV:L/AC:L/PR:N/UI:R/S:U/C:H/I:H/A:H), "no fixed version for the 4.x line" instead of "affected through 5.0.0rc0", reevaluate_after no longer assumes a fixed 5.x exists. Documented the codebase check (git grep over forgelm/): no X-CLIP usage, no checkpoint-conversion path besides HF->GGUF export, zero direct torch.load sites — all loading via from_pretrained, so the vulnerable path is unreachable. Validated: bilingual parity, anchor resolution, user-manual self-contained, site-version, site-claims, site chrome parity, CLI help consistency, ignore-file schema; 23 check_pip_audit tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(trainer): reconcile GRPO eval args so construction doesn't crash on validation split (C1/XP-01) GRPO trains on generation-based rewards, not validation loss, so `_build_grpo_trainer` pops the `eval_dataset`. But the GRPO branch of `_get_training_args_for_type` left `eval_strategy="steps"` in place — the value `_get_common_training_kwargs` sets whenever a validation split exists, which is the *default* pipeline path (auto-created split). A GRPOConfig with `eval_strategy="steps"` and no eval_dataset makes HF/TRL raise `ValueError: ... you didn't pass an eval_dataset` at GRPOTrainer construction. Force `eval_strategy="no"` and clear `eval_steps` in lockstep with the popped eval_dataset (the best-model args were already popped). Regression test `test_grpo_config_disables_eval_when_validation_split_present` asserts ForgeLM's arg-reconciliation invariant (not TRL's raise): with a validation split present, the GRPOConfig kwargs carry `eval_strategy="no"`, no `eval_steps`, and none of the eval-coupled best-model args. Guarded on `GRPOConfig` importability (the actual precondition) rather than `GRPOTrainer`. Findings: F-P2-FAB-05, F-P3-FABLE-01 (XP-01). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(webhook/approve): stop approve/reject AttributeError crash from webhook persist-field drift (C4/XP-03) `_WEBHOOK_PERSIST_FIELDS` had drifted from the real `WebhookConfig` schema: it persisted six fields that don't exist (`notify_on_revert`, `notify_on_awaiting_approval`, `secret_env`, `timeout_seconds`, `retry_count`, `retry_backoff_seconds`) and dropped the real `url`/`notify_on_start`/`timeout`. When `forgelm approve`/`reject` rebuilt a WebhookNotifier from the co-located compliance manifest via a SimpleNamespace, `_resolve_url` read `self.config.url` and raised AttributeError — *after* the model was already promoted and the `human_approval.granted` audit event committed, crashing a successful enterprise run and violating the public exit-code contract. Three-part fix: 1. compliance.py — derive `_WEBHOOK_PERSIST_FIELDS` from `WebhookConfig.model_fields` (single source of truth, secret-bearing `url` excluded) so the persisted shape can never drift from the model again. 2. webhook.py — `_resolve_url` reads `getattr(self.config, "url", None)` so a rebuilt namespace missing `url` resolves cleanly instead of crashing. 3. _approve.py — wrap the post-promotion notifier emit (approve + reject) so a webhook hiccup is non-fatal (a notification is not a gate). Regression tests (TestWebhookPersistRoundTrip): field-set parity vs the live schema; `_resolve_url` tolerates a namespace with no `url`; inline `url` secret never reaches the manifest while `url_env` survives; and an end-to-end rebuild via the real `_build_approval_notifier` fires `notify_success` without crashing. Findings: F-P1-FAB-02, F-P4-OPUS-09 (XP-03). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(pipeline): route auto-reverted approval-gated stage to exit 3, add awaiting_approval discriminator (C2/XP-02) A reverted approval-gated stage was misreported as "awaiting human approval" (exit 4) instead of "eval-failure" (exit 3): when require_human_approval was on, the pipeline eagerly set staging_path before the post-train gates ran, but the benchmark/safety/judge revert branches set success=False without clearing it — and _run_single_stage checked `if result.staging_path` (→ exit 4) BEFORE `if not result.success` (→ exit 3). The revert had already deleted the staging dir, so the run was reported as awaiting approval against a non-existent artefact. TrainResult / the JSON envelope had no field distinguishing the two. Fix: - results.py: add `awaiting_approval: bool` — the authoritative discriminator (set True only when the gate genuinely fires alongside success=True). - trainer.py: the gate sets `awaiting_approval=True`; the three gate revert branches clear `staging_path` (revert deletes the dir). - _pipeline.py: route on `success` first (exit 3), then on the explicit `awaiting_approval` flag (exit 4) — not `bool(staging_path)`, which can survive a revert. - _training.py: single-run path routes on the result discriminator, not the bare config flag. - _result.py: JSON envelope carries `awaiting_approval` (always) + `staging_path` (when staged). - json-output.md (EN+TR): correct the false `success: true ⟺ exit 0` claim and document the full training result envelope (success/awaiting/reverted). Tests: pipeline reverted-gated → exit 3; genuine gate → exit 4 via discriminator; stale-staging-on-failure defends to exit 3; the gate sets the flag and revert clears staging_path; envelope discriminates all three outcomes. Fixed the masking orchestrator test that encoded the success=False-as-gated bug. Findings: F-P1-FAB-12, F-P1-FAB-13, F-P1-FAB-30, F-P2-FAB-02, F-P2-FAB-14 (XP-02). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(pipeline): refuse --resume-from across an unapproved gated stage (C3/P2-FAB-01) The resume skip-list decided "already done" from disk presence: ``status in {completed, gated_pending_approval} and os.path.isdir(output_model)``. A ``gated_pending_approval`` stage's ``output_model`` is its UNAPPROVED staging directory (``final_model.staging.<run_id>``), which exists on disk — so ``--resume-from <later_stage>`` silently skipped the gated stage and chained downstream training from a model no human had signed off, defeating the Article 14 human-approval gate. Decide skip-eligibility on the state-file status, not disk presence: only a ``completed`` stage (with its output_model on disk) is skipped. A ``gated_pending_approval`` stage in the resume range is now refused with EXIT_CONFIG_ERROR and an actionable message telling the operator to approve or reject first — never silently bypassed. (The approval workflow that flips an approved stage to a skippable status is handled in the Wave-1 resume/terminal package; until then resuming past an un-approved gate is correctly blocked.) Regression test: a gated first stage + ``--resume-from dpo_stage`` without approval is refused (EXIT_CONFIG_ERROR) and instantiates zero downstream trainers. Replaces the prior test that asserted the insecure skip. Findings: F-P2-FAB-01. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(compliance): fail verify_audit_log on truncate-to-empty against genesis manifest (C6/P4-OPUS-01) `verify_audit_log` early-returned `VerifyResult(valid=True, entries_count=0)` for an empty log BEFORE the genesis-manifest cross-check ran — so an attacker who truncated the audit log to zero entries passed verification clean, even though the genesis manifest sidecar pins a real non-empty first entry. This is the exact truncation attack the manifest exists to detect; the in-chain hash-continuity walk can't catch it because there are no lines left to walk. - `_verify_genesis_manifest`: when the manifest pins a first entry but entries_count == 0, return valid=False with a truncate-to-empty reason. - `verify_audit_log`: consult the genesis manifest before reporting a clean empty chain; a manifest-less empty log stays legitimately valid. Tests: truncate-to-empty with manifest → valid=False; empty log without a manifest → valid=True (benign first-run); and a re-root tamper (wipe body, keep write-once manifest, write a fresh valid chain) → manifest-mismatch valid=False (P4-OPUS-22 genesis-mismatch coverage that was previously untested). Findings: F-P4-OPUS-01 (+ F-P4-OPUS-22 test coverage). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ci): catalog 6 pipeline.* events, fix guard false positive, wire + test the audit-catalog guard (C7/XP-04) `tools/check_audit_event_catalog.py` exited 1 at HEAD, was wired into no workflow, and had no own test — so six genuine `pipeline.*` stage/decision events (`started`, `force_resume`, `stage_started`, `stage_completed`, `stage_gated`, `stage_reverted`) drifted into the code uncatalogued with zero CI tripwire on a public EU AI Act Art. 12 contract (append-only audit log). - docs/reference/audit_event_catalog.md (+ -tr mirror): add the 6 pipeline.* rows with accurate when-emitted / payload / Article columns. - check_audit_event_catalog.py: tighten the emission detector to match only real emission contexts (log_event/_audit_event first-arg, event= kwarg, "event": JSON value, _EVT_* constant declarations) instead of any quoted dotted string — kills the `training.output_dir` config-path false positive (F-P8-C-12) while dropping zero genuine events (verified empirically). - ci.yml + CLAUDE.md/AGENTS.md gauntlet: wire the guard with --strict so the drift can never recur silently. - tests/test_check_audit_event_catalog.py: happy path, undocumented-event and ghost-row failures, the config-path-not-flagged regression, constant/keyword emission detection, real-repo-in-sync, and meta-assertions that the guard is wired into CI and the gauntlet. Findings: F-P8-C-01 (XP-04: F-P4-OPUS-19 + F-P8-OPUS-03/06), F-P8-C-12. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(model-card): redact synthetic.api_key (and any secret) from the generated README (C5/P1-FAB-01) `SyntheticConfig.model_dump` overrides redaction of `api_key`, but Pydantic v2's parent `ForgeConfig.model_dump()` recurses via its OWN serializer and never calls the child's Python-level override — so the redaction was dead on the parent-serialization path. The model-card exclude set (`{auth, webhook, monitoring}`) also omitted `synthetic`, so a populated `synthetic.api_key` landed in the generated `README.md` in plaintext. Two-layer defence in model_card.py: 1. Add `synthetic` to the section `exclude` (matches auth/webhook/monitoring). 2. `_redact_secrets`: recursively mask any secret-keyed value (`api_key`, `token`, `secret`, `password`, ...) in the serialized config, while keeping `*_env` env-var-NAME references intact. Catches residual secrets in any non-excluded section, forward-compatibly. Chose the model-card-side fix over a config field_serializer to avoid the `merge_pipeline_stage_config` round-trip hazard (a redaction literal becoming the live value — F-P1-FAB-03 / H6 territory). Tests: `synthetic.api_key` never appears in the rendered card; the recursive masker redacts `judge_api_key`/`password` but keeps `*_env` names. Findings: F-P1-FAB-01. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(trainer): apply model.max_length to all trainers; fail loud on unset judge API key env (H1a / F-P3-FABLE-03,18) F-P3-FABLE-03: only the SFT branch passed model.max_length to TRL; DPO/ORPO/ SimPO/KTO silently fell back to TRL's 512/1024 max_length defaults and GRPO to its 512 max_prompt_length default, while the config field, configuration.md, and the Article 11 compliance manifest all claimed model.max_length applied — silent truncation plus a false statement in an audit artefact. Added `_apply_max_length` and wired it into the four preference branches (max_length) and GRPO (max_prompt_length). A future TRL rename now surfaces as a loud TypeError, never silent truncation. F-P3-FABLE-18: a configured `judge_api_key_env` that is unset silently flipped the judge to local mode (judge.py treats api_key=None as "local"), running a different evaluator than configured; with auto_revert=true a failed local load deletes the trained adapters over a misdiagnosed env-var problem. The trainer now raises ConfigError naming the variable BEFORE invoking the judge, so the model is preserved (auto-revert never fires) and the error is actionable. Tests: per-trainer max_length application (DPO/GRPO verified; ORPO/KTO/SimPO skip where trl can't import the config in the local env); judge env unset → ConfigError + run_judge_evaluation never called; env set → API mode runs. Findings: F-P3-FABLE-03, F-P3-FABLE-18. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(model): MoE detection across architectures, unsloth honours lora.method, loud non-CUDA 4-bit, VLM pad guard (H1b / F-P3-FABLE-07,09,10,11) F-P3-FABLE-07: `_apply_moe_config` gated MoE behaviour on `hasattr(config, "num_local_experts")`, which only Mixtral/Phi expose — Qwen3-MoE (`num_experts`) and DeepSeek-V3 (`n_routed_experts`) silently no-op'd despite the README advertising them. Resolve the expert count via an attribute registry and WARN loudly when moe is configured but no expert attribute is found. F-P3-FABLE-09: the unsloth backend passed only the deprecated boolean shortcuts, silently downgrading `method: dora`/`rslora` to plain LoRA and dropping `method: pissa` — identical YAML trained a different algorithm than the transformers backend. Both backends now derive flags from the shared `_resolve_peft_flags`; pissa+unsloth raises ConfigError instead of silently degrading. F-P3-FABLE-10: `load_in_4bit=true` on a non-CUDA host loaded full precision with zero log output while compliance artefacts still claimed "4-bit NF4". Added a loud WARNING that the run is not quantized and artefacts will overstate it. F-P3-FABLE-11: the multimodal path read `tokenizer.pad_token_id` unguarded, but an AutoProcessor exposes pad_token only on its inner `.tokenizer` (AttributeError crash), and loaded VLMs via AutoModelForCausalLM (most are registered under AutoModelForImageTextToText). Resolve the inner tokenizer for pad handling and select the image-text-to-text auto-class when multimodal is enabled. Findings: F-P3-FABLE-07, F-P3-FABLE-09, F-P3-FABLE-10, F-P3-FABLE-11. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(merging): replace torch.quantile with kthvalue so the default TIES merge works on real models (H1c / F-P3-FABLE-19) `_ties_merge_tensor` computed the trim threshold with `torch.quantile`, which hard-fails above 2^24 (16,777,216) elements (PyTorch #64947). Every real-model weight tensor exceeds this — a 7B mlp.gate_proj is ~45M elements — so the DEFAULT MergeConfig method (`ties`) crashed on any non-toy model with the cryptic "quantile() input tensor is too large", swallowed into MergeResult and surfaced as a CLI exit 2 that gave the operator no clue the feature was broken. `kthvalue` has no size limit and yields the same trim threshold: the k-th smallest magnitude, k = trim_fraction · n. DARE was unaffected (no quantile). Regression test: `_ties_merge_tensor` on a tensor just past 2^24 elements returns the same shape without raising and trims ~20% to zero. Findings: F-P3-FABLE-19. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(trainer): self-document OOM batch-size recovery in the manifest + exception-safe restore (H1d / F-P3-FABLE-04) OOM recovery mutates config.training.per_device_train_batch_size in place. The compliance manifest restored the configured (pre-OOM) value while the model card read the effective (post-OOM) value, so the two Article-11 artefacts silently contradicted (manifest=16, README=4) with nothing explaining the discrepancy. The save/restore was also not exception-safe: a manifest-build error skipped the restore, leaving config holding the configured values under the outer BLE001 catch. - Record BOTH values + an explicit `oom_recovery` block (applied, configured_*, effective_*) in the manifest when OOM occurred, so an auditor sees the discrepancy explained rather than two artefacts that just disagree. - Wrap the restore in try/finally so the effective values are always restored. The manifest's primary batch_size keeps the configured value (existing contract + test unchanged); the new block is additive. Tests: oom_recovery block present with both values after a simulated OOM; absent when no OOM; config restored to effective values when manifest generation raises. Findings: F-P3-FABLE-04. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(packaging): drop unused mergekit dep; align all six surfaces to the native merge (H1e / F-P3-FABLE-20) mergekit was declared in the `merging` extra, advertised in README/usage, probed by `forgelm doctor`, and named in config field descriptions + the architecture standard — but never imported anywhere. Model merging is fully native (peft + torch on state dicts), as docs/reference/architecture.md already stated ("no mergekit dependency required"). The unused extra installed a heavy package with its own tight transformers/torch pins that could downgrade a training env for zero benefit, and `forgelm doctor` warned about a "missing merge backend" on systems where merge works perfectly. Aligned all surfaces to the native implementation: removed mergekit from the `merging` extra (kept empty for `pip install forgelm[merging]` backward-compat), dropped the doctor probe row, reworded config.py descriptions, README, usage.md (+ TR), the architecture standard's heavy-deps list + extras matrix, and the merging.py BLE001 rationale. Regression tests: no extra declares mergekit; the `merging` extra is an empty no-op. Findings: F-P3-FABLE-20. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(model): apply MoE expert selection + quantization AFTER get_peft_model (H1f / F-P3-FABLE-02,08) Both MoE expert operations ran BEFORE get_peft_model, so neither did anything the operator asked for: F-P3-FABLE-02: `_freeze_unselected_experts` set requires_grad=False on base expert weights one line before get_peft_model, which then re-freezes every base parameter and makes only the injected `lora_*` adapters trainable — so the post-PEFT trainable set was identical whether experts_to_train was set or not, while the log asserted a selective-training outcome that never materialised. F-P3-FABLE-08: `_apply_moe_expert_quantization` ran before any freezing, so its "recast frozen expert weights" sweep always found nothing (permanent no-op on the non-4bit path); on the default 4-bit path it recast packed Params4bit uint8 storage to bf16, corrupting the quant_state (crash or garbage at first forward). Restructured the flow: `_detect_moe_experts` resolves the count pre-PEFT; `_apply_moe_post_peft` runs selection + quantization AFTER get_peft_model. `_freeze_unselected_experts` now freezes the LoRA adapters of unselected experts (and warns when target_modules cover no experts, so selection has no effect). Quantization skips entirely when the model is 4-bit-loaded and never casts packed uint8/int8 storage; per-tensor recast failures now surface at WARNING. Tests: selection now constrains the adapter set (differs from "all"); no-expert- adapter case warns; 4-bit model skips quantization (uint8 unchanged); detection recognises Qwen3/DeepSeek shapes. Findings: F-P3-FABLE-02, F-P3-FABLE-08. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: address PR review on W0+H1 (ruff format, ConfigError exit-1, revert paths, audit/doc accuracy) CI: `ruff format` config.py (the failing `ruff format --check .` gate). Bandit B615 (from_pretrained without revision pinning) is left as-is — a pre-existing MEDIUM-severity pattern shared by all four from_pretrained calls (no revision pinning anywhere in the project); bandit fails only on HIGH. Code: - trainer.py: `_mark_reverted` clears `final_model_path` and `awaiting_approval` (not just `staging_path`) on every gate-revert branch — the revert deletes the model, so the CLI/JSON envelope must not advertise a stale path, and a reverted run is never awaiting approval. - _training.py + _pipeline.py: route `ConfigError` to EXIT_CONFIG_ERROR (1) before the generic catch, so the judge-env-unset ConfigError (H1a) maps to the config- error exit code instead of EXIT_TRAINING_ERROR (2) in both entrypoints. - _pipeline.py: emit a `pipeline.resume_refused` audit event (requested/blocking stage + status) before refusing a resume across an unapproved gate (Article 14 decision-gate record) — catalogued EN+TR. - model.py: resolve `peft_config` dict (PEFT keys by adapter name) before reading `target_modules` in the no-expert-adapters warning, so it shows the real value. - _approve.py: webhook-failure handlers log the exception TYPE, not the raw object (a transport error can embed the secret-bearing URL/token). Docs: - audit_event_catalog (EN+TR): mark `stage_completed`'s `metrics` (success only) and `auto_revert_triggered` (failure path only) as conditional; add the new `pipeline.resume_refused` row. - architecture standard: "specific version bounds" → "version constraints (a lower bound each; some also carry an upper bound)" to match pyproject. Tests: - ConfigError→exit-1 (pipeline stage + single-run CLI); resume-refusal emits the audit event; revert clears final_model_path + awaiting_approval (seeded True); target_modules surfaces from a dict peft_config; packaging tomli fallback for Python 3.10; GRPOConfig importability guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(trainer): validate judge_api_key_env at preflight to fail fast before training (review) The judge_api_key_env check (F-P3-FABLE-18: an API judge whose env var is unset must not silently flip to local mode) ran only at the post-train judge stage, so a misconfigured run consumed the whole training loop before failing. Move the check into a `_validate_judge_config` preflight called from `__init__` (alongside `_validate_evaluation_config`), so construction raises ConfigError before training starts. `_run_judge_if_configured` now calls the same shared helper (defence-in-depth for the direct entry point), so there is no duplicated logic and the existing judge-time tests stay valid. The CLI ConfigError→exit-1 routing already wraps trainer construction in both entrypoints, so the earlier raise still maps to EXIT_CONFIG_ERROR. Not done — the suggested audit event for this refusal is intentionally skipped: a preflight config-validation error is not a runtime decision gate (the audit-event requirement targets safety/judge/benchmark/approval gate *outcomes* with scores); the sibling preflight `_validate_evaluation_config` likewise raises/warns without audit events; and adding one would introduce a new catalogued event for a non-gate config error before any run-level audit context exists. The ConfigError message + the exit-1 contract are the actionable record. Test: constructing ForgeTrainer with an unset judge_api_key_env raises ConfigError at __init__ (preflight), not only at the judge stage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pip-auditstep because OSV / GHSA published 10 new no-fix advisories overnight (9 torch + 1 markdown).--ignore-vulnin.github/workflows/nightly.ymlwith an inline justification, threat-model carve-out, and re-evaluation condition, matching the existingCVE-2026-1839(transformers) pattern.Issue #37 tracks the active setcomment — Nightly CI failure — 2026-05-07 #37 was a closed nightly-failure issue, not a tracker.Triage detail
torch (9 CVEs, all local attack vector)
torch.mkldnn_max_pool2d— local DoStorch.nn.utils.rnn.pad_packed_sequence— local memory corruptiontorch.nn.utils.rnn.unpack_sequence— local memory corruptiontorch.jit.script— local memory corruptiontorch.lstm_cell— local memory corruptiontorch.jit.jit_module_from_flatbuffer— local memory corruptiontorch.cuda.memory.caching_allocator_delete— local memory corruptiontorch.profiler.profile— local DoS via missingprofiler.stop()pt2Loading Handler — local deserialization (same trust model astorch.load(pickle))All require a local attacker passing malformed inputs to a specific torch API. ForgeLM is a local-CLI training tool; an attacker with that access is already inside the trust boundary. None of the affected APIs are called with attacker-controlled arguments in
forgelm/. Verified againstpypa/advisory-database/vulns/torch/*.yamlon 2026-05-21.markdown (1 CVE, OSV range misclassification)
fixedevent, so every version is flagged; the advisory description says the fix shipped inmarkdown==3.8.1. Installed 3.10.2 is post-fix. Re-evaluate when the OSV record gains afixed: 3.8.1event.Why this isn't
|| truepip-auditruns with|| true(sanctioned carve-out indocs/standards/testing.md) becausetools/check_pip_audit.pythen applies the project's severity policy on the captured JSON. The script fails closed on every UNKNOWN-severity finding — which is every finding in practice, since pip-audit's JSON does not carry OSV severity. Adding documented ignores for accepted-risk CVEs is the prescribed escape hatch (seedocs/reference/supply_chain_security.md— "Suppression").Re-evaluation
Each release cycle: re-run nightly without the ignore set and drop any CVE that:
Test plan
python3 -c "import yaml; yaml.safe_load(open('.github/workflows/nightly.yml'))"— YAML parsespython3 tools/check_anchor_resolution.py --strict— passespython3 tools/check_bilingual_parity.py --strict— passespython3 tools/check_no_analysis_refs.py— passeshas_any_idmatches against{id} | aliases, so PYSEC IDs in--ignore-vulnare honoured (verified by readingpip_audit/_service/interface.py)Nightly Compatibilityworkflow on this branch to confirm the supply-chain job goes greenCloses #58
🤖 Generated with Claude Code
Summary by Sourcery
Update nightly supply-chain workflow to suppress specific no-fix CVEs while documenting risk and reevaluation policy.
Bug Fixes:
Enhancements: