Skip to content

feat: add rclone-backed backup destination types (Google Drive, OneDrive, FTP, SFTP, custom) - #5486

Open
stffinfcti wants to merge 4 commits into
Dokploy:canaryfrom
stffinfcti:bounty/416-backup-destination-types
Open

stffinfcti wants to merge 4 commits into
Dokploy:canaryfrom
stffinfcti:bounty/416-backup-destination-types

Conversation

@stffinfcti

@stffinfcti stffinfcti commented Sep 20, 2026

Copy link
Copy Markdown

Summary

Adds support for additional backup destination types on top of the existing rclone-based S3 flow: Google Drive, OneDrive, FTP, SFTP, and a custom rclone backend.

  • New nullable rcloneConfig text column on destination stores an rclone INI-body config for non-S3 providers (drizzle migration 0197 included).
  • New shared helpers (getRcloneFlags, getRcloneRemotePath, getRcloneBackendType) build the rclone flags and remote path; every backup, restore, retention, volume-backup, and testConnection call site now goes through them. For existing S3 destinations the generated flags/remote are byte-identical to before.
  • Zod validation requires a well-formed rclone config for non-S3 providers and a valid type for custom backends.
  • The destination dialog gains a second provider group plus a conditional rcloneConfig textarea with per-provider placeholders; the list page is retitled "Backup Destinations".
  • Secret-flag log redaction widened to cover single-quoted/bare values and the non-S3 secret flags.

Why this approach

The maintainer's comments on #416 pointed to rclone config-as-argument as the preferred route. The open attempts each cover a slice of that: the OAuth-based approach needs per-provider app registration and has had review feedback pending since mid-2025, while the other open rclone PRs wire a fixed provider subset (mostly FTP/SFTP) through separate code paths and are stale or conflicting. Storing a raw rclone INI config per destination instead means every rclone backend works through one column — including a validated custom type — and a single shared flag/remote builder covers every call site, including retention and volume backups, while keeping the S3 path byte-identical to current behavior.

Testing

  • vitest: 1066 tests passing, including 40+ new cases covering per-provider flag/remote construction, S3 backward compatibility, config validation, and credential redaction. The only failures are 4 pre-existing environmental ones in the nixpacks/docker-swarm tests that need a live daemon.
  • tsc --noEmit clean across all 4 packages; biome clean on changed files.
  • The automated review findings on the previous commit (custom type = s3 flag routing, quoted retention remote paths, credential redaction on failed-backup notifications) are addressed in the latest commit.

@algora-pbc /claim #416

Closes #416

Bounty payout: @stffinfcti

Adds Google Drive, OneDrive, FTP, SFTP and custom rclone backends as
backup destinations. Non-S3 providers store an rclone config body in a
new rcloneConfig column; backup, restore, retention and volume-backup
commands build their flags and remote paths through shared helpers that
keep the S3 path byte-identical.

Closes Dokploy#416

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Comment on lines +109 to +112
const backend = getRcloneBackendType(destination);
if (backend === "s3") {
return getS3Credentials(destination);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Custom S3 Backends Fail

A custom destination with type = s3 passes validation, but this branch handles it as a legacy S3 destination instead of translating its rcloneConfig. The UI stores empty legacy credentials for custom destinations, and getS3Credentials also emits --s3-provider=custom. As a result, connection tests, backups, and restores fail for this accepted configuration. Either reject s3 as a custom type or build its flags from rcloneConfig.

Comment on lines +119 to +124
const rcloneFlags = Object.entries(config)
.filter(([key]) => key !== "type")
.map(
([key, value]) =>
`--${backend}-${key.replaceAll("_", "-")}=${quote([value])}`,
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 security Failures Expose Rclone Secrets

These non-S3 secret flags are embedded in the backup shell command, but failed database and Compose backups forward the resulting ExecError.message directly to notification channels without calling redactRcloneCredentials. Because execAsync includes the failed command in that message, an FTP/SFTP password or OAuth token can be disclosed through Slack, Discord, email, and other configured notifications when an upload fails. Redact the error before passing it to notification dispatchers in every backup implementation.

How this was verified: The generated secret flags flow into the executed command, whose full failure message is forwarded unchanged as the notification error text.

Comment on lines +12 to +16
const SECRET_FLAG_PATTERN =
/(--[\w-]*-(?:access-key-id|secret-access-key|client-secret|secret|token|pass|password\d*|key-pem|key-file|private-key|service-account-credentials|credentials|sas-key|account-key|shared-key|auth-token|bearer-token|key)=)("(?:[^"\\]|\\.)*"|'(?:[^'\\]|\\.)*'|[^\s'"])+/g;

