# Decision Log: ows-royalties API Hardening **Last Updated:** 2026-06-25 **Author:** Michael Rojas **Related:** [TRD.md](TRD.md), Epic [ACC-10607](https://theorchard.atlassian.net/browse/ACC-10607) Decisions are numbered `D1`, `D2`, … Each was pressure-tested through multiple design + adversarial review rounds (and, where noted, empirical benchmarking). --- ## D1 — Typed Python configuration, not YAML **Status:** Decided. **Context:** The kit needs per-endpoint / per-resource control of rate limits, timeouts, and breaker thresholds. An early draft put these in three YAML files validated by pydantic. **Decision:** Use typed Python — a `Resource` enum + `DownstreamPolicy` frozen dataclass (validated in `__post_init__`) for outbound policy, and `@rate_category` decorators + a typed category map for rate limits. Env vars cover only the handful of real ops knobs. **Rationale:** Rate limits bind to routes (code), so a YAML map keyed by endpoint name duplicates routing knowledge and fails open silently on a rename. The outbound table is a finite set of named resources with cross-field invariants. For a Python-only scope, typed config gives one definition (no YAML↔schema duplication), import-time type safety, refactor safety, and simpler tests. YAML's one real advantage — cross-language portability — does not apply; if org-wide cross-language adoption ever materializes, the typed table can emit a JSON-Schema contract. **Alternatives considered:** YAML + pydantic (rejected: stringly-typed keys, duplicate definitions, fails-open on drift); env-only (rejected: doesn't scale to nested per-resource structure); decorators everywhere with no central table (rejected for the outbound side: harder to audit all policy at once). --- ## D2 — A custom circuit breaker, not pybreaker **Status:** Decided (empirically validated). **Context:** The services run under uWSGI with **1 process × 15 threads**. The breaker wraps outbound calls that can be slow (Snowflake reads up to 120s). **Decision:** Ship a small custom circuit breaker (~70 lines, stdlib `threading`) whose lock guards **only** state transitions; the wrapped I/O runs outside the lock. Half-open admits exactly one probe via a non-blocking semaphore decided atomically under the lock; a `success_threshold` of consecutive probe successes is required to re-close (anti-flap); an `on_state_change` callback feeds observability. **Rationale:** Benchmarked, `pybreaker` holds its lock **during** the wrapped I/O — 15 threads through one breaker serialize to concurrency 1, and a single slow Snowflake read would block all 15 worker threads (the opposite of what a breaker is for). The custom design was prototyped and stress-tested: 15-thread concurrency (no serialization), exactly-one half-open probe under contention (no TOCTOU), gate-leak handled by a `finally` release, and correct failure classification — all passing on CPython 3.13 and 3.14. **Alternatives considered:** see D3. --- ## D3 — Breaker library evaluation (why no off-the-shelf lib) **Status:** Decided. **Context:** Before writing a custom breaker, the maintained synchronous options were benchmarked against four criteria: no I/O serialization, half-open single-probe gating, a state-change hook for observability, and failure classification. **Decision:** None fit; build the custom breaker (D2). **Rationale (benchmark results):** | Library | No I/O serialization | Half-open gate | State hook | Classification | |---|---|---|---|---| | pybreaker | ❌ serializes (15 threads → concurrency 1) | — | ✓ | ✓ | | circuitbreaker (fabfuel) | ✅ | ❌ | ❌ | ✓ | | pycircuitbreaker | ✅ | ❌ | ✓ | ~ | | purgatory | — | — | ✓ | ✓ (❌ doesn't import on Py3.13) | No maintained library provided the half-open gate, and the no-serialization + state-hook + classification combination was not available in one solid Py3.13 library; any choice meant wrapping a lib with a custom gate + emission anyway. The custom breaker also removes a dependency from the reference. --- ## D4 — Per-resource adapters, not a uniform call signature **Status:** Decided. **Context:** The four guarded downstreams use incompatible client conventions. **Decision:** `call_downstream(resource, call, *, idempotent=False)` dispatches to a per-`Resource` `(adapter, classifier)`. The adapter applies that client's native timeout and raises on failure; the classifier decides which exceptions count toward the breaker. **Rationale:** A uniform `fn(timeout=(connect, read))` was verified to break: `owsrequest.process` builds a `requests.Request` that accepts no `timeout` and never passes one to `session.send`; the Snowflake executor sets its timeout at construction and is used as a context manager; httpx needs an `httpx.Timeout`. HTTP adapters also must **raise** on a returned 5xx (owsrequest/requests return 5xx as a value), and the Snowflake classifier must count `OperationalError`/`DatabaseError` but pass `ProgrammingError`. `owsrequest` additionally retries 5× internally, so owsrequest-backed resources use `retries=0` to avoid multiplicative retries on scarce worker threads. --- ## D5 — Edge header-trust is a blocking precondition (not an app concern) **Status:** Decided — tracked as [ACC-10614](https://theorchard.atlassian.net/browse/ACC-10614). **Context:** Per-principal rate limiting keys on identity headers (`Orchard-Identity-Id`/`User-Id`/`Requestor-Service`) and the XFF client IP. These are client-settable. **Decision:** Per-principal limiting is only enabled in prod once the HAProxy edge strips and re-injects those headers (same trust class as XFF) and `PROXYFIX_X_FOR` is set to the real hop count per environment. The same per-endpoint category limits apply to every key (user identity, requestor-service, or IP) — finite, never exemption; there is no separate service tier (measured anonymous service traffic against the service is ~0.05/min, so a higher service ceiling would be invented structure; a future high-volume service caller is allowlisted explicitly with a measured limit). The kit merges dark via rate-limit `shadow` mode until then. **Rationale:** Without the edge guarantee, an attacker can mint unlimited rate-limit keys (bypass) or set a victim's identity to burn their bucket (targeted DoS). This belongs to the edge, not the app, but it gates app behavior — hence a standalone ops ticket. --- ## D6 — Extraction seam kept minimal (no premature DI layer) **Status:** Decided. **Context:** The kit is meant to become a shared `owshardening` library across six same-scaffold services. **Decision:** Import the Orchard glue (`owsresponse` renderer, `get_flask_user_id`) directly behind `# extraction seam` comments rather than building a formal hooks/DI layer now. The formal protocol is introduced at extraction time, when a second, non-Orchard consumer exists. **Rationale:** A formal `HardeningHooks` DI object would invert three trivial glue functions while the package stays hard-wired to flask-limiter/flask-compress/the breaker — speculative generality for six identical-scaffold consumers with one implementation. --- ## D7 — No enable/disable flags on protective middleware; rate limiting is shadow|enforce, not on/off **Status:** Decided. **Context:** An early draft gave every capability an `*_ENABLED` flag and made the health-check paths configurable. **Decision:** Security headers, compression, and ProxyFix are **always on** (no enable flags). Rate limiting has no off switch — it runs in a **`shadow`** mode (counts and logs would-be-429s without blocking) or **`enforce`** mode (`RATELIMIT_MODE`, default `enforce`; `shadow` under the test config and during dark-launch). Fixed application routes (`/hello/`, `/ready/`) are **constants**, not configurable. Only genuinely-environmental values are exposed as config: `RATELIMIT_MODE`, `RATELIMIT_STORAGE_URI`, the limit values, and `PROXYFIX_X_FOR`. **Rationale:** A standing "disable the security control in prod" switch is a foot-gun, and configurability you don't need is just surface area to misconfigure. The dark-launch and test-isolation needs that motivated the flags are met by shadow mode (protection is observed, never fully absent) and by the fact that security headers / compression don't affect the existing test suite (the Werkzeug test client sends no `Accept-Encoding`, and headers aren't asserted). Health-check route paths never vary by environment, so they are constants and `/ready/` is simply added to the access-log exclusion at the one wiring site. --- ## D8 — Publish `X-RateLimit-*` and `Retry-After`, in shadow mode too **Status:** Decided. **Context:** The limiter could either stay silent and let clients discover the cap by getting a 429, or advertise the limit state so clients can self-throttle. An early position was to keep responses minimal on the grounds that the leak-free 503/429 bodies deliberately hide internal detail. **Decision:** Every rate-limited response carries `X-RateLimit-Limit`, `X-RateLimit-Remaining`, and `X-RateLimit-Reset` for the **most-constraining window** (fewest remaining, tie-broken by soonest reset), and the enforced 429 additionally carries `Retry-After` derived from the **breached** window's reset (not the governing window's, so a client is never told to retry before the cap that blocked it has cleared). These headers are emitted in **`shadow` mode as well as `enforce`**, so clients can begin adapting before enforcement is switched on. The response *bodies* stay leak-free; only the rate-limit policy is published, in headers. **Rationale:** Rate-limit policy is a client *contract*, not a secret — an attacker probes the limit trivially anyway, so hiding it buys no security while blinding legitimate clients, who would otherwise have no signal short of a 429. Waiting for a 429 is reactive and wasteful (a burned request to learn you are over budget); `Remaining`/`Reset` let a well-behaved client slow down *before* breaching, and publishing them in shadow lets callers adapt during dark-launch. This is distinct from the leak-free-body decision: hiding *which internal circuit/resource* failed is architecture disclosure with no client value; publishing the rate-limit *policy* is the standard, expected API contract (GitHub, Stripe, et al. do the same). `Vary: Accept-Encoding` correctness is handled by flask-compress; the `X-RateLimit-*` values vary by principal but need no `Vary` (they are not cache keys). --- ## D9 — Enforce requires a shared rate-limit store; `memory://` is per-process (test-only) **Status:** Decided — blocking precondition for the enforce flip ([ACC-10613](https://theorchard.atlassian.net/browse/ACC-10613) rollout). **Context:** `RATELIMIT_STORAGE_URI` defaults to `memory://`. The in-process sliding-window store is thread-safe within a worker (correct for the 1-process x 15-thread test/dev shape), but each uWSGI **worker process** keeps its own counters. **Decision:** `memory://` is treated as **test/dev only**. Before rate limiting is flipped to `enforce` in any environment that runs more than one worker process, a **shared backend** (Redis) must back `RATELIMIT_STORAGE_URI`. Until then the kit runs in `shadow`, where per-process drift only skews observed counts, not real traffic. A misconfigured `RATELIMIT_MODE` now fails fast at startup (the mode is parsed to a closed enum), so the control can never silently degrade to never-block. **Rationale:** With M worker processes and a per-process store, the effective global cap is M x the configured value and the `X-RateLimit-*` headers and breach logs differ per worker — which silently invalidates the prod-Datadog-calibrated limits by a multiplicative factor and makes enforcement and observability incoherent. The fix is operational (a config-only store swap), but it gates correctness of the enforce flip, so it is recorded as a precondition alongside the edge header-trust precondition (D5). When the shared store lands, the limiter should also move to a single round-trip per window (hit-and-read-count in one operation) rather than a separate hit + stats read, so the Redis path does not pay 2N round-trips per request.