Repository navigation
The three source findings raised against #11679 after it merged (ordering, runner-width coupling, false absence) - #11902
Conversation
…t merged #11679's park verdict arrived after it landed, so all three findings are on main. Repaired here in one change. (a) THE ORDER WAS COMMENT-VERSUS-CODE, AND WORSE, IT WAS ENCODED IN TWO WITNESS NAMES. converge_controller_app_key called ensure_key_dir, which runs `install -d` and THEN observes, before observe_upper_ancestors -- so a host mutation preceded the observation entitled to refuse the whole converge. No key was ever written (controller_app_key_action decides after every observation), so the residue was an empty root-owned directory rather than a custody hole. But an_unsafe_directory_refuses_before_any_write and a_writable_ancestor_refuses_before_any_write call only the pure decision with supplied observations, so they stayed green while advertising a property the production route did not have. Both repairs, not one: the create is now gated on the ancestor reading (on the refusing arm the directory is only OBSERVED, which is a read), and the two witnesses are renamed to what a claim over a pure function can establish -- refuses before any KEY-CONTENT write. controller_app_key_action is untouched, so the decision keeps its single authority. (b) CUSTODY WAS COUPLED TO RUNNER WIDTH. The entry resolved its host through runner_host_file_subject, which requires an admitted RunnerHostDeploy -- a spec plus an admitted WIDTH. srv2's width is unresolved, so srv2 has no deploy row and the mode terminated on srv2 while describing itself as covering srv1 and srv2. The App key's location, administrator target and custody do not depend on how many slots a host admits, so the module now resolves its own subject from runner_host_spec_for, which names srv2's address and principals independently of width. The privilege law is NOT dropped with the deploy row: the writer may not be the account CI jobs run as, and that comparison keeps its one authority -- gunbc.runner_host_grants gains bootstrap_principal_is_not_the_job_user, and the existing deploy-shaped predicate now calls it rather than being duplicated. The new subject deliberately carries no unit populations: this module verifies no teardown, so a value with empty unit lists would lie about what was established. (c) THE STAGING ABSENCE PROBE COULD FALSELY CONFIRM. `test -e` exit 1 became StagingAbsent, and GNU test -e is stat(path) == 0, so EACCES, ELOOP, ENOTDIR and EIO all answer 1 with an empty stderr -- and StagingAbsent is an input to ControllerAppKeyHeld, so a metadata failure would have been reported as custody held with no residue. Absence is now established by a successful listing of the PARENT, which is gunbc.live_deploy.member_observe's rule rather than a second vocabulary. The departure from that authority is stated rather than silent (DESIGN §3b): entry_presence reaches the far side as the bootstrap principal, and this parent is the key directory at root:root 0700, so an unelevated listing can only ever refuse -- it would answer Indeterminate on every host whether or not residue exists. The same membership question therefore runs over this module's elevated leg. classify_staging_residue is now pure over a listing that SUCCEEDED, so StagingUnobservable has no constructor inside it; that arm belongs to the leg. EXECUTED EVIDENCE: all 21 witnesses in microvm_controller_app_key_converge_witness_test pass, including three new discriminating ones -- srv2 resolves a custody subject although the deploy route still refuses it (which is the pair that would go red if custody were re-coupled to width), and the parent listing establishing absence, residue and the empty-listing case. Retained untouched, as the reviewer asked: the key path consumed from the signer's own JwsSigningKeyRef, the placement refusing the fabric-cell tree, RemoteFileModeOwnerRead closing the octal vocabulary at 0400, the byte-for-byte comparison against the resolved SM version, the full re-read after mutation, no key bytes in any receipt, and the old runner-owned key left alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…eview 69368) Both findings are the same class, both are right, and the sharper point is that this diff applied the correct standard one line earlier -- the privilege predicate was deliberately extracted to keep one authority -- and then broke it twice in the same file. 1. The staging listing argv and the membership read were re-spellings of gunbc.live_deploy.member_observe entry_presence_in_parent. The stated §3b departure argues only that the TRANSPORT must be this module's elevated leg; it does not reach the argv or the predicate, which are the same facts whichever principal runs them. And the copies had ALREADY drifted in the commit that created them: mine trimmed each line, the original does not -- two sources that can answer one question differently, which is the fork §3 forbids and the drift §2 predicts. member_observe now exports the two pure halves, parent_listing_argv and listing_contains_entry, and BOTH entry_presence_in_parent and this module consume them. The elevated transport stays here, because that is the only part that actually differs. 2. The administrator SSH target was re-spelled from runner_host_file_subject_over_obligation. gunbc.runner_host_file_converge now exports administrator_ssh_target -- the endpoint from the fleet reach authority, the principal from the declared administrator, neither depending on the deploy row -- and both subjects consume it. Executed: 21/21 microvm_controller_app_key_converge witnesses still pass, and member_observe's own consumers (member_identity_witness_test) stay green after the extraction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Both findings are right, and the sharper point is the one I'd make against myself: this diff applied the correct standard one line earlier — the privilege predicate was deliberately extracted so the comparison kept one authority — and then broke the same rule twice in the same file. Fixed in 1. The listing argv and the membership read. You're right that my stated §3b departure doesn't reach them: it argues only that the transport must be this module's elevated leg, and the argv and the predicate are the same facts whichever principal runs them. The drift evidence is the part worth keeping — the two copies had already diverged in the commit that created them (mine trimmed each line,
2. The administrator SSH target. Same class. Executed after the change: 21/21 witnesses in — sent from keen-bear-791 |
Ledger-Repair-Judged: docs/design-rung-drops.md Heal-Candidate-Run: 35536516170
…-three-findings # Conflicts: # ROADMAP.md
|
Conflict resolved at The conflict was Resolved by regeneration, not by choosing a side:
Re-verified after the merge rather than assuming: all 21 witnesses in — sent from keen-bear-791 |
…rity the write does The ordering repair stopped `install -d` beneath an untrusted ancestor, but converge_controller_app_key still evaluated clear_staging unconditionally, and clear_staging BEGINS with an elevated leg: run_elevated_leg(staging_remove_argv) = sudo rm -f -- <staging>. The non-write arms of the preceding match produce `none`; they did not skip cleanup. So this execution was representable: /etc/gunbc group- or other-writable -> PathsNotRootOnly -> directory only OBSERVED -> controller_app_key_action refuses -> no key content written -> ELEVATED rm STILL EXECUTES BENEATH THE REFUSED ANCESTOR. The consequence is not residue. A non-root principal who can write an upper ancestor can replace an intermediate path component -- a directory or a symlink -- between the observation and the leg, so the route performs a ROOT MUTATION THROUGH THE EXACT PATH WHOSE CONTAINMENT IT JUST DECLINED TO TRUST. The module note said staging is removed "after every run whether or not a write was attempted" and the code implemented that literally. The correct boundary is narrower and the note now states it: after every attempt WHOSE ANCESTOR AND PARENT-DIRECTORY AUTHORITY HAS BEEN ESTABLISHED. The gate is PATH AUTHORITY and deliberately not the whole action: a content or file-metadata observation may be unobservable while the path is safely root-contained, and cleanup is still desirable there. StagingCleanupAuthorization is sole_constructor, so the bad state is UNWRITABLE rather than merely unreached (DESIGN 4b rung 4) -- admit_staging_cleanup is the only mint, joining upper ancestors = PathsRootOnly, directory = KeyDirRootOnly, and staging's parent being that admitted directory. clear_staging now accepts the sealed authorization instead of a bare subject and a bare path, so a cleanup leg over an unadmitted path has no spelling. The withheld arm carries StagingUnobservable, which controller_app_key_custody already counts as not-held. WHY THE PREVIOUS 21 COULD NOT SEE IT: the ordering controls invoke only the pure decision with supplied values and never execute clear_staging, so a control proving controller_app_key_action refuses was already green while the cleanup mutation ran. Five controls added over the admission fold, which is the whole gate: writable upper ancestor -> withheld; unsafe key directory -> withheld; UNREADABLE key directory -> withheld (blind is not safe); staging path outside the admitted directory -> withheld (the join, not merely the two readings); and root-only ancestors + root-only directory -> an authorization naming the exact parent and staging path. The four withheld arms and the positive control discriminate each other. 26/26 witnesses PASS in one claim_batch over the module. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d, not constructed review 69444, raised as REQUEST_CHANGES against the cleanup-authorization repair. The module note at the sole_constructor claim says clear_staging accepts the sealed authorization "so there is no way to spell a cleanup leg over a subject and a bare path that no admission fold produced". That was true of the subject, the parent and the path -- and NOT of staging_name, which clear_staging still took as an unsealed parameter beside the seal, and which is precisely the value classify_staging_residue KEYS ON. A name that does not match authorization.staging reads a parent listing CONTAINING the residue as StagingAbsent, and that absence feeds ControllerAppKeyHeld: the same false-absence class the parent-listing repair in this PR exists to close. The defect is not a live one -- the single production caller passed path_basename(path: staging) over the same value -- so this is not a behaviour fix. It is a RUNG HONESTY fix (DESIGN 4b meta-obligation 1: the reported rung must equal the rung established by executed evidence). A comment asserting a seal covers a value the seal does not reach is an overclaim, and shipping one inside the PR whose subject is overclaims is the defect in a new place. So staging_name is DERIVED inside clear_staging from authorization.staging and the parameter is deleted, which makes the comment true rather than making the comment softer -- construction over restatement (DESIGN 5). Every value the leg uses now comes from the seal: subject, parent, path, and the residue predicate's key. The note is corrected to say exactly that. 26/26 witnesses PASS in one claim_batch at this source head. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…carry it was dangling
review 69456. Both halves are one defect and one edit fixes both.
staging_cleanup_admission_wire was added for symmetry with the other *_wire
functions in this module and never given a consumer: git grep returned its own
definition and nothing else. DESIGN 3c -- a declaration with no call site in the
closure is dangling, and dangling is red regardless of how well modeled it is.
The consumer it should have had was one line away. The withheld arm at the
converge call site matched StagingCleanupWithheld { reason: _ } and substituted
the constant staging_cleanup_withheld_reason, so the receipt said "root-only path
authority was not established" without saying WHICH of the three joined facts
failed -- while admit_staging_cleanup had just computed exactly that string. An
operator reading that receipt cannot tell whether to fix an ancestor, fix the
directory, or fix a staging path pointing outside it. That is a typed diagnostic
that is not a LOCATED one (DESIGN 5), and it is the flattening this PR exists to
remove, reintroduced one layer down in the repair itself.
So the cause is threaded through and the constant is demoted to a PREFIX, which
is what it always was; the dangling wire is deleted rather than given a
ceremonial call, because the located string is what the receipt needs and the
wire added nothing over it.
A 27th control asserts the three withheld causes are PAIRWISE DISTINCT and
non-empty. That is what makes "located" a property rather than a claim: it
reddens if the causes are ever flattened back onto one constant, which is
precisely the defect 69456 caught.
38/38 witnesses PASS in one claim_batch from the composed head -- 27 App-key
converge plus the 11 member_identity.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ng cleanup, ancestors-before-mutation) and #11845 (parent-listing absence) into the custody roster Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…_converge (§4c) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#11679's park verdict arrived after it landed, so all three findings are on main. One PR, as asked. A fourth finding was raised against this PR during review and is repaired here as (d).
(a) The ordering was worse than comment-versus-code — it was encoded in two witness names
converge_controller_app_keycalledensure_key_dir, which runsinstall -dand then observes, beforeobserve_upper_ancestors— a host mutation ahead of the observation entitled to refuse the whole converge. No key was ever written (controller_app_key_actiondecides after every observation), so the residue was an empty root-owned directory, not a custody hole. Butan_unsafe_directory_refuses_before_any_writeanda_writable_ancestor_refuses_before_any_writecall only the pure decision with supplied observations, so they stayed green while advertising a property the production route did not have. That is an evidence population inviting a future reader to cite a property the code lacks.Both repairs, not one. The create is gated on the ancestor reading — on the refusing arm the directory is only observed, which is a read — and the two witnesses are renamed to what a claim over a pure function can establish: refuses before any KEY-CONTENT write.
controller_app_key_actionis untouched, so the decision keeps its single authority.(b) Custody was coupled to runner width, and srv2 was unreachable
The entry resolved its host through
runner_host_file_subject, which requires an admittedRunnerHostDeploy— a spec plus an admitted width. srv2's width is unresolved, so srv2 has no deploy row and the mode terminated there while the PR described it as covering srv1 and srv2. The key's location, administrator target and custody do not depend on how many slots a host admits, so the module resolves its subject fromrunner_host_spec_for.The privilege law is not dropped with the deploy row:
gunbc.runner_host_grantsgainsbootstrap_principal_is_not_the_job_user, and the existing deploy-shaped predicate now calls it rather than being duplicated (§3). The new subject deliberately carries no unit populations — this module verifies no teardown, so empty unit lists would be fields that lie.(c) The staging absence probe could falsely confirm
test -eexit 1 becameStagingAbsent, and GNUtest -eisstat(path) == 0, so EACCES, ELOOP, ENOTDIR and EIO all answer 1 with an empty stderr — andStagingAbsentfeedsControllerAppKeyHeld, so a metadata failure would have been reported as custody held with no residue. Absence now comes from a successful listing of the parent, which isgunbc.live_deploy.member_observe's rule rather than a second vocabulary.The departure from that authority is stated, not silent (§3b):
entry_presencereaches the far side as the bootstrap principal, and this parent is the key directory atroot:root 0700, so an unelevated listing can only ever refuse — it would answer Indeterminate on every host whether or not residue exists. The same membership question therefore runs over this module's elevated leg.classify_staging_residueis now pure over a listing that succeeded, soStagingUnobservablehas no constructor inside it; that arm belongs to the leg.(d) Cleanup was a root mutation beneath a path the route had just refused to trust
(a) stopped
install -dunder an untrusted ancestor, butconverge_controller_app_keystill evaluatedclear_stagingunconditionally, andclear_stagingopens with an elevated leg:run_elevated_leg(staging_remove_argv(staging))=sudo rm -f -- <staging>. The non-write arms of the preceding match producenone; they did not skip cleanup. So this execution was representable:/etc/gunbcgroup- or other-writable →PathsNotRootOnly→ directory only observed → action refuses → no key content written → elevatedrmstill executes beneath the refused ancestor. That is not residue: a non-root principal who can write an upper ancestor can replace an intermediate path component — a directory or a symlink — between the observation and the leg, so the route performs a root mutation through the exact containment it had just declined to trust.The module note claimed staging is removed "after every run whether or not a write was attempted", and the code implemented that literally. The correct boundary is narrower and the note now states it: after every attempt whose ancestor and parent-directory authority has been established.
Gated on path authority, deliberately not on the whole action — a content or file-metadata observation may be unobservable while the path is safely root-contained, and cleanup is still desirable there.
StagingCleanupAuthorizationissole_constructor, so the bad state is unwritable rather than merely unreached (§4b rung 4).admit_staging_cleanupis its only mint and joins three facts: upper ancestorsPathsRootOnly, directoryKeyDirRootOnly, andpath_parent(staging) == key_dir.clear_stagingtakes the sealed carrier instead of a bare subject and bare path, and derives every value the leg uses from it — subject, parent, path, and the residue predicate's key (review 69444: as a parameter,staging_namewas the one value a caller could still spell freely, and it is exactly whatclassify_staging_residuekeys on, so the stated rung was claimed rather than constructed). The withheld arm carriesStagingUnobservable, whichcontroller_app_key_custodyalready counts as not-held, so the run still refuses.Why (a)'s controls could not see it: they invoke only the pure decision with supplied values and never execute
clear_staging, so a control provingcontroller_app_key_actionrefuses was already green while the cleanup mutation ran.Executed evidence
All 27 witnesses in
microvm_controller_app_key_converge_witness_testpass, plus the 11member_identitywitnesses affected by the sharedmember_observesymbols — 38/38, run in oneclaim_batchfrom the composed head withgunbcandclaim_batchrebuilt from that exact tree.The three added for (a)–(c): srv2 resolves a custody subject while the deploy route still refuses it (the pair that goes red if custody is re-coupled to width), plus the parent listing establishing absence, residue, and the empty-listing case.
The five added for (d), over the admission fold, which is the whole gate: writable upper ancestor → withheld; unsafe key directory → withheld; unreadable key directory → withheld (blind is not safe); staging path outside the admitted directory → withheld, which tests the join rather than merely proving two positive constructors were supplied independently; and the positive control — root-only ancestors + root-only directory → an authorization naming the exact parent and staging path. The four withheld arms and the positive control discriminate each other, so neither a constant-withhold nor a constant-authorize fold stays green. A 27th (
review 69456) asserts the three withheld causes are pairwise distinct and non-empty — the withheld arm had been discarding the causeadmit_staging_cleanupcomputed and substituting one constant, so the receipt could not tell an operator whether to fix an ancestor, the directory, or a staging path pointing outside it. The cause is now threaded through, the constant is demoted to a prefix, and that control reddens if they are ever flattened back together.Composition: merged current main (12 commits, including seed changes under
src/v1/stage0) with no conflict at any file. Thegenerated_artifact_gatedry run is green on the composed tree, and that green is itself controlled — planting one line in.githooks/pre-commitreddens it with a located finding (EXIT=1, committed content differs from authority), so the clean result is a reading rather than a silent pass.Retained untouched, as the review asked
Key path consumed from the signer's own
JwsSigningKeyRef; placement refusing the fabric-cell tree;RemoteFileModeOwnerReadclosing the octal vocabulary at 0400; byte-for-byte comparison against the resolved SM version; full re-read after mutation; no key bytes in any receipt; the old runner-owned key left alone.🤖 Generated with Claude Code