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") == {}