From 074e29ecd153195a08ebd85107d793fccd70023a Mon Sep 17 00:00:00 2001 From: xfodev Date: Tue, 21 Jul 2026 16:19:16 +0200 Subject: [PATCH] fix(selfhost): make env_put's .env write atomic (mv) while preserving the target file's mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #7766 --- scripts/lib/selfhost-deploy-common.sh | 14 ++++-- test/unit/selfhost-deploy-common.test.ts | 62 +++++++++++++++++++++++- 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/scripts/lib/selfhost-deploy-common.sh b/scripts/lib/selfhost-deploy-common.sh index 2b663c11fe..80898fd320 100644 --- a/scripts/lib/selfhost-deploy-common.sh +++ b/scripts/lib/selfhost-deploy-common.sh @@ -51,9 +51,14 @@ env_put() { local key="$1" local value="$2" local file="${3:-$ENV_FILE}" - local dir base tmp + local dir base tmp mode touch "$file" + # Preserve the target file's mode across the atomic rename below. mktemp creates $tmp at 0600, so a bare + # `mv "$tmp" "$file"` would silently narrow $file's permissions to 0600 on every write (#7766). Capture the + # existing mode first and re-apply it to $tmp before the swap. GNU stat with a BSD `stat -f` fallback, + # matching backup-metrics.sh's own stat-portability idiom. + mode="$(stat -c '%a' "$file" 2>/dev/null || stat -f '%Lp' "$file")" dir="$(dirname "$file")" base="$(basename "$file")" tmp="$(mktemp "$dir/.${base}.tmp.XXXXXX")" @@ -75,8 +80,11 @@ env_put() { } } ' "$file" >"$tmp" - cat "$tmp" >"$file" - rm -f "$tmp" + # Atomic swap: a rename can't leave $file truncated/corrupted if the process is killed mid-write, unlike the + # previous `cat "$tmp" >"$file"` truncate-then-copy the same-directory temp file was always meant to enable + # (#7766). chmod first so the rename preserves the target's original mode (see the stat above). + chmod "$mode" "$tmp" + mv "$tmp" "$file" } # Optional Infisical wrapper (#5120): when SELFHOST_USE_INFISICAL=1 (opt-in, off by default), prefixes the diff --git a/test/unit/selfhost-deploy-common.test.ts b/test/unit/selfhost-deploy-common.test.ts index eb3813ece0..c359483feb 100644 --- a/test/unit/selfhost-deploy-common.test.ts +++ b/test/unit/selfhost-deploy-common.test.ts @@ -1,4 +1,4 @@ -import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { chmodSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { spawnSync } from "node:child_process"; @@ -115,3 +115,63 @@ describe("maybe_infisical_run (#5120)", () => { } }); }); + +describe("env_put (#7766 -- atomic write + mode preservation)", () => { + // Source the lib and invoke env_put directly with (key, value, file) positional args. + function runEnvPut(file: string, key: string, value: string) { + const script = `set -euo pipefail; . "${libPath.replace(/\\/g, "/")}"; env_put "$1" "$2" "$3"`; + return spawnSync("bash", ["-c", script, "bash", key, value, file], { encoding: "utf8" }); + } + + function tempEnvFile(contents: string): { dir: string; file: string } { + const dir = mkdtempSync(join(tmpdir(), "loopover-env-put-")); + const file = join(dir, ".env"); + writeFileSync(file, contents); + return { dir, file }; + } + + it("updates an existing key in place, leaving the rest of the file intact", () => { + const { dir, file } = tempEnvFile("FOO=1\nBAR=old\n"); + try { + const r = runEnvPut(file, "BAR", "new"); + expect(r.status, r.stderr).toBe(0); + expect(readFileSync(file, "utf8")).toBe("FOO=1\nBAR=new\n"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("appends a key that is not present yet", () => { + const { dir, file } = tempEnvFile("FOO=1\n"); + try { + const r = runEnvPut(file, "BAZ", "added"); + expect(r.status, r.stderr).toBe(0); + expect(readFileSync(file, "utf8")).toBe("FOO=1\nBAZ=added\n"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("preserves the target file's non-default mode across the write (does not narrow to mktemp's 0600)", () => { + const { dir, file } = tempEnvFile("FOO=1\n"); + try { + chmodSync(file, 0o640); + const r = runEnvPut(file, "FOO", "2"); + expect(r.status, r.stderr).toBe(0); + expect(statSync(file).mode & 0o777).toBe(0o640); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("leaves no leftover temp file behind (an atomic rename, not a copy)", () => { + const { dir, file } = tempEnvFile("FOO=1\n"); + try { + const r = runEnvPut(file, "FOO", "2"); + expect(r.status, r.stderr).toBe(0); + expect(readdirSync(dir).filter((name) => name.includes(".tmp."))).toEqual([]); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); +});