Repository navigation
ssh: shorter connection reuse, and say why an expired certificate was not renewed #348
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
c1745b3
f9337b2
aba6ceb
9a2723c
b6b054d
a5d5149
87739cc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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,22 @@ function readUserText(path: string): string { | |
| return text | ||
| } | ||
|
|
||
| /** 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 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.` | ||
|
cubic-dev-ai[bot] marked this conversation as resolved.
|
||
| } | ||
| 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,9 +1558,8 @@ 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 | ||
|
|
@@ -1671,8 +1689,11 @@ export async function ensureCertForAlias(alias: string, timeoutMs = RENEWAL_REQU | |
| } finally { | ||
| out.staged.discard() | ||
| } | ||
| } catch { | ||
| // Deliberately swallowed. See above. | ||
| } catch (err) { | ||
| // Swallowed unless this ssh is about to fail on it: then one line says why. | ||
| if (certNeedsRenewal(instaCertPath(alias), { marginMs: 0 })) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: Prompt for AI agents |
||
| process.stderr.write(renewalFailureNotice(alias, err) + '\n') | ||
| } | ||
| } finally { | ||
| release?.() | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: This also turns
EACCESand other I/O failures intoAgentSessionMissing, so an expired-certificate renewal falsely reports that no project session exists. Translate expected missing or invalid-session cases only; let unrelated I/O errors reach the generic retry notice.Prompt for AI agents