ssh: shorter connection reuse, and say why an expired certificate was not renewed

src/agent.ts

@@ -23,6 +23,9 @@ export function canonicalTarget(path: string): string {
 }
 const guidance = 'agent session missing, expired, or for another project/environment — run `insta agent setup`'
 
+/** No usable agent session for the project, looked up from the current directory. */
+export class AgentSessionMissing extends Error {}
+
 export async function issueAgentSession(api: SessionApi, projectId?: string): Promise<Session> {
   const pair = generateKeyPairSync('ed25519')
   const client = mode?.client ?? detectAgent(false)?.client ?? 'unknown'
@@ -62,7 +65,7 @@ export async function loadAgentSession(apiUrl: string, projectId: string, cwd =
     if (session.projectId !== projectId || session.apiUrl !== apiUrl.replace(/\/+$/, '') || !session.token || !session.privateKey
       || !Number.isFinite(Date.parse(session.expiresAt)) || Date.parse(session.expiresAt) <= Date.now()) throw new Error()
     return session
-  } catch { throw new Error(guidance) }
+  } catch { throw new AgentSessionMissing(guidance) }
 }
 
 // Which project a request is scoped to, when the path itself does not say. Some project-owned

src/commands/compute.ts

@@ -1,4 +1,6 @@
-import { ApiClient, ApiError, requireProject } from '../api.js'
+import { AgentApprovalRequired, ApiClient, ApiError, requireProject } from '../api.js'
+import { AgentSessionMissing } from '../agent.js'
+import { safeText } from '../config.js'
 import { info, printJson, handleApproval, relayExitCode, writeFileAtomicSync, resolveThroughSymlink } from '../util.js'
 import { resolveComputeServiceId, resolveSoleService, q, parseVolumeGib, parseCount } from './services.js'
 
@@ -824,7 +826,7 @@ import { dirname, join } from 'node:path'
 import { execFileSync } from 'node:child_process'
 import { createHash, randomUUID } from 'node:crypto'
 import {
-  aliasFor, certifiesPublicKey, hasOwnedBlock, isSafeAlias, isSafeConfigValue, isSafeSSHHost, isSafeSSHUsername, isSafeTimestamp, isSSHCertificateRecord, mayWidenCAHost, parseCAPublicKey, planCertAuthority, renderConfigBlock, revertCertAuthority, upsertConfigBlock, type HostEntry, ownedBlock, ownedBlockIsFirst } from './ssh-config.js'
+  ALIAS_SUFFIX, aliasFor, certifiesPublicKey, hasOwnedBlock, isSafeAlias, isSafeConfigValue, isSafeSSHHost, isSafeSSHUsername, isSafeTimestamp, isSSHCertificateRecord, mayWidenCAHost, parseCAPublicKey, planCertAuthority, renderConfigBlock, revertCertAuthority, upsertConfigBlock, type HostEntry, ownedBlock, ownedBlockIsFirst } from './ssh-config.js'
 
 /** Where this CLI keeps its own SSH material. Deliberately NOT ~/.ssh: we never
  *  touch a key the user already had, and a dedicated key pairs with
@@ -1068,6 +1070,7 @@ type MintedCert = CertResponse & { staged: StagedCertificate }
 
 async function mintCert(api: ApiClient, projectId: string, serviceId: string, publicKey: string, alias: string, signal?: AbortSignal): Promise<MintedCert> {
   const res = await api.rawRequest('POST', `/projects/${projectId}/services/${serviceId}/ssh-cert`, { publicKey }, { signal })
+  if (res.status === 202 && res.body?.status === 'approval_required') throw new AgentApprovalRequired(res.body)
   if (res.status < 200 || res.status >= 300) {
     throw new ApiError(res.status, res.body?.error ?? 'could not issue an ssh certificate')
   }
@@ -1522,6 +1525,28 @@ function readUserText(path: string): string {
   return text
 }
 
+/** Another process holds this alias's renewal lock and is renewing it now. */
+class RenewalInProgress extends Error {}
+
+/** The one line a missing or expired certificate that could not be renewed gets: the cause and the fix. */
+export function renewalFailureNotice(alias: string, err: unknown): string {
+  const head = `insta: the SSH certificate for ${alias} is missing or expired`
+  const fix = `insta compute ssh ${alias.slice(0, -ALIAS_SUFFIX.length)}`
+  if (err instanceof RenewalInProgress) {
+    return `${head} and another ssh is renewing it right now. Retry in a moment.`
+  }
+  if (err instanceof AgentSessionMissing) {
+    return `${head}. Agent mode found no agent session for its project in this directory. Run ssh from the project directory, or ask a person to run "${fix}".`
+  }
+  if (err instanceof AgentApprovalRequired) {
+    return `${head} and renewing it needs approval. ${safeText(err.message.replace(/\s+/g, ' ')).trim()} Retry after it is approved.`
+  }
+  if (err instanceof ApiError && err.status === 403 && err.message.endsWith('denied by agent policy')) {
+    return `${head} and this project's agent policy does not allow renewing it.`
+  }
+  return `${head} and could not be renewed. Run "${fix}" to renew it.`
+}
+
 /**
  * The renewal hook OpenSSH runs while PARSING the config, before it connects.
  *
@@ -1539,12 +1564,13 @@ function readUserText(path: string): string {
  *    rule __update-check relies on), so the fast path is genuinely local only
  *    when the hook enters through that name.
  *  - Silent and fail-safe. An unlinked directory, an expired login or a network
- *    outage must not print anything or fail the parse: the existing certificate
- *    stays in place and the login then fails with SSH's own message, not a CLI
- *    error spliced into the middle of an ssh session.
+ *    outage must not fail the parse: the existing certificate stays in place.
+ *    Only an already expired one earns a single stderr line saying why.
  */
 export async function ensureCertForAlias(alias: string, timeoutMs = RENEWAL_REQUEST_TIMEOUT_MS): Promise<void> {
   let release: (() => void) | undefined
+  let attempted = false
+  let cause: unknown
   try {
     if (!isSafeAlias(alias)) return
     // Repair BEFORE the renewal gate, because the stanza can lag the store
@@ -1561,6 +1587,8 @@ export async function ensureCertForAlias(alias: string, timeoutMs = RENEWAL_REQU
       }, { waitMs: KNOWN_HOSTS_LOCK_WAIT_MS })
     }
     if (!certNeedsRenewal(instaCertPath(alias))) return
