Skip to content

fix(clusterplugin): write the cluster config through a safe open - #4873

Merged
mudler merged 1 commit into
kairos-io:masterfrom
ci-forks:fix/4867-clusterplugin-open
Oct 15, 2026
Merged

mudler merged 1 commit into
kairos-io:masterfrom
ci-forks:fix/4867-clusterplugin-open

Conversation

@ci-robbot

Copy link
Copy Markdown
Collaborator

Fixes #4867.

sdk/clusterplugin/plugin.go wrote the provider's cloud-config with
O_CREATE|O_WRONLY|O_TRUNC and nothing else, at a path that holds the cluster
token in cleartext and is rewritten as root on every boot.

What was missing What happened
MkdirAll on the parent a node whose cloud-config directory does not exist yet, or a cluster_config_path under a directory the image does not ship, failed the whole boot event on ENOENT
O_NOFOLLOW a symlink at that path redirected the root write, and O_TRUNC emptied its target
a mode check O_CREATE applies perm only when it creates the file, so a file somebody else had pre-created kept its own mode and received the token
O_NONBLOCK a FIFO parked the boot in open(2), with no writer ever arriving

The fix creates the parent, opens with O_NOFOLLOW|O_NONBLOCK, rejects
anything the fstat says is not a regular file, and sets the mode to 0600
after the open instead of trusting O_CREATE. Only the last path element is
protected by O_NOFOLLOW, so a symlinked cloud-config directory still works.

On the misleading error: all four returns past the JSON unmarshal said
failed to parse boot event. Each step now says what it was doing, and the
two errnos that mean "this path is not a file", ELOOP and ENXIO, name what is
in the way instead of surfacing a bare no such device or address.

Planting any of these needs write access to a persistent path, so this is
robustness rather than a privilege boundary, as the issue says. What it buys
is that the failure is reported, instead of being a truncated stranger's file,
a boot that never finishes, or a token anyone can read.

Tests

New suite, ten specs, in sdk/clusterplugin (the package had none). Each
guard is pinned by a spec that fails when only that guard is removed:

Mutation Spec that fails
remove the MkdirAll creates the directory the cluster config goes in
drop O_NOFOLLOW refuses a symlink at the config path, and leaves its target alone
drop O_NONBLOCK does not block on a named pipe nobody is reading (10s bound, so it fails instead of hanging the suite)
drop the IsRegular guard refuses a named pipe somebody is reading, instead of writing the token into it
drop the Chmod takes the mode down to 0600 on a file somebody else created
restore the blanket error text says what went wrong with the file, not that the event failed to parse

The symlink spec asserts the target keeps its contents, and the FIFO-with-a-reader
spec asserts nothing reached the pipe, so neither passes just because an error
came back.

CGO_ENABLED=0 go test ./sdk/clusterplugin/   ok  10/10 specs
CGO_ENABLED=0 golangci-lint run ./sdk/clusterplugin/...   0 issues

No boot test. clusterplugin has no consumer inside this repository; the
providers that import it live in their own repositories, and the contract they
use, ClusterPlugin.Run, is unchanged.

AI was used to write this change. No human has read it yet.

The cluster plugin wrote the provider's cloud-config with
O_CREATE|O_WRONLY|O_TRUNC and nothing else, at a path that holds the
cluster token in cleartext and is rewritten as root on every boot.

It never created the parent directory, so a node whose cloud-config
directory is not there yet, or a provider that points
cluster_config_path somewhere the image does not ship, failed the whole
boot event on ENOENT.

The open itself trusted the path. A symlink redirected the write and
O_TRUNC emptied whatever it pointed at. A FIFO parked the boot in
open(2) for good. A file somebody else had already created kept its own
mode, because O_CREATE applies perm only when it creates the file, and
then received the cluster token.

MkdirAll the parent, then open with O_NOFOLLOW and O_NONBLOCK, reject
anything that is not a regular file, and set the mode to 0600 after the
open rather than trusting O_CREATE to have done it. Only the last path
element is protected by O_NOFOLLOW, so a symlinked cloud-config
directory keeps working.

Every error past the JSON unmarshal in onBoot also said "failed to parse
boot event", which points the reader at the payload instead of at the
filesystem. Each step now says what it was doing, and the two errnos
that mean "the path is not a file" name what is in the way.

Fixes kairos-io#4867

Signed-off-by: Ettore Di Giacinto <mudler@kairos.io>
@ci-robbot

Copy link
Copy Markdown
Collaborator Author

Overlap note: #4870 (for #4864) edits the same onBoot block in sdk/clusterplugin/plugin.go, adding a MkdirAll of the parent directory, which this PR already does inside writeClusterConfig. Whichever merges first conflicts the other, so this one should rebase on top of #4870 and keep only the safe-open half if #4870 lands first.

