Skip to content

fix(host): forward SSH_AUTH_SOCK to runners and harness CLIs - #4377

Merged
dhruv0811 merged 1 commit into
mainfrom
fix/3531-ssh-auth-sock
Aug 7, 2026
Merged

fix(host): forward SSH_AUTH_SOCK to runners and harness CLIs#4377
dhruv0811 merged 1 commit into
mainfrom
fix/3531-ssh-auth-sock

Conversation

@dhruv0811

Copy link
Copy Markdown
Member

Related issue

Closes #3531

Summary

Every runner-spawned context lost the ssh-agent socket, so any agent doing git-over-SSH or SSH-cert-authenticated tooling failed with dial unix: missing address. In certificate-based auth flows this often surfaces as a confusing 401 Unauthorized from the endpoint rather than an SSH error, because such tools hard-fail without the socket and have no cached-token fallback.

ELI5: your ssh-agent is a doorman holding your keys, and SSH_AUTH_SOCK is the intercom number for reaching them. Omnigent was handing agents a blank intercom number, so they'd knock on the door and get turned away, without any hint that the number was the problem.

Two independent gates dropped it:

  • host→runner: _build_runner_env filters the host env through _RUNNER_ENV_ALLOWLIST, which omitted SSH_AUTH_SOCK. This is also the list both host-daemon modes consult, so this one entry fixes the remote-daemon hop too (issue caveat Sync 🌊: upstream catch-up #3).
  • runner→vendor CLI: clean_agent_env is the shared deny-by-default filter, and its safe base omitted it.
host (has SSH_AUTH_SOCK)
  └─ _build_runner_env  ─────────── gate 1: allowlist ......... FIXED
       └─ runner
            ├─ clean_agent_env ──── gate 2: shared safe base ... FIXED  (all 7 harnesses)
            │    └─ vendor CLI (codex/goose/acp/kimi/hermes/qwen/pi)
            ├─ sys_os_shell ─────── mirrors parent env ......... inherits the fix
            └─ terminal pane ────── os.environ.copy() ......... inherits the fix

Classified as a path, not a bearer secret: it names a unix socket, and reaching the agent behind it still requires the user's own ssh-agent to be running and holding the key. Same footing as KUBECONFIG, already allowlisted on exactly that reasoning.

An active OS sandbox deliberately keeps excluding it: that boundary exists to confine the agent, and signing with the user's keys is what it confines. os_env.py previously justified its exclusion by calling the variable "a credential surface masquerading as a path", contradicting the classification above; that rationale is rewritten to rest on the sandbox boundary, so the codebase states one position instead of both.

Two deliberate divergences from the scope proposed in this comment

Gate 2 is fixed in the shared base, not in codex. The proposal was to patch codex's _clean_codex_env. That function is a thin wrapper over clean_agent_env, which is the spawn-env filter for all seven harnesses. Patching only codex would have left the other six broken while looking fixed, so the fix lands one level down. Smaller diff, and it's the actual root cause.

Gate 3 (shell_environment_policy.inherit="all") is intentionally NOT added, because its premise does not reproduce. On codex-cli 0.144.3 the default already passes SSH_AUTH_SOCK through; only an explicit inherit="core" drops it. Measured with a clean CODEX_HOME to rule out local config:

inherit SSH_AUTH_SOCK present total vars
(default, unset) yes 72
all yes 72
core no
none no

Since default and all are identical, forcing all would buy nothing for the common case, and for the one user who did set inherit = "core" it would silently override a deliberate narrowing of their own env. Omnigent sets no shell_environment_policy anywhere and copies the user's config.toml, so that setting stays the user's call. If someone reproduces a drop on a codex version where the default differs, that's worth a follow-up issue rather than a pre-emptive override here.

Test Plan

pytest tests/test_agent_spawn_env_canary.py \
       tests/host/test_connect.py::test_build_runner_env_allowlists_host_env_and_strips_secrets
  • New test_real_builders_pass_ssh_auth_sock drives all seven harnesses' real spawn-env builders, so a future harness that forgets to filter correctly fails here. Sits beside the existing NODE_EXTRA_CA_CERTS canary, which guards the same "must survive filtering" property.
  • Extended the existing host→runner allowlist test with the socket assertion.
  • Suites run green: test_agent_spawn_env_canary.py (38), test_connect.py, test_os_env*.py, plus goose/kimi/hermes/acp/pi executor suites (354).
  • Wrote a scratch repro driving all three env builders with a planted host env: both gates print DROPPED before the change and the socket path after. Confirmed sys_os_shell was innocent (passes through both before and after), matching the issue's analysis.
  • Verified in code that sys_terminal_launch is a plain os.environ.copy() with no name filtering, so it inherits gate 1 rather than needing its own entry.

Two pre-existing failures on this branch, both reproduced on stashed clean main and unrelated to this change: test_connect.py::test_run_host_process_announces_session_log_dir_on_start and test_codex_hooks_generation.py::test_router_hook_survives_a_shadowing_workspace.

Demo

N/A (no user-visible UI; the change is an env-propagation fix).

Type of change

  • Bug fix
  • Feature
  • UI / frontend change
  • Refactor / chore
  • Docs
  • Test / CI
  • Breaking change

Test coverage

  • Unit tests added / updated
  • Integration tests added / updated
  • E2E tests added / updated
  • Manual verification completed
  • Existing tests cover this change
  • Not applicable

Coverage notes

Automated coverage pins both gates: the per-harness canary covers gate 2 for all seven harnesses, and the allowlist test covers gate 1.

Manual verification was the codex inherit matrix above (real codex sandbox invocations against a clean CODEX_HOME) plus the scratch before/after repro of the two gates. The codex matrix is deliberately not a test: it asserts third-party CLI default behaviour that we don't control and would turn into a false alarm on a vendor version bump.

Not covered automatically: a real end-to-end ssh -T git@github.com through a live ssh-agent inside a runner-spawned pane, which needs a real agent and network. Reviewers wanting that check can start omnigent host without the OMNIGENT_RUNNER_ENV_PASSTHROUGH=SSH_AUTH_SOCK workaround and run echo $SSH_AUTH_SOCK && ssh -T git@github.com in both sys_os_shell and a sys_terminal_launch pane.

Changelog

Agents now inherit your ssh-agent, so git-over-SSH and SSH-cert-authenticated tooling work in agent shells and terminals

@github-actions github-actions Bot added P2-medium Priority: bug with workaround, important feature request size/S Pull request size: S labels Aug 7, 2026
Every runner-spawned context lost the ssh-agent socket, so any agent doing
git-over-SSH or SSH-cert-authenticated tooling failed with "dial unix:
missing address" (often surfacing as a confusing 401 from the endpoint,
since such tools have no cached-token fallback).

