feat: Add configuration export functionality with flexible options#92
feat: Add configuration export functionality with flexible options#92tvarohohlavy wants to merge 60 commits intonicotsx:mainfrom
Conversation
|
Warning Rate limit exceeded@tvarohohlavy has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 9 minutes and 0 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a full-configuration export: client ExportDialog and react-query mutation, server POST /api/v1/config/export controller and DTO/OpenAPI, auth re‑verification, SDK/types/mutation for export, crypto utils exposed, route registration, and README docs. (50 words) Changes
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
app/server/modules/lifecycle/config-export.controller.ts (2)
94-111: Arrays containing objects with secrets are not processed.The recursive processing handles nested objects but skips arrays entirely. If any entity stores secrets inside array elements (e.g., a list of credentials), those secrets would not be excluded or decrypted.
Consider adding array traversal:
} else if (value && typeof value === "object" && !Array.isArray(value)) { result[key] = await processSecrets(value as Record<string, unknown>, secretsMode); + } else if (Array.isArray(value)) { + result[key] = await Promise.all( + value.map(async (item) => + item && typeof item === "object" && !Array.isArray(item) + ? processSecrets(item as Record<string, unknown>, secretsMode) + : item + ) + ); }
99-103: Silent deletion on decryption failure may hide issues.When decryption fails, the field is silently deleted. This could mask configuration or key mismatch problems. Consider logging at debug/warn level.
} else if (secretsMode === "cleartext") { try { result[key] = await cryptoUtils.decrypt(value); - } catch { + } catch (err) { + logger.debug(`Failed to decrypt field '${key}': ${err instanceof Error ? err.message : String(err)}`); delete result[key]; } }Note: This would require importing
loggerfrom../../utils/logger.app/client/components/export-dialog.tsx (5)
37-47: Defer cleanup to avoid potential race conditions.The immediate removal of the anchor element (line 45) and URL revocation (line 46) could cause issues in some browsers where the download hasn't fully initiated. While this typically works, deferring cleanup improves reliability.
Apply this diff to defer cleanup:
function downloadAsJson(data: unknown, filename: string): void { const blob = new Blob([JSON.stringify(data, null, 2)], { type: "application/json" }); const url = URL.createObjectURL(blob); const a = document.createElement("a"); a.href = url; a.download = `${filename}.json`; document.body.appendChild(a); a.click(); - document.body.removeChild(a); - URL.revokeObjectURL(url); + setTimeout(() => { + document.body.removeChild(a); + URL.revokeObjectURL(url); + }, 100); }
49-69: Verify Content-Type before parsing JSON.Line 67 assumes the response is JSON without checking the
Content-Typeheader. If the server returns an error in a different format, this could cause a cryptic parsing error.Apply this diff to add Content-Type validation:
const res = await fetch(url, { credentials: "include" }); if (!res.ok) { throw new Error(`Export failed: ${res.statusText}`); } + const contentType = res.headers.get("content-type"); + if (!contentType?.includes("application/json")) { + throw new Error("Invalid response format: expected JSON"); + } + const data = await res.json(); downloadAsJson(data, filename); }
152-159: Consider useReducer for related state.Eight separate
useStatecalls manage closely-related export options. While functional, consolidating into a singleuseReducerimproves maintainability and makes state transitions more explicit.Example refactor:
type ExportState = { includeIds: boolean; includeTimestamps: boolean; includeRuntimeState: boolean; includeRecoveryKey: boolean; includePasswordHash: boolean; secretsMode: SecretsMode; }; const [options, dispatch] = useReducer( (state: ExportState, action: Partial<ExportState>) => ({ ...state, ...action }), { includeIds: true, includeTimestamps: true, includeRuntimeState: false, includeRecoveryKey: false, includePasswordHash: false, secretsMode: "exclude", } ); // Usage: dispatch({ includeIds: false })
168-190: Consider cleaner conditional property inclusion.Lines 175-177 explicitly pass
undefinedfor conditional options. While functional, using conditional spread operators is more idiomatic and clearer.Apply this diff:
const handleExport = async () => { setIsExporting(true); try { await exportConfig(entityType, { includeIds, includeTimestamps, includeRuntimeState, - includeRecoveryKey: isFullExport ? includeRecoveryKey : undefined, - includePasswordHash: isFullExport ? includePasswordHash : undefined, - secretsMode: hasSecrets ? secretsMode : undefined, + ...(isFullExport && { includeRecoveryKey, includePasswordHash }), + ...(hasSecrets && { secretsMode }), name, id, }); toast.success(`${entityLabel} exported successfully`); setOpen(false);
337-348: Remove unused variable.Line 338 assigns
configfromexportConfigs[entityType]but never uses it.Apply this diff:
export function ExportCard({ entityType, ...props }: Omit<ExportDialogProps, "variant" | "trigger">) { - const config = exportConfigs[entityType]; - return ( <ExportDialog entityType={entityType}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
README.md(1 hunks)app/client/components/export-dialog.tsx(1 hunks)app/client/modules/backups/components/schedule-summary.tsx(2 hunks)app/client/modules/backups/routes/backups.tsx(2 hunks)app/client/modules/notifications/routes/notification-details.tsx(2 hunks)app/client/modules/notifications/routes/notifications.tsx(2 hunks)app/client/modules/repositories/routes/repositories.tsx(2 hunks)app/client/modules/repositories/routes/repository-details.tsx(2 hunks)app/client/modules/settings/routes/settings.tsx(2 hunks)app/client/modules/volumes/routes/volume-details.tsx(2 hunks)app/client/modules/volumes/routes/volumes.tsx(2 hunks)app/server/index.ts(2 hunks)app/server/modules/lifecycle/config-export.controller.ts(1 hunks)app/server/utils/crypto.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (11)
app/client/modules/volumes/routes/volumes.tsx (1)
app/client/components/export-dialog.tsx (1)
ExportDialog(142-335)
app/client/modules/volumes/routes/volume-details.tsx (1)
app/client/components/export-dialog.tsx (1)
ExportDialog(142-335)
app/server/index.ts (1)
app/server/modules/lifecycle/config-export.controller.ts (1)
configExportController(236-355)
app/client/modules/notifications/routes/notifications.tsx (1)
app/client/components/export-dialog.tsx (1)
ExportDialog(142-335)
app/client/modules/repositories/routes/repositories.tsx (1)
app/client/components/export-dialog.tsx (1)
ExportDialog(142-335)
app/client/modules/backups/components/schedule-summary.tsx (1)
app/client/components/export-dialog.tsx (1)
ExportDialog(142-335)
app/client/modules/repositories/routes/repository-details.tsx (1)
app/client/components/export-dialog.tsx (1)
ExportDialog(142-335)
app/server/modules/lifecycle/config-export.controller.ts (5)
app/client/components/export-dialog.tsx (1)
SecretsMode(24-24)app/server/utils/crypto.ts (1)
cryptoUtils(62-66)app/server/db/schema.ts (6)
volumesTable(10-22)repositoriesTable(50-62)notificationDestinationsTable(112-120)backupSchedulesTable(68-95)backupScheduleNotificationsTable(129-144)usersTable(28-35)app/server/db/db.ts (1)
db(13-13)app/server/core/constants.ts (1)
RESTIC_PASS_FILE(5-5)
app/client/modules/settings/routes/settings.tsx (1)
app/client/components/export-dialog.tsx (1)
ExportDialog(142-335)
app/client/modules/backups/routes/backups.tsx (1)
app/client/components/export-dialog.tsx (1)
ExportDialog(142-335)
app/client/modules/notifications/routes/notification-details.tsx (1)
app/client/components/export-dialog.tsx (1)
ExportDialog(142-335)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (28)
README.md (1)
199-218: LGTM! Clear and comprehensive documentation.The export documentation accurately describes the feature capabilities, options, and security considerations. The structure is well-organized and user-friendly.
app/client/modules/repositories/routes/repositories.tsx (1)
123-129: LGTM! Clean integration of export functionality.The ExportDialog is correctly integrated alongside the existing Create Repository action. The
entityType="repositories"prop withoutnameoridwill export all repositories, which is the expected behavior for a list page.app/server/index.ts (1)
25-25: LGTM! Proper authentication enforcement for sensitive export endpoints.The new config export route is correctly integrated with
requireAuthmiddleware, which is essential given that exports can include sensitive data like recovery keys and password hashes. The route follows the existing pattern used by other API endpoints.Also applies to: 55-56
app/client/modules/notifications/routes/notification-details.tsx (2)
146-146: LGTM! Correctly configured for single-item export.The ExportDialog is properly configured with
name={data.name}to export this specific notification configuration. ThetriggerLabel="Export config"provides clear user feedback.
152-152: LGTM! Consistent iconography enhancement.Adding the Trash2 icon to the Delete button improves visual consistency with other action buttons in the interface.
app/client/modules/notifications/routes/notifications.tsx (1)
126-132: LGTM! Consistent pattern for list-level export.The ExportDialog is properly integrated alongside the Create Destination action. Without
nameoridprops, this correctly exports all notification configurations, which is appropriate for a list page.app/client/modules/volumes/routes/volume-details.tsx (2)
165-165: LGTM! Properly configured for single-volume export.The ExportDialog is correctly configured with
name={volume.name}to export this specific volume's configuration. Note that per the TODO in ExportDialog, volume secrets handling will be added in a future PR.
167-167: LGTM! UI consistency enhancement.The Trash2 icon addition maintains visual consistency across the application's action buttons.
app/client/modules/settings/routes/settings.tsx (1)
147-149: LGTM! Appropriate placement for full configuration export.The full config export is correctly positioned in the Settings page with
entityType="full", which enables the security-sensitive options (recovery key and password hash). The explicit "Export All Config (JSON)" label clearly communicates the scope of the export.app/client/modules/backups/components/schedule-summary.tsx (1)
129-129: LGTM! Consistent integration in the action cluster.The ExportDialog is properly configured with
id={schedule.id}for single backup schedule export. Thesize="sm"matches the other action buttons in this cluster, maintaining visual consistency.app/client/modules/volumes/routes/volumes.tsx (2)
3-3: LGTM!Import correctly added for the ExportDialog component.
133-139: LGTM!The header actions are properly grouped with consistent spacing. The ExportDialog is correctly configured with
entityType="volumes"for bulk export of all volumes.app/client/modules/repositories/routes/repository-details.tsx (2)
28-29: LGTM!Imports correctly added for Trash2 icon and ExportDialog component.
161-164: LGTM!The ExportDialog is correctly configured for single-repository export using the
nameprop. The Trash2 icon provides good visual feedback for the destructive delete action.app/client/modules/backups/routes/backups.tsx (2)
3-3: LGTM!Import correctly added for the ExportDialog component.
123-127: LGTM!The export card follows the same visual pattern as the "Create a backup job" card, providing a consistent grid layout. The
variant="card"prop correctly triggers the card-style trigger rendering.app/server/utils/crypto.ts (2)
8-10: LGTM!Clean utility function that correctly uses the existing
encryptionPrefixconstant. The type guard ensures safe string operations.
65-65: LGTM!Properly exported as part of the cryptoUtils object.
app/server/modules/lifecycle/config-export.controller.ts (7)
1-15: LGTM!Imports and dependencies are well-organized, importing only what's needed from the database schema and utilities.
38-44: LGTM!Simple and effective helper for excluding keys from objects.
66-73: LGTM!Good default values: IDs and timestamps included by default, runtime state excluded by default, and secrets mode defaults to "exclude" for secure-by-default behavior.
134-148: LGTM!Volume fetch logic correctly handles filtering by ID (with NaN check) and name, with appropriate 400/404 error responses.
192-218: LGTM!The transformation correctly resolves foreign keys to names for IaC-friendly output. Notification assignments are properly filtered per schedule.
236-293: LGTM!The full export endpoint properly parallelizes database queries and conditionally includes sensitive data (recovery key, password hash) based on explicit opt-in flags. Good security posture with secure defaults.
328-355: LGTM!The backups export endpoint correctly fetches all reference data to resolve names and only supports ID filtering as documented.
app/client/components/export-dialog.tsx (3)
1-35: LGTM!The imports and type definitions are well-structured. The
SecretsModeandExportOptionstypes provide clear, flexible configuration options for the export functionality.
207-334: LGTM!The dialog UI is well-structured with proper accessibility (labels, checkboxes), clear user guidance, and appropriate security warnings for sensitive data exports. The conditional rendering for secrets handling and full export options is clean and maintainable.
108-113: Verify backup schedule identification pattern.The
getFilenamefor backups (line 112) only checksopts.idunlike other entity types which check bothopts.idandopts.name. Industry-standard backup schedule configurations typically include both a backend identifier and a human-readable name field. Confirm whether backup schedules in this system also have anamefield that should be used as a fallback in the filename generation.
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive configuration export functionality to ZeroByte, enabling users to export their configuration as JSON files for backup, migration, or Infrastructure as Code workflows. The implementation provides flexible export options including database IDs, timestamps, runtime state, and various secrets handling modes.
Key Changes
- New export API endpoints at
/api/v1/config/export/*with support for filtering and flexible export options - Shared
ExportDialogReact component used across the UI for consistent export experience - Export functionality added to volumes, repositories, notifications, and backup schedules pages
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 21 comments.
Show a summary per file
| File | Description |
|---|---|
| app/server/utils/crypto.ts | Adds isEncrypted utility function to detect encrypted fields |
| app/server/modules/lifecycle/config-export.controller.ts | New controller implementing export endpoints with filtering, secrets processing, and flexible options |
| app/server/index.ts | Registers the new config export controller routes |
| app/client/components/export-dialog.tsx | Reusable dialog component for export with configurable options |
| app/client/modules/volumes/routes/volumes.tsx | Adds export button to volumes list page |
| app/client/modules/volumes/routes/volume-details.tsx | Adds export button to volume detail page |
| app/client/modules/repositories/routes/repositories.tsx | Adds export button to repositories list page |
| app/client/modules/repositories/routes/repository-details.tsx | Adds export button to repository detail page |
| app/client/modules/notifications/routes/notifications.tsx | Adds export button to notifications list page |
| app/client/modules/notifications/routes/notification-details.tsx | Adds export button to notification detail page |
| app/client/modules/backups/routes/backups.tsx | Adds export card to backup schedules page |
| app/client/modules/backups/components/schedule-summary.tsx | Adds export button to individual schedule summary |
| app/client/modules/settings/routes/settings.tsx | Adds full config export button to settings page |
| README.md | Documents the new export functionality and available options |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
app/client/components/export-dialog.tsx (2)
264-287: High‑risk options (cleartext secrets, recovery key, password hash) may warrant extra confirmation (previously raised)The warnings for:
secretsMode === "cleartext"(plaintext secrets),includeRecoveryKey,includePasswordHashare clear and prominent, but there’s no secondary confirmation or re‑auth step before exporting highly sensitive data.
If the backend doesn’t already enforce an extra check, consider adding a lightweight confirmation flow (e.g., an additional modal or “type YES to confirm” step) when any of these options are enabled, to reduce the chance of accidental disclosure.
Also applies to: 291-320
193-207: Fix keyboard accessibility forcardvariant trigger (previously raised)The
cardvariant still uses a non‑interactive<div>withcursor-pointer, which is not focusable or activatable via keyboard. Users relying on keyboard or assistive tech can’t open the dialog through this trigger.Switching it to an actual
<button>(styled to look like the card) will fix this:- const defaultTrigger = - variant === "card" ? ( - <div className="flex flex-col items-center justify-center gap-2 cursor-pointer h-full w-full"> + const defaultTrigger = + variant === "card" ? ( + <button + type="button" + className="flex flex-col items-center justify-center gap-2 cursor-pointer h-full w-full border-0 bg-transparent p-0 hover:opacity-80 transition-opacity" + > <Download className="h-8 w-8 text-muted-foreground" /> <span className="text-sm font-medium text-muted-foreground"> {triggerLabel ?? `Export ${isSingleItem ? "config" : "configs"}`} </span> - </div> + </button> ) : (
🧹 Nitpick comments (4)
app/client/components/export-dialog.tsx (4)
81-121: Copy forfullexport is slightly awkward; consider special‑casing textThe
fullconfig useslabelPlural: "Full Config", which leads to UI strings likeExport all full config configurations.whenisSingleItemis false. Functionally fine, but the wording is a bit clunky.You might special‑case the description for
entityType === "full"(e.g., “Export the complete ZeroByte configuration.”) instead of relying on the generic{labelPlural.toLowerCase()} configurationsphrasing.Also applies to: 213-218
132-141: Tighten typings fortriggerand clarify behavior when custom trigger is passedTwo small points here:
trigger?: React.ReactNoderelies onReactbeing available as a type namespace, but this file only importsuseStatefrom"react". To avoid any TS namespace issues (especially with React 19 typings), it’s safer to importReactNodeexplicitly and use it directly:-import { useState } from "react"; +import { useState, type ReactNode } from "react"; @@ - trigger?: React.ReactNode; + trigger?: ReactNode;
- When
triggeris provided,variant,size,triggerLabel, andshowIconare effectively ignored for the trigger itself. That’s reasonable, but it’s worth documenting in the prop JSDoc or component description so downstream callers don’t expect those props to apply when passing a custom trigger.
324-332: Optional: Disable primary action while exporting to avoid duplicate requests
loading={isExporting}presumably shows a busy state, but depending on howButtonis implemented, the button might still be clickable and issue multiple export requests.If
Buttondoesn’t already disable itself whenloadingis true, consider explicitly disabling it:- <Button onClick={handleExport} loading={isExporting}> + <Button onClick={handleExport} loading={isExporting} disabled={isExporting}>This is a minor UX safeguard against double‑submits.
338-349: Remove unusedconfiginExportCard(and optionally ensure the wrapper gets used)
configis declared but never used:const config = exportConfigs[entityType];This will trigger lint/TS warnings. It can be safely removed:
-export function ExportCard({ entityType, ...props }: Omit<ExportDialogProps, "variant" | "trigger">) { - const config = exportConfigs[entityType]; - - return ( +export function ExportCard({ entityType, ...props }: Omit<ExportDialogProps, "variant" | "trigger">) { + return ( <ExportDialogAlso, if
ExportCardis still not referenced anywhere in the app, consider either using it in the places where you renderExportDialogwithvariant="card"or deleting it to avoid dead code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/components/export-dialog.tsx(1 hunks)
🔇 Additional comments (2)
app/client/components/export-dialog.tsx (2)
37-70: Export helpers are sound; error propagation looks good
downloadAsJsoncorrectly creates and revokes an object URL, andexportFromApinow surfaces server error text viaError(errorText || \HTTP ${res.status}`)`, which should produce much more informative toasts without overcomplicating status handling. No changes needed here.
143-191: Export options wiring and toast handling look consistentState wiring and the
handleExportcall are coherent:
- Options are mapped correctly into the
ExportOptionsobject.includeRecoveryKey/includePasswordHashare only sent forentityType === "full", andsecretsModeis omitted completely whenhasSecretsis false.- Success and error toasts use the entity label and error message in a useful way, and
isExportingis reset in afinallyblock.This block looks solid as‑is.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
app/client/components/export-dialog.tsx (3)
442-442: Remove unused variable.The
configvariable is extracted but never used in the component.Apply this diff:
export function ExportCard({ entityType, ...props }: Omit<ExportDialogProps, "variant" | "trigger">) { - const config = exportConfigs[entityType]; - return (
441-452: Verify usage of ExportCard component.Previous review comments suggest this wrapper may be unused, with consumers instead calling
ExportDialogwithvariant="card"directly. If the component isn't referenced elsewhere in the codebase, consider removing it to reduce maintenance burden.Run this script to check for usage:
#!/bin/bash # Description: Find all imports and usages of ExportCard in the codebase. # Search for ExportCard imports echo "=== ExportCard imports ===" rg -n 'import.*ExportCard' --type=ts --type=tsx # Search for ExportCard usage (component invocation) echo -e "\n=== ExportCard usage ===" rg -n '<ExportCard' --type=ts --type=tsx # Search for ExportDialog with variant="card" (alternative pattern) echo -e "\n=== ExportDialog with card variant ===" rg -n 'ExportDialog.*variant="card"' --type=ts --type=tsx
248-261: Card variant still lacks keyboard accessibility.The card variant continues to use a non-interactive
<div>element (lines 250-255), preventing keyboard users from activating the export dialog. This accessibility issue was flagged in previous reviews but remains unresolved.Apply this diff to make the card variant keyboard-accessible:
const defaultTrigger = variant === "card" ? ( - <div className="flex flex-col items-center justify-center gap-2 cursor-pointer h-full w-full"> + <button + type="button" + className="flex flex-col items-center justify-center gap-2 cursor-pointer h-full w-full border-0 bg-transparent p-0 hover:opacity-80 transition-opacity" + > <Download className="h-8 w-8 text-muted-foreground" /> <span className="text-sm font-medium text-muted-foreground"> {triggerLabel ?? `Export ${isSingleItem ? "config" : "configs"}`} </span> - </div> + </button> ) : (
🧹 Nitpick comments (2)
app/client/components/export-dialog.tsx (1)
217-238: Distinguish network errors from authentication failures.The error handler (lines 233-234) catches all exceptions and displays "Incorrect password," which can mislead users when the actual failure is a network error or API unavailability.
Apply this diff to provide clearer feedback:
- } catch { - toast.error("Incorrect password"); + } catch (err) { + const message = err instanceof TypeError + ? "Network error. Please check your connection and try again." + : "Incorrect password"; + toast.error(message); } finally {app/server/modules/auth/auth.dto.ts (1)
160-174: Consider documenting error responses in OpenAPI schema.The route descriptor only documents the 200 success response but omits the 401 error responses that can occur when:
- The session is invalid or missing
- The provided password is incorrect
Adding these error response definitions would improve API documentation completeness.
Note: This pattern is consistent with other DTOs in this file (e.g.,
loginDto,getMeDto,changePasswordDto), so this is an optional improvement that could be applied file-wide if desired.export const verifyPasswordDto = describeRoute({ description: "Verify current user password for re-authentication", operationId: "verifyPassword", tags: ["Auth"], responses: { description: "Password verification result", content: { "application/json": { schema: resolver(verifyPasswordResponseSchema), }, }, }, + 401: { + description: "Not authenticated or incorrect password", + content: { + "application/json": { + schema: resolver(verifyPasswordResponseSchema), + }, + }, + }, }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(1 hunks)app/client/components/export-dialog.tsx(1 hunks)app/server/modules/auth/auth.controller.ts(2 hunks)app/server/modules/auth/auth.dto.ts(1 hunks)app/server/modules/auth/auth.service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
🧰 Additional context used
🧬 Code graph analysis (3)
app/server/modules/auth/auth.controller.ts (2)
app/server/modules/auth/auth.dto.ts (3)
verifyPasswordDto(160-174)verifyPasswordBodySchema(151-153)VerifyPasswordDto(176-176)app/server/modules/auth/auth.service.ts (1)
authService(192-192)
app/server/modules/auth/auth.service.ts (2)
app/server/db/db.ts (1)
db(13-13)app/server/db/schema.ts (1)
usersTable(28-35)
app/client/components/export-dialog.tsx (1)
app/server/modules/auth/auth.service.ts (1)
verifyPassword(181-189)
🔇 Additional comments (6)
app/server/modules/auth/auth.service.ts (1)
178-189: LGTM! Secure implementation with appropriate error handling.The method correctly:
- Uses
Bun.password.verifyfor timing-safe password comparison- Returns
falsefor both "user not found" and "incorrect password" cases, preventing user enumeration- Follows a pattern suitable for re-authentication without exposing detailed error information
Note: This method returns a boolean instead of throwing errors (unlike
loginandchangePassword), which is appropriate for the re-authentication use case where the caller needs to distinguish success/failure without exception handling overhead.app/server/modules/auth/auth.controller.ts (3)
15-16: LGTM! Correct imports for the new verification feature.The imports properly include the necessary schema, DTO descriptor, and types for the new
/verify-passwordendpoint.Also applies to: 23-23
145-167: Implementation follows secure authentication patterns.The endpoint correctly:
- Validates the session before processing the request
- Deletes the cookie if the session is invalid
- Uses consistent 401 responses for both authentication failures and incorrect passwords (prevents information leakage)
- Delegates password verification to the service layer
- Follows the same pattern as
/change-passwordfor session handling
145-167: Rate limiting should be verified for password verification and change endpoints.The
/verify-passwordendpoint accepts password input without apparent rate limiting, creating a brute force vulnerability if an attacker compromises a session. The same concern applies to/change-password(lines 122-143).Verify whether rate limiting is implemented at the middleware or application layer for these endpoints. If not present, implement rate limiting to restrict verification attempts per session or IP within a defined time window.
app/server/modules/auth/auth.dto.ts (2)
151-153: LGTM! Appropriate validation for password verification.The schema correctly requires a non-empty password (
string>0) without enforcing a minimum length. This is appropriate since this endpoint verifies an existing password rather than setting a new one, unlikeregisterBodySchemaandchangePasswordBodySchemawhich enforcestring>7for new passwords.
155-158: LGTM! Proper schema and type definitions.The response schema and type exports are correctly structured and follow the established patterns in the codebase.
Also applies to: 176-181
TODO: Fix trash icon in separate PR Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
TODO: Fix Trash icon in different PR Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/modules/notifications/routes/notification-details.tsx(2 hunks)app/client/modules/repositories/routes/repository-details.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/client/modules/notifications/routes/notification-details.tsx (1)
app/client/components/export-dialog.tsx (1)
ExportDialog(153-439)
app/client/modules/repositories/routes/repository-details.tsx (1)
app/client/components/export-dialog.tsx (1)
ExportDialog(153-439)
🔇 Additional comments (3)
app/client/modules/repositories/routes/repository-details.tsx (2)
28-29: Imports for Loader2 and ExportDialog look appropriateThe added imports cleanly wire in the shared
ExportDialogcomponent and keep existingLoader2usage intact; no issues from a correctness or style perspective.
161-161: ExportDialog integration on repository details page is soundThe dialog is placed logically alongside existing actions, and the props (
entityType="repositories",name={data.name}, customtriggerLabel) match the ExportDialog contract and intended UX for single‑repository exports.app/client/modules/notifications/routes/notification-details.tsx (1)
27-28: Imports for icons and ExportDialog look consistentThe new imports for
Bell,TestTube2, andExportDialogare aligned with existing patterns in this module and other routes; no issues from a composition or dependency standpoint.
app/client/modules/notifications/routes/notification-details.tsx
Outdated
Show resolved
Hide resolved
More verbose error handling Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
More verbose error handling Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
app/server/modules/lifecycle/config-export.controller.ts (2)
267-274: Add audit logging for recovery key exports.Exporting the recovery key is a critical security operation that should be logged for audit purposes. Currently, only failures are logged (line 272), but successful exports are not tracked.
Add audit logging when the recovery key is successfully exported:
let recoveryKey: string | undefined; if (includeRecoveryKey) { try { recoveryKey = await Bun.file(RESTIC_PASS_FILE).text(); + logger.info("Recovery key exported in full config export"); } catch { logger.warn("Could not read recovery key file"); } }
70-70: Validate secretsMode parameter to prevent silent bugs.The secretsMode parameter is cast without runtime validation. Invalid values (e.g., "invalid") would bypass TypeScript's type checking and could cause unexpected behavior in
processSecrets.Add validation to ensure only valid values are accepted:
- const secretsMode = (c.req.query("secretsMode") as SecretsMode) || "exclude"; + const secretsModeRaw = c.req.query("secretsMode"); + const validModes: SecretsMode[] = ["exclude", "encrypted", "cleartext"]; + const secretsMode: SecretsMode = secretsModeRaw && validModes.includes(secretsModeRaw as SecretsMode) + ? (secretsModeRaw as SecretsMode) + : "exclude";
🧹 Nitpick comments (4)
app/server/modules/lifecycle/config-export.controller.ts (4)
101-103: Log decryption failures for better debugging.When decryption fails, the field is silently deleted without any indication of which field failed or why. This makes debugging difficult.
Add logging to track decryption failures:
try { result[key] = await cryptoUtils.decrypt(value); - } catch { + } catch (err) { + logger.warn(`Failed to decrypt field "${key}": ${err instanceof Error ? err.message : String(err)}`); delete result[key]; }
105-107: Handle arrays of objects in processSecrets.The function processes nested objects but doesn't handle arrays of objects. If configuration contains arrays with encrypted secrets (e.g., array of credential objects), those secrets won't be processed.
Add array handling to ensure all nested secrets are processed:
+ } else if (Array.isArray(value)) { + result[key] = await Promise.all( + value.map((item) => + item && typeof item === "object" + ? processSecrets(item as Record<string, unknown>, secretsMode) + : item + ) + ); } else if (value && typeof value === "object" && !Array.isArray(value)) { result[key] = await processSecrets(value as Record<string, unknown>, secretsMode); }
260-260: Clarify security implications of volume exports.The TODO comment mentions a future PR but doesn't explicitly warn about current security limitations. Consider making the warning more explicit about potential cleartext exposure of sensitive data.
Apply this diff to clarify the security limitation:
- // TODO: Volumes will have encrypted secrets (e.g., SMB/NFS credentials) in a future PR + // NOTE: Volume exports may currently include sensitive data (e.g., SMB/NFS credentials) + // in cleartext depending on secretsMode. Handle exported files securely. + // TODO: Encrypt volume secrets in the database (tracked in separate PR)
350-350: Consider consistent naming between endpoint and response.The endpoint path is
/export/backupsbut the response key isbackupSchedules. This inconsistency could confuse API consumers.For consistency, consider either:
- Renaming the endpoint to
/export/backup-schedules, or- Using
backupsas the response keyIf you prefer to keep the current naming, this is acceptable but consider documenting the choice.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/server/modules/lifecycle/config-export.controller.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/server/modules/lifecycle/config-export.controller.ts (5)
app/client/components/export-dialog.tsx (1)
SecretsMode(34-34)app/server/utils/crypto.ts (1)
cryptoUtils(62-66)app/server/db/schema.ts (6)
volumesTable(10-22)repositoriesTable(50-62)notificationDestinationsTable(112-120)backupSchedulesTable(68-95)backupScheduleNotificationsTable(129-144)usersTable(28-35)app/server/db/db.ts (1)
db(13-13)app/server/core/constants.ts (1)
RESTIC_PASS_FILE(5-5)
remove unused variable Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…vy/zerobyte into config-export-feature
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
app/server/modules/lifecycle/config-export.controller.ts (7)
70-70: ValidatesecretsModeparameter.The query parameter is cast to
SecretsModewithout runtime validation. Invalid values (e.g., "invalid") would pass through and potentially cause unexpected behavior inprocessSecrets.Based on past review comments, consider validating against allowed values:
- const secretsMode = (c.req.query("secretsMode") as SecretsMode) || "exclude"; + const secretsModeRaw = c.req.query("secretsMode"); + const allowedModes: SecretsMode[] = ["exclude", "encrypted", "cleartext"]; + const secretsMode: SecretsMode = + secretsModeRaw && allowedModes.includes(secretsModeRaw as SecretsMode) + ? (secretsModeRaw as SecretsMode) + : "exclude";
101-103: Log decryption failures.When decryption fails in cleartext mode, the field is silently deleted. As noted in past review comments, consider logging which fields failed to decrypt:
} else if (secretsMode === "cleartext") { try { result[key] = await cryptoUtils.decrypt(value); - } catch { + } catch (err) { + logger.warn(`Failed to decrypt field "${key}": ${err instanceof Error ? err.message : String(err)}`); delete result[key]; } }
105-108: Handle arrays containing objects with secrets.As flagged in past review comments,
processSecretsdoesn't recursively process arrays. If config contains arrays of objects with encrypted fields, those won't be processed according tosecretsMode:} else if (value && typeof value === "object" && !Array.isArray(value)) { result[key] = await processSecrets(value as Record<string, unknown>, secretsMode); + } else if (Array.isArray(value)) { + result[key] = await Promise.all( + value.map((item) => + item && typeof item === "object" + ? processSecrets(item as Record<string, unknown>, secretsMode) + : item + ) + ); }
260-260: Clarify volume secret encryption status.The TODO comments reference "a future PR" for volume secret encryption, but as noted in past review comments, it's unclear what the current security implications are. Consider clarifying whether sensitive data (SMB/NFS credentials) is currently stored in cleartext:
- // TODO: Volumes will have encrypted secrets (e.g., SMB/NFS credentials) in a future PR + // NOTE: Volume secrets (SMB/NFS credentials) are not yet encrypted in the database. + // They will be encrypted in a future PR. Handle exported configs with care.Also applies to: 299-299
267-274: Add audit logging for recovery key export.As flagged in past review comments, exporting the recovery key is a critical security operation that should be logged for audit purposes:
let recoveryKey: string | undefined; if (includeRecoveryKey) { try { + logger.info("Recovery key export requested", { username: c.get("user")?.username }); recoveryKey = await Bun.file(RESTIC_PASS_FILE).text(); } catch { logger.warn("Could not read recovery key file"); } }
314-314: Use detailed error messages consistently.As flagged in past review comments, repositories, notifications, and backups endpoints return generic error messages while the volumes endpoint (line 303) includes detailed errors. Apply the same detailed error pattern for consistency and better debugging:
} catch (err) { logger.error(`Repositories export failed: ${err instanceof Error ? err.message : String(err)}`); - return c.json({ error: "Failed to export repositories" }, 500); + return c.json({ error: `Failed to export repositories: ${err instanceof Error ? err.message : String(err)}` }, 500); }Apply similar changes to lines 325 and 353.
Also applies to: 325-325, 353-353
350-350: Align endpoint path with response key.As noted in past review comments, the endpoint
/export/backupsreturns{ backupSchedules }, creating a naming inconsistency. Consider renaming the endpoint to/export/backup-schedulesor the response key tobackups.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/components/export-dialog.tsx(1 hunks)app/server/modules/lifecycle/config-export.controller.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/components/export-dialog.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
app/server/modules/lifecycle/config-export.controller.ts (4)
app/server/utils/crypto.ts (1)
cryptoUtils(62-66)app/server/db/schema.ts (6)
volumesTable(10-22)repositoriesTable(50-62)notificationDestinationsTable(112-120)backupSchedulesTable(68-95)backupScheduleNotificationsTable(129-144)usersTable(28-35)app/server/db/db.ts (1)
db(13-13)app/server/core/constants.ts (1)
RESTIC_PASS_FILE(5-5)
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…vy/zerobyte into config-export-feature
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
app/server/modules/lifecycle/config-export.controller.ts (5)
320-320: Ensure consistent error messages across endpoints.Lines 320, 331, and 359 use generic error messages that don't include the underlying error details. This is inconsistent with the improved error handling in other endpoints (e.g., line 297, 309) and makes debugging harder.
Apply this diff to improve error messages:
Line 320:
- return c.json({ error: "Failed to export repositories" }, 500); + return c.json({ error: `Failed to export repositories: ${err instanceof Error ? err.message : String(err)}` }, 500);Line 331:
- return c.json({ error: "Failed to export notification destinations" }, 500); + return c.json({ error: `Failed to export notification destinations: ${err instanceof Error ? err.message : String(err)}` }, 500);Line 359:
- return c.json({ error: "Failed to export backup schedules" }, 500); + return c.json({ error: `Failed to export backup schedules: ${err instanceof Error ? err.message : String(err)}` }, 500);Based on past review comments, these endpoints should provide detailed error information like the other routes.
Also applies to: 331-331, 359-359
66-73: Validate secretsMode parameter.Line 70 accepts any string for
secretsModeand silently defaults to "exclude" for invalid values. This can lead to unexpected behavior if a user mistypes the mode.Apply this diff to add validation:
function parseExportParams(c: Context): ExportParams { const includeIds = c.req.query("includeIds") !== "false"; const includeTimestamps = c.req.query("includeTimestamps") !== "false"; const includeRuntimeState = c.req.query("includeRuntimeState") === "true"; - const secretsMode = (c.req.query("secretsMode") as SecretsMode) || "exclude"; + const secretsModeRaw = c.req.query("secretsMode"); + const allowedModes: SecretsMode[] = ["exclude", "encrypted", "cleartext"]; + if (secretsModeRaw && !allowedModes.includes(secretsModeRaw as SecretsMode)) { + throw new Error(`Invalid secretsMode: ${secretsModeRaw}. Must be one of: ${allowedModes.join(", ")}`); + } + const secretsMode: SecretsMode = (secretsModeRaw as SecretsMode) || "exclude"; const excludeKeys = getExcludeKeys(includeIds, includeTimestamps, includeRuntimeState); return { includeIds, includeTimestamps, secretsMode, excludeKeys }; }Based on past review comments, this validation was previously suggested.
84-112: Add array handling to processSecrets.The function recursively processes nested objects (line 106-108) but doesn't handle arrays containing objects with encrypted fields. If configuration includes arrays of objects with secrets, those secrets won't be processed.
Apply this diff to handle arrays:
for (const [key, value] of Object.entries(result)) { if (typeof value === "string" && cryptoUtils.isEncrypted(value)) { if (secretsMode === "exclude") { delete result[key]; } else if (secretsMode === "cleartext") { try { result[key] = await cryptoUtils.decrypt(value); } catch (err) { logger.warn(`Failed to decrypt field "${key}": ${err instanceof Error ? err.message : String(err)}`); delete result[key]; } } + } else if (Array.isArray(value)) { + result[key] = await Promise.all( + value.map((item) => + item && typeof item === "object" + ? processSecrets(item as Record<string, unknown>, secretsMode) + : item + ) + ); } else if (value && typeof value === "object" && !Array.isArray(value)) { result[key] = await processSecrets(value as Record<string, unknown>, secretsMode); } }Based on past review comments, this enhancement was previously suggested to ensure comprehensive secret handling.
198-224: CRITICAL: Backup schedules bypass secret processing.The
transformBackupSchedulesfunction appliesomitKeys(line 218) but never callsprocessSecrets, unlike other entity types that go throughexportEntities→exportEntity→processSecrets. This creates a security inconsistency where backup schedule secrets are not handled according to the specifiedsecretsMode.Apply this diff to ensure consistent secret handling:
function transformBackupSchedules( schedules: typeof backupSchedulesTable.$inferSelect[], scheduleNotifications: typeof backupScheduleNotificationsTable.$inferSelect[], volumeMap: Map<number, string>, repoMap: Map<string, string>, notificationMap: Map<number, string>, params: ExportParams -) { - return schedules.map((schedule) => { +): Promise<Record<string, unknown>[]> { + return Promise.all(schedules.map(async (schedule) => { const assignments = scheduleNotifications .filter((sn) => sn.scheduleId === schedule.id) .map((sn) => ({ ...(params.includeIds ? { destinationId: sn.destinationId } : {}), name: notificationMap.get(sn.destinationId) ?? null, notifyOnStart: sn.notifyOnStart, notifyOnSuccess: sn.notifyOnSuccess, notifyOnFailure: sn.notifyOnFailure, })); - return { + const transformed = { ...omitKeys(schedule as Record<string, unknown>, params.excludeKeys), volume: volumeMap.get(schedule.volumeId) ?? null, repository: repoMap.get(schedule.repositoryId) ?? null, notifications: assignments, }; + return processSecrets(transformed, params.secretsMode); - }); + })); }Update the call sites at lines 262 and 352 to await the result:
- const backupSchedules = transformBackupSchedules( + const backupSchedules = await transformBackupSchedules( backupSchedulesRaw, scheduleNotifications, volumeMap, repoMap, notificationMap, params );And:
- const backupSchedules = transformBackupSchedules( + const backupSchedules = await transformBackupSchedules( result.data, scheduleNotifications, volumeMap, repoMap, notificationMap, params );Based on past review comments, this security issue was previously identified and must be addressed before merge.
273-280: Add audit logging for recovery key export.Exporting the recovery key is a critical security operation that should be logged for audit purposes. Currently, the key is read and included in the response without any audit trail.
Apply this diff to add audit logging:
let recoveryKey: string | undefined; if (includeRecoveryKey) { try { recoveryKey = await Bun.file(RESTIC_PASS_FILE).text(); + logger.warn("Recovery key exported in config export"); } catch { logger.warn("Could not read recovery key file"); } }Based on past review comments, this security concern was previously raised and should be addressed.
🧹 Nitpick comments (2)
app/client/modules/backups/components/schedule-summary.tsx (1)
129-129: ExportDialog wiring for backup schedules looks correct
entityType="backup-schedules"withid={schedule.id}fits the single-schedule export use case and integrates cleanly into the existing action row. Behavior should match the shared ExportDialog semantics without impacting other controls.If you want perfectly consistent full‑width buttons on small screens, consider passing a custom
triggerButton with the sameclassName="w-full sm:w-auto"as the neighbors, but that’s purely cosmetic.app/client/modules/backups/routes/backups.tsx (1)
123-127: Export export-card integration is sound; consider avoiding potential double-card wrappingAdding an export card to the grid with
entityType="backup-schedules"provides a clear, global entry point for exporting schedule configs and fits the existing grid layout.Depending on how
ExportDialogrenders whenvariant="card", the outer<Card>wrapper may be redundant (you might end up with a card inside a card). Ifvariant="card"already renders a full card‑like surface, you could simplify this to render<ExportDialog entityType="backup-schedules" variant="card" />directly as a grid item, or pass a customtriggerinto a single Card if you need tighter control over the layout.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/client/components/export-dialog.tsx(1 hunks)app/client/modules/backups/components/schedule-summary.tsx(2 hunks)app/client/modules/backups/routes/backups.tsx(2 hunks)app/client/modules/notifications/routes/notification-details.tsx(2 hunks)app/client/modules/notifications/routes/notifications.tsx(2 hunks)app/server/modules/lifecycle/config-export.controller.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/client/modules/notifications/routes/notifications.tsx
- app/client/components/export-dialog.tsx
- app/client/modules/notifications/routes/notification-details.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
app/client/modules/backups/components/schedule-summary.tsx (1)
app/client/components/export-dialog.tsx (1)
ExportDialog(153-442)
app/server/modules/lifecycle/config-export.controller.ts (5)
app/client/components/export-dialog.tsx (1)
SecretsMode(34-34)app/server/utils/crypto.ts (1)
cryptoUtils(62-66)app/server/db/schema.ts (6)
volumesTable(10-22)repositoriesTable(50-62)notificationDestinationsTable(112-120)backupSchedulesTable(68-95)backupScheduleNotificationsTable(129-144)usersTable(28-35)app/server/db/db.ts (1)
db(13-13)app/server/core/constants.ts (1)
RESTIC_PASS_FILE(5-5)
app/client/modules/backups/routes/backups.tsx (1)
app/client/components/export-dialog.tsx (1)
ExportDialog(153-442)
🔇 Additional comments (2)
app/client/modules/backups/components/schedule-summary.tsx (1)
2-2: Import ofExportDialogis consistent and correctImport path and usage are aligned with other local component imports; no issues here.
app/client/modules/backups/routes/backups.tsx (1)
3-3: ExportDialog import is appropriateThe added import cleanly matches the new usage below; no concerns.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
README.md (1)
236-236: Remove "on import" phrasing from "Keep encrypted" description.The current text references decryption requirements "on import," but the export feature is documented separately from import (which is handled in PR #93). The description should focus on export behavior only.
Suggested revision for README.md line 236:
- - **Keep encrypted** - Export secrets in encrypted form (requires the same recovery key to decrypt on import) + - **Keep encrypted** - Export secrets in encrypted form (encrypted secrets use the recovery key)This clarifies that the recovery key is the encryption mechanism without implying import scope.
🧹 Nitpick comments (1)
README.md (1)
245-245: Strengthen the sensitive data note to explicitly surface volume secrets limitation.The note at line 245 covers the general case ("Some sensitive data from earlier versions may not be encrypted") and nested config handling, which aligns with the learnings. However, it could be more explicit about volume secrets, which are a known gap per the implementation learnings.
Suggest enhancing the note:
> **Sensitive data handling**: Some sensitive data from earlier versions may not be encrypted in the database. Additionally, nested configuration objects within config fields are exported as-is and not processed separately. Review exported data carefully before sharing, especially when using the "Decrypt" secrets option. + > + > **Known limitation**: Volume secrets are not yet encrypted in the database and may not be properly handled by the secrets options above. Manual review of exported volume credentials is recommended.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: README.md:0-0
Timestamp: 2025-12-17T15:54:21.205Z
Learning: For ZeroByte PR #92, the export configuration feature is separate from import functionality—import is out of scope and handled in a separate PR #93. Documentation for the export feature should focus only on export behavior and not include import-related caveats.
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: app/client/components/export-dialog.tsx:189-196
Timestamp: 2025-12-01T22:25:20.502Z
Learning: Volume secrets in ZeroByte are not yet encrypted in the database (stored as cleartext) and are excluded from the export dialog's secrets handling. The export logic relies on the `isEncrypted()` helper function that checks for the "encv1" encryption prefix, so cleartext volume secrets cannot be automatically detected for exclusion. Once volume secret encryption is implemented, the export logic for volumes will need to be completed to enable proper secrets handling options.
📚 Learning: 2025-12-17T15:54:21.205Z
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: README.md:0-0
Timestamp: 2025-12-17T15:54:21.205Z
Learning: For ZeroByte PR #92, the export configuration feature is separate from import functionality—import is out of scope and handled in a separate PR #93. Documentation for the export feature should focus only on export behavior and not include import-related caveats.
Applied to files:
README.md
📚 Learning: 2025-12-01T22:25:20.502Z
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: app/client/components/export-dialog.tsx:189-196
Timestamp: 2025-12-01T22:25:20.502Z
Learning: Volume secrets in ZeroByte are not yet encrypted in the database (stored as cleartext) and are excluded from the export dialog's secrets handling. The export logic relies on the `isEncrypted()` helper function that checks for the "encv1" encryption prefix, so cleartext volume secrets cannot be automatically detected for exclusion. Once volume secret encryption is implemented, the export logic for volumes will need to be completed to enable proper secrets handling options.
Applied to files:
README.md
📚 Learning: 2025-12-17T15:54:21.205Z
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: README.md:0-0
Timestamp: 2025-12-17T15:54:21.205Z
Learning: In ZeroByte, historically some sensitive data from past versions might not be encrypted in the database, and nested configuration is sometimes present in config fields which will be exported as-is without special handling by the export functionality.
Applied to files:
README.md
…vy/zerobyte into config-export-feature
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
app/server/utils/crypto.ts (1)
96-96: Critical: Inconsistent secret trimming breaks encrypt/decrypt round-trip.Line 96 (
encrypt) reads the secret without trimming, while line 118 (decrypt) trims the secret. IfRESTIC_PASS_FILEcontains trailing whitespace (e.g., a newline), different keys will be derived, causing decryption to fail.🔎 Proposed fix
- const secret = await Bun.file(RESTIC_PASS_FILE).text(); + const secret = (await Bun.file(RESTIC_PASS_FILE).text()).trim();
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.mdapp/client/api-client/@tanstack/react-query.gen.tsapp/client/api-client/sdk.gen.tsapp/client/api-client/types.gen.tsapp/server/app.tsapp/server/utils/crypto.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/client/api-client/types.gen.ts
- app/client/api-client/sdk.gen.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for code formatting and linting with tabs (not spaces), 120 character line width, and double quotes.
Files:
app/server/app.tsapp/client/api-client/@tanstack/react-query.gen.tsapp/server/utils/crypto.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not auto-organize imports in Biome - imports organization is disabled.
Files:
app/server/app.tsapp/client/api-client/@tanstack/react-query.gen.tsapp/server/utils/crypto.ts
app/server/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
All TypeScript imports must include file extensions when targeting Node/Bun environments (using
"type": "module").
Files:
app/server/app.tsapp/server/utils/crypto.ts
app/client/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
React client must use TanStack Query for server state management and auto-generated API client from OpenAPI spec.
Files:
app/client/api-client/@tanstack/react-query.gen.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: README.md:0-0
Timestamp: 2025-12-17T15:54:44.265Z
Learning: For ZeroByte PR #92, the export configuration feature is separate from import functionality—import is out of scope and handled in a separate PR #93. Documentation for the export feature should focus only on export behavior and not include import-related caveats.
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: app/client/components/export-dialog.tsx:189-196
Timestamp: 2025-12-01T22:25:20.502Z
Learning: Volume secrets in ZeroByte are not yet encrypted in the database (stored as cleartext) and are excluded from the export dialog's secrets handling. The export logic relies on the `isEncrypted()` helper function that checks for the "encv1" encryption prefix, so cleartext volume secrets cannot be automatically detected for exclusion. Once volume secret encryption is implemented, the export logic for volumes will need to be completed to enable proper secrets handling options.
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: README.md:0-0
Timestamp: 2025-12-17T15:54:44.265Z
Learning: In ZeroByte, historically some sensitive data from past versions might not be encrypted in the database, and nested configuration is sometimes present in config fields which will be exported as-is without special handling by the export functionality.
📚 Learning: 2025-12-19T18:25:52.583Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-19T18:25:52.583Z
Learning: Applies to app/client/modules/*/routes/**/*.{ts,tsx} : Routes should be organized in feature modules at `app/client/modules/*/routes/`.
Applied to files:
app/server/app.ts
📚 Learning: 2025-12-19T18:25:52.583Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-19T18:25:52.583Z
Learning: Applies to app/server/modules/*/*.ts : Each server module should follow a controller → service → database pattern.
Applied to files:
app/server/app.ts
📚 Learning: 2025-12-19T18:25:52.583Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-19T18:25:52.583Z
Learning: Applies to app/server/index.ts : Server entry point initializes main API server on port 4096 (REST API + serves static frontend).
Applied to files:
app/server/app.ts
📚 Learning: 2025-12-19T18:25:52.583Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-19T18:25:52.583Z
Learning: Applies to app/server/**/*.{ts,tsx,js,jsx} : All TypeScript imports must include file extensions when targeting Node/Bun environments (using `"type": "module"`).
Applied to files:
app/server/app.ts
📚 Learning: 2025-12-19T18:25:52.583Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-19T18:25:52.583Z
Learning: Applies to app/client/**/*.{ts,tsx} : React client must use TanStack Query for server state management and auto-generated API client from OpenAPI spec.
Applied to files:
app/client/api-client/@tanstack/react-query.gen.ts
📚 Learning: 2025-12-19T18:25:52.583Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-19T18:25:52.583Z
Learning: Applies to app/server/utils/restic.ts : Restic password file must have 0600 permissions and never be exposed.
Applied to files:
app/server/utils/crypto.ts
📚 Learning: 2025-12-17T15:54:44.265Z
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: README.md:0-0
Timestamp: 2025-12-17T15:54:44.265Z
Learning: For ZeroByte PR #92, the export configuration feature is separate from import functionality—import is out of scope and handled in a separate PR #93. Documentation for the export feature should focus only on export behavior and not include import-related caveats.
Applied to files:
README.md
📚 Learning: 2025-12-01T22:25:20.502Z
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: app/client/components/export-dialog.tsx:189-196
Timestamp: 2025-12-01T22:25:20.502Z
Learning: Volume secrets in ZeroByte are not yet encrypted in the database (stored as cleartext) and are excluded from the export dialog's secrets handling. The export logic relies on the `isEncrypted()` helper function that checks for the "encv1" encryption prefix, so cleartext volume secrets cannot be automatically detected for exclusion. Once volume secret encryption is implemented, the export logic for volumes will need to be completed to enable proper secrets handling options.
Applied to files:
README.md
📚 Learning: 2025-12-17T15:54:44.265Z
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: README.md:0-0
Timestamp: 2025-12-17T15:54:44.265Z
Learning: In ZeroByte, historically some sensitive data from past versions might not be encrypted in the database, and nested configuration is sometimes present in config fields which will be exported as-is without special handling by the export functionality.
Applied to files:
README.md
🧬 Code graph analysis (2)
app/server/app.ts (1)
app/server/modules/lifecycle/config-export.controller.ts (1)
configExportController(178-259)
app/client/api-client/@tanstack/react-query.gen.ts (2)
app/client/api-client/sdk.gen.ts (2)
Options(7-19)exportFullConfig(403-410)app/client/api-client/types.gen.ts (3)
ExportFullConfigData(2911-2922)ExportFullConfigResponse(2964-2964)ExportFullConfigError(2939-2939)
🔇 Additional comments (3)
app/server/utils/crypto.ts (1)
189-190: LGTM: Public API exposure for export functionality.Exposing
isEncryptedanddecryptutilities supports the config export feature's secret handling requirements.app/client/api-client/@tanstack/react-query.gen.ts (2)
6-7: LGTM: Auto-generated import updates.The imports correctly include the new
exportFullConfigfunction and related types for the export mutation.
947-962: LGTM: Export mutation follows established pattern.The
exportFullConfigMutationimplementation is consistent with other mutations in this auto-generated file, correctly usingthrowOnError: trueand the appropriate error type.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
app/server/modules/lifecycle/config-export.controller.ts (1)
206-207: Move database queries to service layer.The controller is directly querying the database for
backupScheduleNotificationsandusers, which violates the controller → service → database pattern specified in the coding guidelines.Consider adding service methods:
- Add a method like
backupsService.listScheduleNotifications()for line 206- Add a method like
authService.listUsers()for line 207 (or create a dedicated users service if appropriate)This encapsulates data access logic in the service layer and keeps the controller focused on request handling and response assembly.
As per coding guidelines, server modules in
app/server/modules/*/*.tsshould follow a controller → service → database pattern.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/server/modules/lifecycle/config-export.controller.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for code formatting and linting with tabs (not spaces), 120 character line width, and double quotes.
Files:
app/server/modules/lifecycle/config-export.controller.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not auto-organize imports in Biome - imports organization is disabled.
Files:
app/server/modules/lifecycle/config-export.controller.ts
app/server/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
All TypeScript imports must include file extensions when targeting Node/Bun environments (using
"type": "module").
Files:
app/server/modules/lifecycle/config-export.controller.ts
app/server/modules/*/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Each server module should follow a controller → service → database pattern.
Files:
app/server/modules/lifecycle/config-export.controller.ts
app/server/{core/capabilities.ts,modules/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
The application must gracefully degrade when capabilities (rclone, system features) are unavailable.
Files:
app/server/modules/lifecycle/config-export.controller.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: README.md:0-0
Timestamp: 2025-12-17T15:54:44.265Z
Learning: For ZeroByte PR #92, the export configuration feature is separate from import functionality—import is out of scope and handled in a separate PR #93. Documentation for the export feature should focus only on export behavior and not include import-related caveats.
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: app/client/components/export-dialog.tsx:189-196
Timestamp: 2025-12-01T22:25:20.502Z
Learning: Volume secrets in ZeroByte are not yet encrypted in the database (stored as cleartext) and are excluded from the export dialog's secrets handling. The export logic relies on the `isEncrypted()` helper function that checks for the "encv1" encryption prefix, so cleartext volume secrets cannot be automatically detected for exclusion. Once volume secret encryption is implemented, the export logic for volumes will need to be completed to enable proper secrets handling options.
📚 Learning: 2025-12-17T15:54:44.265Z
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: README.md:0-0
Timestamp: 2025-12-17T15:54:44.265Z
Learning: For ZeroByte PR #92, the export configuration feature is separate from import functionality—import is out of scope and handled in a separate PR #93. Documentation for the export feature should focus only on export behavior and not include import-related caveats.
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-01T22:25:20.502Z
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: app/client/components/export-dialog.tsx:189-196
Timestamp: 2025-12-01T22:25:20.502Z
Learning: Volume secrets in ZeroByte are not yet encrypted in the database (stored as cleartext) and are excluded from the export dialog's secrets handling. The export logic relies on the `isEncrypted()` helper function that checks for the "encv1" encryption prefix, so cleartext volume secrets cannot be automatically detected for exclusion. Once volume secret encryption is implemented, the export logic for volumes will need to be completed to enable proper secrets handling options.
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-17T15:54:44.265Z
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: README.md:0-0
Timestamp: 2025-12-17T15:54:44.265Z
Learning: In ZeroByte, historically some sensitive data from past versions might not be encrypted in the database, and nested configuration is sometimes present in config fields which will be exported as-is without special handling by the export functionality.
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
🧬 Code graph analysis (1)
app/server/modules/lifecycle/config-export.controller.ts (7)
app/server/utils/crypto.ts (1)
cryptoUtils(186-191)app/server/utils/logger.ts (1)
logger(36-41)app/server/db/schema.ts (3)
backupSchedulesTable(70-101)backupScheduleNotificationsTable(138-154)usersTable(29-36)app/server/modules/volumes/volume.service.ts (1)
volumeService(397-409)app/server/modules/repositories/repositories.service.ts (1)
repositoriesService(438-451)app/server/modules/notifications/notifications.service.ts (1)
notificationsService(439-449)app/server/core/constants.ts (1)
RESTIC_PASS_FILE(5-5)
🔇 Additional comments (2)
app/server/modules/lifecycle/config-export.controller.ts (2)
98-134: LGTM: Secret processing handles encryption, exclusion, and nested structures correctly.The
processSecretsfunction properly implements all three secret handling modes with automatic encrypted field detection. The recursive handling of arrays and nested objects ensures secrets at any depth are processed consistently. Error handling for decryption failures includes appropriate logging before removing the failed field.
179-262: LGTM: Export endpoint structure and logic are well-organized.The main export endpoint implementation is solid:
- Parallel data fetching optimizes performance (lines 200-208, 223-227)
- Proper validation using schema and DTO (lines 183-184)
- Comprehensive error handling with detailed logging (lines 257-260)
- Security-sensitive operations (recovery key export) include audit logging (line 233)
- Flexible export options support multiple use cases (IaC-friendly defaults)
The response structure with conditional inclusion of metadata, recovery key, and formatted entities aligns well with the PR objectives for backup, migration, and IaC workflows.
- Use authenticated context from requireAuth middleware instead of duplicating session verification logic - Remove unused cookie imports and constants (COOKIE_NAME, COOKIE_OPTIONS) - Add backupScheduleMirrorsTable to export for complete backup config - Include mirror-specific metadata keys (lastCopyAt, lastCopyStatus, lastCopyError)
…d settings components
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
app/server/modules/lifecycle/config-export.controller.ts (1)
4-20: Add .ts extensions to all local imports.All local module imports are missing the required
.tsfile extensions. According to the coding guidelines, server TypeScript files targeting Node/Bun environments with"type": "module"must include file extensions in import statements.🔎 Proposed fix
import { type backupSchedulesTable, backupScheduleNotificationsTable, backupScheduleMirrorsTable, usersTable, -} from "../../db/schema"; -import { db } from "../../db/db"; -import { logger } from "../../utils/logger"; -import { RESTIC_PASS_FILE } from "../../core/constants"; -import { cryptoUtils } from "../../utils/crypto"; -import { authService } from "../auth/auth.service"; -import { volumeService } from "../volumes/volume.service"; -import { repositoriesService } from "../repositories/repositories.service"; -import { notificationsService } from "../notifications/notifications.service"; -import { backupsService } from "../backups/backups.service"; +} from "../../db/schema.ts"; +import { db } from "../../db/db.ts"; +import { logger } from "../../utils/logger.ts"; +import { RESTIC_PASS_FILE } from "../../core/constants.ts"; +import { cryptoUtils } from "../../utils/crypto.ts"; +import { authService } from "../auth/auth.service.ts"; +import { volumeService } from "../volumes/volume.service.ts"; +import { repositoriesService } from "../repositories/repositories.service.ts"; +import { notificationsService } from "../notifications/notifications.service.ts"; +import { backupsService } from "../backups/backups.service.ts"; import { fullExportBodySchema, fullExportDto, type SecretsMode, type FullExportBody } from "./config-export.dto"; -import { requireAuth } from "../auth/auth.middleware"; +import { requireAuth } from "../auth/auth.middleware.ts";As per coding guidelines, all TypeScript imports in server files must include file extensions when targeting Node/Bun.
README.md (1)
250-250: Remove import reference from export-only documentation.The phrase "requires the same recovery key to decrypt on import" mentions import functionality, which is out of scope for this PR and handled separately in PR #93.
🔎 Proposed fix
- - **Keep encrypted** - Export secrets in encrypted form (requires the same recovery key to decrypt on import) + - **Keep encrypted** - Export secrets in encrypted form (decryption requires the same recovery key)Based on learnings: export documentation should focus only on export behavior and not include import-related details.
🧹 Nitpick comments (2)
app/client/components/export-dialog.tsx (2)
180-180: Consider removing import reference from UI help text.The phrase "Requires the same recovery key to decrypt on import" mentions import functionality. While this explains the exported data format, it may be clearer to focus on export behavior only: "Secrets are exported in encrypted form and require the same recovery key to decrypt."
Based on learnings: export feature documentation should focus on export behavior, not import workflows.
214-214: Consider rephrasing to avoid implying import workflow.The phrase "for seamless migration" implies an import workflow that is out of scope for this PR. Consider: "Include the hashed user passwords to preserve authentication data in the export."
Based on learnings: import functionality is handled in PR #93 and is separate from this export feature.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.mdapp/client/api-client/types.gen.tsapp/client/components/export-dialog.tsxapp/client/modules/settings/routes/settings.tsxapp/server/modules/lifecycle/config-export.controller.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for code formatting and linting with
bunx biome check --write ., format only withbunx biome format --write ., or lint withbunx biome lint .
Files:
app/client/modules/settings/routes/settings.tsxapp/server/modules/lifecycle/config-export.controller.tsapp/client/components/export-dialog.tsxapp/client/api-client/types.gen.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use tabs (not spaces) for indentation with a line width of 120 characters
Use double quotes for strings
Do not auto-organize imports - imports organization is disabled in Biome
All imports must include file extensions when targeting Node/Bun, as the project uses"type": "module"
Files:
app/client/modules/settings/routes/settings.tsxapp/server/modules/lifecycle/config-export.controller.tsapp/client/components/export-dialog.tsxapp/client/api-client/types.gen.ts
app/client/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
app/client/**/*.{ts,tsx}: Client uses TanStack Query for server state management
Client uses Radix UI primitives with custom Tailwind styling
Use Server-Sent Events hook (use-server-events.ts) for real-time updates in the client
Files:
app/client/modules/settings/routes/settings.tsxapp/client/components/export-dialog.tsxapp/client/api-client/types.gen.ts
app/client/modules/*/routes/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Routes should be organized in feature modules at
app/client/modules/*/routes/
Files:
app/client/modules/settings/routes/settings.tsx
app/server/modules/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Server follows a modular service-oriented architecture with controller-service-database pattern in each module
Files:
app/server/modules/lifecycle/config-export.controller.ts
🧠 Learnings (17)
📓 Common learnings
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: README.md:0-0
Timestamp: 2025-12-17T15:54:44.265Z
Learning: For ZeroByte PR #92, the export configuration feature is separate from import functionality—import is out of scope and handled in a separate PR #93. Documentation for the export feature should focus only on export behavior and not include import-related caveats.
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: app/client/components/export-dialog.tsx:189-196
Timestamp: 2025-12-01T22:25:20.502Z
Learning: Volume secrets in ZeroByte are not yet encrypted in the database (stored as cleartext) and are excluded from the export dialog's secrets handling. The export logic relies on the `isEncrypted()` helper function that checks for the "encv1" encryption prefix, so cleartext volume secrets cannot be automatically detected for exclusion. Once volume secret encryption is implemented, the export logic for volumes will need to be completed to enable proper secrets handling options.
📚 Learning: 2025-12-17T15:54:44.265Z
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: README.md:0-0
Timestamp: 2025-12-17T15:54:44.265Z
Learning: For ZeroByte PR #92, the export configuration feature is separate from import functionality—import is out of scope and handled in a separate PR #93. Documentation for the export feature should focus only on export behavior and not include import-related caveats.
Applied to files:
app/client/modules/settings/routes/settings.tsxapp/server/modules/lifecycle/config-export.controller.tsREADME.mdapp/client/components/export-dialog.tsx
📚 Learning: 2025-12-01T22:25:20.502Z
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: app/client/components/export-dialog.tsx:189-196
Timestamp: 2025-12-01T22:25:20.502Z
Learning: Volume secrets in ZeroByte are not yet encrypted in the database (stored as cleartext) and are excluded from the export dialog's secrets handling. The export logic relies on the `isEncrypted()` helper function that checks for the "encv1" encryption prefix, so cleartext volume secrets cannot be automatically detected for exclusion. Once volume secret encryption is implemented, the export logic for volumes will need to be completed to enable proper secrets handling options.
Applied to files:
app/server/modules/lifecycle/config-export.controller.tsREADME.mdapp/client/components/export-dialog.tsx
📚 Learning: 2025-12-17T15:54:44.265Z
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: README.md:0-0
Timestamp: 2025-12-17T15:54:44.265Z
Learning: In ZeroByte, historically some sensitive data from past versions might not be encrypted in the database, and nested configuration is sometimes present in config fields which will be exported as-is without special handling by the export functionality.
Applied to files:
app/server/modules/lifecycle/config-export.controller.tsREADME.md
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to app/server/modules/backends/**/*.ts : Volume mounts are stored in `/var/lib/zerobyte/mounts/<volume-name>`
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to app/server/modules/backends/**/*.ts : Each volume backend (NFS, SMB, WebDAV, SFTP, directory) must implement mounting logic using system tools
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : All imports must include file extensions when targeting Node/Bun, as the project uses `"type": "module"`
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to app/server/modules/**/*.ts : Server follows a modular service-oriented architecture with controller-service-database pattern in each module
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to app/server/jobs/**/*.ts : New scheduled jobs must extend `Job` class, implement `run()` method, and be registered in `app/server/modules/lifecycle/startup.ts` with a cron expression
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Do not auto-organize imports - imports organization is disabled in Biome
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to app/server/db/schema.ts : Store timestamps as Unix epoch integers, not ISO strings, in the database
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to app/server/utils/restic.ts : Update `buildEnv()` in `app/server/utils/restic.ts` to handle credentials and configuration when adding a new repository backend
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-28T17:31:39.154Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.154Z
Learning: Applies to app/drizzle/**/*.ts : Never create migration files manually. Always use the provided command to generate migrations
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to **/*.{ts,tsx,js,jsx,json} : Use Biome for code formatting and linting with `bunx biome check --write .`, format only with `bunx biome format --write .`, or lint with `bunx biome lint .`
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use double quotes for strings
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to app/schemas/volumes.ts : Add schemas for new volume backends to `app/schemas/volumes.ts` and update `volumeConfigSchema` discriminated union
Applied to files:
app/client/components/export-dialog.tsx
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to app/client/**/*.{ts,tsx} : Client uses TanStack Query for server state management
Applied to files:
app/client/components/export-dialog.tsx
🔇 Additional comments (8)
app/server/modules/lifecycle/config-export.controller.ts (3)
76-92: LGTM! Password verification properly leverages authenticated context.The password verification correctly uses the authenticated user from
requireAuthmiddleware and only validates the password, avoiding duplicate session handling. This refactoring addresses previous feedback effectively.
98-134: LGTM! Secret processing correctly handles nested structures and errors.The function properly:
- Detects encrypted fields using
cryptoUtils.isEncrypted- Handles arrays of objects recursively
- Logs warnings when decryption fails and safely removes the field
- Processes nested objects
This addresses previous feedback about array handling.
187-265: LGTM! Export route implementation is well-structured.The route properly:
- Enforces authentication via
requireAuthmiddleware- Validates the request body
- Verifies the user's password before proceeding
- Uses service layer for data fetching (good separation of concerns)
- Fetches data in parallel for efficiency
- Logs security-sensitive operations (recovery key export)
- Provides detailed error messages
- Controls sensitive data inclusion via flags
app/client/modules/settings/routes/settings.tsx (1)
270-283: LGTM! Export Configuration section integrates cleanly.The new UI block:
- Follows consistent styling patterns with other sections
- Provides clear description of export functionality
- Uses appropriate Download icon
- Integrates the ExportDialog component properly
README.md (1)
259-259: LGTM! Sensitive data handling note is appropriate.The note properly warns users about:
- Historical data that may not be encrypted
- Nested configuration objects exported as-is
- Need to review exports before sharing
This aligns with the learnings about ZeroByte's data handling.
app/client/components/export-dialog.tsx (3)
21-59: LGTM! Type definitions provide good type safety.The type structure properly:
- Defines mutually exclusive prop variants using discriminated unions
- Prevents conflicting props (custom trigger vs. default trigger props)
- Uses
nevertype to enforce exclusivityThis addresses previous feedback about prop conflicts.
122-136: LGTM! Trigger rendering is keyboard accessible.The card variant now uses a proper
<button>element instead of a<div>, ensuring keyboard accessibility. This addresses previous accessibility feedback.
218-231: LGTM! Password verification is properly integrated.The password field is appropriately:
- Required for all exports
- Integrated inline in the export form (not a separate dialog)
- Accompanied by clear explanatory text
This addresses previous feedback about inline password verification.
…nd computing path for local repos
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/server/modules/lifecycle/config-export.controller.ts (1)
4-20: Add.tsextensions to all local imports.All local module imports are missing the required
.tsfile extensions. As per coding guidelines, server TypeScript files targeting Node/Bun environments with"type": "module"must include file extensions in import statements.
🧹 Nitpick comments (2)
app/server/modules/lifecycle/config-export.controller.ts (2)
120-127: Consider handling nested arrays recursively.The current logic processes arrays of objects but returns nested arrays (arrays within arrays) without recursive processing. If a configuration ever contains
[[{secret: "encv1..."}]], those secrets would be missed.This is likely an edge case that doesn't occur with current data structures, but for completeness:
🔎 Proposed enhancement for nested arrays
} else if (Array.isArray(value)) { result[key] = await Promise.all( value.map(async (item) => - item && typeof item === "object" && !Array.isArray(item) - ? processSecrets(item as Record<string, unknown>, secretsMode) + Array.isArray(item) + ? Promise.all(item.map((nested) => + nested && typeof nested === "object" + ? processSecrets(nested as Record<string, unknown>, secretsMode) + : nested + )) + : item && typeof item === "object" + ? processSecrets(item as Record<string, unknown>, secretsMode) : item, ), );
242-245: Consider validatingconfig.nameis non-empty.If
config.nameis an empty string, the path would resolve to${REPOSITORY_BASE}/which could cause issues during import.🔎 Proposed fix
// For local repos, compute and add the full path if not already present - if (config.backend === "local" && !config.path && typeof config.name === "string") { + if (config.backend === "local" && !config.path && typeof config.name === "string" && config.name.length > 0) { updatedConfig.path = `${REPOSITORY_BASE}/${config.name}`; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/server/modules/lifecycle/config-export.controller.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for code formatting and linting with
bunx biome check --write ., format only withbunx biome format --write ., or lint withbunx biome lint .
Files:
app/server/modules/lifecycle/config-export.controller.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use tabs (not spaces) for indentation with a line width of 120 characters
Use double quotes for strings
Do not auto-organize imports - imports organization is disabled in Biome
All imports must include file extensions when targeting Node/Bun, as the project uses"type": "module"
Files:
app/server/modules/lifecycle/config-export.controller.ts
app/server/modules/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Server follows a modular service-oriented architecture with controller-service-database pattern in each module
Files:
app/server/modules/lifecycle/config-export.controller.ts
🧠 Learnings (15)
📓 Common learnings
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: README.md:0-0
Timestamp: 2025-12-17T15:54:44.265Z
Learning: For ZeroByte PR #92, the export configuration feature is separate from import functionality—import is out of scope and handled in a separate PR #93. Documentation for the export feature should focus only on export behavior and not include import-related caveats.
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: app/client/components/export-dialog.tsx:189-196
Timestamp: 2025-12-01T22:25:20.502Z
Learning: Volume secrets in ZeroByte are not yet encrypted in the database (stored as cleartext) and are excluded from the export dialog's secrets handling. The export logic relies on the `isEncrypted()` helper function that checks for the "encv1" encryption prefix, so cleartext volume secrets cannot be automatically detected for exclusion. Once volume secret encryption is implemented, the export logic for volumes will need to be completed to enable proper secrets handling options.
📚 Learning: 2025-12-17T15:54:44.265Z
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: README.md:0-0
Timestamp: 2025-12-17T15:54:44.265Z
Learning: For ZeroByte PR #92, the export configuration feature is separate from import functionality—import is out of scope and handled in a separate PR #93. Documentation for the export feature should focus only on export behavior and not include import-related caveats.
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-01T22:25:20.502Z
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: app/client/components/export-dialog.tsx:189-196
Timestamp: 2025-12-01T22:25:20.502Z
Learning: Volume secrets in ZeroByte are not yet encrypted in the database (stored as cleartext) and are excluded from the export dialog's secrets handling. The export logic relies on the `isEncrypted()` helper function that checks for the "encv1" encryption prefix, so cleartext volume secrets cannot be automatically detected for exclusion. Once volume secret encryption is implemented, the export logic for volumes will need to be completed to enable proper secrets handling options.
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-17T15:54:44.265Z
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: README.md:0-0
Timestamp: 2025-12-17T15:54:44.265Z
Learning: In ZeroByte, historically some sensitive data from past versions might not be encrypted in the database, and nested configuration is sometimes present in config fields which will be exported as-is without special handling by the export functionality.
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to app/server/modules/backends/**/*.ts : Volume mounts are stored in `/var/lib/zerobyte/mounts/<volume-name>`
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to app/server/modules/backends/**/*.ts : Each volume backend (NFS, SMB, WebDAV, SFTP, directory) must implement mounting logic using system tools
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : All imports must include file extensions when targeting Node/Bun, as the project uses `"type": "module"`
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to app/server/modules/**/*.ts : Server follows a modular service-oriented architecture with controller-service-database pattern in each module
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to app/server/jobs/**/*.ts : New scheduled jobs must extend `Job` class, implement `run()` method, and be registered in `app/server/modules/lifecycle/startup.ts` with a cron expression
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Do not auto-organize imports - imports organization is disabled in Biome
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to app/server/db/schema.ts : Store timestamps as Unix epoch integers, not ISO strings, in the database
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to app/server/utils/restic.ts : Update `buildEnv()` in `app/server/utils/restic.ts` to handle credentials and configuration when adding a new repository backend
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to **/*.{ts,tsx,js,jsx,json} : Use Biome for code formatting and linting with `bunx biome check --write .`, format only with `bunx biome format --write .`, or lint with `bunx biome lint .`
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-28T17:31:39.154Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.154Z
Learning: Applies to app/drizzle/**/*.ts : Never create migration files manually. Always use the provided command to generate migrations
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use double quotes for strings
Applied to files:
app/server/modules/lifecycle/config-export.controller.ts
🔇 Additional comments (7)
app/server/modules/lifecycle/config-export.controller.ts (7)
22-51: Well-structured metadata configuration.The categorization of metadata keys (ids, timestamps, runtimeState) provides clear separation of concerns and makes the filtering logic easy to understand and maintain.
53-63: LGTM!The function correctly handles the early return optimization and performs shallow copy with key deletion appropriately.
65-70: LGTM!Parameter parsing with sensible defaults. Input validation is handled upstream by the
validator("json", fullExportBodySchema)middleware.
72-92: LGTM!Clean implementation leveraging the authenticated context from
requireAuthmiddleware. The defensive null check on line 82 is appropriate for type safety even though the middleware should guarantee the user exists.
136-148: LGTM!Clean composition of filtering and secret processing with proper async handling.
150-183: LGTM!The function correctly resolves human-readable names and applies metadata filtering. As discussed in previous reviews, backup schedules only reference other entities where secrets are stored, so the omission of
processSecretsis intentional and appropriate.
268-281: LGTM!The response structure is well-organized with conditional inclusion of optional fields. Error handling properly logs the full error and returns an appropriate message to the client.
|
@nicotsx |
|
Thank you for your work @tvarohohlavy I'll do 1 more release before merging this into main |
|
erratum 😆 :
|
|
@storm1er Yeah sorry I had other priorities pile up and I don't have as much free time as I wish I had. Unfortunately the work done here doesn't fit exactly how I'd like to have the feature work. So I'll have to close the PR |
feat: Add configuration export functionality with flexible options
Summary
Adds the ability to export ZeroByte configuration as JSON files for backup, migration, or Infrastructure as Code (IaC) workflows.
Features
Changes
ExportDialogUI component/api/v1/config/exportwith query parameter supportNotes
cryptoUtils.isEncrypted()Volume secrets are not encrypted in DB at the moment, needs to be done in separate PRRelated Issues
Summary by CodeRabbit
New Features
UI
Backend
API
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.