Skip to content

Commit 7e86bd6

Browse files
fix: resolve symlinks before atomic write, validate dry-run format support
- _atomic.py: resolve symlinked paths before creating the temp file and os.replace so the referent is updated rather than the link being replaced by a regular file (preserves the symlink and its target). - cli.py: validate write-back format support during --dry-run so the dry run accurately predicts whether the real run would succeed. Unsupported extensions and missing tomli-w are reported as failures in both modes. Addresses Codex review: _atomic.py:38 (P1), cli.py:360 (P2)
1 parent a4925f2 commit 7e86bd6

2 files changed

Lines changed: 32 additions & 9 deletions

File tree

‎src/configdrift/_atomic.py‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,13 @@ def atomic_write_text(path: Path, text: str, encoding: str = "utf-8") -> None:
1818
1919
Creates a temporary file beside *path*, writes + fsyncs, then
2020
``os.replace()`` for an atomic rename. Preserves the original
21-
file's permissions when it already exists.
21+
file's permissions when it already exists. When *path* is a
22+
symlink, resolves it first so the referent is updated rather than
23+
the link being replaced by a regular file.
2224
"""
23-
parent = path.parent
25+
# Resolve symlinks so we update the target file, not replace the link.
26+
resolved = path.resolve() if path.is_symlink() else path
27+
parent = resolved.parent
2428
parent.mkdir(parents=True, exist_ok=True)
2529
fd, tmp = tempfile.mkstemp(dir=parent, suffix=".tmp")
2630
try:
@@ -29,13 +33,13 @@ def atomic_write_text(path: Path, text: str, encoding: str = "utf-8") -> None:
2933
fh.flush()
3034
os.fsync(fh.fileno())
3135
# Preserve target permissions during atomic replacement
32-
if path.exists():
36+
if resolved.exists():
3337
try:
34-
st = path.stat()
38+
st = resolved.stat()
3539
os.chmod(tmp, st.st_mode)
3640
except OSError:
3741
pass
38-
os.replace(tmp, path)
42+
os.replace(tmp, resolved)
3943
except BaseException:
4044
# Clean up temp file on any failure
4145
with contextlib.suppress(OSError):
@@ -47,8 +51,11 @@ def atomic_write_bytes(path: Path, data: bytes) -> None:
4751
"""Atomically write *data* to *path*.
4852
4953
Preserves the original file's permissions when it already exists.
54+
When *path* is a symlink, resolves it first so the referent is
55+
updated rather than the link being replaced by a regular file.
5056
"""
51-
parent = path.parent
57+
resolved = path.resolve() if path.is_symlink() else path
58+
parent = resolved.parent
5259
parent.mkdir(parents=True, exist_ok=True)
5360
fd, tmp = tempfile.mkstemp(dir=parent, suffix=".tmp")
5461
try:
@@ -57,13 +64,13 @@ def atomic_write_bytes(path: Path, data: bytes) -> None:
5764
fh.flush()
5865
os.fsync(fh.fileno())
5966
# Preserve target permissions during atomic replacement
60-
if path.exists():
67+
if resolved.exists():
6168
try:
62-
st = path.stat()
69+
st = resolved.stat()
6370
os.chmod(tmp, st.st_mode)
6471
except OSError:
6572
pass
66-
os.replace(tmp, path)
73+
os.replace(tmp, resolved)
6774
except BaseException:
6875
with contextlib.suppress(OSError):
6976
os.unlink(tmp)

‎src/configdrift/cli.py‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,22 @@ def fix(
357357
continue
358358

359359
if dry_run:
360+
# Validate that the target format supports write-back even in
361+
# dry-run mode so --dry-run accurately predicts whether the
362+
# real run would succeed.
363+
ext = target_path.suffix.lower()
364+
supported_exts = {".json", ".yaml", ".yml", ".toml", ".env"}
365+
if ext == ".toml":
366+
try:
367+
import tomli_w # noqa: F401
368+
except ImportError:
369+
console.print("[red]Error: tomli-w is required to write TOML files. Install with: pip install tomli-w[/red]")
370+
failed_targets.append(str(target_path))
371+
continue
372+
elif ext not in supported_exts:
373+
console.print(f"[red]Error: unsupported format '{ext}' for write-back of {target_path}.[/red]")
374+
failed_targets.append(str(target_path))
375+
continue
360376
console.print(f"[yellow]Dry run: {changes} key(s) would be updated in {target_path}[/yellow]")
361377
else:
362378
ext = target_path.suffix.lower()

0 commit comments

Comments
 (0)