fix(system): sanitize relay-derived terminal output - #70
Conversation
Review 1ABLOCKING TEST GAP
Review method and scopeRisk: high — relay-controlled data reaches live-terminal stderr and headless echoed logs. Reviewed the complete pinned diff from I ran:
All passed. I did not mutate the worktree; the supplied direct- The reachable HTTP controls correctly exercise login-error writing, immediate Residual risk: NEEDS ATTENTION |
Review 1B
The required
No approval or code finding is inferred from an incomplete review slot. |
Initial high-risk review wave — aggregatePinned unchanged head:
Disposition: accept Review 1A's test-gap finding for batch remediation, but do not edit yet. Review policy requires the full unchanged-head wave, so work is blocked until the original Review 1B slot can complete (or the user explicitly selects an allowed alternative). No other findings or follow-ups are available to deduplicate yet. |
Replacement Review 1B — Grok 4.6The main orchestrator selected Review 1BRisk: high — untrusted relay data crosses live-terminal stderr and headless log-echo boundaries. BLOCKING TEST GAP
FOLLOW-UP
Review method and scopeReviewed the complete pinned diff Commands actually run:
I did not mutate the worktree. I did not rerun the deliberate Changed-file sweep:
Production behavior for the two named sinks and the pairing degradation matches the contract. Printable text is preserved; ESC/C0, C1, bidi/format, and empty-after-sanitize are covered on those paths. No backend API change and no general log normalization. The #61 correction comment in Residual risks
NEEDS ATTENTION |
Completed initial high-risk review wave — deduplicated aggregatePinned unchanged head: The main orchestrator explicitly selected Grok 4.6 as replacement Review 1B after durable verification of the genuine Kimi K3-256K quota blocker. This is not a claim that the user authorized the model replacement. The earlier Kimi Deduplicated findings
No other blocking findings, test gaps, or follow-ups were reported. Remediation will now be batched in one changed head; all prior approvals are invalidated, and both reviewer contexts must approve the same final green head. |
Batched remediation pushedFinal candidate head: Dispositions
Deliberate remediation-gate failure proofsExact command for every mutation:
relayErr := err
fmt.Fprintln(os.Stderr, relayErr)Complete failure:
relayErr := err
d.logf("ports poll failed: %v", relayErr)Complete failure:
fmt.Fprintf(os.Stderr, "Registered this system as %s (%q).\n", out.Name, "control")Complete failure: Each mutation was reverted. The identical gate command passed green afterward. Local gofmt, diff check, |
Final Review 1A — Terra remediation verificationNo BLOCKING, BLOCKING TEST GAP, or FOLLOW-UP findings. Prior blocker verified resolved:
Reviewed complete final diff Verified exact head and green GitHub checks. Ran Residual risk remains intentionally bounded: APPROVE |
Final replacement Review 1B — Grok 4.6 remediation verificationReview 1B (remediation)Risk: high — untrusted relay data still crosses live-terminal stderr and headless log-echo boundaries. No remaining Prior findings
Remediation and final diffProduction files are unchanged from Review method and scopeFollow-up on unchanged production behavior plus the gate remediation. Pinned final head Commands actually run:
Did not mutate the worktree. Did not rerun the three deliberate gate-failure proofs; those were checked against the analyzer and the supplied messages. Changed-file sweep of the final seven-file diff: Residual risks
APPROVE |
Final remediation review aggregateExact approved head:
The branch is behind current |
73e332c to
9f0dc7f
Compare
Post-sync Final Review 1A — TerraNo BLOCKING, BLOCKING TEST GAP, or FOLLOW-UP findings. Post-sync verification completed for head
Residual risk remains the deliberately bounded APPROVE |
Post-sync Final replacement Review 1B — Grok 4.6Review 1B (post-sync)Risk: high — untrusted relay data still crosses live-terminal stderr and headless log-echo boundaries. No Replay
Independently recomputed:
Both are Intervening main
Durable history (unchanged dispositions)
No new evidence to relitigate those dispositions. Review method and scopePost-sync verification of an unchanged binary diff after rebase onto current main. Contract: only the final Correction (2026-08-23): active batch contract on nicodes/ormos-be#420. Commands actually run:
Did not mutate the worktree. Did not rerun deliberate gate-failure mutations; the analyzer text is byte-identical to the approved head. Changed-file sweep of Residual risks
APPROVE |
Post-sync final approval aggregateExact final head:
Merge gates are satisfied for this exact head. |
Summary
%qallowlistRisk
Risk: high — untrusted relay data crosses live terminal and headless log output boundaries.
Acceptance contract
Implements only the final Correction (2026-08-23): active batch contract on nicodes/ormos-be#420. The earlier unusable batch section remains untouched.
audit.jsonlbidi handling remains explicitly out of scope, and no backend API behavior changes.Links nicodes/ormos-be#420
Epic: nicodes/ormos-be#493
Verification
mise exec -- go test -count=1 ./internal/system -run '^(TestRelayDerivedOutputGate|TestLoginErrorOutputSanitisesRelayText|TestDeviceStartRejectsValuesEmptyAfterSanitising|TestHeadlessPortsPollEchoSanitisesRelayStatus)$'test -z "$(mise exec -- gofmt -l .)"mise exec -- go vet ./...mise exec -- go test -race -count=1 ./...mise exec -- govulncheck ./...(0 reachable vulnerabilities)Deliberate source-gate failure proof
Temporary reachable mutation after
performLogininrunSystem:Command (exact parameters):
Observed complete failure:
The mutation was reverted, and the identical command passed.
Controls and reachability
/device/startresponse and pass them through the exact writer used after TUI teardown; a local configuration error is the known-good siblingpollPortstick throughfetchConfiguredPortsandlogf's echo writer; a local tunnel log is the known-good siblingdeviceStartvalidation path; a printable code/URL response is the control%qfor the relay-chosen registered system name is the gate's explicit proven-safe formatting allowlist