ESP32_Serial_Swiss_Army_Knife/docs/agent/design-decisions.md

Durable design constraints and decisions

Only constraints supported by implementation or current project documentation belong here. When original rationale is unknown, the entry describes the observable constraint without inventing intent.

One broker mediates all production serial transports

Decision: USB CDC, WebSocket, and role-user SSH access UART1 through session_broker; transports do not independently own the serial service.

Rationale/evidence: The broker is initialized after the serial service and all transport implementations connect broker clients. It is the normal serial RX consumer and TX gate. Project documentation requires one writer and multiple observers.

Consequence for future changes: New serial transports must become broker clients. Do not bypass writer checks or consume serial_service RX directly. serial_service_start() is not idempotent, so admission code must reconcile check/start races as the existing transports do. Broker paths enter serial-service APIs while holding the broker mutex; preserve that lock order and do not call back into the broker while holding the serial state mutex. Preserve binary transparency and avoid in-band ownership control.

Relevant files: src/session_broker.{h,c}, src/serial_service.{h,c}, src/usb_cdc_transport.c, src/web_serial_transport.c, src/ssh_transport.c

Slow clients are isolated by bounded per-client storage

Decision: UART RX is drained and copied into independent bounded broker output streams; a full observer loses only its own copy.

Rationale/evidence: session_broker accounts per-client dropped bytes instead of blocking fan-out. The roadmap records slow-client isolation as a project-wide constraint.

Consequence for future changes: Do not replace fan-out with a blocking shared queue. Any added transport must tolerate partial/no-progress reads and expose drop/backpressure counters.

Relevant files: src/session_broker.c, src/session_broker.h, docs/roadmap.md

Physical UART ownership and logical writer ownership remain separate

Decision: rs232_port_owner controls whether diagnostics or the serial service may manipulate UART/MAX3243 hardware; the broker separately controls which connected client may write.

Rationale/evidence: The code has explicit NONE, PHASE0, SERVICE, and FAULT hardware states plus broker client/writer IDs.

Consequence for future changes: A writer lease never authorizes direct UART/GPIO access. Active hardware tests must claim PHASE0; the production service must claim SERVICE. Boot-time static-safe GPIO setup and service-owned static-mode restoration are explicit exceptions to this cooperative gate. Ambiguous cleanup must keep the transceiver safe and require reboot rather than clearing fault casually.

Relevant files: src/rs232_port_owner.{h,c}, src/rs232_hw_test.c, src/serial_service.c, src/session_broker.c

Resource IDs are generation-safe

Decision: Broker clients, SSH/WebSocket slots, queued admin work, and user principals carry generations or random stable IDs to reject stale references and slot reuse.

Rationale/evidence: Broker IDs encode slot generation; transports track slot generations; admin tokens include session/slot generation; user principal currentness includes account ID and authentication generation.

Consequence for future changes: Preserve transport-slot generations and account-authentication generations as distinct concepts. Validate tokens immediately before side effects and discard late work after disconnect/reuse/revocation.

Phase 8D.2 adds a third identity: non-reused 64-bit originating web-session IDs in serial tickets/slots. 8D.3 rejects zero IDs; Basic authentication/cache are removed. Session-specific cleanup must not become account-wide cleanup; account-name notification intentionally covers deletion/recreation. Invalidate cookie records before requesting transport cleanup, and retain authoritative session/principal checks when notification fails. The transport epoch cancels in-flight ticket publication without taking store and transport locks together.

Relevant files: src/session_broker.{h,c}, src/ssh_transport.c, src/web_serial_transport.c, src/admin_ssh_console.c, src/user_database.{h,c}

UART0 is the physical recovery authority

Decision: UART0 remains independent of UART1 and networking. Initial administrator bootstrap and explicit unavailable-user-database recovery are restricted to UART0.

Rationale/evidence: main.c configures UART0 separately; command policy and user handlers deny these operations remotely. README/roadmap identify UART0 as the trusted recovery console.

Consequence for future changes: Network failures or credential corruption must not remove UART0 recovery. Do not expose bootstrap/recovery through web or admin SSH without an explicit security redesign.

Relevant files: src/main.c, src/admin_ssh_console.c, src/user_console.c, docs/roadmap.md

Admin SSH and user SSH are different routes

Decision: A role-user SSH session becomes a broker serial client. A role-admin session enters the administration console and never obtains a broker client/writer lease.

