[jyuan-dev]

Closes #432

Problem

When orx establishes connections to remote compute hosts (especially multi-hop bastion/jump hosts or OTP/password authentications), it creates an authenticated SSH multiplex master socket (/tmp/orx-ssh-<uid>-<hash>/<target_hash>).

However, when coding agents in chat sessions or shell commands execute ssh <host> ..., OpenSSH reads ~/.ssh/config. Without the matching ControlPath in ~/.ssh/config:

1. The agent's ssh command fails with authentication errors.
2. The agent spends tens of thousands of tokens inspecting /proc, searching environment files, testing ssh -vvv, and trying to recover.
3. The reproduction or task fails even though orx already has an active, authenticated tunnel to that host.

Solution

OpenSSH natively supports Include directives (OpenSSH 7.3+). This PR adds a managed SSH config include:

1. Managed Config File: ~/.config/openresearch/ssh_config (via crate::config::config_dir().join("ssh_config")).
2. Auto-sync: When a host connects or passes preflight checks in orx, orx updates its entry in ~/.config/openresearch/ssh_config:

   # Managed automatically by OpenResearch (orx). Do not edit manually.
   
   Host <host_alias>
     User <user>
     Port <port>
     ControlMaster auto
     ControlPath /tmp/orx-ssh-<uid>-<hash>/<target_hash>
   
   Match all

   • Host & User Matching: OpenSSH strips user@ when matching Host entries, so user@host targets emit Host <host> with an explicit User <user> directive.
   • Port Isolation: Port specifications (-p <port> or :port) are recorded on the Host block so commands intended for default/other ports do not inadvertently attach to a master socket bound to a different port.
   • Scope Isolation: The managed file ends with Match all to prevent trailing host scope from leaking into subsequent directives on older OpenSSH versions (< 8.4).
   • Concurrency Safety: Updates are serialized using an exclusive lockfile (managed_ssh_config.lock).
3. Include Directive: Ensures Include ~/.config/openresearch/ssh_config is in ~/.ssh/config.
   • Global Settings Preservation: The Include directive is inserted immediately before the first Host or Match section (or appended if none exist), preserving any global options specified at the top of ~/.ssh/config.
   • Exact Path Matching: Avoids false positives from generic includes like /etc/ssh/ssh_config by validating canonical and tilde-expanded paths.
   • Symlink Support: Canonicalizes the config path so symlinked dotfile repositories are safely updated in-place without breaking symlinks.
4. Compatibility: Omits ControlPersist for maximum server compatibility, relying directly on orx's active background master process.

Testing

• upsert_managed_host_entry_creates_new_file_and_updates: Verifies new file creation, entry updates, and preserving sibling host blocks.
• upsert_managed_host_entry_handles_user_and_port: Verifies user@host splitting, port extraction, and Match all scoping.
• ensure_ssh_config_include_preserves_global_settings_before_hosts: Verifies that global directives preceding Host blocks remain untouched and global.
• ensure_ssh_config_include_does_not_false_positive_on_other_includes: Verifies that unrelated Include /etc/ssh/ssh_config lines do not block installation.
• ensure_ssh_config_include_supports_symlink: Verifies symlinked ~/.ssh/config files are resolved and written atomically without breaking symlinks.
• ensure_ssh_config_include_in_inserts_and_is_idempotent: Verifies idempotency and missing file creation.
• Verified cargo fmt --all --check, cargo clippy --all-targets -- -D warnings, and full test suite (cargo test --locked).

[sox8502]

@greptileai review

[greptile-apps]

Confidence Score: 4/5

[High risk] Adds SSH configuration file generation on connection.

The PR is not yet safe to merge because a bare-alias entry can still take precedence over a custom-port target’s socket.

Findings

1.  Bare alias socket takes precedence ▶

Fix with agent prompt

### Issue 1
src/jobs/ssh.rs:155-158
When a bare alias and a custom-port target share a hostname, the bare alias’s `Host` block sets `ControlPath` in OpenSSH’s first config pass. The custom-port target sets its path only in `Match final`, which cannot replace that earlier value. Even `ssh -p <custom-port> user@host` can therefore reuse the bare alias’s socket instead of the requested target’s authenticated connection.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR adds a managed OpenSSH config include so external SSH commands can reuse orx connections. The latest changes incorporate the effective port into socket paths and replace stale no-explicit-port entries when a configured port changes.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[orx connects to target] --> B[Create multiplex master socket]
  B --> C[Sync managed host entry]
  C --> D[Include from user SSH config]
  D --> E[External ssh command evaluates socket settings]

Loading

_{Reviews (8) · Last reviewed commit: "fix(ssh): fold effective port into contr..."}

[jyuan-dev]

Thanks for the thorough review! We've addressed all 6 findings in commit 8da4514 / c2c8f20:

