From 25a3d2ea3f8637679da19e9bbfb8e784ba796ea1 Mon Sep 17 00:00:00 2001 From: Chris Rowe Date: Thu, 28 Aug 2025 19:50:02 -0600 Subject: [PATCH 1/8] Add catch_dotenv hook and corresponding tests to manage .env files --- pre_commit_hooks/catch_dotenv.py | 182 ++++++++++++++++++ tests/catch_dotenv_test.py | 316 +++++++++++++++++++++++++++++++ 2 files changed, 498 insertions(+) create mode 100644 pre_commit_hooks/catch_dotenv.py create mode 100644 tests/catch_dotenv_test.py diff --git a/pre_commit_hooks/catch_dotenv.py b/pre_commit_hooks/catch_dotenv.py new file mode 100644 index 00000000..f4e14a20 --- /dev/null +++ b/pre_commit_hooks/catch_dotenv.py @@ -0,0 +1,182 @@ +#!/usr/bin/env python +from __future__ import annotations + +import argparse +import os +import re +import tempfile +from collections.abc import Sequence +from typing import Iterable + +# --- Defaults / Constants --- +DEFAULT_ENV_FILE = ".env" # Canonical env file name +DEFAULT_GITIGNORE_FILE = ".gitignore" +DEFAULT_EXAMPLE_ENV_FILE = ".env.example" +GITIGNORE_BANNER = "# Added by pre-commit hook to prevent committing secrets" + +_KEY_REGEX = re.compile(r"^\s*(?:export\s+)?([A-Za-z_][A-Za-z0-9_]*)\s*=") + + +def _atomic_write(path: str, data: str) -> None: + """Write text to path atomically (best-effort).""" + # Using same directory for atomic os.replace semantics on POSIX. + fd, tmp_path = tempfile.mkstemp(dir=os.path.dirname(path) or ".") + try: + with os.fdopen(fd, "w", encoding="utf-8", newline="") as tmp_f: + tmp_f.write(data) + os.replace(tmp_path, path) + finally: # Clean up if replace failed + if os.path.exists(tmp_path): # pragma: no cover (rare failure case) + try: + os.remove(tmp_path) + except OSError: # pragma: no cover + pass + + +def ensure_env_in_gitignore(env_file: str, gitignore_file: str, banner: str) -> bool: + """Normalize `.gitignore` so it contains exactly one banner + env line at end. + + Returns True if the file was created or its contents changed, False otherwise. + Strategy: read existing lines, strip trailing blanks, remove any prior occurrences of + the banner or env_file (even if duplicated), then append a single blank line, + banner, and env_file. Produces an idempotent final layout. + """ + try: + if os.path.exists(gitignore_file): + with open(gitignore_file, "r", encoding="utf-8") as f: + lines = f.read().splitlines() + else: + lines = [] + except OSError as exc: + print(f"ERROR: unable to read '{gitignore_file}': {exc}") + return False + + original = list(lines) + + # Trim trailing blank lines + while lines and not lines[-1].strip(): + lines.pop() + + # Remove existing occurrences (exact match after strip) + filtered: list[str] = [ln for ln in lines if ln.strip() not in {env_file, banner}] + + if filtered and filtered[-1].strip(): + filtered.append("") # ensure single blank before banner + elif not filtered: # empty file -> still separate section visually + filtered.append("") + + filtered.append(banner) + filtered.append(env_file) + + new_content = "\n".join(filtered) + "\n" + if original == filtered: + return False + try: + _atomic_write(gitignore_file, new_content) + return True + except OSError as exc: # pragma: no cover + print(f"ERROR: unable to write '{gitignore_file}': {exc}") + return False + + +def create_example_env(src_env: str, example_file: str) -> bool: + """Write example file containing only variable keys from real env file. + + Returns True if file written (or updated), False on read/write error. + Lines accepted: optional 'export ' prefix then KEY=...; ignores comments & duplicates. + """ + try: + with open(src_env, "r", encoding="utf-8") as f_env: + lines = f_env.readlines() + except OSError as exc: + print(f"ERROR: unable to read '{src_env}': {exc}") + return False + + seen: set[str] = set() + keys: list[str] = [] + for line in lines: + stripped = line.strip() + if not stripped or stripped.startswith('#'): + continue + m = _KEY_REGEX.match(stripped) + if not m: + continue + key = m.group(1) + if key not in seen: + seen.add(key) + keys.append(key) + + header = [ + '# Generated by catch-dotenv hook.', + '# Variable names only – fill in sample values as needed.', + '', + ] + body = [f"{k}=" for k in keys] + try: + _atomic_write(example_file, "\n".join(header + body) + "\n") + return True + except OSError as exc: # pragma: no cover + print(f"ERROR: unable to write '{example_file}': {exc}") + return False + + +def _has_env(filenames: Iterable[str], env_file: str) -> bool: + """Return True if any staged path refers to a target env file by basename.""" + return any(os.path.basename(name) == env_file for name in filenames) + + +def _find_repo_root(start: str = '.') -> str: + """Ascend from start until a directory containing '.git' is found. + + Falls back to absolute path of start if no parent contains '.git'. This mirrors + typical pre-commit execution (already at repo root) but makes behavior stable + when hook is invoked from a subdirectory (e.g. for direct ad‑hoc testing). + """ + cur = os.path.abspath(start) + prev = None + while cur != prev: + if os.path.isdir(os.path.join(cur, '.git')): + return cur + prev, cur = cur, os.path.abspath(os.path.join(cur, os.pardir)) + return os.path.abspath(start) + + +def _print_failure(env_file: str, gitignore_file: str, example_created: bool, gitignore_modified: bool) -> None: + parts: list[str] = [f"Blocked committing '{env_file}'."] + if gitignore_modified: + parts.append(f"Added to '{gitignore_file}'.") + if example_created: + parts.append("Example file generated.") + parts.append(f"Remove '{env_file}' from the commit and commit again.") + print(" ".join(parts)) + + +def main(argv: Sequence[str] | None = None) -> int: + """Main function for the pre-commit hook.""" + parser = argparse.ArgumentParser(description="Block committing environment files (.env).") + parser.add_argument('filenames', nargs='*', help='Staged filenames (supplied by pre-commit).') + parser.add_argument('--create-example', action='store_true', help='Generate example env file (.env.example).') + args = parser.parse_args(argv) + env_file = DEFAULT_ENV_FILE + # Resolve repository root (directory containing .git) so writes happen there + repo_root = _find_repo_root('.') + gitignore_file = os.path.join(repo_root, DEFAULT_GITIGNORE_FILE) + example_file = os.path.join(repo_root, DEFAULT_EXAMPLE_ENV_FILE) + env_abspath = os.path.join(repo_root, env_file) + + if not _has_env(args.filenames, env_file): + return 0 + + gitignore_modified = ensure_env_in_gitignore(env_file, gitignore_file, GITIGNORE_BANNER) + example_created = False + if args.create_example: + # Source env is always looked up relative to repo root + if os.path.exists(env_abspath): + example_created = create_example_env(env_abspath, example_file) + + _print_failure(env_file, gitignore_file, example_created, gitignore_modified) + return 1 # Block commit + + +if __name__ == '__main__': + raise SystemExit(main()) diff --git a/tests/catch_dotenv_test.py b/tests/catch_dotenv_test.py new file mode 100644 index 00000000..c9e14241 --- /dev/null +++ b/tests/catch_dotenv_test.py @@ -0,0 +1,316 @@ +from __future__ import annotations + +import os +import threading +import time +from pathlib import Path +import shutil +import re +import pytest + +from pre_commit_hooks.catch_dotenv import main, ensure_env_in_gitignore, GITIGNORE_BANNER, DEFAULT_ENV_FILE, DEFAULT_EXAMPLE_ENV_FILE, DEFAULT_GITIGNORE_FILE + +# Tests cover hook behavior: detection gating, .gitignore normalization, example +# file generation parsing edge cases, idempotency, and preservation of existing +# content. Each test isolates a single behavioral contract. + + +@pytest.fixture() +def env_file(tmp_path: Path) -> Path: + """Copy shared resource .env into tmp workspace as the canonical .env. + + All tests rely on this baseline content (optionally appending extra lines + for edge cases) to ensure consistent parsing behavior. + """ + # __file__ => /tests/catch_dotenv_test.py + # parents[0] = /tests, parents[1] = + # Source file stored as test.env in repo (cannot commit a real .env in CI) + resource_env = Path(__file__).resolve().parents[1] / 'testing' / 'resources' / 'test.env' + dest = tmp_path / DEFAULT_ENV_FILE + shutil.copyfile(resource_env, dest) + return dest + + +def run_hook(tmp_path: Path, staged: list[str], create_example: bool = False) -> int: + cwd = os.getcwd() + os.chdir(tmp_path) + try: + args = staged[:] + if create_example: + args.append('--create-example') + return main(args) + finally: + os.chdir(cwd) + + +def test_no_env_file(tmp_path: Path, env_file: Path): + """Hook should no-op (return 0) if .env not staged even if it exists.""" + (tmp_path / 'foo.txt').write_text('x') + assert run_hook(tmp_path, ['foo.txt']) == 0 + + +def test_blocks_env_and_updates_gitignore(tmp_path: Path, env_file: Path): + """Staging .env triggers block (exit 1) and appends banner + env entry.""" + ret = run_hook(tmp_path, [DEFAULT_ENV_FILE]) + assert ret == 1 + gi = (tmp_path / DEFAULT_GITIGNORE_FILE).read_text().splitlines() + assert gi[-2] == GITIGNORE_BANNER + assert gi[-1] == DEFAULT_ENV_FILE + + +def test_env_present_but_not_staged(tmp_path: Path, env_file: Path): + """Existing .env on disk but not staged should not block commit.""" + assert run_hook(tmp_path, ['unrelated.txt']) == 0 + + +def test_idempotent_gitignore(tmp_path: Path, env_file: Path): + """Re-running after initial normalization leaves .gitignore unchanged.""" + g = tmp_path / DEFAULT_GITIGNORE_FILE + g.write_text(f"{GITIGNORE_BANNER}\n{DEFAULT_ENV_FILE}\n") + first = run_hook(tmp_path, [DEFAULT_ENV_FILE]) + assert first == 1 + content1 = g.read_text() + second = run_hook(tmp_path, [DEFAULT_ENV_FILE]) + assert second == 1 + assert g.read_text() == content1 # unchanged + + +def test_gitignore_with_existing_content_preserved(tmp_path: Path, env_file: Path): + """Existing entries stay intact; banner/env appended at end cleanly.""" + g = tmp_path / DEFAULT_GITIGNORE_FILE + g.write_text('node_modules/\n# comment line\n') # no trailing newline section markers + run_hook(tmp_path, [DEFAULT_ENV_FILE]) + lines = g.read_text().splitlines() + # original content should still be at top + assert lines[0] == 'node_modules/' + assert '# comment line' in lines[1] + # Last two lines should be banner + env file + assert lines[-2:] == [GITIGNORE_BANNER, DEFAULT_ENV_FILE] + + +def test_gitignore_duplicates_are_collapsed(tmp_path: Path, env_file: Path): + """Multiple prior duplicate banner/env lines collapse to single pair.""" + g = tmp_path / DEFAULT_GITIGNORE_FILE + g.write_text(f"other\n{GITIGNORE_BANNER}\n{DEFAULT_ENV_FILE}\n{GITIGNORE_BANNER}\n{DEFAULT_ENV_FILE}\n\n\n") + run_hook(tmp_path, [DEFAULT_ENV_FILE]) + lines = g.read_text().splitlines() + assert lines.count(GITIGNORE_BANNER) == 1 + assert lines.count(DEFAULT_ENV_FILE) == 1 + assert lines[-2:] == [GITIGNORE_BANNER, DEFAULT_ENV_FILE] + + +def test_create_example(tmp_path: Path, env_file: Path): + """Example file includes discovered keys; values stripped to KEY=.""" + ret = run_hook(tmp_path, [DEFAULT_ENV_FILE], create_example=True) + assert ret == 1 + example = (tmp_path / DEFAULT_EXAMPLE_ENV_FILE).read_text().splitlines() + key_lines = [ln for ln in example if ln and not ln.startswith('#')] + # All key lines should be KEY= + assert all(re.match(r'^[A-Za-z_][A-Za-z0-9_]*=$', ln) for ln in key_lines) + # Spot check a few known keys from resource file + for k in ['BACKEND_CONTAINER_PORT=', 'ACCESS_TOKEN_SECRET=', 'SUPABASE_SERVICE_KEY=']: + assert k in key_lines + + +def test_create_example_duplicate_key_variant_ignored(tmp_path: Path, env_file: Path): + """Appending whitespace duplicate of existing key should not duplicate in example.""" + with open(env_file, 'a', encoding='utf-8') as f: + f.write('BACKEND_CONTAINER_PORT =999 # duplicate variant\n') + run_hook(tmp_path, [DEFAULT_ENV_FILE], create_example=True) + lines = (tmp_path / DEFAULT_EXAMPLE_ENV_FILE).read_text().splitlines() + key_lines = [ln for ln in lines if ln and not ln.startswith('#')] + assert key_lines.count('BACKEND_CONTAINER_PORT=') == 1 + + +def test_gitignore_without_trailing_newline(tmp_path: Path, env_file: Path): + """Normalization works when original .gitignore lacks trailing newline.""" + g = tmp_path / DEFAULT_GITIGNORE_FILE + g.write_text('existing_line') # no newline at EOF + run_hook(tmp_path, [DEFAULT_ENV_FILE]) + lines = g.read_text().splitlines() + assert lines[0] == 'existing_line' + assert lines[-2:] == [GITIGNORE_BANNER, DEFAULT_ENV_FILE] + + +def test_ensure_env_in_gitignore_normalizes(tmp_path: Path, env_file: Path): + """Direct API call collapses duplicates and produces canonical tail layout.""" + g = tmp_path / DEFAULT_GITIGNORE_FILE + g.write_text(f"{GITIGNORE_BANNER}\n{DEFAULT_ENV_FILE}\n{GITIGNORE_BANNER}\n{DEFAULT_ENV_FILE}\n\n") + modified = ensure_env_in_gitignore(DEFAULT_ENV_FILE, str(g), GITIGNORE_BANNER) + assert modified is True + lines = g.read_text().splitlines() + # final two lines should be banner + env + assert lines[-2:] == [GITIGNORE_BANNER, DEFAULT_ENV_FILE] + # only one occurrence each + assert lines.count(GITIGNORE_BANNER) == 1 + assert lines.count(DEFAULT_ENV_FILE) == 1 + + +def test_source_env_file_not_modified(tmp_path: Path, env_file: Path): + """Hook must not alter original .env (comments and formatting stay).""" + original = env_file.read_text() + run_hook(tmp_path, [DEFAULT_ENV_FILE], create_example=True) + assert env_file.read_text() == original + + +def test_large_resource_env_parsing(tmp_path: Path, env_file: Path): + """Generate example from resource env; assert broad key coverage & format.""" + ret = run_hook(tmp_path, [DEFAULT_ENV_FILE], create_example=True) + assert ret == 1 + example_lines = (tmp_path / DEFAULT_EXAMPLE_ENV_FILE).read_text().splitlines() + key_lines = [ln for ln in example_lines if ln and not ln.startswith('#')] + assert len(key_lines) > 20 + assert all(re.match(r'^[A-Za-z_][A-Za-z0-9_]*=$', ln) for ln in key_lines) + for k in ['BACKEND_CONTAINER_PORT=', 'SUPABASE_SERVICE_KEY=', 'ACCESS_TOKEN_SECRET=']: + assert k in key_lines + + +def test_failure_message_content(tmp_path: Path, env_file: Path, capsys): + """Hook stdout message should contain key phrases when blocking commit.""" + ret = run_hook(tmp_path, [DEFAULT_ENV_FILE], create_example=True) + assert ret == 1 + out = capsys.readouterr().out.strip() + assert "Blocked committing" in out + assert DEFAULT_GITIGNORE_FILE in out + assert "Example file generated" in out + assert "Remove '.env'" in out + + +def test_create_example_when_env_missing(tmp_path: Path, env_file: Path): + """--create-example with no .env staged or present should no-op (exit 0). + + Uses env_file fixture (requirement: all tests use fixture) then removes the + copied .env to simulate absence. + """ + env_file.unlink() + ret = run_hook(tmp_path, ['unrelated.txt'], create_example=True) + assert ret == 0 + assert not (tmp_path / DEFAULT_EXAMPLE_ENV_FILE).exists() + + +def test_gitignore_is_directory_error(tmp_path: Path, env_file: Path, capsys): + """If .gitignore path is a directory, hook should print error and still block.""" + gitignore_dir = tmp_path / DEFAULT_GITIGNORE_FILE + gitignore_dir.mkdir() + ret = run_hook(tmp_path, [DEFAULT_ENV_FILE]) + assert ret == 1 # still blocks commit + out = capsys.readouterr().out + assert "ERROR:" in out # read failure logged + + +def test_env_example_overwrites_existing(tmp_path: Path, env_file: Path): + """Pre-existing example file with junk should be overwritten with header & keys.""" + example = tmp_path / DEFAULT_EXAMPLE_ENV_FILE + example.write_text('junk=1\nSHOULD_NOT_REMAIN=2\n') + run_hook(tmp_path, [DEFAULT_ENV_FILE], create_example=True) + content = example.read_text().splitlines() + assert content[0].startswith('# Generated by catch-dotenv') + assert any(ln.startswith('BACKEND_CONTAINER_PORT=') for ln in content) + assert 'junk=1' not in content + assert 'SHOULD_NOT_REMAIN=2' not in content + + +def test_large_gitignore_normalization_performance(tmp_path: Path, env_file: Path): + """Very large .gitignore remains normalized quickly (functional smoke).""" + g = tmp_path / DEFAULT_GITIGNORE_FILE + # Generate many lines with scattered duplicates of banner/env + lines = [f"file_{i}" for i in range(3000)] + [GITIGNORE_BANNER, DEFAULT_ENV_FILE] * 3 + g.write_text("\n".join(lines) + "\n") + start = time.time() + run_hook(tmp_path, [DEFAULT_ENV_FILE]) + elapsed = time.time() - start + result_lines = g.read_text().splitlines() + assert result_lines[-2:] == [GITIGNORE_BANNER, DEFAULT_ENV_FILE] + assert result_lines.count(GITIGNORE_BANNER) == 1 + assert result_lines.count(DEFAULT_ENV_FILE) == 1 + # Soft performance expectation: should finish fast (< 0.5s on typical dev machine) + assert elapsed < 0.5 + + +def test_concurrent_gitignore_writes(tmp_path: Path, env_file: Path): + """Concurrent ensure_env_in_gitignore calls result in canonical final state.""" + g = tmp_path / DEFAULT_GITIGNORE_FILE + # Seed with messy duplicates + g.write_text(f"other\n{GITIGNORE_BANNER}\n{DEFAULT_ENV_FILE}\n\n") + + def worker(): + ensure_env_in_gitignore(DEFAULT_ENV_FILE, str(g), GITIGNORE_BANNER) + + threads = [threading.Thread(target=worker) for _ in range(5)] + for t in threads: + t.start() + for t in threads: + t.join() + lines = g.read_text().splitlines() + assert lines[-2:] == [GITIGNORE_BANNER, DEFAULT_ENV_FILE] + assert lines.count(GITIGNORE_BANNER) == 1 + assert lines.count(DEFAULT_ENV_FILE) == 1 + + +def test_mixed_staged_files(tmp_path: Path, env_file: Path): + """Staging .env with other files still blocks and only normalizes gitignore once.""" + other = tmp_path / 'README.md' + other.write_text('hi') + ret = run_hook(tmp_path, [DEFAULT_ENV_FILE, 'README.md']) + assert ret == 1 + lines = (tmp_path / DEFAULT_GITIGNORE_FILE).read_text().splitlines() + assert lines[-2:] == [GITIGNORE_BANNER, DEFAULT_ENV_FILE] + + +def test_already_ignored_env_with_variations(tmp_path: Path, env_file: Path): + """Pre-existing ignore lines with spacing normalize to single canonical pair.""" + g = tmp_path / DEFAULT_GITIGNORE_FILE + g.write_text(f" {DEFAULT_ENV_FILE} \n{GITIGNORE_BANNER}\n {DEFAULT_ENV_FILE}\n") + run_hook(tmp_path, [DEFAULT_ENV_FILE]) + lines = g.read_text().splitlines() + assert lines[-2:] == [GITIGNORE_BANNER, DEFAULT_ENV_FILE] + assert lines.count(DEFAULT_ENV_FILE) == 1 + + +def test_subdirectory_invocation(tmp_path: Path, env_file: Path): + """Running from a subdirectory still writes gitignore/example at repo root.""" + sub = tmp_path / 'subdir' + sub.mkdir() + # simulate repository root marker + (tmp_path / '.git').mkdir() + # simulate running hook from subdir while staged path relative to repo root + cwd = os.getcwd() + os.chdir(sub) + try: + ret = main(['../' + DEFAULT_ENV_FILE]) + gi = (tmp_path / DEFAULT_GITIGNORE_FILE).read_text().splitlines() + finally: + os.chdir(cwd) + assert ret == 1 + assert gi[-2:] == [GITIGNORE_BANNER, DEFAULT_ENV_FILE] + + +def test_atomic_write_failure_gitignore(monkeypatch, tmp_path: Path, env_file: Path, capsys): + """Simulate os.replace failure during gitignore write to exercise error path.""" + def boom(*a, **k): + raise OSError('replace-fail') + monkeypatch.setattr('pre_commit_hooks.catch_dotenv.os.replace', boom) + modified = ensure_env_in_gitignore(DEFAULT_ENV_FILE, str(tmp_path / DEFAULT_GITIGNORE_FILE), GITIGNORE_BANNER) + assert modified is False + out = capsys.readouterr().out + assert 'ERROR: unable to write' in out + + +def test_atomic_write_failure_example(monkeypatch, tmp_path: Path, env_file: Path, capsys): + """Simulate os.replace failure when writing example env file.""" + def boom(*a, **k): + raise OSError('replace-fail') + monkeypatch.setattr('pre_commit_hooks.catch_dotenv.os.replace', boom) + ok = False + # create_example_env requires source .env to exist; env_file fixture provides it in tmp_path root + cwd = os.getcwd() + os.chdir(tmp_path) + try: + ok = main([DEFAULT_ENV_FILE, '--create-example']) == 1 + finally: + os.chdir(cwd) + # hook still blocks; but example creation failed -> message should not claim Example file generated + assert ok is True + out = capsys.readouterr().out + assert 'Example file generated' not in out + assert 'ERROR: unable to write' in out From 33746f52ec5025885834516973ebf16c08ef14b9 Mon Sep 17 00:00:00 2001 From: Chris Rowe Date: Thu, 28 Aug 2025 20:09:17 -0600 Subject: [PATCH 2/8] Refactor catch_dotenv hook and tests for improved error handling and clarity --- pre_commit_hooks/catch_dotenv.py | 65 +++++++++++--------------------- tests/catch_dotenv_test.py | 20 +++++----- 2 files changed, 31 insertions(+), 54 deletions(-) diff --git a/pre_commit_hooks/catch_dotenv.py b/pre_commit_hooks/catch_dotenv.py index f4e14a20..6e1a418a 100644 --- a/pre_commit_hooks/catch_dotenv.py +++ b/pre_commit_hooks/catch_dotenv.py @@ -4,12 +4,13 @@ import argparse import os import re +import sys import tempfile from collections.abc import Sequence from typing import Iterable -# --- Defaults / Constants --- -DEFAULT_ENV_FILE = ".env" # Canonical env file name +# Defaults / constants +DEFAULT_ENV_FILE = ".env" DEFAULT_GITIGNORE_FILE = ".gitignore" DEFAULT_EXAMPLE_ENV_FILE = ".env.example" GITIGNORE_BANNER = "# Added by pre-commit hook to prevent committing secrets" @@ -18,8 +19,7 @@ def _atomic_write(path: str, data: str) -> None: - """Write text to path atomically (best-effort).""" - # Using same directory for atomic os.replace semantics on POSIX. + """Atomic-ish text write: write to same-dir temp then os.replace.""" fd, tmp_path = tempfile.mkstemp(dir=os.path.dirname(path) or ".") try: with os.fdopen(fd, "w", encoding="utf-8", newline="") as tmp_f: @@ -34,24 +34,19 @@ def _atomic_write(path: str, data: str) -> None: def ensure_env_in_gitignore(env_file: str, gitignore_file: str, banner: str) -> bool: - """Normalize `.gitignore` so it contains exactly one banner + env line at end. - - Returns True if the file was created or its contents changed, False otherwise. - Strategy: read existing lines, strip trailing blanks, remove any prior occurrences of - the banner or env_file (even if duplicated), then append a single blank line, - banner, and env_file. Produces an idempotent final layout. - """ + """Normalize .gitignore tail (banner + env) collapsing duplicates. Returns True if modified.""" try: if os.path.exists(gitignore_file): with open(gitignore_file, "r", encoding="utf-8") as f: - lines = f.read().splitlines() + original_text = f.read() + lines = original_text.splitlines() else: + original_text = "" lines = [] except OSError as exc: - print(f"ERROR: unable to read '{gitignore_file}': {exc}") + print(f"ERROR: unable to read {gitignore_file}: {exc}", file=sys.stderr) return False - - original = list(lines) + original_content_str = original_text if lines else "" # post-read snapshot # Trim trailing blank lines while lines and not lines[-1].strip(): @@ -69,27 +64,23 @@ def ensure_env_in_gitignore(env_file: str, gitignore_file: str, banner: str) -> filtered.append(env_file) new_content = "\n".join(filtered) + "\n" - if original == filtered: + if new_content == (original_content_str if original_content_str.endswith("\n") else original_content_str + ("" if not original_content_str else "\n")): return False try: _atomic_write(gitignore_file, new_content) return True except OSError as exc: # pragma: no cover - print(f"ERROR: unable to write '{gitignore_file}': {exc}") + print(f"ERROR: unable to write {gitignore_file}: {exc}", file=sys.stderr) return False def create_example_env(src_env: str, example_file: str) -> bool: - """Write example file containing only variable keys from real env file. - - Returns True if file written (or updated), False on read/write error. - Lines accepted: optional 'export ' prefix then KEY=...; ignores comments & duplicates. - """ + """Generate .env.example with unique KEY= lines (no values).""" try: with open(src_env, "r", encoding="utf-8") as f_env: lines = f_env.readlines() except OSError as exc: - print(f"ERROR: unable to read '{src_env}': {exc}") + print(f"ERROR: unable to read {src_env}: {exc}", file=sys.stderr) return False seen: set[str] = set() @@ -125,41 +116,27 @@ def _has_env(filenames: Iterable[str], env_file: str) -> bool: return any(os.path.basename(name) == env_file for name in filenames) -def _find_repo_root(start: str = '.') -> str: - """Ascend from start until a directory containing '.git' is found. - - Falls back to absolute path of start if no parent contains '.git'. This mirrors - typical pre-commit execution (already at repo root) but makes behavior stable - when hook is invoked from a subdirectory (e.g. for direct ad‑hoc testing). - """ - cur = os.path.abspath(start) - prev = None - while cur != prev: - if os.path.isdir(os.path.join(cur, '.git')): - return cur - prev, cur = cur, os.path.abspath(os.path.join(cur, os.pardir)) - return os.path.abspath(start) def _print_failure(env_file: str, gitignore_file: str, example_created: bool, gitignore_modified: bool) -> None: - parts: list[str] = [f"Blocked committing '{env_file}'."] + parts: list[str] = [f"Blocked committing {env_file}."] if gitignore_modified: - parts.append(f"Added to '{gitignore_file}'.") + parts.append(f"Updated {gitignore_file}.") if example_created: - parts.append("Example file generated.") - parts.append(f"Remove '{env_file}' from the commit and commit again.") + parts.append("Generated .env.example.") + parts.append(f"Remove {env_file} from the commit and retry.") print(" ".join(parts)) def main(argv: Sequence[str] | None = None) -> int: - """Main function for the pre-commit hook.""" + """Hook entry-point.""" parser = argparse.ArgumentParser(description="Block committing environment files (.env).") parser.add_argument('filenames', nargs='*', help='Staged filenames (supplied by pre-commit).') parser.add_argument('--create-example', action='store_true', help='Generate example env file (.env.example).') args = parser.parse_args(argv) env_file = DEFAULT_ENV_FILE - # Resolve repository root (directory containing .git) so writes happen there - repo_root = _find_repo_root('.') + # Use current working directory as repository root (simplified; no ascent) + repo_root = os.getcwd() gitignore_file = os.path.join(repo_root, DEFAULT_GITIGNORE_FILE) example_file = os.path.join(repo_root, DEFAULT_EXAMPLE_ENV_FILE) env_abspath = os.path.join(repo_root, env_file) diff --git a/tests/catch_dotenv_test.py b/tests/catch_dotenv_test.py index c9e14241..a2f16106 100644 --- a/tests/catch_dotenv_test.py +++ b/tests/catch_dotenv_test.py @@ -171,9 +171,9 @@ def test_failure_message_content(tmp_path: Path, env_file: Path, capsys): assert ret == 1 out = capsys.readouterr().out.strip() assert "Blocked committing" in out - assert DEFAULT_GITIGNORE_FILE in out - assert "Example file generated" in out - assert "Remove '.env'" in out + assert DEFAULT_GITIGNORE_FILE in out # updated path appears + assert "Generated .env.example." in out + assert "Remove .env" in out def test_create_example_when_env_missing(tmp_path: Path, env_file: Path): @@ -194,8 +194,8 @@ def test_gitignore_is_directory_error(tmp_path: Path, env_file: Path, capsys): gitignore_dir.mkdir() ret = run_hook(tmp_path, [DEFAULT_ENV_FILE]) assert ret == 1 # still blocks commit - out = capsys.readouterr().out - assert "ERROR:" in out # read failure logged + captured = capsys.readouterr() + assert "ERROR:" in captured.err # error now printed to stderr def test_env_example_overwrites_existing(tmp_path: Path, env_file: Path): @@ -268,7 +268,7 @@ def test_already_ignored_env_with_variations(tmp_path: Path, env_file: Path): def test_subdirectory_invocation(tmp_path: Path, env_file: Path): - """Running from a subdirectory still writes gitignore/example at repo root.""" + """Running from a subdirectory now writes .gitignore relative to CWD (simplified behavior).""" sub = tmp_path / 'subdir' sub.mkdir() # simulate repository root marker @@ -277,8 +277,8 @@ def test_subdirectory_invocation(tmp_path: Path, env_file: Path): cwd = os.getcwd() os.chdir(sub) try: - ret = main(['../' + DEFAULT_ENV_FILE]) - gi = (tmp_path / DEFAULT_GITIGNORE_FILE).read_text().splitlines() + ret = main(['../' + DEFAULT_ENV_FILE]) # staged path relative to subdir + gi = (sub / DEFAULT_GITIGNORE_FILE).read_text().splitlines() finally: os.chdir(cwd) assert ret == 1 @@ -292,8 +292,8 @@ def boom(*a, **k): monkeypatch.setattr('pre_commit_hooks.catch_dotenv.os.replace', boom) modified = ensure_env_in_gitignore(DEFAULT_ENV_FILE, str(tmp_path / DEFAULT_GITIGNORE_FILE), GITIGNORE_BANNER) assert modified is False - out = capsys.readouterr().out - assert 'ERROR: unable to write' in out + captured = capsys.readouterr() + assert 'ERROR: unable to write' in captured.err def test_atomic_write_failure_example(monkeypatch, tmp_path: Path, env_file: Path, capsys): From 989ac68f299a6fd8f0f34de9cf416a15d2ad32c4 Mon Sep 17 00:00:00 2001 From: Chris Rowe Date: Thu, 28 Aug 2025 20:29:12 -0600 Subject: [PATCH 3/8] Improve error handling and clarity in catch_dotenv hook and tests --- pre_commit_hooks/catch_dotenv.py | 71 +++++++++++++++++++-------- testing/resources/test.env | 82 ++++++++++++++++++++++++++++++++ tests/catch_dotenv_test.py | 6 ++- 3 files changed, 137 insertions(+), 22 deletions(-) create mode 100644 testing/resources/test.env diff --git a/pre_commit_hooks/catch_dotenv.py b/pre_commit_hooks/catch_dotenv.py index 6e1a418a..1131f6de 100644 --- a/pre_commit_hooks/catch_dotenv.py +++ b/pre_commit_hooks/catch_dotenv.py @@ -19,22 +19,30 @@ def _atomic_write(path: str, data: str) -> None: - """Atomic-ish text write: write to same-dir temp then os.replace.""" + """Atomically (best-effort) write text. + + Writes to a same-directory temporary file then replaces the target with + os.replace(). This is a slight divergence from most existing hooks which + write directly, but here we intentionally reduce the (small) risk of + partially-written files because the hook may be invoked rapidly / in + parallel (tests exercise concurrent normalization). Keeping this helper + local avoids adding any dependency. + """ fd, tmp_path = tempfile.mkstemp(dir=os.path.dirname(path) or ".") try: with os.fdopen(fd, "w", encoding="utf-8", newline="") as tmp_f: tmp_f.write(data) os.replace(tmp_path, path) finally: # Clean up if replace failed - if os.path.exists(tmp_path): # pragma: no cover (rare failure case) + if os.path.exists(tmp_path): # (rare failure case) try: os.remove(tmp_path) - except OSError: # pragma: no cover + except OSError: pass -def ensure_env_in_gitignore(env_file: str, gitignore_file: str, banner: str) -> bool: - """Normalize .gitignore tail (banner + env) collapsing duplicates. Returns True if modified.""" +def _read_gitignore(gitignore_file: str) -> tuple[str, list[str]]: + """Read and parse .gitignore file content.""" try: if os.path.exists(gitignore_file): with open(gitignore_file, "r", encoding="utf-8") as f: @@ -45,14 +53,17 @@ def ensure_env_in_gitignore(env_file: str, gitignore_file: str, banner: str) -> lines = [] except OSError as exc: print(f"ERROR: unable to read {gitignore_file}: {exc}", file=sys.stderr) - return False - original_content_str = original_text if lines else "" # post-read snapshot + raise + return original_text if lines else "", lines + +def _normalize_gitignore_lines(lines: list[str], env_file: str, banner: str) -> list[str]: + """Normalize .gitignore lines by removing duplicates and adding canonical tail.""" # Trim trailing blank lines while lines and not lines[-1].strip(): lines.pop() - # Remove existing occurrences (exact match after strip) + # Remove existing occurrences filtered: list[str] = [ln for ln in lines if ln.strip() not in {env_file, banner}] if filtered and filtered[-1].strip(): @@ -62,14 +73,35 @@ def ensure_env_in_gitignore(env_file: str, gitignore_file: str, banner: str) -> filtered.append(banner) filtered.append(env_file) + return filtered + + +def ensure_env_in_gitignore(env_file: str, gitignore_file: str, banner: str) -> bool: + """Ensure canonical banner + env tail in .gitignore. + + Returns True only when the file content was changed. Returns False both + when unchanged and on IO errors (we intentionally conflate for the simple + hook contract; errors are still surfaced via stderr output). + """ + try: + original_content_str, lines = _read_gitignore(gitignore_file) + except OSError: + return False + filtered = _normalize_gitignore_lines(lines, env_file, banner) new_content = "\n".join(filtered) + "\n" - if new_content == (original_content_str if original_content_str.endswith("\n") else original_content_str + ("" if not original_content_str else "\n")): + + # Normalize original content to a single trailing newline for comparison + normalized_original = original_content_str + if normalized_original and not normalized_original.endswith("\n"): + normalized_original += "\n" + if new_content == normalized_original: return False + try: _atomic_write(gitignore_file, new_content) return True - except OSError as exc: # pragma: no cover + except OSError as exc: print(f"ERROR: unable to write {gitignore_file}: {exc}", file=sys.stderr) return False @@ -107,7 +139,7 @@ def create_example_env(src_env: str, example_file: str) -> bool: _atomic_write(example_file, "\n".join(header + body) + "\n") return True except OSError as exc: # pragma: no cover - print(f"ERROR: unable to write '{example_file}': {exc}") + print(f"ERROR: unable to write '{example_file}': {exc}", file=sys.stderr) return False @@ -116,26 +148,25 @@ def _has_env(filenames: Iterable[str], env_file: str) -> bool: return any(os.path.basename(name) == env_file for name in filenames) - - def _print_failure(env_file: str, gitignore_file: str, example_created: bool, gitignore_modified: bool) -> None: - parts: list[str] = [f"Blocked committing {env_file}."] + # Match typical hook output style: one short line per action. + print(f"Blocked committing {env_file}.") if gitignore_modified: - parts.append(f"Updated {gitignore_file}.") + print(f"Updated {gitignore_file}.") if example_created: - parts.append("Generated .env.example.") - parts.append(f"Remove {env_file} from the commit and retry.") - print(" ".join(parts)) + print("Generated .env.example.") + print(f"Remove {env_file} from the commit and retry.") def main(argv: Sequence[str] | None = None) -> int: """Hook entry-point.""" - parser = argparse.ArgumentParser(description="Block committing environment files (.env).") + parser = argparse.ArgumentParser(description="Blocks committing .env files.") parser.add_argument('filenames', nargs='*', help='Staged filenames (supplied by pre-commit).') parser.add_argument('--create-example', action='store_true', help='Generate example env file (.env.example).') args = parser.parse_args(argv) env_file = DEFAULT_ENV_FILE - # Use current working directory as repository root (simplified; no ascent) + # Use current working directory as repository root (pre-commit executes + # hooks from the repo root). repo_root = os.getcwd() gitignore_file = os.path.join(repo_root, DEFAULT_GITIGNORE_FILE) example_file = os.path.join(repo_root, DEFAULT_EXAMPLE_ENV_FILE) diff --git a/testing/resources/test.env b/testing/resources/test.env new file mode 100644 index 00000000..1479aedc --- /dev/null +++ b/testing/resources/test.env @@ -0,0 +1,82 @@ +# ============================================================================= +# DUMMY SECRETS FOR DOTENV TEST +# ============================================================================= + +# Container Internal Ports (what each service listens on inside containers) +BACKEND_CONTAINER_PORT=3000 # FastAPI server internal port +FRONTEND_CONTAINER_PORT=3001 # Vite dev server internal port + +# External Access (what users/browsers connect to) +CADDY_EXTERNAL_PORT=80 # External port exposed to host system + +# URLs (how different components reference each other) +BASE_HOSTNAME=http://localhost +PUBLIC_FRONTEND_URL=${BASE_HOSTNAME}:${CADDY_EXTERNAL_PORT} +LEGACY_BACKEND_DIRECT_URL=${BASE_HOSTNAME}:${BACKEND_CONTAINER_PORT} # Deprecated: direct backend access +VITE_BROWSER_API_URL=${BASE_HOSTNAME}:${CADDY_EXTERNAL_PORT}/api # Frontend API calls through Caddy + +# Environment +NODE_ENV=development +# Supabase +SUPABASE_PROJECT_ID=979090c33e5da06f67921e70 +SUPABASE_PASSWORD=1bbad0861dbca0bad3bd58ac90fd87e1cfd13ebbbeaed730868a11fa38bf6a65 +SUPABASE_URL=https://${SUPABASE_PROJECT_ID}.supabase.co +DATABASE_URL=postgresql://postgres.${SUPABASE_PROJECT_ID}:${SUPABASE_PASSWORD}@aws-0-us-west-1.pooler.supabase.com:5432/postgres +SUPABASE_SERVICE_KEY=f37f35e070475d4003ea0973cc15ef8bd9956fd140c80d247a187f8e5b0d69d70a9555decd28ea405051bf31d1d1f949dba277f058ba7c0279359ccdeda0f0696ea803403b8ad76dbbf45c4220b45a44a66e643bf0ca575dffc69f22a57c7d6c693e4d55b5f02e8a0da192065a38b24cbed2234d005661beba6d58e3ef234e0f +SUPABASE_S3_STORAGE_ENDPOINT=${SUPABASE_URL}/storage/v1/s3 +SUPABASE_STORAGE_BUCKET=my-bucket +SUPABASE_REGION=us-west-1 +SUPABASE_S3_ACCESS_KEY_ID=323157dcde28202bda94ff4db4be5266 +SUPABASE_S3_SECRET_ACCESS_KEY=d37c900e43e9dfb2c9998fa65aaeea703014504bbfebfddbcf286ee7197dc975 + +# Storage (aliases for compatibility) +STORAGE_URL=https://b8991834720f5477910eded7.supabase.co/storage/v1/s3 +STORAGE_BUCKET=my-bucket +STORAGE_ACCESS_KEY=FEvMws2HMGW96oBMx6Cg98pP8k3h4eki +STORAGE_SECRET_KEY=shq7peEUeYkdzuUDohoK6qx9Zpjvjq6Zz2coUDvyQARM3qk9QryKZmQqRmz4szzM +STORAGE_REGION=us-west-1 +STORAGE_SKIP_BUCKET_CHECK=true + +# Authentication +ACCESS_TOKEN_SECRET=ghp_c9d4307ceb82d06b522c1a5e37a8b5d0BMwJpgMT +REFRESH_TOKEN_SECRET=09cb1b7920aea0d2b63ae3264e27595225ca7132f92f4cc5eff6dc066957118d +JWT_ALGORITHM=HS256 + +# Mail +MAIL_FROM=noreply@example.com + +# Chrome Browser +CHROME_TOKEN=ac126eb015837628b05ff2f0f568ff46 +CHROME_PROXY_HOST=chrome +CHROME_PROXY_PORT=3002 +CHROME_PROXY_SSL=false +CHROME_HEALTH=true +CHROME_PORT=8080 + +# Test Configuration (for e2e) +TEST_HOST=${BASE_HOSTNAME} +TEST_TIMEOUT=35 +TEST_EMAIL=test@example.com +TEST_PASSWORD=changeme +POSTGRES_PORT=5432 +MINIO_PORT=9000 +REDIS_PORT=6379 + +# Database and Storage Paths +SQLITE_DB_PATH=database.db +TEST_DB_PATH=tests/testdb.duckdb +STATIC_FILES_DIR=/app/static + +# AI +OPENAI_API_KEY = "sk-proj-a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0" +COHERE_API_KEY = "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0" +OR_API_KEY = "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0" +AZURE_API_KEY = "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6" +GEMINI_API_KEY = "AIzaSyA1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r" +VERTEXAI_API_KEY = "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0" +REPLICATE_API_KEY = "r8_a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9" +REPLICATE_API_TOKEN = "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0" +ANTHROPIC_API_KEY = "sk-ant-a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0u1v2w3x4y5z6a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6" +INFISICAL_TOKEN = "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0" +NOVITA_API_KEY = "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0" +INFINITY_API_KEY = "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0" diff --git a/tests/catch_dotenv_test.py b/tests/catch_dotenv_test.py index a2f16106..5e545603 100644 --- a/tests/catch_dotenv_test.py +++ b/tests/catch_dotenv_test.py @@ -311,6 +311,8 @@ def boom(*a, **k): os.chdir(cwd) # hook still blocks; but example creation failed -> message should not claim Example file generated assert ok is True - out = capsys.readouterr().out + captured = capsys.readouterr() + out = captured.out + err = captured.err assert 'Example file generated' not in out - assert 'ERROR: unable to write' in out + assert 'ERROR: unable to write' in err From c7f0dae9a41088745f8787ca7d4a23d051c5516a Mon Sep 17 00:00:00 2001 From: Chris Rowe Date: Thu, 28 Aug 2025 20:53:45 -0600 Subject: [PATCH 4/8] Refactor catch_dotenv hook and tests for improved readability and consistency --- pre_commit_hooks/catch_dotenv.py | 120 +++++++++++----- setup.cfg | 1 + tests/catch_dotenv_test.py | 238 +++++++++++++++++++++++-------- 3 files changed, 260 insertions(+), 99 deletions(-) diff --git a/pre_commit_hooks/catch_dotenv.py b/pre_commit_hooks/catch_dotenv.py index 1131f6de..f85c23b5 100644 --- a/pre_commit_hooks/catch_dotenv.py +++ b/pre_commit_hooks/catch_dotenv.py @@ -6,16 +6,16 @@ import re import sys import tempfile +from collections.abc import Iterable from collections.abc import Sequence -from typing import Iterable # Defaults / constants -DEFAULT_ENV_FILE = ".env" -DEFAULT_GITIGNORE_FILE = ".gitignore" -DEFAULT_EXAMPLE_ENV_FILE = ".env.example" -GITIGNORE_BANNER = "# Added by pre-commit hook to prevent committing secrets" +DEFAULT_ENV_FILE = '.env' +DEFAULT_GITIGNORE_FILE = '.gitignore' +DEFAULT_EXAMPLE_ENV_FILE = '.env.example' +GITIGNORE_BANNER = '# Added by pre-commit hook to prevent committing secrets' -_KEY_REGEX = re.compile(r"^\s*(?:export\s+)?([A-Za-z_][A-Za-z0-9_]*)\s*=") +_KEY_REGEX = re.compile(r'^\s*(?:export\s+)?([A-Za-z_][A-Za-z0-9_]*)\s*=') def _atomic_write(path: str, data: str) -> None: @@ -28,16 +28,16 @@ def _atomic_write(path: str, data: str) -> None: parallel (tests exercise concurrent normalization). Keeping this helper local avoids adding any dependency. """ - fd, tmp_path = tempfile.mkstemp(dir=os.path.dirname(path) or ".") + fd, tmp_path = tempfile.mkstemp(dir=os.path.dirname(path) or '.') try: - with os.fdopen(fd, "w", encoding="utf-8", newline="") as tmp_f: + with os.fdopen(fd, 'w', encoding='utf-8', newline='') as tmp_f: tmp_f.write(data) os.replace(tmp_path, path) finally: # Clean up if replace failed if os.path.exists(tmp_path): # (rare failure case) try: os.remove(tmp_path) - except OSError: + except OSError: pass @@ -45,38 +45,51 @@ def _read_gitignore(gitignore_file: str) -> tuple[str, list[str]]: """Read and parse .gitignore file content.""" try: if os.path.exists(gitignore_file): - with open(gitignore_file, "r", encoding="utf-8") as f: + with open(gitignore_file, encoding='utf-8') as f: original_text = f.read() lines = original_text.splitlines() else: - original_text = "" + original_text = '' lines = [] except OSError as exc: - print(f"ERROR: unable to read {gitignore_file}: {exc}", file=sys.stderr) + print( + f"ERROR: unable to read {gitignore_file}: {exc}", + file=sys.stderr, + ) raise - return original_text if lines else "", lines + return original_text if lines else '', lines -def _normalize_gitignore_lines(lines: list[str], env_file: str, banner: str) -> list[str]: - """Normalize .gitignore lines by removing duplicates and adding canonical tail.""" +def _normalize_gitignore_lines( + lines: list[str], + env_file: str, + banner: str, +) -> list[str]: + """Normalize .gitignore lines by removing duplicates and canonical tail.""" # Trim trailing blank lines while lines and not lines[-1].strip(): lines.pop() # Remove existing occurrences - filtered: list[str] = [ln for ln in lines if ln.strip() not in {env_file, banner}] + filtered: list[str] = [ + ln for ln in lines if ln.strip() not in {env_file, banner} + ] if filtered and filtered[-1].strip(): - filtered.append("") # ensure single blank before banner + filtered.append('') # ensure single blank before banner elif not filtered: # empty file -> still separate section visually - filtered.append("") + filtered.append('') filtered.append(banner) filtered.append(env_file) return filtered -def ensure_env_in_gitignore(env_file: str, gitignore_file: str, banner: str) -> bool: +def ensure_env_in_gitignore( + env_file: str, + gitignore_file: str, + banner: str, +) -> bool: """Ensure canonical banner + env tail in .gitignore. Returns True only when the file content was changed. Returns False both @@ -89,27 +102,30 @@ def ensure_env_in_gitignore(env_file: str, gitignore_file: str, banner: str) -> return False filtered = _normalize_gitignore_lines(lines, env_file, banner) - new_content = "\n".join(filtered) + "\n" + new_content = '\n'.join(filtered) + '\n' # Normalize original content to a single trailing newline for comparison normalized_original = original_content_str - if normalized_original and not normalized_original.endswith("\n"): - normalized_original += "\n" + if normalized_original and not normalized_original.endswith('\n'): + normalized_original += '\n' if new_content == normalized_original: return False try: _atomic_write(gitignore_file, new_content) return True - except OSError as exc: - print(f"ERROR: unable to write {gitignore_file}: {exc}", file=sys.stderr) + except OSError as exc: + print( + f"ERROR: unable to write {gitignore_file}: {exc}", + file=sys.stderr, + ) return False def create_example_env(src_env: str, example_file: str) -> bool: """Generate .env.example with unique KEY= lines (no values).""" try: - with open(src_env, "r", encoding="utf-8") as f_env: + with open(src_env, encoding='utf-8') as f_env: lines = f_env.readlines() except OSError as exc: print(f"ERROR: unable to read {src_env}: {exc}", file=sys.stderr) @@ -136,33 +152,51 @@ def create_example_env(src_env: str, example_file: str) -> bool: ] body = [f"{k}=" for k in keys] try: - _atomic_write(example_file, "\n".join(header + body) + "\n") + _atomic_write(example_file, '\n'.join(header + body) + '\n') return True except OSError as exc: # pragma: no cover - print(f"ERROR: unable to write '{example_file}': {exc}", file=sys.stderr) + print( + f"ERROR: unable to write '{example_file}': {exc}", + file=sys.stderr, + ) return False def _has_env(filenames: Iterable[str], env_file: str) -> bool: - """Return True if any staged path refers to a target env file by basename.""" + """Return True if any staged path refers to target env file by basename.""" return any(os.path.basename(name) == env_file for name in filenames) -def _print_failure(env_file: str, gitignore_file: str, example_created: bool, gitignore_modified: bool) -> None: +def _print_failure( + env_file: str, + gitignore_file: str, + example_created: bool, + gitignore_modified: bool, +) -> None: # Match typical hook output style: one short line per action. print(f"Blocked committing {env_file}.") if gitignore_modified: print(f"Updated {gitignore_file}.") if example_created: - print("Generated .env.example.") + print('Generated .env.example.') print(f"Remove {env_file} from the commit and retry.") def main(argv: Sequence[str] | None = None) -> int: """Hook entry-point.""" - parser = argparse.ArgumentParser(description="Blocks committing .env files.") - parser.add_argument('filenames', nargs='*', help='Staged filenames (supplied by pre-commit).') - parser.add_argument('--create-example', action='store_true', help='Generate example env file (.env.example).') + parser = argparse.ArgumentParser( + description='Blocks committing .env files.', + ) + parser.add_argument( + 'filenames', + nargs='*', + help='Staged filenames (supplied by pre-commit).', + ) + parser.add_argument( + '--create-example', + action='store_true', + help='Generate example env file (.env.example).', + ) args = parser.parse_args(argv) env_file = DEFAULT_ENV_FILE # Use current working directory as repository root (pre-commit executes @@ -175,14 +209,26 @@ def main(argv: Sequence[str] | None = None) -> int: if not _has_env(args.filenames, env_file): return 0 - gitignore_modified = ensure_env_in_gitignore(env_file, gitignore_file, GITIGNORE_BANNER) + gitignore_modified = ensure_env_in_gitignore( + env_file, + gitignore_file, + GITIGNORE_BANNER, + ) example_created = False if args.create_example: # Source env is always looked up relative to repo root if os.path.exists(env_abspath): - example_created = create_example_env(env_abspath, example_file) - - _print_failure(env_file, gitignore_file, example_created, gitignore_modified) + example_created = create_example_env( + env_abspath, + example_file, + ) + + _print_failure( + env_file, + gitignore_file, + example_created, + gitignore_modified, + ) return 1 # Block commit diff --git a/setup.cfg b/setup.cfg index 14f7a91c..65a319c4 100644 --- a/setup.cfg +++ b/setup.cfg @@ -29,6 +29,7 @@ exclude = [options.entry_points] console_scripts = + catch-dotenv = pre_commit_hooks.catch_dotenv:main check-added-large-files = pre_commit_hooks.check_added_large_files:main check-ast = pre_commit_hooks.check_ast:main check-builtin-literals = pre_commit_hooks.check_builtin_literals:main diff --git a/tests/catch_dotenv_test.py b/tests/catch_dotenv_test.py index 5e545603..4f0363d6 100644 --- a/tests/catch_dotenv_test.py +++ b/tests/catch_dotenv_test.py @@ -1,18 +1,24 @@ from __future__ import annotations import os +import re +import shutil import threading import time from pathlib import Path -import shutil -import re + import pytest -from pre_commit_hooks.catch_dotenv import main, ensure_env_in_gitignore, GITIGNORE_BANNER, DEFAULT_ENV_FILE, DEFAULT_EXAMPLE_ENV_FILE, DEFAULT_GITIGNORE_FILE +from pre_commit_hooks.catch_dotenv import DEFAULT_ENV_FILE +from pre_commit_hooks.catch_dotenv import DEFAULT_EXAMPLE_ENV_FILE +from pre_commit_hooks.catch_dotenv import DEFAULT_GITIGNORE_FILE +from pre_commit_hooks.catch_dotenv import ensure_env_in_gitignore +from pre_commit_hooks.catch_dotenv import GITIGNORE_BANNER +from pre_commit_hooks.catch_dotenv import main -# Tests cover hook behavior: detection gating, .gitignore normalization, example -# file generation parsing edge cases, idempotency, and preservation of existing -# content. Each test isolates a single behavioral contract. +# Tests cover hook behavior: detection gating, .gitignore normalization, +# example file generation parsing edge cases, idempotency, and preservation of +# existing content. Each test isolates a single behavioral contract. @pytest.fixture() @@ -25,13 +31,18 @@ def env_file(tmp_path: Path) -> Path: # __file__ => /tests/catch_dotenv_test.py # parents[0] = /tests, parents[1] = # Source file stored as test.env in repo (cannot commit a real .env in CI) - resource_env = Path(__file__).resolve().parents[1] / 'testing' / 'resources' / 'test.env' + resource_env = ( + Path(__file__).resolve().parents[1] / + 'testing' / 'resources' / 'test.env' + ) dest = tmp_path / DEFAULT_ENV_FILE shutil.copyfile(resource_env, dest) return dest -def run_hook(tmp_path: Path, staged: list[str], create_example: bool = False) -> int: +def run_hook( + tmp_path: Path, staged: list[str], create_example: bool = False, +) -> int: cwd = os.getcwd() os.chdir(tmp_path) try: @@ -43,13 +54,15 @@ def run_hook(tmp_path: Path, staged: list[str], create_example: bool = False) -> os.chdir(cwd) -def test_no_env_file(tmp_path: Path, env_file: Path): +def test_no_env_file(tmp_path: Path, env_file: Path) -> None: """Hook should no-op (return 0) if .env not staged even if it exists.""" (tmp_path / 'foo.txt').write_text('x') assert run_hook(tmp_path, ['foo.txt']) == 0 -def test_blocks_env_and_updates_gitignore(tmp_path: Path, env_file: Path): +def test_blocks_env_and_updates_gitignore( + tmp_path: Path, env_file: Path, +) -> None: """Staging .env triggers block (exit 1) and appends banner + env entry.""" ret = run_hook(tmp_path, [DEFAULT_ENV_FILE]) assert ret == 1 @@ -58,12 +71,12 @@ def test_blocks_env_and_updates_gitignore(tmp_path: Path, env_file: Path): assert gi[-1] == DEFAULT_ENV_FILE -def test_env_present_but_not_staged(tmp_path: Path, env_file: Path): +def test_env_present_but_not_staged(tmp_path: Path, env_file: Path) -> None: """Existing .env on disk but not staged should not block commit.""" assert run_hook(tmp_path, ['unrelated.txt']) == 0 -def test_idempotent_gitignore(tmp_path: Path, env_file: Path): +def test_idempotent_gitignore(tmp_path: Path, env_file: Path) -> None: """Re-running after initial normalization leaves .gitignore unchanged.""" g = tmp_path / DEFAULT_GITIGNORE_FILE g.write_text(f"{GITIGNORE_BANNER}\n{DEFAULT_ENV_FILE}\n") @@ -75,10 +88,14 @@ def test_idempotent_gitignore(tmp_path: Path, env_file: Path): assert g.read_text() == content1 # unchanged -def test_gitignore_with_existing_content_preserved(tmp_path: Path, env_file: Path): +def test_gitignore_with_existing_content_preserved( + tmp_path: Path, env_file: Path, +) -> None: """Existing entries stay intact; banner/env appended at end cleanly.""" g = tmp_path / DEFAULT_GITIGNORE_FILE - g.write_text('node_modules/\n# comment line\n') # no trailing newline section markers + g.write_text( + 'node_modules/\n# comment line\n', + ) # no trailing newline section markers run_hook(tmp_path, [DEFAULT_ENV_FILE]) lines = g.read_text().splitlines() # original content should still be at top @@ -88,10 +105,15 @@ def test_gitignore_with_existing_content_preserved(tmp_path: Path, env_file: Pat assert lines[-2:] == [GITIGNORE_BANNER, DEFAULT_ENV_FILE] -def test_gitignore_duplicates_are_collapsed(tmp_path: Path, env_file: Path): +def test_gitignore_duplicates_are_collapsed( + tmp_path: Path, env_file: Path, +) -> None: """Multiple prior duplicate banner/env lines collapse to single pair.""" g = tmp_path / DEFAULT_GITIGNORE_FILE - g.write_text(f"other\n{GITIGNORE_BANNER}\n{DEFAULT_ENV_FILE}\n{GITIGNORE_BANNER}\n{DEFAULT_ENV_FILE}\n\n\n") + g.write_text( + f"other\n{GITIGNORE_BANNER}\n{DEFAULT_ENV_FILE}\n" + f"{GITIGNORE_BANNER}\n{DEFAULT_ENV_FILE}\n\n\n", + ) run_hook(tmp_path, [DEFAULT_ENV_FILE]) lines = g.read_text().splitlines() assert lines.count(GITIGNORE_BANNER) == 1 @@ -99,7 +121,7 @@ def test_gitignore_duplicates_are_collapsed(tmp_path: Path, env_file: Path): assert lines[-2:] == [GITIGNORE_BANNER, DEFAULT_ENV_FILE] -def test_create_example(tmp_path: Path, env_file: Path): +def test_create_example(tmp_path: Path, env_file: Path) -> None: """Example file includes discovered keys; values stripped to KEY=.""" ret = run_hook(tmp_path, [DEFAULT_ENV_FILE], create_example=True) assert ret == 1 @@ -108,12 +130,20 @@ def test_create_example(tmp_path: Path, env_file: Path): # All key lines should be KEY= assert all(re.match(r'^[A-Za-z_][A-Za-z0-9_]*=$', ln) for ln in key_lines) # Spot check a few known keys from resource file - for k in ['BACKEND_CONTAINER_PORT=', 'ACCESS_TOKEN_SECRET=', 'SUPABASE_SERVICE_KEY=']: + for k in [ + 'OPENAI_API_KEY=', + 'ACCESS_TOKEN_SECRET=', + 'SUPABASE_SERVICE_KEY=', + ]: assert k in key_lines -def test_create_example_duplicate_key_variant_ignored(tmp_path: Path, env_file: Path): - """Appending whitespace duplicate of existing key should not duplicate in example.""" +def test_create_example_duplicate_key_variant_ignored( + tmp_path: Path, env_file: Path, +) -> None: + """Appending whitespace duplicate of existing key should not duplicate + in example. + """ with open(env_file, 'a', encoding='utf-8') as f: f.write('BACKEND_CONTAINER_PORT =999 # duplicate variant\n') run_hook(tmp_path, [DEFAULT_ENV_FILE], create_example=True) @@ -122,7 +152,9 @@ def test_create_example_duplicate_key_variant_ignored(tmp_path: Path, env_file: assert key_lines.count('BACKEND_CONTAINER_PORT=') == 1 -def test_gitignore_without_trailing_newline(tmp_path: Path, env_file: Path): +def test_gitignore_without_trailing_newline( + tmp_path: Path, env_file: Path, +) -> None: """Normalization works when original .gitignore lacks trailing newline.""" g = tmp_path / DEFAULT_GITIGNORE_FILE g.write_text('existing_line') # no newline at EOF @@ -132,11 +164,20 @@ def test_gitignore_without_trailing_newline(tmp_path: Path, env_file: Path): assert lines[-2:] == [GITIGNORE_BANNER, DEFAULT_ENV_FILE] -def test_ensure_env_in_gitignore_normalizes(tmp_path: Path, env_file: Path): - """Direct API call collapses duplicates and produces canonical tail layout.""" +def test_ensure_env_in_gitignore_normalizes( + tmp_path: Path, env_file: Path, +) -> None: + """Direct API call collapses duplicates and produces canonical tail + layout. + """ g = tmp_path / DEFAULT_GITIGNORE_FILE - g.write_text(f"{GITIGNORE_BANNER}\n{DEFAULT_ENV_FILE}\n{GITIGNORE_BANNER}\n{DEFAULT_ENV_FILE}\n\n") - modified = ensure_env_in_gitignore(DEFAULT_ENV_FILE, str(g), GITIGNORE_BANNER) + g.write_text( + f"{GITIGNORE_BANNER}\n{DEFAULT_ENV_FILE}\n" + f"{GITIGNORE_BANNER}\n{DEFAULT_ENV_FILE}\n\n", + ) + modified = ensure_env_in_gitignore( + DEFAULT_ENV_FILE, str(g), GITIGNORE_BANNER, + ) assert modified is True lines = g.read_text().splitlines() # final two lines should be banner + env @@ -146,37 +187,55 @@ def test_ensure_env_in_gitignore_normalizes(tmp_path: Path, env_file: Path): assert lines.count(DEFAULT_ENV_FILE) == 1 -def test_source_env_file_not_modified(tmp_path: Path, env_file: Path): +def test_source_env_file_not_modified( + tmp_path: Path, env_file: Path, +) -> None: """Hook must not alter original .env (comments and formatting stay).""" original = env_file.read_text() run_hook(tmp_path, [DEFAULT_ENV_FILE], create_example=True) assert env_file.read_text() == original -def test_large_resource_env_parsing(tmp_path: Path, env_file: Path): - """Generate example from resource env; assert broad key coverage & format.""" +def test_large_resource_env_parsing( + tmp_path: Path, env_file: Path, +) -> None: + """Generate example from resource env; assert broad key coverage & + format. + """ ret = run_hook(tmp_path, [DEFAULT_ENV_FILE], create_example=True) assert ret == 1 - example_lines = (tmp_path / DEFAULT_EXAMPLE_ENV_FILE).read_text().splitlines() + example_lines = ( + (tmp_path / DEFAULT_EXAMPLE_ENV_FILE).read_text().splitlines() + ) key_lines = [ln for ln in example_lines if ln and not ln.startswith('#')] assert len(key_lines) > 20 assert all(re.match(r'^[A-Za-z_][A-Za-z0-9_]*=$', ln) for ln in key_lines) - for k in ['BACKEND_CONTAINER_PORT=', 'SUPABASE_SERVICE_KEY=', 'ACCESS_TOKEN_SECRET=']: + for k in [ + 'BACKEND_CONTAINER_PORT=', + 'SUPABASE_SERVICE_KEY=', + 'ACCESS_TOKEN_SECRET=', + ]: assert k in key_lines -def test_failure_message_content(tmp_path: Path, env_file: Path, capsys): +def test_failure_message_content( + tmp_path: Path, + env_file: Path, + capsys: pytest.CaptureFixture[str], +) -> None: """Hook stdout message should contain key phrases when blocking commit.""" ret = run_hook(tmp_path, [DEFAULT_ENV_FILE], create_example=True) assert ret == 1 out = capsys.readouterr().out.strip() - assert "Blocked committing" in out + assert 'Blocked committing' in out assert DEFAULT_GITIGNORE_FILE in out # updated path appears - assert "Generated .env.example." in out - assert "Remove .env" in out + assert 'Generated .env.example.' in out + assert 'Remove .env' in out -def test_create_example_when_env_missing(tmp_path: Path, env_file: Path): +def test_create_example_when_env_missing( + tmp_path: Path, env_file: Path, +) -> None: """--create-example with no .env staged or present should no-op (exit 0). Uses env_file fixture (requirement: all tests use fixture) then removes the @@ -188,18 +247,28 @@ def test_create_example_when_env_missing(tmp_path: Path, env_file: Path): assert not (tmp_path / DEFAULT_EXAMPLE_ENV_FILE).exists() -def test_gitignore_is_directory_error(tmp_path: Path, env_file: Path, capsys): - """If .gitignore path is a directory, hook should print error and still block.""" +def test_gitignore_is_directory_error( + tmp_path: Path, + env_file: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + """If .gitignore path is a directory, hook should print error and still + block. + """ gitignore_dir = tmp_path / DEFAULT_GITIGNORE_FILE gitignore_dir.mkdir() ret = run_hook(tmp_path, [DEFAULT_ENV_FILE]) assert ret == 1 # still blocks commit captured = capsys.readouterr() - assert "ERROR:" in captured.err # error now printed to stderr + assert 'ERROR:' in captured.err # error now printed to stderr -def test_env_example_overwrites_existing(tmp_path: Path, env_file: Path): - """Pre-existing example file with junk should be overwritten with header & keys.""" +def test_env_example_overwrites_existing( + tmp_path: Path, env_file: Path, +) -> None: + """Pre-existing example file with junk should be overwritten with header + & keys. + """ example = tmp_path / DEFAULT_EXAMPLE_ENV_FILE example.write_text('junk=1\nSHOULD_NOT_REMAIN=2\n') run_hook(tmp_path, [DEFAULT_ENV_FILE], create_example=True) @@ -210,12 +279,17 @@ def test_env_example_overwrites_existing(tmp_path: Path, env_file: Path): assert 'SHOULD_NOT_REMAIN=2' not in content -def test_large_gitignore_normalization_performance(tmp_path: Path, env_file: Path): +def test_large_gitignore_normalization_performance( + tmp_path: Path, env_file: Path, +) -> None: """Very large .gitignore remains normalized quickly (functional smoke).""" g = tmp_path / DEFAULT_GITIGNORE_FILE # Generate many lines with scattered duplicates of banner/env - lines = [f"file_{i}" for i in range(3000)] + [GITIGNORE_BANNER, DEFAULT_ENV_FILE] * 3 - g.write_text("\n".join(lines) + "\n") + lines = ( + [f"file_{i}" for i in range(3000)] + + [GITIGNORE_BANNER, DEFAULT_ENV_FILE] * 3 + ) + g.write_text('\n'.join(lines) + '\n') start = time.time() run_hook(tmp_path, [DEFAULT_ENV_FILE]) elapsed = time.time() - start @@ -223,12 +297,17 @@ def test_large_gitignore_normalization_performance(tmp_path: Path, env_file: Pat assert result_lines[-2:] == [GITIGNORE_BANNER, DEFAULT_ENV_FILE] assert result_lines.count(GITIGNORE_BANNER) == 1 assert result_lines.count(DEFAULT_ENV_FILE) == 1 - # Soft performance expectation: should finish fast (< 0.5s on typical dev machine) + # Soft performance expectation: should finish fast + # (< 0.5s on typical dev machine) assert elapsed < 0.5 -def test_concurrent_gitignore_writes(tmp_path: Path, env_file: Path): - """Concurrent ensure_env_in_gitignore calls result in canonical final state.""" +def test_concurrent_gitignore_writes( + tmp_path: Path, env_file: Path, +) -> None: + """Concurrent ensure_env_in_gitignore calls result in canonical final + state. + """ g = tmp_path / DEFAULT_GITIGNORE_FILE # Seed with messy duplicates g.write_text(f"other\n{GITIGNORE_BANNER}\n{DEFAULT_ENV_FILE}\n\n") @@ -247,8 +326,12 @@ def worker(): assert lines.count(DEFAULT_ENV_FILE) == 1 -def test_mixed_staged_files(tmp_path: Path, env_file: Path): - """Staging .env with other files still blocks and only normalizes gitignore once.""" +def test_mixed_staged_files( + tmp_path: Path, env_file: Path, +) -> None: + """Staging .env with other files still blocks and only normalizes + gitignore once. + """ other = tmp_path / 'README.md' other.write_text('hi') ret = run_hook(tmp_path, [DEFAULT_ENV_FILE, 'README.md']) @@ -257,18 +340,29 @@ def test_mixed_staged_files(tmp_path: Path, env_file: Path): assert lines[-2:] == [GITIGNORE_BANNER, DEFAULT_ENV_FILE] -def test_already_ignored_env_with_variations(tmp_path: Path, env_file: Path): - """Pre-existing ignore lines with spacing normalize to single canonical pair.""" +def test_already_ignored_env_with_variations( + tmp_path: Path, env_file: Path, +) -> None: + """Pre-existing ignore lines with spacing normalize to single + canonical pair. + """ g = tmp_path / DEFAULT_GITIGNORE_FILE - g.write_text(f" {DEFAULT_ENV_FILE} \n{GITIGNORE_BANNER}\n {DEFAULT_ENV_FILE}\n") + g.write_text( + f" {DEFAULT_ENV_FILE} \n{GITIGNORE_BANNER}\n" + f" {DEFAULT_ENV_FILE}\n", + ) run_hook(tmp_path, [DEFAULT_ENV_FILE]) lines = g.read_text().splitlines() assert lines[-2:] == [GITIGNORE_BANNER, DEFAULT_ENV_FILE] assert lines.count(DEFAULT_ENV_FILE) == 1 -def test_subdirectory_invocation(tmp_path: Path, env_file: Path): - """Running from a subdirectory now writes .gitignore relative to CWD (simplified behavior).""" +def test_subdirectory_invocation( + tmp_path: Path, env_file: Path, +) -> None: + """Running from a subdirectory now writes .gitignore relative to CWD + (simplified behavior). + """ sub = tmp_path / 'subdir' sub.mkdir() # simulate repository root marker @@ -277,7 +371,9 @@ def test_subdirectory_invocation(tmp_path: Path, env_file: Path): cwd = os.getcwd() os.chdir(sub) try: - ret = main(['../' + DEFAULT_ENV_FILE]) # staged path relative to subdir + ret = main( + ['../' + DEFAULT_ENV_FILE], + ) # staged path relative to subdir gi = (sub / DEFAULT_GITIGNORE_FILE).read_text().splitlines() finally: os.chdir(cwd) @@ -285,31 +381,49 @@ def test_subdirectory_invocation(tmp_path: Path, env_file: Path): assert gi[-2:] == [GITIGNORE_BANNER, DEFAULT_ENV_FILE] -def test_atomic_write_failure_gitignore(monkeypatch, tmp_path: Path, env_file: Path, capsys): - """Simulate os.replace failure during gitignore write to exercise error path.""" - def boom(*a, **k): +def test_atomic_write_failure_gitignore( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + env_file: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + """Simulate os.replace failure during gitignore write to exercise error + path. + """ + def boom(*_a: object, **_k: object) -> None: raise OSError('replace-fail') monkeypatch.setattr('pre_commit_hooks.catch_dotenv.os.replace', boom) - modified = ensure_env_in_gitignore(DEFAULT_ENV_FILE, str(tmp_path / DEFAULT_GITIGNORE_FILE), GITIGNORE_BANNER) + modified = ensure_env_in_gitignore( + DEFAULT_ENV_FILE, + str(tmp_path / DEFAULT_GITIGNORE_FILE), + GITIGNORE_BANNER, + ) assert modified is False captured = capsys.readouterr() assert 'ERROR: unable to write' in captured.err -def test_atomic_write_failure_example(monkeypatch, tmp_path: Path, env_file: Path, capsys): +def test_atomic_write_failure_example( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + env_file: Path, + capsys: pytest.CaptureFixture[str], +) -> None: """Simulate os.replace failure when writing example env file.""" - def boom(*a, **k): + def boom(*_a: object, **_k: object) -> None: raise OSError('replace-fail') monkeypatch.setattr('pre_commit_hooks.catch_dotenv.os.replace', boom) ok = False - # create_example_env requires source .env to exist; env_file fixture provides it in tmp_path root + # create_example_env requires source .env to exist; env_file fixture + # provides it in tmp_path root cwd = os.getcwd() os.chdir(tmp_path) try: ok = main([DEFAULT_ENV_FILE, '--create-example']) == 1 finally: os.chdir(cwd) - # hook still blocks; but example creation failed -> message should not claim Example file generated + # hook still blocks; but example creation failed -> message should + # not claim Example file generated assert ok is True captured = capsys.readouterr() out = captured.out From 3e8b0c9e1a3fdf73692651816c2c9b5aae4179d2 Mon Sep 17 00:00:00 2001 From: Chris Rowe Date: Thu, 28 Aug 2025 21:03:14 -0600 Subject: [PATCH 5/8] register catch-dotenv hook --- .pre-commit-hooks.yaml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml index a98a1e57..fa385c6c 100644 --- a/.pre-commit-hooks.yaml +++ b/.pre-commit-hooks.yaml @@ -23,6 +23,13 @@ entry: check-builtin-literals language: python types: [python] +- id: catch-dotenv + name: catch dotenv files + description: blocks committing .env files. optionally creates value-sanitized .env.example. + entry: catch-dotenv + language: python + pass_filenames: true + always_run: false - id: check-case-conflict name: check for case conflicts description: checks for files that would conflict in case-insensitive filesystems. From 476396a2b9ab0260609f38a67836984ac587338d Mon Sep 17 00:00:00 2001 From: Chris Rowe Date: Thu, 28 Aug 2025 21:25:38 -0600 Subject: [PATCH 6/8] add catch-dotenv hook to README and improve test clarity --- README.md | 6 ++++++ pre_commit_hooks/catch_dotenv.py | 2 +- tests/catch_dotenv_test.py | 4 ++-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 9ee16774..2a7c19c5 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,12 @@ Require literal syntax when initializing empty or zero Python builtin types. - Ignore this requirement for specific builtin types with `--ignore=type1,type2,…`. - Forbid `dict` keyword syntax with `--no-allow-dict-kwargs`. +#### `catch-dotenv` +Prevents committing `.env` files to version control and optionally generates `.env.example` files. + - Use `--create-example` to generate a `.env.example` file with variable names but no values. + - Automatically adds `.env` to `.gitignore` if not already present. + - Helps prevent accidental exposure of secrets and sensitive configuration. + #### `check-case-conflict` Check for files with names that would conflict on a case-insensitive filesystem like MacOS HFS+ or Windows FAT. diff --git a/pre_commit_hooks/catch_dotenv.py b/pre_commit_hooks/catch_dotenv.py index f85c23b5..fedbe4c0 100644 --- a/pre_commit_hooks/catch_dotenv.py +++ b/pre_commit_hooks/catch_dotenv.py @@ -57,7 +57,7 @@ def _read_gitignore(gitignore_file: str) -> tuple[str, list[str]]: file=sys.stderr, ) raise - return original_text if lines else '', lines + return original_text, lines def _normalize_gitignore_lines( diff --git a/tests/catch_dotenv_test.py b/tests/catch_dotenv_test.py index 4f0363d6..399cffde 100644 --- a/tests/catch_dotenv_test.py +++ b/tests/catch_dotenv_test.py @@ -95,7 +95,7 @@ def test_gitignore_with_existing_content_preserved( g = tmp_path / DEFAULT_GITIGNORE_FILE g.write_text( 'node_modules/\n# comment line\n', - ) # no trailing newline section markers + ) # existing content with trailing newline run_hook(tmp_path, [DEFAULT_ENV_FILE]) lines = g.read_text().splitlines() # original content should still be at top @@ -372,7 +372,7 @@ def test_subdirectory_invocation( os.chdir(sub) try: ret = main( - ['../' + DEFAULT_ENV_FILE], + [str(Path('..') / DEFAULT_ENV_FILE)], ) # staged path relative to subdir gi = (sub / DEFAULT_GITIGNORE_FILE).read_text().splitlines() finally: From 206d0a31e7232b569964942c27ac17a911dfabee Mon Sep 17 00:00:00 2001 From: Chris Rowe Date: Thu, 28 Aug 2025 21:38:58 -0600 Subject: [PATCH 7/8] Refactor env_file fixture to dynamically locate repository root and improve test isolation --- tests/catch_dotenv_test.py | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/tests/catch_dotenv_test.py b/tests/catch_dotenv_test.py index 399cffde..e7eb8d3a 100644 --- a/tests/catch_dotenv_test.py +++ b/tests/catch_dotenv_test.py @@ -28,13 +28,18 @@ def env_file(tmp_path: Path) -> Path: All tests rely on this baseline content (optionally appending extra lines for edge cases) to ensure consistent parsing behavior. """ - # __file__ => /tests/catch_dotenv_test.py - # parents[0] = /tests, parents[1] = + # Find repository root by looking for .git directory + test_file_path = Path(__file__).resolve() + repo_root = test_file_path + while repo_root.parent != repo_root: # Stop at filesystem root + if (repo_root / '.git').exists(): + break + repo_root = repo_root.parent + else: + raise RuntimeError('Could not find repository root (.git directory)') + # Source file stored as test.env in repo (cannot commit a real .env in CI) - resource_env = ( - Path(__file__).resolve().parents[1] / - 'testing' / 'resources' / 'test.env' - ) + resource_env = repo_root / 'testing' / 'resources' / 'test.env' dest = tmp_path / DEFAULT_ENV_FILE shutil.copyfile(resource_env, dest) return dest @@ -144,8 +149,15 @@ def test_create_example_duplicate_key_variant_ignored( """Appending whitespace duplicate of existing key should not duplicate in example. """ - with open(env_file, 'a', encoding='utf-8') as f: + # Create a copy of the env_file to avoid contaminating the fixture + modified_env = tmp_path / 'modified.env' + shutil.copyfile(env_file, modified_env) + with open(modified_env, 'a', encoding='utf-8') as f: f.write('BACKEND_CONTAINER_PORT =999 # duplicate variant\n') + + # Override the env file path for this test + original_env = tmp_path / DEFAULT_ENV_FILE + shutil.copyfile(modified_env, original_env) run_hook(tmp_path, [DEFAULT_ENV_FILE], create_example=True) lines = (tmp_path / DEFAULT_EXAMPLE_ENV_FILE).read_text().splitlines() key_lines = [ln for ln in lines if ln and not ln.startswith('#')] From 423fd2344b590608ec34e7630d9a2169ee91a256 Mon Sep 17 00:00:00 2001 From: Chris Rowe Date: Thu, 28 Aug 2025 22:14:59 -0600 Subject: [PATCH 8/8] Add tests for error handling in env file operations and example creation for 100% cov --- tests/catch_dotenv_test.py | 106 +++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/tests/catch_dotenv_test.py b/tests/catch_dotenv_test.py index e7eb8d3a..4ad5d007 100644 --- a/tests/catch_dotenv_test.py +++ b/tests/catch_dotenv_test.py @@ -442,3 +442,109 @@ def boom(*_a: object, **_k: object) -> None: err = captured.err assert 'Example file generated' not in out assert 'ERROR: unable to write' in err + + +def test_atomic_write_cleanup_failure( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + env_file: Path, +) -> None: + """Test rare case where os.remove fails during cleanup after os.replace + failure. + """ + def failing_remove(_path: str) -> None: + # Simulate os.remove failure during cleanup + raise OSError('remove-fail') + + def failing_replace(*_a: object, **_k: object) -> None: + # First fail os.replace to trigger cleanup path + raise OSError('replace-fail') + + monkeypatch.setattr( + 'pre_commit_hooks.catch_dotenv.os.replace', failing_replace, + ) + monkeypatch.setattr( + 'pre_commit_hooks.catch_dotenv.os.remove', failing_remove, + ) + + # This should not raise an exception even if both replace and remove fail + modified = ensure_env_in_gitignore( + DEFAULT_ENV_FILE, + str(tmp_path / DEFAULT_GITIGNORE_FILE), + GITIGNORE_BANNER, + ) + assert modified is False + + +def test_create_example_read_error( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + env_file: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + """Test OSError when reading source env file for create_example.""" + def failing_open(*_args: object, **_kwargs: object) -> None: + raise OSError('Permission denied') + + # Mock open to fail when trying to read the env file + monkeypatch.setattr('builtins.open', failing_open) + + from pre_commit_hooks.catch_dotenv import create_example_env + + result = create_example_env(str(env_file), str(tmp_path / 'test.example')) + assert result is False + + captured = capsys.readouterr() + assert 'ERROR: unable to read' in captured.err + + +def test_malformed_env_lines_ignored(tmp_path: Path, env_file: Path) -> None: + """Test that malformed env lines that don't match regex are ignored.""" + # Create env file with malformed lines + malformed_env = tmp_path / 'malformed.env' + malformed_content = [ + 'VALID_KEY=value', + 'invalid-line-no-equals', + '# comment line', + '', # empty line + '=INVALID_EQUALS_FIRST', + 'ANOTHER_VALID=value2', + 'spaces in key=invalid', + '123_INVALID_START=value', # starts with number + ] + malformed_env.write_text('\n'.join(malformed_content)) + + # Copy to .env location + shutil.copyfile(malformed_env, tmp_path / DEFAULT_ENV_FILE) + + # Run create-example - should only extract valid keys + run_hook(tmp_path, [DEFAULT_ENV_FILE], create_example=True) + + example_lines = ( + (tmp_path / DEFAULT_EXAMPLE_ENV_FILE).read_text().splitlines() + ) + key_lines = [ln for ln in example_lines if ln and not ln.startswith('#')] + + # Should only have the valid keys + assert 'VALID_KEY=' in key_lines + assert 'ANOTHER_VALID=' in key_lines + assert len([k for k in key_lines if '=' in k]) == 2 # Only 2 valid keys + + +def test_create_example_when_source_missing( + tmp_path: Path, env_file: Path, +) -> None: + """Test --create-example when source .env doesn't exist but .env is + staged. + """ + # Remove the source .env file but keep it in the staged files list + env_file.unlink() # Remove the .env file + + # Stage .env even though it doesn't exist on disk + ret = run_hook(tmp_path, [DEFAULT_ENV_FILE], create_example=True) + + # Hook should still block commit + assert ret == 1 + + # But no example file should be created since source doesn't exist + assert not (tmp_path / DEFAULT_EXAMPLE_ENV_FILE).exists()