Code audit: tracked findings (terminal, X11, engine, native helpers)

[ExTV]

Overview

This is a consolidated tracking issue for findings from a code review pass over the current main. None are crashes or data-loss bugs; the migration/overlay/boot paths and the dual-backend abstraction are clean. The items below cluster in the terminal input layer, the X11 viewer, and the guest-side C helpers.

Each item lists the affected file, the concrete impact, and a suggested fix. They will be addressed incrementally; checkboxes track progress.

Status: the three Medium items below shipped in v1.2.5. The Low items remain open.

Priority legend: Medium = address first (one is a local-network exposure, one is a resource leak on a common path). Low = self-healing or requires an abnormal precondition.

---

Medium

• Reserved-port shadowing can expose the in-app desktop on the local network  security  (fixed in v1.2.5)
  app/src/main/java/com/excp/podroid/service/PodroidService.kt
  The implicit X11 display (host port 5900) and audio (4713) forwards are only injected when no existing rule already uses that host port. A user-created port-forward on 5900 or 4713 is persisted without the loopback flag (it isn't serialized, so it defaults to off), shadows the implicit loopback-bound rule, and binds 0.0.0.0 on the next boot instead of 127.0.0.1. That exposes an unauthenticated X session and the raw audio stream to the local network. There is no visible symptom because the in-app viewer connects to 127.0.0.1.
  Fix: treat 5900/4713 as reserved host ports (reject them in the add-forward path and dialog validation), and always inject the loopback-bound forwards in the launch path after removing any pre-existing rule on those ports. SSH (9922) is intentionally LAN-reachable and must not be changed.

• X11 viewer leaks its socket, read coroutine, and audio stream on exit  resource-leak  (fixed in v1.2.5)
  app/src/main/java/com/excp/podroid/ui/screens/x11/X11ViewModel.kt
disconnect() cancels the session job but never closes the RFB socket. The read loop is parked in a blocking native read with no socket timeout, and coroutine cancellation cannot interrupt it; on an idle remote desktop (no incremental updates arriving) the cleanup finally never runs, so the socket, its IO thread, and the audio stream leak on every exit from the X11 screen and accumulate across navigation cycles.
  Fix: close the RFB socket in disconnect() (wrapped for idempotency) so the read unblocks and the existing cleanup path runs. This mirrors how AudioStreamer.stop() already unblocks its own read.

• A stuck local client can wedge the host-bridge daemon  reliability  (fixed in v1.2.5)
  build-rootfs/host-bridge/podroid-hostd.c
  The serial CLI accept loop reads the request line with no timeout. A guest process that connects but never sends a newline blocks the single-threaded daemon indefinitely, hanging every subsequent podroid-notify / podroid-forward / podroid-open / podroid-power call for the session. The host-channel side already guards against this with a 5s read timeout; the guest-CLI side does not.
  Fix: use the existing read_line_timeout(..., HOST_TIMEOUT_S) helper for the CLI request read; the existing <= 0 handling then drops a non-responsive client.

---

Low

• Boot-stage detector is reused across stop/start with a feed/reset race  concurrency
app/src/main/java/com/excp/podroid/engine/BootStageDetector.kt, engine/QemuEngine.kt
  The QEMU backend reuses one detector instance and resets it in place at start(), while the previous boot-monitor coroutine may still be calling feed() (no join, no happens-before edge on the mutable fields). A stale chunk can corrupt the new run's scan offsets or prematurely flip state to Running. The AVF backend already allocates a fresh detector per run.
  Fix: allocate a fresh detector per run (mirror the AVF pattern) instead of reusing and resetting one instance.

• QEMU child stdout pipe is never drained  resource-leak
app/src/main/java/com/excp/podroid/engine/QemuEngine.kt
  Only the child's stderr is consumed; stdout is left as an unread OS pipe. This is harmless today because nothing is routed to stdout, but user-supplied QEMU extra args such as -monitor stdio could fill the pipe buffer over a long session and deadlock the VM with no diagnostic.
  Fix: discard the child's stdout before start (do not merge it into stderr, which feeds error formatting).

• AVF disk-create failure is swallowed and boots a partial image  reliability
app/src/main/java/com/excp/podroid/engine/avf/AvfEngine.kt
  On the create path, a setLength failure is logged and swallowed, and the 0-byte file is returned and booted, surfacing as an opaque early-boot stop rather than a clear storage error. It self-heals on the next boot via the grow path; impact is diagnostic clarity. The same swallow exists in the QEMU path for parity.
  Fix: rethrow as an IOException with a user-facing message so the start error path reports it clearly.

• Terminal auto-reconnect has no backoff or attempt cap  reliability
app/src/main/java/com/excp/podroid/ui/screens/terminal/TerminalViewModel.kt
  When the bridge session finishes while the VM is still Running, a reconnect is signaled unconditionally. A half-wedged chardev that accepts the connection then immediately EOFs can spawn a fresh bridge process plus its threads many times per second until the VM state changes.
  Fix: add a reconnect governor (attempt cap plus a minimum interval between attempts) and fall back to a "disconnected, tap to retry" state when exceeded.

• Fast pinch-zoom drops font-size steps  bug
app/src/main/java/com/excp/podroid/ui/screens/terminal/TerminalViewModel.kt
  Pinch handling computes the next font size from a DataStore-backed StateFlow whose value has not committed yet, so rapid callbacks within one gesture read the same stale value and collapse several intended steps into one. Self-corrects on the next slow pinch; nothing is corrupted.
  Fix: keep a synchronous in-memory font size as the step base; optionally debounce the persisted write to the end of the gesture.

• Pinch-zoom and the font slider use inconsistent bounds  bug
app/src/main/java/com/excp/podroid/ui/screens/terminal/TerminalViewModel.kt, ui/screens/terminal/TerminalScreen.kt
  Pinch coerces the font size to 8..48, but the Quick Settings slider range is 10..36, so pinch can persist a size the slider then silently snaps away from (and a comment incorrectly claims the two are in sync).
  Fix: share one bound constant between both surfaces and clamp in the setter.

• Ctrl with sub-64 code points is dropped  correctness
app/src/main/java/com/excp/podroid/ui/screens/terminal/TerminalViewModel.kt
  The control transform is applied only for code points 64..127, and the handler returns true unconditionally, short-circuiting the terminal view's canonical mapping for lower code points. Most notably Ctrl+Space no longer sends NUL (breaks readline set-mark, Emacs C-Space).
  Fix: return false for the ctrl combinations the handler does not itself transform, so the terminal view applies the canonical sub-64 mapping.

• Update check backs off a full day on transient network failure  reliability
app/src/main/java/com/excp/podroid/data/repository/UpdateRepository.kt
  The failure path persists the 24h check timestamp for every error, including offline / DNS / timeout, so a single offline launch suppresses the next in-app update check for a day.
  Fix: on an IOException, write a short retry floor (around 15-30 min) instead of the full 24h timestamp; keep the 24h timestamp only for definitive outcomes where GitHub was actually reached.

• Unchecked allocation and base64 result in podroid-notify  bug
build-rootfs/host-bridge/podroid-hostd.c
  The notify path writes to an unchecked malloc result and passes a possibly-NULL base64 encode result to snprintf("%s", ...), unlike the adjacent title field which is guarded. Under musl this produces a malformed NOTIFY line rather than a crash, but the asymmetry should be closed.
  Fix: NULL-check the allocation and guard the encoded body the same way as the title.

• vsock-agent post-fork bind failure leaves a dead listener  reliability
build-rootfs/vsock-agent/podroid-vsock-agent.c
  The listener spawn records the {vport, pid} and rewrites the config before the forked child reports its bind result, so a bind failure (for example during a fast remove-then-add window) advertises a forward that is not actually listening. It self-corrects on the next control operation via the liveness probe, and the config file is never read back at runtime, so there is no persisted corruption.
  Fix: pipe a bind success/failure byte from the child to the parent before the parent commits the listener row, on both the TCP and UDP paths.