diff --git a/.coveragerc b/.coveragerc index c6decdae..f1fb18dc 100644 --- a/.coveragerc +++ b/.coveragerc @@ -2,6 +2,7 @@ source = audio_library chapters + credential_registry diarize job_store mcp_driver diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 78f2e184..eaf753e7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,7 +36,7 @@ jobs: python -m pip install --disable-pip-version-check --no-index --no-deps --no-build-isolation -e . - name: Compile check - run: python -m py_compile media_shrinker.py config_file.py presets.py saas_web.py mcp_driver.py job_store.py + run: python -m py_compile media_shrinker.py config_file.py presets.py saas_web.py mcp_driver.py job_store.py credential_registry.py - name: Run tests run: python -m unittest discover -s tests -v diff --git a/AGENTS.md b/AGENTS.md index 090d7d2d..53b0d9a7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -43,10 +43,11 @@ repo. job store, and open PRs adding API-key auth and usage metering), so it *will* read runtime secrets/config (API keys, DB creds, endpoints). When you add them, source them from the KV, not `os.getenv`. -- **Known deviation to migrate:** the in-flight API-key auth work reads keys from - a `CODEC_CARVER_API_KEYS` environment variable — that is exactly the anti-pattern - above. Move it to read from the credential registry (env may still be the - bootstrap transport that *populates* the KV, never the runtime source). +- **API keys:** `saas_web.py` request-time auth reads + `credential_registry.CredentialRegistry` only. `CODEC_CARVER_API_KEYS` is + bootstrap transport into `bootstrap_from_mapping` at process start (and in + tests). Do not add request-time `os.getenv("CODEC_CARVER_API_KEYS")` back. + Decision record: `docs/doctoring/api-credential-registry.md`. ### Code exploration - There is no `.codegraph/` index in this repo today, so use normal search diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md new file mode 100644 index 00000000..11fb00ba --- /dev/null +++ b/ARCHITECTURE.md @@ -0,0 +1,70 @@ +# Architecture + +Codec Carver is a standalone conversion service that also works as a git +submodule. The Python CLI, FastAPI upload UI, MCP tool, and Rust +`codec-carver-core` binary can run alone. When naruon or another +ContextualWisdomLab service imports the module, `convert_file` is the +stable in-process port. + +```text + +------------------+ + browser / API -->| saas_web.py | GET / GET /health + | require_api_key |----> credential_registry + | /shrink /jobs | (api_credentials) + +--------+---------+ + | + v + media_shrinker.py ----> ffmpeg / ffprobe + | + +--------------+--------------+ + | | + job_store.py rust-core/ + (jobs table; codec-carver-core + rename to (library CLI) + conversion_jobs is + follow-up debt) +``` + +## Credential port + +`credential_registry.CredentialRegistry` is the provider-neutral +credential port. Other CWL services can depend on this module without +importing FastAPI. Bootstrap transport may be an environment snapshot; +request-time verification may not. See +`docs/doctoring/api-credential-registry.md`. + +## Core ERD (auth + jobs) + +```text +api_credentials + credential_id PK + key_digest UK + lifecycle_status + created_at + updated_at + rotated_at + revoked_at + expires_at + +jobs -- existing; one-word name is known debt + id PK + status + created_at + updated_at + output_path + output_name + error + temp_dir +``` + +Both tables are in 3NF: non-key attributes depend only on the primary +key. Usage metering still keys rows by plaintext `api_key` (`usage`); +rebinding that table to `credential_id` is a later migration. + +## Next actions + +1. Land this registry. Close HMAC-only sentinels (#376, #421) as + superseded once compare_digest no longer sees raw Unicode headers. +2. Keep Cloud Agent environment work on #427; do not mix it here. +3. Rename `jobs` → `conversion_jobs` in a dedicated migration. +4. Add production fail-closed bind policy without request-time env reads. diff --git a/CHANGELOG.md b/CHANGELOG.md index 9313538b..8e2a6755 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,9 +5,11 @@ - 다중 파일 업로드 선택 시 즉각적인 파일 개수 피드백 및 제한 초과 경고 메시지 추가 - 일괄 업로드 폼에 대상 바이트 프리셋 버튼과 총 파일 크기 미리보기를 추가하여 사용성을 개선했습니다. - 클라이언트 측 폼 검증 시 하드코딩된 '5 GiB' 텍스트를 동적으로 변환되도록 수정하고 일괄 업로드 폼에 최대 크기(MAX_UPLOAD_BYTES) 검증 피드백을 추가했습니다. +- `GET /health`는 인증 없이 `{"status":"ok","service":"codec-carver"}`를 반환합니다. 로드 밸런서와 Cloud Agent `start`는 이 URL을 프로브하면 됩니다. ### Changed - 순수 영숫자 토큰은 정규식 호출을 건너뛰되 다국어·문장부호 토큰화 결과는 기존 의미와 동일하게 유지합니다. 근거, 한계, APA 7 참고문헌은 [`docs/doctoring/token-fast-path-equivalence.md`](docs/doctoring/token-fast-path-equivalence.md)에 기록했습니다. +- SaaS API 키 인증은 요청 시점에 환경 변수를 읽지 않습니다. 시작 시 `CODEC_CARVER_API_KEYS`를 부트스트랩 수송으로만 사용해 `api_credentials` 레지스트리에 SHA-256 다이제스트를 넣고, 요청은 UTF-8 다이제스트를 `hmac.compare_digest`로 전량 비교합니다. 운영자는 키를 회전·폐기한 뒤 해당 시크릿을 헤더에서 제거하면 됩니다. 근거는 [`docs/doctoring/api-credential-registry.md`](docs/doctoring/api-credential-registry.md)입니다. ### Fixed - 단일·일괄 대상 크기 입력을 비웠을 때 이전 custom validity와 `aria-invalid` 상태를 즉시 초기화해 현재 필수 입력 상태를 정확히 전달합니다. diff --git a/CLAUDE.md b/CLAUDE.md index cc7870fb..33473a18 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -23,7 +23,7 @@ python3 -m unittest tests.test_media_shrinker -v python3 -m unittest tests.test_job_store.TestCreateAndGet.test_create_get_roundtrip # Compile check (CI runs this on all four modules) -python -m py_compile media_shrinker.py saas_web.py mcp_driver.py job_store.py +python -m py_compile media_shrinker.py saas_web.py mcp_driver.py job_store.py credential_registry.py # CLI (omit --execute for a dry run that only lists candidates) codec-carver /path/to/recordings --execute --output-dir under_2gb @@ -49,7 +49,8 @@ Four flat top-level modules (declared as `py-modules` in `pyproject.toml`; there - **`media_shrinker.py`** — the core engine and CLI, deliberately stdlib-only (external work happens in `ffmpeg`/`ffprobe` subprocesses). The console script `codec-carver` maps to `media_shrinker:main`. Pipeline for a batch run: `find_candidates` scans the root (pruned `os.walk`, excludes the output dir and `--exclude-dir-prefix` dirs) → per file, `convert_file` probes with ffprobe (`probe_media` / `_parse_probe_payload`), detects silence and builds a split plan for long sources (`detect_silence_intervals`, `parse_silencedetect_intervals`, `build_segments`) → each segment gets a `ConversionPlan` (`build_audio_plan` prefers FLAC; `build_opus_plan` is the fallback when a FLAC output exceeds the target size) → `_execute_plan` runs ffmpeg and `preserve_file_attributes` restores permissions/timestamps/xattrs best-effort → `write_report` emits a JSON report. `convert_file(source, root=..., output_dir=..., target_bytes=...)` is the programmatic API that the web and MCP layers call. - **`saas_web.py`** — single-file FastAPI upload UI (the `[web]` extra; what the Docker image serves). Streams one upload into a temp workspace, calls `media_shrinker.convert_file`, and returns the first generated output as a download. Middleware enforces a 5 GiB upload cap and security headers. Processing is synchronous per request. - **`mcp_driver.py`** — FastMCP server (the `[mcp]` extra) exposing a single `shrink_media` tool that wraps `convert_file`. -- **`job_store.py`** — stdlib-only SQLite (WAL) durable job store intended for async/worker job tracking. It is tested but not yet wired into `saas_web.py`. Callers pass `now` explicitly; the store never calls `datetime.now()` itself. +- **`job_store.py`** — stdlib-only SQLite (WAL) durable job store intended for async/worker job tracking. It is tested but not yet wired into `saas_web.py`. Callers pass ``now`` explicitly; the store never calls ``datetime.now()`` itself. +- **`credential_registry.py`** — stdlib-only SQLite (WAL) hashed API-key registry (`api_credentials`). Request-time auth in `saas_web.py` reads only this store. `CODEC_CARVER_API_KEYS` is bootstrap transport into `bootstrap_from_mapping`, never a request-time `os.getenv`. Callers pass ``now`` for expiry and rotation. Supporting directories: `fuzz/` holds Atheris harnesses plus seed corpora for the three untrusted-input parsing surfaces (`parse_silencedetect_intervals`, `_parse_probe_payload`, `build_segments`); the same invariants run as Hypothesis property tests in `tests/test_fuzz_properties.py` so they execute in the normal suite. `docs/papers/` holds the fuzzing survey the harness design references. diff --git a/credential_registry.py b/credential_registry.py new file mode 100644 index 00000000..38ba2461 --- /dev/null +++ b/credential_registry.py @@ -0,0 +1,435 @@ +"""SQLite-backed API credential registry for request-time authentication. + +Runtime authentication reads only this registry. The process environment is +bootstrap transport: callers pass an explicit mapping into +:func:`bootstrap_from_mapping` at startup. Request handlers must not call +``os.getenv``. + +Stored rows keep a SHA-256 digest of the UTF-8 key, a two-word table name +(``api_credentials``), and an explicit lifecycle. Callers pass ``now`` so +expiry and rotation stay deterministic in tests. + +Example:: + + store = CredentialRegistry("/var/lib/carver/api_credentials.db") + bootstrap_from_mapping(store, {"CODEC_CARVER_API_KEYS": "alpha,beta"}, now=now) + credential_id = store.verify(presented_header, now=now) +""" + +from __future__ import annotations + +import hashlib +import hmac +import sqlite3 +import threading +import uuid +from collections.abc import Iterator, Mapping +from contextlib import contextmanager +from datetime import datetime + +#: Transport variable that may populate the registry at startup only. +BOOTSTRAP_ENV_NAME = "CODEC_CARVER_API_KEYS" + +#: Allowed lifecycle values for ``api_credentials.lifecycle_status``. +VALID_LIFECYCLE_STATUSES = frozenset({"active", "rotated", "revoked"}) + +#: Maximum UTF-8 size of one API key (bootstrap or presented header). +MAX_KEY_BYTES = 256 + +#: Maximum number of keys accepted from one bootstrap mapping. +MAX_BOOTSTRAP_KEYS = 32 + +_SCHEMA = """ +CREATE TABLE IF NOT EXISTS api_credentials ( + credential_id TEXT PRIMARY KEY, + key_digest TEXT NOT NULL UNIQUE, + lifecycle_status TEXT NOT NULL, + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL, + rotated_at TEXT, + revoked_at TEXT, + expires_at TEXT +) +""" + +_COLUMNS = ( + "credential_id", + "key_digest", + "lifecycle_status", + "created_at", + "updated_at", + "rotated_at", + "revoked_at", + "expires_at", +) + + +class CredentialRegistryError(ValueError): + """Raised when a registry mutation names an unknown credential.""" + + +class InvalidApiKeyError(ValueError): + """Raised when a bootstrap or register payload fails validation. + + The exception message names the broken rule only. It never includes the + rejected secret. + """ + + +def digest_api_key(api_key: str) -> str: + """Return the SHA-256 hex digest of ``api_key`` encoded as UTF-8. + + Args: + api_key: Presented or configured secret. + + Returns: + A 64-character hexadecimal digest used for storage and comparison. + """ + + return hashlib.sha256(api_key.encode("utf-8")).hexdigest() + + +def parse_bootstrap_api_keys(raw: str) -> list[str]: + """Parse a comma-separated bootstrap transport string. + + Args: + raw: Operator-supplied transport payload. This function does not + read the process environment. + + Returns: + Distinct, stripped keys in the order they appeared. + + Raises: + InvalidApiKeyError: If any key is empty after filtering only when + a remaining key is malformed, duplicated, overlong, contains a + control character, or the set exceeds :data:`MAX_BOOTSTRAP_KEYS`. + """ + + keys = [part.strip() for part in raw.split(",") if part.strip()] + if len(keys) > MAX_BOOTSTRAP_KEYS: + raise InvalidApiKeyError("bootstrap API key count exceeds the maximum") + seen: set[str] = set() + for key in keys: + _validate_api_key(key) + if key in seen: + raise InvalidApiKeyError("bootstrap API keys contain a duplicate") + seen.add(key) + return keys + + +def _validate_api_key(api_key: str) -> None: + """Reject empty, overlong, or control-character secrets. + + Args: + api_key: Candidate secret. + + Raises: + InvalidApiKeyError: If the secret is not acceptable. The message + never echoes ``api_key``. + """ + + if not api_key: + raise InvalidApiKeyError("API key must not be empty") + if any(ord(char) < 32 or ord(char) == 127 for char in api_key): + raise InvalidApiKeyError("API key contains a control character") + if len(api_key.encode("utf-8")) > MAX_KEY_BYTES: + raise InvalidApiKeyError("API key exceeds the maximum encoded length") + + +def bootstrap_from_mapping( + store: CredentialRegistry, + mapping: Mapping[str, str], + *, + now: datetime, +) -> dict[str, str]: + """Import keys from an explicit mapping into ``store``. + + Only :data:`BOOTSTRAP_ENV_NAME` is read from ``mapping``. The process + environment is not consulted. + + Args: + store: Destination registry. + mapping: Bootstrap transport, typically a snapshot of ``os.environ`` + taken once at process start. + now: Clock used for ``created_at`` / ``updated_at``. + + Returns: + Mapping of plaintext bootstrap key to ``credential_id``. The return + value is for the startup caller only; do not log it. + """ + + raw = mapping.get(BOOTSTRAP_ENV_NAME, "") + imported: dict[str, str] = {} + for key in parse_bootstrap_api_keys(raw): + imported[key] = store.register(key, now=now) + return imported + + +class CredentialRegistry: + """Durable, thread-safe store of hashed API credentials. + + Args: + db_path: Filesystem path of the SQLite database. Created with the + ``api_credentials`` schema if it does not exist. ``":memory:"`` + is rejected because each operation opens a fresh connection. + """ + + def __init__(self, db_path: str) -> None: + """Open or create the registry file and ensure the schema exists. + + Args: + db_path: Path to the SQLite database file. + + Raises: + ValueError: If ``db_path`` is ``":memory:"``. + """ + + if db_path == ":memory:": + raise ValueError( + "CredentialRegistry requires a file path; ':memory:' " + "databases do not survive the short-lived connections " + "this store uses" + ) + self._db_path = str(db_path) + self._lock = threading.Lock() + with self._connect() as conn: + conn.execute(_SCHEMA) + + def __repr__(self) -> str: + """Return a secret-free debug representation.""" + + return f"CredentialRegistry(db_path={self._db_path!r})" + + def __str__(self) -> str: + """Return the same secret-free text as :meth:`__repr__`.""" + + return self.__repr__() + + @contextmanager + def _connect(self) -> Iterator[sqlite3.Connection]: + """Open a short-lived WAL-mode connection. + + Yields: + A ``sqlite3.Connection`` with row factory ``sqlite3.Row``. + """ + + conn = sqlite3.connect(self._db_path, timeout=30.0) + try: + conn.row_factory = sqlite3.Row + conn.execute("PRAGMA journal_mode=WAL") + yield conn + conn.commit() + finally: + conn.close() + + @staticmethod + def _row_to_dict(row: sqlite3.Row) -> dict[str, str | None]: + """Convert a registry row into a plaintext-free record dict. + + Args: + row: A ``sqlite3.Row`` from ``api_credentials``. + + Returns: + A dict with :data:`_COLUMNS` keys. The original secret is never + present. + """ + + return {key: row[key] for key in _COLUMNS} + + @staticmethod + def _compare_digests(left: str, right: str) -> bool: + """Compare two hex digests with ``hmac.compare_digest``. + + Args: + left: Presented digest. + right: Stored digest. + + Returns: + ``True`` when the digests are equal. + """ + + return hmac.compare_digest(left, right) + + @staticmethod + def _is_verifiable(record: dict[str, str | None], now: datetime) -> bool: + """Return whether ``record`` may authenticate at ``now``. + + Args: + record: A row from :meth:`list_records`. + now: Caller-supplied clock. + + Returns: + ``True`` when the credential is ``active`` and not expired. + """ + + if record["lifecycle_status"] != "active": + return False + expires_at = record["expires_at"] + if expires_at is None: + return True + return datetime.fromisoformat(expires_at) > now + + def register( + self, + api_key: str, + *, + now: datetime, + expires_at: datetime | None = None, + ) -> str: + """Insert ``api_key`` as an active credential, or return the existing id. + + Args: + api_key: Secret to hash and store. Never persisted in plaintext. + now: Timestamp for ``created_at`` / ``updated_at`` on insert. + expires_at: Optional expiry. ``None`` means no expiry. + + Returns: + The ``credential_id`` for this digest. + + Raises: + InvalidApiKeyError: If ``api_key`` fails validation. + """ + + _validate_api_key(api_key) + key_digest = digest_api_key(api_key) + timestamp = now.isoformat() + expiry = expires_at.isoformat() if expires_at is not None else None + with self._lock, self._connect() as conn: + existing = conn.execute( + "SELECT credential_id FROM api_credentials WHERE key_digest = ?", + (key_digest,), + ).fetchone() + if existing is not None: + return str(existing["credential_id"]) + credential_id = uuid.uuid4().hex + conn.execute( + "INSERT INTO api_credentials (" + " credential_id, key_digest, lifecycle_status," + " created_at, updated_at, expires_at" + ") VALUES (?, ?, 'active', ?, ?, ?)", + (credential_id, key_digest, timestamp, timestamp, expiry), + ) + return credential_id + + def verify(self, presented_key: str, *, now: datetime) -> str | None: + """Return the matching active credential id, or ``None``. + + Comparison walks every verifiable digest so a first-match hit is + not a timing signal. Digests are UTF-8 SHA-256 hex strings, so + ``hmac.compare_digest`` never sees mixed ``str``/``bytes``. + + Args: + presented_key: Value of the ``X-API-Key`` header. + now: Clock used for expiry. + + Returns: + The matching ``credential_id``, or ``None`` when the header is + missing, overlong, or does not match an active unexpired key. + """ + + if not presented_key: + return None + if len(presented_key.encode("utf-8")) > MAX_KEY_BYTES: + return None + presented_digest = digest_api_key(presented_key) + matched_id: str | None = None + for record in self.list_records(): + if not self._is_verifiable(record, now): + continue + stored_digest = record["key_digest"] + assert stored_digest is not None + if self._compare_digests(presented_digest, stored_digest): + matched_id = record["credential_id"] + return matched_id + + def has_active_credentials(self, *, now: datetime) -> bool: + """Return whether any credential is verifiable at ``now``. + + Args: + now: Caller-supplied clock. + + Returns: + ``True`` when middleware should require a matching header. + """ + + return any(self._is_verifiable(record, now) for record in self.list_records()) + + def get(self, credential_id: str) -> dict[str, str | None] | None: + """Fetch one plaintext-free record. + + Args: + credential_id: Registry identifier. + + Returns: + The record dict, or ``None`` if the id is unknown. + """ + + with self._lock, self._connect() as conn: + row = conn.execute( + "SELECT * FROM api_credentials WHERE credential_id = ?", + (credential_id,), + ).fetchone() + return self._row_to_dict(row) if row is not None else None + + def list_records(self) -> list[dict[str, str | None]]: + """List every credential without plaintext secrets. + + Returns: + Records ordered by ``created_at`` then ``credential_id``. + """ + + with self._lock, self._connect() as conn: + rows = conn.execute( + "SELECT * FROM api_credentials" + " ORDER BY created_at, credential_id" + ).fetchall() + return [self._row_to_dict(row) for row in rows] + + def rotate(self, credential_id: str, next_api_key: str, *, now: datetime) -> str: + """Retire ``credential_id`` and register ``next_api_key``. + + Args: + credential_id: Current credential to mark ``rotated``. + next_api_key: Replacement secret. + now: Clock for ``rotated_at`` / ``updated_at``. + + Returns: + The new ``credential_id``. + + Raises: + CredentialRegistryError: If ``credential_id`` does not exist. + InvalidApiKeyError: If ``next_api_key`` fails validation. + """ + + if self.get(credential_id) is None: + raise CredentialRegistryError("credential does not exist") + new_id = self.register(next_api_key, now=now) + timestamp = now.isoformat() + with self._lock, self._connect() as conn: + conn.execute( + "UPDATE api_credentials SET lifecycle_status = 'rotated'," + " updated_at = ?, rotated_at = ? WHERE credential_id = ?", + (timestamp, timestamp, credential_id), + ) + return new_id + + def revoke(self, credential_id: str, *, now: datetime) -> None: + """Mark ``credential_id`` revoked so it can no longer verify. + + Args: + credential_id: Credential to revoke. + now: Clock for ``revoked_at`` / ``updated_at``. + + Raises: + CredentialRegistryError: If ``credential_id`` does not exist. + """ + + timestamp = now.isoformat() + with self._lock, self._connect() as conn: + cursor = conn.execute( + "UPDATE api_credentials SET lifecycle_status = 'revoked'," + " updated_at = ?, revoked_at = ? WHERE credential_id = ?", + (timestamp, timestamp, credential_id), + ) + if cursor.rowcount == 0: + raise CredentialRegistryError("credential does not exist") diff --git a/docs/doctoring/api-credential-registry.md b/docs/doctoring/api-credential-registry.md new file mode 100644 index 00000000..fc2e43a0 --- /dev/null +++ b/docs/doctoring/api-credential-registry.md @@ -0,0 +1,77 @@ +# API credential registry + +## Decision + +Codec Carver stores API-key verification material in a stdlib SQLite +registry (`credential_registry.py`, table `api_credentials`). Request +handlers in `saas_web.py` read only that registry. They do not call +`os.getenv` and they do not compare plaintext secrets from the process +environment. + +`CODEC_CARVER_API_KEYS` remains **bootstrap transport**. Process start +(and tests) pass an explicit mapping into `bootstrap_from_mapping`. After +import, the plaintext keys exist only in the startup caller’s memory; +the database keeps SHA-256 digests, a lifecycle status (`active`, +`rotated`, `revoked`), and optional expiry. + +## What to do next + +1. Put current and next keys in `CODEC_CARVER_API_KEYS` only for the + first start (or a dedicated bootstrap command). Restart loads them + into `api_credentials`. +2. Send `X-API-Key` on every request except `GET /` and `GET /health`. +3. To rotate: call `CredentialRegistry.rotate` (or re-bootstrap the next + key, then revoke the old id). Stop sending the retired secret. +4. Do not log registry listings, exception text, or headers; they are + built to omit plaintext, and operators should keep it that way. + +## Technical basis + +Digital identity guidance treats a memorized or presented secret as a +verifier secret: store a one-way digest, compare in a way that does not +leak the secret through errors, and support authenticator lifecycle +(issue, rotate, revoke) with an explicit clock (Grassi et al., 2017). +OWASP’s API authentication guidance likewise requires rejecting +unauthenticated access to non-public operations without reflecting +credentials in responses (OWASP, 2023). + +HMAC comparison (`hmac.compare_digest`) is the stdlib bound for +equal-length byte strings (Krawczyk et al., 1997). This registry hashes +both the stored secret and the presented header as UTF-8 SHA-256 hex +before comparison so mixed `str`/`bytes` cannot raise `TypeError` on +hostile Unicode, and every active digest is compared so a first-match +hit is not a timing signal. + +The table name is two words (`api_credentials`). Columns depend only on +`credential_id` (3NF). Callers pass `now`; the store does not call +`datetime.now()`. + +Local development stays fail-open when the registry has no verifiable +row so a laptop `uvicorn` still serves the upload form. Production +fail-closed bind policy (non-loopback + empty registry) is the next +slice and must not be implemented by reading extra environment +variables inside request handlers. + +## Verification and rollback + +- `python3 -m unittest tests.test_credential_registry tests.test_saas_web.TestApiKeyAuth tests.test_saas_web.TestCredentialWiring -v` +- Non-ASCII, overlong, duplicate, control-character, rotated, revoked, + expired, concurrent-read, and secret-redaction cases must stay green. +- Roll back by restoring `saas_web.require_api_key` only if you also + restore the tests; do not reintroduce request-time + `os.environ.get("CODEC_CARVER_API_KEYS")`. + +## References + +Grassi, P. A., Garcia, M. E., & Fenton, J. L. (2017). *Digital identity +guidelines: Authentication and lifecycle management* (NIST Special +Publication 800-63B). National Institute of Standards and Technology. +https://doi.org/10.6028/NIST.SP.800-63B + +Krawczyk, H., Bellare, M., & Canetti, R. (1997). *HMAC: Keyed-hashing +for message authentication* (RFC 2104). Internet Engineering Task Force. +https://doi.org/10.17487/RFC2104 + +OWASP. (2023). *OWASP API security top 10 2023*. Open Worldwide +Application Security Project. +https://owasp.org/API-Security/editions/2023/en/0x11-t10/ diff --git a/pyproject.toml b/pyproject.toml index 91884dfe..07d1371d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -78,6 +78,7 @@ codec-carver-library = "audio_library:main" py-modules = [ "chapters", "config_file", + "credential_registry", "diarize", "job_store", "mcp_driver", diff --git a/saas_web.py b/saas_web.py index 63265e94..27ac726c 100644 --- a/saas_web.py +++ b/saas_web.py @@ -1,21 +1,88 @@ """FastAPI upload UI for shrinking one media file through Codec Carver.""" import json -import hmac import logging import os import shutil import tempfile import uuid import zipfile +from contextlib import asynccontextmanager from datetime import datetime, timezone from pathlib import Path + from fastapi import FastAPI, UploadFile, File, BackgroundTasks, Form, Request from fastapi.responses import HTMLResponse, FileResponse, JSONResponse + +from credential_registry import ( + CredentialRegistry, + bootstrap_from_mapping, +) from job_store import JobStore import media_shrinker -app = FastAPI(title="Codec Carver SaaS") + +def _default_credential_registry_path() -> Path: + """Return the default SQLite path for hashed API credentials.""" + + return Path(tempfile.gettempdir()) / "codec_carver_api_credentials.sqlite3" + + +def _new_credential_registry(db_path: Path | None = None) -> CredentialRegistry: + """Construct a registry on ``db_path`` or the process default file.""" + + path = db_path if db_path is not None else _default_credential_registry_path() + return CredentialRegistry(str(path)) + + +CREDENTIAL_REGISTRY = _new_credential_registry() + + +def configure_credential_registry(store: CredentialRegistry) -> None: + """Replace the process registry used by request-time authentication. + + Args: + store: Registry that request handlers will read. Startup and tests + call this after bootstrap; request handlers do not. + """ + + global CREDENTIAL_REGISTRY + CREDENTIAL_REGISTRY = store + + +def get_credential_registry() -> CredentialRegistry: + """Return the process credential registry. + + Returns: + The registry last installed by :func:`configure_credential_registry` + or the default empty file-backed store. + """ + + return CREDENTIAL_REGISTRY + + +def request_clock() -> datetime: + """Return the clock used for credential expiry during a request. + + Tests patch this function. Production uses aware UTC wall time. + """ + + return datetime.now(timezone.utc) + + +@asynccontextmanager +async def _app_lifespan(app: FastAPI): + """Load bootstrap transport into the registry once, then serve requests.""" + + bootstrap_from_mapping( + get_credential_registry(), + dict(os.environ), + now=request_clock(), + ) + yield + + +app = FastAPI(title="Codec Carver SaaS", lifespan=_app_lifespan) MAX_UPLOAD_BYTES = 5 * 1024 * 1024 * 1024 MAX_REQUEST_BYTES = MAX_UPLOAD_BYTES + 10 * 1024 * 1024 MAX_BATCH_FILES = 20 @@ -83,39 +150,34 @@ async def limited_receive(): except RequestTooLarge: return JSONResponse(status_code=413, content={"error": "Payload Too Large"}) -def get_configured_api_keys(): - """Return the API keys configured via the CODEC_CARVER_API_KEYS env var. +# Upload UI and liveness stay reachable without a key so a browser can open +# the form and a load balancer can confirm the process is up. +_PUBLIC_GET_PATHS = frozenset({"/", "/health"}) - The variable holds a comma-separated list of keys. Whitespace around each - key is stripped and empty entries are ignored. Keys are read from the - environment at request time (not import time) so tests can patch the - environment easily and key rotation needs no server restart. Returns an - empty list when the variable is unset or contains no usable keys, which - leaves the service open (today's default behaviour). - """ - raw = os.environ.get("CODEC_CARVER_API_KEYS", "") - return [key.strip() for key in raw.split(",") if key.strip()] +def _is_public_get(request: Request) -> bool: + """Return True when ``request`` is an unauthenticated GET probe or UI page.""" + + return request.method == "GET" and request.url.path in _PUBLIC_GET_PATHS @app.middleware("http") async def require_api_key(request: Request, call_next): - """Enforce opt-in API-key authentication on all endpoints except GET /. - - When one or more keys are configured via CODEC_CARVER_API_KEYS, every - request other than GET / (the upload UI page) must carry an X-API-Key - header matching a configured key; comparison uses hmac.compare_digest to - stay constant-time. Requests failing the check receive a 401 JSON error - without echoing any key material. When no keys are configured, all - requests pass through unchanged. + """Enforce opt-in API-key authentication except on public GET probes. + + Request-time authentication reads only :func:`get_credential_registry`. + It does not call ``os.getenv`` or parse ``CODEC_CARVER_API_KEYS``. When + the registry has at least one verifiable credential, every request other + than GET / and GET /health must present a matching ``X-API-Key``. + Failures return 401 without echoing key material. An empty registry + leaves local development fail-open. """ - configured_keys = get_configured_api_keys() - if configured_keys and not (request.method == "GET" and request.url.path == "/"): + registry = get_credential_registry() + now = request_clock() + if registry.has_active_credentials(now=now) and not _is_public_get(request): provided_key = request.headers.get("x-api-key", "") - if not any( - hmac.compare_digest(provided_key, key) for key in configured_keys - ): + if registry.verify(provided_key, now=now) is None: return JSONResponse( status_code=401, content={"error": "Invalid or missing API key"}, @@ -512,6 +574,18 @@ async def get_ui(): return HTML_TEMPLATE +@app.get("/health") +async def health() -> dict[str, str]: + """Return a liveness payload for load balancers and environment start. + + This path stays auth-exempt so a probe can confirm the process is + listening without presenting an API key. It does not report job-store + or ffmpeg readiness. + """ + + return {"status": "ok", "service": "codec-carver"} + + @app.post("/shrink") def shrink_media( background_tasks: BackgroundTasks, diff --git a/tests/test_credential_registry.py b/tests/test_credential_registry.py new file mode 100644 index 00000000..81d378b0 --- /dev/null +++ b/tests/test_credential_registry.py @@ -0,0 +1,312 @@ +"""Tests for the SQLite API credential registry. + +These cases follow the buyer-facing contract in issues #329 and #373: +bootstrap may read a transport environment mapping once; request-time +verification reads only hashed material from ``api_credentials``. +""" + +from __future__ import annotations + +import os +import tempfile +import threading +import unittest +from datetime import datetime, timedelta, timezone + +from credential_registry import ( + BOOTSTRAP_ENV_NAME, + CredentialRegistry, + CredentialRegistryError, + InvalidApiKeyError, + bootstrap_from_mapping, + digest_api_key, + parse_bootstrap_api_keys, +) + +T0 = datetime(2026, 8, 16, 12, 0, 0, tzinfo=timezone.utc) +T1 = T0 + timedelta(hours=1) +T2 = T0 + timedelta(days=1) + + +class CredentialRegistryTestCase(unittest.TestCase): + """Base fixture: a fresh registry on a temporary SQLite file.""" + + def setUp(self) -> None: + """Create an isolated registry file for each test.""" + + self._tmp = tempfile.TemporaryDirectory() + self.addCleanup(self._tmp.cleanup) + self.db_path = os.path.join(self._tmp.name, "api_credentials.db") + self.store = CredentialRegistry(self.db_path) + + +class TestParseBootstrapApiKeys(unittest.TestCase): + """The comma-separated transport string is parsed without reading os.environ.""" + + def test_strips_whitespace_and_drops_empty_entries(self) -> None: + """Operators can paste `` a ,, b ,`` and still get two keys.""" + + self.assertEqual(parse_bootstrap_api_keys(" a ,, b ,"), ["a", "b"]) + + def test_empty_or_whitespace_only_is_no_keys(self) -> None: + """An unset-equivalent transport string must not invent credentials.""" + + self.assertEqual(parse_bootstrap_api_keys(""), []) + self.assertEqual(parse_bootstrap_api_keys(" , ,"), []) + + def test_rejects_control_characters(self) -> None: + """A key with a newline or NUL is a configuration error, not a secret.""" + + with self.assertRaises(InvalidApiKeyError): + parse_bootstrap_api_keys("good-key,bad\nkey") + with self.assertRaises(InvalidApiKeyError): + parse_bootstrap_api_keys("bad\x00key") + + def test_rejects_duplicates(self) -> None: + """Duplicate bootstrap keys are a rotation mistake, not two identities.""" + + with self.assertRaises(InvalidApiKeyError): + parse_bootstrap_api_keys("alpha,alpha") + + def test_rejects_overlong_key(self) -> None: + """Oversized keys are rejected before they reach the digest column.""" + + with self.assertRaises(InvalidApiKeyError): + parse_bootstrap_api_keys("k" * 257) + + def test_rejects_too_many_keys(self) -> None: + """A bounded set keeps comparison work predictable.""" + + payload = ",".join(f"key-{i:02d}" for i in range(33)) + with self.assertRaises(InvalidApiKeyError): + parse_bootstrap_api_keys(payload) + + +class TestRegisterAndVerify(CredentialRegistryTestCase): + """Register stores a digest; verify compares UTF-8 digests across the set.""" + + def test_register_verify_roundtrip(self) -> None: + """A freshly registered key authenticates and returns a stable id.""" + + credential_id = self.store.register("secret-key", now=T0) + self.assertEqual(self.store.verify("secret-key", now=T0), credential_id) + record = self.store.get(credential_id) + assert record is not None + self.assertEqual(record["lifecycle_status"], "active") + self.assertEqual(record["key_digest"], digest_api_key("secret-key")) + self.assertNotIn("secret-key", record.values()) + + def test_wrong_key_returns_none(self) -> None: + """A non-matching header follows the same 401 path as a missing header.""" + + self.store.register("secret-key", now=T0) + self.assertIsNone(self.store.verify("wrong-key", now=T0)) + self.assertIsNone(self.store.verify("", now=T0)) + + def test_non_ascii_key_roundtrip(self) -> None: + """UTF-8 keys must verify; hostile Unicode must not raise TypeError.""" + + key = "키-α-🔑" + credential_id = self.store.register(key, now=T0) + self.assertEqual(self.store.verify(key, now=T0), credential_id) + self.assertIsNone(self.store.verify("키-α-🔑x", now=T0)) + + def test_verify_does_not_short_circuit(self) -> None: + """Every active digest is compared so first-match timing is not a signal.""" + + first = self.store.register("key-one", now=T0) + self.store.register("key-two", now=T0) + compared: list[str] = [] + original = self.store._compare_digests + + def tracking(left: str, right: str) -> bool: + compared.append(right) + return original(left, right) + + self.store._compare_digests = tracking # type: ignore[method-assign] + self.assertEqual(self.store.verify("key-one", now=T0), first) + self.assertEqual(len(compared), 2) + + def test_overlong_presented_key_is_rejected_without_lookup(self) -> None: + """A huge X-API-Key is a 401, not a digest DoS against the registry.""" + + self.store.register("secret-key", now=T0) + self.assertIsNone(self.store.verify("k" * 257, now=T0)) + + def test_list_records_never_includes_plaintext(self) -> None: + """Admin listings expose identifiers and lifecycle, never the secret.""" + + self.store.register("secret-key", now=T0) + listing = self.store.list_records() + self.assertEqual(len(listing), 1) + blob = repr(listing) + str(listing) + self.assertNotIn("secret-key", blob) + self.assertIn("key_digest", listing[0]) + self.assertIn("credential_id", listing[0]) + + def test_repr_hides_secrets(self) -> None: + """``repr`` of the registry must not echo a registered key.""" + + self.store.register("secret-key", now=T0) + self.assertNotIn("secret-key", repr(self.store)) + self.assertNotIn("secret-key", str(self.store)) + + def test_duplicate_register_is_idempotent(self) -> None: + """Re-importing the same key during bootstrap does not create a second row.""" + + first = self.store.register("secret-key", now=T0) + second = self.store.register("secret-key", now=T1) + self.assertEqual(first, second) + self.assertEqual(len(self.store.list_records()), 1) + + def test_memory_path_rejected(self) -> None: + """Short-lived connections cannot share an in-memory SQLite database.""" + + with self.assertRaises(ValueError): + CredentialRegistry(":memory:") + + def test_register_rejects_empty_key(self) -> None: + """An empty secret is a configuration error, not an open credential.""" + + with self.assertRaises(InvalidApiKeyError): + self.store.register("", now=T0) + + +class TestLifecycle(CredentialRegistryTestCase): + """Rotated, revoked, and expired credentials must not authenticate.""" + + def test_rotated_key_no_longer_verifies(self) -> None: + """After rotation the previous secret is retired and the next one works.""" + + old_id = self.store.register("old-key", now=T0) + new_id = self.store.rotate(old_id, "new-key", now=T1) + self.assertNotEqual(old_id, new_id) + self.assertIsNone(self.store.verify("old-key", now=T1)) + self.assertEqual(self.store.verify("new-key", now=T1), new_id) + self.assertEqual(self.store.get(old_id)["lifecycle_status"], "rotated") + + def test_revoked_key_no_longer_verifies(self) -> None: + """Revocation is immediate at the supplied clock.""" + + credential_id = self.store.register("secret-key", now=T0) + self.store.revoke(credential_id, now=T1) + self.assertIsNone(self.store.verify("secret-key", now=T1)) + self.assertEqual(self.store.get(credential_id)["lifecycle_status"], "revoked") + + def test_expired_key_no_longer_verifies(self) -> None: + """Expiry is evaluated from the caller-supplied clock, not wall time.""" + + credential_id = self.store.register("secret-key", now=T0, expires_at=T1) + self.assertEqual(self.store.verify("secret-key", now=T0), credential_id) + self.assertIsNone(self.store.verify("secret-key", now=T2)) + + def test_has_active_credentials_respects_expiry(self) -> None: + """Middleware fail-open only when no verifiable credential remains.""" + + self.assertFalse(self.store.has_active_credentials(now=T0)) + self.store.register("secret-key", now=T0, expires_at=T1) + self.assertTrue(self.store.has_active_credentials(now=T0)) + self.assertFalse(self.store.has_active_credentials(now=T2)) + + def test_rotate_unknown_id_raises(self) -> None: + """Rotation of a missing id is an operator error.""" + + with self.assertRaises(CredentialRegistryError): + self.store.rotate("missing", "new-key", now=T1) + + def test_revoke_unknown_id_raises(self) -> None: + """Revocation of a missing id is an operator error.""" + + with self.assertRaises(CredentialRegistryError): + self.store.revoke("missing", now=T1) + + +class TestBootstrapFromMapping(CredentialRegistryTestCase): + """Env is transport into the registry; the mapping is passed explicitly.""" + + def test_bootstrap_imports_keys_and_is_idempotent(self) -> None: + """A second bootstrap with the same transport does not duplicate rows.""" + + mapping = {BOOTSTRAP_ENV_NAME: "key-one,key-two"} + first = bootstrap_from_mapping(self.store, mapping, now=T0) + second = bootstrap_from_mapping(self.store, mapping, now=T1) + self.assertEqual(sorted(first), sorted(second)) + self.assertEqual(len(self.store.list_records()), 2) + self.assertEqual(self.store.verify("key-one", now=T1), first["key-one"]) + self.assertEqual(self.store.verify("key-two", now=T1), first["key-two"]) + + def test_bootstrap_ignores_other_environ_keys(self) -> None: + """Only the named transport variable is read from the mapping.""" + + bootstrap_from_mapping( + self.store, + {"OTHER": "not-a-key", BOOTSTRAP_ENV_NAME: "only-this"}, + now=T0, + ) + self.assertIsNone(self.store.verify("not-a-key", now=T0)) + self.assertIsNotNone(self.store.verify("only-this", now=T0)) + + def test_bootstrap_does_not_read_process_environment(self) -> None: + """A process-level secret must not leak into the registry unless mapped.""" + + previous = os.environ.get(BOOTSTRAP_ENV_NAME) + os.environ[BOOTSTRAP_ENV_NAME] = "process-secret" + try: + bootstrap_from_mapping(self.store, {}, now=T0) + self.assertIsNone(self.store.verify("process-secret", now=T0)) + self.assertFalse(self.store.has_active_credentials(now=T0)) + finally: + if previous is None: + os.environ.pop(BOOTSTRAP_ENV_NAME, None) + else: + os.environ[BOOTSTRAP_ENV_NAME] = previous + + def test_empty_mapping_leaves_registry_open(self) -> None: + """Local default remains fail-open until an operator bootstraps keys.""" + + bootstrap_from_mapping(self.store, {}, now=T0) + self.assertFalse(self.store.has_active_credentials(now=T0)) + + +class TestConcurrency(CredentialRegistryTestCase): + """Concurrent verifies during register must not raise or drop a valid key.""" + + def test_concurrent_verify_during_register(self) -> None: + """Readers keep working while another thread inserts a digest.""" + + self.store.register("seed-key", now=T0) + errors: list[BaseException] = [] + + def reader() -> None: + try: + for _ in range(40): + self.store.verify("seed-key", now=T0) + self.store.verify("missing", now=T0) + except BaseException as exc: # pragma: no cover - failure path + errors.append(exc) + + threads = [threading.Thread(target=reader) for _ in range(4)] + for thread in threads: + thread.start() + self.store.register("late-key", now=T1) + for thread in threads: + thread.join() + self.assertEqual(errors, []) + self.assertIsNotNone(self.store.verify("late-key", now=T1)) + + +class TestExceptionsDoNotLeakSecrets(CredentialRegistryTestCase): + """Logs, exceptions, and reprs must stay free of presented key material.""" + + def test_invalid_key_error_does_not_echo_secret(self) -> None: + """Validation errors name the rule, not the rejected secret.""" + + secret = "leak-me-please\x01" + with self.assertRaises(InvalidApiKeyError) as caught: + parse_bootstrap_api_keys(secret) + self.assertNotIn("leak-me-please", str(caught.exception)) + self.assertNotIn("leak-me-please", repr(caught.exception)) + + +if __name__ == "__main__": # pragma: no cover + unittest.main() diff --git a/tests/test_saas_web.py b/tests/test_saas_web.py index 3b57e033..aa22bc13 100644 --- a/tests/test_saas_web.py +++ b/tests/test_saas_web.py @@ -5,14 +5,22 @@ import tempfile import unittest import zipfile +from datetime import datetime, timezone from unittest.mock import patch, MagicMock from pathlib import Path from types import SimpleNamespace +from credential_registry import ( + BOOTSTRAP_ENV_NAME, + CredentialRegistry, + bootstrap_from_mapping, + parse_bootstrap_api_keys, +) + try: from fastapi import BackgroundTasks from fastapi.testclient import TestClient - from fastapi.responses import Response + from fastapi.responses import JSONResponse, Response import saas_web from saas_web import app @@ -37,6 +45,11 @@ def test_get_ui(self): self.assertEqual(response.status_code, 200) self.assertIn(b"Codec Carver SaaS", response.content) + def test_get_health(self): + response = client.get("/health") + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json(), {"status": "ok", "service": "codec-carver"}) + def test_get_ui_includes_accessible_file_input_helpers(self): response = client.get("/") self.assertEqual(response.status_code, 200) @@ -676,7 +689,31 @@ def test_get_ui_includes_batch_upload_form(self): _HAS_FASTAPI, "fastapi not installed (optional integration dependency)" ) class TestApiKeyAuth(unittest.TestCase): - """Tests for the opt-in CODEC_CARVER_API_KEYS authentication middleware.""" + """Tests for registry-backed API-key authentication middleware.""" + + AUTH_NOW = datetime(2026, 8, 16, 12, 0, 0, tzinfo=timezone.utc) + + def setUp(self) -> None: + """Keep a handle so each test can restore the process registry.""" + + self._previous_registry = saas_web.get_credential_registry() + self._tmp = tempfile.TemporaryDirectory() + self.addCleanup(self._restore_registry) + self.addCleanup(self._tmp.cleanup) + + def _restore_registry(self) -> None: + """Put the process registry back after a test.""" + + saas_web.configure_credential_registry(self._previous_registry) + + def _install_keys(self, *keys: str) -> CredentialRegistry: + """Install a fresh hashed registry containing ``keys``.""" + + store = CredentialRegistry(os.path.join(self._tmp.name, "api_credentials.db")) + for key in keys: + store.register(key, now=self.AUTH_NOW) + saas_web.configure_credential_registry(store) + return store def _post_shrink(self, headers=None): """POST a minimal /shrink request and return the response.""" @@ -688,11 +725,11 @@ def _post_shrink(self, headers=None): headers=headers or {}, ) - def test_no_env_var_leaves_endpoints_open(self): - with patch.dict(os.environ): - os.environ.pop("CODEC_CARVER_API_KEYS", None) - response = self._post_shrink() + def test_empty_registry_leaves_endpoints_open(self): + """Local default stays fail-open until an operator bootstraps keys.""" + self._install_keys() + response = self._post_shrink() self.assertEqual(response.status_code, 200) self.assertEqual( response.json(), @@ -700,25 +737,28 @@ def test_no_env_var_leaves_endpoints_open(self): ) def test_missing_header_rejected_when_keys_configured(self): - with patch.dict(os.environ, {"CODEC_CARVER_API_KEYS": "secret-key"}): - response = self._post_shrink() + """A configured registry rejects anonymous mutating requests.""" + self._install_keys("secret-key") + response = self._post_shrink() self.assertEqual(response.status_code, 401) self.assertEqual(response.json(), {"error": "Invalid or missing API key"}) self.assertNotIn("secret-key", response.text) def test_wrong_key_rejected(self): - with patch.dict(os.environ, {"CODEC_CARVER_API_KEYS": "secret-key"}): - response = self._post_shrink(headers={"X-API-Key": "wrong-key"}) + """A non-matching header is the same 401 as a missing header.""" + self._install_keys("secret-key") + response = self._post_shrink(headers={"X-API-Key": "wrong-key"}) self.assertEqual(response.status_code, 401) self.assertEqual(response.json(), {"error": "Invalid or missing API key"}) self.assertNotIn("secret-key", response.text) def test_correct_key_reaches_handler(self): - with patch.dict(os.environ, {"CODEC_CARVER_API_KEYS": "secret-key"}): - response = self._post_shrink(headers={"X-API-Key": "secret-key"}) + """A matching header reaches the handler without leaking the secret.""" + self._install_keys("secret-key") + response = self._post_shrink(headers={"X-API-Key": "secret-key"}) self.assertEqual(response.status_code, 200) self.assertEqual( response.json(), @@ -726,66 +766,183 @@ def test_correct_key_reaches_handler(self): ) def test_get_ui_always_open_without_key(self): - with patch.dict(os.environ, {"CODEC_CARVER_API_KEYS": "secret-key"}): - response = client.get("/") + """The upload form stays reachable so a browser can start a job.""" + self._install_keys("secret-key") + response = client.get("/") self.assertEqual(response.status_code, 200) self.assertIn(b"Codec Carver SaaS", response.content) + def test_health_stays_open_without_key(self): + """Liveness stays auth-exempt so a probe can confirm the process is up.""" + + self._install_keys("secret-key") + response = client.get("/health") + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json(), {"status": "ok", "service": "codec-carver"}) + def test_job_api_requires_key_when_configured(self): - with patch.dict(os.environ, {"CODEC_CARVER_API_KEYS": "secret-key"}): - response = client.get("/jobs/missing") - allowed = client.get("/jobs/missing", headers={"X-API-Key": "secret-key"}) + """Job status is not a public probe; it requires a matching key.""" + self._install_keys("secret-key") + response = client.get("/jobs/missing") + allowed = client.get("/jobs/missing", headers={"X-API-Key": "secret-key"}) self.assertEqual(response.status_code, 401) self.assertEqual(response.json(), {"error": "Invalid or missing API key"}) self.assertEqual(allowed.status_code, 404) - def test_multiple_comma_separated_keys_all_valid(self): - with patch.dict( - os.environ, {"CODEC_CARVER_API_KEYS": "key-one,key-two,key-three"} - ): - for key in ("key-one", "key-two", "key-three"): - response = self._post_shrink(headers={"X-API-Key": key}) - self.assertEqual(response.status_code, 200, key) - rejected = self._post_shrink(headers={"X-API-Key": "key-four"}) + def test_multiple_registered_keys_all_valid(self): + """Current and next rotation secrets can both authenticate.""" + self._install_keys("key-one", "key-two", "key-three") + for key in ("key-one", "key-two", "key-three"): + response = self._post_shrink(headers={"X-API-Key": key}) + self.assertEqual(response.status_code, 200, key) + rejected = self._post_shrink(headers={"X-API-Key": "key-four"}) self.assertEqual(rejected.status_code, 401) - def test_whitespace_around_keys_is_stripped(self): - with patch.dict(os.environ, {"CODEC_CARVER_API_KEYS": " key-one , key-two "}): - response = self._post_shrink(headers={"X-API-Key": "key-one"}) - self.assertEqual(response.status_code, 200) - response = self._post_shrink(headers={"X-API-Key": "key-two"}) - self.assertEqual(response.status_code, 200) - rejected = self._post_shrink(headers={"X-API-Key": " key-one "}) + def test_non_ascii_key_reaches_handler(self): + """Middleware verifies UTF-8 secrets without compare_digest TypeError. - self.assertEqual(rejected.status_code, 401) + Starlette's TestClient encodes headers as ASCII, so this test calls + the middleware with a request-like object instead of httpx. + """ - def test_empty_entries_are_ignored(self): - with patch.dict(os.environ, {"CODEC_CARVER_API_KEYS": "key-one,, ,"}): - response = self._post_shrink(headers={"X-API-Key": "key-one"}) - self.assertEqual(response.status_code, 200) - rejected = self._post_shrink(headers={"X-API-Key": ""}) + key = "키-α-🔑" + self._install_keys(key) + + async def _invoke(presented: str): + request = SimpleNamespace( + method="POST", + url=SimpleNamespace(path="/shrink"), + headers={"x-api-key": presented}, + ) + async def call_next(_request=None): + return JSONResponse({"ok": True}) + + return await saas_web.require_api_key(request, call_next) + + allowed = asyncio.run(_invoke(key)) + rejected = asyncio.run(_invoke("키-α")) + self.assertEqual(allowed.status_code, 200) self.assertEqual(rejected.status_code, 401) + self.assertNotIn(key, rejected.body.decode("utf-8")) - def test_only_empty_entries_leave_endpoints_open(self): - with patch.dict(os.environ, {"CODEC_CARVER_API_KEYS": " , ,"}): - response = self._post_shrink() + def test_request_auth_does_not_read_bootstrap_env(self): + """A process-level transport secret is ignored after registry install.""" + self._install_keys("registry-key") + os.environ[BOOTSTRAP_ENV_NAME] = "process-secret" + try: + allowed = self._post_shrink(headers={"X-API-Key": "registry-key"}) + ignored = self._post_shrink(headers={"X-API-Key": "process-secret"}) + finally: + os.environ.pop(BOOTSTRAP_ENV_NAME, None) + self.assertEqual(allowed.status_code, 200) + self.assertEqual(ignored.status_code, 401) + + def test_bootstrap_mapping_then_request(self): + """Startup bootstrap from an explicit mapping is the supported path.""" + + store = CredentialRegistry(os.path.join(self._tmp.name, "boot.db")) + bootstrap_from_mapping( + store, + {BOOTSTRAP_ENV_NAME: " key-one , key-two "}, + now=self.AUTH_NOW, + ) + saas_web.configure_credential_registry(store) + self.assertEqual( + self._post_shrink(headers={"X-API-Key": "key-one"}).status_code, + 200, + ) + self.assertEqual( + self._post_shrink(headers={"X-API-Key": "key-two"}).status_code, + 200, + ) + self.assertEqual( + self._post_shrink(headers={"X-API-Key": " key-one "}).status_code, + 401, + ) + + def test_empty_bootstrap_mapping_leaves_endpoints_open(self): + """Whitespace-only transport does not lock the service.""" + + store = CredentialRegistry(os.path.join(self._tmp.name, "empty.db")) + bootstrap_from_mapping(store, {BOOTSTRAP_ENV_NAME: " , ,"}, now=self.AUTH_NOW) + saas_web.configure_credential_registry(store) + response = self._post_shrink() self.assertEqual(response.status_code, 200) self.assertEqual( response.json(), {"error": "Invalid target_bytes value. Must be greater than 0."}, ) - def test_get_configured_api_keys_parsing(self): - with patch.dict(os.environ, {"CODEC_CARVER_API_KEYS": " a ,, b ,"}): - self.assertEqual(saas_web.get_configured_api_keys(), ["a", "b"]) - with patch.dict(os.environ): - os.environ.pop("CODEC_CARVER_API_KEYS", None) - self.assertEqual(saas_web.get_configured_api_keys(), []) + def test_bootstrap_parser_does_not_need_environ(self): + """The transport parser is a pure function of the supplied string.""" + + self.assertEqual(parse_bootstrap_api_keys(" a ,, b ,"), ["a", "b"]) + self.assertEqual(parse_bootstrap_api_keys(""), []) + + +@unittest.skipUnless( + _HAS_FASTAPI, "fastapi not installed (optional integration dependency)" +) +class TestCredentialWiring(unittest.TestCase): + """Cover the process-level registry helpers used at startup.""" + + def test_default_path_and_clock_are_usable(self) -> None: + """Operators can locate the default file and the request clock is aware.""" + + path = saas_web._default_credential_registry_path() + self.assertEqual(path.name, "codec_carver_api_credentials.sqlite3") + clock = saas_web.request_clock() + self.assertIsNotNone(clock.tzinfo) + + def test_new_registry_accepts_explicit_path(self) -> None: + """Tests and startup can point the registry at a dedicated file.""" + + with tempfile.TemporaryDirectory() as tmp: + store = saas_web._new_credential_registry(Path(tmp) / "custom.db") + self.assertFalse(store.has_active_credentials(now=datetime.now(timezone.utc))) + default_store = saas_web._new_credential_registry() + self.assertIsInstance(default_store, CredentialRegistry) + + def test_public_get_paths(self) -> None: + """Only GET / and GET /health skip the credential check.""" + + self.assertTrue( + saas_web._is_public_get(SimpleNamespace(method="GET", url=SimpleNamespace(path="/health"))) + ) + self.assertTrue( + saas_web._is_public_get(SimpleNamespace(method="GET", url=SimpleNamespace(path="/"))) + ) + self.assertFalse( + saas_web._is_public_get(SimpleNamespace(method="POST", url=SimpleNamespace(path="/health"))) + ) + self.assertFalse( + saas_web._is_public_get(SimpleNamespace(method="GET", url=SimpleNamespace(path="/jobs/x"))) + ) + + def test_lifespan_bootstraps_from_explicit_environ_snapshot(self) -> None: + """Lifespan imports transport keys once into the installed registry.""" + + previous = saas_web.get_credential_registry() + with tempfile.TemporaryDirectory() as tmp: + store = CredentialRegistry(os.path.join(tmp, "life.db")) + saas_web.configure_credential_registry(store) + try: + async def _run() -> None: + async with saas_web._app_lifespan(saas_web.app): + pass + + with patch.dict(os.environ, {BOOTSTRAP_ENV_NAME: "life-key"}): + asyncio.run(_run()) + self.assertIsNotNone( + store.verify("life-key", now=datetime.now(timezone.utc)) + ) + finally: + saas_web.configure_credential_registry(previous) @unittest.skipUnless(