Allow CLI to connect to Wendy Lite devices via USB

[gmondada]

Wendy Lite devices connected to your Mac, Linux and Windows computer via USB are now showing up with their device name in Wendy CLI. Both wendy discover and wendy run work (except on Windows where compilation to WASM is not yet supported).
The device uses the default USB vendor ID and product ID of any ESP32 chip. Therefore, the device is still seen by Espressif development tools and if you connect with a terminal, you can still see logs.

[github-actions]

AI Security Review

This pull request adds USB Serial/JTAG (USJ) transport support to the Wendy embedded firmware, enabling CLI connectivity to Wendy Lite devices over USB. The change introduces a new UART framing layer (wendy_com_uart), a new USJ task component (wendy_usj), a new protocol command (get_device_identity), and refactors the link abstraction to support both TLS and UART transports. From a security perspective, the most significant concern is that the new USB/UART transport operates without any transport-layer security (no TLS, no authentication, no session establishment), meaning any host with physical USB access can issue arbitrary device commands — including app push, reboot, and the new identity query — without credential verification. Secondary findings include unauthenticated device identity disclosure, unvalidated escape-sequence state transitions that can disrupt an active TLS session, missing error checks on fcntl, a hard-coded fallback string ("???"), and minor logging of internal state details. No PCI DSS or HIPAA domains are touched; GDPR risk is low but present (device identity strings may constitute personal data depending on deployment).

Security & Compliance Review — PR #20: Wendy Lite USB/UART Support

Executive Summary

This pull request adds USB Serial/JTAG (USJ) transport support to the Wendy embedded firmware, enabling CLI connectivity to Wendy Lite devices over USB. The change introduces a new UART framing layer (wendy_com_uart), a new USJ task component (wendy_usj), a new protocol command (get_device_identity), and refactors the link abstraction to support both TLS and UART transports. From a security perspective, the most significant concern is that the new USB/UART transport operates without any transport-layer security (no TLS, no authentication, no session establishment), meaning any host with physical USB access can issue arbitrary device commands — including app push, reboot, and the new identity query — without credential verification. Secondary findings include unauthenticated device identity disclosure, unvalidated escape-sequence state transitions that can disrupt an active TLS session, missing error checks on fcntl, a hard-coded fallback string ("???"), and minor logging of internal state details. No PCI DSS or HIPAA domains are touched; GDPR risk is low but present (device identity strings may constitute personal data depending on deployment).

---

Findings Table

Severity	Standards	File	Line(s)	Title
CRITICAL	SOC2-CC6, ISO27001-A.9, NIST-IA-2	wendy_usj/src/wendy_usj.c, wendy_com/src/wendy_com_link.c	All UART link paths	No authentication or encryption on USB/UART transport
HIGH	SOC2-CC6, ISO27001-A.9, NIST-AC-3	wendy_usj/src/wendy_usj.c	~95–115	Unauthenticated escape-sequence mode switching can hijack active TLS session
HIGH	SOC2-C1, ISO27001-A.8, NIST-SC-8	wendy_com/src/wendy_com_cmd.c	~91–117	Device identity disclosed over unauthenticated plaintext channel
MEDIUM	SOC2-CC7, ISO27001-A.12, NIST-AU-3	wendy_com/src/wendy_com_agent.c	~148	Command ID logged without authentication context — information leakage
MEDIUM	ISO27001-A.8, NIST-SI-10	wendy_com/src/wendy_com_link.c	~465–470, ~487–492	fcntl return values not checked; silent non-blocking mode failure
MEDIUM	SOC2-CC8, ISO27001-A.8	wendy_usj/src/wendy_usj.c	~87	Static wcom_operation shared across re-entrant _enter_com_exec calls
LOW	ISO27001-A.8, NIST-SI-10	wendy_com/src/wendy_com_cmd.c	~98	Hard-coded fallback "???" strings leak implementation detail and may cause protocol confusion
LOW	SOC2-CC7, ISO27001-A.12	wendy_usj/src/wendy_usj.c	~113, ~118–120	termios flags logged at INFO level — may expose tty state to log consumers
INFORMATIONAL	SOC2-CC9, ISO27001-A.8	wendy_usj/CMakeLists.txt	~21–24	Injecting sources directly into a third-party IDF component library
INFORMATIONAL	GDPR/Privacy	wendy_com_msg.proto	~33–37	id, name, display_name fields may constitute personal data

