From 5c8bc923e9660dfd35666c792cb30278f99c941f Mon Sep 17 00:00:00 2001 From: bincxz <16399091+binaricat@users.noreply.github.com> Date: Mon, 11 May 2026 16:01:18 +0800 Subject: [PATCH] fix(ssh): per-host keepalive for SFTP + port forwarding too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the maintainer review on PR #947 — terminal SSH was the only path that honored per-host keepalive overrides. SFTP and port forwarding share the same NetcattyJumpHost type but their builders weren't resolving keepalive per-hop, and their bridges hardcoded the old 10s/3 defaults. Net result: a router-as-bastion in a chain still got killed when reached via the SFTP file panel or a port-forwarding tunnel, even though the user had toggled per-host override. Plumbing: - useSftpHostCredentials / buildSftpHostCredentials: accept optional terminalSettings; call resolveHostKeepalive() for the target and each jump entry; emit keepaliveInterval / keepaliveCountMax in the returned NetcattySSHOptions. - useSftpConnections + useSftpState + SftpStateOptions thread the setting down. SftpSidePanel passes the global terminalSettings prop it already has from TerminalLayer. - portForwardingService.startPortForward: accepts terminalSettings as an 8th argument, resolves per-host (target + each jump), and populates the bridge payload. - usePortForwardingState.startTunnel and usePortForwardingAutoStart forward the new parameter; App.tsx supplies terminalSettings (via a ref in the once-on-launch auto-start effect so changing global keepalive later doesn't re-fire it). Bridges: - sftpBridge.cjs target connect: now also reads keepaliveCountMax from options (was hardcoded 3). 10s/3 stays as the bridge-level fallback to preserve the #669 protection when the renderer hasn't supplied a value. - sftpBridge.cjs jump hop: reads jump.keepaliveInterval / jump.keepaliveCountMax, then falls back to the target-call options (matches the symmetric SSH bridge change). - portForwardingBridge.cjs: reads keepaliveInterval / keepaliveCountMax from the IPC payload; same 10s/3 fallback. Types: - NetcattyJumpHost already grew keepalive fields earlier; this commit also adds them to PortForwardOptions so the IPC contract is explicit. End-to-end: a chain `[router-as-bastion, cloud-host]` with the router host's keepaliveOverride=true / interval=0 now correctly disables keepalive on the router hop for terminal SSH AND SFTP AND port forwarding, while the cloud target still gets the resolved 30s/10 default for each path. Co-Authored-By: Claude Opus 4.7 (1M context) --- App.tsx | 3 ++- application/state/sftp/types.ts | 6 +++++ application/state/sftp/useSftpConnections.ts | 4 +++- .../state/sftp/useSftpHostCredentials.ts | 22 +++++++++++++++--- .../state/usePortForwardingAutoStart.ts | 10 +++++++- application/state/usePortForwardingState.ts | 3 ++- application/state/useSftpState.ts | 1 + components/SftpSidePanel.tsx | 5 +++- components/TerminalLayer.tsx | 1 + electron/bridges/portForwardingBridge.cjs | 9 +++++++- electron/bridges/sftpBridge.cjs | 23 +++++++++++++++---- global.d.ts | 4 ++++ .../services/portForwardingService.ts | 16 +++++++++++-- 13 files changed, 91 insertions(+), 16 deletions(-) diff --git a/App.tsx b/App.tsx index 56a555627..40c0f2d3d 100755 --- a/App.tsx +++ b/App.tsx @@ -629,7 +629,7 @@ function App({ settings }: { settings: SettingsState }) { const effectiveHost = resolveEffectiveHost(host); void startTunnel(rule, effectiveHost, hosts.map(resolveEffectiveHost), keys, identities, (status, error) => { if (status === "error" && error) toast.error(error); - }, rule.autoStart); + }, rule.autoStart, terminalSettings); return; } @@ -836,6 +836,7 @@ function App({ settings }: { settings: SettingsState }) { identities, proxyProfiles, groupConfigs, + terminalSettings, }); // Sync tray menu data + handle tray actions diff --git a/application/state/sftp/types.ts b/application/state/sftp/types.ts index 170be7360..8f52bfc96 100644 --- a/application/state/sftp/types.ts +++ b/application/state/sftp/types.ts @@ -64,4 +64,10 @@ export interface SftpStateOptions { useCompressedUpload?: boolean; defaultShowHiddenFiles?: boolean; autoConnectLocalOnMount?: boolean; + /** + * Global SSH keepalive settings, forwarded through to per-SFTP-connection + * keepalive resolution so a host that has opted into its own override + * is honored for SFTP browsing too (not just the terminal session). + */ + terminalSettings?: { keepaliveInterval: number; keepaliveCountMax: number }; } diff --git a/application/state/sftp/useSftpConnections.ts b/application/state/sftp/useSftpConnections.ts index b8f109094..9047532b5 100644 --- a/application/state/sftp/useSftpConnections.ts +++ b/application/state/sftp/useSftpConnections.ts @@ -11,6 +11,7 @@ interface UseSftpConnectionsParams { hosts: Host[]; keys: SSHKey[]; identities: Identity[]; + terminalSettings?: { keepaliveInterval: number; keepaliveCountMax: number }; leftTabsRef: MutableRefObject<{ tabs: SftpPane[]; activeTabId: string | null }>; rightTabsRef: MutableRefObject<{ tabs: SftpPane[]; activeTabId: string | null }>; leftTabs: { tabs: SftpPane[] }; @@ -44,6 +45,7 @@ export const useSftpConnections = ({ hosts, keys, identities, + terminalSettings, leftTabsRef, rightTabsRef, leftTabs, @@ -65,7 +67,7 @@ export const useSftpConnections = ({ createEmptyPane, autoConnectLocalOnMount = true, }: UseSftpConnectionsParams): UseSftpConnectionsResult => { - const getHostCredentials = useSftpHostCredentials({ hosts, keys, identities }); + const getHostCredentials = useSftpHostCredentials({ hosts, keys, identities, terminalSettings }); const { listLocalFiles, listRemoteFiles } = useSftpDirectoryListing(); const connect = useCallback( diff --git a/application/state/sftp/useSftpHostCredentials.ts b/application/state/sftp/useSftpHostCredentials.ts index d6616c1ad..de1b74cca 100644 --- a/application/state/sftp/useSftpHostCredentials.ts +++ b/application/state/sftp/useSftpHostCredentials.ts @@ -1,12 +1,19 @@ import { useCallback } from "react"; -import type { Host, Identity, SSHKey } from "../../../domain/models"; +import type { Host, Identity, SSHKey, TerminalSettings } from "../../../domain/models"; import { isEncryptedCredentialPlaceholder, sanitizeCredentialValue } from "../../../domain/credentials"; import { resolveBridgeKeyAuth, resolveHostAuth } from "../../../domain/sshAuth"; +import { resolveHostKeepalive } from "../../../domain/host"; + +// Fallback used when no global TerminalSettings are wired through (older +// call sites or tests). Matches DEFAULT_TERMINAL_SETTINGS so behavior is +// identical whether or not the caller passes settings. +const FALLBACK_KEEPALIVE = { keepaliveInterval: 30, keepaliveCountMax: 10 }; interface UseSftpHostCredentialsParams { hosts: Host[]; keys: SSHKey[]; identities: Identity[]; + terminalSettings?: Pick; } export const buildSftpHostCredentials = ({ @@ -14,7 +21,9 @@ export const buildSftpHostCredentials = ({ hosts, keys, identities, + terminalSettings, }: UseSftpHostCredentialsParams & { host: Host }): NetcattySSHOptions => { + const globalKeepalive = terminalSettings ?? FALLBACK_KEEPALIVE; if (host.proxyProfileId && !host.proxyConfig) { throw new Error(`Saved proxy for host "${host.label || host.hostname}" is missing. Open host settings and select a valid proxy.`); } @@ -79,6 +88,7 @@ export const buildSftpHostCredentials = ({ ) { throw new Error(`Saved credentials for jump host "${jumpHost.label || jumpHost.hostname}" cannot be decrypted on this device. Open host settings and re-enter them.`); } + const hopKeepalive = resolveHostKeepalive(jumpHost, globalKeepalive); return { hostname: jumpHost.hostname, port: jumpHost.port || 22, @@ -101,6 +111,8 @@ export const buildSftpHostCredentials = ({ } : undefined, identityFilePaths: jumpKeyAuth.identityFilePaths, + keepaliveInterval: hopKeepalive.interval, + keepaliveCountMax: hopKeepalive.countMax, }; }); } @@ -129,6 +141,7 @@ export const buildSftpHostCredentials = ({ throw new Error("Saved credentials cannot be decrypted on this device. Open host settings and re-enter them."); } + const targetKeepalive = resolveHostKeepalive(host, globalKeepalive); return { hostname: host.hostname, username: resolved.username, @@ -144,6 +157,8 @@ export const buildSftpHostCredentials = ({ jumpHosts: jumpHosts && jumpHosts.length > 0 ? jumpHosts : undefined, sudo: host.sftpSudo, identityFilePaths: keyAuth.identityFilePaths, + keepaliveInterval: targetKeepalive.interval, + keepaliveCountMax: targetKeepalive.countMax, }; }; @@ -151,8 +166,9 @@ export const useSftpHostCredentials = ({ hosts, keys, identities, + terminalSettings, }: UseSftpHostCredentialsParams) => useCallback( - (host: Host): NetcattySSHOptions => buildSftpHostCredentials({ host, hosts, keys, identities }), - [hosts, identities, keys], + (host: Host): NetcattySSHOptions => buildSftpHostCredentials({ host, hosts, keys, identities, terminalSettings }), + [hosts, identities, keys, terminalSettings], ); diff --git a/application/state/usePortForwardingAutoStart.ts b/application/state/usePortForwardingAutoStart.ts index 9ce670a2e..26127ece8 100644 --- a/application/state/usePortForwardingAutoStart.ts +++ b/application/state/usePortForwardingAutoStart.ts @@ -24,6 +24,7 @@ export interface UsePortForwardingAutoStartOptions { identities: Identity[]; proxyProfiles: ProxyProfile[]; groupConfigs: GroupConfig[]; + terminalSettings?: { keepaliveInterval: number; keepaliveCountMax: number }; } const AUTO_START_PROXY_NOT_READY_ERROR = "Proxy or jump host configuration is not ready"; @@ -103,6 +104,7 @@ export const usePortForwardingAutoStart = ({ identities, proxyProfiles, groupConfigs, + terminalSettings, }: UsePortForwardingAutoStartOptions): void => { const autoStartExecutedRef = useRef(false); const hostsRef = useRef(hosts); @@ -110,6 +112,8 @@ export const usePortForwardingAutoStart = ({ const identitiesRef = useRef(identities); const proxyProfilesRef = useRef(proxyProfiles); const groupConfigsRef = useRef(groupConfigs); + const terminalSettingsRef = useRef(terminalSettings); + terminalSettingsRef.current = terminalSettings; const isHostAuthReady = useCallback((host: Host, seen = new Set()): boolean => { if (!host || seen.has(host.id)) return true; @@ -238,7 +242,7 @@ export const usePortForwardingAutoStart = ({ } const host = resolveEffectiveHost(rawHost); - return startPortForward(rule, host, resolveEffectiveHosts(hostsRef.current), keysRef.current, identitiesRef.current, onStatusChange, true); + return startPortForward(rule, host, resolveEffectiveHosts(hostsRef.current), keysRef.current, identitiesRef.current, onStatusChange, true, terminalSettingsRef.current); }; setReconnectCallback(handleReconnect); @@ -304,6 +308,10 @@ export const usePortForwardingAutoStart = ({ updateStoredRuleStatus(rule.id, status, error); }, true, // Enable reconnect for auto-start rules + // Read via ref so adjusting global keepalive after launch doesn't + // re-trigger the auto-start effect (its dep array is intentionally + // stable to fire once on vault init). + terminalSettingsRef.current, ); } }; diff --git a/application/state/usePortForwardingState.ts b/application/state/usePortForwardingState.ts index 900734c1b..9c1d55927 100644 --- a/application/state/usePortForwardingState.ts +++ b/application/state/usePortForwardingState.ts @@ -387,11 +387,12 @@ export const usePortForwardingState = (): UsePortForwardingStateResult => { error?: string, ) => void, enableReconnect = false, + terminalSettings?: { keepaliveInterval: number; keepaliveCountMax: number }, ) => { return startPortForward(rule, host, hosts, keys, identities, (status, error) => { setRuleStatus(rule.id, status, error); onStatusChange?.(status, error ?? undefined); - }, enableReconnect); + }, enableReconnect, terminalSettings); }, [setRuleStatus], ); diff --git a/application/state/useSftpState.ts b/application/state/useSftpState.ts index 6ed230edf..4abc8bfa8 100644 --- a/application/state/useSftpState.ts +++ b/application/state/useSftpState.ts @@ -174,6 +174,7 @@ export const useSftpState = ( hosts, keys, identities, + terminalSettings: options?.terminalSettings, leftTabsRef, rightTabsRef, leftTabs, diff --git a/components/SftpSidePanel.tsx b/components/SftpSidePanel.tsx index 8da608700..8864748f3 100644 --- a/components/SftpSidePanel.tsx +++ b/components/SftpSidePanel.tsx @@ -71,6 +71,7 @@ interface SftpSidePanelProps { setEditorWordWrap: (value: boolean) => void; onGetTerminalCwd?: () => Promise; onRequestTerminalFocus?: () => void; + terminalSettings?: { keepaliveInterval: number; keepaliveCountMax: number }; } const SftpSidePanelInner: React.FC = ({ @@ -98,6 +99,7 @@ const SftpSidePanelInner: React.FC = ({ setEditorWordWrap, onGetTerminalCwd, onRequestTerminalFocus, + terminalSettings, }) => { const { t } = useI18n(); const hostWriteSource = writableHosts ?? hosts; @@ -119,7 +121,8 @@ const SftpSidePanelInner: React.FC = ({ useCompressedUpload: sftpUseCompressedUpload, defaultShowHiddenFiles: sftpShowHiddenFiles, autoConnectLocalOnMount: false, - }), [fileWatchHandlers, sftpUseCompressedUpload, sftpShowHiddenFiles]); + terminalSettings, + }), [fileWatchHandlers, sftpUseCompressedUpload, sftpShowHiddenFiles, terminalSettings]); const sftp = useSftpState(hosts, keys, identities, sftpOptions); const { diff --git a/components/TerminalLayer.tsx b/components/TerminalLayer.tsx index 1df7edd80..086a13000 100644 --- a/components/TerminalLayer.tsx +++ b/components/TerminalLayer.tsx @@ -2394,6 +2394,7 @@ const TerminalLayerInner: React.FC = ({ setEditorWordWrap={setEditorWordWrap} onGetTerminalCwd={getTerminalCwd} onRequestTerminalFocus={refocusActiveTerminalSession} + terminalSettings={terminalSettings} /> ); })} diff --git a/electron/bridges/portForwardingBridge.cjs b/electron/bridges/portForwardingBridge.cjs index ef3fb5660..ca7ae8d61 100644 --- a/electron/bridges/portForwardingBridge.cjs +++ b/electron/bridges/portForwardingBridge.cjs @@ -82,6 +82,8 @@ async function startPortForward(event, payload) { jumpHosts = [], identityFilePaths, legacyAlgorithms, + keepaliveInterval: resolvedKeepaliveInterval, + keepaliveCountMax: resolvedKeepaliveCountMax, } = payload; const conn = new SSHClient(); @@ -110,12 +112,17 @@ async function startPortForward(event, payload) { } }; + // Keepalive resolved by the renderer (per-host override applied), with a + // 10s/3 fallback — a forwarded TCP tunnel doesn't generate idle data + // flow, so NAT/firewall state tables drop it within ~30-60s without + // keepalive packets. const connectOpts = { host: hostname, port: port, username: username || 'root', readyTimeout: 120000, // 2 minutes for 2FA input - keepaliveInterval: 10000, + keepaliveInterval: resolvedKeepaliveInterval > 0 ? resolvedKeepaliveInterval * 1000 : 10000, + keepaliveCountMax: resolvedKeepaliveInterval > 0 ? (resolvedKeepaliveCountMax ?? 3) : 3, // Enable keyboard-interactive authentication (required for 2FA/MFA) tryKeyboard: true, algorithms: buildAlgorithms(legacyAlgorithms), diff --git a/electron/bridges/sftpBridge.cjs b/electron/bridges/sftpBridge.cjs index 795783e71..356817a4a 100644 --- a/electron/bridges/sftpBridge.cjs +++ b/electron/bridges/sftpBridge.cjs @@ -923,14 +923,25 @@ async function connectThroughChainForSftp(event, options, jumpHosts, targetHost, // Set to 0 (unlimited) since complex operations add many temp listeners conn.setMaxListeners(0); + // Per-hop keepalive. Each jump entry now carries its own resolved + // values when the renderer supplies them; fall back to the target + // call's options for backward compat with older serializers and + // ultimately to a 10s default so an idle SFTP browse over a NAT + // doesn't drop (the original #669 protection). + const hopInterval = jump.keepaliveInterval != null + ? jump.keepaliveInterval + : options.keepaliveInterval; + const hopCountMax = jump.keepaliveCountMax != null + ? jump.keepaliveCountMax + : (options.keepaliveCountMax ?? 3); // Build connection options const connOpts = { host: jump.hostname, port: jump.port || 22, username: jump.username || 'root', readyTimeout: 120000, // 2 minutes to allow for keyboard-interactive (2FA/MFA) - keepaliveInterval: 10000, - keepaliveCountMax: 3, + keepaliveInterval: hopInterval > 0 ? hopInterval * 1000 : 10000, + keepaliveCountMax: hopInterval > 0 ? hopCountMax : 3, // Enable keyboard-interactive authentication (required for 2FA/MFA) tryKeyboard: true, algorithms: buildSftpAlgorithms(options.legacyAlgorithms), @@ -1435,10 +1446,12 @@ async function openSftp(event, options) { // keepalive packets the connection sits with zero data flow while the // user is just browsing files, and NAT/firewall state tables drop the // idle TCP connection after ~30-60s (the exact symptom of #669). - // Honor an explicitly configured positive keepaliveInterval (seconds); - // otherwise default to 10s, matching the SFTP jump host path below. + // Honor an explicitly configured positive keepaliveInterval (seconds) + // — including per-host overrides resolved by the renderer; otherwise + // default to 10s. countMax is forwarded from the renderer too with a + // backward-compat fallback of 3. keepaliveInterval: options.keepaliveInterval > 0 ? options.keepaliveInterval * 1000 : 10000, - keepaliveCountMax: 3, + keepaliveCountMax: options.keepaliveInterval > 0 ? (options.keepaliveCountMax ?? 3) : 3, algorithms: buildSftpAlgorithms(options.legacyAlgorithms), }; diff --git a/global.d.ts b/global.d.ts index 14de6b98f..2740a7294 100644 --- a/global.d.ts +++ b/global.d.ts @@ -145,6 +145,10 @@ declare global { jumpHosts?: NetcattyJumpHost[]; identityFilePaths?: string[]; legacyAlgorithms?: boolean; + // Resolved keepalive for the target connection (caller has already + // applied host override / global fallback). interval in seconds. + keepaliveInterval?: number; + keepaliveCountMax?: number; } interface PortForwardResult { diff --git a/infrastructure/services/portForwardingService.ts b/infrastructure/services/portForwardingService.ts index 15ec618fc..431b2b8cd 100644 --- a/infrastructure/services/portForwardingService.ts +++ b/infrastructure/services/portForwardingService.ts @@ -4,9 +4,14 @@ * for establishing and managing SSH port forwarding tunnels. */ -import { Host, Identity, PortForwardingRule, SSHKey } from '../../domain/models'; +import { Host, Identity, PortForwardingRule, SSHKey, TerminalSettings } from '../../domain/models'; import { isEncryptedCredentialPlaceholder, sanitizeCredentialValue } from '../../domain/credentials'; import { resolveBridgeKeyAuth, resolveHostAuth } from '../../domain/sshAuth'; +import { resolveHostKeepalive } from '../../domain/host'; + +// Fallback matching DEFAULT_TERMINAL_SETTINGS so older call sites that don't +// thread terminalSettings still get the cloud-friendly defaults. +const FALLBACK_KEEPALIVE = { keepaliveInterval: 30, keepaliveCountMax: 10 }; import { logger } from '../../lib/logger'; import { localStorageAdapter } from '../persistence/localStorageAdapter'; import { STORAGE_KEY_PF_RECONNECT_CANCEL } from '../config/storageKeys'; @@ -361,8 +366,10 @@ export const startPortForward = async ( keys: SSHKey[], identities: Identity[], onStatusChange: (status: PortForwardingRule['status'], error?: string) => void, - enableReconnect = false + enableReconnect = false, + terminalSettings?: Pick, ): Promise<{ success: boolean; error?: string }> => { + const globalKeepalive = terminalSettings ?? FALLBACK_KEEPALIVE; const bridge = netcattyBridge.get(); // Clear any existing reconnect timer @@ -440,6 +447,7 @@ export const startPortForward = async ( ) { throw new Error(`Saved credentials for jump host "${jumpHost.label || jumpHost.hostname}" cannot be decrypted on this device. Open host settings and re-enter them.`); } + const hopKeepalive = resolveHostKeepalive(jumpHost, globalKeepalive); return { hostname: jumpHost.hostname, port: jumpHost.port || 22, @@ -462,6 +470,8 @@ export const startPortForward = async ( } : undefined, identityFilePaths: jumpKeyAuth.identityFilePaths, + keepaliveInterval: hopKeepalive.interval, + keepaliveCountMax: hopKeepalive.countMax, }; }); } @@ -542,6 +552,8 @@ export const startPortForward = async ( jumpHosts: jumpHosts && jumpHosts.length > 0 ? jumpHosts : undefined, identityFilePaths: keyAuth.identityFilePaths, legacyAlgorithms: host.legacyAlgorithms, + keepaliveInterval: resolveHostKeepalive(host, globalKeepalive).interval, + keepaliveCountMax: resolveHostKeepalive(host, globalKeepalive).countMax, }); if (!result.success) {