SEC-010: Bound remote SSH output

[budzeg]

Summary

Bound hostile SSH and remote-bootstrap output before it can cause unbounded memory retention or log amplification. The patch checks raw line and stream bytes before retained growth; separately caps protocol records, retained protocol bytes, diagnostic lines, redacted stderr tails, and tunnel stderr; and preserves the complete successful listdir envelope of DIR + 2,000 entries + LIST-DONE.

Repository history makes lifecycle ownership part of the security boundary. Remote SSH (3c234437) deliberately uses a durable remote Goose daemon behind a disposable local loopback tunnel; later generation/incarnation and log-drain hardening (baee7994) established stale-work rejection, resumable reconnects, and native-speed bounded draining as constraints. This fix therefore leaves daemon lifetime and reconnect policy unchanged. Output readers only report bounded-read failures; the owner of the Tokio Child exclusively terminates and reaps, and reader tasks finish before lifecycle/reconnect disposition. The new established-supervisor regression exercises that same owner path after readiness.

Remote SSH remains experiment-gated. The original feature excludes Windows remote hosts; that is distinct from using a supported Windows local Berd client. No current architectural law directly governs remote subprocess output (LAWS/README.md, LAWS/AGENTS.md, LAWS/CHAT.md).

The registry's older generation-owned numeric-PID teardown for disconnect/app exit is not represented as solved here. Replacing that authority with owner cancellation or an identity-safe handle is separate lifecycle-hardening scope.

Related issue

N/A — no public issue was opened because the underlying report is security-sensitive. No duplicate public issue or PR was found during the pre-submission search.

Testing

Validated resulting tree 33e0ac4a6e1629ad979fad825b27139e921c8d11, now exact head 49739fc01bf2081e9d355436429b7bf5c391a351, on origin/main 5c31189a4d55437888bb8d0f921efada827c7d86; combined binary-diff SHA-256 7c3b744ce3f41b0b527ef64f408861aa46a5259874cd401970709d9c5eb05c17.

• bin/just tauri-fmt-check — passed
• git diff --check origin/main...HEAD — passed
• bin/just tauri-check — passed
• bin/just clippy — passed across all four lanes
• bin/just tauri-test — passed
• Readiness-timeout, pre-readiness stderr-overflow, and established-supervisor stderr-overflow owner kill/reap/task-disposition regressions — passed
• Three timing-sensitive shell-contract tests — 15/15 individual repetitions passed on the prior behavior-equivalent head and 15/15 on clean origin/main
• History-led Zeg Squad review — VSec Bot, Geeky, and Vuln Buster independently returned PASS with no blocker on exact head 49739fc0 and reproduced the tree, base, and binary-diff fingerprint
• Copilot findings addressed in the resulting tree — invalid protocol UTF-8 has a distinct classification; task failures identify pipe and read/write operation; an already-exited child wins over an elapsed wait deadline; a completed stderr rejection wins over simultaneous child exit; and the tunnel reader field reuses its task alias. Focused regressions cover each case; fresh exact-head Copilot review recommends approval with 0 new comments

The lifecycle runtime regressions are Unix-only because their fixtures use sh, sleep, and yes. Windows receives Rust compile/test-lane coverage; this does not claim an equivalent native Windows runtime process-lifecycle test.

The fully concurrent app-crate suite is not clean on the baseline: the prior behavior-equivalent head produced 957 passed / 10 failed, while clean origin/main produced 945 passed / 12 failed. Both runs showed the same unrelated layout-default and provider-config failures plus timing-sensitive shell-contract failures; clean main had additional failures. This is baseline-noise evidence, not a claim that the fully concurrent suite is clean.

[Copilot]

🟡 Changes recommended

A few newly introduced diagnostic/error paths are ambiguous or misleading (and one minor type duplication), which should be corrected to keep security-limit rejections actionable.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR hardens the remote-backend SSH tunnel and remote bootstrap path by bounding untrusted stdout/stderr parsing and retention to prevent unbounded memory growth and log amplification, while preserving the allowed maximum listdir protocol envelope.

Changes:

• Introduces an allocation-bounded line reader (read_bounded_lines) that enforces per-line and per-stream byte limits before retained growth.
• Updates the SSH tunnel stderr reader to enforce byte/line/log caps and to report reader-limit violations back to the child owner for kill/reap + task disposal.
• Bounds remote bootstrap stdout/stderr parsing, caps protocol record count/retained bytes, and adds regression tests for timeout and overflow ownership behavior.

File summaries

File	Description
src-tauri/src/services/remote_backend/tunnel.rs	Replaces unbounded lines() stderr reading with bounded parsing + owner-controlled kill/reap and reader-task lifecycle management.
src-tauri/src/services/remote_backend/mod.rs	Wires in the new bounded-output module and adapts supervisor/establish flows to use the new tunnel lifecycle helpers.
src-tauri/src/services/remote_backend/daemon.rs	Bounds bootstrap stdout/stderr, caps protocol record/retained bytes, and adds tests for boundary envelopes + timeout kill/reap.
src-tauri/src/services/remote_backend/bounded_output.rs	New bounded line reader implementation + unit tests ensuring limits and UTF-8-split handling.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

There are confirmed edge-case correctness issues in process-wait timeout handling and tunnel stderr task disposal that can cause misclassification or loss of the output-rejection signal.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 3
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The changes consistently enforce bounded reads/retention across tunnel + bootstrap paths and include targeted regressions for the key lifecycle and race edge-cases introduced by the new reader tasks.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[morgmart]

🤖 Automated code review

APPROVE. The completed static review of exact range 5c31189...49739fc found no publishable findings. The four changed files and all supplied allowed anchors are confined to the Rust remote-backend implementation, so this is non-UI work. All six supplied prior threads are resolved, have substantive human replies, and describe issues fixed at this head. Supplied GitHub check evidence is structurally valid but pending; required checks still govern merge readiness.

Deterministic publication result: 0 blocking and 0 non-blocking inline finding(s) publishable; 0 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.

Pending checks: 1 check(s) are not complete.

This approval reflects the completed code review only; merge readiness remains governed by the repository's required checks.