From 3ecebdb839f1049e29cadd05f1d4698a35c7af42 Mon Sep 17 00:00:00 2001 From: Gareth Date: Sun, 12 Apr 2026 15:05:53 -0700 Subject: [PATCH] fix: further simplify SFTP setup and key copying --- internal/api/backresthandler.go | 40 +---- .../features/repositories/AddRepoModal.tsx | 165 +++++++++--------- 2 files changed, 88 insertions(+), 117 deletions(-) diff --git a/internal/api/backresthandler.go b/internal/api/backresthandler.go index 22f3c636..496794f5 100644 --- a/internal/api/backresthandler.go +++ b/internal/api/backresthandler.go @@ -280,53 +280,29 @@ func (s *BackrestHandler) SetupSftp(ctx context.Context, req *connect.Request[v1 if port == "" { port = "22" } - user := req.Msg.Username - password := req.Msg.Password // Optional if runtime.GOOS == "windows" { return nil, connect.NewError(connect.CodeUnimplemented, errors.New("automated SFTP setup is not supported on Windows")) } - // 1. Host Key Verification/Addition - if err := sftputil.AddHostKey(host, port, env.SSHDir()); err != nil { - return connect.NewResponse(&v1.SetupSftpResponse{ - Error: fmt.Sprintf("Failed to add host key: %v", err), - }), nil - } - - // 2. Generate Key + // 1. Generate key pair (local, always succeeds) _, pubBytes, keyPath, err := sftputil.GenerateKey(host, env.SSHDir()) if err != nil { return nil, fmt.Errorf("failed to generate key: %w", err) } - pubKeyStr := string(pubBytes) - - // 3. Install if password provided - if password != nil { - if err := sftputil.InstallKey(host, port, user, *password, pubBytes); err != nil { - return connect.NewResponse(&v1.SetupSftpResponse{ - Error: fmt.Sprintf("Failed to install key: %v", err), - }), nil - } - - // Verify - privPEM, err := os.ReadFile(keyPath) - if err != nil { - return nil, fmt.Errorf("failed to read generated private key for verification: %w", err) - } - - if err := sftputil.VerifyConnection(host, port, user, privPEM); err != nil { - return connect.NewResponse(&v1.SetupSftpResponse{ - Error: fmt.Sprintf("Key installed but verification failed: %v", err), - }), nil - } + // 2. Scan remote host key into known_hosts (network, non-fatal) + var hostKeyWarning string + if err := sftputil.AddHostKey(host, port, env.SSHDir()); err != nil { + zap.S().Warnf("SFTP host key scan failed for %s: %v", host, err) + hostKeyWarning = fmt.Sprintf("Could not scan host key (%v). Add the host key to known_hosts manually or ensure the host is reachable.", err) } return connect.NewResponse(&v1.SetupSftpResponse{ - PublicKey: pubKeyStr, + PublicKey: string(pubBytes), KeyPath: keyPath, KnownHostsPath: filepath.Join(env.SSHDir(), "known_hosts"), + Error: hostKeyWarning, }), nil } diff --git a/webui/src/features/repositories/AddRepoModal.tsx b/webui/src/features/repositories/AddRepoModal.tsx index 87a653ef..39c3da98 100644 --- a/webui/src/features/repositories/AddRepoModal.tsx +++ b/webui/src/features/repositories/AddRepoModal.tsx @@ -108,6 +108,7 @@ interface SftpConfigSectionProps { onChangeIdentityFile: (path: string) => void; port: number | null; onChangePort: (port: number | null) => void; + knownHostsPath: string; onChangeKnownHostsPath: (path: string) => void; isWindows: boolean; } @@ -118,70 +119,48 @@ const SftpConfigSection = ({ onChangeIdentityFile, port, onChangePort, + knownHostsPath, onChangeKnownHostsPath, isWindows, }: SftpConfigSectionProps) => { - // Setup Keys state - const [sftpUsername, setSftpUsername] = useState(""); - const [sftpPassword, setSftpPassword] = useState(""); const [setupLoading, setSetupLoading] = useState(false); - const [generatedPublicKey, setGeneratedPublicKey] = useState( - null, - ); + const [generatedPublicKey, setGeneratedPublicKey] = useState(null); + const [hostKeyWarning, setHostKeyWarning] = useState(null); + const [keyCopied, setKeyCopied] = useState(false); if (isWindows) return null; - const handleSetupKeys = async () => { + const handleGenerateKey = async () => { setSetupLoading(true); setGeneratedPublicKey(null); + setHostKeyWarning(null); try { if (!uri) return; - // Simple parse of URI for host/port if not fully robust - let host = ""; + + // Parse host and port from the SFTP URI + const authority = uri.replace("sftp:", "").split("/")[0]; + const hostPart = authority.includes("@") ? authority.split("@")[1] : authority; + let host = hostPart; let defaultPort = "22"; - const uriParts = uri.replace("sftp:", "").split("/"); - const authority = uriParts[0]; - let hostPart = authority; - if (authority.includes("@")) { - setSftpUsername(authority.split("@")[0]); - hostPart = authority.split("@")[1]; - } - if (hostPart.includes(":")) { - host = hostPart.split(":")[0]; - defaultPort = hostPart.split(":")[1]; - } else { - host = hostPart; + [host, defaultPort] = hostPart.split(":"); } - // Override from manual input if username is set there - const username = sftpUsername || uri.match(/([^@]+)@/)?.[1] || ""; - const res = await backrestService.setupSftp({ - host: host, + host, port: port ? port.toString() : defaultPort, - username: username, - password: sftpPassword || undefined, + username: "", }); - if (res.error) { - throw new Error(res.error); - } - onChangeIdentityFile(res.keyPath); onChangeKnownHostsPath(res.knownHostsPath); if (res.publicKey) { setGeneratedPublicKey(res.publicKey); } - alerts.success( - "Created SSH keypair at " + - res.keyPath + - " and updated known hosts file at " + - res.knownHostsPath, - ); - alerts.success( - "Updated restic flags to use the SSH keypair and known hosts file.", - ); + if (res.error) { + setHostKeyWarning(res.error); + } + alerts.success("Generated SSH keypair at " + res.keyPath); } catch (e: any) { alerts.error(formatErrorAlert(e, "SFTP Setup Failed")); } finally { @@ -195,34 +174,22 @@ const SftpConfigSection = ({ - Bootstrap SSH Key (Optional) + Setup SSH Key (Optional) - Enter your SSH credentials here. When you click "Setup Keys", - backrest will generate an SSH key pair. + Click "Generate Key" to create an SSH key pair for this host. + Backrest will attempt to scan the host key into known_hosts automatically. + You will then need to add the generated public key to{" "} + ~/.ssh/authorized_keys on the remote server. - - setSftpUsername(e.target.value)} - /> - - - setSftpPassword(e.target.value)} - /> - @@ -237,8 +204,7 @@ const SftpConfigSection = ({ Key Generated Successfully! - Please add the following public key to your server's{" "} - ~/.ssh/authorized_keys file: + Add the following public key to ~/.ssh/authorized_keys on the remote server: { navigator.clipboard.writeText(generatedPublicKey || ""); - alerts.success("Key copied to clipboard"); + setKeyCopied(true); + setTimeout(() => setKeyCopied(false), 2000); }} + colorPalette={keyCopied ? "green" : undefined} > - Copy + {keyCopied ? "Copied!" : "Copy"} + {hostKeyWarning && ( + + + Host key scan failed: {hostKeyWarning} + + + )} )} @@ -288,6 +263,17 @@ const SftpConfigSection = ({ defaultValue={"22"} /> + + + onChangeKnownHostsPath(e.target.value)} + /> + ); }; @@ -325,11 +311,30 @@ export const AddRepoModal = ({ template }: { template: Repo | null }) => { ? toJson(RepoSchema, template, { alwaysEmitImplicit: true }) : toJson(RepoSchema, repoDefaults, { alwaysEmitImplicit: true }), ); - // Reset SFTP fields when template changes (or is null) - if (!template) { - setSftpIdentityFile(""); - setSftpPort(null); - setSftpKnownHostsPath(""); + + setSftpIdentityFile(""); + setSftpPort(null); + setSftpKnownHostsPath(""); + + if (template?.uri?.startsWith("sftp:")) { + // Populate SFTP fields by parsing the existing sftp.args flag + const sftpArgsFlag = (template.flags || []).find( + (f) => f.includes("sftp.args") || f.includes("sftp.command"), + ); + if (sftpArgsFlag) { + const argsMatch = sftpArgsFlag.match(/sftp\.args=['"]?(.+?)['"]?\s*$/); + if (argsMatch) { + const argsStr = argsMatch[1].replace(/^'|'$/g, ""); + const identityMatch = argsStr.match(/-i\s+["']?([^\s"']+)["']?/); + if (identityMatch) setSftpIdentityFile(identityMatch[1]); + const portMatch = argsStr.match(/-p\s+(\d+)/); + if (portMatch) setSftpPort(parseInt(portMatch[1], 10)); + const knownHostsMatch = argsStr.match( + /-oUserKnownHostsFile=["']?([^\s"']+)["']?/, + ); + if (knownHostsMatch) setSftpKnownHostsPath(knownHostsMatch[1]); + } + } } }, [template]); @@ -359,13 +364,8 @@ export const AddRepoModal = ({ template }: { template: Repo | null }) => { // read the current value without flags being a reactive dependency. flagsRef.current = (formData.flags as string[]) || []; - // Logic to update flags based on SFTP inputs + // Keep sftp.args flag in sync with the SFTP config fields. useEffect(() => { - // If we are editing, we don't touch the flags. The user can edit them manually. - if (template) { - return; - } - const uri = getField(["uri"]); if (!uri?.startsWith("sftp:")) { return; @@ -379,31 +379,26 @@ export const AddRepoModal = ({ template }: { template: Repo | null }) => { f && !f.includes("sftp.args") && !f.includes("sftp.command"), ); + // Always include -oBatchMode=yes; quote paths to handle spaces. let sftpArgs = "-oBatchMode=yes"; - let argsChanged = false; if (sftpIdentityFile) { let cleanPath = sftpIdentityFile; if (cleanPath.startsWith("@")) { cleanPath = cleanPath.substring(1); } - sftpArgs += ` -i ${cleanPath}`; - argsChanged = true; + sftpArgs += ` -i "${cleanPath}"`; } if (sftpPort && sftpPort !== 0 && sftpPort !== 22) { sftpArgs += ` -p ${sftpPort}`; - argsChanged = true; } if (sftpKnownHostsPath) { - sftpArgs += ` -oUserKnownHostsFile=${sftpKnownHostsPath}`; - argsChanged = true; + sftpArgs += ` -oUserKnownHostsFile="${sftpKnownHostsPath}"`; } - if (argsChanged) { - newFlags.push(`--option=sftp.args='${sftpArgs}'`); - } + newFlags.push(`--option=sftp.args='${sftpArgs}'`); const sortedCurrent = [...currentFlags].sort(); const sortedNew = [...newFlags].sort(); @@ -416,7 +411,6 @@ export const AddRepoModal = ({ template }: { template: Repo | null }) => { sftpIdentityFile, sftpPort, sftpKnownHostsPath, - template, // flags intentionally omitted: flagsRef avoids a circular dep where any // user edit to flags would re-trigger the effect and erase empty rows. ]); @@ -794,13 +788,14 @@ export const AddRepoModal = ({ template }: { template: Repo | null }) => { {/* SFTP Specific Fields */} - {getField(["uri"])?.startsWith("sftp:") && !template && ( + {getField(["uri"])?.startsWith("sftp:") && (