Two independent gates dropped it:

- `_build_runner_env` filters the host env through `_RUNNER_ENV_ALLOWLIST`,
  which omitted SSH_AUTH_SOCK. This is also the list both host-daemon modes
  consult, so the one entry fixes the daemon hop too, including remote mode.
- `clean_agent_env` is the shared deny-by-default filter for every vendor
  CLI, and its safe base omitted it. Fixing the shared base covers all
  seven harnesses rather than only the one whose report surfaced this.

Classified as a path, not a bearer secret: it names a unix socket, and
reaching the agent behind it still requires the user's own ssh-agent to be
running and holding the key. Same footing as KUBECONFIG, already allowlisted.

An ACTIVE OS sandbox deliberately keeps excluding it: that boundary exists
to confine the agent, and signing with the user's keys is what it confines.
`os_env.py` previously justified its exclusion by calling the variable "a
credential surface masquerading as a path", which contradicts the
classification above; that rationale is rewritten to rest on the sandbox
boundary instead, so the codebase states one position.

Downstream paths needed no change: `sys_os_shell` (sandbox inactive) and
`sys_terminal_launch` both mirror the parent env, so they inherit the fix.

Codex's `shell_environment_policy.inherit` was reported as a third gate
requiring omnigent to force `inherit="all"`. It does not reproduce: on
codex-cli 0.144.3 the default already passes SSH_AUTH_SOCK through
(identical 72-var env), and only an explicit `inherit="core"` drops it.
Forcing `all` would override that deliberate user choice, so no override
is added.

