Repository navigation
Arm the fail-safe recorder/audit orchestrator hooks (item 28.2 stage 1) - #167
Merged
Merged
Conversation
Declare the stage-1 orchestration registry (record, lease, state_dir, dispatch_tools) so orch_dispatch_ledger, orch_prompt_stamp, orch_resume_audit, and orch_resume_barrier arm. These four hooks are recorder/warn-only and cannot block by construction. Extend tools/check-orchestration-registry.py with a strict top-level key allowlist and per-key validation for the new arming keys (record; lease with a non-bool bounded max_age_hours; state_dir; dispatch_tools). Add tools/test-orch-stage1-hooks.py (behaviour and wedge-safety tests that invoke the installed hook in a throwaway repo) and a committed-registry change-carries-check in tools/test-orchestration-registry.py, all wired into the orchestration-registry CI workflow. The enumerator-dependent scheduling guards (orch_stop_guard, orch_yield_tool), the TODO.md AEI migration, and the mode record are deferred to stage 2. The live lease makes scope live, which arms the Stop family; the operator escape sentinel neutralizes that until the stage-2 flip.
….2 stage 1, QA round 1) Close two codex adversarial-QA findings. HIGH: the registry gate accepted a declared path with a lone surrogate code point (U+D800..U+DFFF), which the installed hook cannot UTF-8-encode, so resolving it (orch_resume_barrier realpath, aiqt_hooks.py:9828) crashes the PreToolUse handler and the dispatcher fails closed exit 2. Broaden the path check (_has_control_char -> _has_unsafe_char) to reject surrogates across every declared-path surface (record, lease, state_dir, dispatch_tools, companion_stores). MEDIUM: the committed-registry change-carries-check skipped on an absent registry; make it fail, so a tree lacking the arming change turns CI red. Disclose the OS-agnostic _is_absolute residual (a Windows-style path reads as absolute) as a known LOW. Add five surrogate-reject tests (48 gate tests, 31 hook tests).
Deploying cleanlanguageai with
|
| Latest commit: |
64fdaaa
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9134e6c2.ai-language.pages.dev |
| Branch Preview URL: | https://item-28-2-stage1-arm-recorde.ai-language.pages.dev |
…age 1, QA round 2) Close round-2 codex QA HIGH-1: point state_dir at /opt/cleanlanguage/orch-state (a non-store, all-traversable dir) instead of the record store, so a store-blind session's unreadable state_dir can no longer drive write_scope_guard's _load_write_scope to 'bad' and deny covered writes. The operator escape sentinel is moved to the new location by lab_infra (verified at source). LOW fixes: catch RecursionError in the JSON parse for a clean gate diagnostic on deeply-nested input (still fail-closed); name surrogates in the path-rejection messages and docstring; correct the max_age_hours comment (the (0,8760] cap is a gate-side sanity bound, stricter than the hook, which applies no upper lease bound); make the prompt_stamp counter-reset test load-bearing by seeding nonzero counters (proven by mutation). Disclose the CI-safe FIFO-at-registry-path plain-open residual (lab_infra's O_NONBLOCK reader is the stage-2 consolidation point).
…nd 3) Close round-3 codex QA HIGH: the gate validated record and state paths independently, so it accepted a registry where a declared record/lease path collides with the machine-state namespace (e.g. record.handoff = <state_dir>/resume-barrier.json). orch_resume_audit unconditionally opens <state_dir>/resume-barrier.json with mode 'w' (aiqt_hooks.py:9775, 9783-9785), which would clobber the declared record. Add a cross-field check: when state_dir is declared, reject any record.* or lease.path that equals or is inside it. companion_stores is not checked (a state_dir may legitimately sit under a companion store; the hazard is a record/lease file landing on a machine-state file). LOW: make the resume-audit barrier test load-bearing (seed an active barrier, assert a clean audit clears it, proven by mutation); broaden the disclosed FIFO residual to note a declared record/state path pointing at a special file blocks the hook's own read (out of the gate's scope, CI-safe).
…2 stage 1, QA round 4) Close round-4 codex findings. HIGH-1: the record/state collision check (normpath + startswith) was bypassed by path-spelling aliases (state_dir '/', leading-'//' spellings); replace it with an os.path.commonpath containment over spell-normalized paths, so a record/lease path on or under state_dir is rejected however spelled. HIGH-2 (best-effort + disclosed): reject an over-long path (component > 255, or whole path > 4096 bytes) -- a pure string check closing the ENAMETOOLONG denial; and, best-effort, reject a state_dir that exists at gate time but is not a directory (closing the /etc/passwd ENOTDIR denial). The irreducible residual (a not-yet-existing state_dir unusable at runtime) is disclosed: it fails safe to a covered-write denial the operator fixes, and write_scope_guard is defence-in-depth, not a security boundary. LOW: correct the docstring overclaim -- record/lease/state_dir are the behaviourally load-bearing arming keys; dispatch_tools with an empty list is identical to omitting it (declared for honesty). Tests: 61 gate + 31 hook, all pass.
…ir (item 28.2 stage 1, QA round 5) Close two more adversarial-QA findings, both refinements of the round-4 checks. (1) The length check bounded state_dir at PATH_MAX, but the hooks derive longer paths under it (<state_dir>/write-scope.json, ...), so a near-limit state_dir would hit ENAMETOOLONG at runtime -> a write_scope_guard covered-write denial. Reserve STATE_DERIVED_RESERVE (64 bytes, >= the longest derived basename 'attestations-validated.json' plus margin): a state_dir whose length + separator + reserve exceeds PATH_MAX is rejected. (2) The best-effort gate-time directory check swallowed a known ENOTDIR (state_dir '/etc/passwd/' or a path through a regular file) as absence and accepted it; classify three ways -- FileNotFoundError is genuine absence (still allowed, the disclosed not-yet-created residual), any other OSError (ENOTDIR, ENAMETOOLONG, ...) is a known-unusable path -> reject, an existing non-directory -> reject. LOW: the corrupted-registry barrier test seeded the wrong state directory (the bad-registry handler reads the XDG default); prime it via the audit and assert the barrier's actual behaviour, proven load-bearing by mutation. Tests: 65 gate + 31 hook, all pass.
…g; bound the gate (item 28.2 stage 1, QA round 6) Two further accepted-input refinements from adversarial QA of the round-5 gate, both in the gate's ACCEPTED-INPUT space (the committed registry was unaffected in every round); with these the stage-1 gate is bounded. (1) Existing-symlink alias collision. The record/state collision check compared only lexically-normalized spellings, so a record path that reaches state_dir through an existing symlink component (record.handoff /run/shm/resume-barrier.json with state_dir /dev/shm, where /run/shm already points to /dev/shm) passed the lexical check, and orch_resume_audit would then open its barrier over the declared record. Add _contains_or_equal(child, parent) and run BOTH the lexical containment check and a resolved-path check: reject when os.path.realpath of a record/lease path equals or sits inside os.path.realpath of state_dir. A genuine sibling directory is still accepted. (2) POSIX component splitting. The over-long-component check split on a character class treating a backslash as a separator; on POSIX a backslash is a filename byte, so an intrinsically-overlong final component (200 + backslash + 100 = 301 bytes) was undercounted and, with an absent parent masking lstat, accepted: a string-detectable ENAMETOOLONG that becomes a persistent covered-write denial. Split on the forward slash only. The bound: the remaining residual is the disclosed irreducible runtime-usability class (a future symlink swap after validation, the not-yet-created state_dir runtime directory-usability, a FIFO or special declared path). These are runtime path semantics a commit-time string gate cannot soundly close; they fail safe to a covered-write denial the operator fixes, and write_scope_guard is defence in depth, not a security boundary. The disclosure is consolidated at the gate residual block. Both findings reproduced RED without the fix and GREEN with it. Tests: 68 gate + 31 hook, all pass; the committed registry still validates.
…-state-dir collision (item 28.2 stage 1, QA round 7)
Round-7 adversarial QA found a further fixable accepted-input HIGH (the
committed registry was unaffected, as in every round): the record/lease-vs-
state_dir collision check runs only when state_dir is declared, but when
state_dir is OMITTED the hooks do not stop using a state dir. They fall back to
${XDG_STATE_HOME:-~/.local/state}/aiqt-guardrails/orch/<repo-key>
(_state_dir_from_registry, aiqt_hooks.py:7638-7651), and orch_resume_audit still
opens <state_dir>/resume-barrier.json with mode "w" there, so a record/lease
path landing inside that default dir is clobbered exactly as the round-3
declared-state_dir collision. It is that collision's surviving sibling through
the fallback branch; the reproduction showed gate-exit-0 then an actual hook
clobber of the declared record.
Fix: require state_dir whenever record or lease is declared. This closes the
whole class at gate time, declared and default alike: a gate-passing armed
registry never falls back to the XDG default, and the existing lexical and
resolved collision checks then always apply. dispatch_tools carries tool names,
not a path, so it cannot collide and does not trigger the requirement
(test_dispatch_tools_with_strings pins that scope, staying accepted).
Tests: two reject cases (record-without-state_dir, lease-without-state_dir),
each RED without the fix (the pre-fix gate accepted both at exit 0);
test_lease_without_max_age now declares state_dir so it still isolates max_age
optionality under the new contract. 70 gate + 31 hook tests pass; the committed
registry still validates (exit 0).
…(item 28.2 stage 1, QA round 8) Round-8 adversarial QA found (MEDIUM, mutation-proven) that the round-7 rule "a declared record or lease requires state_dir" masked the targeted rejection of the pre-existing reject fixtures that omit state_dir: each such fixture rejected on the missing-state_dir reason regardless of whether its OWN targeted check (max_age bounds, record shape, path validation, surrogate, ...) fired, so a regression of that check would go undetected. This is a change-carries-check coverage erosion introduced by the round-7 requirement. Fix (test-file only; the gate is unchanged and its accepted-input space is unaffected): the _reject helper now injects a disjoint, absent state_dir for a reject fixture that declares record or lease without one, so the round-7 rule is inert for that fixture and only its targeted check can reject it. The disjoint path avoids any collision and the absent path keeps the state_dir lstat check inert, so nothing but the intended check fires. The two test_*_without_state_dir_rejected tests, whose target IS the rule, opt out with add_state_dir=False. Verified by mutation: with the gate's max_age check disabled, the suite now FAILS on all five lease.max_age reject tests (isolation restored; before this fix they passed under the same mutation, masked). 70 tests pass on the unmutated gate; the committed registry still validates (exit 0).
…e (item 28.2 stage 1, QA round 8) Round-8 adversarial QA (weighted cross-family review) found a fixable defect: PATH_MAX (4096) INCLUDES the terminating NUL, so the longest USABLE pathname is 4095 bytes. The whole-path length check used `> PATH_MAX`, wrongly accepting a 4096-byte declared path that the kernel rejects at runtime (os.stat -> ENAMETOOLONG, errno 36); for a lease/state path the hook's stat then falls to an OSError and, with no mode declared, an inactive scope (fail-safe, not destructive; the committed registry's short paths are unaffected). NAME_MAX (255) EXCLUDES the NUL, so the per-component check correctly stays `> NAME_MAX`. Reproduced: os.stat on a 4096-byte path raises errno 36, and the pre-fix gate accepted a 4096-byte lease.path (exit 0). Fix: reject `>= PATH_MAX` on the whole-path check, and the same off-by-one on the sibling derived-path reserve check (its practical impact was already absorbed by the 64-byte reserve versus 27-byte real basenames; fixed for correctness and consistency as the class). A completeness probe confirms no code `> PATH_MAX` comparison remains. Verified RED->GREEN on the exact boundary: a 4096-byte lease.path now exits 1, a 4095-byte path stays exit 0 (no over-rejection), and the committed registry still validates (exit 0). Tests: 72 gate (two new boundary tests) + 31 hook, all pass.
…al collision branch (item 28.2 stage 1, QA round 10) Round-10 adversarial QA (claude PASS; gemini two findings reproduced and adjudicated non-blocking; codex infra-empty, owed a re-run) confirmed the committed registry safe and the round-9 hard-link HIGH non-exploitable. Three sub-blocker defects are fixed here, comment and test only, no behaviour change and the committed registry unchanged: THE BOUND now discloses post-gate filesystem aliasing (a hard link, or a symlink planted at a derived state path such as the barrier name) as a silent-clobber residual and corrects the earlier overstatement that every residual was a fail-safe denial (the barrier write is a direct truncating open with no S_ISREG guard, aiqt_hooks.py:9783); it also discloses the POSIX backslash-Windows collision blind spot (unreachable for the committed POSIX artefact, forward-slash caught, mixed pairs fail closed); and a change-carries-check test pins the lexical collision branch, a mutation survivor, proven RED without that branch. Gate suite 73 tests, hook suite 31 tests, both green.
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.
Item 28.2 stage 1: arm the fail-safe recorder/audit orchestrator hooks
⛔ HOLD MERGE — do NOT auto-merge on green. The maintainer has explicitly held this merge: it arms guardrail hooks that load at the next session launch (high-blast-radius). This overrides the repository's auto-merge-on-green convention. The merge gate is: green CI and clean tri-family adversarial QA and the operator escape sentinel present/validated (already verified) and the maintainer's explicit go.
What this does
Declares the stage-1 orchestration registry so four recorder / warn-only hooks arm:
orch_dispatch_ledger— records async dispatches (prevents the wrong-revision async-binding class)orch_prompt_stamp— clock-stamps genuine human input, resets denial countersorch_resume_audit— reconciles handoff / record surfaces at SessionStart (warn-only; cannot block)orch_resume_barrier— warns once on an out-of-allowlist mutation while armed (blocks nothing)Registry keys added:
record,lease(max_age_hours24),state_dir,dispatch_tools: [];companion_storeskept byte-identical.Gate + tests
tools/check-orchestration-registry.py: a strict top-level key allowlist + per-key validation (record; lease with a non-bool boundedmax_age_hours; state_dir; dispatch_tools). The existing presence / UTF-8 / version / companion_stores logic is untouched.tools/test-orchestration-registry.py: 43 tests, including a change-carries-check on the committed registry (RED on a two-key tree) and per-key reject cases.tools/test-orch-stage1-hooks.py(new): 31 tests that invoke the installed hook in a throwaway repo, proving each armed behaviour present — and absent under the two-key registry, so the keys are load-bearing — plus adversarial wedge-safety (bad registry, missing / unwritable state_dir, stale lease, and the Stop-familyNO_ENUMERATORsurface). It durably pinsorch_stop_guard's fail-open on a bad registry (refuting a QA claim that it fails closed).Omitted keys (deferred to stage 2)
enumerator,yield_tools(the scheduling guards),mode(needs a derivedOperating-mode:record — the canonical file is JSON),attestations,mistakes_register,staleness,escape,truth. The strict allowlist mechanically keepslease.holder_is_session_idout (a wrong holder token would silently disarm scope).The lease / Stop side-effect and the sentinel
Declaring
leasemakes scope live, which also armsorch_stop_guard(it shares the scope predicate); with no enumerator it would deny each turn-end once (bounded, fail-open). Per the maintainer's decision this is neutralized by a standing operator escape sentinel created by lab_infra as root:/opt/cleanlanguage/orch-state/ESCAPE-ALLOW-YIELD(root-owned, 0644, regular file, single hard link, no symlinked component) — verified at source against_orch_escape_active. lab_infra removes it at the stage-2 flip.state_diris outside the store (QA round 2)state_diris/opt/cleanlanguage/orch-state— a non-store, cleanlanguage-owned, mode-0711 (all-traversable) directory, not the record store. Round-2 QA (codex) found that pointingstate_dirinto the store would couplewrite_scope_guard(it reads<state_dir>/write-scope.json) to store accessibility, denying covered writes for a store-blind session; relocating it to an all-traversable dir removes that coupling (any session canlstatthe absent floor → fail-open). The sentinel and the recorders' runtime state live there.QA
Three tri-family adversarial rounds (staggered): round 1 fixed a surrogate-path gate gap (→ hook exit 2) and an absent-registry skip; round 2 relocated
state_dir(above) and polished LOWs (RecursionError diagnostic, surrogate naming, a corrected comment, a load-bearing counter-reset test, a disclosed CI-safe FIFO residual); round 3 confirms convergence. Each fix was verified red-before/green-after and re-verified at source by the orchestrator.Arming evidence (verified at source)
open-findings.md,pending-decisions.md,session-handoff.md,session-state.md)..aiqt/orchestration.local.json(the CI-blind whole-file override) exists._orch_escape_activecondition.state_dir/opt/cleanlanguage/orch-state/exists (non-store, all-traversable, mode 0711).Residual (disclosed)
Stage 1 adds no unbounded or fail-closed path beyond the pre-existing bad-registry class that the current
companion_storesregistry already carries unchanged. Scope is repo-coarse (holder_is_session_iddeferred). After merge, one live-session observation (a guard-events or ledger row) confirms scope actually went live.