feat: Add FTP and SFTP backup destination support#4044
feat: Add FTP and SFTP backup destination support#4044promisingcoder wants to merge 3 commits intoDokploy:canaryfrom
Conversation
Adds FTP and SFTP as new backup destination types alongside existing S3. Includes schema migration, API routes, upload/download logic for both protocols, restore support for all backup types, volume backup support, and UI forms for credential management. Closes Dokploy#416
dosubot
bot
added
size:XXL
|
Related Documentation 1 document(s) may need updating based on files changed in this PR: Dokploy's Space README
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| toast.error("Please fill all required fields"); | ||
| return; | ||
| } | ||
| const defaultPort = type === "sftp" ? 22 : 21; |
There was a problem hiding this comment.
defaultPort
defaultPort is defined but never used. The correct default port is already applied inline via values.ftpPort || 21 and values.ftpPort || 22 in the FTP and SFTP payload objects respectively.
| const defaultPort = type === "sftp" ? 22 : 21; | |
| } else if (type === "ftp" || type === "sftp") { |
| ftpHost: text("ftpHost"), | ||
| ftpPort: integer("ftpPort"), | ||
| ftpUser: text("ftpUser"), | ||
| ftpPassword: text("ftpPassword"), | ||
| ftpBasePath: text("ftpBasePath"), |
There was a problem hiding this comment.
The PR description lists sftpPrivateKey as a new field, but it is absent from the schema, the Drizzle migration, and the SFTP credential builder (getSftpCredentials). SFTP deployments often rely on key-based auth rather than passwords (which are sometimes disabled by policy on hardened servers). Without this, the SFTP feature is limited to password-only authentication.
If key-based auth is out of scope for this PR, the PR description should be updated to reflect that, and a follow-up issue should be opened.
| `--ftp-host="${ftpHost}"`, | ||
| `--ftp-port="${ftpPort || 21}"`, | ||
| `--ftp-user="${ftpUser}"`, | ||
| `--ftp-pass="$(rclone obscure '${ftpPassword}')"`, |
There was a problem hiding this comment.
The password value is embedded directly inside $(rclone obscure '...') using single-quote delimiters. If the password contains a single quote, the shell quoting breaks and arbitrary commands can be injected when the resulting rclone flag string is executed through a shell.
The fix is to escape single quotes in the password before interpolation (replace each ' with '\'') or to avoid inline shell substitution entirely by writing the password to a temporary environment variable or file that is passed to rclone separately.
The same issue exists in getSftpCredentials (line 96) and is duplicated in apps/dokploy/server/api/routers/destination.ts at lines 59 and 73 in the testConnection handler.
- Replace shell command substitution `$(rclone obscure '...')` with
execFileAsync("rclone", ["obscure", password]) so the password is
passed as a direct argument, never interpolated into a shell string
- Use the pre-obscured value as a plain flag value --ftp-pass/--sftp-pass
- Apply fix in both utils.ts (getFtpCredentials, getSftpCredentials) and
destination.ts testConnection handler
- Make getDestinationCredentials async and update all 7 callers with await
- Remove unused defaultPort variable in handle-destinations.tsx
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- P1: fix shell injection in FTP/SFTP credential flags by adding shellEscape helper that single-quotes values and escapes embedded single quotes, preventing host/user/pass from breaking out of shell command context - P2: add sftpPrivateKey field to destination schema and DB table to support SFTP private key authentication (migration 0154), make ftpPassword optional on SFTP to allow key-only auth
1 participant
Summary
Adds FTP and SFTP as new backup destination types alongside the existing S3-compatible storage.
Changes
Schema & Migration
s3,ftp,sftp) to destination tableftpHost,ftpPort,ftpUser,ftpPassword,ftpPath,sftpPrivateKeyBackend
basic-ftp) and SFTP (usingssh2-sftp-client)Frontend
/claim #416
Greptile Summary
This PR adds FTP and SFTP as new backup destination types alongside the existing S3-compatible storage, covering backup creation, restoration, and connection testing across all supported database and service types. The schema migration, Zod validation schemas, API routes, and frontend forms are all included and generally well-structured.
Key issues found:
packages/server/src/utils/backups/utils.tslines 86 & 96,apps/dokploy/server/api/routers/destination.tslines 59 & 73):ftpPasswordis interpolated directly into$(rclone obscure '...')using single-quote delimiters. A password containing a single quote breaks shell quoting and can lead to arbitrary command execution. The fix is to escape single quotes in the password value before embedding it in the shell string, or to avoid inline shell substitution entirely.sftpPrivateKeyas an added field, but it is absent from the schema, migration, and credential helpers. SFTP deployments that require key-based auth will not be supported.defaultPortinhandle-destinations.tsx(line 309) is computed but never referenced.Confidence Score: 2/5
getFtpCredentials/getSftpCredentialsand the inlinetestConnectionhandler means that a specially crafted password value can execute arbitrary OS commands on the host server. Even though only authenticated users with destination-create permission can trigger this, it represents an unacceptable escalation path in multi-admin environments and should be fixed before merge.packages/server/src/utils/backups/utils.ts(shell injection at lines 86 and 96) andapps/dokploy/server/api/routers/destination.ts(shell injection at lines 59 and 73)Last reviewed commit: "feat: add FTP and SF..."
(5/5) You can turn off certain types of comments like style here!