AkurAI-Build

public
Latest change 2a1193541cf008c406be9f347e44150a58a66130 - docs: preserve the architecture review, backlog, and schema policy by Olafur Bui
# Architect Review 5/10 — Static Artifact Pipeline (`bunfork-static-v1`)

Scope: `src/artifact.rs` (770 lines, full read), `src/server.rs` static serving (lines 256–660), `README.md` §`bunfork-static-v1`, `tests/fixtures/static/{mpa,spa,base_path}`, `tests/static_http.rs` (513 lines, skimmed).

## What is solid
- Deterministic manifest writing: `BTreeMap` + `serde_json::to_vec_pretty` + `create_new` temp file + atomic rename + parent-dir fsync (artifact.rs:533–570); byte-identical re-admission is unit-tested (artifact.rs:672–689).
- Admission/preload hardening is thorough: `O_NOFOLLOW`, `nlink==1`, dev/ino identity re-check after hashing (artifact.rs:398–478; server.rs:342–390), symlink/special-file/hidden-path rejection, bounded 50k/256MiB/1GiB with overflow-checked accumulation.
- Schema is strictly fail-closed: `deny_unknown_fields`, duplicate-key-rejecting custom deserializer (artifact.rs:646–681), manifest size cap, manifest forced outside the root (artifact.rs:288–306).
- HTTP contract tests are real end-to-end process tests covering ETag/304, HEAD parity, 405, traversal, SPA exclusions, base-path redirects (tests/static_http.rs:19–146).

## Prioritized findings

**1. P1 — `resolve()` revalidates the whole manifest per request.** artifact.rs:178 calls `validate_manifest`, which iterates every file entry (path parse + digest check; up to 50k entries) on *every* HTTP request (server.rs:407). The manifest is already validated at `read_manifest`/`verify` before bind and is immutable behind `Arc`. Drop this from the hot path (or `debug_assert!`). This is O(n) latency and CPU per request for large artifacts.

**2. P1 — Direct requests to `.br`/`.gz` siblings are mislabeled.** Precompressed siblings are admitted as ordinary files, so `GET /app.js.br` resolves as a normal file; `content_type()` strips the `.br` suffix (server.rs:598–601) yielding `text/javascript; charset=utf-8` with **no** `Content-Encoding` — raw brotli bytes served as JS. Either exclude siblings from direct resolution (404), or serve them as `application/octet-stream` without suffix-stripping.

**3. P1 — Admit/serve size limits diverge (1 GiB vs 512 MiB).** Admission passes at ≤1 GiB (artifact.rs:17) but serving fails closed above 512 MiB (server.rs:36,300–304). A successfully admitted and `deploy`ed artifact can be unbootable; `.br`+`.gz` siblings also triple-count toward the preload cap. Enforce (or at least warn about) the serve limit at `admit`/`deploy` time, or align the constants.

**4. P1 — `TrailingSlash::Redirect` and `Directory` are behaviorally identical.** The only consumer treats them as one arm (artifact.rs:217–224); no other code distinguishes them. The v1 schema is freezing a three-value enum with two-value semantics. Implement the distinct behavior (e.g. `Redirect` canonicalizes `/guide/` → `/guide`) or collapse to two variants before the schema ships.

**5. P2 — Redirect `Location` is built from the *decoded* path without re-encoding.** artifact.rs:221 formats decoded segments into `Location`; a space or non-ASCII directory name produces a malformed (unencoded) Location header, or a 400 when `HeaderValue::from_str` rejects it (server.rs:425–435). Percent-re-encode segments when constructing redirects.

**6. P2 — Hidden/sensitive denylists reject legitimate sites.** Any dot-prefixed segment is unadmittable and unservable, so `.well-known/` (ACME, `security.txt`, `assetlinks.json`) can never work (artifact.rs:489–495, 600–607). A top-level `data/` directory — common for static JSON datasets — is rejected as "sensitive" (artifact.rs:496–508). Provide an explicit opt-in allowlist rather than hard failure.

**7. P2 — SPA fallback exclusion is a hard-coded heuristic.** artifact.rs:628–643 blends a framework prefix list (`_next/`, `_nuxt/`…) with "dot in last segment", so routes like `/docs/node.js` or `/releases/v1.2` 404 instead of falling back, and the list will drift with frameworks. Move exclusions into the manifest (producer-declared) in a future schema rev; `deny_unknown_fields` + exact schema-string match means any such addition is a hard version break, so plan the v2 path now.

**8. P2 — Minor robustness/DX gaps.** `MAX_FILES` counts only files; directory count/depth are unbounded during scan (artifact.rs:334–392) — adversarial deep trees stall admission. Startup reads+hashes the artifact twice (`verify` server.rs:264, then preload re-hash server.rs:294–318) — mergeable into one pass. `doctor`/`verify` failure is a single opaque message ("does not match its manifest", artifact.rs:150–157; main.rs:463–478) — report the first divergent path. `content_type` lacks `webmanifest`, `mp4/webm`, `mp3/ogg` (octet-stream + `nosniff` breaks media playback).

## Verdict
Architecture is sound and genuinely fail-closed; determinism and TOCTOU discipline are above average. Fix #1 (per-request O(n) validation) and #2 (mislabeled precompressed siblings) before calling v1 done; #3/#4 are schema/limit decisions that get expensive to change after v1 freezes.