fix(config): make a settings save durable, and keep it private

[frahlg]

The bug

config.yaml is the file the gateway boots from. Every settings save from the
UI rewrites it, and the writer did this:

tmp := path + ".tmp"
if err := os.WriteFile(tmp, data, 0644); err != nil { ... }
return os.Rename(tmp, path)

No fsync of the temp file, no fsync of the directory. A rename is only atomic
for bytes that have already reached the disk, so a power cut between the write
and the flush can publish a truncated or zero-length config.yaml — on a
device whose whole point is coming back up unattended. The directory fsync
matters for the same reason one step up: without it the rename itself can be
lost, leaving the old config and a stray .tmp.

Every other durable write in the repo already does this correctly —
state/parquet.go (file sync, rename, syncDir), driverrepo's atomic
activation, nova/identity, state/homelink_credentials. Config was the one
hole.

Second bug: the temp file was created 0644 and the rename carried that mode
onto a file holding MQTT passwords, API keys and OAuth refresh tokens.

The fix

SaveAtomic now uses the ordering the rest of the repo uses: create the temp
0600, write, fsync the file, rename, fsync the directory.

Three details worth review:

• Both sync failures are reported, not swallowed. The caller's contract is
  "the config is now saved". A save that could not be made durable has not met
  it, and the settings UI should not tell the operator otherwise.
• O_EXCL after clearing a stale temp. OpenFile only applies the mode
  when it creates the file. If a previous save was interrupted and left a
  0644 temp behind, reusing it would have carried 0644 through the rename
  and leaked the secrets anyway. Clearing first and creating exclusively also
  means the write refuses to follow a symlink planted at that path.
• Saves are serialized. The settings handlers hold no write lock across a
  save (CfgMu is only RLocked to snapshot the old config for the diff), and
  all savers share one temp path. Two overlapping POST /api/config requests
  could rename half of each other's bytes over config.yaml. A package-level
  mutex is the cheap fix and keeps the single predictable temp name — the
  alternative, os.CreateTemp, scatters a fresh secret-bearing file per
  interrupted save.

config.SaveAtomic is the only writer of config.yaml: bootstrap.go,
nova_claim.go and the API's injected SaveConfig seam all route through it.
The updater's rollback path stages snapshot files itself but already uses
0600 (go/cmd/ftw-updater/main.go:1055, :1084), so nothing bypasses this.

On the test seam

The two syncs live in a small durableWriter struct rather than being called
directly, and SaveAtomic delegates to an unexported saveAtomic(w, path, c).
That is the least ceremony I could find that lets a test assert what actually
matters here: that the file sync happens before the rename and the
directory sync after it, and that either failure fails the save. Asserting
"a save works" cannot distinguish the fixed code from the broken code — the
broken code also produced a correct file on a machine that does not lose power.
Production always uses defaultDurableWriter; nothing else assigns those
fields.

Tests

Three new tests beside the existing SaveAtomic ones, each verified to fail
against a mutation of the fix:

Test	Mutation it catches
TestSaveAtomicWritesOwnerOnlyMode (3 cases: new file, replacing a 0644 config, stale 0644 temp)	mode back to 0644 → all three fail
TestSaveAtomicSyncsFileBeforeRenameAndDirAfter	drop the file fsync → sync order = [dir]; move the dir fsync before the rename → "the directory was fsynced before the rename"
TestSaveAtomicReportsSyncFailure (file sync fails, dir sync fails)	_ = w.syncFile(f) → "error = , want it to report no space left on device"

The file-sync-fails case also asserts the previous config is still on disk
untouched, and both cases assert no temp file survives.

Operator-visible change

After the first save through the UI, config.yaml becomes 0600 owned by the
process user (UID 100 in the container). An operator who created the file by
hand as a different user and reads it without sudo will notice. That is the
intended trade — the file holds credentials.

Contention

Branched from master. #732 and #736 touch config.go in the schema
section only; neither diff mentions SaveAtomic, and neither touches
config_test.go. No open PR touches go/internal/config/config.go's writer.

make verify clean (run in an isolated worktree — the shared checkout has an
unrelated agent's in-flight pvmodel build break in it).

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ba47477a8f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Apply an owner-only ACL on Windows

On the supported Windows build shipped by make release (Makefile:179-190), passing 0o600 to os.OpenFile does not establish Unix-style owner permissions: Go only maps the write bit to the Windows read-only attribute, and the new file inherits its directory ACL. Thus, when config.yaml is stored in a directory readable by another local account, the MQTT passwords, API keys, and refresh tokens remain readable despite this privacy fix (and the new mode assertion would report 0666 on Windows). Use a Windows-specific ACL implementation, or otherwise create the file with an owner-only security descriptor.

Useful? React with 👍 / 👎.