feat(grants): route every privilege grant through one validated path - #602
Merged
Merged
Conversation
A sudoers grant is the most consequential file this harness writes, and it was
written two different ways.
lib/systems-capabilities.sh had a correct installer -- validate a temp file,
move it into place only once visudo accepts it -- but its only consumer was
retired and it was left with zero call sites. The next component that needed a
grant did not find it and wrote its own, which installed at the live path and
validated afterwards:
printf '%s\n' "$content" > /etc/sudoers.d/name
visudo -cf /etc/sudoers.d/name
Under set -e a rejected policy aborts the run and stays on disk, and an invalid
file in /etc/sudoers.d makes sudo refuse to run for every user on the host,
including the operator's recovery path. The duplicate was not just redundant,
it was the dangerous one.
lib/grants.sh is now the single path: declare, validate in isolation, install
atomically, compare content and ownership separately so drift reports which one
moved. The kimaki bridge routes through it and its private writer is deleted.
verify.sh gains the seam nobody owned. It never read /etc/sudoers.d, so a grant
that was hand-placed, widened, or left group-writable looked exactly like one
installed correctly. It now asserts root:root 0440 and valid policy syntax on
every grant, and reports the ones this harness does not install -- which on the
first host checked was four of five.
Also fixes a hardcoded GNU stat -c in tests/agent-state-ownership.sh that broke
the suite on macOS, using the portable helper the repo already has.
Refs #601
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.
Implements #601.
The bug this removes
lib/systems-capabilities.shcontained a correct sudoers installer — validate a temp file,mvinto place only oncevisudoaccepts it — with zero call sites, orphaned when its only consumer was retired. The next component that needed a grant didn't find it and wrote its own (bridges/kimaki.sh:537), which installed at the live path and validated afterwards:setup.shandupgrade.shboth run underset -e, so a rejected policy aborts the run and stays on disk. An invalid file in/etc/sudoers.dmakessudorefuse to run for every user on the host — including the operator's recovery path. Two implementations of one thing, and the one in service was the one that could lock you out of the box.What replaces it
lib/grants.sh— the single path a grant reaches a host. Declare, validate in isolation, install atomically.grant_declarerecords without touching the host, so a declaration is readable before anything is installed. Re-declaring replaces rather than appends.grant_validaterunsvisudoagainst a temp file thatsudonever reads.grant_installvalidates first, then replaces atomically. The live path only ever holds a policyvisudoalready accepted.grant_content_matchesandgrant_ownership_okare separate, so drift reports which invariant moved. A grant with the right words and the wrong mode is not a correct grant —sudoignores a group-writable policy file.grant_undeclared_filesis the census.The kimaki bridge routes through it; its private writer is deleted.
systems_capabilities_install_sudoersis deleted — this is its logic, extracted to where a second caller could find it.Library functions
warnand return rather than callingerror, whichexits. Whether a refused grant is fatal belongs to the caller — the kimaki bridge treats it as fatal, a census or dry run does not — and a library that exits takes that choice away. (Found by writing the test: the first version callederrorand killed the test run mid-suite.)verify.sh gains the seam nobody owned
verify.shnever read/etc/sudoers.d, so a grant that was hand-placed, widened, or left group-writable looked exactly like one installed correctly. It now assertsroot:root 0440and valid policy syntax on every grant, and reports which ones this harness does not install.Run against the production agent host, unmodified:
Four of five, named — including
homeboy-edge-apply, which I placed by hand last night and which has exactly the defect #601 describes. The census is deliberately a note, not a failure: an operator-placed grant is not wrong, and failing every existing host on day one would get this check disabled rather than acted on. Declaring those grants is follow-up work.Tests
tests/privilege-grants.sh, 22 assertions, wired into CI. The ones that matter are negative:"An existing valid grant survives a rejected update" is the regression that matters: a host with a working grant, and an upgrade that computes a broken one.
Full affected suite on Linux — 14 suites, all passing, including
service-identity-adoptionwhich asserts the generated sudoers content byte-for-byte, so the refactor is byte-compatible:Also fixes a hardcoded GNU
stat -cintests/agent-state-ownership.shthat broke that suite on macOS, usingfile_mode— the portable helper this repo already has.Not in scope
Declaring the three hand-placed grants, which is the other half of #601 and a behaviour change to what gets installed on existing hosts. This PR makes that possible and measurable; it does not grant anything new.
AI assistance: implemented by Claude Opus 4.5 via Claude Code, driven by Chris. The model traced the dead call site through git history, found the install-before-validate ordering, wrote the library, tests, and verify seam, and ran the affected suites on a Linux host plus the new verify section against the production agent host.