Menu
AkurAI-Build
publicLatest change 17d1b13a435cb2fd55f330f4ce5694e1b0d2a131 - docs: record the header-read/idle-connection bound as a triggered backlog item (refs #2) by Olafur Bui
# Review backlog — consciously deferred findings Each entry records why the finding was not fixed immediately and what would trigger revisiting it. Nothing here is forgotten scope; it is deliberate. ## Design changes with triggers - **Header-read and idle keep-alive connection bounds (R2/R3 P1 residual).** `axum::serve` on both listeners caps in-flight *requests* (`GlobalConcurrencyLimitLayer`, 1024) and times out request handling (`TimeoutLayer`, 30 s), but neither engages until hyper has parsed a complete request head: a client that trickles headers, or holds an idle keep-alive socket, is bounded by neither. On the intentionally LAN-visible `0.0.0.0:3100` bind this permits slow-header/idle socket accumulation. It is not remotely code-exploitable beyond FD/connection pressure, and bearer auth is unaffected. Trigger: any internet-adjacent exposure without a buffering reverse proxy that already bounds header-read time, or observed connection/FD pressure. The fix replaces `axum::serve` with a `hyper_util::server::conn::auto::Builder` accept loop configured with `header_read_timeout(REQUEST_TIMEOUT)` plus a connection-level `Semaphore` whose permit lives for the connection task, validated by a raw-`TcpStream` test in `tests/static_http.rs` that asserts a partial request head is closed within `REQUEST_TIMEOUT + margin`. - **Read-only search connections (R4 P2, R9 P1).** One `Arc<Mutex<Connection>>` serializes all database work; WAL is enabled but its reader/writer concurrency is unused. This matches the deliberate "ponytail" scale posture and keeps locking trivially correct. Trigger: measured search latency under concurrent write load, or any real workload where `/api/ready` p99 exceeds tens of milliseconds. The fix is a small read-only connection (or 2–3 reader pool) for search and readiness that must respect the shared/exclusive file-lock protocol used by migrate/restore. - **Native asset precompression and brotli responses (R3 P2, R9 P1).** Native `/assets` are gzip-compressed per request by `CompressionLayer` and brotli is not offered (the `compression-br` feature pulls a large C-free but heavy dependency tree). Correctness is unaffected and ETag/304 revalidation bounds repeat work. Trigger: measured CPU cost on asset-heavy native sites. The fix precompresses public assets once at preload, reusing the static-mode negotiation. - **Merged verify+preload pass (R5 P2).** Static startup reads and hashes the artifact twice (verify, then preload re-hash). Both passes are bounded by the 512 MiB limit; merging them saves startup time only. - **Backup verification pass count (R4 P2).** Backup runs full-file integrity checks on live, temp, and destination copies. This is a deliberate safety-over-speed choice; revisit only if backup latency on large databases becomes operationally painful. ## Refactors judged not worth the churn yet - **Typed errors at the db/artifact boundary (R1 P1).** `VectorInputError` plus an `anyhow` downcast selects 400 vs 500. A `thiserror`-style enum would be cleaner, but the conversion touches every call site in ~3,000 lines of verified code for no behavioral gain. Trigger: the next change that adds a new error category to the API surface. - **Full response-assembly dedupe (R1 P2).** The riskiest duplication (the If-None-Match comparison) is now the shared `if_none_match_matches`; the remaining divergence between `static_file_response` (negotiation, Vary, strong ETags) and `public_asset` (weak ETags, no negotiation) is intentional behavior, not drift. - **`static_http.rs` harness slimming (R8 P2).** The self-built raw-HTTP client is heavyweight, but it is the only coverage that exercises a real listener process (SIGTERM, no-runtime-state). Move header-matrix cases in-process opportunistically when they next need editing. ## Product decisions documented as intended behavior - **Tracing defaults are failure-only (R2 P2).** The default filter (`bunfork=info,tower_http=info`) intentionally suppresses per-request DEBUG events from `TraceLayer`; set `RUST_LOG=tower_http=debug` for access logs. Making INFO-level access logging the default was rejected as noisy for a LAN appliance. - **Plaintext public binds warn instead of failing (R6 P2).** `0.0.0.0:3100` is documented as intentionally LAN-visible and the quick start depends on it. The Origin fallback now fails closed on non-loopback binds, which was the actual exploitable edge; a mandatory `--allow-plaintext-public` flag would break the documented first-run flow. - **Environment-variable secrets persist in process memory (R6 P2).** `std::env::remove_var` is `unsafe` in edition 2024 and this crate denies `unsafe_code`. Child processes are scrubbed (`run_process`, `build_frontend`), key files take precedence over env vars, and the README documents env transport as the weaker option. - **Global flags are accepted by commands that ignore them (R7 P2).** Restructuring clap to scope `--tenant`/`--model`/`--database` per subcommand breaks the documented global-options interface for marginal benefit. Note: clap also accepts global flags after the subcommand. - **CSP inline-script hashes at admission (R6 P2).** `admit` could extract inline `<script>` hashes and emit a hash-based CSP, removing `'unsafe-inline'` for static sites. Valuable, but it belongs to the v2 manifest schema work (see static-schema-evolution.md) because the hashes must be recorded in the manifest to survive the frozen-artifact model. ## Schema-v2 items Tracked in [../static-schema-evolution.md](../static-schema-evolution.md): `.well-known` opt-in (R5 P2), producer-declared SPA fallback exclusions (R5 P2), admit-time cache classes replacing the hard-coded framework namespaces (R10 P1), and CSP hash emission (R6 P2).