Files
ESP32_Serial_Swiss_Army_Knife/docs/agent/design-decisions.md
T

13 KiB

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. 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. Hardware tests must claim PHASE0; production service must claim SERVICE. 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.

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

Self-affecting admin SSH actions drain output before execution

Decision: Remote reboot, SSH stop/disconnect, and host-key replacement are deferred until acknowledgement output leaves the administration and transport buffers.

Rationale/evidence: admin_ssh_console has a separate bounded control task and pending-action state. Immediate execution would sever the session before confirmation is delivered.

Consequence for future changes: Commands that invalidate their own transport/session must integrate with deferred control rather than acting synchronously from the dispatcher. Prevent new input while the action is pending.

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

Authentication uses copied principals and fail-safe currentness checks

Decision: Network sessions retain secret-free copied principals. Account mutations invalidate generations/IDs, explicitly request targeted transport revocation at the command layer, and rely on ongoing currentness checks as the fail-safe.

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.

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

Security material and configuration use bounded, versioned NVS records

Decision: Application settings, users, and identities use separate fixed/versioned NVS blobs. Serial, Wi-Fi, 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.

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, atomic commit-or-fail for user and identity mutation.

Relevant files: src/serial_config.c, src/wifi_config.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, and reconciliation.

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}

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 alone calls wolfSSH, 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 call wolfSSH, 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 records a reproduced watchdog stall in mbedTLS external-RAM hardware-AES DMA and uncoordinated mbedTLS/wolfSSL hardware locks; the software-crypto build passed the documented concurrency retest.

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. 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