diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index baa631b..06926da 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -93,6 +93,9 @@ jobs: - name: CLI config and Docker planning tests run: uv run pytest tests/unit/test_cli.py tests/unit/test_graph_manifest.py -q + - name: Minimum CLI dependency compatibility + run: uv run python scripts/test_minimum_cli_dependencies.py + embedded-seekdb-smoke: name: Embedded SeekDB Smoke runs-on: ubuntu-latest @@ -135,6 +138,9 @@ jobs: - name: CLI Docker smoke run: make test-cli-docker + - name: Container environment boundary smoke + run: uv run python scripts/test_container_env_boundary.py + sample-graphs: name: Sample Graphs runs-on: ubuntu-latest diff --git a/pyproject.toml b/pyproject.toml index 188459c..e3c94f1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,6 +25,7 @@ dependencies = [ "pymysql>=1.1.0", "langchain>=0.3.9", "mcp>=1.27.1,<2", + "python-dotenv>=1.0,<1.3", "scalar-fastapi>=1.0.3", ] diff --git a/scripts/dotenv_conformance.py b/scripts/dotenv_conformance.py new file mode 100644 index 0000000..03f7c08 --- /dev/null +++ b/scripts/dotenv_conformance.py @@ -0,0 +1,285 @@ +"""Shared dotenv interpolation cases for supported-version and handoff tests.""" + +from __future__ import annotations + +import importlib.metadata +import io +import json +import os +import tempfile +from pathlib import Path + +from dotenv.parser import parse_stream +from dotenv.variables import Variable, parse_variables + +DOTENV_CONFORMANCE_CASES = ( + { + "name": "missing", + "contents": "CONF_MISSING=prefix-${PR69_MISSING}-suffix\n", + "ambient": {}, + "container_expected": {"CONF_MISSING": "prefix-${PR69_MISSING}-suffix"}, + "container_absent": (), + }, + { + "name": "duplicate-order", + "contents": ( + "CONF_ORIGIN=https://first.example\n" + "CONF_ORDERED=${CONF_ORIGIN}/v1\n" + "CONF_ORIGIN=https://second.example\n" + ), + "ambient": {}, + "container_expected": { + "CONF_ORIGIN": "https://second.example", + "CONF_ORDERED": "https://first.example/v1", + }, + "container_absent": (), + }, + { + "name": "broad-names", + "contents": "A.B=dotted\n1LEADING=digit\nCONF_BROAD=${A.B}-${1LEADING}\n", + "ambient": {}, + "container_expected": {"CONF_BROAD": "dotted-digit"}, + "container_absent": (), + }, + { + "name": "multiline-default", + "contents": 'CONF_MULTILINE="${PR69_MISSING:-first line\nsecond line}"\n', + "ambient": {}, + "container_expected": {"CONF_MULTILINE": "first line\nsecond line"}, + "container_absent": (), + }, + { + "name": "bare-default", + "contents": "CONF_BARE=$PR69_MISSING\nCONF_DEFAULT=${PR69_MISSING:-fallback}\n", + "ambient": {}, + "container_expected": { + "CONF_BARE": "$PR69_MISSING", + "CONF_DEFAULT": "fallback", + }, + "container_absent": (), + }, + { + "name": "empty-valueless", + "contents": ( + "CONF_EMPTY=\n" + "CONF_VALUELESS\n" + "CONF_FROM_EMPTY=${CONF_EMPTY:-fallback}\n" + "CONF_FROM_VALUELESS=${CONF_VALUELESS:-fallback}\n" + ), + "ambient": {}, + "container_expected": { + "CONF_EMPTY": "", + "CONF_FROM_EMPTY": "", + "CONF_FROM_VALUELESS": "", + }, + "container_absent": ("CONF_VALUELESS",), + }, + { + "name": "allowed-disallowed-ambient", + "contents": ( + "CONF_ALLOWED=${OPENAI_ALLOWED_SOURCE}\n" + "CONF_DISALLOWED=${PR69_DISALLOWED_SECRET}\n" + ), + "ambient": { + "OPENAI_ALLOWED_SOURCE": "allowlisted-source", + "PR69_DISALLOWED_SECRET": "host-sensitive-value", + }, + "container_expected": { + "CONF_ALLOWED": "allowlisted-source", + "CONF_DISALLOWED": "${PR69_DISALLOWED_SECRET}", + }, + "container_absent": ("PR69_DISALLOWED_SECRET",), + }, +) + +CROSS_LAYER_CONFORMANCE_CASES = ( + { + "name": "config-dotenv-reference", + "config_env": "./config.env", + "config_dotenv": "CONF_SOURCE=https://dotenv.example\n", + "cli_dotenv": "CONF_RESULT=${CONF_SOURCE}/v1\n", + "shell_env": {}, + "expected": { + "CONF_SOURCE": "https://dotenv.example", + "CONF_RESULT": "https://dotenv.example/v1", + }, + "absent": (), + }, + { + "name": "config-mapping-reference", + "config_env": {"CONF_SOURCE": "https://mapping.example"}, + "config_dotenv": None, + "cli_dotenv": "CONF_RESULT=${CONF_SOURCE}/v1\n", + "shell_env": {}, + "expected": { + "CONF_SOURCE": "https://mapping.example", + "CONF_RESULT": "https://mapping.example/v1", + }, + "absent": (), + }, + { + "name": "final-allowlisted-shell-override", + "config_env": {"OPENAI_API_KEY": "from-config"}, + "config_dotenv": None, + "cli_dotenv": "CONF_RESULT=${OPENAI_API_KEY}\n", + "shell_env": {"OPENAI_API_KEY": "from-shell"}, + "expected": { + "CONF_RESULT": "from-config", + "OPENAI_API_KEY": "from-shell", + }, + "absent": (), + }, + { + "name": "config-dotenv-valueless", + "config_env": "./config.env", + "config_dotenv": "CONF_TOMBSTONE=from-config-dotenv\n", + "cli_dotenv": "CONF_TOMBSTONE\nCONF_RESULT=${CONF_TOMBSTONE:-fallback}\n", + "shell_env": {}, + "expected": { + "CONF_TOMBSTONE": "from-config-dotenv", + "CONF_RESULT": "", + }, + "absent": (), + }, + { + "name": "config-mapping-valueless", + "config_env": {"CONF_TOMBSTONE": "from-config-mapping"}, + "config_dotenv": None, + "cli_dotenv": "CONF_TOMBSTONE\nCONF_RESULT=${CONF_TOMBSTONE:-fallback}\n", + "shell_env": {}, + "expected": { + "CONF_TOMBSTONE": "from-config-mapping", + "CONF_RESULT": "", + }, + "absent": (), + }, + { + "name": "config-dotenv-valueless-does-not-mask-shell-for-next-file", + "config_env": "./config.env", + "config_dotenv": "OPENAI_API_KEY\n", + "cli_dotenv": "CONF_RESULT=${OPENAI_API_KEY}\n", + "shell_env": {"OPENAI_API_KEY": "from-shell"}, + "expected": { + "OPENAI_API_KEY": "from-shell", + "CONF_RESULT": "from-shell", + }, + "absent": (), + }, +) + +CONFORMANCE_AMBIENT_MODES = ( + ("clean", {}), + ( + "hostile", + { + "OPENAI_API_KEY": "ambient-provider-key", + "OPENAI_ALLOWED_SOURCE": "ambient-allowed-source", + "PR69_MISSING": "ambient-missing-value", + "PR69_DISALLOWED_SECRET": "host-sensitive-value", + "A.B": "ambient-dotted-value", + "1LEADING": "ambient-digit-value", + }, + ), +) + + +def _dotenv_keys(contents: str) -> set[str]: + keys: set[str] = set() + for binding in parse_stream(io.StringIO(contents)): + if binding.key is not None: + keys.add(binding.key) + if binding.value is not None: + keys.update(atom.name for atom in parse_variables(binding.value) if isinstance(atom, Variable)) + return keys + + +def _conformance_env_keys() -> frozenset[str]: + keys: set[str] = set() + for case in DOTENV_CONFORMANCE_CASES: + keys.update(_dotenv_keys(case["contents"])) + keys.update(case["ambient"]) + keys.update(case["container_expected"]) + keys.update(case["container_absent"]) + for case in CROSS_LAYER_CONFORMANCE_CASES: + config_env = case["config_env"] + if isinstance(config_env, dict): + keys.update(config_env) + if case["config_dotenv"] is not None: + keys.update(_dotenv_keys(case["config_dotenv"])) + keys.update(_dotenv_keys(case["cli_dotenv"])) + keys.update(case["shell_env"]) + keys.update(case["expected"]) + keys.update(case["absent"]) + return frozenset(keys) + + +DOTENV_CONFORMANCE_ENV_KEYS = _conformance_env_keys() + + +def assert_runtime_conformance(*, expected_dotenv_version: str | None = None) -> None: + """Compare the runtime loader with the installed python-dotenv version.""" + from dotenv import dotenv_values + + from agentseek_api.cli import build_runtime_env + + if expected_dotenv_version is not None: + assert importlib.metadata.version("python-dotenv") == expected_dotenv_version + + previous = {key: os.environ.get(key) for key in DOTENV_CONFORMANCE_ENV_KEYS} + try: + with tempfile.TemporaryDirectory(prefix="agentseek-dotenv-") as directory: + root = Path(directory) + for mode_name, mode_ambient in CONFORMANCE_AMBIENT_MODES: + for index, case in enumerate(DOTENV_CONFORMANCE_CASES): + for key in DOTENV_CONFORMANCE_ENV_KEYS: + os.environ.pop(key, None) + os.environ.update(mode_ambient) + os.environ.update(case["ambient"]) + env_file = root / f"{mode_name}-{index}.env" + env_file.write_text(case["contents"], encoding="utf-8") + upstream = dotenv_values(env_file) + expected = {key: value for key, value in upstream.items() if value is not None} + expected.update({key: os.environ[key] for key in upstream.keys() & os.environ.keys()}) + actual = build_runtime_env( + config_path=None, + env_file=str(env_file), + cwd=root, + base_env=dict(os.environ), + ) + assertion = f"{mode_name}/{case['name']}" + assert {key: actual[key] for key in expected} == expected, assertion + assert all( + key not in actual + for key, value in upstream.items() + if value is None and key not in os.environ + ), assertion + + for index, case in enumerate(CROSS_LAYER_CONFORMANCE_CASES): + for key in DOTENV_CONFORMANCE_ENV_KEYS: + os.environ.pop(key, None) + os.environ.update(mode_ambient) + os.environ.update(case["shell_env"]) + config_path = root / f"cross-layer-{mode_name}-{index}.json" + config_path.write_text( + json.dumps({"graphs": {"chat": "chat.graph:graph"}, "env": case["config_env"]}), + encoding="utf-8", + ) + if case["config_dotenv"] is not None: + (root / "config.env").write_text(case["config_dotenv"], encoding="utf-8") + env_file = root / f"cross-layer-{mode_name}-{index}.env" + env_file.write_text(case["cli_dotenv"], encoding="utf-8") + actual = build_runtime_env( + config_path=config_path, + env_file=str(env_file), + cwd=root, + base_env=dict(os.environ), + ) + assertion = f"{mode_name}/{case['name']}" + assert {key: actual[key] for key in case["expected"]} == case["expected"], assertion + assert all(key not in actual for key in case["absent"]), assertion + finally: + for key, value in previous.items(): + if value is None: + os.environ.pop(key, None) + else: + os.environ[key] = value diff --git a/scripts/test-cli-docker.sh b/scripts/test-cli-docker.sh index 3998042..d2f1132 100644 --- a/scripts/test-cli-docker.sh +++ b/scripts/test-cli-docker.sh @@ -8,12 +8,14 @@ IMAGE_TAG="${IMAGE_TAG:-agentseek-api-cli-smoke:latest}" DB_CONTAINER="${DB_CONTAINER:-agentseek-cli-mysql}" APP_CONTAINER="${APP_CONTAINER:-agentseek-up-8123}" APP_CONTAINER_AUTOBUILD="${APP_CONTAINER_AUTOBUILD:-agentseek-up-8124}" +APP_CONTAINER_SECURITY="${APP_CONTAINER_SECURITY:-agentseek-up-8125}" PG_CONTAINER="${PG_CONTAINER:-agentseek-cli-postgres}" TMP_DIR="${TMP_DIR:-$ROOT_DIR/.tmp/cli-docker}" cleanup() { docker rm -f "$APP_CONTAINER" >/dev/null 2>&1 || true docker rm -f "$APP_CONTAINER_AUTOBUILD" >/dev/null 2>&1 || true + docker rm -f "$APP_CONTAINER_SECURITY" >/dev/null 2>&1 || true docker rm -f "$DB_CONTAINER" >/dev/null 2>&1 || true docker rm -f "$PG_CONTAINER" >/dev/null 2>&1 || true } @@ -21,6 +23,7 @@ cleanup() { print_logs() { docker logs "$APP_CONTAINER" || true docker logs "$APP_CONTAINER_AUTOBUILD" || true + docker logs "$APP_CONTAINER_SECURITY" || true docker logs "$DB_CONTAINER" || true docker logs "$PG_CONTAINER" || true } @@ -116,6 +119,34 @@ if ! uv run python scripts/verify_docker_api.py --base-url http://127.0.0.1:8123 exit 1 fi +cat >"$TMP_DIR/disallowed.env" <<'EOF' +OPENAI_API_KEY=${PR69_DISALLOWED_SECRET} +EOF + +if ! env -u OPENAI_API_KEY PR69_DISALLOWED_SECRET=host-sensitive-value uv run agentseek-api up \ + --config "$CONFIG_PATH" \ + --image "$IMAGE_TAG" \ + --port 8125 \ + --env-file "$TMP_DIR/disallowed.env" \ + --recreate; then + print_logs + exit 1 +fi + +for _ in $(seq 1 60); do + if docker inspect "$APP_CONTAINER_SECURITY" --format '{{.State.Running}}' 2>/dev/null | grep -q true; then + break + fi + sleep 1 +done + +CONTAINER_SECRET="$(docker exec "$APP_CONTAINER_SECURITY" python -c 'import os; print(os.environ["OPENAI_API_KEY"])')" +if [[ "$CONTAINER_SECRET" != '${PR69_DISALLOWED_SECRET}' || "$CONTAINER_SECRET" == 'host-sensitive-value' ]]; then + print_logs + echo "Container environment expanded a host-only secret." >&2 + exit 1 +fi + DUPLICATE_STDERR="$TMP_DIR/up-duplicate.stderr" set +e uv run agentseek-api up \ diff --git a/scripts/test_container_env_boundary.py b/scripts/test_container_env_boundary.py new file mode 100644 index 0000000..c51202e --- /dev/null +++ b/scripts/test_container_env_boundary.py @@ -0,0 +1,145 @@ +"""Verify the shared dotenv matrix through the real container handoff.""" + +from __future__ import annotations + +import json +import os +import subprocess +import sys +import tempfile +from pathlib import Path + +from dotenv_conformance import ( + CONFORMANCE_AMBIENT_MODES, + CROSS_LAYER_CONFORMANCE_CASES, + DOTENV_CONFORMANCE_CASES, + DOTENV_CONFORMANCE_ENV_KEYS, +) + + +def _inspect_real_up( + *, + root: Path, + config: Path, + env_file: Path, + port: int, + process_env: dict[str, str], +) -> dict[str, str]: + container_name = f"agentseek-up-{port}" + try: + result = subprocess.run( + [ + sys.executable, + "-m", + "agentseek_api.cli", + "up", + "--config", + str(config), + "--image", + "python:3.12-slim", + "--port", + str(port), + "--env-file", + str(env_file), + "--recreate", + ], + cwd=root, + env=process_env, + check=False, + capture_output=True, + text=True, + ) + if result.returncode != 0: + raise RuntimeError(f"agentseek-api up smoke failed: {result.stderr.strip()}") + inspected = subprocess.run( + ["docker", "inspect", container_name, "--format", "{{json .Config.Env}}"], + check=True, + capture_output=True, + text=True, + ) + assert "host-sensitive-value" not in inspected.stdout + return dict(entry.split("=", maxsplit=1) for entry in json.loads(inspected.stdout)) + finally: + subprocess.run( + ["docker", "rm", "-f", container_name], + check=False, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + + +def main() -> None: + from agentseek_api.cli import _CONTAINER_ENV_PREFIXES + + inherited_allowlisted = { + key: value + for key, value in os.environ.items() + if key.startswith(_CONTAINER_ENV_PREFIXES) + } + clean_env = { + key: value + for key, value in os.environ.items() + if not key.startswith(_CONTAINER_ENV_PREFIXES) and key not in DOTENV_CONFORMANCE_ENV_KEYS + } + + with tempfile.TemporaryDirectory(prefix="agentseek-container-env-") as directory: + root = Path(directory) + package = root / "chat" + package.mkdir() + (package / "__init__.py").write_text("", encoding="utf-8") + (package / "graph.py").write_text("graph = object()\n", encoding="utf-8") + + port = 18125 + for mode_name, mode_ambient in CONFORMANCE_AMBIENT_MODES: + for index, case in enumerate(DOTENV_CONFORMANCE_CASES): + config = root / f"matrix-{mode_name}-{index}.json" + config.write_text('{"graphs":{"chat":"chat.graph:graph"}}\n', encoding="utf-8") + env_file = root / f"matrix-{mode_name}-{index}.env" + env_file.write_text(case["contents"], encoding="utf-8") + process_env = {**clean_env, **mode_ambient, **case["ambient"]} + + actual = _inspect_real_up( + root=root, + config=config, + env_file=env_file, + port=port, + process_env=process_env, + ) + + assertion = f"{mode_name}/{case['name']}" + expected = case["container_expected"] + assert {key: actual[key] for key in expected} == expected, assertion + assert all(key not in actual for key in case["container_absent"]), assertion + port += 1 + + for index, case in enumerate(CROSS_LAYER_CONFORMANCE_CASES): + config = root / f"cross-layer-{mode_name}-{index}.json" + config.write_text( + json.dumps({"graphs": {"chat": "chat.graph:graph"}, "env": case["config_env"]}), + encoding="utf-8", + ) + if case["config_dotenv"] is not None: + (root / "config.env").write_text(case["config_dotenv"], encoding="utf-8") + env_file = root / f"cross-layer-{mode_name}-{index}.env" + env_file.write_text(case["cli_dotenv"], encoding="utf-8") + process_env = {**clean_env, **mode_ambient, **case["shell_env"]} + + actual = _inspect_real_up( + root=root, + config=config, + env_file=env_file, + port=port, + process_env=process_env, + ) + + assertion = f"{mode_name}/{case['name']}" + assert {key: actual[key] for key in case["expected"]} == case["expected"], assertion + assert all(key not in actual for key in case["absent"]), assertion + port += 1 + + os.environ.update(inherited_allowlisted) + print("container dotenv conformance passed") + + +if __name__ == "__main__": + main() diff --git a/scripts/test_minimum_cli_dependencies.py b/scripts/test_minimum_cli_dependencies.py new file mode 100644 index 0000000..c048bbb --- /dev/null +++ b/scripts/test_minimum_cli_dependencies.py @@ -0,0 +1,54 @@ +"""Verify the installed CLI runs with the declared minimum dotenv stack.""" + +from __future__ import annotations + +import subprocess +import sys +import tempfile +from pathlib import Path + + +def main() -> None: + repository = Path(__file__).resolve().parents[1] + with tempfile.TemporaryDirectory(prefix="agentseek-minimum-") as directory: + environment = Path(directory) / ".venv" + subprocess.run(["uv", "venv", "--python", sys.executable, str(environment)], check=True) + python = environment / ("Scripts/python.exe" if sys.platform == "win32" else "bin/python") + subprocess.run( + [ + "uv", + "pip", + "install", + "--python", + str(python), + "pydantic-settings==2.4.0", + "pydantic==2.8.0", + "python-dotenv==1.0.0", + ], + check=True, + ) + subprocess.run(["uv", "pip", "install", "--python", str(python), "--no-deps", "-e", str(repository)], check=True) + cli = environment / ("Scripts/agentseek-api.exe" if sys.platform == "win32" else "bin/agentseek-api") + result = subprocess.run([str(cli), "version"], check=True, capture_output=True, text=True) + assert result.stdout.strip() == "agentseek-api 0.2.1" + conformance = subprocess.run( + [ + str(python), + "-c", + ( + "import sys; " + f"sys.path.insert(0, {str(repository / 'scripts')!r}); " + "from dotenv_conformance import assert_runtime_conformance; " + "assert_runtime_conformance(expected_dotenv_version='1.0.0')" + ), + ], + check=False, + capture_output=True, + text=True, + ) + if conformance.returncode != 0: + raise RuntimeError(f"Minimum dependency dotenv conformance failed: {conformance.stderr.strip()}") + + +if __name__ == "__main__": + main() diff --git a/src/agentseek_api/cli.py b/src/agentseek_api/cli.py index 0530d64..d4d059d 100644 --- a/src/agentseek_api/cli.py +++ b/src/agentseek_api/cli.py @@ -14,6 +14,10 @@ from pathlib import Path from typing import TextIO +from dotenv.main import with_warn_for_invalid_lines +from dotenv.parser import parse_stream +from dotenv.variables import Literal, Variable, parse_variables + from agentseek_api import __version__ from agentseek_api.settings import DEFAULT_API_PORT @@ -146,21 +150,118 @@ def discover_config_path(*, explicit_path: str | None, cwd: Path) -> Path | None return None -def _parse_env_file(env_file: Path) -> dict[str, str]: - values: dict[str, str] = {} - for line_number, raw_line in enumerate(env_file.read_text(encoding="utf-8").splitlines(), start=1): - line = raw_line.strip() - if not line or line.startswith("#"): +def _resolve_env_value( + value: str, + *, + context: dict[str, str | None], + preserve_unresolved: bool, +) -> str: + """Resolve a value with python-dotenv's grammar and missing-value rules.""" + atoms = list(parse_variables(value)) + if not preserve_unresolved: + return "".join(atom.resolve(context) for atom in atoms) + + parts: list[str] = [] + for atom in atoms: + if isinstance(atom, Literal): + parts.append(atom.value) continue - if line.startswith("export "): - line = line[len("export ") :].strip() - if "=" not in line: - raise CliError(f"Env file '{env_file}' has an invalid line {line_number}: '{raw_line}'.") - key, value = line.split("=", maxsplit=1) - values[key.strip()] = value.strip().strip("\"'") + if not isinstance(atom, Variable): + raise TypeError(f"Unsupported python-dotenv interpolation atom: {type(atom).__name__}") + if atom.name in context: + parts.append(context[atom.name] or "") + elif atom.default is not None: + parts.append(atom.default) + else: + parts.append(f"${{{atom.name}}}") + return "".join(parts) + + +def _parse_env_file( + env_file: Path, + *, + context: dict[str, str | None], + preserve_unresolved: bool, +) -> dict[str, str | None]: + """Parse and interpolate dotenv bindings in physical source order.""" + local_context = dict(context) + values: dict[str, str | None] = {} + with env_file.open(encoding="utf-8") as stream: + bindings = with_warn_for_invalid_lines(parse_stream(stream)) + for binding in bindings: + if binding.key is None: + continue + if binding.value is None: + # A valueless binding participates in interpolation just as it + # does in python-dotenv, but is not exported to child processes. + local_context[binding.key] = None + values[binding.key] = None + continue + resolved = _resolve_env_value( + binding.value, + context=local_context, + preserve_unresolved=preserve_unresolved, + ) + values[binding.key] = resolved + local_context[binding.key] = resolved + context.update({key: value for key, value in values.items() if value is not None}) return values +def _apply_env_layer(env: dict[str, str], layer: dict[str, str | None]) -> None: + for key, value in layer.items(): + if value is not None: + env[key] = value + + +def _build_env( + *, + config_path: Path | None, + env_file: str | None, + cwd: Path, + shell_env: dict[str, str], + preserve_unresolved: bool, +) -> dict[str, str]: + env: dict[str, str] = {} + interpolation_context: dict[str, str | None] = dict(shell_env) + config: CliConfig | None = _load_cli_config(config_path) if config_path is not None else None + if config is not None: + if config.env_file is not None: + _apply_env_layer( + env, + _parse_env_file( + config.env_file, + context=interpolation_context, + preserve_unresolved=preserve_unresolved, + ), + ) + # JSON env mappings are literal values. They form the next precedence + # layer and are available to interpolation in the CLI dotenv layer. + env.update(config.env_mapping) + interpolation_context.update(config.env_mapping) + if config.auth_path: + env["AUTH_MODULE_PATH"] = config.auth_path + interpolation_context["AUTH_MODULE_PATH"] = config.auth_path + if env_file: + resolved_env_file = _resolve_path(env_file, cwd=cwd) + if not resolved_env_file.exists(): + raise CliError(f"Env file '{resolved_env_file}' does not exist.") + _apply_env_layer( + env, + _parse_env_file( + resolved_env_file, + context=interpolation_context, + preserve_unresolved=preserve_unresolved, + ), + ) + # The launching shell is both the initial interpolation context and the + # highest-precedence output layer. + env.update(shell_env) + if config_path is not None: + env["AGENTSEEK_GRAPHS"] = str(config_path) + return env + + def _resolve_path_from_config(path_text: str, *, config_path: Path) -> Path: path = Path(path_text).expanduser() if not path.is_absolute(): @@ -277,22 +378,14 @@ def build_runtime_env( cwd: Path, base_env: dict[str, str] | None = None, ) -> dict[str, str]: - env = dict(os.environ if base_env is None else base_env) - config: CliConfig | None = _load_cli_config(config_path) if config_path is not None else None - if config is not None: - if config.env_file is not None: - env.update(_parse_env_file(config.env_file)) - env.update(config.env_mapping) - if config.auth_path: - env["AUTH_MODULE_PATH"] = config.auth_path - if env_file: - resolved_env_file = _resolve_path(env_file, cwd=cwd) - if not resolved_env_file.exists(): - raise CliError(f"Env file '{resolved_env_file}' does not exist.") - env.update(_parse_env_file(resolved_env_file)) - if config_path is not None: - env["AGENTSEEK_GRAPHS"] = str(config_path) - return env + shell_env = dict(os.environ if base_env is None else base_env) + return _build_env( + config_path=config_path, + env_file=env_file, + cwd=cwd, + shell_env=shell_env, + preserve_unresolved=False, + ) def build_uvicorn_command(*, host: str, port: int, reload_enabled: bool) -> list[str]: @@ -623,11 +716,12 @@ def _ambient_container_env() -> dict[str, str]: def build_container_env(*, config_path: Path, env_file: str | None, cwd: Path) -> dict[str, str]: - env = build_runtime_env( + env = _build_env( config_path=config_path, env_file=env_file, cwd=cwd, - base_env=_ambient_container_env(), + shell_env=_ambient_container_env(), + preserve_unresolved=True, ) env["AGENTSEEK_GRAPHS"] = _container_config_path(config_path=config_path, cwd=cwd) auth_module_path = env.get("AUTH_MODULE_PATH") diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 71a5de9..c740f62 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -3,13 +3,35 @@ import argparse import importlib import io +import json +import os import tomllib from dataclasses import dataclass from pathlib import Path import pytest +from agentseek_api.cli import _CONTAINER_ENV_PREFIXES from agentseek_api.services.langgraph_service import LangGraphService +from scripts.dotenv_conformance import ( + CROSS_LAYER_CONFORMANCE_CASES, + DOTENV_CONFORMANCE_CASES, + DOTENV_CONFORMANCE_ENV_KEYS, +) + + +@pytest.fixture(autouse=True) +def _clean_ambient_container_env(monkeypatch: pytest.MonkeyPatch) -> None: + """Keep allowlisted host variables out of tests unless a test opts in.""" + for key in tuple(os.environ): + if key.startswith(_CONTAINER_ENV_PREFIXES): + monkeypatch.delenv(key) + + +def test_python_dotenv_dependency_is_available() -> None: + from dotenv import dotenv_values + + assert callable(dotenv_values) @dataclass @@ -223,9 +245,12 @@ def fake_scheduler_main() -> int: assert cli_module.os.environ.get(sentinel_key) == "before" -def test_dev_command_accepts_langgraph_cli_flags_and_env_file(tmp_path: Path) -> None: +def test_dev_command_accepts_langgraph_cli_flags_and_env_file( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: from agentseek_api.cli import main + monkeypatch.delenv("AUTH_MODULE_PATH", raising=False) config_path = _write_basic_langgraph_config(tmp_path) env_file = tmp_path / ".env" env_file.write_text("AUTH_MODULE_PATH=test.module:backend\n", encoding="utf-8") @@ -255,9 +280,13 @@ def test_dev_command_accepts_langgraph_cli_flags_and_env_file(tmp_path: Path) -> assert capture.env["AUTH_MODULE_PATH"] == "test.module:backend" -def test_dev_command_loads_config_env_mapping_and_auth_path(tmp_path: Path) -> None: +def test_dev_command_loads_config_env_mapping_and_auth_path( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: from agentseek_api.cli import main + for key in ("OPENAI_API_KEY", "FEATURE_FLAG", "AUTH_MODULE_PATH"): + monkeypatch.delenv(key, raising=False) package_dir = tmp_path / "chat" package_dir.mkdir() (package_dir / "__init__.py").write_text("", encoding="utf-8") @@ -292,9 +321,13 @@ def test_dev_command_loads_config_env_mapping_and_auth_path(tmp_path: Path) -> N assert capture.env["AUTH_MODULE_PATH"] == f"{(tmp_path / 'auth.py').resolve()}:auth" -def test_dev_command_merges_config_env_file_before_cli_env_file(tmp_path: Path) -> None: +def test_dev_command_merges_config_env_file_before_cli_env_file( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: from agentseek_api.cli import main + for key in ("TOKEN", "SHARED"): + monkeypatch.delenv(key, raising=False) config_path = _write_basic_langgraph_config(tmp_path) config_env = tmp_path / "config.env" config_env.write_text("TOKEN=from-config\nSHARED=config\n", encoding="utf-8") @@ -327,6 +360,33 @@ def test_dev_command_merges_config_env_file_before_cli_env_file(tmp_path: Path) assert capture.env["SHARED"] == "override" +def test_dev_command_preserves_dotenv_default_and_bare_variable_syntax( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + from agentseek_api.cli import main + + monkeypatch.delenv("API_ORIGIN", raising=False) + config_path = _write_basic_langgraph_config(tmp_path) + env_file = tmp_path / "defaults.env" + env_file.write_text( + "OPENAI_BASE_URL=${API_ORIGIN:-https://default.example.test}/v1\n" + "BARE_REFERENCE=$API_ORIGIN\n", + encoding="utf-8", + ) + capture = _RunCapture() + + exit_code = main( + ["dev", "--config", str(config_path), "--env-file", str(env_file), "--no-reload"], + runner=capture, + cwd=tmp_path, + ) + + assert exit_code == 0 + assert capture.env is not None + assert capture.env["OPENAI_BASE_URL"] == "https://default.example.test/v1" + assert capture.env["BARE_REFERENCE"] == "$API_ORIGIN" + + def test_dev_command_rejects_unsupported_langgraph_flags(tmp_path: Path) -> None: from agentseek_api.cli import main @@ -836,31 +896,297 @@ def test_build_command_plans_docker_build_from_generated_dockerfile(tmp_path: Pa assert 'CMD ["python", "-m", "agentseek_api.cli", "serve", "--host", "0.0.0.0", "--port", "2024"]' in generated -def test_build_runtime_env_rejects_invalid_env_lines(tmp_path: Path) -> None: +def test_build_runtime_env_parses_exported_values(tmp_path: Path) -> None: from agentseek_api.cli import build_runtime_env + config_path = _write_basic_langgraph_config(tmp_path) env_file = tmp_path / ".env" - env_file.write_text("BROKEN_LINE\n", encoding="utf-8") + env_file.write_text( + '# comment\nexport TOKEN="quoted # value\nnext"\nPLAIN=value # inline comment\n', + encoding="utf-8", + ) - with pytest.raises(RuntimeError, match="invalid line 1"): - build_runtime_env(config_path=None, env_file=str(env_file), cwd=tmp_path, base_env={}) + env = build_runtime_env(config_path=config_path, env_file=str(env_file), cwd=tmp_path, base_env={}) + assert env["TOKEN"] == "quoted # value\nnext" + assert env["PLAIN"] == "value" + assert env["AGENTSEEK_GRAPHS"] == str(config_path.resolve()) -def test_build_runtime_env_parses_exported_values(tmp_path: Path) -> None: + +def test_build_runtime_env_ignores_dotenv_entries_without_values(tmp_path: Path) -> None: from agentseek_api.cli import build_runtime_env + config_path = _write_basic_langgraph_config(tmp_path) + env_file = tmp_path / ".env" + env_file.write_text("MALFORMED_LINE\nTOKEN=present\n", encoding="utf-8") + + env = build_runtime_env(config_path=config_path, env_file=str(env_file), cwd=tmp_path, base_env={}) + + assert "MALFORMED_LINE" not in env + assert env["TOKEN"] == "present" + + +def test_build_runtime_env_shell_values_override_config_and_cli_dotenv(tmp_path: Path) -> None: + from agentseek_api.cli import build_runtime_env + + config_path = _write_basic_langgraph_config(tmp_path) + config_env = tmp_path / "config.env" + config_env.write_text("TOKEN=from-config\n", encoding="utf-8") + config_path.write_text( + """ +{ + "graphs": {"chat": "chat.graph:graph"}, + "env": "./config.env" +} +""".strip(), + encoding="utf-8", + ) + cli_env = tmp_path / "override.env" + cli_env.write_text("TOKEN=from-cli-file\n", encoding="utf-8") + + env = build_runtime_env( + config_path=config_path, + env_file=str(cli_env), + cwd=tmp_path, + base_env={"TOKEN": "from-shell"}, + ) + + assert env["TOKEN"] == "from-shell" + + +@pytest.mark.parametrize( + "case", + DOTENV_CONFORMANCE_CASES, + ids=[case["name"] for case in DOTENV_CONFORMANCE_CASES], +) +def test_runtime_dotenv_interpolation_conforms_to_python_dotenv( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + case: dict[str, object], +) -> None: + from dotenv import dotenv_values + + from agentseek_api.cli import build_runtime_env + + for key in DOTENV_CONFORMANCE_ENV_KEYS: + monkeypatch.delenv(key, raising=False) + ambient = case["ambient"] + assert isinstance(ambient, dict) + for key, value in ambient.items(): + monkeypatch.setenv(key, value) + env_file = tmp_path / ".env" + contents = case["contents"] + assert isinstance(contents, str) + env_file.write_text(contents, encoding="utf-8") + expected = {key: value for key, value in dotenv_values(env_file).items() if value is not None} + + actual = build_runtime_env( + config_path=None, + env_file=str(env_file), + cwd=tmp_path, + base_env=dict(os.environ), + ) + + assert {key: actual[key] for key in expected} == expected + + +def test_runtime_dotenv_interpolation_sees_prior_layers_before_final_shell_override(tmp_path: Path) -> None: + from agentseek_api.cli import build_runtime_env + + config_env = tmp_path / "config.env" + config_env.write_text("ORIGIN=https://config.example\n", encoding="utf-8") + config_path = tmp_path / "langgraph.json" + config_path.write_text( + '{"graphs":{"chat":"chat.graph:graph"},"env":"./config.env"}', + encoding="utf-8", + ) + cli_env = tmp_path / "cli.env" + cli_env.write_text("RESULT=${ORIGIN}/v1\n", encoding="utf-8") + + env = build_runtime_env( + config_path=config_path, + env_file=str(cli_env), + cwd=tmp_path, + base_env={"ORIGIN": "https://shell.example"}, + ) + + assert env["RESULT"] == "https://config.example/v1" + assert env["ORIGIN"] == "https://shell.example" + + +@pytest.mark.parametrize( + "case", + CROSS_LAYER_CONFORMANCE_CASES, + ids=[case["name"] for case in CROSS_LAYER_CONFORMANCE_CASES], +) +def test_runtime_cross_layer_dotenv_conformance(tmp_path: Path, case: dict[str, object]) -> None: + from agentseek_api.cli import build_runtime_env + + config_path = tmp_path / "langgraph.json" + config_path.write_text( + json.dumps({"graphs": {"chat": "chat.graph:graph"}, "env": case["config_env"]}), + encoding="utf-8", + ) + config_dotenv = case["config_dotenv"] + if isinstance(config_dotenv, str): + (tmp_path / "config.env").write_text(config_dotenv, encoding="utf-8") + cli_env = tmp_path / "cli.env" + cli_dotenv = case["cli_dotenv"] + assert isinstance(cli_dotenv, str) + cli_env.write_text(cli_dotenv, encoding="utf-8") + shell_env = case["shell_env"] + assert isinstance(shell_env, dict) + + actual = build_runtime_env( + config_path=config_path, + env_file=str(cli_env), + cwd=tmp_path, + base_env=shell_env, + ) + + expected = case["expected"] + assert isinstance(expected, dict) + assert {key: actual[key] for key in expected} == expected + absent = case["absent"] + assert isinstance(absent, tuple) + assert all(key not in actual for key in absent) + + +def test_cli_dotenv_interpolation_sees_literal_config_mapping(tmp_path: Path) -> None: + from agentseek_api.cli import build_runtime_env + + config_path = tmp_path / "langgraph.json" + config_path.write_text( + '{"graphs":{"chat":"chat.graph:graph"},"env":{"ORIGIN":"https://mapping.example"}}', + encoding="utf-8", + ) + cli_env = tmp_path / "cli.env" + cli_env.write_text("RESULT=${ORIGIN}/v1\n", encoding="utf-8") + + env = build_runtime_env(config_path=config_path, env_file=str(cli_env), cwd=tmp_path, base_env={}) + + assert env["RESULT"] == "https://mapping.example/v1" + + +def test_higher_precedence_valueless_binding_keeps_lower_export(tmp_path: Path) -> None: + from agentseek_api.cli import build_runtime_env + + config_env = tmp_path / "config.env" + config_env.write_text("TOKEN=from-config\n", encoding="utf-8") + config_path = tmp_path / "langgraph.json" + config_path.write_text( + '{"graphs":{"chat":"chat.graph:graph"},"env":"./config.env"}', + encoding="utf-8", + ) + cli_env = tmp_path / "cli.env" + cli_env.write_text("TOKEN\nRESULT=${TOKEN:-fallback}\n", encoding="utf-8") + + env = build_runtime_env(config_path=config_path, env_file=str(cli_env), cwd=tmp_path, base_env={}) + + assert env["TOKEN"] == "from-config" + assert env["RESULT"] == "" + + +def test_container_dotenv_uses_full_grammar_but_preserves_unavailable_references( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + from agentseek_api.cli import build_container_env + config_path = _write_basic_langgraph_config(tmp_path) env_file = tmp_path / ".env" env_file.write_text( - "# comment\nexport TOKEN='quoted-value'\nPLAIN=value\n", + "A.B=dotted\n" + "1LEADING=digit\n" + "OPENAI_BASE_URL=${A.B}-${1LEADING}-${PR69_DISALLOWED_SECRET}\n", encoding="utf-8", ) + monkeypatch.setenv("PR69_DISALLOWED_SECRET", "host-sensitive-value") - env = build_runtime_env(config_path=config_path, env_file=str(env_file), cwd=tmp_path, base_env={}) + env = build_container_env(config_path=config_path, env_file=str(env_file), cwd=tmp_path) - assert env["TOKEN"] == "quoted-value" - assert env["PLAIN"] == "value" - assert env["AGENTSEEK_GRAPHS"] == str(config_path.resolve()) + assert env["OPENAI_BASE_URL"] == "dotted-digit-${PR69_DISALLOWED_SECRET}" + assert "PR69_DISALLOWED_SECRET" not in env + + +def test_container_dotenv_preserves_physical_duplicate_order(tmp_path: Path) -> None: + from agentseek_api.cli import build_container_env + + config_path = _write_basic_langgraph_config(tmp_path) + env_file = tmp_path / ".env" + env_file.write_text( + "API_ORIGIN=https://first.example\n" + "OPENAI_BASE_URL=${API_ORIGIN}/v1\n" + "API_ORIGIN=https://second.example\n", + encoding="utf-8", + ) + + env = build_container_env(config_path=config_path, env_file=str(env_file), cwd=tmp_path) + + assert env["OPENAI_BASE_URL"] == "https://first.example/v1" + + +@pytest.mark.parametrize("config_env", ["./config.env", {"API_ORIGIN": "https://mapping.example"}]) +def test_container_dotenv_sees_selected_config_layer( + tmp_path: Path, + config_env: str | dict[str, str], +) -> None: + from agentseek_api.cli import build_container_env + + if isinstance(config_env, str): + (tmp_path / "config.env").write_text("API_ORIGIN=https://dotenv.example\n", encoding="utf-8") + expected_origin = "https://dotenv.example" + else: + expected_origin = "https://mapping.example" + config_path = tmp_path / "langgraph.json" + config_path.write_text( + json.dumps({"graphs": {"chat": "chat.graph:graph"}, "env": config_env}), + encoding="utf-8", + ) + cli_env = tmp_path / "cli.env" + cli_env.write_text("OPENAI_BASE_URL=${API_ORIGIN}/v1\n", encoding="utf-8") + + env = build_container_env(config_path=config_path, env_file=str(cli_env), cwd=tmp_path) + + assert env["OPENAI_BASE_URL"] == f"{expected_origin}/v1" + + +def test_container_dotenv_resolves_allowlisted_but_not_disallowed_ambient_reference( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + from agentseek_api.cli import build_container_env + + config_path = _write_basic_langgraph_config(tmp_path) + env_file = tmp_path / ".env" + env_file.write_text( + "ALLOWED_COPY=${OPENAI_API_KEY}\n" + "DISALLOWED_COPY=${PR69_DISALLOWED_SECRET}\n", + encoding="utf-8", + ) + monkeypatch.setenv("OPENAI_API_KEY", "allowlisted-value") + monkeypatch.setenv("PR69_DISALLOWED_SECRET", "host-sensitive-value") + + env = build_container_env(config_path=config_path, env_file=str(env_file), cwd=tmp_path) + + assert env["ALLOWED_COPY"] == "allowlisted-value" + assert env["DISALLOWED_COPY"] == "${PR69_DISALLOWED_SECRET}" + assert "PR69_DISALLOWED_SECRET" not in env + + +def test_build_container_env_does_not_interpolate_disallowed_host_values( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + from agentseek_api.cli import build_container_env + + monkeypatch.setenv("PR69_DISALLOWED_SECRET", "host-sensitive-value") + config_path = _write_basic_langgraph_config(tmp_path) + env_file = tmp_path / ".env" + env_file.write_text("OPENAI_API_KEY=${PR69_DISALLOWED_SECRET}\n", encoding="utf-8") + + env = build_container_env(config_path=config_path, env_file=str(env_file), cwd=tmp_path) + + assert env["OPENAI_API_KEY"] == "${PR69_DISALLOWED_SECRET}" + assert "PR69_DISALLOWED_SECRET" not in env def test_build_runtime_env_rejects_invalid_config_env_shape(tmp_path: Path) -> None: @@ -1071,7 +1397,10 @@ def test_up_command_plans_docker_run_with_recreate_and_env_file(tmp_path: Path) config_path = _write_basic_langgraph_config(tmp_path) env_file = tmp_path / "docker.env" env_file.write_text( - "METADATA_DB_URL=sqlite+aiosqlite:////tmp/agentseek.db\nOCEANBASE_HOST=host.docker.internal\n", + "METADATA_DB_URL=sqlite+aiosqlite:////tmp/agentseek.db\n" + "OCEANBASE_HOST=host.docker.internal\n" + "API_ORIGIN=https://api.example.test\n" + "OPENAI_BASE_URL=${API_ORIGIN}/v1\n", encoding="utf-8", ) capture = _RunCapture() @@ -1112,6 +1441,7 @@ def test_up_command_plans_docker_run_with_recreate_and_env_file(tmp_path: Path) assert container_env["AGENTSEEK_GRAPHS"] == "/deps/agent/langgraph.json" assert container_env["METADATA_DB_URL"] == "sqlite+aiosqlite:////tmp/agentseek.db" assert container_env["OCEANBASE_HOST"] == "host.docker.internal" + assert container_env["OPENAI_BASE_URL"] == "https://api.example.test/v1" def test_up_command_supports_docker_compose_sidecars(tmp_path: Path) -> None: @@ -1338,6 +1668,69 @@ def test_up_command_passes_ambient_env_into_container(tmp_path: Path, monkeypatc assert container_env["OPENAI_API_KEY"] == "ambient-key" +def test_up_command_resolves_same_file_references_without_expanding_disallowed_host_values( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + from agentseek_api.cli import main + + config_path = _write_basic_langgraph_config(tmp_path) + env_file = tmp_path / "docker.env" + env_file.write_text( + "API_ORIGIN=https://api.example.test\n" + "OPENAI_BASE_URL=${API_ORIGIN}/v1\n" + "OPENAI_API_KEY=${PR69_DISALLOWED_SECRET}\n", + encoding="utf-8", + ) + monkeypatch.setenv("PR69_DISALLOWED_SECRET", "host-sensitive-value") + capture = _RunCapture() + + exit_code = main( + [ + "up", + "--config", + str(config_path), + "--image", + "agentseek:test", + "--env-file", + str(env_file), + ], + runner=capture, + cwd=tmp_path, + ) + + assert exit_code == 0 + assert capture.calls is not None + container_env = _docker_env_from_run_command(capture.calls[1]) + assert container_env["OPENAI_BASE_URL"] == "https://api.example.test/v1" + assert container_env["OPENAI_API_KEY"] == "${PR69_DISALLOWED_SECRET}" + assert "PR69_DISALLOWED_SECRET" not in container_env + + +def test_up_command_preserves_dotenv_default_and_bare_variable_syntax(tmp_path: Path) -> None: + from agentseek_api.cli import main + + config_path = _write_basic_langgraph_config(tmp_path) + env_file = tmp_path / "docker.env" + env_file.write_text( + "OPENAI_BASE_URL=${MISSING_API_ORIGIN:-https://default.example.test}/v1\n" + "BARE_REFERENCE=$MISSING_API_ORIGIN\n", + encoding="utf-8", + ) + capture = _RunCapture() + + exit_code = main( + ["up", "--config", str(config_path), "--image", "agentseek:test", "--env-file", str(env_file)], + runner=capture, + cwd=tmp_path, + ) + + assert exit_code == 0 + assert capture.calls is not None + container_env = _docker_env_from_run_command(capture.calls[1]) + assert container_env["OPENAI_BASE_URL"] == "https://default.example.test/v1" + assert container_env["BARE_REFERENCE"] == "$MISSING_API_ORIGIN" + + def test_up_command_prefers_agentseek_json_without_explicit_flag(tmp_path: Path) -> None: diff --git a/uv.lock b/uv.lock index 5d02a21..2ec5ede 100644 --- a/uv.lock +++ b/uv.lock @@ -49,6 +49,7 @@ dependencies = [ { name = "pydantic" }, { name = "pydantic-settings" }, { name = "pymysql" }, + { name = "python-dotenv" }, { name = "redis" }, { name = "scalar-fastapi" }, { name = "sqlalchemy" }, @@ -92,6 +93,7 @@ requires-dist = [ { name = "pydantic", specifier = ">=2.8.0" }, { name = "pydantic-settings", specifier = ">=2.4.0" }, { name = "pymysql", specifier = ">=1.1.0" }, + { name = "python-dotenv", specifier = ">=1.0,<1.3" }, { name = "redis", specifier = ">=5.0.0" }, { name = "scalar-fastapi", specifier = ">=1.0.3" }, { name = "sqlalchemy", specifier = ">=2.0.0" },