Co-authored-by: Isaac
Signed-off-by: Dhruv Gupta <dhruv.gupta@databricks.com>
@omnigent-ci

omnigent-ci Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Polly AI Review

Blocking issues

None. The change is small, correct, and verified: applying the diff to main and running the cited tests yields 39 passed. SSH_AUTH_SOCK is added to exactly the two filter surfaces the description identifies — _RUNNER_ENV_ALLOWLIST in host/connect.py (gate 1, also the remote-daemon path) and BASE_ALLOW_EXACT in inner/agent_env.py (gate 2, shared by all seven harnesses). The new test_real_builders_pass_ssh_auth_sock drives all seven real spawn-env builders via SPAWN_ENV_BUILDERS, and test_every_harness_is_covered_by_a_real_builder keeps that set exhaustive, so a future harness that forgets to filter correctly fails here.

Security vulnerabilities

No vulnerability, but this deliberately widens a capability boundary and deserves an explicit note. Forwarding SSH_AUTH_SOCK grants every runner-spawned context (sys_os_shell, terminal panes, coding sub-agents) the ability to ask the user's live ssh-agent to sign arbitrary challenges — i.e. authenticate as the user to any host/repo the agent's loaded keys reach. That is a real, agent-usable capability, not merely a path.

The classification is nonetheless sound and internally consistent:

  • It rests on the same reasoning already applied to KUBECONFIG (a path whose backing resource still requires a running local helper), so the codebase is not adopting a new, looser policy — it's extending an existing one.
  • The change correctly does not touch the active OS-sandbox default in os_env.py: SSH_AUTH_SOCK remains absent from _DEFAULT_ENV_PASSTHROUGH and its prefixes, so a confined agent still doesn't get it unless a spec opts in. Only the justifying comment was rewritten, resolving the prior contradiction (the file previously called it "a credential surface masquerading as a path" while the allowlists now treat it as a path).
  • No secret value is introduced or logged; only the socket-path variable name is added to allow-lists.

Net: the security posture is a considered, documented trade-off consistent with prior decisions — acceptable.

Non-blocking notes

  • The rewritten os_env.py comment says an opting-in sandbox spec should "grant the socket path too." That's advisory prose only — no code enables an automatic grant, and env_passthrough already accepts arbitrary names — but a reader could misread it as describing behavior the diff adds. Consider phrasing it as guidance ("if you opt in via env_passthrough, include SSH_AUTH_SOCK") to avoid implying an automatic sandbox grant.
  • No behavior differs when SSH_AUTH_SOCK is unset (the allowlist/base-exact filters simply skip absent keys), so there's no regression risk for users without an ssh-agent — the canary test only asserts the passthrough when present, which is the correct scope.

Summary

A tight, well-reasoned fix: two omitted allowlist entries were the root cause, the fix lands at the shared filter (all seven harnesses) rather than patching one harness, and the sandbox boundary is intentionally left untouched. Tests are meaningful and pass. The one substantive judgment call — forwarding the ssh-agent socket to runner contexts — is a deliberate boundary widening, but it's consistent with the existing KUBECONFIG policy and correctly excluded from the active sandbox. No blocking issues; ready to merge after considering the minor comment-wording nit. No visual demonstration is needed (backend env-forwarding fix with no user-visible surface).


Automated review by Polly · workflow run