---

Detailed Findings

---

CRITICAL — No Authentication or Encryption on USB/UART Transport

Standards: SOC2-CC6, ISO27001-A.9, NIST SP 800-53 IA-2, SC-8

Description:
The existing TLS transport enforces mutual authentication and encryption. The new UART/USB transport has no equivalent: any host that presents the correct USB VID/PID (default ESP32 values, as noted in the PR description) and sends the escape sequence ESC + 'm' can immediately open a full-privilege command channel. All commands available over TLS — including app_push_begin, app_push_data, app_push_end, app_start, app_stop, and reboot — are dispatched identically regardless of transport. Physical USB access therefore grants complete, unauthenticated device control.

Relevant snippet:

// wendy_com_link.c
ch->state = WCOM_LINK_STATE_CONNECTED;
ch->type = LINK_TYPE_UART;
ch->uart = uart;
// ... no authentication step before CONNECTED state
_fire_state_change_handlers(ch->id, ch->state);
return ch->id;

// wendy_com_agent.c — identical dispatch regardless of transport type
switch (cmd.which_params) {
case WendyComCommand_protocol_version_tag: ...
case WendyComCommand_app_push_begin_tag:  ...
case WendyComCommand_app_stop_tag:        ...

Remediation:

1. Introduce a WCOM_LINK_STATE_AUTHENTICATING state for UART links before CONNECTED.
2. Implement a challenge-response handshake (e.g., device sends a random nonce; host signs with a pre-shared key or device-enrolled certificate) before any command is accepted.
3. Alternatively, define a reduced command-set allow-list for UART links and block privileged commands (app_push_*, reboot) at the dispatch layer based on ch->type.
4. At minimum, document the threat model in a security design note so downstream integrators are aware of the risk.

---

HIGH — Unauthenticated Escape-Sequence Mode Switching Can Hijack Active TLS Session

Standards: SOC2-CC6, ISO27001-A.9, NIST SP 800-53 AC-3

Description:
The _task_main loop reads raw bytes from the USB serial port and processes escape sequences independently of whether a TLS session is active on the same physical device. The escape command ESC + 'o' (USJ_MODE_OFF) or ESC + 'c' (USJ_MODE_CONSOLE) is processed even when s_com_link_id >= 0. An attacker who can inject bytes into the USB stream (e.g., via a compromised USB hub or malicious host driver) could tear down an in-progress authenticated TLS session or redirect traffic.

Additionally, when a COM-mode link disconnects, _on_com_state_change calls _handle_esc_command(s_com_uart.last_esc_cmd) using the last byte seen before the EOF. This means that crafted data received over the UART can trigger arbitrary mode transitions, including re-entering COM mode.

Relevant snippet:

// wendy_usj.c
} else if (esc_pending) {
    _handle_esc_command(b);   // no validation of current state
    esc_pending = false;

// _on_com_state_change
if (s_com_uart.last_esc_cmd) {
    _handle_esc_command(s_com_uart.last_esc_cmd);  // attacker-controlled byte

Remediation:

1. Validate state transitions: only allow mode changes from defined predecessor states (e.g., OFF → CONSOLE, OFF → COM); reject transitions that would interrupt an active session.
2. Sanitise last_esc_cmd: only act on it if it is one of the explicitly expected command bytes; reject anything else rather than passing it to _handle_esc_command.
3. Clear last_esc_cmd after use to prevent replay of the same transition.

---

HIGH — Device Identity Disclosed Over Unauthenticated Plaintext Channel

Standards: SOC2-C1, ISO27001-A.8, NIST SP 800-53 SC-8

Description:
The new get_device_identity command returns id, name, and display_name fields. On the UART transport these are sent in plaintext over USB without any authentication. Depending on the deployment, these values may uniquely identify the device and its owner (satisfying GDPR Article 4 "personal data" definition if the device is associated with a natural person). There is no access control check before the identity response is returned.

Relevant snippet:

// wendy_com_cmd.c
WendyComResult wcom_cmd_get_device_identity(WendyComDeviceIdentity *out)
{
    ESP_LOGI(TAG, "GET_DEVICE_IDENTITY");
    const char *id = "???", *name = "???", *display_name = "???";
    if (_app_delegate && _app_delegate->on_get_device_identity)
        _app_delegate->on_get_device_identity(&id, &name, &display_name);
    // ... immediately encodes and returns to caller, no transport check

Remediation:

1. Gate wcom_cmd_get_device_identity (and all other commands) behind the authentication step described in the CRITICAL finding.
2. If identity must be available before authentication (e.g., for device discovery), return only a non-sensitive public identifier (e.g., a random device UUID) and withhold human-readable name and display name until post-auth.
3. Add a link_id / transport-type parameter to wcom_cmd_get_device_identity so it can apply per-transport policy.

---

MEDIUM — Command ID Logged Without Authentication Context

Standards: SOC2-CC7, ISO27001-A.12, NIST SP 800-53 AU-3

Description:
The log line added in wendy_com_agent.c records the request_id of every received command before authentication is confirmed. On the UART transport this means unauthenticated probing activity is logged identically to legitimate authenticated traffic. Log consumers cannot distinguish an attack from normal use. Furthermore, logging prior to authentication means that an adversary can inflate log volume as a minor DoS against log storage.

Relevant snippet:

// wendy_com_agent.c
ESP_LOGI(TAG, "ch%d received cmd id %d", link->link_id, cmd.request_id);
switch (cmd.which_params) {

Remediation:

1. Include transport type in the log tag or message so SIEM/log analysis can distinguish UART from TLS.
2. Consider demoting this to ESP_LOGD (debug) for production builds to reduce log volume and prevent information leakage through logs.
3. Once authentication is implemented, log authentication state alongside command receipt.

---

MEDIUM — fcntl Return Values Not Checked

Standards: ISO27001-A.8, NIST SP 800-53 SI-10

Description:
In both wcom_add_tls_link and wcom_add_uart_link, the call to fcntl(fd, F_GETFL, 0) can return -1 on error (e.g., if fd is invalid). The subsequent fcntl(fd, F_SETFL, flags | O_NONBLOCK) would then set O_NONBLOCK on ~0 | O_NONBLOCK (effectively random flags) or fail silently. If non-blocking mode is not actually set, the select()-based event loop will block indefinitely on the first read or write, causing a denial of service on the main task.

Relevant snippet:

// wendy_com_link.c
int flags = fcntl(ch->fd, F_GETFL, 0);   // unchecked
fcntl(ch->fd, F_SETFL, flags | O_NONBLOCK);  // unchecked

Remediation:

int flags = fcntl(ch->fd, F_GETFL, 0);
if (flags < 0) {
    ESP_LOGE(TAG, "fcntl F_GETFL failed: %d", errno);
    // handle error: close fd, return -1
}
if (fcntl(ch->fd, F_SETFL, flags | O_NONBLOCK) < 0) {
    ESP_LOGE(TAG, "fcntl F_SETFL O_NONBLOCK failed: %d", errno);
    // handle error
}

---

MEDIUM — Static wcom_operation Shared Across Re-entrant Calls

Standards: SOC2-CC8, ISO27001-A.8

Description:
In _handle_esc_command, the wcom_operation struct op is declared static. If _enter_com_exec is called while a previous operation is still queued or executing (e.g., due to rapid back-to-back escape sequences), the second call overwrites the func pointer in the shared struct. This is a race condition between the calling task and the wcom main task processing op. In the worst case it could redirect execution to a stale function pointer.

Relevant snippet:

// wendy_usj.c
static void _handle_esc_command(uint8_t cmd)
{
    static struct wcom_operation op;   // shared across all calls
    ...
    case WENDY_COM_UART_ESC_CMD_COM:
        op.func = _enter_com_exec;
        wcom_exec(&op);  // op may still be in use

Remediation:

1. Guard with a flag or mutex: only submit a new COM-mode operation if no COM link is currently active.
2. Alternatively, allocate op on the stack or use a pool, ensuring each call has its own instance with a known lifetime.
3. The existing s_com_link_id >= 0 check should be applied before setting op.func and calling wcom_exec.

---

LOW — Hard-Coded Fallback "???" Strings

Standards: ISO27001-A.8, NIST SP 800-53 SI-10

Description:
When _app_delegate is NULL or on_get_device_identity is unset, the identity response returns id="???", name="???", display_name="???". A CLI consumer receiving this may silently accept it as a valid identity, causing display or matching logic errors. It also confirms to an unauthenticated requester that the device has no identity configured, which is operational reconnaissance information.

Relevant snippet:

const char *id = "???", *name = "???", *display_name = "???";
if (_app_delegate && _app_delegate->on_get_device_identity)
    _app_delegate->on_get_device_identity(&id, &name, &display_name);

Remediation:

1. Return WendyComResult_WENDY_COM_RESULT_UNKNOWN_ERROR (or a dedicated NOT_CONFIGURED error code) if the delegate is absent, rather than sending placeholder strings.
2. Do not encode the identity fields into the response when the delegate is not registered.

---

LOW — termios Flags Logged at INFO Level

Standards: SOC2-CC7, ISO27001-A.12

Description:
The c_iflag and c_oflag values of the /dev/usbserjtag tty are logged at ESP_LOGI level before and after modification. In a production device these logs may be captured by a cloud logging pipeline. While termios flags are not secrets, logging internal driver state at INFO level is noisy and may expose configuration details useful to an attacker performing device fingerprinting.

Relevant snippet:

ESP_LOGI(TAG, "termios before: c_iflag=0x%08lx c_oflag=0x%08lx", ...);
// ...
ESP_LOGI(TAG, "termios after:  c_iflag=0x%08lx c_oflag=0x%08lx", ...);

Remediation:
Demote both log lines to ESP_LOGD (debug level) so they are compiled out in release builds (CONFIG_LOG_DEFAULT_LEVEL typically excludes debug).

---

INFORMATIONAL — Injecting Sources into Third-Party IDF Component

Standards: SOC2-CC9, ISO27001-A.8

Description:
wendy_usj/CMakeLists.txt adds usb_serial_jtag_vfs.c directly to the esp_driver_usb_serial_jtag component library via target_sources(${usj_lib} PRIVATE ...). This bypasses the upstream component's own build guards and may silently break when IDF is upgraded. From a software supply chain perspective, this creates an implicit dependency on the internal structure of a third-party component.

Relevant snippet:

idf_component_get_property(usj_lib esp_driver_usb_serial_jtag COMPONENT_LIB)
target_sources(${usj_lib} PRIVATE "${usj_dir}/src/usb_serial_jtag_vfs.c")

Remediation:

1. Document the exact IDF version this has been tested with and add a cmake_minimum_required or version assertion.
2. Track this as technical debt; replace with an upstream-supported mechanism when Espressif exposes it (or contribute a Kconfig option upstream).
3. Add a CI check that fails if the referenced source file does not exist after an IDF update.

---

INFORMATIONAL — Device Identity Fields May Constitute Personal Data

Standards: GDPR Art. 4, Art. 5(1)(b), SOC2-P-series

Description:
The WendyComDeviceIdentity message contains id, name, and display_name fields. If Wendy Lite devices are deployed in consumer contexts where the device name or display name reflects the owner's name or location (e.g., "Alice's Kitchen Sensor"), these fields may qualify as personal data under GDPR. There is no apparent retention policy, consent mechanism, or data-minimisation control in this diff.

Remediation:

1. Audit what values are actually populated into name and display_name by the application delegate.
2. If names could identify natural persons, classify the data and ensure appropriate lawful basis, minimisation, and deletion-on-request support exists at the application layer.
3. Consider documenting the data classification of these fields in the protocol specification.

---

Compliance Summary

Framework	Checked	Violations Found
SOC 2 (CC6, CC7, CC8, CC9, C1)	✅	Yes — CC6 (no auth on UART), CC7 (log context), CC8 (static op race), CC9 (third-party injection), C1 (plaintext identity)
ISO/IEC 27001:2022 (A.8, A.9, A.12)	✅	Yes — A.9 (no authentication), A.8 (unchecked fcntl, static op, fallback strings, third-party CMake), A.12 (log quality)
PCI DSS v4.0	✅	Not applicable — no payment/card data flows in this diff
GDPR / Privacy	✅	Potential risk — device identity fields may be personal data; no consent/retention controls visible
HIPAA	✅	Not applicable — no health/medical data in this diff
NIST SP 800-53 / CSF 2.0 (AC, AU, IA, SC, SI)	✅	Yes — IA-2 (no authentication), AC-3 (no authorisation on UART), SC-8 (no encryption), SI-10 (unchecked fcntl), AU-3 (log context)