Files
alkcall/docs/reviews/consumer-findings-ledger.md
T
glm-5.3-flash a2d72f9737 docs(ledger): file CF-004 — services_schema_handler discloses Internal/ACL-restricted op specs
Found via alkhttp Review 002 (PRJ-16): the services/schema handler
does a bare registry.registration(name) with no Visibility and no
AccessControl check, so POST /call (and the MCP call tool) can fetch
any Internal op's complete spec unauthenticated. The GET /schema route
and MCP schema tool enforce the pre-checks; the /call path is the
hole.
2026-08-30 10:50:28 +00:00

8.3 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

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

  • 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 does a bare registry.registration(&name) and returns spec_to_json(&reg.spec) verbatim, with no Visibility check and no AccessControl::check(peer_identity). The built-in op itself is External with default ACL, so alkhttp's per-route pre-checks (which guard only the outer op name services/schema) cannot see the inner name a caller smuggles in the input: POST /call {"operation":"services/schema","input":{"name":"secret/op"}} returns the full spec — input/output/error schemas and access_control.required_scopes — of any Internal or admin-scoped op, unauthenticated. The GET /schema route and the MCP schema tool both enforce internal-op + ACL pre-checks on the same underlying operation; the /call path (and the MCP call tool dispatching through it) is the hole.
  • Impact: unauthenticated disclosure of the complete contract of every Internal operation, including the exact scopes needed for escalation (the "reads the scopes needed to escalate" shape review 001's PRJ-06 called out). Disclosure only — the op's handler is not executed.
  • Suggested direction: enforce inside the handler (the complete fix, covers every transport): resolve the peer identity from OperationContext, then apply the same two checks the HTTP route does — is_internal_op-equivalent visibility rejection (spec-404) and AccessControl::check → FORBIDDEN envelope. Defensive note: some existing wire tests exercise services/schema with default-ACL ops and would still pass; the new behavior is only observable against Internal/ACL-restricted registrations.
  • alkhttp side: an alkhttp-local mitigation (input pre-check in call_handler's dispatch path) is planned as review-002 remediation regardless — but it is a per-transport patch; the handler-level fix here is the one that closes the hole for the wire and every future transport.
  • Status: open — no alkcall change made.

CF-003 — publish_schema compile failure is fail-open on the wire dispatch path (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 fails to compile (jsonschema::options().build(&schema) errors, e.g. a non-array "required", a non-string "type", an invalid pattern regex), the pump logs warn!("publish_schema failed to compile; chunks will not be validated") and proceeds with validator: None.
  • Impact: silent unvalidated ingest, per-request. A Pub op (local registration or remote import) whose schema is un-compilable by the pinned jsonschema dialect silently receives arbitrary JSON over the wire that the registration declares validated — the handler's schema guarantee is transport-dependent and quietly false. One broken schema in the registry turns that op into an unvalidated ingest path with only a warn log as the tell. (Per-request recompile cost is the secondary issue; alkhttp's gateway cache eliminated both.)
  • Suggested direction: fail-closed — on compile failure, reject the chunk (or abort the sink dispatch) with INTERNAL-class CallError naming the compile failure in the log only (untrusted schema text off the wire), never a silent skip; cache the compiled validator per registration (hot-reload by schema-value comparison; see alkhttp's src/gateway/schema_cache.rs for the worked shape). Complementary hygiene: reject un-compilable publish_schema values at registration time (OperationRegistry::register observing spec.publish_schema, or with_publish_schema validating eagerly) so the failure never reaches traffic.
  • alkhttp side: fixed in alkhttp 1572a9d (gateway cache + lazy resolution + stream-terminal fail-closed). The wire path is out of this crate's scope (AGENTS.md: no forking alkcall types; the pump is alkcall-internal), so the same invariant currently holds only on the HTTP path.
  • Status: open — no alkcall change made.

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)

  • 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) — when a parsed chunk header carries length > MAX_CHUNK_LEN, the ChunkError::TooLarge arm allocates let mut discard = vec![0u8; length as usize]; before reading: the buffer is sized from the peer's untrusted 8-byte header, and length is a u32 (0xFFFFFFFF ≈ 4 GiB).
  • Impact: memory-shape DoS, not a correctness bug (the resync/skip itself is correct — alkcall's own resync test covers it). Any authenticated peer can send header [ch][len = 0xFFFFFFFF] and then dribble; the allocation is pinned for as long as the payload takes to arrive (a dribbling peer can hold it indefinitely). Via the alkhttp WS path this is trivially reachable by any authenticated browser; K concurrent connections × 4 GiB headers → OOM before the dribble even matters. The normal payload arm (:159, vec![0u8; header.length as usize]) is safe — parse_header rejects length > MAX_CHUNK_LEN, so it is bounded at 16 MiB; only the TooLarge skip arm is unbounded.
  • Suggested direction: stream-skip with a bounded fixed buffer (e.g. a 64 KiB stack/fixed buffer, read_exact loop consuming length bytes) instead of the pre-sized allocation. Optionally bound cumulative skipped bytes per connection before tearing it down, so a peer cannot loop TooLarge headers to burn bandwidth/CPU forever.
  • alkhttp side: nothing to change there — this is demux-internal. alkhttp's own read-side hardening (review-001-ws-pump-consolidation: explicit inbound max_message_size/max_frame_size, idle-read timeout) bounds the WS ingress, but a header that survives those caps still hits this arm, so the fix belongs on the alkcall side.
  • Status: open — no alkcall change made.

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

  • Found in: alkhttp from_wss drop-monitor race tests (WS-02/CON-02, review-001-ws-eof-signal). When the transport mux dies mid-call, the write path fails fast and alkcall's call_single_stream write-failure mapping produces a non-retryable INTERNAL: failed to write request frame — even though the call never reached the producer and a reconnect/retry would be safe.
  • Impact: consumers that drop connections under load (WS EOF, network flaps) get non-retryable errors for in-flight calls that are provably not processed. Callers must either tolerate-or-discriminate two envelope shapes for the same transport-death race, or miss retry opportunities.
  • Suggested direction: classify write failures that occur before any response frame could arrive as retryable (CONNECTION_CLOSED or equivalent) — or expose the distinction so consumers can decide.
  • alkhttp side: the race test tolerates both outcomes for now (asserts retryable only when the call is provably in-flight). If/when alkcall fixes the mapping, alkhttp's ws-eof-signal test should tighten back to retryable-only.
  • Status: open — no alkcall change made.

Resolved

(empty)