Rationale/evidence: Role routing is explicit after SSH authentication. The administrative shell is intended for command execution, not multiplexed serial data.

Consequence for future changes: Do not silently give administrators both streams or infer that higher privilege means UART1 ownership. A route-switch feature would require explicit protocol, lifecycle, and authorization design.

Relevant files: src/ssh_transport.c, src/admin_ssh_console.{h,c}, src/session_broker.c

One dispatcher executes the canonical command registry

Decision: UART0 and admin SSH submit complete lines to one fixed queue; one task is the sole caller of esp_console_run().

Rationale/evidence: The implementation treats ESP-IDF console execution as non-reentrant and removes the need for separate remote command implementations.

Consequence for future changes: Register one canonical handler rather than creating a second SSH dispatcher. Long commands/prompts block all administration, so keep handlers bounded or explicitly asynchronous. Preserve output routing and remote principal checks.

Relevant files: src/admin_ssh_console.c, src/main.c, src/console_input.c, all src/*_console.c

Selected self-affecting admin SSH actions use bounded deferred control

Decision: Admin SSH exit, remote reboot, SSH stop/disconnect, and host-key rotate/reset are deferred until command state and administration/transport application buffers appear drained, with a ten-second limit and short final delay.

Rationale/evidence: admin_ssh_console has a separate bounded control task and pending-action state. The check is a best-effort application-buffer heuristic, not peer-delivery confirmation. User account mutations and their immediate revocation calls do not use this path.

Consequence for future changes: Actions that would invalidate their own SSH transport should integrate with deferred control when acknowledgement preservation matters. Prevent new input while an action is pending, keep the wait bounded, and do not describe it as guaranteed delivery.

Phase 8D.4 routes drain/lifecycle operations through a firmware-lifetime immutable owner adapter on the existing control task, outside console locks. Tokens include a transport namespace; owners revalidate full identity and marshal to their transport APIs. SELF_CLOSE targets the invoking frontend while existing SSH action meanings remain unchanged. Unsupported actions must fail before side effects. The two console slots remain a shared bounded pool, with no hypothetical browser capacity allocated. The 8D.5 prerequisite additionally requires owner currentness on the dispatcher, outside console locks, before commands and during prompts; account currentness alone cannot establish originating browser-session liveness. Recheck token identity after external validation, reject revoked submitted replies, and wipe consumed output. Polling is not a hard cancellation deadline and cannot roll back arbitrary handlers; owners retain admission/input/output/lifecycle responsibilities.

8D.7 first slice: WEB also supports reboot and explicit HTTPS stop on the existing control task, with originating-session/principal validation after drain/delay. Stop is service-wide, not admin-socket-only; serial isolation applies to selector/SELF_CLOSE, not explicit HTTPS shutdown. Buffered input observed during deferral is wiped, including an incoming frame whose payload read races cancellation. Keep unsupported identity/credential/network paths blocked until separately implemented; no new executor or delivery guarantee.

8D.7 second certificate slice: Exact parsed browser web certificate rotate --force schedules a typed action, not command replay. Use the existing request-queue union and immutable owner dispatcher_actions mask to hand off after drain/200 ms to the existing 12 KiB dispatcher: crypto/NVS must not run on the 4 KiB control stack. Preserve queue capacity, pending-input gating through execution, token/principal/session revalidation and executing-slot reservation across self-detach. Zero mask preserves legacy SSH execution. WEB revalidates before transactional certificate commit → stop → start; early errors short-circuit and failed stop retains HTTPD ownership without start. Lifecycle failure after commit does not restore the old identity. Browser trust/relogin and UART0/SSH recovery are explicit operational consequences; USB/SSH are not stopped. Bounded acknowledgement/drain is neither an execution deadline nor receipt proof. No stack-size/task/route expansion; owner mask and local scratch still need target accounting/high-water evidence, not host sizeof assumptions. Other credential/account/network/SSH mutations remain blocked pending bounded owner slices.

Relevant files: src/admin_ssh_console.c, src/system_console.c, src/ssh_console.c, src/ssh_transport.c, src/web_admin_transport.c, src/web_console.c

Authentication uses copied principals and fail-safe currentness checks

8D.5 web owner extension: Browser and runtime SSH admission allocate from the same two console slots; a physical SSH slot is not a console index. WEB supports owner-relative self-close only and rejects unsupported network/lifecycle/account mutations at parsed command policy before execution. One web-admin socket and two tickets do not increase six-socket HTTPD capacity; disable LRU rather than evict retained serial clients. The optional owner uses one PSRAM-only payload and ESP timer scheduling, not a new task. HTTPD alone sends/shuts down its verified current fd. Do not use IDF's queued raw-sock_db * close from admin polling: free/reuse before that work executes could close a replacement. Detach must fence queue submissions before HTTPD stop; retire queued markers only after successful stop, retaining ownership across failures. No browser UI or generic HTTP command runner is part of this boundary.

Decision: Network sessions retain secret-free copied principals. Account mutations invalidate generations/IDs; after commit, the command layer requests best-effort targeted transport revocation, while ongoing currentness checks are authoritative.

Rationale/evidence: user_database issues principals without secrets; web/SSH check currentness during admission and active sessions. Mutating console paths call transport revocation hooks.

Consequence for future changes: Do not retain pointers to database records or treat login as permanently authoritative. New authenticated sessions/transports must revalidate at admission, before sensitive input, and periodically or on relevant events. Database mutation APIs alone do not perform transport notification, and notification failure must not roll back an already committed mutation.

Relevant files: src/user_database.{h,c}, src/user_console.c, src/web_server.c, src/web_serial_transport.c, src/ssh_transport.c

Typed serial mutations share the administration dispatcher

8D.9: HTTPD performs bounded typed admission/result reads only; serial reconfiguration and NVS execute on the existing dispatcher so CLI commands cannot interleave. One global pending slot rejects concurrent work; copied session/principal plus non-reused ID fence stale queued work. A 30-second deadline is checked on dequeue, not a cancellation timer or execution limit. Admitted mutations may finish after revocation; completed results can be replaced. Keep explicit uncertain-outcome recovery and never automatically retry mutations. Apply/Defaults are RAM-only, Save persists working device state rather than browser drafts, and Reset follows canonical apply/persist/best-effort-rollback ordering. Navigation preserves broker clients/writer lease, while explicit serial reconfiguration can discard serial-service pending data. No generic command runner/job history is exposed. See src/web_serial_settings.{c,h} and docs/phase8d9_implementation.md.

Typed account selection is checked inside the database mutation lock

Current 8D.10 slice 2: Extend conditional identity checks to password replacement; create uses canonical duplicate/capacity/commit policy. Keep generation separate from commit: the protected bodyless generated-value POST returns one transient value, performs no mutation and retains no retrieval state. Browser saved acknowledgement is context-bound UX, not delivery proof or server authorization. Queued credentials require a one-second periodic timer to cancel/wipe non-executing work at the 30-second deadline plus scheduling latency; execution copies then wipes shared inputs, with local wiping after admitted database work returns. Neither timer nor logout cancels admitted commits. Self password/role/delete uses immediate canonical target revocation, not deferred acknowledgement: 401/disconnect is uncertain and requires relogin/inspection before any explicit retry. Generation is independently optionally registered, preserving failure isolation and restart behavior at 23 handlers. No shell restriction change, secret result/history, new executor or 8D.11 work. Implementation is complete, host-tested/build-verified, not target accepted; timer runtime costs remain unmeasured. See contracts, build and attributed test evidence in docs/phase8d10_implementation.md.

The following first-slice exclusions are historical and superseded by slice 2:

8D.10 first slice: Accounts HTTPD routes expose a compact zero-wait list without password/key data and submit role/delete IDs to the existing dispatcher. Do not use the larger blocking CLI snapshot on HTTPD. Initiating-session currentness is checked before operation admission; target username/account ID/auth generation is compared under the database lock before candidate staging. Conditional and CLI mutations share invariant/commit logic. Notify only the target's web/SSH sessions after successful calls; notification failure does not undo persistence. Separate bounded Serial/Accounts slots do not create another executor. Completed results remain replaceable, no mutation auto-retry, and navigation is not cancellation. Self-target and create/password/generated-secret delivery are intentionally excluded until the next slice defines safe delivery/reconnect semantics. src/web_account_settings.{c,h}, src/user_database.{c,h}, docs/phase8d10_implementation.md.

Browser authentication has a narrow version-pinned HTTPD boundary

8D.8 read-only settings: Reuse bodyless GET cookie/current-admin policy and the existing bounded browser API/errors; no CSRF mutation semantics on a read. Obtain working serial config/running atomically with a zero-wait existing serial mutex, never block HTTPD on stop/reconfiguration or inspect NVS. Navigation changes view/input only, preserving both terminal sockets/lease/output; Settings session validation must not supersede serial-admission checks. One optional exact-GET URI raises only handler capacity to 17. The private adapter stages both descriptor/name allocations before publishing, avoiding the installed 5.5.0 public registration's freed table pointer on strdup failure. Serialized startup/exact matcher only, normal HTTPD allocation/free ownership; re-audit this boundary on SDK changes. Existing public registration callers are not refactored by this phase.

8D.6 terminal separation: Browser selection never reconnects serial or requests/releases a writer lease. Hidden connected terminals continue draining with separate bounded scrollback/pending writes and visible browser-drop accounting; only selected keyboard input is sent. Admin admission/reopen is explicit, close is isolated, and logout/expiry/pagehide tears down both routes. Keep the two page-lifetime input subscriptions stable across switches and remove socket callbacks on close. UI role hiding complements, never replaces, backend authorization. Existing unsupported WEB lifecycle/account-command restrictions remain for 8D.7.

Decision: 8D.3 uses web_cookie_auth plus digest-only session/challenge stores, mandatory Origin/CSRF mutations and no live session/challenge/ticket eviction. Four one-hour absolute sessions deliberately interrupt long serial connections at expiry. No Basic compatibility path remains.

Browser Origin serialization: Authentication POST fetches use mode: 'cors' while retaining fixed same-origin URLs, credentials: 'same-origin', redirect rejection and CSP connect-src 'self'. Under no-referrer, non-CORS POST mode can serialize Origin as null (confirmed in Firefox during M1 testing). Do not fix that by accepting null server-side or weakening CSP/referrer policy; no cross-origin server permission is added.

HTTPD boundary: web_httpd_adapter alone includes private ESP-IDF 5.5.0 structures. Public request getters expose only the first field, so the adapter validates bounded parsed headers/rejects duplicates. /ws/serial is an ordinary GET until authenticated transport admission explicitly sends 101 and installs the frame handler; automatic HTTPD upgrades happen before URI handlers. Preserve right-aligned unread pending bytes when wiping request memory. Two Set-Cookie calls append pointer-backed fields, whose distinct buffers must survive through send. HTTPD logs above ERROR are compiled out to prevent header/ticket exposure.

Consequence: The version guard is not a source-hash guarantee. Re-audit layout, scratch/pending ownership, logging and handshake/frame dispatch on SDK changes; do not scatter private accesses through application code or assume host doubles prove real socket behavior. No SDK patch is currently applied. See docs/phase8d3_implementation.md for verification and target gates.

Relevant files: src/web_cookie_auth.{c,h}, src/web_session_store.{c,h}, src/web_httpd_adapter.{c,h}, src/web_server.c, src/web_serial_transport.c, src/CMakeLists.txt.

Security material and configuration use bounded, versioned NVS records

Decision: Application settings, users, and identities use separate fixed/versioned NVS blobs. Serial, Wi-Fi, mDNS-hostname, and local-UI working edits are RAM-only until explicitly saved. User mutations and HTTPS/SSH identity changes commit directly as part of the operation. Invalid ordinary configuration generally selects RAM defaults without erasing storage; malformed security material fails closed and needs explicit reset.

Rationale/evidence: Serial, Wi-Fi, local UI, web security, users, and SSH security each validate schema/size and own their namespace. User/security mutations build and validate candidate state before committing it; security modules avoid silently replacing an established identity. The live user database remains internal while its 5,360-byte candidate is a persistent PSRAM-preferred allocation with internal fallback and is wiped after every transaction.

Consequence for future changes: Add schema versions and transactional candidate validation. Do not overwrite unknown records automatically; provide explicit migration/reset behavior. Preserve the distinct persistence contracts: explicit save/load/default/reset for working configuration and per-blob commit-before-live-install for user and identity mutation. Keep candidate ownership mutex-local and wipe/free it on initialization or recovery failure. Recheck external-buffer staging in the flash/NVS implementation when upgrading from the pinned ESP-IDF 5.5 baseline. Pre-bootstrap legacy credential rotation spans web_sec/material and user_db/database, is not cross-namespace atomic, and relies on boot reconciliation after interruption.

Relevant files: src/serial_config.c, src/wifi_config.c, src/mdns_config.c, src/mdns_service.c, src/local_ui_config.c, src/web_security.c, src/user_database.c, src/ssh_security.c

NVS is persistence, not a physical security boundary

Decision: The current firmware stores Wi-Fi credentials, recovery credentials, and TLS/SSH private keys in unencrypted application NVS. The reserved NVS-key partition does not enable encryption.

Rationale/evidence: partitions.csv, README security notes, and current code show no NVS-encryption setup. Original rationale for deferring encryption is outside the implementation; the observable limitation is explicit.

Consequence for future changes: Do not claim resistance to flash extraction. Avoid increasing stored secret exposure. Enabling encryption requires migration/recovery planning, not just changing the partition table.

Relevant files: partitions.csv, README.md, src/web_security.c, src/ssh_security.c, src/wifi_config.c

Wi-Fi callbacks enqueue; the manager owns policy

Decision: ESP event callbacks copy bounded event data into the Wi-Fi manager queue. A permanent manager task performs driver operations, profile/AP policy, deadlines, reconciliation, and station mDNS announcement transitions. mDNS initializes at most once, remains allocated across transient disconnects while its component handlers withdraw/re-enable the STA interface, and treats failure as nonfatal.

Rationale/evidence: Callback paths avoid blocking, NVS, and policy work. Manager deadlines consult authoritative driver/netif state so dropped events are recoverable.

Consequence for future changes: Keep callbacks short and nonblocking. Add state transitions to the manager rather than directly invoking Wi-Fi policy from consoles, UI, or callbacks. Preserve queue-drop observability.

Relevant files: src/wifi_manager.{h,c}, src/wifi_config.{h,c}, src/mdns_service.{h,c}, src/mdns_config.{h,c}

Optional local UI cannot become a core dependency

Decision: The OLED/display may fail without stopping serial, UART0, USB, or networking. The UI consumes copied snapshots and calls public APIs; it never parses CLI output or joins the broker.

Rationale/evidence: main.c logs display failures and continues. local_status_ui collects snapshots before display frames and exposes limited confirmed controls.

Consequence for future changes: Keep OLED/I2C work bounded and outside service locks. Do not put credentials or core ownership into UI state. A missing display must remain nonfatal.

Relevant files: src/main.c, src/local_display.{h,c}, src/local_status_ui.c, src/local_ui_config.c

Hardware and library access has designated owners

Decision: The serial task owns UART1 while active, local_display owns I2C/framebuffer access, the SSH owner task owns runtime wolfSSH contexts/calls after caller-side library initialization, and the console dispatcher alone runs registered commands.

Rationale/evidence: These constraints are enforced by module structure, mutex/task assertions, and transport indirection. Original rationale varies; the observable effect is serialized library/hardware access.

Consequence for future changes: Cross-task requests should use existing queues/public APIs. Do not make post-initialization wolfSSH calls, mutate display frames, or run console handlers from arbitrary tasks.

Relevant files: src/serial_service.c, src/local_display.c, src/ssh_transport.c, src/admin_ssh_console.c

Software cryptography settings are a validated concurrency workaround

Decision: wolfSSL ESP32 AES/SHA acceleration is disabled, and HTTPS uses software AES for PSRAM-backed TLS records. Internal task stacks are retained where cache-disable safety matters.

Rationale/evidence: Root CMakeLists.txt disables wolfSSL hardware crypto. The roadmap reports a reproduced watchdog stall involving mbedTLS external-RAM hardware-AES DMA, uncoordinated mbedTLS/wolfSSL hardware locks, and a successful software-crypto concurrency retest; no standalone execution record is checked in.

Consequence for future changes: Do not remove these definitions as a performance cleanup. Any re-enablement needs target-hardware concurrency testing with simultaneous USB, WebSocket, SSH, and serial traffic plus watchdog/stack telemetry.

Relevant files: CMakeLists.txt, src/CMakeLists.txt, docs/roadmap.md, relevant sdkconfig.defaults crypto settings

Embedded web assets are checked-in generated artifacts

Decision: Vendored xterm assets are compressed and embedded ahead of the normal firmware build; src/web_assets_data.c is compiled directly.

Rationale/evidence: src/CMakeLists.txt lists generated data as a source, and web_assets/SOURCES.md documents pinned versions, hashes, and deterministic gzip inputs.

Consequence for future changes: Edit authored web UI separately. Changes to its inline bootstrap loader must update the hard-coded CSP hash atomically and preserve the response security policy. When dependency assets change, follow the documented provenance/generation process and review generated diffs; do not hand-edit arrays or regenerate assets during unrelated work.

Relevant files: web_assets/SOURCES.md, web_assets/generate_embedded_assets.py, src/web_assets_data.{h,c}, src/web_ui.c