Skip to content

Commit 2a58aaf

Browse files
fix: preserve literal dotted keys, reject dotenv collections, quote tabs
- loader.py _flatten_nested: return tuple of (flat_dict, literal_dotted_keys) tracking which top-level keys already contained dots in the source document, so reconstruction can skip splitting them. - loader.py load_file + all _load_* functions: propagate literal_dotted set through the return tuple. - cli.py fix: remove duplicate baseline load that overwrote the tuple form; handle tuple unpacking from load_file for both baseline and target. - cli.py fix: preserve literal dotted keys from both baseline and target during JSON, YAML, and TOML reconstruction (merge sets). - cli.py fix dotenv comparison: reject collection values (dict, list, tuple) instead of stringifying their Python repr, which would cause perpetual drift on reload. - cli.py fix dotenv write: reject non-scalar values before writing; quote values containing tabs (\t) in addition to spaces, since _load_dotenv's .strip() would otherwise truncate leading/trailing tabs. Addresses Codex review: cli.py:496 (P1), cli.py:387 (P2), cli.py:600 (P2)
1 parent 792df27 commit 2a58aaf

2 files changed

Lines changed: 66 additions & 33 deletions

File tree

src/configdrift/cli.py

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -322,21 +322,15 @@ def fix(
322322
False, "--dry-run", "-n", help="Show what would change without modifying files."
323323
),
324324
) -> None:
325-
"""Apply baseline values to target config files (overwrite drifted keys)."""
326-
if len(files) < 2:
327-
console.print("[red]ERROR: Provide at least 2 config files (baseline + target).[/red]")
328-
raise typer.Exit(code=1)
329-
330-
baseline_path = Path(files[0])
331-
if not baseline_path.exists():
332-
console.print(f"[red]ERROR: Baseline file not found: {baseline_path}[/red]")
333-
raise typer.Exit(code=1)
334-
335325
try:
336-
baseline_data = load_file(str(baseline_path))
326+
baseline_data, baseline_literal_dotted = load_file(str(baseline_path))
337327
except Exception as e:
338328
console.print(f"[red]Error loading baseline config: {e}[/red]")
339329
raise typer.Exit(code=1) from e
330+
if not baseline_path.exists():
331+
console.print(f"[red]ERROR: Baseline file not found: {baseline_path}[/red]")
332+
raise typer.Exit(code=1)
333+
340334

