Repository navigation
One argv grounds both the command and its grant: privileged operations stop being binary paths - #9152
Conversation
…plied by the principal's roster Re-applied onto current main after #9091 squash-merged. The squash collapsed the base branch's two commits into one, so this branch's copies of them conflicted -- ordinary fallout of stacking under a squash-merge policy, not a content conflict. Only this change's own three files are carried forward; the privilege closure itself is already on main. WHAT THIS CLOSES. extdeps.github.actions_runner still wrote `sudo install` and `sudo rm` as bare literals, making it a second, silent authoring of "these two verbs are available as root" beside gunbc.fleet_posix_accounts. Those literals were correct BY COINCIDENCE -- install and rm happen to be roster members and nothing kept them so. The previous revision of that same function is the proof the coincidence is unreliable: it also wrote `sudo tar` and `sudo chown`, which never were, and the only thing that ever reported it was a wet apply on srv3. ActionsRunnerPrivilegedBinaries carries the two paths in. That module knows the SHAPE of runner installation -- fetch, verify, place, own -- and does not know which principal performs it or what that principal may execute. THE ACCESSOR IS DELIBERATELY NOT IMPORTED THERE. Pulling ci_runner_sudo_binary_path into extdeps would make a module describing an upstream product depend on this repository's principal policy, so the shape could no longer be reused by a caller with a different one. gunbc.runner_slot_provision gunbc_runner_privileged_binaries is the seam, and both paths come from the closed CiRunnerSudoBinary vocabulary. VERIFIED ON THE REBASED TREE, not assumed from a clean patch application: slot_extract_sudoes_only_granted_binaries returns true against current main. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s stop being binary paths srv3's converge apply refused at its first privileged command with zero slots mutated, and the cause was two authorities answering one question. The roster granted BINARY PATHS (fleet_posix_ci_runner_sudo_grants: install, rm, systemctl, apt-get, visudo, tailscale) while extdeps.github.actions_runner independently rebuilt an argv from those paths. Nothing joined them, so a grant for /usr/bin/install did not imply authorization for the argv the script actually ran. The disagreement was discoverable only by a wet apply on a real host. THE CONSTRUCTION. gunbc.executor_privileged_operation declares the operation once -- EnsureOwnedDirectory / RemoveDirectoryTree / SystemdDaemonReload, parameterized rather than a nullary enum of binaries -- and DERIVES both renderings from one argv projection: executor_privileged_operation_argv the words ..._sudoers_command = join(argv, " ") what authorizes ..._shell_command = sudo + quoted argv what executes A grant that fails to authorize its own command now has no spelling. Quoting is shell-level and invisible to sudo, which receives argv after the shell has parsed it, so one argv grounds both renderings without either knowing the other's escaping. WHAT WAS DELETED, not accumulated beside. ActionsRunnerPrivilegedBinaries and gunbc_runner_privileged_binaries() are gone; extdeps no longer rebuilds argv at all, it splices commands it is handed and stays agnostic. That is a net removal of a parallel rendering. TWO NARROWINGS the antagonist review of 2026-08-24 surfaced, both real reductions in authority: run_as is now root rather than ALL (ALL=(ALL) authorized the grantee to run as ANY user, including other service accounts), and no wildcard reaches the rendered file. /opt/actions-runner/* would match `..` -- fnmatch's * declines to cross `/`, and `..` contains none -- so it would authorize `rm -rf /opt/actions-runner/..`. Every managed path is named in full instead. TWO MODULES, and the split is structural rather than tidy: gunbc.runner_host_grants joins the operation authority to a host's slot roster, because an operation module that also read the roster would close an import cycle with runner_slot_provision, and acyclicity is the one structural law the import graph has. A §3 RESIDUAL IS RECORDED WITH A TRIGGER, not only prose: the fleet-converge state directory was spelled three times in fleet_converge_plan and named nowhere -- once as a bare literal in the lease guard, twice more as the leading segment of the generation store and lock paths. This declares it once; the row dissolves when fleet_converge_plan derives all three from it. EVIDENCE, six witnesses green by execution with RED measured rather than asserted. The load-bearing one is witness_executed_command_is_the_granted_command: it reaches the command the provisioning script RUNS through runner_slot_provision's real seam and the command the sudoers file GRANTS through the renderer, and asserts they are one word vector. Mutation receipt: dropping the quoting from the shell projection returns false, restoring returns true. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…f inferring it Every claim about srv3's grant state so far rests on ONE observed refusal -- a single argv under /usr/bin/install -- generalized into statements about six binaries and two principals. That is the denominator error this repo keeps paying for, and it is cheap to close: sudo -n -l is read-only, needs no privilege, and answers exactly what is unknown. The mode reports three things and mutates nothing: which account the converge apply actually runs as, what that account may sudo without a password, and whether the in-run fleet key authenticates as the deploy row's ssh principal and what THAT principal may sudo. The third is the one that decides whether a bootstrap path already exists -- if the fleet key reaches a sudo-capable briansrls, convergence can install its own grants with no new credential, and the grantee is never the source of its own authority. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review 55610 is right that a hand-written shell observation step in the workflow is a DESIGN section 6 scaffold tell -- raw shell implementing semantics expressible in .dag, with no carrier and no dissolution trigger -- and it is a separate concern from the model change this PR makes. It is preserved on session/warm-tern-755-grants-observe and will land with a .dag authority rather than as hand-authored workflow shell. Also fixes the static break review 55610 found: fleet_converge_plan_witness_test still called the deleted gunbc_runner_privileged_binaries and passed the removed job_user argument. Its assertions additionally spelled the sudo commands as UNQUOTED literals, which would have gone permanently red against the posix-quoted rendering -- so they are now built from the authority (executor_privileged_operation_shell_command) rather than restated, which is the drift this PR exists to remove. Verified green by execution. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Both findings verified and fixed in Finding 2 — the static break — was real and worse than reported. Beyond what the review names: that witness's assertions also spelled the sudo commands as unquoted string literals ( Finding 1 — the hand-written shell — is also correct, and it is out. A raw shell observation step with no Worth recording why that observation matters and why it has not run: every claim about srv3's grant state still rests on one observed refusal — a single argv under — sent from warm-tern-755 |
…tu, not the deploy row's ssh_target srv3's grants are installed and ghrunner now holds 44 of them, but the FACT THAT MADE THAT POSSIBLE lived only in a chat transcript, which is where knowledge goes to be lost. This commits it. WHAT WAS UNKNOWN AND IS NOW A ROW. A grantee cannot install its own sudoers grant, so the first install on any host needs a different principal -- and on the executor fleet that principal is NOT the deploy row's ssh_target. Probed against the live host with the fleet key (fleet-automation-ssh-key, fingerprint verified before contacting anything): briansrls Permission denied -- the ssh_target the deploy row NAMES does not accept it fleet-automation Permission denied root Permission denied ghrunner authenticates, nologin -- the grantee, a service account ubuntu full shell, (ALL) NOPASSWD: ALL -- the bootstrap principal WHY IT IS A ROW RATHER THAN A RUNBOOK LINE. The operator did not know the briansrls password and reasonably believed the fleet credential already carried sudo. It does -- under ubuntu, not under the account the deploy row names. That one fact was the entire blocker on going 5 -> 21 runners, it is not derivable from anything else in the tree, and recovering it cost probing five accounts against a live host. Its half-life in prose is one session. THE LAW IS WALLED, NOT NARRATED. bootstrap_principal_is_not_the_grantee is a predicate with a witness, so a future edit pointing both at one account fails rather than quietly creating an account that authorizes itself. RED is authorable: set the bootstrap login to job_user and it goes false. WHAT THIS STILL DOES NOT DO, stated so the gap is tracked rather than rediscovered: the install itself was performed BY HAND over ssh -- out-of-band actuation, the DESIGN section 6 tell. The staged/validate/install/read-back transaction is modeled nowhere for the executor path; Spark has one (gunbc.spark.managed_grant_install) and the executor does not. This row makes the missing piece namable. Next: an executor grant-install transaction consuming these rows, so no host is ever again bootstrapped from a terminal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…leet_posix_accounts import Both from review 55614, both mine, both introduced by this PR's own edits. The duplicate is the more interesting one: my scripted import insertion added actions_runner_base_dir to a block that already carried it, so one import list named one symbol twice. Whether the ingestor refuses it or dedupes it silently, a symbol that is its own second name is the section 3 violation this PR is otherwise about removing. The fleet_posix_accounts import (ci_runner_sudo_binary_path, SudoInstall, SudoRm) had zero usages outside its own import line after the refactor -- the argv is now derived inside executor_privileged_operation, which is the whole point, so this file no longer reads binary paths at all. Verified by counting usages with the import line excluded rather than by eye. Join witness re-run green after both removals. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ovisions dead slots MEASURED, not inferred. All 13 slots newly provisioned on srv3 came up, authenticated, reached 'Listening for Jobs', and were then refused by GitHub: An error occurred: Runner version v2.334.0 is deprecated and cannot receive messages. Runner listener exit with terminated error, stop the service, no retry needed. ExecStart exits 0/SUCCESS, systemd restarts it, and the unit sits in activating/auto-restart forever. So the slot count went 8 -> 21 while the RUNNING count stayed at 5: the provisioning path is fine and the artifact it installs cannot take work. THE ROW IS REPLACED, NOT ADDED BESIDE. A deprecated pin is not a version that merely lags; keeping it selectable would only preserve a way to build dead slots. Digests are the upstream's own, published in actions/runner's v2.336.0 release body, and the emitted installer verifies with sha256sum -c, so a wrong value fails at the host instead of installing an unverified tarball. NOT TOUCHED: gunbc.fleet_runner_connectivity incident_runner_version and the capacity-plan fixture keep 2.334.0. Those record a PAST incident and a fixed test input; rewriting them would falsify history to satisfy a grep. STANDING RISK THIS EXPOSES, worth stating because it is fleet-wide rather than srv3-local: the five runners still serving on srv3 hold agent registrations minted BEFORE the deprecation. They are grandfathered, not healthy, and will not survive a restart. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t three lists srv4 had zero ghrunner grants, so the converge apply could not run there at all. Rather than hand-write a second sudoers file, this adds two call sites of runner_host_sudoers_content -- the population follows each host's own slot roster and job_user, so srv1 and srv4 derive their grants from the same function srv3 already uses. Measured: srv4 renders 46 grants, zero wildcards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
THE FUSION WAS DELIBERATE AND ITS NOTE ASKED FOR THIS CHANGE. extdeps.systemd.systemctl already says enable and start are 'SEPARATE OPERATIONS, deliberately not enable --now', and then fuses them in systemctl_enable_now_command for exactly one caller -- runner slot activation -- with a note saying the split 'is its own change with its own witness'. This is that change. WHY THE TWO ACTIONS ARE NOT INTERCHANGEABLE. start runs the unit now, under prerequisites that hold now. enable declares it WANTED PERSISTENTLY, so it starts again after a reboot, at a moment when the cache instance and compile pool that gunbc.runner_activation gates on may not hold. Fused, one admission decision is spent on both, and the post-reboot start inherits an admission measured against a machine state that no longer exists. So both actions consume the admission here; enable is not the harmless half. WHAT THE MISSING SECOND CUT COST, measured rather than argued. The emitted apply.sh carries daemon-reload and disable and nothing that starts a runner -- enable count 0, start count 0. Fleet convergence therefore called a host converged when its slots existed on disk and served no traffic. srv1 held FIFTY slot directories with five runners running and nothing in the pipeline could notice. Convergence was measuring filesystem state while the subject is SERVICE CAPACITY. THREE ORTHOGONAL AXES, NOT ONE LIFECYCLE ENUM, because systemd holds every combination: a unit can be installed-but-disabled-and-running or enabled-but-inactive. A single ordered enum would have to pick an order that does not exist and would make the off-diagonal states unrepresentable rather than merely unusual. The fused spelling now has no representation: MaterializeRunner, EnableRunnerUnit and StartRunnerUnit are separate values, so 'enable and start' cannot be written as one opaque step. UNOBSERVED REFUSES RATHER THAN DEFAULTING on every axis. An axis nobody measured is not a disabled unit and not an inactive one; rendering it as either is the empty-observation narrow -- silently uncovered rather than merely expensive. EVIDENCE: 13 witnesses covering the full truth table, including the off-diagonal rows (enabled-but-inactive starts only; disabled-but-active enables only, which matters because fusing would restart a runner mid-job) and the three unobserved-axis refusals. Green by execution. Discriminating RED measured by mutation, not asserted: removing the admission check returns false, restoring returns true. NOT YET WIRED to a production actuator, and that is the next change rather than a claim made here -- runner_activate_command still has no production caller, and the workflow still has no activation mode. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review 55653. The header has now been wrong twice in two different ways, which is why it takes a parameter rather than a constant. It named executor_privileged_operation.dag until the module split moved the renderer (review 55626); it then named srv3's entry point for EVERY host once srv1 and srv4 rendered through the same function -- so srv1's installed file told a reader to reproduce it with srv3's command, on a production host, in the one string a human reads while deciding whether a sudoers file is safe to regenerate. A constant cannot be right here: the reproducing entry point is a fact about the HOST, not about the renderer, so it is supplied by the caller that knows which host it is rendering. Verified by execution -- srv1, srv3 and srv4 each now name their own entry point. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… consumer here Review 55653 is right that gunbc.runner_service_standing has no production consumer, and the reason it cannot get one in THIS PR is a measurement rather than an omission: gunbc.fleet_converge_plan observes nothing about build-cache or compile-pool readiness -- zero matches for BuildCacheInstanceReady, HostCompilePoolReady or admit_runner_activation anywhere in that module. The activation admission's inputs are receipts that path does not produce. So wiring it into slot_apply_lines here would leave two options, and both are worse than splitting. Fabricating the readiness receipts to make the gate open is the fabricated-plausible-output failure this repository refuses outright. Emitting a gate that can never open makes activation strictly worse than today, where at least a human starts the units and knows they did. The model, its 13-row truth table and its mutation receipt are preserved on session/warm-tern-755-runner-activation and land with the observation layer that feeds the admission -- observe enabled and active per slot, mint the readiness receipts, then a reviewed activation plan with its own subject and fresh observations. That is the change the side-chat analysis of 2026-08-25 describes, and it is a family, not a line appended to the install script. WHAT THIS PR STILL DOES NOT FIX, stated so it is not read as closed: apply.sh emits daemon-reload and disable and nothing that starts a runner, so convergence still calls a host converged when its slots serve no traffic. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Both findings verified and acted on in Finding 2 — the wrong entry point — was real, and it was the second time that string has been wrong. It named It now takes a parameter rather than a constant, because a constant cannot be right: the reproducing entry point is a fact about the host, not about the renderer. Verified by execution — srv1, srv3 and srv4 each name their own. Finding 1 — the unwired planner — is also correct, and I removed it from this PR rather than wire it. The reason it cannot get a consumer here is a measurement, not an omission: That left two ways to wire it into The model, its 13-row truth table and its mutation receipt are preserved on Not claimed as fixed: For context on what this PR did deliver: the grant/argv unification is live on three hosts, and the runner bump took the fleet from 10 serving runners to 39 (srv1 5→21, srv3 5→18). The deprecated — sent from warm-tern-755 |
…ody form CI's parse phase refused five sites across two witness files: source annotation sits inside a declaration body. Only module-item grain is modeled; move it above the declaration it describes. Both were mine, added in this PR. §4c is explicit that the initial .dag realization admits only standalone leading // blocks attached to module-scope declarations, and that trailing, body, unattached and block-comment forms refuse until separately modeled. I wrote body-form comments because they read better next to the expression they explain, which is exactly the temptation the rule exists to refuse. WHY THIS CLASS IS WORTH A COMMIT MESSAGE rather than a silent fix: DESIGN records that an in-body annotation on #8976 refused strict preparation for three hours while downstream measurements kept producing valid-looking values, and two lanes drew opposite conclusions from runs where the defect could not appear at all. A comment is not a small edit in this substrate -- it is the one edit class that can mask an entire phase. The content is preserved, hoisted above the declaration it describes. Both witnesses re-run green. LOCAL GAP THIS EXPOSED, recorded rather than left: this session's gunbc binary predates --required-ci, so the parse sweep could not be run before pushing and the class was caught by CI instead. A grep for indented // inside declaration bodies is a usable local proxy and is clean across every .dag this branch touches. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review 55676. A modeled argv must not launder an unmarked string realization: shell is one transport among N, and a function that concatenates a program is a realization detail the canonical emitter should eventually own. The marker was missing and now binds to the same dissolution target this module's sibling already uses -- gunbc.host_effect host_effect_apply -- because the two dissolve together or not at all. ONE CORRECTION TO THE FINDING'S FRAMING, which does not change the verdict. 'Introduces a hand-built shell program' overstates what the diff did to the string surface: extdeps.github.actions_runner on main already carries THREE inline join(["sudo ", ...]) concatenations, already under a Scaffold disposition (actions_runner_slot_extract_script_scaffold), and this PR replaced them with ONE derived rendering. The debt predates the diff and the diff reduces its authorings from three to one. What was genuinely missing was the marker on the new home, which is what this row supplies -- so the gate fires correctly even though the population it names got smaller rather than larger. Join witness re-run green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Fixed in One correction to the framing, which does not change the verdict. "Introduces a hand-built shell program" overstates what the diff did to the string surface: So the gate fired correctly on a population that got smaller, not larger — what was missing was the marker on the new home. I mention it only because a future reader comparing the finding to the diff would otherwise look for a string surface this PR created, and there isn't one. Join witness re-run green. Separately, for anyone triaging CI here: — sent from warm-tern-755 |
The operator's own words when I explained the bootstrap principal by hand: 'this knowledge is all supposed to be encoded in the workflows/models? because i will honestly forget at this point.' That applies with more force to a design worked out across a chat thread, which no future session can read. WHAT IT RECORDS, and why each part is not obvious from the tree: The incident chain in order, because each blocker MASKED the next and the record is otherwise unrecoverable -- privilege blocked materialization; repairing it exposed that install never activates; activating exposed that the pinned release was provider-deprecated. The privilege work was necessary and could never have been sufficient: it proved the host could run the installer, never that GitHub would accept what was installed. Three open defects with their receipts: membership compares only slot_dir, so a release bump repairs nothing that already exists -- the receipt is 29 slot directories deleted BY HAND to force re-provisioning, replacement being inducible only by turning a member into an absence. apply.sh carries enable count 0, start count 0. And ProviderAdmissibility was never modeled beside integrity and availability, so an exact digest proved we faithfully installed the wrong operational version. Serving vs restartable capacity as a carried COUNT rather than a caveat -- srv3 was ServingButNotRestartable and nothing could say so. The compatibility probe question, settled: the deprecation class is answered by a fresh incarnation reaching the broker's message endpoint, because the broker maps RunnerVersionTooOld to exactly the access-denied failure measured on srv3. A canary JOB is only needed to prove scheduler handoff, which is a different question and not what deprecated us. That distinction is the difference between a probe this repo can build and one it cannot. And two rules that are counter-intuitive enough to be worth writing: do not make 'latest' the authority, and do NOT restart the old runners first -- restart is precisely what converts latent old capacity into visible failure. This is a plan document, not a carrier. It claims nothing is implemented that is not, and its final section separates landed from built-not-landed from open. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Checked this one against the tree and it does not hold — the witness exists and executes.
It landed with the predicate in the same commit ( Rather than just assert that, here is the execution receipt for the stronger claim — that the wall discriminates rather than being permanently green. Mutation: point
So it is not prose with a function shape: an edit that makes the grantee its own bootstrap principal fails here. Working tree is clean — the mutation was reverted. I want to flag why this is worth more than a correction, because the finding names a real class I have been caught by twice on this PR. Both times the fix was to remove the unwired thing, so "authored but unconsumed" was the right prior to apply here. It just happens to be wrong on this specific symbol, and the way to tell the difference is execution rather than No change pushed for this item. — sent from warm-tern-755 |
srv3's converge apply refused at its first privileged command with zero slots mutated. The cause was two authorities answering one question.
The roster granted binary paths (
install,rm,systemctl,apt-get,visudo,tailscale) whileextdeps.github.actions_runnerindependently rebuilt an argv from those paths. Nothing joined them — so a grant for/usr/bin/installdid not imply authorization for the argv the script actually ran. The disagreement was discoverable only by a wet apply on a real host.The construction
gunbc.executor_privileged_operationdeclares the operation once — parameterized, not a nullary enum of binaries — and derives both renderings from one argv projection:executor_privileged_operation_argv..._sudoers_commandjoin(argv, " ")..._shell_commandsudo+ quoted argvA grant that fails to authorize its own command now has no spelling. Quoting is shell-level and invisible to sudo — which receives argv after the shell has parsed it — so one argv grounds both renderings without either knowing the other's escaping.
Deleted, not accumulated beside
ActionsRunnerPrivilegedBinariesandgunbc_runner_privileged_binaries()are gone. extdeps no longer rebuilds argv at all; it splices commands it is handed and stays agnostic. Net removal of a parallel rendering, per §2 dissolution-on-climb.Two narrowings (from the antagonist review)
run_asis nowroot, notALL.ALL=(ALL)authorized the grantee to run the command as any user, including other service accounts./opt/actions-runner/*matches..— fnmatch's*declines to cross/, and..contains none — so it would authorizerm -rf /opt/actions-runner/... Every managed path is named in full.Two modules, structurally
gunbc.runner_host_grantsjoins the operation authority to a host's slot roster. An operation module that also read the roster would close an import cycle withrunner_slot_provision, and acyclicity is the one structural law the import graph has.A §3 residual, with a trigger
The fleet-converge state directory was spelled three times in
fleet_converge_planand named nowhere: once as a bare literal in the lease guard, twice more as the leading segment of the generation store and lock paths. This declares it once; the row dissolves whenfleet_converge_planderives all three from it.Evidence
Six witnesses, green by execution, RED measured rather than asserted.
The load-bearing one is
witness_executed_command_is_the_granted_command: it reaches the command the provisioning script runs throughrunner_slot_provision's real seam, and the command the sudoers file grants through the renderer, and asserts they are one word vector.Mutation receipt: dropping the quoting from the shell projection returns
false; restoring returnstrue.🤖 Generated with Claude Code