Publish the remaining writes by rename instead of truncating (#841) - #843
Conversation
Four production writers still truncated their destination and wrote over it, so a crash or a concurrent reader could find a half-written file at a name that is supposed to hold a complete one. state.json is the worst of them. It is what bootroot reads back to know what it already did, so a torn write is not a stale record but no record at all: the next run fails to parse it and falls back to nothing. bootler staggers its two rotation units ten minutes apart because of this, which is a workaround in another repository standing in for a guarantee this function should provide itself. It now goes through fs_util::atomic_write_blocking and stays synchronous, so its callers are unchanged. The certificate writer is now the key writer beside it: both share one stage-then-rename core, so the mode and the policy's group ownership land while the file is still at its temporary path. The asymmetry that had the key staging and the certificate truncating is gone, and so is the umask's say in the published cert mode. The two init outputs, --summary-json and --root-token-output, already flushed their bytes but published them by truncating the destination, leaving the directory entry unflushed on a first write. They now stage and rename too, keeping the pre-write tightening of an existing destination: that guards the older credentials sitting at the path, which a fresh inode renamed over them does not. The durability question is decided per file and recorded at each site. state.json and the two init outputs are read back — to resume, or by the operator — so they take the directory flush. A certificate is reissued at the next renewal, which is the reasoning the key already records, so it declines the flush and says so in the same terms. Closes #841
The four writers the issue enumerated now stage and rename, but write_ca_bundle was still a plain fs::write over its destination, so the criterion that no production write site truncates in place did not hold. The bundle has the reader the certificate has: bootroot-agent re-reads it to rebuild its trust store while a rotation may be rewriting it, and fast-poll rewrites it on every apply. It now goes through the same publish_staged core as the key and the certificate, so the mode and the policy's group ownership land while the file is still at its temporary path, and it declines the directory flush for the reason they do — a bundle lost to a crash is rewritten by the next rotation. The manual still described the two init outputs in terms of the create-mode-then-chmod write they no longer perform, and named only the key as atomic where the certificate and the bundle now are. Both language versions are corrected; the durability decision is recorded there as well, since it is the operator who is told the file survives a power loss. The changelog entry had absorbed the opening sentence of the init-stdin entry below it, fusing two unrelated fixes into one paragraph, and claimed an existing destination's owner is preserved — which holds for the writers that go through atomic_write_blocking, but not for the certificate, whose rename re-owns the destination to the writer. A world-readable destination is now covered end to end for the token file: the tightening narrows the credential already sitting there and the staged publish gives the new one an inode that was never wider. Part of #841
The entry opened with "File modes are unchanged" and then explained that the modes used to be left to the umask, which cannot both be true. Certificates, CA bundles and the two init outputs did carry their modes before; state.json had none of its own, so on a host whose umask made it narrower than 0644 the next write now widens it. That is a change an operator can observe, so it is named rather than folded into a claim that nothing moved. Part of #841
The truncating write these two replaced opened the destination with O_TRUNC, which follows the final symlink, so an operator who pointed --root-token-output or --summary-json at a link had the file delivered to the link's target. Both preflights accept that on purpose: they resolve the link and judge the target's mode, and reject it only when the target is not a regular file. Staging and renaming broke it silently. The rename replaced the link itself, so the operator lost the link, the target kept the previous run's credentials, and the write reported success either way. The pre-write tightening made it worse by narrowing that stale target to 0600 on its way past. The destination is now resolved before it is tightened and staged, so the rename lands on the target exactly as the truncating write did. The preflight probe follows the same resolution: it is the directory the staging will use that has to accept a new file, and for a link into another directory that is the target's, not the link's. Probing the wrong one would pass the preflight and fail the write after OpenBao has already been wiped, which is the trap the probe exists to prevent. Resolution is not a security check and does not pretend to be. It follows whatever the link points at, so a caller whose destination an untrusted user can plant still has to refuse the symlink, the way atomic_rewrite_owned_no_symlink does for the secret_id path. Part of #841
`resolve_symlink_destination` handed a dangling symlink back unchanged, so the staged write renamed over the link itself: the operator lost the link and a freshly minted root token landed in the directory holding it rather than the one the link named. The truncating write this replaced followed the link on `O_CREAT` and created the target, and reproducing that is the whole point of resolving at all. `canonicalize` cannot say where a dangling link points, having nothing to resolve against, so the link text is read instead — absolute or relative to the link's own directory — and followed to the end of the chain. A cycle has no end, so the caller is handed back the path it named. The preflight gains the same fidelity for free: it probes the directory the resolution picks, which for a link into a directory that does not exist yet is now that directory, created before the destructive sequence rather than discovered after it. Part of #841
`publish_staged` takes its ownership from the `--cert-group` policy and nothing else, so the rename hands the certificate and the CA bundle to whoever ran the rotation where the truncating write they replaced kept the previous owner. That is the right answer for these three files — the gid they need is the one the policy names, and reading it back off the destination would let an owner the policy already replaced outlive it — but it is the opposite of what `fs_util::atomic_write_blocking` does a few hundred lines away, and nothing said so. A reader who found the two staging primitives had to infer which one preserves ownership from their bodies, and the wrong inference is the kind that only shows up as a daemon that can no longer read its own config. The comment names the decision and the reason the other primitive decides differently. The summary also still claimed the primitive was shared by the key and the certificate; the bundle joined them. Part of #841
The two behaviours the branch changed with no test of their own: the init summary shares the token writer's staging, tightening and symlink resolution, but was only covered through the token file beside it, and the two reinit preflights now resolve a link before probing without anything holding them to it. A probe that went back to the link's own directory would pass preflight and leave the write to fail after OpenBao has been wiped, which is the trap the check exists to prevent. Part of #841
|
[Reviewer Round 1] I found three compatibility problems that should be resolved before merging:
Minor documentation drift: src/commands/init/steps/orchestrator.rs:293 says the summary preflight only runs on reinit, but |
|
[Review Verdict Round 1: NOT_APPROVED] |
The shared cert, key and bundle publisher built its staging name from a &str, so a destination whose file name is not valid UTF-8 was refused outright. A Unix file name is bytes, and the writes this replaced never looked at them; the name is now built as an OsStr so the same paths keep working. Part of #841
Publishing through a staged temporary meant stating a mode, and a stated 0644 widened a state.json that an operator had narrowed by hand or that a restrictive umask had created narrow: the write this replaced opened the destination in place and left its mode alone. The mode is now read off the destination, and 0644 applies only where there is no file to read it from. The same write followed a symlinked path to its target, where a rename replaces the link itself, so the destination is resolved first as the two init outputs already do. Part of #841
Resolving a symlinked destination handed a cycle back to the caller unchanged, and the rename that followed replaced the operator's link with a regular file and reported success. The truncating write these replace answered ELOOP there. Resolution now fails instead, so both init preflights refuse the path before OpenBao is wiped rather than leaving the write to discover it afterwards. Also corrects the summary writer's note on where its preflight runs: init calls it too, and what the pre-write tightening covers is the window between that call and the write. Part of #841
|
[Author Round 1] All three items accepted; the third one I widened slightly. Pushed as [P1] Preserve failure for cyclic output symlinks — Fixed
Both preflights map the failure into their existing Tests, at all three levels rather than only the resolver: [P2] Do not widen an existing state file under a restrictive umask — Fixed
One half I did not restore: a While there: [P3] Keep non-UTF-8 destination filenames supported — Fixed
Documentation drift at
|
|
[Reviewer Round 2] The Round 1 items are resolved and are not carried forward: cyclic symlink destinations now fail before either preflight or writer can replace a link,
|
|
[Review Verdict Round 2: NOT_APPROVED] |
Publishing state.json now costs three disk round trips — the staged write, its flush, and the flush of the directory that names it — where the truncating write it replaced cost one. Every async command path was calling the synchronous writer directly, so those round trips ran on a Tokio runtime thread, which is what the issue's own constraint rules out: keep the writer synchronous, and wrap it at the call site the way the rotation-state writers do. Give StateFile an async entry point beside the synchronous one, sharing a blocking core: the JSON is serialized on the async side so only owned data crosses into spawn_blocking. The async production callers move to it, including the three helpers whose only production caller is async and which are now async themselves. `infra install` and `service update` run outside any runtime and keep the synchronous entry point, as do the tests, which must not need a runtime to write a state file. Part of #841
The certificate, the key and the CA bundle staged and renamed through cert_group's own implementation, which predates fs_util's and had grown a second copy of the staging create, the file flush, the chmod, the chown, a name-allocation retry loop and the rename. Two copies of a publish routine is one too many when both are load-bearing for whether a reader can see a torn file. Give fs_util a single staged publish and route both through it. The two axes the callers actually differ on become arguments: where the new inode's ownership comes from, and whether the directory entry is flushed. atomic_write_blocking is now that primitive with the destination's ownership and the flush; the cert group's writers are it with the --cert-group policy's gid and no flush. Each answer is documented where it is chosen, since the wrong inference surfaces as a daemon that cannot read a file it could read before. Two properties come free. The staged file is created 0600 and reaches its final mode only at the temporary name, so the guarantee #593 asked of the key file now holds for every caller; and the temporary's name is the primitive's own, so a destination whose file name is not valid UTF-8 needs nothing special of it. Part of #841
|
[Author Round 2] Both items accepted; the second one only partly as written. Pushed as [P1] Keep the synchronous state writer off Tokio runtime workers — Fixed
Every production call site reached from an async path moves to it — Coverage for the boundary: [P1] Use the required shared atomic-write primitive for the certificate — Partially addressedYou are right that there were two staging implementations and that one of them had to go. There now is one: What I did not do is put the cert on
So the primitive gained those two axes, narrowly — On the ownership change specifically — pushed back, with the reasoning now recorded in code and a test pinning it:
If you would still rather the cert preserve the destination uid, say so and I will make Docs and CI
|
|
[Reviewer Round 3] The two Round 2 items are resolved and are not carried forward: every production async
|
|
[Review Verdict Round 3: NOT_APPROVED] |
The four writers #841 enumerated left the acceptance criterion unmet: "no production write site in the crate truncates a destination in place" is crate-wide, and roughly twenty writers outside that list still opened their destination with O_TRUNC. They fall into three groups. Configuration with a live reader — agent.toml, .env, ca.json and its template, openbao.hcl, the responder config and template, the OpenBao Agent configs, the compose overrides — each had a process on the other side: docker compose interpolating .env on every invocation, step-ca parsing ca.json at boot, a sidecar re-rendering a template on a fixed interval, bootroot-agent re-reading agent.toml on every ACME retry. That last one is the sharpest: #613 moved service add off a truncating write because a reload landing in the gap reports "profile not found in reloaded config", and the three writers that edit the same file afterwards had kept it. Credentials are the second group. Those were a write followed by set_key_permissions, so besides the torn read they left the file at its final path under the umask's mode for the length of a chmod. Staging applies the mode to the temporary, so it holds from the moment the file appears. The sibling issue holds save_unseal_keys and eab::write_key_file back for exactly that window; both are untouched here, but a site being re-plumbed for the torn-read fix anyway does not get to keep it. The third group is init's rollback restore, which puts a snapshotted file back while the containers init started may still be reading it. Two wrappers name the durability decision at each site: atomic_write flushes the containing directory, atomic_replace renames and stops. The split is the issue's own rule — a file read back to resume, or holding a credential OpenBao will not hand out again, flushes; one regenerated by the next renewal, the next render, or a re-run of its command does not, because a disk round trip per write is real and init performs dozens. A staged temporary inherits no mode from the file it replaces, so a writer that used to truncate in place has to state one. preserved_mode reads it off the destination where there is one, keeping a file an operator narrowed by hand narrow across every later write, and falls back to a stated default only on a create. Part of #841
`update_dotenv_key` now costs three disk round trips where the truncating write cost one, and `init`'s database password rotation calls it from an async fn. A Tokio worker parked on those is a worker polling nothing else, and on a current-thread runtime it is the only worker there is. An async entry point beside the synchronous one moves the read, the rewrite and both flushes onto a blocking thread, the way `StateFile::save_async` does. The synchronous callers in `infra install` are unchanged; it runs outside any runtime. `Messages` gains `Clone` so the bundle can cross into the closure. Not `Copy`: the crate passes `&Messages` through several hundred signatures and `clippy::trivially_copy_pass_by_ref` would demand every one of them change. Part of #841
|
[Author Round 3]
|
|
[Reviewer Round 4] The Round 3 P1 is resolved and is not carried forward: the updated diff routes the remaining production truncating writers through the staging primitive, leaving only the two explicitly exempt secret writers and the reinit probes.
|
|
[Review Verdict Round 4: NOT_APPROVED] |
An fsync persists the inode as it stands. Taking it right after write_all, as all three staging paths did, left the chown and the chmod that follow it in memory only: a crash could recover a durably named, fully written file wearing the temporary's own 0600 and the writing process's primary group instead of the mode and the policy gid it was published with. The directory flush does not cover this — it makes the name durable and says nothing about the inode behind it. Part of #841
O_TRUNC follows a symlink at the final path component; rename replaces it. So every writer converted here from a truncating write silently changed what an operator's link means: the link is destroyed, and the file it pointed at keeps the previous contents while the write reports success. The configuration bootroot renders is written through the link again, on the reasoning state.json and the two init outputs already used — .env, ca.json and its template, openbao.hcl, the responder and OpenBao Agent configs, the compose overrides, the rollback restore. Two classes keep the bare rename, and now say why: a credential, where a link is a redirection vector and the reader reads the configured path anyway; and agent.toml and the cert beside the key, whose creating writer has published at the name since #613 and #593, so resolving one here would only make two writers of one file disagree. Part of #841
`rotate openbao-recovery --output` names a destination the operator chose, exactly as init's --summary-json and --root-token-output do, and the truncating write it replaced delivered through a link there. Renaming over the link would put the keys in a directory nobody picked while the target kept the superseded ones. The credentials that keep publishing at their own name are the ones inside the secrets tree, at paths bootroot chose; the distinction is who names the path, which the primitive's doc now says. Part of #841
Three comments and the manual still described the staging order the way it used to be. The order changed; the prose did not. Part of #841
|
[Author Round 4]
Also updated: Verification on |
|
[Author Round 5]
Verification: CI is green on |
|
[Reviewer Round 6] The Round 5 items are resolved and are not carried forward: remote bootstrap now uses
|
|
[Review Verdict Round 6: NOT_APPROVED] |
The new section opened by saying every file bootroot produces is published by stage-then-rename, and the three guarantees under it read as covering all of them. The OpenBao unseal-keys file and the service EAB credentials file still write in place, so an operator reading this would plan around a torn read and a permission window that are still there. Name the two and say the rest of the section is about the writers that do stage. Part of #841
|
[Author Round 6]
No code changed. |
CI failed executing a fake docker a test had just written: the kernel refuses to execute a file any process holds open for writing, and a fork for one thread's spawn duplicates the descriptor another thread is writing its own fake through, holding it until that child execs. The nine init step tests this branch moved onto self-contained fakes widened the window enough for it to land (rust-lang/rust#74214). Retrying at each spawn would not cover it, because production spawns some of these fakes on a test's behalf and must not grow a retry to serve the tests. Both writers now exec the fake once themselves, through a probe argument the script exits on before recording anything, and no writable descriptor to it can appear afterwards — so every later spawn, whoever makes it, succeeds. Part of #841
|
[Reviewer Round 7] The Round 6 P2 is resolved and is not carried forward: docs/en/cli.md:109 and docs/ko/cli.md:102 now scope the stage-then-rename guarantees to the staged writers and explicitly name
|
|
[Review Verdict Round 7: NOT_APPROVED] |
The previous commit waited out the ETXTBSY a fake executable can be refused with, by exec-probing it in a retry loop with a 5ms backoff. That is synchronisation by timing, which the project guidance rules out for tests, and it left every fake carrying a probe argument it had to recognise and exit on. The window can be closed instead of waited out. The kernel refuses to execute a file that any process holds open for writing, and what holds one here is a fork for some other thread's spawn duplicating the writer's descriptor until that child execs. A fork copies only the forking process's own descriptors, so handing the write to a child process means no descriptor on a fake is ever in this process's table to be copied: no fork can inherit one, and every spawn of the fake — a test's own, or one production makes on its behalf — succeeds. No sleep, no retry, no probe argument. Passing the destination as an argument rather than in the script text also drops the quoting question: a name holding a quote, a space or a byte that is not UTF-8 reaches the writer intact. Part of #841
|
[Author Round 7]
Full CI is green on |
|
[Reviewer Round 8] The Round 7 P2 is resolved and is not carried forward. The sleep/retry loop is gone, and moving fake-executable writes into a child process removes the parent-process descriptor that made an unrelated fork retain a writable executable; the author's scope rationale is sound.
|
|
[Review Verdict Round 8: NOT_APPROVED] |
The two rotate-side role_id backfills renamed without flushing, on the grounds that the file is re-readable from OpenBao, while every other AppRole credential file — including the role_id written beside a secret_id at init and at service add, and the one the override branch of this same function writes — flushed. That left one file name with two durability answers, and no way to state the contract in the manual that was true of both. A role_id is re-readable, but only on the next rotate run: until then a lost directory entry is an agent or sidecar that cannot log in, which is the same class of outage the flush is there to prevent. Both writers run only when the file is missing, so the round trip is not on any repeated path. Part of #841
|
[Author Round 8]
Verification on |
|
[Reviewer Round 9] The Round 8 P2 is resolved and is not carried forward. The two formerly unflushed rotation-side
|
|
[Review Verdict Round 9: NOT_APPROVED] |
The rustdoc on publish_staged_blocking said every production file bootroot publishes reaches disk through it, and so carries the no-torn-read and final-mode guarantees. Two writers do not: the unseal-keys file and eab.json still write over the destination in place and chmod afterwards, exactly as the manual already records. Naming them here keeps the implementation documentation from promising a property the crate does not yet have. Part of #841
|
[Author Round 9] Both items accepted;
|
|
[Reviewer Round 10]
The Round 9 test-plan item is resolved: the full-CI checkbox is now checked. |
|
[Review Verdict Round 10: NOT_APPROVED] |
The rustdoc on publish_staged_blocking called itself the one staging implementation in the crate and said every staged production file reaches disk through it. The override credential writers do not: they stage and rename a temporary of their own because they need ownership taken from the parent directory or read back through symlink_metadata, and a publish that refuses an existing name rather than replacing it. Those are production paths for a relocated role_id, secret_id and eab.json, not test helpers, and neither policy generalises to the files the shared publisher writes. Naming them keeps the claim true and records why the two implementations stay apart. Part of #841
|
[Author Round 10]
No production behaviour changes in this round; the change is documentation only. Head commit: |
|
[Reviewer Round 11]
|
|
[Review Verdict Round 11: NOT_APPROVED] |
The rustdoc on atomic_write_blocking still called its callee the routine every staged publish in the crate goes through. That is the claim the publisher's own rustdoc was corrected for: the override credential writers stage and rename a temporary of their own, because they need ownership taken from the parent directory and a publish that refuses an existing name. Pointing the wrapper at the scoped description instead keeps the two from disagreeing about which writers the guarantee covers. Part of #841
|
[Author Round 11]
Head is |
The comment justified the reload fallback by naming agent.toml writers that rewrite the file non-atomically, and both are now staged: bootroot-remote's apply moved to a rename earlier on this branch, and the fast-poll appliers were already there before it. Left as it was, it points a reader at a race no bootroot writer can still lose, and hides the two cases the fallback does cover — an operator editing the file in place, and a profile that service remove --strip-config genuinely deleted rather than momentarily hid. The fallback itself is unchanged. Part of #841
The enum said every staged publish has to choose between the two answers below it. The override credential writers stage a temporary of their own and take neither, deriving ownership from the parent directory or reading it back through symlink_metadata, so the claim described writers it does not govern. Same overreach the surrounding rustdoc was already narrowed for, in the one place that narrowing had not reached. Part of #841
2 participants
Closes #841
Summary
Every production writer in the crate that published its file by truncating the destination now stages a temporary in the same directory and
renames it into place, so a reader — anotherbootrootinvocation,bootroot-agent, a container mounting the file, or the operator — sees either the whole previous file or the whole new one, never a torn one. All of them, and the writers #805 already staged, go through one publish routine:fs_util::publish_staged_blocking— the exception being the override credential writers, which keep a staged publish of their own for the reasons given below.The four sites the issue enumerated:
StateFile::save(src/state.rs) goes throughfs_util::atomic_write_blockingand stays synchronous, so its synchronous callers are unchanged. It takes the directory flush: this is the filebootrootreads back to resume, so a torn write is not a stale record but no record at all. A doc comment records thatbootler's ten-minute stagger between its two rotation units is no longer load-bearing for this file — concurrent writers now see one version or the other. Nothing inbootleris touched; that is its own repository's follow-up. Async callers use theStateFile::save_asyncentry point beside it (below).cert_group::write_cert_filenow publishes exactly aswrite_key_filebeside it, with the mode and the--cert-grouppolicy's group ownership landing while the file is still at its temporary path. The asymmetry where the key staged and the certificate truncated is gone. It declines the directory flush, in the terms the key already used: a certificate lost to a crash is reissued at the next renewal.write_init_summary_jsonandwrite_root_token_file(src/commands/init/steps/orchestrator.rs) keep their pre-write tightening of an existing destination — that guards the older credentials already sitting at the path, which a fresh inode renamed over them does not — and gain staging and a rename around it. Both take the directory flush; they are written once duringinitand read by an operator afterwards.The rest of the crate
The issue makes "no production write site in the crate truncates a destination in place" an acceptance criterion, and the enumerated four do not satisfy it on their own. Every remaining production writer is converted here too. They fall into three groups.
agent.toml(service update's rerender,service remove --strip-config, andbootroot-remote's apply),.env,ca.jsonand itsOpenBaoAgent template,openbao.hcl, the HTTP-01 responder config and template, the twoOpenBaoAgentagent.hclfiles, and the four generated compose overrides. Each had a process on the other side of it —docker composeinterpolating.envon every invocation, step-ca parsingca.jsonat boot, a sidecar re-rendering a template on a fixed interval,bootroot-agentre-readingagent.tomlon every ACME retry. Theagent.tomlwriters are the sharpest case:service addwas moved off a truncating write by bootroot-agent burns renewal retries when reloaded agent.toml temporarily loses profile #613 precisely because a reload landing in the gap reports "profile not found in reloaded config", and the three writers that edit the same file afterwards had kept it.OpenBaoAgentrole_id/secret_idfiles, the servicesecret_id/role_idinside the secrets tree, the step-ca CA password, theOpenBaorecovery-key output, and the remote bootstrap artifact. These werewritefollowed byset_key_permissions, so besides the torn read they left the file at its final path under the umask's mode for the length of a chmod. Staging applies the mode to the temporary, so it holds from the moment the file appears. The issue's sibling holdssave_unseal_keysandeab::write_key_fileback for exactly this permission window; those two are untouched here as it directs, but a site being re-plumbed for the torn-read fix anyway does not get to keep the window.init's rollback restore (steps::rollback_file), which puts a snapshotted file back on the failure path while the containersinitstarted may still be reading it.grep -rn 'fs::write\|create(true)' src/, filtered to non-test code, now returns exactly four lines:eab::write_key_fileandsave_unseal_keys, held for the sibling issue, and the tworeinitpreflight probes, which create a uniquely named marker and delete it rather than publishing a file.One general-purpose publisher, four spellings
cert_grouphad its own staging-and-rename, predatingfs_util's and older than this issue: a second copy of the staging create, the file flush, the chmod, the chown, a name-allocation retry loop and the rename. Both are nowfs_util::publish_staged_blocking, with the two axes the callers actually differ on as arguments — where the new inode's ownership comes from (StagedOwner) and whether the directory entry is flushed (StagedDurability). Callers reach it through four wrappers rather than directly:atomic_writeatomic_replaceatomic_write_blockingatomic_replace_blockingatomic_write_through_symlinkatomic_replace_through_symlinkatomic_write_through_symlink_blockingatomic_replace_through_symlink_blockingcert_groupis the one caller that goes in directly, withStagedOwner::PolicyGroup.The override credential writers —
create_owned_credential_noclobber,write_owned_file_replaceandatomic_rewrite_owned_no_symlink, for arole_id,secret_idoreab.jsonrelocated into an operator-provisioned, agent-owned directory — keep a staged publish of their own. They already staged before this branch and reach the same two guarantees, but on ownership and clobbering they need what the shared publisher deliberately does not offer: the uid/gid comes from the parent directory or is read back throughsymlink_metadata, and a name already present is refused rather than replaced. Neither policy generalises to the files above, so folding them together would make one of the two callers wrong. The rustdoc onpublish_staged_blockingscopes its claim accordingly.Each site records which of the two it picked and why, because the wrong inference surfaces as a daemon that can no longer read a file it could read before, or as a disk round trip on every write in a loop that did not need one. The split follows the issue's own rule: a file the program reads back to resume, or that holds a credential the stack logs in with — losing one of those takes an operator or another rotation to put back, not the next write — flushes; a file that is regenerated by the next renewal, the next sidecar render, or a re-run of the command that produced it does not. Every
AppRolecredential file is on the flushed side,role_idincluded: it is re-readable fromOpenBao, but only on the nextrotaterun, and until then the agent or sidecar it belongs to cannot log in. Both backfills that write one run only when the file is missing, so that flush is not on any repeated path.Two properties come free from the shared routine — the staged file is created
0600and reaches its final mode only at the temporary name, so the guarantee #593 asked of the key file now holds for every caller, and the temporary's name is the primitive's own, so a destination whose file name is not valid UTF-8 (which a Unix path may be, and which the writes this replaced never looked at) needs nothing special of it.Two more pieces the enumerated four required:
write_ca_bundle(src/fs_util.rs) was still a plainfs::write. The bundle has the reader the certificate has — the agent re-reads it to rebuild its trust store while a rotation may be rewriting it — so it publishes the same way and makes the same durability decision.fs_util::resolve_symlink_destinationexists because the truncating write these replaced opened withO_TRUNC, which follows the final symlink, where a rename replaces it. See Symlinked destinations below.Symlinked destinations
O_TRUNCfollows a link at the final path component and delivers the bytes to its target;rename(2)replaces the name it is given. So a conversion done naively changes what an operator's link means — the link is destroyed and the file it pointed at keeps the previous contents, while the write reports success. Which answer each file takes is a decision, and every writer here states it:_through_symlinkwrappers, which resolve first and publish at the target. That is the configuration bootroot renders into its own tree —.env,ca.jsonand itsOpenBaoAgent template,openbao.hcl, the HTTP-01 responder config and template, the twoOpenBaoAgentagent.hclfiles, the compose overrides,init's rollback restore,state.json— and the output paths named on the command line:init --summary-json,init --root-token-output,rotate openbao-recovery --output, andbootroot-remote bootstrap'sagent.tomldestination. A dangling link is followed through its own text (canonicalizehas nothing to resolve against), matching whatO_CREATthrough the link used to do. A chain that loops back on itself resolves to nothing that can be published without destroying a link, so resolution fails with theELOOPthe truncating write reported.atomic_rewrite_owned_no_symlink, unchanged).agent.tomland the issued cert, key and bundle publish at the path too, because a writer that already renames over that name predates this branch:service::local_configsince bootroot-agent burns renewal retries when reloaded agent.toml temporarily loses profile #613,write_key_filesince Allow cert/key delivery to non-root container clients via configurable group ownership #593. A link an operator puts at one of those paths does not survive the command that creates the file, so resolving it in the writers that only edit it would leave two writers of one file disagreeing rather than preserve anything. Making the certificate match the key here is the same asymmetry-removal the issue asked for. Theagent.tomla bootstrap target carries is the other way round and sits in the first group:bootroot-remote bootstrapis the writer that creates the file there, so a link at its destination is one the operator arranged and one the truncating write it replaces followed.The two
initpreflights accept a symlinked destination on purpose: they resolve the link and judge the target's mode. The destination is resolved before it is tightened and staged, and the preflight probes the directory the staging will actually use, so a link into a not-yet-existing directory is caught before OpenBao is wiped rather than after; a cyclic destination is refused before the wipe rather than failing after it. Resolution is explicitly not a security check, and says so — a caller whose destination an untrusted user can plant still has to refuse the symlink.Keeping the flushes off the runtime
Publishing
state.jsonnow costs three disk round trips where the truncating write cost one, and every async command path was calling the synchronous writer directly.StateFilegains an async entry point beside the synchronous one —save_async, sharing a blocking core, serializing the JSON on the async side so only owned data crosses intospawn_blocking— and the async production callers move to it. Three helpers whose only production caller is async (write_state_file,reinit::write_minimal_state,service remove'sfinalize_removal) are async themselves for the same reason.infra installandservice updaterun outside any runtime and keep the synchronous entry point, as do the tests, which must not need a runtime to write a state file. This is the pattern #839 established for the rotation-state writers.Modes and ownership
File modes are unchanged —
CERT_FILE_MODEstays0644, the orchestrator pair stays0600— but each is now applied before the file is published rather than set after the bytes landed. A staged temporary inherits no mode from the file it replaces, so a writer that used to truncate in place has to state one.fs_util::preserved_modeis the shared answer: read the mode off the destination where there is one, so a file an operator narrowed by hand (or a restrictive umask created narrow) keeps that mode across every later write exactly as the in-place write left it, and fall back to a stated default only on a create. The default is0644forstate.json,.env,ca.json,openbao.hcland the compose overrides — what the umask produced — and0600for everything inside the secrets tree, where the policy's constant is the answer and a stale wider mode must not outlive the file it was attached to. A create on a host with a non-default umask is the only case that can observe a difference;CHANGELOG.mdnames it.The staged file is
fsynced after its ownership and mode are applied, not at the bytes. Anfsyncpersists the inode as it stands, so flushing first would leave the chown and the chmod in memory only: a crash could recover a durably named, fully written file wearing the temporary's own0600and the writer's primary group instead of the mode and the policy gid it was published with. The directory flush does not cover that — it makes the name durable and says nothing about the inode behind it.Ownership is the one place the callers answer differently, deliberately.
StagedOwner::Destinationcarries the destination's uid/gid across the rename: those files have no ownership policy to restate, and a re-owned one the daemon cannot read is an outage.StagedOwner::PolicyGrouptakes ownership from the--cert-grouppolicy instead, aswrite_key_filehas since #593: the gid these files need is the one the policy names, re-reading it off the destination would let a stale owner outlive the policy that replaced it, all three land world- or group-readable by that policy so no consumer loses access, and the rename needs only the directory's permission — a writer that could not replace the destination before is not made to fail on a chown it has no privilege for. A unit test pins both answers against the same seeded destination.Documentation
docs/en/cli.mdanddocs/ko/cli.mdgain a How bootroot writes files section: what stage-then-rename guarantees a reader, that the final mode holds from the moment the file appears, that a rename installs a new inode (so a single-file bind mount needs the container restarted where a directory mount does not), which files take the directory flush and which do not, and which paths keep being written through a symlink you put there and which replace it. The per-command prose that described the twoinitoutputs in terms of the create-mode-then-chmod write they no longer perform, and named only the key as atomic where the certificate and the bundle now are, is corrected. The section is scoped to the writers converted here: it names the two this change does not cover — the OpenBao unseal-keys file and the serviceeab.json, held for the sibling issue — and says the guarantees below it do not apply to them.CHANGELOG.mdnames the same two exclusions, so the release notes and the manual promise the same set.FastPollState::save'screate_dir_allchain stays out of scope, as the issue directs.Test plan
cargo clippy --all-targets -- -D warningsis cleancargo fmt -- --config group_imports=StdExternalCrateleaves no diffRUSTDOCFLAGS="-D warnings" cargo doc --no-deps --document-private-itemsis clean, and./scripts/check-docs.shbuilds the manual with--strictscripts/preflight/ci/check.shpasses end to end (fmt, clippy, rustdoc, Python, Biome, markdownlint, docs, audit)cargo testpasses (1041 bin + 424 lib tests green locally)grep -rn 'fs::write\|create(true)' src/, filtered to non-test code, returns onlyeab::write_key_file,save_unseal_keys, and the tworeinitpreflight probesStateFile::savepublishes a new inode over an existingstate.json, leaves no temporary behind, keeps an existing mode and lands at0644on a createStateFile::saveis still synchronous, andsave_asyncpublishes the same file — same inode replacement, same preserved mode, same symlink resolution — from a current-thread runtimesavereached from an async path goes throughsave_async; the two synchronous command paths (infra install,service update) are unchangedwrite_cert_file,write_key_fileandwrite_ca_bundleall publish by rename, with the policy's mode and gid applied before the renameStagedOwner::Destinationcarries a seeded gid across the rename andStagedOwner::PolicyGroupdoes not, so neither ownership answer can drift into the otheratomic_replaceinstalls a new inode, applies the requested mode, and leaves no staged sibling — the no-flush spelling is still a staged publishpreserved_modereads an existing destination's mode and falls back to the caller's default only on a createwrite_dotenvandupdate_dotenv_keyboth publish a new inode, leave no temporary, and keep an operator-narrowed.envat0600write_ca_bundlestill creates a missing parent directory before stagingwrite_init_summary_jsonandwrite_root_token_filepublish by rename at0600, and still tighten a pre-existing world-readable destination before writinginitoutputs write through a symlinked destination to the link's target, including a dangling link, and leave the link in place_through_symlinkwrappers deliver to a link's target and leave the link standing, create a dangling link's target, and refuse a cycle without touching the links in it; the bare wrappers publish at the name over the same seeded linkwrite_dotenvandupdate_dotenv_keyupdate the target of a symlinked.envand leave the link in placererender_local_managed_profilepublishesagent.tomlat its own name over a symlinked destination, matchingservice addbootroot-remote bootstrapwritesagent.tomlthrough a symlinked destination and leaves the operator's link in place, the opposite answer from the control node's writers of that file namevalidate_root_token_output_pathandvalidate_summary_json_output_pathprobe the directory the staged write will use, not the one holding the linkresolve_symlink_destinationpasses a regular file and a not-yet-existing path through unchanged, follows live and dangling links and chains, and fails on a cycle without touching the links in itinitwriters and both preflights refuse a cyclic destination, and leave the operator's links in placeEILSEQ, verified in arust:1-slimcontainer)AppRolerole_idandsecret_idwriter flushes the containing directory, so the durability contract the manuals state holds for the whole credential pair and not only half of itStateFile::save's comment states that concurrent writers now see one version or the other, sobootler's stagger is no longer load-bearingdockerexecutables the convertedinitandrotatetests write are written by a child process, so no descriptor on one is ever in this process's table for aforkto duplicate and no later spawn — production's included — can be refused withETXTBSY; the writer delivers the exact bytes at the exact name, including one holding a quote, a space or a byte that is not UTF-8 (the last verified in arust:1-slimcontainer, since APFS rejects it withEILSEQ)scripts/preflight/ci/e2e-matrix.shlocally, through the Round 3 tree: both lifecycles, the rotation/recovery matrix, reinit recovery, step-ca SANs and OpenBao TLS no-delta all pass. It stops atopenbao-tls-reown, which needs passwordlesssudothis macOS host does not have. Every commit since is covered by the same matrix on CI,openbao-tls-reownincluded, on the head commit below.39284d4(previously green on8f971c9) — all 18 checks, including everyDocker E2Ematrix job (local-hosts,local-no-hosts,remote-hosts,remote-no-hosts,rotation,reinit-recovery,stepca-san,two-instance,openbao-tls-no-delta,openbao-tls-reown)