Skip to content

Commit 074e29e

Browse files
committed
fix(selfhost): make env_put's .env write atomic (mv) while preserving 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 #7766
1 parent 9d95c96 commit 074e29e

2 files changed

Lines changed: 72 additions & 4 deletions

File tree

scripts/lib/selfhost-deploy-common.sh

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,14 @@ env_put() {
5151
local key="$1"
5252
local value="$2"
5353
local file="${3:-$ENV_FILE}"
54-
local dir base tmp
54+
local dir base tmp mode
5555

5656
touch "$file"
57+
# Preserve the target file's mode across the atomic rename below. mktemp creates $tmp at 0600, so a bare
58+
# `mv "$tmp" "$file"` would silently narrow $file's permissions to 0600 on every write (#7766). Capture the
59+
# existing mode first and re-apply it to $tmp before the swap. GNU stat with a BSD `stat -f` fallback,
60+
# matching backup-metrics.sh's own stat-portability idiom.
61+
mode="$(stat -c '%a' "$file" 2>/dev/null || stat -f '%Lp' "$file")"
5762
dir="$(dirname "$file")"
5863
base="$(basename "$file")"
5964
tmp="$(mktemp "$dir/.${base}.tmp.XXXXXX")"
@@ -75,8 +80,11 @@ env_put() {
7580
}
7681
}
7782
' "$file" >"$tmp"
78-
cat "$tmp" >"$file"
79-
rm -f "$tmp"
83+
# Atomic swap: a rename can't leave $file truncated/corrupted if the process is killed mid-write, unlike the
84+
# previous `cat "$tmp" >"$file"` truncate-then-copy the same-directory temp file was always meant to enable
85+
# (#7766). chmod first so the rename preserves the target's original mode (see the stat above).
86+
chmod "$mode" "$tmp"
87+
mv "$tmp" "$file"
8088
}
8189

8290
# Optional Infisical wrapper (#5120): when SELFHOST_USE_INFISICAL=1 (opt-in, off by default), prefixes the

test/unit/selfhost-deploy-common.test.ts

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
1+
import { chmodSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs";
22
import { tmpdir } from "node:os";
33
import { join, resolve } from "node:path";
44
import { spawnSync } from "node:child_process";
@@ -115,3 +115,63 @@ describe("maybe_infisical_run (#5120)", () => {
115115
}
116116
});
117117
});
118+
119+
describe("env_put (#7766 -- atomic write + mode preservation)", () => {
120+
// Source the lib and invoke env_put directly with (key, value, file) positional args.
121+
function runEnvPut(file: string, key: string, value: string) {
122+
const script = `set -euo pipefail; . "${libPath.replace(/\\/g, "/")}"; env_put "$1" "$2" "$3"`;
123+
return spawnSync("bash", ["-c", script, "bash", key, value, file], { encoding: "utf8" });
124+
}
125+
126+
function tempEnvFile(contents: string): { dir: string; file: string } {
127+
const dir = mkdtempSync(join(tmpdir(), "loopover-env-put-"));
128+
const file = join(dir, ".env");
129+
writeFileSync(file, contents);
130+
return { dir, file };
131+
}
132+
133+
it("updates an existing key in place, leaving the rest of the file intact", () => {
134+
const { dir, file } = tempEnvFile("FOO=1\nBAR=old\n");
135+
try {
136+
const r = runEnvPut(file, "BAR", "new");
137+
expect(r.status, r.stderr).toBe(0);
138+
expect(readFileSync(file, "utf8")).toBe("FOO=1\nBAR=new\n");
139+
} finally {
140+
rmSync(dir, { recursive: true, force: true });
141+
}
142+
});
143+
144+
it("appends a key that is not present yet", () => {
145+
const { dir, file } = tempEnvFile("FOO=1\n");
146+
try {
147+
const r = runEnvPut(file, "BAZ", "added");
148+
expect(r.status, r.stderr).toBe(0);
149+
expect(readFileSync(file, "utf8")).toBe("FOO=1\nBAZ=added\n");
150+
} finally {
151+
rmSync(dir, { recursive: true, force: true });
152+
}
153+
});
154+
155+
it("preserves the target file's non-default mode across the write (does not narrow to mktemp's 0600)", () => {
156+
const { dir, file } = tempEnvFile("FOO=1\n");
157+
try {
158+
chmodSync(file, 0o640);
159+
const r = runEnvPut(file, "FOO", "2");
160+
expect(r.status, r.stderr).toBe(0);
161+
expect(statSync(file).mode & 0o777).toBe(0o640);
162+
} finally {
163+
rmSync(dir, { recursive: true, force: true });
164+
}
165+
});
166+
167+
it("leaves no leftover temp file behind (an atomic rename, not a copy)", () => {
168+
const { dir, file } = tempEnvFile("FOO=1\n");
169+
try {
170+
const r = runEnvPut(file, "FOO", "2");
171+
expect(r.status, r.stderr).toBe(0);
172+
expect(readdirSync(dir).filter((name) => name.includes(".tmp."))).toEqual([]);
173+
} finally {
174+
rmSync(dir, { recursive: true, force: true });
175+
}
176+
});
177+
});

0 commit comments

Comments
 (0)