Files
alkcall/docs/reviews/consumer-findings-ledger.md
T
glm-5.3-flash ed741c34e0 feat(cf-006/cf-007): per-call opener identity for open-op hooks; ADR-016 code-list sweep
Batch-fixes every remaining open alkcall-side finding from downstream
consumers so 0.7.0 is the only release they need to absorb.

- CF-006 (the CF-005 corollary): run_open_wrapper derives a per-call
  AuthContext — the opener's dispatch-resolved identity (the same
  identity the ACL gate and cap check saw) overlaid onto the
  install-time context — and passes it to both the establisher and
  the pump handler. Identity-less calls keep the install-time
  identity (no synthetic-anonymous rewrite); transport-truthful
  fields are never rewritten. Signatures unchanged — behavior-only;
  identical on per-connection registries, hub-forwarded opens now
  show the end client. Gates:
  open_wrapper_overlays_per_call_identity_on_install_time_auth +
  open_wrapper_keeps_install_time_identity_when_call_identityless.
- CF-007 (alkhttp review 006 Part C doc drift): ADR-016 amended to
  the eight-code list — ALREADY_EXISTS + CONNECTION_CLOSED in the
  Context, §3 table, and from_openapi collision rule; new §2a
  documents the undelivered-vs-ambiguous write-failure distinction.
- ChannelPlan type doc now states the Send + Sync payload constraint
  (alktunnels POC F-1 re-derived it by compiler error).
- Ledger: CF-006 and CF-007 filed + resolved; CF-005's corollary
  note points at CF-006; the Open section is empty. ADR-049 gains
  the per-call-identity note. Changelog 0.7.0 covers the batch.

Sweep result (all downstream reviews): no other open alkcall items —
alktunnels W3/F-2 are downstream-by-design, alktty R3/P14 are closed
constraints, alknet has none; OQ-24/37/39/40/41 stay
deferred-by-design (no consumer pull yet).

Verification: cargo test (631) + --all-features (648), clippy
(all-targets, all-features, -D warnings), fmt --check, doc
--no-deps, wasm32 check, semver-checks (no update required),
publish --dry-run.
2026-09-07 11:26:12 +00:00

13 KiB
Raw Blame History

Consumer findings ledger — alkhttp consuming alkcall

Findings from writing the first real consumer (alkhttp) against alkcall. alkcall is deliberately minimal; expect missing features and edge-case bugs to surface here first. Extract items into alkcall tasks/reviews as the project sees fit.

Format: date | found-in (alkhttp context) | severity | status.


Open

(none)


Resolved