1. Unrelated include blocks installation:

   • Fixed in is_matching_include by requiring an exact path match, tilde match (~/...), or filesystem canonical match against managed_ssh_config_path(). Lines like Include /etc/ssh/ssh_config no longer trigger a false positive. Added unit test ensure_ssh_config_include_does_not_false_positive_on_other_includes.

2. Include changes global settings:

   • Fixed include placement in ensure_ssh_config_include_in to insert immediately before the first Host or Match block rather than prepending at line 0, preserving all global options. In addition, the managed config now concludes with Match all to prevent scope leakage on OpenSSH < 8.4. Added unit test ensure_ssh_config_include_preserves_global_settings_before_hosts.

3. User-prefixed hosts never match:

   • Fixed via target_details which strips any user@ prefix to define Host <host> and records User <user> inside the block, matching standard OpenSSH resolution rules. Added unit test upsert_managed_host_entry_handles_user_and_port.

4. Socket ignores destination port:

   • Fixed by extracting ports (-p <port> in extra_opts or :port in destination) in target_details and writing Port <port> into the Host block so external commands only match when connecting to the intended port.

5. Concurrent syncs lose hosts:

   • Fixed by wrapping sync_managed_ssh_host with an exclusive file lock (open_lifecycle_lock_at on managed_ssh_config.lock), serializing parallel session syncs and preventing lost updates.

6. Symlinked config fails silently:

   • Fixed by canonicalizing user_config_path before writing, allowing atomic writes to write directly to the underlying target file without breaking symlinks. Errors from include installation are also logged as warnings rather than swallowed. Added unit test ensure_ssh_config_include_supports_symlink.

7. Cleaned up argument builders:

   • Removed eager disk I/O side effects from forward_args and interactive_args; sync is now performed cleanly during session establishment / preflight.

@greptileai review

[jyuan-dev]

Addressed the remaining 2 findings in commit 2e31de4:

1. Connections overwrite each other:

   • For targets specifying a user and/or port, we now generate scoped Match host <host> [user <user>] [exec "test %p = <port>"] directives rather than bare Host <host> stanzas.
   • Entries are keyed by (host, user, port), allowing multiple live targets on the same host (e.g. root@ssh3.vast.ai:22022 and root@ssh3.vast.ai:22023) to maintain independent ControlPath master sockets without collisions or overwrites.
   • Entries are sorted by specificity descending so OpenSSH evaluates specific match conditions prior to generic host fallbacks.
   • Added unit test upsert_managed_host_entry_preserves_distinct_targets_on_same_host.

2. Tab-separated sections hide include:

   • Replaced naive prefix checks with is_ssh_section_header, which case-insensitively matches Host and Match keywords followed by any whitespace (including tabs \t) or =.
   • Added case- and tab-insensitive matching for existing Include directives.
   • Added unit test ensure_ssh_config_include_handles_tab_separated_sections.

@greptileai review

[jyuan-dev]

Addressed the 3 findings in commit ba593855:

1. Custom port match misses socket:

   • Updated custom-port Match conditions to test exec "test %p = <port> -o %p = 22".
   • When an agent or command runs ssh user@host without an explicit -p, OpenSSH evaluates %p as 22. The stanza matches, configures Port <port>, and reuses the master ControlPath connection.

2. Generic host selects wrong socket:

   • Scoped bare host and alias stanzas with Match host <host> exec "test %p = 22".
   • Unrelated commands specifying a custom port (e.g. ssh -p 22023 user@host) evaluate %p = 22023 and will no longer match or hijack bare alias sockets.

3. Old entries shadow new matches:

   • ManagedHostEntry now tracks structured attributes (host, user, port, socket_path), and upsert_managed_host_entry always re-renders all entries via render_managed_entry.
   • Legacy configs with Host host headers and inner User/Port directives are automatically migrated and normalized to properly scoped Match blocks.

4. Tests:

   • Added upsert_managed_host_entry_migrates_legacy_format_and_prevents_shadowing to verify automatic legacy format migration.
   • Added managed_config_openssh_resolution using ssh -G to verify resolution against the real OpenSSH binary across custom ports, default ports, bare aliases, and unrelated ports.

@greptileai review

[jyuan-dev]

Addressed the 2 findings in commit 7b894a5:

1. Explicit port 22 selects another socket:

   • Replaced single-pass exec "test %p = <port> -o %p = 22" with two-stage resolution.
   • Pass 1 configures default Port <port> (and User <user>).
   • Pass 2 (Match final) tests exec "test %p = <port>" after CLI options and user config have resolved.
   • When dialing without -p, Pass 1 sets Port <port> so Pass 2 matches %p = <port> and reuses the multiplexed socket. When an explicit, conflicting -p 22 (or -p 9999) flag is specified on the CLI, it overrides config Port so Pass 2's %p = <port> check evaluates to false, completely bypassing the custom-port socket.

