Add Déjà Dup Backups extension - #331
Conversation
6027750 to
316b850
Compare
|
@clankus-aurelius review |
|
Thanks for contributing an extension to Vicinae! 👋 Before publication, this pull request receives two reviews:
🔴 Contributor changes requested. Address the blocking inline findings and push a new commit; the bot will review it automatically. 5 blocking findings must be addressed. The automated reviewer examines only the current commit. New commits invalidate its previous decision and start another review. |
clankus-aurelius
left a comment
There was a problem hiding this comment.
The extension has publication-blocking credential handling and restore-safety issues. It also misrepresents search scope and performs background repository access while automatic indexing is disabled.
Automated review found 5 publication-blocking issues.
| } | ||
| } | ||
|
|
||
| /** The restic repository passphrase (Déjà Dup "Backup encryption password"). */ | ||
| export async function getPassphrase(): Promise<string> { | ||
| // A manually entered password wins — the escape hatch for Flatpak or custom keyrings. | ||
| const manual = prefs().backupPassword; | ||
| if (manual) return manual; | ||
|
|
||
| const pass = await secretLookup(["owner", "deja-dup", "type", "passphrase"]); | ||
| if (pass) return pass; | ||
|
|
There was a problem hiding this comment.
🔴 Blocking — Credentials are read directly from the system keyring
Rule: SECURITY-003
secret-tool directly accesses Déjà Dup passphrases and OAuth refresh tokens, bypassing Vicinae-managed credential storage and lifecycle.
Suggested resolution: Have users store the backup passphrase through Vicinae LocalStorage and authorize cloud accounts independently with OAuth.PKCEClient instead of reading another application's keyring.
| "org.gnome.DejaDup.Google", | ||
| "client_id", | ||
| GOOGLE_CLIENT_ID, | ||
| ])) || (await secretLookup(["client_id", GOOGLE_CLIENT_ID])); | ||
|
|
||
| if (!refresh) { | ||
| throw new UnsupportedError( | ||
| "Could not find the Google Drive token in the keyring. Open Déjà Dup once to reconnect the account, then try again.", | ||
| ); | ||
| } | ||
|
|
||
| const body = new URLSearchParams({ | ||
| client_id: GOOGLE_CLIENT_ID, | ||
| refresh_token: refresh, | ||
| grant_type: "refresh_token", | ||
| }); | ||
| const res = await fetch(GOOGLE_TOKEN_URL, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/x-www-form-urlencoded" }, | ||
| body: body.toString(), | ||
| }); | ||
| if (!res.ok) { | ||
| throw new Error(`Google token refresh failed (HTTP ${res.status}). Re-authorise Déjà Dup.`); | ||
| } | ||
| const data = (await res.json()) as { access_token?: string; expires_in?: number }; | ||
| if (!data.access_token) { | ||
| throw new Error("Google token refresh returned no access token."); | ||
| } | ||
| const expiresAt = now + (data.expires_in ?? 3600) * 1000; | ||
| const cached: CachedToken = { access_token: data.access_token, expires_at: expiresAt }; | ||
| await writeFile(cachePath, JSON.stringify(cached), { mode: 0o600 }); | ||
|
|
||
| return { token: data.access_token, expiry: new Date(expiresAt).toISOString() }; | ||
| } | ||
|
|
||
| /* --- Microsoft OneDrive (Personal) --- */ | ||
| const MS_CLIENT_ID = "5291592c-3c09-44fb-a275-5027aa238645"; | ||
| const MS_TOKEN_URL = "https://login.microsoftonline.com/common/oauth2/v2.0/token"; | ||
| const MS_GRAPH_DRIVE = "https://graph.microsoft.com/v1.0/me/drive?select=id"; | ||
|
|
||
| interface CachedMsAuth { | ||
| tokenJson: string; | ||
| driveId: string; | ||
| expires_at: number; | ||
| } | ||
|
|
||
| async function getMicrosoftAuth(): Promise<{ tokenJson: string; driveId: string }> { | ||
| const cachePath = await tokenCachePath("microsoft"); | ||
| const now = Date.now(); | ||
| try { | ||
| const c: CachedMsAuth = JSON.parse(await readFile(cachePath, "utf8")); | ||
| if (c.tokenJson && c.driveId && c.expires_at - 60_000 > now) { | ||
| return { tokenJson: c.tokenJson, driveId: c.driveId }; | ||
| } | ||
| } catch { | ||
| // fall through to refresh | ||
| } | ||
|
|
||
| const refresh = | ||
| (await secretLookup(["xdg:schema", "org.gnome.DejaDup.Microsoft", "client_id", MS_CLIENT_ID])) || | ||
| (await secretLookup(["client_id", MS_CLIENT_ID])); | ||
| if (!refresh) { | ||
| throw new UnsupportedError( | ||
| "Could not find the OneDrive token in the keyring. Open Déjà Dup once to reconnect OneDrive, then try again.", | ||
| ); | ||
| } | ||
|
|
||
| const body = new URLSearchParams({ | ||
| client_id: MS_CLIENT_ID, | ||
| refresh_token: refresh, | ||
| grant_type: "refresh_token", | ||
| scope: "offline_access Files.ReadWrite", | ||
| }); | ||
| const res = await fetch(MS_TOKEN_URL, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/x-www-form-urlencoded" }, | ||
| body: body.toString(), | ||
| }); | ||
| if (!res.ok) throw new Error(`OneDrive token refresh failed (HTTP ${res.status}). Re-authorise Déjà Dup.`); | ||
| const data = (await res.json()) as { access_token?: string; refresh_token?: string; expires_in?: number }; | ||
| if (!data.access_token) throw new Error("OneDrive token refresh returned no access token."); | ||
|
|
||
| const expiresAt = now + (data.expires_in ?? 3600) * 1000; | ||
| const tokenJson = JSON.stringify({ | ||
| access_token: data.access_token, | ||
| token_type: "Bearer", | ||
| refresh_token: data.refresh_token ?? refresh, | ||
| expiry: new Date(expiresAt).toISOString(), | ||
| }); | ||
|
|
||
| const dres = await fetch(MS_GRAPH_DRIVE, { | ||
| headers: { Authorization: `Bearer ${data.access_token}` }, | ||
| }); | ||
| if (!dres.ok) throw new Error(`Could not read the OneDrive drive id (HTTP ${dres.status}).`); | ||
| const drive = (await dres.json()) as { id?: string }; | ||
| if (!drive.id) throw new Error("Could not determine the OneDrive drive id."); | ||
|
|
||
| await writeFile(cachePath, JSON.stringify({ tokenJson, driveId: drive.id, expires_at: expiresAt }), { | ||
| mode: 0o600, | ||
| }); | ||
| return { tokenJson, driveId: drive.id }; | ||
| } | ||
|
|
||
| /* --- Network (gvfs) and removable-drive mount resolution --- */ | ||
|
|
||
| async function resolveRemoteMount(uri: string, folder: string): Promise<string> { | ||
| let host = ""; | ||
| try { | ||
| host = new URL(uri).hostname; | ||
| } catch { | ||
| throw new UnsupportedError(`Could not parse the network address "${uri}".`); | ||
| } | ||
| const uid = typeof process.getuid === "function" ? process.getuid() : 1000; | ||
| const gvfsDir = `/run/user/${uid}/gvfs`; | ||
| const entries = await readdir(gvfsDir).catch(() => [] as string[]); | ||
| const mount = entries.find((e) => e.includes(`host=${host}`) || e.includes(`server=${host}`)); | ||
| if (!mount) { | ||
| throw new UnsupportedError( | ||
| `The network location ${uri} isn't mounted. Open it once in your file manager (Files) so it mounts, then try again.`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🔴 Blocking — OAuth tokens are cached in ad-hoc files
Rule: SECURITY-003
Google access tokens and Microsoft access/refresh tokens are persisted as JSON files under environment.supportPath. Vicinae requires OAuth tokens to use OAuth.PKCEClient.setTokens, getTokens, and removeTokens.
Suggested resolution: Replace the custom token files and refresh requests with separate OAuth.PKCEClient flows for Google and Microsoft, and regenerate package-lock.json after upgrading @vicinae/api to 0.24.0 if needed.
| * line is the snapshot object, the rest are nodes. Without `--recursive` and with an | ||
| * explicit directory path, restic returns only that subtree's direct children (plus the | ||
| * ancestor dirs it traverses, which we filter out). Passing no path would list the ENTIRE | ||
| * snapshot recursively — for a large home dir that is hundreds of MB, so always pass one. | ||
| */ | ||
| export async function listDir( | ||
| snapshotId: string, | ||
| path: string, | ||
| config?: DejaConfig, | ||
| ): Promise<ResticNode[]> { |
There was a problem hiding this comment.
🔴 Blocking — Backup password is exposed in Flatpak command arguments
Rule: SECURITY-003
Every repository environment value, including RESTIC_PASSWORD, is converted to a --env=KEY=value command-line argument. The password is therefore visible through process command-line inspection.
Suggested resolution: Do not place credentials in Flatpak arguments. Use a credential-passing mechanism that keeps secrets out of argv, or declare Flatpak repository access unsupported when this cannot be done safely.
| } | ||
| /> | ||
| <RestoreActions config={config} snapshot={snapshot} node={node} /> | ||
| </ActionPanel> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🔴 Blocking — Restore to original location can overwrite live files
Rule: CORRECTNESS-001
The action restores directly into /. Restic recreates snapshot paths there and can overwrite existing files, so this can replace current user data without confirmation and contradicts the stated create-only behavior.
Suggested resolution: Detect destination collisions and require explicit destructive confirmation, or restore into a staging directory and copy only paths that do not already exist.
| { | ||
| "name": "search-files", | ||
| "title": "Search Backup Files", | ||
| "subtitle": "Déjà Dup", |
There was a problem hiding this comment.
🔴 Blocking — Search command overstates its scope
Rule: MANIFEST-001
The command says it searches across backup snapshots, but SearchFiles indexes and searches only snaps[0], the latest snapshot.
Suggested resolution: Describe the command as searching the latest snapshot, or implement selection and indexing across snapshots.
| "subtitle": "Déjà Dup", | |
| "description": "Search for files in your latest backup snapshot", |
| await new Promise<void>((resolve) => { | ||
| execFile("deja-dup", ["--backup"], async (err) => { | ||
| if (err) { | ||
| await showToast({ | ||
| style: Toast.Style.Failure, | ||
| title: "Could not start backup", | ||
| message: err.message, | ||
| }); | ||
| } else { | ||
| await showHUD("Backup started in Déjà Dup"); | ||
| } | ||
| resolve(); | ||
| }); |
There was a problem hiding this comment.
🟠 Warning — Back Up Now ignores the detected installation flavor
Rule: CORRECTNESS-001
The command always executes the host deja-dup binary. A Flatpak installation—the supported case described by the extension—normally requires flatpak run org.gnome.DejaDup --backup, so this command fails when no host binary exists.
Suggested resolution: Route backup launches through the detected flavor and configured installation, sharing one launcher implementation with the other commands.
| export default async function IndexLatest() { | ||
| try { | ||
| const snaps = await listSnapshots(); | ||
| await pruneOrphanIndexes(snaps.map((s) => s.short_id)); | ||
|
|
||
| if (!autoIndexEnabled()) return; |
There was a problem hiding this comment.
🟠 Warning — Disabled automatic indexing still accesses the repository
Rule: MANIFEST-001
The six-hour command calls listSnapshots() before checking autoIndexEnabled(). Consequently, the default disabled setting still unlocks credentials and accesses remote repositories periodically, despite presenting indexing as on-demand when off.
Suggested resolution: Check autoIndexEnabled() before accessing the repository; perform orphan cleanup during interactive access or another operation that already has the snapshot list.
| export default async function IndexLatest() { | |
| try { | |
| const snaps = await listSnapshots(); | |
| await pruneOrphanIndexes(snaps.map((s) => s.short_id)); | |
| if (!autoIndexEnabled()) return; | |
| try { | |
| if (!autoIndexEnabled()) return; | |
| const snaps = await listSnapshots(); | |
| await pruneOrphanIndexes(snaps.map((s) => s.short_id)); |
|
Just to add my own comment after the agent did so, I think this extension tries to duplicate way too much non trivial functionnality from deja dup (including careful handling of google/microsoft tokens) and as such I don't think it is a good idea. |
Adds Déjà Dup Backups — browse, search and restore files from Déjà Dup (restic) backups from Vicinae.
Commands
Backends
Local folder, removable drive, network server (SMB/SFTP/WebDAV/Nextcloud), Google Drive, OneDrive, and rclone remotes. Native, Snap and (best-effort) Flatpak installs are auto-detected, with manual overrides in preferences.
All repository access is read-only (
restic --no-lock); restores only ever create new files in the chosen target.Tested end-to-end on native Déjà Dup with a Google Drive backend; other backends follow Déjà Dup's own source mapping.