Skip to content

Commit 2076f0a

Browse files
author
Dylan Huang
committed
Refactor dotenv loading to use explicit paths in CLI and API modules
- improving environment variable management and preventing conflicts with other .env files.
1 parent c8774a6 commit 2076f0a

File tree

4 files changed

+216
-5
lines changed

4 files changed

+216
-5
lines changed

eval_protocol/cli.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,8 +286,10 @@ def main():
286286
from dotenv import load_dotenv
287287

288288
# .env.dev for development-specific overrides, .env for general
289+
# Use explicit paths to avoid find_dotenv() searching up the directory tree
290+
# and potentially finding a different .env file (e.g., in some other repo)
289291
load_dotenv(dotenv_path=Path(".") / ".env.dev", override=True)
290-
load_dotenv(override=True)
292+
load_dotenv(dotenv_path=Path(".") / ".env", override=True)
291293
except ImportError:
292294
pass
293295

eval_protocol/mcp/mcp_multi_client.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ class FunctionLike(BaseModel):
1313
parameters: Any = None
1414

1515

16-
from dotenv import load_dotenv
1716
from mcp import ClientSession, StdioServerParameters
1817
from mcp.client.stdio import stdio_client
1918
from mcp.client.streamable_http import streamablehttp_client
@@ -26,8 +25,6 @@ class FunctionLike(BaseModel):
2625
MCPMultiClientConfiguration,
2726
)
2827

29-
load_dotenv() # load environment variables from .env
30-
3128

