alkhttp/tasks/gateway/review-001-publish-schema-validation-robust.md

---
id: review-001-publish-schema-validation-robust
name: Fix /publish schema validation fail-open + per-request recompilation (post-remediation)
status: completed
depends_on: []
scope: narrow
risk: high
impact: component
level: implementation
tags: [gateway, review-001, follow-up]
---

## Description

Follow-up to review-001-gateway-publish-semantics (GW-01): the landed
fix compiles the op's `publish_schema` **on every `/publish` request**
(`src/gateway/routes.rs:254-268`) and, when compilation fails, logs a
warning and proceeds with **no validation** — chunks flow to the handler
unvalidated. Two remediation-introduced problems:

- **Fail-open on compile failure** — this reintroduces, conditionally,
  the exact transport-dependent invariant GW-01 closed: a handler that
  registered a `publish_schema` (or a remote import whose schema is
  un-compilable by jsonschema 0.46's dialect) silently receives
  arbitrary JSON over the HTTP path that the wire path would have
  aborted. A registry with one broken schema becomes an unvalidated
  ingest path with only a `tracing::warn` as the tell.
- **Per-request compile cost** — `jsonschema::options().build()` on the
  hot path is CPU + allocation per publish call; a hostile or merely
  busy caller multiplies it.

Fix direction (implementer's choice on mechanism): compile **once per
registration** — cache the compiled `Validator` (or the compile error)
against the op in the gateway state, invalidated on re-registration —
and make compile failure **loud and closed**: the request path surfaces
a server-fault (500, `INTERNAL`-class with the compile error logged at
error level), never a silent skip. Consider whether adapters can reject
un-compilable `publish_schema` values at import/registration time so
the failure lands before any traffic (adapter-side validation where the
schema originates), with the gateway cache as defense in depth.

## Acceptance Criteria

- [x] Compiled validator (or error) cached per registration; no per-request recompile (test: compile count / perf shape not asserted, but code path is registration-keyed)
- [x] Un-compilable `publish_schema` → request fails loudly (500/INTERNAL, error logged), chunks never flow unvalidated (test)
- [x] Schema re-registration (hot reload) picks up the new schema (test)
- [x] A `publish_schema`-registered Pub op still rejects an invalid chunk (existing GW-01 gate stays green)
- [x] `cargo test` and `cargo clippy --all-targets -- -D warnings` pass

## References

- docs/reviews/001-initial-implementation-review.md (Part C, GW-01)
- tasks/gateway/review-001-gateway-publish-semantics.md (the landed fix + rationale)
- src/gateway/routes.rs:254-268 (the fail-open)

## Notes

Mechanism: new `src/gateway/schema_cache.rs` —
`PublishSchemaCache`, a cloneable `Arc<RwLock<HashMap<String,
CacheEntry>>>` held on `RouterState` (built empty per
`HttpAdapter`/router construction; `GatewayState` extracts it via
`FromRef`). Each entry stores the compiled `Arc<jsonschema::Validator>`
(or a `Failed` marker) **plus the raw schema `Value`** that produced
it. Lookup compares the registry's current `publish_schema` value
against the cached one: equal → serve the cached validator/failure;
different → recompile. This gives hot-reload correctness by value
equality without lifetime-coupling to registry internals; under the
documented assemble-then-serve invariant (the same one
`CachedOpenAPIDoc` relies on) each schema compiles exactly once per
process, and the value check is defense in depth for re-assembly
paths.

Fail-closed semantics: a failed compile is **cached as a failure** —
the error is logged once at `error` level (never retried or re-logged
per request), and the wire message is a generic INTERNAL naming the
compile failure without echoing schema internals (an untrusted
schema's error text must not reach the wire; same discipline as the
`/openapi.json` cache-miss 500).

Resolution is **lazy**, resolved against the cache inside the chunk
stream's first poll, not in the handler prologue: `invoke_sink` owns
the 404/403/422 pre-checks (GW-11), so an unknown/internal/wrong-type
op is rejected before any schema lookup or compile happens — no cache
population or error-log spam from ops a caller cannot reach.

Stream terminality fix (found by the spy-handler test): the old
`NdjsonChunkStream` yielded `Err` items but kept streaming — a handler
that drained the stream (instead of aborting on first `Err`) would
have kept receiving **unvalidated** chunks after a violation or a
compile failure. The wire pump (`alkcall dispatch.rs:599-600`) does
`send(Err)` + `break`, i.e. the error item is terminal and the channel
closes. `NdjsonChunkStream` now mirrors that exactly: the first `Err`
item (schema violation, bad JSON, read failure, or compile failure)
sets `done` and the stream ends — `Ok` chunks can never flow after an
error on this transport either.

Adapter-side registration-time rejection was considered and **not**
pursued here: alkcall's `OperationRegistry::register` does not observe
`publish_schema` compilability, and adding that would be an alkcall
surface change (this crate consumes `alkcall = "0.1.1"` from crates-
io) — the gateway cache is the defense in depth the task prescribed.
Worth noting for alkcall: the identical compile-fail-open pattern
exists on the wire path (`alkcall/src/protocol/dispatch.rs:356-366`,
warn-and-skip) — upstream fix candidate, out of scope for this crate.

## Summary

Implemented in three parts:

- **`src/gateway/schema_cache.rs`**: `PublishSchemaCache` — compile-
  once, value-keyed invalidation, cached failures, `error!`-level compile
  logging, generic non-leaking wire error (`CompileFailed::call_error`
  → `CallError::internal`). 5 unit tests (no-schema/unknown-op →
  `Ok(None)`; compile+validate; `Arc::ptr_eq` cache identity; failure
  stays failed).
- **`src/gateway/routes.rs`**: `publish_handler` uses the cache;
  `PublishSchemaState` (Unresolved → Unvalidated/Validated/Failed)
  resolves lazily on first chunk poll, after `invoke_sink`'s
  pre-checks; `NdjsonChunkStream` is terminal-on-first-`Err` (wire
  parity). Route tests: uncompilable schema → 500 `INTERNAL` with
  non-echoing message; spy handler receives 0 `Ok` chunks and exactly
  one terminal `INTERNAL` error; hot-reload re-registration picks up
  the replacement schema (validator identity + semantics asserted);
  all pre-existing GW-01 gates (reject-invalid, accept-valid,
  first-chunk validation, no-schema passthrough) stay green.
- **`src/server/{state,adapter}.rs`** + `gateway/mod.rs`: cache plumbed
  through `RouterState`/`GatewayState` at all construction sites.

Verification: `cargo test` 308 passed (default), `--all-features` all
suites green (379+), `cargo clippy --all-targets -- -D warnings`
clean (default + all-features), `cargo fmt --check` clean.
`--no-default-features` warnings are pre-existing on the base commit.