+    if (!readAliasStore()[alias]) return // not an alias this CLI manages: stay silent
+    attempted = true
 
     // An IDE opens several connections at once and `scp` adds more, so the
     // near-expiry certificate is observed by every one of them simultaneously
@@ -1572,7 +1600,10 @@ export async function ensureCertForAlias(alias: string, timeoutMs = RENEWAL_REQU
     // a lock that can hang `ssh` itself -- strictly worse than the duplicate
     // request it would prevent. The loser simply lets the winner renew.
     release = acquireRenewalLock(alias)
-    if (!release) return
+    if (!release) {
+      cause = new RenewalInProgress()
+      return
+    }
     // Re-checked after the lock. Without this the second process through the
     // door renews again over the certificate the first just wrote -- the lock
     // would serialise the stampede instead of collapsing it.
@@ -1671,10 +1702,14 @@ export async function ensureCertForAlias(alias: string, timeoutMs = RENEWAL_REQU
     } finally {
       out.staged.discard()
     }
-  } catch {
-    // Deliberately swallowed. See above.
+  } catch (err) {
+    cause = err
   } finally {
     release?.()
+    // Silent unless this ssh is about to fail: any attempt that left the certificate expired says why.
+    if (attempted && certNeedsRenewal(instaCertPath(alias), { marginMs: 0 })) {
+      process.stderr.write(renewalFailureNotice(alias, cause) + '\n')
+    }
   }
 }
 

src/commands/ssh-config.ts

@@ -186,9 +186,7 @@ export function renderConfigBlock(o: ConfigBlockOpts): string {
       // full ssh-agent gets intermittent, unexplainable auth failures.
       '  IdentitiesOnly yes',
     )
-    // Connection multiplexing collapses scp, an IDE's several connections and a
-    // second terminal onto ONE connection; without it a single developer can
-    // reach the per-service session cap in an afternoon.
+    // Multiplexing spares a handshake per ssh, and 60s keeps a master from outliving a pod that scaled to zero.
     //
     // The socket is keyed on %C -- a hash of (local host, remote host, port,
     // user) -- not %r@%h:%p. A ControlPath is a Unix-domain socket, whose path
