Do not clobber a symlinked config file on save #90

Merged

danny merged 1 commit from issue89_symlink_write into main

2026-07-10 08:59:26 +01:00

claude commented

2026-07-10 08:24:14 +01:00

Collaborator

Closes #89.

See the issue comment for the root-cause writeup. Summary: atomicWriteConfigFile writes via a temp file plus os.Rename for atomicity, but rename(2) does not follow a symlink at the destination - it replaces whatever is there unconditionally. If --config pointed at a symlink, a web UI save destroyed it and left a plain regular file in its place.

Fix: resolve the real target via filepath.EvalSymlinks once at the top (falling back to the given path if resolution fails, e.g. a genuinely missing file) and operate on that resolved path throughout - permission lookup, temp file directory, and the final rename target. This also fixes a related latent bug: the temp file needs to live in the same directory as the real rename target, not necessarily the symlink's own directory, or os.Rename could fail outright with EXDEV even before the clobbering problem.

Tested against a real symlink (same directory, and separately across two different temp directories to exercise the cross-filesystem-safe temp-file placement), plus an end-to-end test through the real HTTP save handler. Confirmed all three tests actually fail without the fix (by temporarily reverting it and re-running) before restoring it, not just that they pass with it.

make lint/make test clean.

Closes #89. See the issue comment for the root-cause writeup. Summary: `atomicWriteConfigFile` writes via a temp file plus `os.Rename` for atomicity, but `rename(2)` does not follow a symlink at the destination - it replaces whatever is there unconditionally. If `--config` pointed at a symlink, a web UI save destroyed it and left a plain regular file in its place. Fix: resolve the real target via `filepath.EvalSymlinks` once at the top (falling back to the given path if resolution fails, e.g. a genuinely missing file) and operate on that resolved path throughout - permission lookup, temp file directory, and the final rename target. This also fixes a related latent bug: the temp file needs to live in the same directory as the real rename target, not necessarily the symlink's own directory, or `os.Rename` could fail outright with `EXDEV` even before the clobbering problem. Tested against a real symlink (same directory, and separately across two different temp directories to exercise the cross-filesystem-safe temp-file placement), plus an end-to-end test through the real HTTP save handler. Confirmed all three tests actually fail without the fix (by temporarily reverting it and re-running) before restoring it, not just that they pass with it. `make lint`/`make test` clean.

claude added 1 commit

2026-07-10 08:24:14 +01:00

Don't clobber a symlinked config file on save

CI / lint (push) Successful in 27s

Details

CI / build (push) Successful in 26s

Details

CI / lint (pull_request) Successful in 25s

Details

CI / build (pull_request) Successful in 30s

Details

CI / release (push) Has been skipped

Details

CI / release (pull_request) Has been skipped

Details

844b36e59d

atomicWriteConfigFile wrote via a temp file plus os.Rename for
atomicity, but rename(2) doesn't follow a symlink at the destination
- it replaces whatever's there unconditionally. If --config pointed
at a symlink, saving from the web UI destroyed it and left a plain
file in its place.

Resolve the real target via filepath.EvalSymlinks once up front and
operate on that throughout, including the temp file's directory -
which also avoids a cross-filesystem rename failure if the symlink's
target lives in a different directory.

Closes #89

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

claude requested review from danny

2026-07-10 08:24:25 +01:00

danny scheduled this pull request to auto merge when all checks succeed

2026-07-10 08:58:34 +01:00