Repository navigation
spark serving: observe the managed account and its key-file mode, before authoring any row about it - #8817
Merged
Conversation
…e from the host (#8377 marker) The 19:19 serving observation carried no account field of any kind. Every fact about the managed executor account -- that it exists, what uid/gid it holds, what shell it has, and whether its authorized_keys file is readable by anyone but its owner -- was unobserved, so any fleet_posix_accounts row authored today would have been authored from inference. This extends the observation instead. SparkManagedAccountObservation is four states, not an Optional: ManagedAccountObserved, ManagedAccountKeyFileUnread (the account is there, the key file is not readable), ManagedAccountAbsent, and ManagedAccountProbeRefused. An account that does not exist and a key file we could not read have different remedies; an Optional would collapse them. The file mode is read with `ls -ld`, not `stat -c`. That is not a style choice: portable_remote_word admits only [A-Za-z0-9._-/@:=,], so no stat format string can ever be a portable word -- `%` is not in the allowlist. Two wet runs established it, and the second (`%a:%U:%G`, no spaces, refused identically) is what proved my first diagnosis wrong. Symbolic mode is the better carrier here anyway: the property this exists to record is that the key file is not group- or world-readable, which `-rw-------` states directly. Also repaired in passing, both found by making this change: - spark_serving_observe_host declared -> SparkServingObservation while returning a SparkServingHostObservation. The declared return was a lie the typechecker accepted. - spark_serving_observe_ci_with_context carried a second, hand-copied per-host render join. Adding the account line to one of them left the other silently short. Extracted spark_serving_host_observation_lines so both read through one authority rather than adding the line twice. WET RECEIPT (2026-08-21, both converged hosts, unchanged 2/2): account host=srv5 login=gunbc-automation uid=1001 gid=1001 shell=/bin/bash authorized_keys_mode=-rw------- authorized_keys_owner=gunbc-automation account host=srv6 login=gunbc-automation uid=1001 gid=1001 shell=/bin/bash authorized_keys_mode=-rw------- authorized_keys_owner=gunbc-automation Authoring the fleet_posix_accounts rows FROM this receipt is the follow-up, deliberately not in this PR: the observation has to exist before rows can cite it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…decoder (review 54471)
spark_serving_account_field returned "" for a missing index and the caller cast that
into NonEmptyStr. Two things were wrong and only one is about types. The carrier exists
to make an empty value unrepresentable, so laundering one through `as NonEmptyStr`
inverts the rung it holds -- and downstream, a truncated getent row and a well-formed
one both arrived as ManagedAccountObserved with no way to tell them apart. That is the
fabricated-plausible-output failure, inside a decoder written to be honest about partial
states.
Worse: the test asserted it. a_truncated_passwd_row_decodes_without_crashing checked
`(u as String) == ""` sitting inside ManagedAccountObserved -- a test written to prove
robustness was the thing pinning the laundered carrier in place, and is why this reached
review at all.
CONSTRUCTION, NOT A CHECK
- the field reader returns String?; missing and empty are both Absent (for these four
columns an empty column is exactly as unusable as an absent one, and separate states
would be a distinction with no difference in remedy)
- SparkPasswdRow carries four proven-non-empty fields; decoding lands there or not at
all, so no consumer downstream is holding a checked value rather than a proven one
- SparkKeyFileFacts likewise for mode and owner
- ManagedAccountRowMalformed is the one new variant. Undecodable `ls -ld` output does
NOT get a second one: it is a key file we could not read, which ManagedAccountKeyFileUnread
already says, and minting a variant there would be the no-remedy-difference split the
passwd case genuinely does have
FOUR TESTS REPLACE THE ONE THAT CODIFIED THE BUG, paired so neither half passes vacuously:
truncated row refuses; well-formed row still observes (differing by exactly one thing);
all-columns-present-but-blank-shell refuses too, closing the other route to an empty
NonEmptyStr; undecodable key output keeps the account and reports unread.
Also fixed: the KeyFileUnread cause string still read "stat exit=" after the probe moved
to `ls -ld`, which would have misdirected anyone reading a wet receipt.
WET RECEIPT after the rework (2026-08-21, both converged hosts, unchanged 2/2):
account host=srv5 login=gunbc-automation uid=1001 gid=1001 shell=/bin/bash
authorized_keys_mode=-rw------- authorized_keys_owner=gunbc-automation
account host=srv6 (identical)
observation host=srv5 state=observed digest=d3f70212498a072f
observation host=srv6 state=observed digest=d3f70212498a072f
Re-run wet rather than typechecked, because the change is on the live decode path and a
typecheck is not a consumer.
Co-Authored-By: Claude Opus 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.
Marker #6 of the #8377 spark-serving
dissolve-onset. The parent lane's ruling on this one wasexplicit and is what shaped the PR: observe first. Authoring
fleet_posix_accountsrows frominference is the exact error that got a module deleted two PRs ago (#8811), so this PR extends the
observation and stops there. The rows are the follow-up, authored from the receipt below and citing
it.
What was unobserved
The 19:19 serving observation had no account field of any kind. Not a partial one -- none. So
every fact a
fleet_posix_accountsrow would assert was, as of this morning, unmeasured on theconverged hosts.
The four states
SparkManagedAccountObservationis a coproduct, not anOptional:ManagedAccountObservedManagedAccountKeyFileUnreadManagedAccountAbsentgetent passwdfound nothingManagedAccountProbeRefused"The account does not exist" and "we could not read its key file" have different remedies. An
Optionalcollapses them, and the second one matters more than a missing field: sshd refuses keyson an over-permissive file, so an account that exists with a world-readable
authorized_keysisnon-functional in a way that looks like a present account.
Why
ls -ldand notstat -cNot a style choice.
portable_remote_wordadmits only[A-Za-z0-9._-/@:=,], and%is not in thatset -- no
statformat string can ever be a portable word through this transport. I got thiswrong first: I blamed the spaces in
%a %U %Gand respelled it%a:%U:%G, which was refusedidentically. The second refusal is what falsified the first diagnosis; reading the allowlist rather
than pattern-matching the error is what produced the real one.
Symbolic mode is the better carrier here regardless. The property this observation exists to record
is the key file is not group- or world-readable, which
-rw-------states directly and600states by convention.
Two defects found by making the change
Both were pre-existing, and neither is cosmetic:
spark_serving_observe_hostdeclared-> SparkServingObservationwhile returning aSparkServingHostObservation. The typechecker accepted the lie.spark_serving_observe_ci_with_contextcarried a second, hand-copied per-host render join.Adding the account line to one left the other silently short -- the account line was simply absent
from the wet receipt while everything typechecked. Fixed by extracting
spark_serving_host_observation_linesso both read through one authority. Duplicating theaddition would have been the §3 fork; killing the fork is what the receipt then proved.
Wet receipt (2026-08-21, both converged hosts)
Convergence is unchanged at 2/2 -- both legs are reads (
getent passwd,ls -ld); neitheractuates.
Discriminating RED
the_passwd_fields_are_read_at_their_real_positionsasserts uid/gid/shell come from their actualgetentcolumn indices. It goes red on any off-by-one in the field split, which is the failure thisparser can plausibly have and the one a shape-only test would pass over.
🤖 Generated with Claude Code