Skip to content

fix(ssh): agent signing failures no longer trigger password prompts - #23455

Closed
juliusmarminge wants to merge 6 commits into
mainfrom
fix/issue-23413
Closed

juliusmarminge wants to merge 6 commits into
mainfrom
fix/issue-23413

Conversation

@juliusmarminge

Copy link
Copy Markdown
Member

Problem

When a locked SSH agent cannot sign, each desktop reconnect opens an SSH password prompt even though unlocking the agent is needed. A local OpenSSH integration fixture reproduces this on both password-enabled and key-only hosts: two connection attempts produce two password requests.

Change

Return the original SSH error when OpenSSH reports an agent signing failure, before entering password fallback. Keep existing password, keyboard-interactive, and encrypted-key passphrase fallback unchanged.

Scope and approval

Fixes #23413

Triaged bug via the automated issue flow. The production change is a four-line signing-error guard in the shared SSH package; no renderer, contract, setting, or reconnect-policy changes.

Verification

  • vp test run packages/ssh/src/auth.test.ts packages/ssh/src/tunnel.test.ts: 16 passed at fa4f43f61f. The same final test fixture against base production has 3 failures and 13 passes, specifically the signing-error prompt assertions: red output, green output.
  • vp run check:changed passes: formatting, lint, unused-code checks, typechecks for four affected packages, and the two focused test files (16 tests). Typechecks report existing suggestions on unchanged code.
  • Linux x86_64, OpenSSH 9.6p1: real SshEnvironmentManager, real SSH client/local sshd, and a protocol-agent fixture advertising an accepted ED25519 key. Each row runs two connection attempts under identical conditions. After the fix, both failures retain the original SshCommandError, exit 255, and byte-identical stderr. Validated assertions.
Runtime fixture Before: 4040867bb4 After: fa4f43f61f
Agent refuses signing; password-enabled host 2 prompts 0 prompts; signing error preserved
Agent disconnects during signing; password-enabled host 2 prompts 0 prompts; communication error preserved
Agent refuses signing; key-only host 2 prompts 0 prompts; signing error preserved

The tests also retain prompts for identity-listing failures, ordinary password/keyboard-interactive denials, and standalone publickey denials. A real encrypted-key SSH control confirms that a key-only host can require a passphrase through askpass.

Evidence index and reproducible fixture scripts. The visual matrix is empty because no render paths changed; these are service transcripts rather than desktop captures. Independent proof audit: sufficient for all six operational rows.

Unverified: macOS/Electron IPC and dialog presentation, real 1Password/Secretive lock/unlock and recovery, Windows, and successful manager-mediated passphrase retry. cannot-verify desktop.

Models: Claude Opus 5.5 and GPT-6.1 Sol; harnesses: Claude and Codex in T3 Code.

juliusmarminge and others added 6 commits October 22, 2026 07:15
…sword

Reproduces #23413.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…password

A failed ssh-agent signature or a Permission denied list without password or
keyboard-interactive was treated as a password-recoverable failure, so every
connection attempt opened a password prompt that could not help and hid the
real agent error. Only offer the prompt when the server accepts a password
method and the agent did not fail.

Fixes #23413

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Matching any "communication with agent failed" text also caught
"error fetching identities", where no key was offered and a password may
still work. Narrow the check to ssh's sign_and_send_pubkey ... from agent
diagnostic, and keep the prompt for the identities-fetch case.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-only servers

Only an ssh-agent signing failure suppresses the password prompt; other
auth failures, including publickey-only denials, retry as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. label Oct 22, 2026
@juliusmarminge juliusmarminge added bugfix Fix PR opened by an issue lead; enters the PR loop and removed vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. labels Oct 22, 2026 — with GitCafe Production
@github-actions github-actions Bot added the size:XS 0-9 changed lines (additions + deletions). label Oct 22, 2026
@juliusmarminge juliusmarminge added the pr:needs-julius Needs Julius: route gate, unverifiable surface, or blocked label Oct 22, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This targeted fix changes the existing SSH authentication fallback by suppressing password prompts after agent signing failures and preserving the original error, with focused regression coverage. Human review is required because the production change is in authentication-sensitive SSH code.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between 96bae54 and fa4f43f.

📒 Files selected for processing: 2
  • packages/ssh/src/auth.ts
  • packages/ssh/src/tunnel.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

📝 Walkthrough

The SSH authentication classifier now excludes agent signing failures from authentication-failure classification. Parameterized tests check the resulting password-prompt behavior alongside agent communication failures and authentication denials.

Changes: SSH authentication

Layer / File(s) Summary
Agent signing failure classification and prompt tests
packages/ssh/src/auth.ts, packages/ssh/src/tunnel.test.ts
isSshAuthFailure excludes normalized agent signing failure messages matching the added pattern. Tests verify environment failure and password-prompt counts for agent errors and authentication denials. No-prompt cases also check the SshCommandError and original stderr.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: t3dotgg

Merge Risk: ⚪ Minimal · up to fa4f4

The change appears ready to merge after normal checks; no actionable issue was identified in the supplied evidence.

Pre-merge checks: Passed 4
Check name Status Explanation
Description check Passed The description covers the problem, change, scope, linked issue, focused verification, observed results, and unverified environments. It does not include explicit maintainer approval, but the linked t…
Title check Passed The title is concise, specific, and accurately describes the main change: agent signing failures no longer trigger SSH password prompts.
Linked Issues check Passed The PR addresses issue #23413. isSshAuthFailure now excludes OpenSSH sign_and_send_pubkey: signing failed ... from agent: errors. The tunnel therefore returns the original SshCommandError and do…
Out of Scope Changes check Passed The reported changes are limited to the shared SSH authentication guard and focused tunnel tests. The tests verify the issue behavior and protect existing fallback behavior. No unrelated renderer, con…
Full details: Description check

Explanation

The description covers the problem, change, scope, linked issue, focused verification, observed results, and unverified environments. It does not include explicit maintainer approval, but the linked triaged issue and detailed scope explanation provide sufficient context for this focused bug fix.

Full details: Linked Issues check

Explanation

The PR addresses issue #23413. isSshAuthFailure now excludes OpenSSH sign_and_send_pubkey: signing failed ... from agent: errors. The tunnel therefore returns the original SshCommandError and does not request a password. Added tests cover signing failures, communication failures, key-only hosts, and retained prompts for identity-listing, publickey, password, and keyboard-interactive failures. The change leaves normal agent success and existing fallback paths unchanged.

Full details: Out of Scope Changes check

Explanation

The reported changes are limited to the shared SSH authentication guard and focused tunnel tests. The tests verify the issue behavior and protect existing fallback behavior. No unrelated renderer, contract, setting, reconnect-policy, or production subsystem changes are reported.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB −7 B (−0.1%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB −7 B (−0.6%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.9 KiB 20.9 KiB 0 B (0.0%) 29.3 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB +7 B (+0.1%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB +7 B (+0.6%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB 0 B (0.0%) 29.3 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: 96bae54 · PR result: fa4f43f · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Copy link
Copy Markdown
Member Author

Closing in favour of #23437, which makes the same change.

@juliusmarminge juliusmarminge removed the pr:needs-julius Needs Julius: route gate, unverifiable surface, or blocked label Oct 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Fix PR opened by an issue lead; enters the PR loop size:XS 0-9 changed lines (additions + deletions).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Locked screen makes T3 ask for an SSH password every few minutes when the SSH agent can't sign

1 participant