Skip to content

env_put()'s .env write is non-atomic despite its own comment claiming atomicity #7766

Description

@JSONbored

Context

env_put() in scripts/lib/selfhost-deploy-common.sh creates a same-directory temp file via mktemp (line 59), whose header comment (lines 45-49) explicitly justifies this as guaranteeing an atomic swap ("guarantees cat "$tmp" >"$file" never crosses a filesystem boundary"). But the actual write is cat "$tmp" >"$file" followed by rm -f "$tmp" (lines 78-79) — a truncate-then-copy, not a rename. This is exactly the non-atomic operation the same-directory temp file was supposed to make safe via mv. A crash/kill/power-loss mid-write (this runs during self-host deploys, e.g. env_put LOOPOVER_IMAGE "$IMAGE" at the end of deploy-selfhost-image.sh) can leave .env truncated or corrupted.

This codebase already uses the correct idiom elsewhere: scripts/backup-metrics.sh:59, scripts/browserless-metrics.sh:94, and scripts/export-ams-reporting-db.sh:126,140,165 all do mv "$tmp" "$FILE" for the same same-directory-temp-file reason.

Requirements

Change env_put()'s write from cat "$tmp" >"$file"; rm -f "$tmp" to an atomic mv "$tmp" "$file", mirroring the pattern already used in backup-metrics.sh/browserless-metrics.sh/export-ams-reporting-db.sh. One caveat the fix must handle: mktemp creates the temp file at mode 600, while the target file (created via touch at line 56 if it doesn't already exist) may have a different mode. A naive mv would silently narrow .env's permissions on every write. The fix must preserve the target file's original mode across the swap (e.g. stat the existing file's mode before the mktemp call and chmod the temp file to match before the mv, or an equivalent approach) — do not ship a bare mv that can change .env's permissions as a side effect.

Deliverables

  • env_put() writes via mv (atomic rename) instead of cat+rm.
  • The target file's original mode is preserved across the swap — verified by a test that sets a non-default mode on the target file, calls env_put, and asserts the mode is unchanged.
  • A regression test proving a simulated interruption mid-write (or at minimum, the atomicity of the final rename) doesn't corrupt the file — match whatever level of rigor test/unit/selfhost-deploy-common.test.ts's existing maybe_infisical_run tests use.

Test Coverage Requirements

test/unit/selfhost-deploy-common.test.ts exists but does not cover env_put today. Add tests for both the atomicity fix and the mode-preservation requirement.

Expected Outcome

.env writes during self-host deploys are truly atomic (a crash mid-write can never leave a truncated/corrupted file), and the file's permissions are never silently changed as a side effect of the fix.

Links & Resources

scripts/lib/selfhost-deploy-common.sh:45-49 (the comment claiming atomicity), :56-79 (env_put's actual implementation); scripts/backup-metrics.sh:59, scripts/browserless-metrics.sh:94, scripts/export-ams-reporting-db.sh:126,140,165 (the correct mv pattern already used elsewhere in this repo)

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions