Plan 07: Cloud Run hardening

[jstjoe]

Summary

Turns the unified API into something deployable on Cloud Run.

• Auth — X-API-Key middleware, keys from API_KEYS env (Secret Manager in prod). Probes (/v1/health, /v1/ready, /metrics) skip auth so Cloud Run + scrapers can hit them. Refuses to start with no keys configured (set AUTH_DISABLED=1 for dev).
• Input limits — body-size middleware (413), Pydantic max_length on text (422), per-request timeout via asyncio.wait_for (504). Defaults: 200 KB body, 100k char text, 30s timeout. All overridable by env.
• Observability — structlog JSON logs (Cloud Logging auto-parses), per-request UUID bound via contextvars + emitted as X-Request-ID header, Prometheus /metrics with custom counters: pii_detector_calls_total, pii_detector_latency_seconds, pii_spans_detected_total.
• Readiness split — /v1/health is liveness (always 200), /v1/ready is readiness (200 only when EAGER_LOAD detectors are loaded and Skyflow creds look complete). Cloud Run probe wiring in cloudrun.yaml.
• Rate limiting — slowapi keyed on API key; defaults 60/min general, 30/min for /redact, 60/min for /detect. Per-instance counter — caveat documented.
• Graceful shutdown — InflightTracker waits for in-flight requests on SIGTERM (bounded by SHUTDOWN_DRAIN_SECONDS=20); new requests get 503 once draining starts; probes still pass.
• CI — .github/workflows/ci.yml runs ruff + pytest matrix (3.10/3.11/3.12) + Docker smoke build on every PR. release.yml builds the full image on tag and pushes to Artifact Registry via Workload Identity Federation.
• Cloud Run deploy — api/deploy/cloudrun.yaml service spec + api/deploy/README.md with one-time setup (Artifact Registry, secrets, WIF), per-release commands, knob explanations, rollback.

Test plan

• uv run pytest api/tests/ — 25 passing (auth, limits, rate-limit, shutdown, routes)
• uvx ruff check api/ — clean
• docker build -f api/Dockerfile --build-arg INCLUDE_OPF=0 --build-arg INCLUDE_GLINER=0 --build-arg INCLUDE_PRESIDIO=0 -t privacy-api:smoke . boots, /v1/health returns 200
• Local smoke: API_KEYS=test DEFAULT_DETECTOR=opf EAGER_LOAD=opf uvicorn opf_api.main:app — 401 without key, 200 with, 504 on slow text, 429 after burst
• CI workflow turns green on this PR

🤖 Generated with Claude Code

[jstjoe]

Hey @claude can you review please? Looks like we have failures in CI for this.

[claude]

Claude finished @jstjoe's task in 3m 42s —— View job

---

PR Review: Plan 07 — Cloud Run Hardening

Tasks

• Gather context (PR metadata, changed files)
• Check CI failures
• Review changed files
• Post full review

---

CI Failures — Root Cause

Both failing jobs have the same root cause: the privacy-filter directory is referenced in the repo but doesn't exist on disk.

1. Test jobs (all 3 Python versions) — "Install workspace" step

× Failed to build `local-privacy @ file:///...`
├─▶ Failed to parse entry: `opf`
╰─▶ `opf` references a workspace in `tool.uv.sources`, but is not a workspace member

pyproject.toml (root) has:

[tool.uv.workspace]
members = ["eval", "api", "privacy-filter"]   # ← privacy-filter doesn't exist

Because the directory is missing, uv can't register it as a workspace member, so opf = { workspace = true } in api/pyproject.toml and eval/pyproject.toml fails to resolve.

2. Docker smoke build

COPY privacy-filter ./privacy-filter
ERROR: "/privacy-filter": not found

api/Dockerfile:28 tries to copy the directory, but it's absent from the build context. Fix this →

Fix: the privacy-filter package (providing opf) needs to either be committed to the repo, or the workspace config + Dockerfile need to drop references to it and install opf from PyPI.

---

Code Review

The overall design is solid — the auth/limits/observability/shutdown/rate-limit split is clean, the middleware ordering comment is accurate, and the test coverage is good. Here are the issues worth addressing:

Bugs

routes.py:120 — falsy latency_ms silently discarded

latency_ms = float(result.get("latency_ms") or (time.perf_counter() - t0) * 1000)

If a detector returns latency_ms=0.0 (valid), 0.0 or x evaluates to x, so the detector's own reading is thrown away and replaced with the wall-clock time. Should be:

raw = result.get("latency_ms")
latency_ms = float(raw) if raw is not None else (time.perf_counter() - t0) * 1000

Fix this →

Security

observability.py:62 — client-controlled log injection via X-Request-ID

rid = request.headers.get("x-request-id") or uuid.uuid4().hex

An attacker can send X-Request-ID: ","injected":"payload" and tamper with the structured log JSON. Either always generate server-side (uuid.uuid4().hex) and echo it back in the response, or validate it's a 32-hex-char UUID before accepting:

import re
_UUID_RE = re.compile(r'^[0-9a-f]{32}$')
raw_rid = request.headers.get("x-request-id", "")
rid = raw_rid if _UUID_RE.match(raw_rid) else uuid.uuid4().hex

Fix this →

rate_limit.py:17 — full API key stored as rate-limit bucket key

return f"key:{key}"

For in-memory storage this is inconsequential. But if you ever move to Redis/Memorystore (already mentioned in the docstring), the plaintext key will be written there. Use a hash: f"key:{hashlib.sha256(key.encode()).hexdigest()[:16]}".

limits.py:38 — chunked bodies bypass the 200 KB guard

The code comment is honest about this: "Bodies without Content-Length fall through." Worth calling out explicitly: an attacker omitting Content-Length (chunked transfer encoding) gets no size check. FastAPI will still parse it, so this is a genuine gap for large payloads. A streaming read-and-reject approach would close it, but that's more complex — at minimum document this clearly in the README.

Test Infrastructure

conftest.py:19-24 — internal rate-limiter storage access

inner = getattr(storage, "storage", None)
if isinstance(inner, dict):
    inner.clear()

This reaches into slowapi private internals. If slowapi changes its storage structure the teardown silently does nothing and tests start polluting each other. The test_rate_limit.py tests probably already isolate themselves by using separate app instances — worth confirming, or rebuilding limiter fresh per test instead.

Minor

• schemas.py:15 — max_length=text_max_length() is captured at import time, not read dynamically. This is intentional but means TEXT_MAX_LENGTH env changes require a restart to take effect. The comment in limits.py says defaults are "overridable by env" — worth clarifying that this applies only at startup.
• shutdown.py InflightTracker — single-threaded asyncio so the non-atomic +=/-= is safe. Would break under --workers N uvicorn (multiple processes each get their own tracker). Cloud Run uses single-process, so fine as-is, but worth a comment if you ever add worker support.
• ci.yml:34 — uv sync --all-extras from the workspace root is the right command once privacy-filter exists. No change needed there.

---

Summary

The hardening design is well-executed — the auth middleware, probe bypass logic, drain semantics, and metric naming are all good. The blocking issues are:

1. privacy-filter directory missing — fix this first; it's what's breaking both uv sync and Docker.
2. routes.py:120 latency_ms falsy-check bug — wrong result when detectors report 0ms latency.
3. observability.py:62 log injection — X-Request-ID should be validated before trusting.

View CI run |