export const redactRcloneCredentials = (command: string): string => {
return command
.replace(/(--s3-access-key-id=)"[^"]*"/g, '$1"[REDACTED]"')
.replace(/(--s3-secret-access-key=)"[^"]*"/g, '$1"[REDACTED]"');
return command.replace(SECRET_FLAG_PATTERN, '$1"[REDACTED]"');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 security Apostrophes Bypass Full Redaction

The new expression only consumes one simple single-quoted segment. A secret containing an apostrophe is shell-quoted as concatenated segments such as 'it'\''s'; this pattern replaces only 'it' and leaves the remaining secret suffix in logs and error output. Add coverage for shell-quote's embedded-apostrophe representation and redact the complete argument.

How this was verified: Comparing the accepted single-quoted branch with the concatenated shell representation shows that matching stops at the first closing quote.

@greptile-apps

greptile-apps Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Comments Outside Diff

These findings sit on lines the diff does not cover, so they could not be posted inline. Each one leaves this list once its file changes.

  • P1 Retention Splits Remote Paths packages/server/src/utils/backups/index.ts:155

    Non-S3 remote roots can contain spaces, such as a Drive folder named Team Backups, but retention inserts backupFilesPath into the list and delete commands without shell quoting. Upload and listing quote the same remote successfully, while this pipeline splits it into multiple arguments and silently catches the failure. This leaves old database and volume backups undeleted. The same unquoted construction also appears in packages/server/src/utils/volume-backups/utils.ts.

@devin-ai-integration
devin-ai-integration Bot force-pushed the bounty/416-backup-destination-types branch from da5e3e9 to ca88409 Compare September 20, 2026 09:30
@stffinfcti
stffinfcti force-pushed the bounty/416-backup-destination-types branch from ca88409 to 74198ad Compare September 20, 2026 09:31
Remote roots containing spaces (e.g. a Drive folder 'Team Backups')
were split into multiple args, so retention cleanup silently failed
and old backups were never deleted. Quote the remote path the same
way the upload commands already do.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@stffinfcti
stffinfcti force-pushed the bounty/416-backup-destination-types branch from 74198ad to 3d86055 Compare September 20, 2026 09:32
stffinfcti and others added 2 commits September 20, 2026 10:31
- Route custom destination flags through rcloneConfig so type = s3
  receives backend flags instead of empty legacy S3 credentials
- Harden redactRcloneCredentials to consume POSIX escaped-apostrophe
  quoting ('it'\''s') as a single token
- Apply redactRcloneCredentials to restore emit/throw paths, retention
  cleanup logging, and TRPC error responses surfaced to clients

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ion gaps

- Route rclone credentials through RCLONE_* env vars instead of
  --backend-secret= argv flags (getRcloneEnv); env is delivered to remote
  servers via a 0600 SFTP env file sourced in a subshell, so it never
  appears in ps output or error text. ExecError sanitizes stored
  command/stdout/stderr via redactRcloneCredentials.
- Require a Remote Path (bucket) for custom destinations whose
  rcloneConfig type is s3, so the resolved :s3: remote can't land in a
  different bucket.
- redact.ts: match '[^']*' single-quote segments plus \. escapes so
  POSIX concatenated quoting ('it'\''s', trailing backslash) is fully
  consumed.
- Retention: pipe 'rclone lsf' into 'rclone delete --files-from -'
  instead of xargs -I{} filename interpolation (filenames with spaces or
  quotes silently broke deletion and could inject into the delete arg).

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ability to backup to more destination types

1 participant