2. Alias misses its configured port:

   • Bare alias targets (user: None, port: None) are now rendered as standard Host <host> stanzas without %p = 22 restrictions.
   • When an alias like node is configured with a custom port (e.g. Port 22022) in ~/.ssh/config or invoked with ssh -p 22022 node, OpenSSH matches the alias stanza and reuses the active master socket.

3. Tests:

   • Updated managed_config_openssh_resolution to verify both fixes with the real ssh -G binary:
     • ssh -p 22 alice@myhost bypasses custom-port sockets (cp: None, port: 22).
     • ssh node and ssh -p 22022 node match and reuse the master socket (cp: /tmp/socknode).

@greptileai review

[greptile-apps]

Bare alias socket takes precedence

When a bare alias and a custom-port target share a hostname, the bare alias’s Host block sets ControlPath in OpenSSH’s first config pass. The custom-port target sets its path only in Match final, which cannot replace that earlier value. Even ssh -p <custom-port> user@host can therefore reuse the bare alias’s socket instead of the requested target’s authenticated connection.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/jobs/ssh.rs
Line: 151-154

Comment:
**Bare alias socket takes precedence**

When a bare alias and a custom-port target share a hostname, the bare alias’s `Host` block sets `ControlPath` in OpenSSH’s first config pass. The custom-port target sets its path only in `Match final`, which cannot replace that earlier value. Even `ssh -p <custom-port> user@host` can therefore reuse the bare alias’s socket instead of the requested target’s authenticated connection.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[jyuan-dev]

Addressed finding in commit 3309323:

User-only match ignores port:

• For user-scoped targets without an explicit port (e.g. orx up --remote user@host), render_managed_entry now emits two stages:
  • Pass 1 scopes Match host <host> user <user> with User <user>.
  • Pass 2 binds ControlPath in Match final host <host> user <user> exec "test %p = 22".
• Calls to ssh user@host or ssh -p 22 user@host match the default port 22 and reuse the active master socket.
• Commands specifying a different port (e.g. ssh -p 22023 user@host) evaluate %p = 22023, failing the port check and bypassing the master connection to avoid hijacking.
• Updated parse_managed_entries to link Pass 1 and Pass 2 so repeated upserts preserve unconfigured ports without spurious Port 22 insertion, allowing user-configured ports in ~/.ssh/config to take effect unimpeded.
• Added unit tests upsert_managed_host_entry_user_only_does_not_inject_port_and_is_idempotent and extended managed_config_openssh_resolution with OpenSSH verification.

@greptileai review

[jyuan-dev]

Addressed finding in commit 2b66510:

Configured port blocks socket reuse:

• Rather than hardcoding port 22 in Match final ... exec "test %p = 22", resolve_effective_port now inspects the target via ssh -G to determine the target's effective configured port (respecting any Port <port> setting in ~/.ssh/config or system configs, defaulting to 22 if unconfigured).
• render_managed_entry checks exec "test %p = <effective_port>". When invoking ssh user@host, OpenSSH resolves the configured port and matches the active master socket.
• Conflicting CLI port invocations (such as -p 22 or -p 9999 when the configured port is 22022) fail the %p check and bypass the socket, preserving safety against session hijacking.
• ManagedHostEntry tracks both explicit_port (from CLI/target) and effective_port (from config/OpenSSH), ensuring re-upsertion remains idempotent and never injects a spurious Port option into Pass 1.
• Added unit test upsert_managed_host_entry_user_only_respects_configured_port.

@greptileai review

[jyuan-dev]

Addressed finding in commit 95b09aa:

Old port reuses socket:

• Updated control_path(target) to fold in resolve_effective_port(target) into the hash. Targets whose configured ports change between syncs now deterministically resolve to different socket paths, preventing active old-port masters from being attached to by new sessions.
• In upsert_managed_host_entry, retention matching for targets without explicit ports ((None, None)) now evaluates to true (i.e. matches and replaces any existing entry for the same (host, user) without an explicit port). This ensures re-syncing a target when its SSH config port changes cleanly replaces the old entry rather than leaving a stale entry behind.
• Added unit tests control_path_differs_for_configured_ports and upsert_managed_host_entry_replaces_old_entry_when_configured_port_changes.

@greptileai review

[paperclover]

thanks for this, and for pushing on agent connection reuse in #432. the feature landed in main through a different route, and #862 keeps it working on the new run engine: plain ssh, scp and rsync to a host reuse orx's authenticated connection through the managed openresearch_config include, so an agent on an MFA host doesn't get a second prompt. since this patch overlaps that, closing it rather than asking you to rework it.