fix(selfhost): make env_put's .env write atomic (mv) while preserving the target file's mode - #7835
Conversation
… the target file's mode env_put() in scripts/lib/selfhost-deploy-common.sh created a same-directory temp file specifically (its own comment justified this as enabling an atomic swap), but then wrote via `cat "$tmp" >"$file"; rm -f "$tmp"` — a truncate-then-copy, not a rename. A crash/kill/power-loss mid-write (this runs during self-host deploys, e.g. `env_put LOOPOVER_IMAGE "$IMAGE"`) can leave .env truncated/corrupted. Swap to an atomic `mv "$tmp" "$file"`, mirroring the same-directory-temp-file + mv idiom already used in backup-metrics.sh / browserless-metrics.sh / export-ams-reporting-db.sh. Caveat handled: mktemp creates $tmp at 0600, so a bare mv would silently narrow .env's permissions on every write — capture the target's existing mode (GNU `stat -c '%a'` with a BSD `stat -f '%Lp'` fallback, matching backup-metrics.sh's stat-portability idiom) and chmod $tmp to match before the mv. Adds env_put tests to test/unit/selfhost-deploy-common.test.ts covering in-place update, append-when-absent, mode preservation, and no-leftover-temp-file. Closes JSONbored#7766
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7835 +/- ##
=======================================
Coverage 91.37% 91.37%
=======================================
Files 729 729
Lines 74671 74671
Branches 22791 22790 -1
=======================================
Hits 68229 68229
Misses 5396 5396
Partials 1046 1046
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-21 14:52:24 UTC
Review summary Nits — 4 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
What & why
env_put()(scripts/lib/selfhost-deploy-common.sh) creates a same-directory temp file viamktemp— its own comment justifies this as enabling an atomic swap — but then writes withcat "$tmp" >"$file"; rm -f "$tmp", a truncate-then-copy, not a rename. A crash/kill/power-loss mid-write (this runs during self-host deploys, e.g.env_put LOOPOVER_IMAGE "$IMAGE"at the end ofdeploy-selfhost-image.sh) can leave.envtruncated/corrupted — exactly what the same-directory temp file was meant to prevent.Change
Swap to an atomic
mv "$tmp" "$file", mirroring the same-directory-temp +mvidiom already used inbackup-metrics.sh/browserless-metrics.sh/export-ams-reporting-db.sh.Caveat handled:
mktempcreates$tmpat0600, so a baremvwould silently narrow.env's permissions on every write. The fix captures the target's existing mode first (GNUstat -c '%a'with a BSDstat -f '%Lp'fallback — the same stat-portability idiombackup-metrics.shuses) andchmods$tmpto match before themv.Adds
env_puttests totest/unit/selfhost-deploy-common.test.ts(which had none): in-place key update, append-when-absent, mode preservation (set0640, assert unchanged), and no-leftover-temp-file (proving a rename, not a copy).scripts/**/.shis outside Codecov'scoverage.include, socodecov/patchdoesn't gate it — the tests run in the backend vitest suite. Verified locally: 8 tests in the file pass (4 new), a 41-test selfhost regression sweep is green,bash -nclean, roottsc --noEmitclean, andgit diff --checkclean.Closes #7766