Repository navigation
Conversation
… creates it The fs.after stage that should create and harden /usr/local/cloud-config was guarded on the directory being absent, and kairos-agent's runstage() calls MkdirAll on every cloud-init path at the top of the same call that then runs the three fs stages. The guard was therefore always false and neither the mkdir nor the chmod ever ran. DirPerm is 0777 and MkdirAll applies the umask, so the directory is created 0755 root:root. 10_accounting.yaml is what normally corrects that, but it runs in initramfs, earlier in the same boot, and it is guarded on the directory already existing. On the first boot after a UKI install the directory does not exist yet, so both guards are false and the directory stays 0755 for the boot on which a provider writes cluster.kairos.yaml into it. Drop the unreachable guard, make the commands idempotent, and set the owner and mode that 10_accounting.yaml already establishes, so the first boot matches every later one. Not the 0600 the dead stage asked for: a directory without the execute bit cannot be traversed, which would take away the access the admin group is deliberately granted. Fixes kairos-io#4874 Signed-off-by: Ettore Di Giacinto <mudler@kairos.io>
Picking this up for review. |
itxaka-agent
left a comment
There was a problem hiding this comment.
Automated triage agent (
kairos-triage-agent) running as@itxaka-agent.
This comment was generated by software. A human maintainer can override any
action taken here.
Approve. Traced the dead guard in 00_rootfs.yaml against runstage.go's cloud-init path setup and confirmed it can never run — MkdirAll on /usr/local/cloud-config happens before the fs.after stage is evaluated, so [ ! -d ... ] is always false. Confirmed 10_accounting.yaml's self-heal only applies once the directory already exists, which is why the first UKI boot stays 0755. The new test in cloud_config_dir_test.go fails against the old guard and passes against the new one. No QEMU boot run is attached for this boot-path change — worth doing before merge, but not a blocker from a static read.
Audit trail: round 0, verdict approve. Review: #4875 (review) |
The problem
00_rootfs.yamlhas anfs.afterstage whose job is to create/usr/local/cloud-configand take the world bits off it. It can never run.agent/pkg/utils/runstage.gocallsMkdirAllon every entry ofconstants.GetCloudInitPaths(), which includes/usr/local/cloud-config/, atthe top of the same function call that then runs
fs.before,fsandfs.after. So[ ! -d /usr/local/cloud-config ]is always false by the timethe stage is evaluated. Same process, a few lines apart, not a race.
constants.DirPermisos.ModeDir | os.ModePermandos.MkdirAllapplies theumask. No Kairos unit sets
UMask=, so the directory is created 0755root:root.
10_accounting.yamlis what normally corrects that, toroot:admin0770. Itruns in
initramfs, earlier in the same boot, and it is guarded on thedirectory already existing. On the first boot after a UKI install the directory
does not exist yet, which is what #4864 reports, so both guards are false, and
the directory stays 0755 for the whole boot. That is the boot on which a
provider writes
cluster.kairos.yamlinto it. From the second boot theaccounting stage finds it and hardens it, which is why this self-heals and has
gone unnoticed.
The change
Drop the unreachable guard, make the three commands idempotent, and set the
owner and mode that
10_accounting.yamlalready establishes, so the boot thatcreates the directory matches every later one.
Not the
chmod 600the dead stage asked for. 0600 on a directory clears theexecute bit, so nothing but root could traverse it, and that takes away the
access the admin group is deliberately granted. Same shape as #4707. Anyone
repairing the guard without reading the mode would have broken the boot.
If an image has no
admingroup thechownfails and thechmodstillapplies, leaving root:root 0770, which is no weaker than what is asked for.
Verification
kairos-init/pkg/bundledread the shipped stage out ofEmbeddedConfigs, evaluate its guard and run its commands against atemporary directory pre-created the way
runstage()leaves it (0755). Theyassert the resulting mode, that the directory stays traversable by its group,
that it is still created when missing, and that running it twice is stable.
A reintroduced
[ ! -d ... ]guard fails them.master's00_rootfs.yaml:the stage ran but left /tmp/.../cloud-config at 0755, three specs red. Green with the change.well, not only through a shell, and it leaves the directory at 0770.
go test ./kairos-init/...passes.Not booted. This is a boot-path cloud-config and unit tests cannot prove a boot
works, so the per-boot ordering claims above come from reading
runstage.goand the stage list, not from a VM.
Scope
Only
00_rootfs.yamlis touched. #4872 changes the guards in10_accounting.yamland #4870 creates the directory on the UKI install path;neither overlaps with this file.
AI disclosure
Written by an AI agent. No human read the code before this was opened.
Fixes #4874