CF-005 — connect-side serving path (from_connection_with_serving) has no caller-identity capture; scope-gated ops are satisfiable only via payload auth_token (2026-09-07) — RESOLVED 2026-09-07

  • Found in: alktunnels reverse-flow POC (alktunnels-reverse-poc, summary at /workspace/@alkdev/alktunnels/docs/research/reverse-poc-summary.md §W1; verified against alkcall 0.6.0). The worker is the connect side AND the serving side (ADR-022 §2 both-sides): it serves its tunnel open op to the hub over the connection it dialed. The open op is scope-gated (AccessControl.required_scopes), but the identity the ACL sees comes from Dispatcher::dispatch_start → resolve_identity(connection_identity, payload), and:
    • connection.identity() is None — from_connection_with_serving builds channel 0 internally via Connection::from_source (client.rs channel-0 block) and never sets an identity; there is no capture point for the transport-authenticated peer (mTLS/QUIC identity) on this path.
    • The only caller-identity path is the payload auth_token → ServingConfig.identity_provider (resolve_from_token).
    • Corollary: the AuthContext closed over by register_openable*'s wrapper (what the establisher and pump handler receive) is the connection-establishment-time context, not the per-call opener's identity — on the accept side the install hook receives the transport AuthContext and can channel0_conn.set_identity, but from_connection_with_serving has no equivalent seam.
  • Impact: a connect-side serving op with a scope gate cannot authenticate the legitimate peer by transport identity — the POC had to attach auth_token payloads from the hub side (CallConnection::call_with_payload). Acceptable for hub-forwarding topologies (from_call's ADR-017 §7 token path), but a gap for direct connect-side serving where the transport already authenticated the peer.
  • Verification: confirmed statically (the chain Connection::from_source → empty OnceLock identity → dispatch_start → resolve_identity(None, payload) → AccessControl::check(None) fails closed) and empirically (e2e duplex tests reproducing the POC shape; the tokenless scope-gated call was denied FORBIDDEN: authentication required while the token path succeeded).
  • Fix (both remediations (a) + (b), 2026-09-07): the caller identity for the connect-side serving dispatch resolves in precedence order — (1) payload auth_token → ServingConfig.identity_provider (unchanged; the hub-forwarding / browser-token fallback, ADR-017 §7); (2) new ServingConfig.identity: Option<Identity> — the explicit override (remediation (a)); (3) the transport connection's identity, propagated automatically: from_connection_with_serving copies connection.identity() to the channel-0 connection via set_identity before the serving loop starts (remediation (b) — the native-client mTLS/QUIC key-based path; mirrors the accept side's install-hook set_identity). Also added the public core::auth::NoopIdentityProvider (resolves nothing) — the identity-less posture and the ServingConfig::default() identity provider; three private test copies of it existed.
    • Behavior change (semver-relevant): ServingConfig gained an identity field — struct literals must add identity: None (0.x; changelog note). The call half is unaffected (spawn_dispatch takes the consumer-owned Connection, so the consumer can set_identity before wrapping).
    • The corollary (register_openable's closed-over establishment-time AuthContext vs the per-call opener identity) was resolved in the same batch (2026-09-07, CF-006 below).
  • Status: resolved — 2026-09-07 (regression gates: cf005_* tests in src/channels/client.rs).

CF-006 — open-op establisher/pump handler receive the install-time AuthContext, never the per-call opener's identity (CF-005 corollary) (2026-09-07) — RESOLVED 2026-09-07

  • Found in: the CF-005 verification pass (same alktunnels reverse-flow POC W1 material; the ledger's CF-005 entry named it as a "design follow-up, tracked separately"). run_open_wrapper (src/channels/operations.rs) closed over the install_channel_zero-time AuthContext and passed auth.clone() verbatim to both the establisher (ADR-049 §1) and the pump handler (OpenHandler's 4th parameter). The per-call opener's identity — the same identity the ACL gate and the per-identity cap check saw — reached neither hook. On a per-connection registry (ADR-047 §4) install-time identity IS the caller's, so the gap was invisible there; it shows on hub-forwarded opens (the establisher saw the hub, not the end client) and on the connect-side serving path (token- or ServingConfig.identity-resolved callers). alktty review 002 R3 independently recorded the same seam (closed per-connection by design, with the "registry must stay per-connection" constraint).
  • Impact: an establisher doing per-principal authorization or ownership checks on the backend could not see the actual opener; the pump handler tagging sessions by principal saw the hub.
  • Fix (2026-09-07): run_open_wrapper derives a per-call AuthContext — the opener's dispatch-resolved identity overlaid onto the install-time context — and passes it to both the establisher and the pump handler. When the call has no resolved identity, the install-time identity is kept (anonymous-keep; no synthetic-anonymous rewrite of a real install-time identity). Transport-truthful fields (alpn, remote_addr, tls_client_fingerprint) are never rewritten. Signatures are unchanged (OpenEstablisher, OpenHandler still take AuthContext) — behavior-only, additive; downstream code compiles unchanged. Regression gates: open_wrapper_overlays_per_call_identity_on_install_time_auth + open_wrapper_keeps_install_time_identity_when_call_identityless in src/channels/operations.rs.
  • Also in this batch: ChannelPlan's type doc now spells out the Send + Sync payload constraint (alktunnels POC F-1 re-derived it by compiler error — a doc line prevents the next consumer repeating that).
  • Status: resolved — 2026-09-07.

CF-007 — ADR-016's protocol-code list is stale: omits ALREADY_EXISTS and CONNECTION_CLOSED (2026-09-07) — RESOLVED 2026-09-07

  • Found in: alkhttp review 006, Part C (alkhttp/docs/reviews/006-alkcall-0.3.0-consequence-review.md L207–233, L319–324 — "upstream doc drift … flagged for the next alkcall doc pass"). ADR-016 said "six codes" and its table listed only the original six, while the wire vocabulary had grown: ALREADY_EXISTS (ADR-022 collision sub-amendment, 0.3.0) and CONNECTION_CLOSED (CF-001, 0.4.x, the retryable provably-undelivered-call code). Downstream consumers already handle both; the authoritative ADR was the thing that was wrong.
  • Fix (2026-09-07): ADR-016 amended — Status notes the 2026-09-07 extension to eight; the Context paragraph and the §3 table carry both codes with their semantics (ALREADY_EXISTS non-retryable, registration state; CONNECTION_CLOSED retryable — the only protocol code a caller may auto-retry), a §2a subsection documents the undelivered-vs-ambiguous write-failure distinction, the from_openapi collision rule lists all eight, and the cross-references cite the two amendments. Doc-only; the wire surface is unchanged.
  • Status: resolved — 2026-09-07.

CF-004 — services_schema_handler discloses Internal/ACL-restricted op specs — no visibility or AccessControl check (2026-08-30) — RESOLVED 2026-08-31

  • Found in: alkhttp Review 002, finding PRJ-16 (alkhttp/docs/reviews/002-post-remediation-review.md, Part F', verified against tree 91483a7 + alkcall source). Filed to this ledger 2026-08-30. src/registry/discovery.rs:327-343 (services_schema_handler) — the handler did a bare registry.registration(&name) and returned spec_to_json(&reg.spec) verbatim, with no Visibility check and no AccessControl::check(peer_identity).
  • Fix: the handler now applies the same two gates as OperationRegistry::invoke — the Internal-visibility rejection (!ctx.internal → spec-404) and AccessControl::check with the same identity resolution invoke() uses (handler_identity when internal, identity otherwise). Restricted ops return NOT_FOUND (spec-404) rather than FORBIDDEN — no information leaks about the restricted surface's existence or shape. The gate is in the handler itself, so every transport (wire /call, HTTP routes, MCP tools) is covered.
  • Status: resolved — 2026-08-30.

CF-003 — publish_schema compile failure is fail-open on the wire dispatch path (2026-08-30) — RESOLVED 2026-08-30

  • Found in: alkhttp Review 001 post-remediation task review-001-publish-schema-validation-robust (the GW-01/#1 follow-up work). While fixing alkhttp's HTTP-side instance of the pattern, the identical instance was verified on the wire dispatch path: src/protocol/dispatch.rs:352-366 (Dispatcher::dispatch, OperationType::Pub arm) — when a Pub op's publish_schema failed to compile, the pump logged a warn and proceeded with validator: None (silent unvalidated ingest, plus per-request recompile cost).
  • Fix: fail-closed at the source — publish_schema is now compiled at registration time in both insertion points (OperationRegistry::register and OperationRegistryBuilder::store); an un-compilable schema is a registration error, so an unvalidated-ingest path can never be constructed. The compiled validator is cached per-op (OperationRegistry::publish_validator) and the dispatch path consumes the cache — the per-request compile is gone. The from_call import path inherits the guarantee (imports register through the same choke point); forwarded-chunk validation on the producer side is the remote peer's dispatch path and now inherits the same fail-closed property.
  • Behavior change (semver-relevant): register/builder methods return Err for an un-compilable publish_schema (previously: accepted). Code that registered garbage schemas will now get a registration error instead of a silently-unvalidated op.
  • Status: resolved — 2026-08-30.

CF-002 — demux TooLarge skip allocates the full peer-declared length up front (up to ~4 GiB from an 8-byte header) (2026-08-30) — RESOLVED 2026-08-30

  • Found in: alkhttp Review 001, finding WS-12 (cross-crate; docs/reviews/001-initial-implementation-review.md, Part B, verified against tree 4a825d3). Filed to this ledger 2026-08-30. src/channels/adapter.rs:144 (run_demux_loop_for_client) — the ChunkError::TooLarge arm allocated let mut discard = vec![0u8; length as usize]; before reading: the buffer was sized from the peer's untrusted 8-byte header (u32 length, up to ~4 GiB).
  • Fix: stream-skip with a fixed 64 KiB buffer + read_exact loop consuming length bytes — no allocation sized from peer input. Plus a cumulative skipped-bytes budget (256 MiB) that tears down the connection when a peer loops TooLarge headers to burn bandwidth/CPU. The budget resets on each valid chunk, so legitimate isolated oversized chunks (the resync path, covered by the existing test) are unaffected. The existing demux_resyncs_after_oversized_chunk test passes unchanged.
  • Status: resolved — 2026-08-30.

CF-001 — call_single_stream write-failure maps to non-retryable INTERNAL (2026-08-29) — RESOLVED 2026-08-30

  • Found in: alkhttp from_wss drop-monitor race tests (WS-02/CON-02, review-001-ws-eof-signal). When the transport mux died mid-call, the write path failed fast and the write-failure mapping produced a non-retryable INTERNAL: failed to write request frame — even though the call never reached the producer and a reconnect/retry would be safe.
  • Fix: new retryable CallError::connection_closed constructor (CONNECTION_CLOSED code, retryable: true), applied only to write failures where the call is provably undelivered: call.requested frame write failures on all consumer paths (call/subscribe/publish, single-stream and stream-per-request; the publish pump tags its write stages so only the request-frame stage is retryable). Mid-publish and completed-frame write failures stay INTERNAL (delivery ambiguous — retry unsafe), and the producer-side fail_all(...) on connection close stays INTERNAL. The new code string is additive to the wire error vocabulary; CONNECTION_CLOSED is new — consumers should treat unknown codes per their existing policy (the retryable flag is the machine-readable signal).
  • Status: resolved — 2026-08-30. alkhttp follow-up: the review-001-ws-eof-signal race test can now tighten back to retryable-only asserts.