config: atomic config replacement does not sync file data or parent directory

[PierrunoYT]

Version / branch / commit

Source-reviewed on main at 99721c7.

OS and environment

Linux amd64; Go 1.26.6. AI-assisted source review. No power-loss test was performed; exact failure behavior depends on the OS/filesystem and its write-ordering guarantees.

Steps to reproduce / validation needed

1. Use a disposable filesystem/VM and perform a config mutation that reaches writeConfigData.
2. Observe the write/close/rename syscall sequence; no file or directory sync is issued by this function.
3. For an end-to-end durability reproduction, inject a simulated crash/power loss after successful return but before writeback, then remount and inspect the config.

Step 3 is proposed validation, not an executed reproduction. This report establishes the missing durability barriers from the source, not a guaranteed corruption outcome on every filesystem.

Expected behavior

If a successful config update is intended to survive a system crash, persist the temporary file before replacement and persist the directory update where supported. Otherwise document that success provides atomic visibility, not crash durability.

Actual behavior / source evidence

writeConfigData:976-1008 writes a sibling temporary file, closes it and renames it over the destination. It does not call tmp.Sync or sync the parent directory before returning success.

The current code correctly avoids an ordinary partial in-place overwrite. However, acknowledgement can precede durable storage of the replacement. An update may be lost after a system crash; stronger corruption claims require filesystem-specific testing.

Suggested fix / regression coverage

Sync the temporary file before close/rename, then sync the parent directory where supported. Propagate failures and define platform-specific behavior. Add fault-injection coverage for sync failures and ordering; use a disposable crash-consistency harness for the end-to-end guarantee.

Related work

Merged #631 addresses securefile and credstore, not config.writeConfigData. #960 addresses cross-process read-modify-write locking, which is a different property.

Verification

Existing go vet ./... and go test ./... passed during the audit. No crash-durability test was run. This is a low-priority durability gap, not evidence of corruption during ordinary successful writes.

[Vasanthdev2004]

Confirmed on main: writeConfigData writes the temp file, closes it and renames it over the config, with no sync on the file or the directory. A successful return promises atomic visibility, not that the new config survives a crash.

Approving the narrow fix you describe, in the shape internal/sessions/store.go already uses: tmp.Sync() before the close, then the parent directory synced after the rename, with both failures returned. That store's syncDir skips Windows, where a directory handle can't be synced, and this should do the same rather than fail there. A seam that makes each sync fail is enough coverage. I wouldn't ask for a crash harness.