Harden atomic config writes: createFileSync zero-byte window, symlink/bind-mount, mode preservation, and castConfigValue key mapping #1143

Summary

A cross-model review (Codex + Gemini) of the 4.7 backport of the atomic config-write fix (HarperFast/harperdb#3129) surfaced four config-write hardening items that are present in the current v5 line (5.1.0-beta.1), not just 4.7. None is a regression in the atomic-write fix (PR #493) itself — they're residual edges of writing config via temp-file + rename, plus one pre-existing comparison bug. Filing here so v5 and 4.7 stay aligned (fix here, then re-backport).

All locations are in config/configUtils.js.

Findings

1. createConfigFile reintroduces a zero-byte window on the create/install path — significant

createConfigFile calls fs.createFileSync(configFilePath) (L189) to create an empty file at the real config path before atomicWriteFile(configFilePath, …) (L200). Between the createFileSync and the rename, a concurrent reader (worker thread / config watcher) observes a 0-byte file — the exact rootPath/path.join(undefined, …) failure the atomic write was meant to eliminate, just on the install path instead of the update path.

• Fix: drop the redundant fs.createFileSync(configFilePath) (the temp-file write + rename already creates the target atomically), or only ensureDirSync the parent directory.

2. atomicWriteFile breaks symlinked / file-bind-mounted config — significant

fs.renameSync(tempPath, filePath) (L103) replaces the path entry itself. If harperdb-config.yaml is a symlink (K8s ConfigMap, Docker secret) or a file bind-mount, the rename replaces the symlink with a regular file (severing the live-update binding) or fails with EXDEV/EBUSY where the previous in-place fs.writeFileSync would have written through.

• Fix: resolve fs.realpathSync(filePath) (when it exists) and write/rename against the resolved target; or explicitly document that symlinked/bind-mounted config files are unsupported. Worth a Fabric/Docker deployment gut-check on how the config file is actually mounted.

3. atomicWriteFile does not preserve target mode/owner — significant

The temp file is created with the process umask (typically 0644). After rename it replaces the original, so an admin's chmod 600 (secret protection) is widened to 0644, ownership can change if the writer runs under a different uid, and a 0444 "do not mutate" guard is bypassed (rename only needs directory write permission).

• Fix: fs.statSync the existing target and apply its mode (and owner where applicable) to the temp file before the rename.

4. updateConfigValue skip-if-unchanged compares the raw arg, not the mapped config key — significant

The equality check casts and looks up the raw arg:

const castedValue = castConfigValue(arg, parsedArgs[arg]);          // configUtils.js:554
if (!_.isEqual(castedValue, flatConfigObj[arg.toLowerCase()])) { … }

castConfigValue's string-preservation branch is keyed on exact CONFIG_PARAMS names (e.g. clustering_nodeName). Passing the unmapped arg (e.g. clustering_nodename) misses that branch, so CLUSTERING_NODENAME=1 casts "1" → number 1, _.isEqual(1, "1") is false, and the config is rewritten on every boot even though the write path maps to clustering_nodeName and writes "1" back. Aliases (e.g. LOG_TO_STDSTREAMS) likewise look up flatConfigObj.log_to_stdstreams instead of logging_stdstreams and always rewrite. This defeats the "skip if unchanged" goal added in 4a68092b for the affected params.

• Fix: resolve the mapped key first and use it for both cast and lookup, matching the write path: const configParam = CONFIG_PARAM_MAP[arg.toLowerCase()] ?? arg; then castConfigValue(configParam, …) and flatConfigObj[configParam.toLowerCase()].

Notes

• Add issue templates #1 and Default to attempting to serve index.html for a path #4 are corroborated, low-risk correctness fixes. [WIP] Enable Node.js Type Stripping #2 and Commit changes to package-lock.json from running npm install #3 are robustness gaps inherent to atomic-write-via-rename; [WIP] Enable Node.js Type Stripping #2 needs a deployment decision (how is the config mounted in Fabric/Docker?).
• Related: PR fix: write Harper config files atomically #493 (atomic write), 1c73a1c5 (unique temp path), 4a68092b (env cast), and the EPERM/EACCES retry + temp-cleanup follow-up now in atomicWriteFile.
• Surfaced during: HarperFast/harperdb#3129 (4.7 backport).

🤖 Generated with Claude Code