From c2f9c993f26321a429c393f375ad4c298a08fad8 Mon Sep 17 00:00:00 2001 From: domverse Date: Mon, 17 Aug 2026 23:43:38 +0200 Subject: [PATCH] chore: run the app from a venv and read configuration from a dotenv MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Running the application depended on work_item/scripts/python, which is the work-item helper's launcher: it probes for conda with `command -v`, cannot see a lazily-defined shell function, and falls back to a bare system interpreter with none of the dependencies installed. The application now installs into an ordinary virtualenv instead. That exposed two packaging faults. Pillow, numpy, and scipy were declared test-only although the application imports them at runtime — thumbnails decode through Pillow and the perceptual hash is a DCT over decoded pixels — and an editable install failed outright because setuptools could not choose between photo_pipeline, migrations, and work_item. Both are fixed here, and the OpenAI client moves to a `vision` extra so a review-only install does not pull an API client it never calls. Config.from_env now reads `.env`, or the file named by PHOTO_PIPELINE_ENV_FILE, before it looks at the environment. The file is parsed, never executed: KEY=value lines, comments, optional quotes, no interpolation and no export. Anything already exported wins, so the file is standing configuration and the shell stays the override for one run. The archived CLI's names are mapped as aliases (LLM_API_KEY/GEMINI_API_KEY, LLM_BASE_URL, LIBRARY), so an existing photo_analyzer.env configures the application unchanged. .env, *.env, .venv/, and *.egg-info/ are ignored: the real configuration file holds a live provider key and must never be committed. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01SH4M1PHPwJsM9btcg8hzCs --- .gitignore | 9 +++++ README.md | 39 ++++++++++++++++++-- photo_pipeline/config.py | 58 +++++++++++++++++++++++++++++ pyproject.toml | 26 ++++++++++--- tests/story_traceability.json | 5 ++- tests/unit/test_env_file.py | 69 +++++++++++++++++++++++++++++++++++ 6 files changed, 195 insertions(+), 11 deletions(-) create mode 100644 tests/unit/test_env_file.py diff --git a/.gitignore b/.gitignore index 6ebfe19..36775dc 100644 --- a/.gitignore +++ b/.gitignore @@ -20,3 +20,12 @@ _IGNORE/ # Test failure evidence (US07-04) .artifacts/ + +# Any dotenv, not only the default name. +*.env + +# Local virtualenv for running the app. +.venv/ + +# setuptools editable-install metadata. +*.egg-info/ diff --git a/README.md b/README.md index 6d97d30..c3564a4 100644 --- a/README.md +++ b/README.md @@ -7,16 +7,49 @@ archive workflow. Planning lives in `INTEGRATED_PIPELINE_CONCEPT.md` and ## Application (`photo_pipeline`) The target application lives in `photo_pipeline/` (FastAPI + SQLAlchemy + Alembic). -Run it with: +Install it into a virtualenv once: ```bash -python -m photo_pipeline migrate # apply database migrations -python -m photo_pipeline serve # start the API + static review UI (127.0.0.1:8000) +python3.12 -m venv .venv +.venv/bin/pip install -e ".[vision]" # drop [vision] for a review-only install ``` +Then run the two processes: + +```bash +.venv/bin/python -m photo_pipeline migrate # apply database migrations +.venv/bin/python -m photo_pipeline serve # API + review UI at 127.0.0.1:8000/app/ +.venv/bin/python -m photo_pipeline worker # second terminal: runs the jobs +``` + +The server enqueues work and serves the UI; nothing actually scans, scores, +analyses, uploads, or archives without a worker. `work_item/scripts/python` is the +*helper's* launcher — it prefers Conda base and falls back to a bare system +interpreter, so it is not how the application is run. + Configuration comes from `PHOTO_PIPELINE_*` environment variables (see `photo_pipeline/config.py`); secrets are referenced, never logged. +### Configuration file + +`.env` in the working directory is read at startup, or any path named by +`PHOTO_PIPELINE_ENV_FILE`. It is parsed, never executed: `KEY=value` lines, +`#` comments, optional quotes — no interpolation and no `export`. **Anything already +exported wins**, so the file is the standing configuration and the shell is the +override for one run. + +The archived CLI's variable names still work, so an existing `photo_analyzer.env` +can be used as-is: + +| in the file | applied as | +|---|---| +| `LLM_API_KEY` / `GEMINI_API_KEY` | `OPENAI_API_KEY` | +| `LLM_BASE_URL` | `OPENAI_BASE_URL` | +| `LIBRARY` | `PHOTO_PIPELINE_LIBRARY_ROOTS` | + +`.env` and `*.env` are gitignored and denied by the work-item safety checks: the +file holds a real key and must never be committed. + ### API access (US07-02) The app listens on loopback, so its attacker is another page in the same browser. diff --git a/photo_pipeline/config.py b/photo_pipeline/config.py index a483c49..6a9dd1b 100644 --- a/photo_pipeline/config.py +++ b/photo_pipeline/config.py @@ -18,6 +18,61 @@ from typing import Mapping from pydantic import BaseModel, ConfigDict, SecretStr ENV_PREFIX = "PHOTO_PIPELINE_" +ENV_FILE_VAR = f"{ENV_PREFIX}ENV_FILE" +DEFAULT_ENV_FILE = Path(".env") + +# The archived CLI's variable names, so the configuration file an operator already +# has keeps working. The vision provider reads the OpenAI SDK's names, and the +# library root is configuration here rather than a bare path (US07-01 donor). +LEGACY_ALIASES = { + "LLM_API_KEY": "OPENAI_API_KEY", + "GEMINI_API_KEY": "OPENAI_API_KEY", + "LLM_BASE_URL": "OPENAI_BASE_URL", + "LIBRARY": f"{ENV_PREFIX}LIBRARY_ROOTS", +} + + +def parse_env_file(text: str) -> dict[str, str]: + """``KEY=value`` lines into a mapping. Comments, blanks, and quotes handled. + + Deliberately not a shell: no interpolation, no ``export``, no multi-line values. + A configuration file that can run code is a configuration file that can be a + vulnerability. + """ + values: dict[str, str] = {} + for line in text.splitlines(): + line = line.strip() + if not line or line.startswith("#") or "=" not in line: + continue + key, _, raw = line.partition("=") + key = key.strip() + if not key or key.startswith("#"): + continue + value = raw.strip().strip('"').strip("'") + values[key] = value + alias = LEGACY_ALIASES.get(key) + if alias: + values.setdefault(alias, value) + return values + + +def load_env_file(path: Path | str | None = None) -> dict[str, str]: + """Load ``PHOTO_PIPELINE_ENV_FILE`` (or ``./.env``) into the environment. + + Anything already exported wins: a file is the standing configuration, the shell + is what you meant *this time*. Returns what it applied, which is what the CLI + prints — names only, never values. + """ + candidate = path or os.environ.get(ENV_FILE_VAR) or DEFAULT_ENV_FILE + candidate = Path(candidate) + if not candidate.is_file(): + return {} + applied = {} + for key, value in parse_env_file(candidate.read_text()).items(): + if key not in os.environ: + os.environ[key] = value + applied[key] = value + return applied class Config(BaseModel): @@ -68,6 +123,9 @@ class Config(BaseModel): @classmethod def from_env(cls, environ: Mapping[str, str] | None = None) -> "Config": env = os.environ if environ is None else environ + if environ is None: + load_env_file() # a file never overrides what the shell already set + env = os.environ data: dict = {} for name in cls.model_fields: raw = env.get(ENV_PREFIX + name.upper()) diff --git a/pyproject.toml b/pyproject.toml index 6a4900a..8ec5e04 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,18 +8,32 @@ dependencies = [ "sqlalchemy>=2.0", "alembic>=1.13", "pydantic>=2.7", -] - -[project.optional-dependencies] -test = [ - "pytest>=8", - "httpx>=0.27", + # Imaging is runtime, not test-only: thumbnails decode through Pillow, and the + # perceptual hash is a DCT over the decoded pixels (services/hashing.py). "pillow>=10", "numpy>=1.26", "scipy>=1.11", +] + +[project.optional-dependencies] +# The cloud vision provider. Optional because the analysis stage is the only thing +# that needs it, and a local review-only install should not pull an API client. +vision = ["openai>=1.30"] +test = [ + "pytest>=8", + "httpx>=0.27", "playwright>=1.40", "pytest-playwright>=0.4", ] + +[build-system] +requires = ["setuptools>=68"] +build-backend = "setuptools.build_meta" + +[tool.setuptools] +# The importable application. ``migrations`` and ``work_item`` live beside it but +# are not part of the package; without this, an editable install cannot guess. +packages = ["photo_pipeline"] # Browser end-to-end tests also require: python -m playwright install chromium [tool.ruff] diff --git a/tests/story_traceability.json b/tests/story_traceability.json index a36c1c6..658c160 100644 --- a/tests/story_traceability.json +++ b/tests/story_traceability.json @@ -15,9 +15,10 @@ "tests/characterization/test_webapp_query.py" ], "US01-02": [ - "tests/unit/test_config.py", + "tests/integration/test_app_lifecycle.py", "tests/integration/test_migrations.py", - "tests/integration/test_app_lifecycle.py" + "tests/unit/test_config.py", + "tests/unit/test_env_file.py" ], "US01-03": [ "tests/unit/test_path_policy.py", diff --git a/tests/unit/test_env_file.py b/tests/unit/test_env_file.py new file mode 100644 index 0000000..2657b51 --- /dev/null +++ b/tests/unit/test_env_file.py @@ -0,0 +1,69 @@ +"""Configuration from a dotenv file, including the archived CLI's variable names. + +An operator who already has a ``photo_analyzer.env`` should not have to rewrite it +to run the application it was replaced by. The file is standing configuration; the +shell is what you meant this time, so the shell always wins. +""" + +from __future__ import annotations + +import os + +from photo_pipeline.config import Config, load_env_file, parse_env_file + +SAMPLE = """ +# The archived CLI's shape, comments and all. +LLM_API_KEY=not-real +LLM_BASE_URL="https://example.invalid/v1beta/openai/" +LLM_MODEL='gemini-2.5-flash' +LIBRARY=/tmp/pictures + +MAX_WORKERS=4 +# commented=ignored +malformed line without an equals sign +""" + + +def test_the_file_is_parsed_and_never_executed(): + values = parse_env_file(SAMPLE) + + assert values["LLM_BASE_URL"] == "https://example.invalid/v1beta/openai/" # quotes stripped + assert values["LLM_MODEL"] == "gemini-2.5-flash" + assert values["MAX_WORKERS"] == "4" + assert "commented" not in values and "malformed line without an equals sign" not in values + + +def test_the_archived_cli_names_still_configure_the_application(): + values = parse_env_file(SAMPLE) + + assert values["OPENAI_API_KEY"] == "not-real" + assert values["OPENAI_BASE_URL"] == "https://example.invalid/v1beta/openai/" + assert values["PHOTO_PIPELINE_LIBRARY_ROOTS"] == "/tmp/pictures" + + +def test_an_explicit_shell_variable_beats_the_file(tmp_path, monkeypatch): + path = tmp_path / "photo_analyzer.env" + path.write_text(SAMPLE) + monkeypatch.setenv("OPENAI_API_KEY", "from-the-shell") + monkeypatch.delenv("PHOTO_PIPELINE_LIBRARY_ROOTS", raising=False) + + applied = load_env_file(path) + + assert "OPENAI_API_KEY" not in applied, "the file overrode an exported value" + assert os.environ["OPENAI_API_KEY"] == "from-the-shell" + assert os.environ["PHOTO_PIPELINE_LIBRARY_ROOTS"] == "/tmp/pictures" + assert Config.from_env().library_roots[0].name == "pictures" + + +def test_the_file_is_found_through_its_variable(tmp_path, monkeypatch): + path = tmp_path / "custom.env" + path.write_text("PHOTO_PIPELINE_PORT=9123\n") + monkeypatch.delenv("PHOTO_PIPELINE_PORT", raising=False) + monkeypatch.setenv("PHOTO_PIPELINE_ENV_FILE", str(path)) + monkeypatch.chdir(tmp_path) # no ./.env here, so only the variable can find it + + assert Config.from_env().port == 9123 + + +def test_a_missing_file_is_not_an_error(tmp_path): + assert load_env_file(tmp_path / "nothing-here.env") == {}