@dhruv0811
dhruv0811 enabled auto-merge (squash) August 7, 2026 20:20
@dhruv0811
dhruv0811 force-pushed the fix/3531-ssh-auth-sock branch from 781db8b to bc3c5f6 Compare August 7, 2026 20:22
@dhruv0811
dhruv0811 merged commit 43762a9 into main Aug 7, 2026
91 of 93 checks passed
@dhruv0811
dhruv0811 deleted the fix/3531-ssh-auth-sock branch August 7, 2026 20:52
@github-actions github-actions Bot added the no-doc-update Merged PR does not need a docs update label Aug 7, 2026
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

🏷️ Doc impact: no-doc-update

Adds SSH_AUTH_SOCK to internal env-propagation allowlists so ssh-agent auth survives runner/harness/sandbox boundaries — an internal fix that doesn't change any documented user-facing surface or default.

Auto-classified on merge. Set the label manually before merging to override. · run

Mortified2896 pushed a commit to Mortified2896/omnigent that referenced this pull request Aug 8, 2026
Forward-port PR #82 onto the upstream-0.9 reconcile base (64216aa)
preserving all newer upstream changes including:

- SSH_AUTH_SOCK in _RUNNER_ENV_ALLOWLIST (omnigent-ai#4377)
- workspace cwd in _spawn_runner_proc (#3419de8d)
- codex spec= parameter in resolve_native_codex_launch (omnigent-ai#4208)
- runner event rejection as failed/ERROR (omnigent-ai#4354)
- pi-native inline family from selected model's family (omnigent-ai#4348)

The picker semantics from PR #82 are preserved:
- pi-native supports host.model_options (prelaunch model picker)
- pi_native_provider_launch gets an explicit selection arg so the
  rendered provider matches the user's pick and a bad selection cannot
  silently fall back to an unrelated Pi login
- inline picker renders in the new-session composer before opening the
  gear modal; both share the same model state
- harness changes (Pi/Codex/Claude) change the catalog
- Default omits model_override; explicit pick persists it
- provider/catalog failures surface status='failed' rather than
  fabricating a default

Tests:
- 76 pi-native credentials tests pass
- 40 pi-native extension tests pass
- 58 native-terminal autocreate tests pass
- 57 web flow tests pass (NewChatDialog)
- All 3 pre-existing host connect environment failures unchanged
  (Path.home() tilde expansion issues) - not regressions
Mortified2896 pushed a commit to Mortified2896/omnigent that referenced this pull request Aug 8, 2026
Forward-port PR #82 onto the upstream-0.9 reconcile base (64216aa)
preserving all newer upstream changes including:

- SSH_AUTH_SOCK in _RUNNER_ENV_ALLOWLIST (omnigent-ai#4377)
- workspace cwd in _spawn_runner_proc (#3419de8d)
- codex spec= parameter in resolve_native_codex_launch (omnigent-ai#4208)
- runner event rejection as failed/ERROR (omnigent-ai#4354)
- pi-native inline family from selected model's family (omnigent-ai#4348)

The picker semantics from PR #82 are preserved:
- pi-native supports host.model_options (prelaunch model picker)
- pi_native_provider_launch gets an explicit selection arg so the
  rendered provider matches the user's pick and a bad selection cannot
  silently fall back to an unrelated Pi login
- inline picker renders in the new-session composer before opening the
  gear modal; both share the same model state
- harness changes (Pi/Codex/Claude) change the catalog
- Default omits model_override; explicit pick persists it
- provider/catalog failures surface status='failed' rather than
  fabricating a default

Tests:
- 76 pi-native credentials tests pass
- 40 pi-native extension tests pass
- 58 native-terminal autocreate tests pass
- 57 web flow tests pass (NewChatDialog)
- All 3 pre-existing host connect environment failures unchanged
  (Path.home() tilde expansion issues) - not regressions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-doc-update Merged PR does not need a docs update P2-medium Priority: bug with workaround, important feature request size/S Pull request size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SSH_AUTH_SOCK dropped at the host→runner env boundary, breaking ssh-agent auth in every runner-spawned context

1 participant