3229
class MCPMultiClient:
3330
"""

eval_protocol/quickstart/svg_agent/vercel_svg_server/api/init.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,14 @@
1313
from flask import Flask, request, jsonify
1414
from openai import OpenAI
1515
import openai
16+
from pathlib import Path
17+
1618
from dotenv import load_dotenv
1719

1820
from eval_protocol import Status, InitRequest, FireworksTracingHttpHandler, RolloutIdFilter
1921

20-
load_dotenv()
22+
# Use explicit path to avoid find_dotenv() searching up the directory tree
23+
load_dotenv(dotenv_path=Path(".") / ".env")
2124

2225
# Configure logging so INFO and below go to stdout, WARNING+ to stderr.
2326
# This avoids Vercel marking INFO logs as [error] (stderr).

tests/test_no_implicit_dotenv.py

Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
"""
2+
Test to ensure load_dotenv() is never called without an explicit path.
3+
4+
When load_dotenv() is called without a dotenv_path argument, it uses find_dotenv()
5+
which searches up the directory tree for a .env file. This can cause unexpected
6+
behavior when running the CLI from a subdirectory, as it may find a .env file
7+
in a parent directory (e.g., the python-sdk repo's .env) instead of the intended
8+
project's .env file.
9+
10+
This test scans all Python files in the SDK to ensure that every call to
11+
load_dotenv() includes an explicit dotenv_path argument.
12+
"""
13+
14+
import ast
15+
import os
16+
from pathlib import Path
17+
from typing import List, Set, Tuple
18+
19+
# Directories to scan for implicit load_dotenv calls
20+
SCAN_DIRECTORIES = [
21+
"eval_protocol",
22+
]
23+
24+
# Directories to exclude from scanning (relative to repo root)
25+
EXCLUDE_DIRECTORIES: Set[str] = {
26+
".venv",
27+
".git",
28+
"__pycache__",
29+
".pytest_cache",
30+
".mypy_cache",
31+
"node_modules",
32+
"build",
33+
"dist",
34+
".eggs",
35+
"*.egg-info",
36+
}
37+
38+
39+
def find_implicit_load_dotenv_calls(file_path: Path) -> List[Tuple[int, str]]:
40+
"""
41+
Parse a Python file and find any load_dotenv() calls without explicit dotenv_path.
42+
43+
Returns a list of (line_number, code_snippet) tuples for violations.
44+
"""
45+
violations = []
46+
47+
try:
48+
with open(file_path, "r", encoding="utf-8") as f:
49+
source = f.read()
50+
except (IOError, UnicodeDecodeError):
51+
return violations
52+
53+
try:
54+
tree = ast.parse(source, filename=str(file_path))
55+
except SyntaxError:
56+
return violations
57+
58+
for node in ast.walk(tree):
59+
if isinstance(node, ast.Call):
60+
# Check if this is a call to load_dotenv
61+
func_name = None
62+
if isinstance(node.func, ast.Name):
63+
func_name = node.func.id
64+
elif isinstance(node.func, ast.Attribute):
65+
func_name = node.func.attr
66+
67+
if func_name == "load_dotenv":
68+
# Check if dotenv_path is provided as a positional or keyword argument
69+
has_explicit_path = False
70+
71+
# Check positional arguments (dotenv_path is the first positional arg)
72+
if node.args:
73+
has_explicit_path = True
74+
75+
# Check keyword arguments
76+
for keyword in node.keywords:
77+
if keyword.arg == "dotenv_path":
78+
has_explicit_path = True
79+
break
80+
81+
if not has_explicit_path:
82+
# Get the source line for context
83+
try:
84+
lines = source.splitlines()
85+
line = lines[node.lineno - 1].strip() if node.lineno <= len(lines) else "<unknown>"
86+
except (IndexError, AttributeError):
87+
line = "<unknown>"
88+
89+
violations.append((node.lineno, line))
90+
91+
return violations
92+
93+
94+
def _should_exclude_dir(dir_name: str) -> bool:
95+
"""Check if a directory should be excluded from scanning."""
96+
return dir_name in EXCLUDE_DIRECTORIES or dir_name.startswith(".")
97+
98+
99+
def _scan_directory(directory: Path, repo_root: Path) -> List[Tuple[Path, int, str]]:
100+
"""Scan a directory for implicit load_dotenv calls."""
101+
all_violations: List[Tuple[Path, int, str]] = []
102+
103+
for root, dirs, files in os.walk(directory):
104+
# Filter out excluded directories in-place to prevent os.walk from descending into them
105+
dirs[:] = [d for d in dirs if not _should_exclude_dir(d)]
106+
107+
for filename in files:
108+
if not filename.endswith(".py"):
109+
continue
110+
111+
file_path = Path(root) / filename
112+
violations = find_implicit_load_dotenv_calls(file_path)
113+
114+
for line_no, code in violations:
115+
all_violations.append((file_path, line_no, code))
116+
117+
return all_violations
118+
119+
120+
def test_no_implicit_load_dotenv_calls():
121+
"""
122+
Ensure no load_dotenv() calls exist without an explicit dotenv_path argument.
123+
124+
This prevents the CLI from accidentally loading .env files from parent directories
125+
when running from a subdirectory.
126+
"""
127+
repo_root = Path(__file__).parent.parent
128+
129+
all_violations: List[Tuple[Path, int, str]] = []
130+
131+
for scan_dir in SCAN_DIRECTORIES:
132+
directory = repo_root / scan_dir
133+
if directory.exists():
134+
violations = _scan_directory(directory, repo_root)
135+
all_violations.extend(violations)
136+
137+
if all_violations:
138+
error_msg = [
139+
"Found load_dotenv() calls without explicit dotenv_path argument.",
140+
"This can cause the CLI to load .env files from parent directories unexpectedly.",
141+
"",
142+
"Violations:",
143+
]
144+
for file_path, line_no, code in all_violations:
145+
try:
146+
rel_path = file_path.relative_to(repo_root)
147+
except ValueError:
148+
rel_path = file_path
149+
error_msg.append(f" {rel_path}:{line_no}: {code}")
150+
151+
error_msg.extend(
152+
[
153+
"",
154+
"Fix by providing an explicit path:",
155+
" load_dotenv(dotenv_path=Path('.') / '.env', override=True)",
156+
"",
157+
]
158+
)
159+
160+
assert False, "\n".join(error_msg)
161+
162+
163+
def test_load_dotenv_ast_detection():
164+
"""Test that our AST detection correctly identifies implicit vs explicit calls."""
165+
import tempfile
166+
167+
# Test case: implicit call (should be detected)
168+
implicit_code = """
169+
from dotenv import load_dotenv
170+
load_dotenv()
171+
load_dotenv(override=True)
172+
load_dotenv(verbose=True, override=True)
173+
"""
174+
175+
# Test case: explicit call (should NOT be detected)
176+
explicit_code = """
177+
from dotenv import load_dotenv
178+
load_dotenv(dotenv_path='.env')
179+
load_dotenv('.env')
180+
load_dotenv(Path('.') / '.env')
181+
load_dotenv(dotenv_path=Path('.') / '.env', override=True)
182+
load_dotenv(env_file_path) # positional arg counts as explicit
183+
"""
184+
185+
with tempfile.NamedTemporaryFile(mode="w", suffix=".py", delete=False) as f:
186+
f.write(implicit_code)
187+
implicit_file = Path(f.name)
188+
189+
with tempfile.NamedTemporaryFile(mode="w", suffix=".py", delete=False) as f:
190+
f.write(explicit_code)
191+
explicit_file = Path(f.name)
192+
193+
try:
194+
implicit_violations = find_implicit_load_dotenv_calls(implicit_file)
195+
explicit_violations = find_implicit_load_dotenv_calls(explicit_file)
196+
197+
# Should find 3 violations in implicit code
198+
assert len(implicit_violations) == 3, (
199+
f"Expected 3 implicit violations, got {len(implicit_violations)}: {implicit_violations}"
200+
)
201+
202+
# Should find 0 violations in explicit code
203+
assert len(explicit_violations) == 0, (
204+
f"Expected 0 explicit violations, got {len(explicit_violations)}: {explicit_violations}"
205+
)
206+
207+
finally:
208+
implicit_file.unlink()
209+
explicit_file.unlink()

0 commit comments

Comments
 (0)