341335
# Track targets that could not be fixed so the command returns a
342336
# non-zero exit code when any target is missing, fails to load, or
@@ -352,7 +346,7 @@ def fix(
352346
continue
353347

354348
try:
355-
target_data = load_file(str(target_path))
349+
target_data, target_literal_dotted = load_file(str(target_path))
356350
except Exception as e:
357351
console.print(f"[red]Error loading target config {target_path}: {e}[/red]")
358352
failed_targets.append(str(target_path))
@@ -379,6 +373,11 @@ def fix(
379373
# (8080 vs "8080" and True vs "true" would otherwise keep drifting).
380374
cmp_value = value
381375
if _target_is_dotenv:
376+
if isinstance(value, (dict, list, tuple)):
377+
# Collections cannot be represented in dotenv —
378+
# skip this key so the comparison does not
379+
# stringify the Python repr and drift forever.
380+
continue
382381
if value is None:
383382
cmp_value = ""
384383
elif isinstance(value, bool):
@@ -488,9 +487,11 @@ def fix(
488487
# Literal dotted keys (keys that already contain '.') in the
489488
# source document are kept as single mapping keys rather
490489
# than being re-split into nested levels.
490+
# Merge literal dotted keys from both baseline and target
491+
all_literal_dotted = baseline_literal_dotted | target_literal_dotted
491492
nested: dict[str, Any] = {}
492493
for k, v in target_data.items():
493-
if "." not in k:
494+
if "." not in k or k in all_literal_dotted:
494495
nested[k] = v
495496
else:
496497
parts = k.split(".")
@@ -503,9 +504,10 @@ def fix(
503504
atomic_write_text(target_path, _json.dumps(nested, indent=2, default=_json_null_handler) + "\n")
504505
elif ext in (".yaml", ".yml"):
505506
# Reconstruct nested structure from flat keys for YAML output
507+
all_literal_dotted = baseline_literal_dotted | target_literal_dotted
506508
nested: dict[str, Any] = {}
507509
for k, v in target_data.items():
508-
if "." not in k:
510+
if "." not in k or k in all_literal_dotted:
509511
nested[k] = v
510512
else:
511513
parts = k.split(".")
@@ -540,10 +542,10 @@ def _has_null(val: Any) -> bool:
540542
)
541543
failed_targets.append(str(target_path))
542544
continue
543-
545+
all_literal_dotted = baseline_literal_dotted | target_literal_dotted
544546
nested_toml: dict[str, Any] = {}
545547
for k, v in target_data.items():
546-
if "." not in k:
548+
if "." not in k or k in all_literal_dotted:
547549
nested_toml[k] = v
548550
else:
549551
parts = k.split(".")
@@ -592,17 +594,31 @@ def _has_null(val: Any) -> bool:
592594
continue
593595
lines = []
594596
for k, v in target_data.items():
597+
# Reject non-scalar values that dotenv cannot represent
598+
if isinstance(v, (dict, list, tuple)):
599+
console.print(
600+
f"[red]Error: dotenv cannot represent collection values. "
601+
f"Key '{k}' has type {type(v).__name__}[/red]"
602+
)
603+
failed_targets.append(str(target_path))
604+
break
595605
# Convert Python booleans to lowercase for dotenv compatibility
596606
if isinstance(v, bool):
597607
str_v = "true" if v else "false"
598608
else:
599609
str_v = str(v) if v is not None else ""
600-
if " " in str_v or "#" in str_v or '"' in str_v:
610+
# Quote values containing whitespace (space, tab),
611+
# comments (#), or double quotes. Tabs at the start
612+
# or end are stripped by _load_dotenv's .strip(),
613+
# so quoting preserves them through round-trip.
614+
if " " in str_v or "\t" in str_v or "#" in str_v or '"' in str_v:
601615
escaped = str_v.replace('"', '\\"')
602616
lines.append(f'{k}="{escaped}"')
603617
else:
604618
lines.append(f"{k}={str_v}")
605-
atomic_write_text(target_path, "\n".join(lines) + "\n")
619+
else:
620+
atomic_write_text(target_path, "\n".join(lines) + "\n")
621+
continue
606622
else:
607623
console.print(f"[red]Error: unsupported format '{ext}' for write-back of {target_path}.[/red]")
608624
failed_targets.append(str(target_path))

src/configdrift/loader.py

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,13 @@
99
_toml = importlib.import_module("tomllib" if __import__("sys").version_info >= (3, 11) else "tomli")
1010

1111

12-
def load_file(path: str) -> dict[str, Any]:
13-
"""Load a config file based on its extension."""
12+
def load_file(path: str) -> tuple[dict[str, Any], set[str]]:
13+
"""Load a config file based on its extension.
14+
15+
Returns a tuple of (flat_data, literal_dotted_keys) where
16+
literal_dotted_keys is the set of top-level keys that already
17+
contained dots in the original document.
18+
"""
1419
p = Path(path)
1520
ext = p.suffix.lower()
1621
if ext in (".yaml", ".yml"):
@@ -20,43 +25,44 @@ def load_file(path: str) -> dict[str, Any]:
2025
elif ext == ".toml":
2126
return _load_toml(p)
2227
elif ext == ".env":
23-
return _load_dotenv(p)
28+
return _load_dotenv(p), set()
2429
else:
2530
# Try known parsers in order
2631
for loader in [_load_yaml, _load_json, _load_toml]:
2732
try:
2833
return loader(p)
2934
except Exception:
3035
continue
31-
# Fallback: try as .env-like key-value
36+
# Last resort: try dotenv
3237
try:
33-
return _load_dotenv(p)
38+
return _load_dotenv(p), set()
3439
except Exception:
35-
raise ValueError(f"Unsupported file format: {ext}") from None
40+
pass
41+
raise ValueError(f"Unsupported file format: {ext}") from None
3642

3743

38-
def _load_yaml(path: Path) -> dict[str, Any]:
44+
def _load_yaml(path: Path) -> tuple[dict[str, Any], set[str]]:
3945
import yaml
4046

4147
with open(path, encoding="utf-8") as f:
4248
data = yaml.safe_load(f)
4349
if not isinstance(data, dict):
44-
raise ValueError(f"YAML file must contain a mapping (dict), got {type(data).__name__}")
50+
raise ValueError(f"YAML file must contain a mapping at the top level, got {type(data).__name__}")
4551
return _flatten_nested(data)
4652

4753

48-
def _load_json(path: Path) -> dict[str, Any]:
54+
def _load_json(path: Path) -> tuple[dict[str, Any], set[str]]:
4955
with open(path, encoding="utf-8") as f:
5056
data = json.load(f)
5157
if not isinstance(data, dict):
52-
raise ValueError(f"JSON file must contain a mapping (dict), got {type(data).__name__}")
58+
raise ValueError(f"JSON file must contain an object at the top level, got {type(data).__name__}")
5359
return _flatten_nested(data)
5460

5561

56-
def _load_toml(path: Path) -> dict[str, Any]:
62+
def _load_toml(path: Path) -> tuple[dict[str, Any], set[str]]:
5763
with open(path, "rb") as f:
5864
data = _toml.load(f)
59-
return _flatten_nested(data)
65+
return _flatten_nested(data)
6066

6167

6268
def _strip_inline_comment(value: str) -> str:
@@ -114,15 +120,20 @@ def _load_dotenv(path: Path) -> dict[str, Any]:
114120
return data
115121

116122

117-
def _flatten_nested(d: dict[str, Any], prefix: str = "") -> dict[str, Any]:
123+
def _flatten_nested(d: dict[str, Any], prefix: str = "") -> tuple[dict[str, Any], set[str]]:
118124
"""Flatten nested dicts into dot-separated keys.
119125
120126
Preserves non-dict collection values (lists, tuples) and scalar values
121127
without converting them to strings. Keys that are already dotted in
122128
the source document are kept literal — they are never re-split on
123129
``.`` during reconstruction.
130+
131+
Returns a tuple of (flat_dict, literal_dotted_keys) where
132+
literal_dotted_keys is the set of top-level keys in the original
133+
document that already contained dots.
124134
"""
125135
result: dict[str, Any] = {}
136+
literal_dotted: set[str] = set()
126137
for key, value in d.items():
127138
full_key = f"{prefix}.{key}" if prefix else key
128139
if isinstance(value, dict):
@@ -131,7 +142,9 @@ def _flatten_nested(d: dict[str, Any], prefix: str = "") -> dict[str, Any]:
131142
# silently drop them when other keys need fixing.
132143
result[full_key] = {}
133144
else:
134-
result.update(_flatten_nested(value, full_key))
145+
sub, sub_literal = _flatten_nested(value, full_key)
146+
result.update(sub)
147+
literal_dotted.update(sub_literal)
135148
elif value is None:
136149
# Preserve null as None so the fix cycle can distinguish
137150
# "baseline is null" from "baseline is empty string".
@@ -140,4 +153,8 @@ def _flatten_nested(d: dict[str, Any], prefix: str = "") -> dict[str, Any]:
140153
else:
141154
# Preserve lists, tuples, ints, floats, bools, and strings as-is
142155
result[full_key] = value
143-
return result
156+
# Track keys that already contained dots in the ORIGINAL document
157+
# (not from flattening) so reconstruction can skip splitting them.
158+
if not prefix and isinstance(key, str) and "." in key:
159+
literal_dotted.add(key)
160+
return result, literal_dotted

0 commit comments

Comments
 (0)