Skip to content

oom(12/29): bound SSH remote payloads, flow control & roster - #10305

Closed
nwparker wants to merge 1 commit into
oom/11-b-providers-usagefrom
oom/12-b-ssh
Closed

nwparker wants to merge 1 commit into
oom/11-b-providers-usagefrom
oom/12-b-ssh

Conversation

@nwparker

@nwparker nwparker commented Jul 24, 2026 •

Copy link
Copy Markdown
Contributor

OOM hardening slice 12/29 — bound SSH remote payloads, flow control & roster

Part of a 29-PR stack re-landing #10179 ("bound OOM-prone accumulators across Orca"), which was reverted in #10255 for being too large (~1,600 files). This is slice 12/29 (base: oom/11-b-providers-usage). Merge in numeric order. The full stack's end-state is byte-for-byte the commit that passed #10179's complete CI matrix (36k+ tests, multi-OS).

Summary

Re-lands the SSH slice of the OOM-fix: bounds every unbounded SSH accumulator — relay frame decoder, JSON-RPC encode/parse, file/git stream reassembly, SSH-config Include expansion, connection-generation registry, directory transfers, key-file reads, remote-probe outputs, port-scan results, relay marker/log/listing reads, and system-SSH stderr tails — with explicit count/byte/depth/concurrency ceilings and matching regression tests.

ELI5

Orca talks to remote machines over SSH. Before, several places would keep swallowing whatever the far side sent — a giant file, a huge config, a flood of tiny packets — until Orca ran out of memory and crashed. This change puts a sensible "that's too big, stop" limit on each of those spots. The limits are set high enough that normal work never notices, but a broken or malicious remote can no longer balloon memory. A few things now get politely truncated (long error messages, oversized logs) instead of kept whole.

Proof of OOM fix

  • Relay frame decoder: MAX_BUFFERED_FRAME_CHUNKS=4096 + 8KB tiny-fragment coalescing; oversized frames (>16MB MAX_MESSAGE_SIZE) stream-discarded so a 13-byte header can't pin 4GiB — relay-protocol.ts, tests relay-protocol.test.ts + ssh-channel-multiplexer.test.ts
  • JSON-RPC encode uses stringifyJsonWithinByteLimit(MAX_MESSAGE_SIZE=16MB) instead of materializing the full string first (test asserts Buffer.from never called) — relay-protocol.ts / relay-protocol.test.ts
  • Git response stream cap MAX_GIT_RESPONSE_STREAM_BYTES=64MB, MAX_GIT_RESPONSE_STREAM_CHUNKS=4096; isGitResponseStreamMarker now requires Number.isSafeInteger — relay-protocol.ts, ssh-git-response-stream-assembler.ts, ssh-git-response-stream-reader.test.ts
  • SSH stream reassembly budgets: MAX_SSH_STREAM_ASSEMBLY_BYTES=128MB shared, per-reader MAX_PRE_METADATA_STREAM_FRAMES=64/8MB, shared MAX_SSH_PRE_METADATA_STREAM_FRAMES=256/32MB, 30s inactivity deadline — ssh-stream-reader-memory.ts / ssh-stream-reader-memory.test.ts
  • SSH-config Include expansion: SSH_CONFIG_INCLUDE_LIMITS {expandedBytes 16MB, expandedLines 200k, fileBytes 1MB, files 1024, globMatches 256, nestingDepth 16, sourceBytes 16MB} + MAX_GLOB_PATTERN_SEGMENTS=64 bounded glob — ssh-config-expansion-budget.ts, ssh-config-bounded-glob.ts, tests ssh-config-include-expansion-bounds.test.ts / ssh-config-bounded-glob.test.ts
  • Connection-generation registry: MAX_TRACKED=4096, MAX_RETAINED_LEASES=8192, target-id 64KB / retained 4MB, lease-based eviction — ssh-connection-generation.ts / ssh-connection-generation.test.ts
  • Directory transfer budget: maximumEntries=4096, depth=64, pathBytes=16KB, retainedPathBytes=4MB, fileBytes=16MB, totalFileBytes=32MB, Windows package 48MB — ssh-directory-transfer-budget.ts, tests ssh-directory-transfer-budget.test.ts / sftp-upload.test.ts
  • SSH key file read cap SSH_KEY_FILE_MAX_BYTES=1MB — ssh-key-file.ts / ssh-key-file.test.ts
  • Bounded remote probes: relay base-dir listing 4096 entries/256KB, node-candidate 256/64KB + 64KB profile reads, Windows liveness 4096 dir entries/1024 pipes, marker reads 1KB, Windows log tail 64KB — respective *.ts with .test.ts
  • System-SSH stderr/stdout bounded to 64KB tail (SystemSshOutputTail) with truncation notice; connection error clamped via clampSshConnectionError — system-ssh-output-tail.ts, ssh-connection.ts, tests system-ssh-output-tail.test.ts / ssh-connection.test.ts