@ci-robbot

Copy link
Copy Markdown
Collaborator Author

Picking this up for QA.

@ci-robbot

Copy link
Copy Markdown
Collaborator Author

QA Result: PASS

Check Result
Environment PR head d9b7f0fa, A/B against its base 170bfbb8 (no commits to sdk/clusterplugin on master since; merges cleanly, go test ./sdk/clusterplugin/ ok on the head). Node: kairos-hadron-v0.5.3-standard-amd64-generic-v4.3.0-209-g75c70ff06-k3sv1.36.4+k3s1 installed to disk, UEFI, QEMU/KVM, amd64
Reproduction Nothing in this repository calls ClusterPlugin; the callers are the external cluster providers. So I built a minimal provider shaped like provider-k3s (ClusterPlugin{Provider: ...} returning a yip boot stage that writes a marker) twice, from the PR and from the base, CGO_ENABLED=0, and installed it as /usr/local/system/providers/agent-provider-qacluster on the node. It was driven by real reboots (the initramfs stage kairos-agent notify agent.boot from 02_notify.yaml, boot_id checked to change) and by the same command run by hand for the symlink and FIFO cases, which would wedge a real boot on base
Expected Parent directory created; a pre-existing file ends up 0600; a symlink at the path is refused and its target is left alone; a FIFO is refused at once instead of hanging; the written config is still picked up by the boot stage
Actual See the table below. Every case reproduces on base and is fixed on the PR
Proofs proof.txt gist: provider hashes, ls -la, journal lines, exit codes
Case Base 170bfbb8 PR d9b7f0fa
Real boot, default /usr/local/cloud-config/cluster.kairos.yaml pre-created 0644 file stays -rw-r--r-- and receives the cluster token file becomes -rw-------; boot stage ran it (/run/qa-cluster-marker = token=qa-secret-token-4873 role=init)
Real boot, cluster_config_path: /usr/local/qa/bootdir/sub/cluster.yaml (parent missing) immucore: failed to run kairos-agent notify agent.boot: exit status 1, failed to parse boot event: open ...: no such file or directory, no file directory created, cluster.yaml written -rw-------, no provider error in the journal
Path is a symlink to a file holding precious data target overwritten with the cluster config (185 bytes) refusing to write the cluster config to .../link.yaml: it is a symlink, not a regular file; target still precious data (14 bytes)
Path is a FIFO kairos-agent notify agent.boot blocks, killed by timeout 20 (rc 124 after 20 s) rc 1 after 0 s: refusing to write the cluster config to /tmp/f.yaml: it is a named pipe, not a regular file

Summary: On an installed node, a cluster provider built with this change writes its config through a missing parent directory, tightens a pre-existing file to 0600, and refuses symlinks and FIFOs with clear errors. The base build truncates the symlink target, hangs on the FIFO, and fails the boot event when the parent directory is missing. The provider's yip config is still applied at boot.

Also seen, not part of this ticket: when a provider returns an error, kairos-agent notify exits 1, so a single provider error fails the whole initramfs notify step. That is existing behaviour of the bus (agent/internal/bus/bus.go) and is unchanged here.

@ci-robbot ci-robbot closed this Oct 1, 2026
@ci-robbot ci-robbot reopened this Oct 1, 2026
@github-project-automation github-project-automation Bot moved this from Done ✅ to Under review 🔍 in 🧙Issue tracking board Oct 1, 2026
@ci-robbot ci-robbot closed this Oct 10, 2026
@ci-robbot ci-robbot reopened this Oct 10, 2026
@ci-robbot

Copy link
Copy Markdown
Collaborator Author

This is approved and mergeable, and the red is not this branch. The failing checks are leftovers from the retired pull_request_target pipeline that #5269 removed on 2026-10-06 (authorize plus the skipped job placeholders, all dated 2026-10-01); they are never recomputed. The current pipeline ran green on this head on 2026-10-10, and the three Analyze legs are the repo-wide CodeQL toggle in #5364. Not merging master in on purpose, since the push would dismiss the approval.

@ci-robbot ci-robbot closed this Oct 14, 2026
@ci-robbot ci-robbot reopened this Oct 14, 2026
@ci-robbot ci-robbot closed this Oct 15, 2026
@ci-robbot ci-robbot reopened this Oct 15, 2026
@mudler
mudler merged commit 87bc051 into kairos-io:master Oct 15, 2026
80 of 87 checks passed

This branch was previously deployed

1 inactive deployment
fork-pr-builds — d9b7f0fa Deployed Oct 1, 2026 by ci-robbot via authorize #948
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

clusterplugin writes the cluster config through an unsafe open, and reports a misleading error

2 participants