Repository navigation
Conversation
…of duties
The human-approval gate records who approved (FORGELM_OPERATOR, else the OS user) next to who trained, and writes
both to the audit log. It does not compare the two, and both identities are self-asserted. Several documents said
otherwise:
- the ISO 27001 mapping ("enforces trainer != approver") and the SOC 2 mapping ("approval gate operator != trainer");
- the Statement of Applicability row for A.8.2 ("non-trainer operator");
- the deployer guide, the change-management SOP and the user manual ("dual-controlled", "two distinct operator IDs
prove segregation of duties");
- the deployer guide's checklist, which said F-compliance-110 raises ConfigError if require_human_approval is missing
for a strict risk tier. It only checks that the safety evaluation is enabled; the CLI wizard turns the approval gate
on for those tiers, hand-written YAML is not checked.
The same documents already said the right thing elsewhere (the SoA row for A.5.3, access_control.md, the approve
reference); this brings the rest in line. English and Turkish. The design spec in docs/design is a record of intent and
is left as written; the website copy that says "dual-controlled" is a separate change.
Refs #73, #99, #90
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Sorry @cemililik, you've used your own review budget of 250,000 diff characters for the last 7 days. You can request another review in 13 hours and 47 minutes by commenting |
Contributor
|
Warning Review limit reachedEnable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Next included review available in 49 minutes. View limit details
Comment |
Reviewer's GuideThis documentation-only change aligns English and Turkish compliance materials with the implementation: the human-approval gate records trainer and approver identities for audit purposes but does not enforce segregation of duties or reject self-approval. It also corrects the strict-tier configuration guidance and records the changes in the unreleased changelog. Sequence diagram for audit identity recording without segregation enforcementsequenceDiagram
participant Trainer
participant ForgeLM
participant AuditLog
participant IdentityProvider
participant Approver
Trainer->>ForgeLM: training.started
ForgeLM->>AuditLog: Record trainer identity
Approver->>ForgeLM: forgelm approve
ForgeLM->>IdentityProvider: _resolve_approver_identity()
IdentityProvider-->>ForgeLM: FORGELM_OPERATOR or getpass.getuser()
ForgeLM->>AuditLog: Record human_approval.granted
Note over ForgeLM,AuditLog: ForgeLM does not compare identities or reject self-approval
AuditLog-->>IdentityProvider: Deployer compares identities externally
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
3 of 4 tasks
Collaborator
Author
|
Superseded by #1000: the wave-0 fixes are combined by theme into five pull requests to cut the review load. This branch's commits are included there unchanged apart from the |
cemililik
added a commit
that referenced
this pull request
Oct 18, 2026
… six-language site copy, described as the code behaves (#1000) ## Summary Compliance- and privacy-facing wording corrected to what the code does, plus the six-language site copy that repeated the same claims. Six earlier branches combined into one pull request, replacing #983, #986, #988 and #991 (to be closed as superseded) and two branches that were never opened. **This is compliance wording; please read the before/after tables.** ##⚠️ Localization exception `docs/standards/localization.md` keeps German, French, Spanish and Chinese as deferred tiers and says not to machine-translate them. At the owner's decision I **wrote the DE / FR / ES / ZH site strings myself**, with no native review. English and Turkish are authoritative; the other four need a native reader before they can be trusted. A follow-up issue for that is drafted, not yet filed. ## Related issues Refs #73, #99, #90, #94, #101, #87, #114, #75, #86, #89, #92, #98, #110, #112. The code halves stay open: whether the gate should ever compare trainer and approver, failing closed on a missing safety verdict (breaking), attaching SBOMs to the release. ## Changes | Theme | Said | Now says | |---|---|---| | **Segregation of duties** (ISO 27001 / SOC 2 mappings, SoA, SOP, deployer guide, manual) | the approval gate "enforces trainer ≠ approver", "dual-controlled" | it **records** trainer and approver identities and does not compare them; both are self-asserted; keeping them different people is the deployer's control. `require_human_approval` is not enforced for strict tiers by `ConfigError` (it only is when safety evaluation is off) | | **GDPR / KVKK manual page** | identifiable subjects are removed; "no specific person's data is recoverable from the model"; an `international_transfers` setting; audit report with source attribution and SHA-256 manifest; retention "auto-deletes originals" | eight identifier formats are masked, names and free text are not; the report is counts and summaries, not written to the audit log; retention is reported, not enforced; memorisation is not measured | | **No-verdict safety evaluation** (8 documents + TR) | exit 0 ⇒ every gate passed; classifier load failure halts the run; classification errors count as unsafe | a safety evaluation that produced no verdict (`evaluation_completed: false`) keeps the model and exits 0 (4 with the approval gate) regardless of `auto_revert`; code unchanged | | **SBOM** (reference, SOP, guides, manuals, mappings) | `gh release download …`; "re-emit from the git tag"; "licence extraction in SBOM" | workflow-run artifacts, 90-day retention; `gh run download`; the file is the CI runner's `pip list` (test tooling included, no dependency graph, no licences) | | **Article 12 / 14** (audit-event catalogue, audit-log and human-oversight pages) | the audit log is "mandated by Article 12"; the gate "implements" Article 14 | the log is development-time evidence that supports Annex IV; Article 12 logging and Article 14 in-use oversight belong to the deployed system | | **Site, six languages** | benchmark failure "fails (exit 3)"; "dual-controlled"; Slack/Teams/Discord; "PII redacted"; "satisfying the access-request side of the GDPR"; wizard "Enforce automatic deletion"; "SHA-256 manifest"; DeepSpeed "presets" | exit 3 only with `auto_revert: true`; records the approver; Slack-format, Teams and Discord need a relay; common formats masked, names not detected; reverse-pii finds only what is literally in the scanned files; retention mode reserved; SHA-256 sidecar; bring your own DeepSpeed JSON | The site GGUF strings describe the `.sha256` sidecar, the retention strings the reserved mode, and the DeepSpeed strings the preset caveat; they match the GGUF, code and docs pull requests, so **merge those first**. A new site test keeps the retired phrases out of English, Turkish and the HTML (mutation-checked). ### What this changes for a deployer A DPIA or audit response can no longer cite ForgeLM for: segregation of duties being enforced, personal data being removed or unrecoverable from the model, the SBOM being on the release page, or the audit log being the Article 12 mechanism. ## Type - [x] Documentation (and site copy) ## Testing - The full `CLAUDE.md` gauntlet from a clean worktree of this branch: exit 0. (An earlier run failed `tests/test_turkish_glossary_terms.py` because the Article 12 rewrite wrote "Ek IV" instead of "Annex IV", fixed here, then two timing-based regex linearity tests failed while two gauntlets ran at once; they pass alone and in this final run.) - `node --test tests/site/*.test.cjs`: 81 pass; English ↔ Turkish keys in lockstep. ## Checklist - [x] English and Turkish updated together (Turkish wording needs the owner's review) - [ ] DE / FR / ES / ZH reviewed by a native speaker — **not done** - [x] CHANGELOG entries under `[Unreleased]` 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 5.5 <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
Stops six documents from saying the human-approval gate enforces segregation of duties or "dual control". It records who trained and who approved and writes both to the audit log; it does not compare them, and both identities are self-asserted. A deployer citing these pages to an auditor would have claimed a control ForgeLM does not provide.
Related issues
Refs #73, #99, #90 — the documentation halves are resolved here. The website copy ("dual-controlled" in
site/compliance.htmland six locales oftranslations.js, #94/#99) is a separate change; whether the code should ever compare the two identities is left open on the issues (see below).Changes — each row shows what it said and what it says now
docs/reference/iso27001_control_mapping{,-tr}.mdA.5.3FORGELM_OPERATOR, else the OS user) so it is auditable; ForgeLM does not reject self-approval; keep the identities disjoint at your identity providerdocs/reference/soc2_trust_criteria_mapping{,-tr}.mdCC1.2docs/qms/statement_of_applicability{,-tr}.mdA.8.2docs/guides/iso_soc2_deployer_guide{,-tr}.mdConfigErrorif you forget"require_human_approvalevaluation.safety.enabledis off for strict tiers; hand-written YAML without the gate is accepted; the CLI wizard turns the gate on for those tiersdocs/usermanuals/{en,tr}/operations/iso-soc2-deployer.mddocs/qms/sop_change_management{,-tr}.mdChecked against the code
_resolve_approver_identity()returnsFORGELM_OPERATOR, elsegetpass.getuser();forgelm approvelogs it and never compares it with the run's operator._enforce_safety_gate_for_strict_tier(forgelm/config.py) raises only when the safety evaluation is not enabled.evaluation.require_human_approval = Truefor strict tiers (forgelm/wizard/_orchestrator.py); the browser wizard offers it as a toggle.What this does and does not change for a deployer
A DPIA or an audit response can no longer cite ForgeLM for "trainer ≠ approver is enforced". The pages say what it does give: both identities on record, in an append-only chain. This lowers what could be relied on, and it matches
statement_of_applicability.md(A.5.3),access_control.mdandapprove_subcommand.md, which already said it. I did not touchdocs/design/iso27001_soc2_alignment.md: it is a design record, not a claim.Not done here
training.startedoperator isuser@hostwhile the approver is bareuser, so a naive comparison would miss the default case and could give a false assurance. That is a design decision, not a docs fix.compliance.art14.body,compliance.iso.change.body, six locales) andsite/compliance.html.Type
Testing
CLAUDE.mdgauntlet from a clean worktree of this branch: exit 0.Checklist
[Unreleased]🤖 Generated with Claude Code
Summary by Sourcery
Align compliance documentation with the approval gate’s actual behavior by distinguishing auditable identity recording from deployer-enforced segregation of duties.
Enhancements:
Documentation: