From 989ac68f299a6fd8f0f34de9cf416a15d2ad32c4 Mon Sep 17 00:00:00 2001 From: Chris Rowe Date: Thu, 28 Aug 2025 20:29:12 -0600 Subject: [PATCH] Improve error handling and clarity in catch_dotenv hook and tests --- pre_commit_hooks/catch_dotenv.py | 73 ++++++++++++++++++++-------- testing/resources/test.env | 82 ++++++++++++++++++++++++++++++++ tests/catch_dotenv_test.py | 6 ++- 3 files changed, 138 insertions(+), 23 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 6e1a418..1131f6d 100644 --- a/pre_commit_hooks/catch_dotenv.py +++ b/pre_commit_hooks/catch_dotenv.py @@ -19,22 +19,30 @@ _KEY_REGEX = re.compile(r"^\s*(?:export\s+)?([A-Za-z_][A-Za-z0-9_]*)\s*=") 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 - 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")): + +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" + + # 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 0000000..1479aed --- /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 a2f1610..5e54560 100644 --- a/tests/catch_dotenv_test.py +++ b/tests/catch_dotenv_test.py @@ -311,6 +311,8 @@ def test_atomic_write_failure_example(monkeypatch, tmp_path: Path, env_file: Pat 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