Regressions

Normal-path (ordinary use, below the new limits): ⚠️ needs sign-off:

  • low: uploadDirectory now throws 'Upload source changed during transfer' / readLocalUploadFile throws 'File changed during upload' when a file's type or size changes mid-transfer, where the old code silently skipped/completed. (trigger: Concurrent modification of the local source tree during an SSH upload.)
  • low: ssh-port-scanner.ts now routes remote scan results through admitSshDetectedPorts, which hard-caps the port list at SSH_DETECTED_PORTS_MAX_ENTRIES=50 (scanLimit = min(length,50)). A remote reporting >50 detected listening ports will only surface the first 50 in the port-forwarding UI. Confirmed behavior change vs BASE (which used result.ports directly). Below 50 ports there is no change. (trigger: Remote host exposes more than 50 detected listening/forwardable ports in a single scan.)
  • medium: The bounded SSH-config glob no longer preserves BASE's recursive ** behavior. /Users/nwparker/orca/workspaces/orca/oom-followup/src/main/ssh/ssh-config-bounded-glob.ts:55-65 applies path.matchesGlob to exactly one directory segment, while /Users/nwparker/orca/workspaces/orca/oom-followup/src/main/ssh/ssh-config-include-expander.ts:158-176 routes every glob through it. A normal Include **/*.conf therefore imports only files exactly one directory below the config directory, omitting root-level and deeper aliases that BASE's fs.globSync imported. (trigger: A user's SSH config uses a recursive Include such as Include **/*.conf; this occurs below every new count, byte, and depth ceiling.)
  • low: SSH port discovery now retains and publishes only the first 50 relay rows (ssh-port-scanner.ts:79-84 via SSH_DETECTED_PORTS_MAX_ENTRIES = 50). On a busy but legitimate shared/server remote with more than 50 listening endpoints, later development ports disappear from Orca's detected-port UI. (trigger: The remote reports more than 50 detected listening ports in one scan.)

Overload-only (extreme input / sustained backpressure) — intended, documented degradations:

  • Directory upload/removal (uploadDirectory, uploadDirectoryViaSystemSsh, removeDirectorySftp) throws SshDirectoryTransferCapacityError past 4096 entries / depth 64 / 32MB total — callers are limited to relay-package deploy/GC, which is a fixed small bundle far below these caps
  • Git response streams >64MB or >4096 chunks, and >128MB of concurrent SSH stream reassembly, now reject with StreamProtocolError instead of allocating
  • SSH-config Include expansion silently truncates (with console.warn) only for pathological configs (>1024 files, >16MB expanded, >16 nesting depth, >256 glob matches)
  • SSH key files >1MB are skipped (key silently ignored → falls back to other auth) — no real key approaches 1MB
  • Remote node-candidate probe returns null (falls back to login-shell resolution) only when >256 candidates or >64KB output; NVM_DIR set past 64KB in a giant .bashrc no longer detected
  • Relay marker/liveness/listing commands emit a too-large sentinel and skip GC only when remote dirs exceed 4096 entries / 256KB
  • SSH connection error strings shown to the user and system-SSH stderr in error details are now truncated (clampSshConnectionError; 64KB tail with '[earlier system SSH output truncated]' notice) instead of shown whole. (only when: Only when the remote emits an unusually large error/stderr payload; ordinary errors are unaffected.)
  • Directory upload/remove over SSH now aborts with SshDirectoryTransferCapacityError instead of best-effort continuing when a tree exceeds 4096 entries or depth 64 (Windows system-ssh path also 16MB/file, 32MB total, 48MB package). (only when: Reachable only if the relay package being deployed ever grew past these caps; current relay bundle is well under them, so effectively overload-only.)
  • SSH connection error strings are clamped to 16KB (clampSshConnectionError -> SSH_CONNECTION_ERROR_MAX_UTF8_BYTES) and system-ssh stderr is tailed to a 64KB SystemSshOutputTail with an '[earlier system SSH output truncated]' notice. Ordinary error messages are far below these sizes and are unaffected. (only when: Remote/system ssh emits an unusually large error or stderr payload.)
  • Large diagnostics and command output are intentionally shortened: connection errors are clamped to 16 KiB, system-SSH stderr keeps a 64 KiB tail with a truncation notice, and SSH-hosted Orca CLI stdout/stderr are each capped at 8 MiB. (only when: An SSH failure or remote CLI command emits output beyond the corresponding ceiling; ordinary short diagnostics are unchanged.)
  • SSH relay directory upload/removal now fails with SshDirectoryTransferCapacityError beyond 4,096 entries, depth 64, 16 MiB per file, or 32 MiB total; Windows system-SSH packaging is additionally capped at 48 MiB. Concurrent local source mutation is also detected and fails instead of being silently skipped/completed. (only when: The relay deployment bundle exceeds a transfer ceiling, or the local bundle changes type/size while upload is in progress; the current fixed relay bundle is expected to remain well below the ceilings.)

Build / CI

  • Part of the Main services + CLI group; this group's cumulative tree is interlinked, so it becomes typecheck-green at its checkpoint PR Add dismiss button to update notifications #25 (whole-repo interconnection — see stack README). Content is exact production code.

Adversarial review

  • 3 skeptic lens(es) incl. a frontier-model (GPT) cross-check. Sign-off: FLAGGED — see above. Residual risk: high.
  • Verified the SSH slice (git diff aab1129..6eb70d8 over 12-B-ssh.filelist). No confirmed normal-path regression; every new ceiling is either pre-existing or far above ordinary use. Sign off = true.

CEILINGS VERIFIED AS OVERLOAD-ONLY:

  • Directory-transfer caps (4096 entries / depth 64 / 16MB per file / 32MB total / 48MB Windows package): the ONLY callers of uploadDirectory / removeDirectorySftp / uploadDirectoryViaSystemSshWindows are the relay-bundle deploy and GC paths (ssh-connection.SshConnection.uploadDirectory <- ssh-relay-deploy.ts uploadDirectoryForConnection; removeDirectorySftp has no live caller other than the relay helpers). User-initiated file/folder operations in the SSH filesystem provider go through the relay RPC fs.deletePath and single-file uploadFile/uploadBuffer, NOT these budgeted tree walkers. The relay bundle (out/relay/) is a fixed small node bundle shipped with Orca, well under 4096 files / 32MB; packaging CI would catch growth. Confirmed not user-facing.
  • Git response stream 64MB / 4096 chunks: the git runner caps command output at DEFAULT_GIT_MAX_BUFFER = 10MB (src/main/git/runner.ts:254), so a serialized git result reaching 64MB is unreachable in normal use.
  • File-stream caps MAX_PREVIEWABLE_BINARY_SIZE=50MB / MAX_TEXT_FILE_SIZE=10MB are PRE-EXISTING (moved verbatim from ssh-filesystem-stream-reader.ts into ssh-file-stream-assembler.ts), not new limits.
  • SSH-config include limits (1024 files, 16MB expanded, 16 nesting depth, 256 glob matches, 1MB/file): the 1MB per-file cap pre-existed (MAX_INCLUDE_FILE_BYTES); the rest are orders of magnitude above any real ssh config.
  • SSH key file 1MB, remote node probe (256 candidates / 64KB / 64KB profile / 4KB NVM_DIR), relay marker 1KB, base-dir listing 4096/256KB, liveness probe 4096/1024: all far above real inputs; each degrades safely (falls back / skips GC), none corrupts.
  • 128MB concurrent SSH stream-assembly budget and 32MB/256-frame pre-metadata budget: gated well above single-file preview sizes.

CORRECTNESS CHECKS PASSED:

  • FrameDecoder oversized-frame handling keeps MAX_MESSAGE_SIZE=16MB unchanged; normal <=16MB JSON-RPC frames still decode. The stream-discard + coalesce logic stays synchronized (regression tests in relay-protocol.test.ts).
  • spawnProxyCommand now applies pause/resume backpressure — order-preserving, an improvement not a regression.
  • ssh-channel-multiplexer dispose splice(0) is behavior-equivalent to the prior loop+clear.
  • advanceSshConnectionGeneration now returns number|null and no-ops without a retained lease; verified callers in ipc/ssh.ts (separate slice) always retainSshConnectionGeneration at connect time (lines 147/833/1072/1193) before advancing, and ignore the null return where used — coherent at TARGET (which passed CI).
  • readLocalUploadFile mid-read size-change guard only affects the relay bundle (not user files).

MINOR INACCURACIES IN PRIOR REVIEW (not defects):

  • Connection error string is clamped to a 16KB UTF-8 PREFIX (SSH_CONNECTION_ERROR_MAX_UTF8_BYTES via clampSshConnectionError), not a '64KB tail'. The 64KB tail applies separately to system-SSH stderr/probe output (SystemSshOutputTail). Both thresholds sit above any real message.

RESIDUAL ITEM FOR HUMAN AWARENESS (low): the port scanner in this slice now routes relay results through admitSshDetectedPorts, whose cap SSH_DETECTED_PORTS_MAX_ENTRIES = 50 lives in src/shared/ssh-retained-payload-admission.ts (a different slice). It keeps only the first 50 detected listening ports surfaced to the UI/auto-forward. Ordinary single-developer remotes stay well under 50, so I did not classify this as a normal-path regression, but a very busy multi-service remote box could have some detected ports silently dropped from the port list.

  • Correctness notes: src/main/ssh/ssh-connection-generation.ts: advanceSshConnectionGeneration() changed signature from number to number|null and now no-ops (returns null) for a target that has no retained lease, instead of creating/advancing one. Correctness depends on callers in src/main/ipc/ssh.ts (a different slice) first calling retainSshConnectionGeneration; verify the caller wiring lands together and handles null. Validated together at TARGET/CI.

Files

75 files changed (+4032 / −904).


🤖 Decomposed, reviewed & assembled by Claude Code from the reverted #10179. Draft — do not merge out of order.

@nwparker
nwparker force-pushed the oom/11-b-providers-usage branch from c4f8a58 to cc3d8f7 Compare July 24, 2026 07:48
@nwparker
nwparker force-pushed the oom/11-b-providers-usage branch from cc3d8f7 to 232dc84 Compare July 25, 2026 02:21
@nwparker
nwparker force-pushed the oom/11-b-providers-usage branch from 232dc84 to 90a6967 Compare July 25, 2026 02:39
@nwparker
nwparker force-pushed the oom/11-b-providers-usage branch from 90a6967 to 5ba5b23 Compare July 25, 2026 03:10
Files: 75 applied, 0 deleted (from 6eb70d8)

Co-authored-by: Orca <help@stably.ai>
@nwparker
nwparker force-pushed the oom/11-b-providers-usage branch from 5ba5b23 to 0830b4d Compare July 25, 2026 03:34
@nwparker

Copy link
Copy Markdown
Contributor Author

Superseded by #10548, which re-scopes this work to the bound-at-ingress doctrine (see #10533 and src/shared/memory-safety/README.md).

An audit found the original #10179 scope was ~50x the size of the fix for the crash it targeted, and that bounds had landed ahead of their callers. Every file in these slices was re-adjudicated individually: ingress bounds (transport frames, PTY flow control, on-disk transcripts, parsing, enumeration, media decode, user-keyed accumulators) were kept; interior ceilings behind an already-bounded input were dropped. 1,882 files → 1,155, full suite green, and no main feature regressed.

No memory protection was lost — only speculation.

@nwparker nwparker closed this Jul 25, 2026
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