@@ -208,7 +206,7 @@ export function renderConfigBlock(o: ConfigBlockOpts): string {
       lines.push(
         '  ControlMaster auto',
         '  ControlPath ~/.insta/ssh/cm-%C',
-        '  ControlPersist 10m',
+        '  ControlPersist 60s',
       )
     }
     if (o.ensureCertCommand) {

src/config.ts

@@ -299,7 +299,7 @@ async function readLinkPlane(root: string): Promise<{ projectId: string; apiUrl:
 
 /** Text safe to echo to a terminal: C0 and C1 control characters and DEL removed — ESC and the
  *  single-byte C1 introducers (U+009B CSI among them) alike, so no escape sequence survives. */
-function safeText(text: string): string {
+export function safeText(text: string): string {
   return String(text).replace(/[\u0000-\u001f\u007f-\u009f]/g, '')
 }
 

test/ssh-config.test.ts

@@ -477,6 +477,11 @@ describe.skipIf(!ssh || process.platform === 'win32')('effective configuration (
     expect(cp.length, `ControlPath is ${cp.length} bytes (limit 104): ${cp}`).toBeLessThan(104)
   })
 
+  it('drops a multiplexed connection a minute after its last session', () => {
+    // A master outliving the 300s scale-to-zero idle stays pinned to a pod that is gone.
+    expect(effective('api.insta', rendered()).get('controlpersist')).toBe('60')
+  })
+
   it('does not let a % in the path reach the ControlPath tokens', () => {
     // ControlPath carries tokens we MEANT, and `ssh -G` does expand that one --
     // so it is the check that the escaping did not spill outside the two path
@@ -966,13 +971,18 @@ describe('renewal hook is silent and fail-safe', () => {
     await quietly('api.insta')
   })
 
-  // A record exists and the certificate is missing, so this path goes all the
-  // way to the platform -- which is not there. Everything downstream of the
-  // certificate check lives inside the same silent boundary.
-  it('says nothing when the platform cannot be reached', async () => {
+  // No certificate and no platform, so this ssh will fail: the hook says why, on stderr only.
+  it('says only why, in one stderr line, when the platform cannot be reached', async () => {
     writeAliasStore({ 'api.insta': { projectId: 'p1', branch: 'main', serviceId: 's1', host: 'h', username: 'u' } })
     expect(existsSync(instaCertPath('api.insta'))).toBe(false)
-    await quietly('api.insta')
+    const log = vi.spyOn(console, 'log').mockImplementation(() => {})
+    const err = vi.spyOn(console, 'error').mockImplementation(() => {})
+    const out = vi.spyOn(process.stdout, 'write').mockImplementation(() => true)
+    const errOut = vi.spyOn(process.stderr, 'write').mockImplementation(() => true)
+    await expect(computeSSH(undefined, { ensureCert: 'api.insta' })).resolves.toBeUndefined()
+    for (const s of [log, err, out]) expect(s, 'the renewal hook printed into the ssh session').not.toHaveBeenCalled()
+    expect(errOut.mock.calls.map((c) => String(c[0])).join('')).toBe(
+      'insta: the SSH certificate for api.insta is missing or expired and could not be renewed. Run "insta compute ssh api" to renew it.\n')
   })
 
   it('says nothing, and touches nothing, for an alias that is not ours', async () => {
@@ -1288,7 +1298,7 @@ describe('the generated config works on Windows, where multiplexing does not', (
     for (const platform of ['darwin', 'linux'] as const) {
       const out = renderConfigBlock({ entries: [entry()], identityFile: '/home/dev/.insta/ssh/id_ed25519', knownHostsFile: KNOWN_HOSTS, platform })
       expect(out, `${platform} lost connection multiplexing`).toContain('ControlMaster auto')
-      expect(out).toContain('ControlPersist 10m')
+      expect(out).toContain('ControlPersist 60s')
       // Keyed on %C, never %r@%h:%p: a Unix-domain socket path caps at 104 bytes
       // and the route-key user plus the gateway host overflowed it.
       expect(out, `${platform} ControlPath must be the fixed-length %C form`).toContain('ControlPath ~/.insta/ssh/cm-%C')