Skip to content

fix: bound control pipe request reads - #6

Draft
RomainROCH wants to merge 2 commits into
devfrom
fix/ipc-request-bounds
Draft

RomainROCH wants to merge 2 commits into
devfrom
fix/ipc-request-bounds

Conversation

@RomainROCH

@RomainROCH RomainROCH commented Oct 4, 2026 •

Copy link
Copy Markdown
Owner

An incomplete control-pipe client can currently hold the single reader indefinitely. Limit each newline-terminated request to 4096 wire bytes and one total second using nonblocking Win32 byte reads. Empty, malformed, oversized, disconnected and unterminated requests are rejected; rejected connections also keep nonblocking replies so an unread large error cannot stall the next accept. Valid commands retain their existing decoder and reply behavior.

The isolated regression harness runs the production control/status loops on unique endpoints with an inert UI consumer. It verifies size boundaries and fragments, silent and trickling clients, disconnect without dispatch, recovery, status availability before the control deadline, and orderly shutdown with an incomplete client. Control permissions and the same-user/elevation policy remain unchanged.

Validation:

  • Original unbounded reader reproduced: no reply after two seconds with the partial client still open; dedicated status still replied.
  • Formatting and cargo check passed. Forty-one safe unit executions and six IPC executions passed, including five IPC unit cases shared by both harnesses. Final observed status latency was approximately 10 ms and partial/trickle rejection approximately one second.
  • MSVC release build, help/version and embedded asInvoker manifest passed. Tests/check/Clippy used Rust 1.96.0 on the installed GNU Windows toolchain; the release and CLI smoke used the installed MSVC toolchain.
  • Strict Clippy still reports inherent_to_string in autostart and redundant_closure in the existing status wake helper. Both were reproduced on the untouched base; allowing only those two lint classes passes with other warnings denied.
  • The native power-request test and system smoke were excluded because they change power or Task Scheduler state. Live cross-elevation control was not tested. Serial connection handling and existing UI/valid-response waits remain outside this targeted correction.

Independent production and test/documentation reviews found no remaining blocker in this correction.

This draft targets dev. The branch starts at the published main merge commit 942fd5919cf7cf3baddf4614becb877ca0483da1, which already contains the current dev tip. The diff against dev is limited to src/ipc.rs, tests/ipc_request_limits.rs, and docs/SECURITY_AUDIT.md. No additional elevation requirement is introduced. The full system smoke was not run locally; the Windows CI result is recorded below.

Windows CI

CI run 37222760065 passed for head 60399554db753cb010a6770e24ec1b8fa2dcae93: cargo check, 42 unit-test executions plus 6 IPC test executions, cargo build --release, and smoke tests (17/17 passed, 4 skipped). The skipped smoke cases cover two non-elevated status checks, the absent Update Orchestrator reboot task, and autostart without an interactive linked token. The workflow artifact was uploaded successfully. This run does not include strict Clippy or live cross-elevation validation.

RomainROCH and others added 2 commits October 4, 2026 04:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant