-
Notifications
You must be signed in to change notification settings - Fork 112
config export feature #293
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
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>
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>
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
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…vy/zerobyte into config-export-feature
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>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…d settings components
…nd computing path for local repos
WalkthroughAdds a password-protected full-configuration export end-to-end: server DTO/controller/service and crypto helpers; client SDK/types and react-query mutation; ExportDialog UI; downloadFile utility; and updates to settings and recovery-key download flows. Tests and README formatting updates included. 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 |
a89684d to
081a10a
Compare
7b59bba to
3f55067
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
app/client/components/export-dialog.tsx (1)
96-109: Consider addingautocomplete="current-password"to the password input.For better UX and password manager integration, explicitly setting the autocomplete attribute helps browsers and password managers correctly identify this field as requiring the user's current password for re-authentication.
🔎 Proposed fix
<Input id="export-password" type="password" value={password} onChange={(e) => setPassword(e.target.value)} placeholder="Enter your password to export" required + autocomplete="current-password" />app/client/lib/utils.ts (1)
32-47: Consider exportingDownloadFileMimeTypefor type-safe usage by consumers.The type is useful for callers who may want to explicitly type their mimeType parameter. Also, the function handles edge cases well with proper resource cleanup.
🔎 Proposed fix
-type DownloadFileMimeType = "text/plain" | "application/json"; +export type DownloadFileMimeType = "text/plain" | "application/json";app/server/modules/system/system.service.ts (2)
141-147: Unnecessary async inexportEntityandexportEntities.These functions don't perform any asynchronous operations. Using
asyncandPromise.alladds unnecessary overhead. Consider making them synchronous.🔎 Proposed fix
-async function exportEntity(entity: Record<string, unknown>, params: FullExportBody) { +function exportEntity(entity: Record<string, unknown>, params: FullExportBody) { return filterMetadataOut(entity, params.includeMetadata); } -async function exportEntities<T extends Record<string, unknown>>(entities: T[], params: FullExportBody) { - return Promise.all(entities.map((e) => exportEntity(e, params))); +function exportEntities<T extends Record<string, unknown>>(entities: T[], params: FullExportBody) { + return entities.map((e) => exportEntity(e, params)); }Then update the usage in
exportConfig:-const [exportVolumes, exportRepositories, exportNotifications, exportUsers] = await Promise.all([ - exportEntities(volumes, params), - exportEntities(repositories, params), - exportEntities(notifications, params), - exportEntities(users, params), -]); +const [exportVolumes, exportRepositories, exportNotifications, exportUsers] = [ + exportEntities(volumes, params), + exportEntities(repositories, params), + exportEntities(notifications, params), + exportEntities(users, params), +];
184-198: Inconsistent metadata filtering forbackupSchedules.
backupSchedulesuses inline transformation viatransformBackupScheduleswhich applies metadata filtering directly, while other entities useexportEntities. This works but creates an inconsistent pattern.Consider applying
exportEntitiestobackupSchedulesfor consistency, or documenting why the different approach is necessary (likely because of the nested notifications/mirrors).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
app/client/api-client/@tanstack/react-query.gen.tsapp/client/api-client/sdk.gen.tsapp/client/api-client/types.gen.tsapp/client/components/export-dialog.tsxapp/client/lib/utils.tsapp/client/modules/auth/routes/download-recovery-key.tsxapp/client/modules/settings/routes/settings.tsxapp/server/modules/auth/auth.service.tsapp/server/modules/system/system.controller.tsapp/server/modules/system/system.dto.tsapp/server/modules/system/system.service.tsapp/server/utils/crypto.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/components/export-dialog.tsxapp/server/modules/system/system.dto.tsapp/client/modules/settings/routes/settings.tsxapp/server/modules/auth/auth.service.tsapp/client/api-client/sdk.gen.tsapp/server/utils/crypto.tsapp/client/modules/auth/routes/download-recovery-key.tsxapp/server/modules/system/system.service.tsapp/client/api-client/types.gen.tsapp/client/lib/utils.tsapp/client/api-client/@tanstack/react-query.gen.tsapp/server/modules/system/system.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/client/components/export-dialog.tsxapp/server/modules/system/system.dto.tsapp/client/modules/settings/routes/settings.tsxapp/server/modules/auth/auth.service.tsapp/client/api-client/sdk.gen.tsapp/server/utils/crypto.tsapp/client/modules/auth/routes/download-recovery-key.tsxapp/server/modules/system/system.service.tsapp/client/api-client/types.gen.tsapp/client/lib/utils.tsapp/client/api-client/@tanstack/react-query.gen.tsapp/server/modules/system/system.controller.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/components/export-dialog.tsxapp/client/modules/settings/routes/settings.tsxapp/client/api-client/sdk.gen.tsapp/client/modules/auth/routes/download-recovery-key.tsxapp/client/api-client/types.gen.tsapp/client/lib/utils.tsapp/client/api-client/@tanstack/react-query.gen.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/system/system.dto.tsapp/server/modules/auth/auth.service.tsapp/server/modules/system/system.service.tsapp/server/modules/system/system.controller.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.tsxapp/client/modules/auth/routes/download-recovery-key.tsx
🧠 Learnings (11)
📓 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/components/export-dialog.tsxapp/client/modules/settings/routes/settings.tsxapp/server/modules/system/system.service.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/modules/repositories/repositories.dto.ts : Add DTO schemas in `app/server/modules/repositories/repositories.dto.ts` when adding a new repository backend
Applied to files:
app/server/modules/system/system.dto.tsapp/server/modules/system/system.service.tsapp/server/modules/system/system.controller.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
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/server/modules/system/system.dto.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/client/**/*.{ts,tsx} : Client uses TanStack Query for server state management
Applied to files:
app/client/modules/settings/routes/settings.tsxapp/client/api-client/@tanstack/react-query.gen.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/utils/restic.ts : Restic password file is stored in `/var/lib/zerobyte/restic/password` and auto-generated on first run
Applied to files:
app/client/modules/settings/routes/settings.tsxapp/client/modules/auth/routes/download-recovery-key.tsxapp/server/modules/system/system.controller.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
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/client/api-client/sdk.gen.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/utils/restic.ts : Update `buildRepoUrl()` in `app/server/utils/restic.ts` when adding a new repository backend
Applied to files:
app/client/modules/auth/routes/download-recovery-key.tsx
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
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/system/system.service.tsapp/server/modules/system/system.controller.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/modules/backends/backend.ts : Update backend factory in `app/server/modules/backends/backend.ts` when adding a new volume backend
Applied to files:
app/server/modules/system/system.service.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/utils/restic.ts : Restic password file must have 0600 permissions and should never be exposed
Applied to files:
app/server/modules/system/system.controller.ts
🧬 Code graph analysis (5)
app/client/components/export-dialog.tsx (2)
app/client/api-client/@tanstack/react-query.gen.ts (1)
exportFullConfigMutation(1002-1014)app/client/lib/utils.ts (1)
downloadFile(33-47)
app/client/modules/settings/routes/settings.tsx (2)
app/client/lib/utils.ts (1)
downloadFile(33-47)app/client/components/export-dialog.tsx (1)
ExportDialog(24-125)
app/client/modules/auth/routes/download-recovery-key.tsx (1)
app/client/lib/utils.ts (1)
downloadFile(33-47)
app/server/modules/system/system.service.ts (3)
app/server/modules/system/system.dto.ts (1)
FullExportBody(88-88)app/server/db/schema.ts (9)
BackupScheduleNotification(166-166)BackupScheduleMirror(201-201)volumesTable(10-22)repositoriesTable(51-63)backupSchedulesTable(70-102)notificationDestinationsTable(122-130)backupScheduleNotificationsTable(139-155)backupScheduleMirrorsTable(172-189)usersTable(29-36)app/server/db/db.ts (1)
db(14-14)
app/client/api-client/@tanstack/react-query.gen.ts (2)
app/client/api-client/sdk.gen.ts (2)
Options(7-19)exportFullConfig(432-439)app/client/api-client/types.gen.ts (3)
ExportFullConfigData(3341-3349)ExportFullConfigResponse(3391-3391)ExportFullConfigError(3366-3366)
🔇 Additional comments (12)
app/server/utils/crypto.ts (1)
189-190: LGTM!The newly exported
isEncryptedanddecryptutilities are well-implemented and appropriately documented. These exports enable the config export feature to handle encrypted database values correctly.app/server/modules/auth/auth.service.ts (1)
178-189: LGTM!The
verifyPasswordmethod is correctly implemented for re-authentication purposes. It follows the established patterns in this service and properly handles the case where the user doesn't exist by returningfalserather than throwing an error.app/client/modules/auth/routes/download-recovery-key.tsx (1)
14-14: LGTM!The refactoring to use the centralized
downloadFileutility is a good improvement that reduces code duplication and improves maintainability across the codebase.Also applies to: 35-35
app/client/modules/settings/routes/settings.tsx (2)
3-3: LGTM!The refactoring to use the
downloadFileutility is consistent with the same improvement made in the download-recovery-key route, promoting code reuse.Also applies to: 27-27, 83-83
262-275: LGTM!The new Export Configuration section is well-integrated into the settings page, following the established UI patterns and structure used by other sections.
app/client/components/export-dialog.tsx (1)
24-42: LGTM! Well-structured mutation with proper cleanup.Good practices observed: password is cleared on success (line 35), error messages are parsed and displayed via toast. The mutation correctly uses the generated
exportFullConfigMutationfactory.app/client/api-client/sdk.gen.ts (1)
428-439: Auto-generated code follows established patterns.The new
exportFullConfigendpoint is correctly generated with proper typing and headers, consistent with other endpoints in this file.app/client/api-client/@tanstack/react-query.gen.ts (1)
998-1014: Auto-generated mutation follows established patterns.The
exportFullConfigMutationcorrectly uses the specificExportFullConfigErrortype instead ofDefaultError, providing better type safety for error handling. This is consistent with other mutations that have specific error types.app/server/modules/system/system.service.ts (1)
186-191: Verify: UserpasswordHashis exported intentionally.The export includes
passwordHashin the users array. Per the PR description, this appears intentional so users can keep their same password after import. However, this is a sensitive field.Ensure:
- The export file is clearly communicated as sensitive to users
- The password verification requirement (line 3 imports
FullExportBodywhich requires password) provides adequate protectionapp/server/modules/system/system.dto.ts (2)
82-108: Well-structured export schemas with appropriate defaults.Good design choices:
includeMetadatadefaults tofalse(safer default)passwordis required (no optional marker)- Response schema uses loose
unknown[]typing for flexibility, which is acceptable for an export format that may evolveThe users schema explicitly includes
passwordHashas optional, which aligns with the intentional design to preserve user passwords for import.
114-144: Route descriptor properly documents authentication requirements.The 401 response description "Password required for export or authentication failed" clearly communicates the dual authentication layer (session auth + password re-verification).
app/client/api-client/types.gen.ts (1)
3340-3391: Auto-generated types correctly reflect the API contract.The types are properly generated from the OpenAPI spec, matching the server-side DTOs. The
bodyis correctly marked as optional at the type level while the server enforces required fields via validation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/server/modules/system/system.dto.ts (1)
90-108: Consider stronger typing for collection schemas.Using
unknown[]?forvolumes,repositories,backupSchedules, andnotificationDestinationsprovides flexibility but loses OpenAPI documentation benefits. If these structures are stable, defining their schemas would improve API discoverability.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/server/modules/system/system.controller.tsapp/server/modules/system/system.dto.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/system/system.dto.tsapp/server/modules/system/system.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/system/system.dto.tsapp/server/modules/system/system.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/system/system.dto.tsapp/server/modules/system/system.controller.ts
🧠 Learnings (5)
📓 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-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/modules/repositories/repositories.dto.ts : Add DTO schemas in `app/server/modules/repositories/repositories.dto.ts` when adding a new repository backend
Applied to files:
app/server/modules/system/system.dto.tsapp/server/modules/system/system.controller.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
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/server/modules/system/system.dto.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
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/system/system.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: 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/system/system.controller.ts
🧬 Code graph analysis (1)
app/server/modules/system/system.dto.ts (1)
app/client/api-client/types.gen.ts (1)
ExportFullConfigResponse(3391-3391)
🪛 GitHub Actions: Checks
app/server/modules/system/system.controller.ts
[error] 10-10: no-unused-imports: Several imports are unused. This is fixable by removing unused imports.
🔇 Additional comments (3)
app/server/modules/system/system.controller.ts (1)
68-87: LGTM!The
/exportendpoint correctly implements the password verification flow matching the existing/restic-passwordpattern, and now properly callssystemService.exportConfig(body)to return the configuration data. This addresses the previous review concern about the incomplete endpoint.app/server/modules/system/system.dto.ts (2)
83-88: LGTM!The request body schema is well-defined with a sensible default for
includeMetadataand required password field.
116-146: LGTM!The route descriptor follows the established pattern with comprehensive response definitions for success and error cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/server/modules/system/system.controller.ts (1)
85-85: Consider explicitly specifying the 200 status code.For consistency with other endpoints in this controller (e.g., line 25), explicitly passing the status code improves code clarity.
🔎 Suggested improvement
- return c.json(res); + return c.json(res, 200);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/server/modules/system/system.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/system/system.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/system/system.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/system/system.controller.ts
🧠 Learnings (3)
📓 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-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
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/system/system.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: 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/system/system.controller.ts
🧬 Code graph analysis (1)
app/server/modules/system/system.controller.ts (4)
app/server/modules/system/system.dto.ts (2)
fullExportDto(116-146)fullExportBodySchema(83-86)app/server/db/db.ts (1)
db(14-14)app/server/db/schema.ts (1)
usersTable(29-36)app/server/modules/system/system.service.ts (1)
systemService(204-208)
🔇 Additional comments (1)
app/server/modules/system/system.controller.ts (1)
6-7: LGTM! Import additions are correct.The new imports are properly used in the
/exportendpoint implementation.
53c6a88 to
99ccfa4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/server/modules/system/system.dto.ts (2)
94-97: Consider adding type definitions for collection arrays.The schema uses
"unknown[]?"forvolumes,repositories,backupSchedules, andnotificationDestinations. While this provides flexibility, it sacrifices type safety and validation. If these arrays have predictable structures, consider defining proper schemas (even if minimal) to catch malformed exports early.This is likely intentional for flexibility during import/export evolution, but documenting the expected structure would help maintainability.
112-126: Consider adding error response schemas.The route descriptor only defines a 200 success response. For complete OpenAPI documentation and consistency with other routes, consider adding error response schemas for common failure cases:
- 401: Authentication failure (invalid password)
- 500: Export generation failure
This would improve API documentation and client-side error handling.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/client/api-client/@tanstack/react-query.gen.tsapp/client/api-client/sdk.gen.tsapp/client/api-client/types.gen.tsapp/server/modules/system/system.dto.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/client/api-client/sdk.gen.ts
- app/client/api-client/@tanstack/react-query.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
bunx biome check --write ., format only withbunx biome format --write ., or lint withbunx biome lint .
Files:
app/server/modules/system/system.dto.tsapp/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/server/modules/system/system.dto.tsapp/client/api-client/types.gen.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/system/system.dto.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/api-client/types.gen.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.
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-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/modules/repositories/repositories.dto.ts : Add DTO schemas in `app/server/modules/repositories/repositories.dto.ts` when adding a new repository backend
Applied to files:
app/server/modules/system/system.dto.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
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/server/modules/system/system.dto.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
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/client/api-client/types.gen.ts
🧬 Code graph analysis (2)
app/server/modules/system/system.dto.ts (1)
app/client/api-client/types.gen.ts (1)
ExportFullConfigResponse(3374-3374)
app/client/api-client/types.gen.ts (1)
app/server/modules/system/system.dto.ts (1)
ExportFullConfigResponse(110-110)
🔇 Additional comments (1)
app/client/api-client/types.gen.ts (1)
3341-3374: Auto-generated types reflect server schema.This file is auto-generated from the OpenAPI specification. The types correctly mirror the server-side DTO schema defined in
app/server/modules/system/system.dto.ts. Any concerns about the export structure (such as including password hashes or the use ofArray<unknown>for collections) should be addressed in the server-side schema source rather than here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/server/modules/system/system.dto.ts (1)
83-86: Documentation forincludeMetadatastill missing.As noted in a previous review, this parameter lacks documentation explaining which fields are excluded when
false. Consider adding an OpenAPI description.
🧹 Nitpick comments (3)
app/server/modules/system/__tests__/system.controller.test.ts (1)
91-145: Consider organizing export tests within existing describe blocks.The new export validation tests (lines 91-122) are placed outside the "input validation" describe block (lines 57-89), while similar
restic-passwordvalidation tests are inside it. For consistency and better test organization, consider moving these tests into the appropriate describe blocks.🔎 Suggested organization
describe("input validation", () => { test("should return 400 for invalid payload on restic-password", async () => { // existing test... }); test("should return 401 for incorrect password on restic-password", async () => { // existing test... }); + + test("should return 400 for invalid payload on full export", async () => { + // move test here... + }); + + test("should return 401 for incorrect password on full export", async () => { + // move test here... + }); }); - - test("should return 400 for invalid payload on full export", async () => { - // ... - }); - - test("should return 401 for incorrect password on full export", async () => { - // ... - });app/server/modules/system/system.service.ts (1)
145-151: Unnecessary async wrappers on synchronous operations.Both
exportEntityandexportEntitiesare markedasyncbut only call the synchronousfilterMetadataOut. ThePromise.allinexportEntitiesadds overhead without benefit.🔎 Suggested simplification
-async function exportEntity(entity: Record<string, unknown>, params: ExportParams) { - return filterMetadataOut(entity, params.includeMetadata); +function exportEntity(entity: Record<string, unknown>, params: ExportParams) { + return filterMetadataOut(entity, params.includeMetadata); } -async function exportEntities<T extends Record<string, unknown>>(entities: T[], params: ExportParams) { - return Promise.all(entities.map((e) => exportEntity(e, params))); +function exportEntities<T extends Record<string, unknown>>(entities: T[], params: ExportParams) { + return entities.map((e) => exportEntity(e, params)); }Then update line 190 to remove the unnecessary
await Promise.all:- const [exportVolumes, exportRepositories, exportNotifications, exportedUsersWithHash] = await Promise.all([ - exportEntities(volumes, params), - exportEntities(repositories, params), - exportEntities(notifications, params), - exportEntities(users, params), - ]); + const exportVolumes = exportEntities(volumes, params); + const exportRepositories = exportEntities(repositories, params); + const exportNotifications = exportEntities(notifications, params); + const exportedUsersWithHash = exportEntities(users, params);app/server/modules/system/system.dto.ts (1)
94-97: Consider defining schemas for exported collections.Using
unknown[]?forvolumes,repositories,backupSchedules, andnotificationDestinationsprovides no type information in the OpenAPI spec. API consumers won't know the structure of these arrays.This is acceptable for an initial implementation, but consider defining proper schemas in the future to improve API documentation and client type generation.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
README.mdapp/client/api-client/@tanstack/react-query.gen.tsapp/client/api-client/sdk.gen.tsapp/client/api-client/types.gen.tsapp/server/modules/system/__tests__/system.controller.test.tsapp/server/modules/system/system.controller.tsapp/server/modules/system/system.dto.tsapp/server/modules/system/system.service.ts
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- app/client/api-client/sdk.gen.ts
- app/client/api-client/@tanstack/react-query.gen.ts
- app/server/modules/system/system.controller.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
bunx biome check --write ., format only withbunx biome format --write ., or lint withbunx biome lint .
Files:
app/server/modules/system/__tests__/system.controller.test.tsapp/client/api-client/types.gen.tsapp/server/modules/system/system.service.tsapp/server/modules/system/system.dto.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/system/__tests__/system.controller.test.tsapp/client/api-client/types.gen.tsapp/server/modules/system/system.service.tsapp/server/modules/system/system.dto.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/system/__tests__/system.controller.test.tsapp/server/modules/system/system.service.tsapp/server/modules/system/system.dto.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/api-client/types.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-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
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/client/api-client/types.gen.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/modules/backends/backend.ts : Update backend factory in `app/server/modules/backends/backend.ts` when adding a new volume backend
Applied to files:
app/server/modules/system/system.service.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
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/system/system.service.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/modules/repositories/repositories.dto.ts : Add DTO schemas in `app/server/modules/repositories/repositories.dto.ts` when adding a new repository backend
Applied to files:
app/server/modules/system/system.dto.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
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/server/modules/system/system.dto.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:
app/server/modules/system/system.dto.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/utils/restic.ts : Restic password file must have 0600 permissions and should never be exposed
Applied to files:
app/server/modules/system/system.dto.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/system/system.dto.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/system/system.dto.ts
🧬 Code graph analysis (4)
app/server/modules/system/__tests__/system.controller.test.ts (1)
app/test/helpers/auth.ts (1)
createTestSession(5-24)
app/client/api-client/types.gen.ts (1)
app/server/modules/system/system.dto.ts (1)
ExportFullConfigResponse(109-109)
app/server/modules/system/system.service.ts (2)
app/server/db/schema.ts (9)
BackupScheduleNotification(166-166)BackupScheduleMirror(201-201)volumesTable(10-22)repositoriesTable(51-63)backupSchedulesTable(70-102)notificationDestinationsTable(122-130)backupScheduleNotificationsTable(139-155)backupScheduleMirrorsTable(172-189)usersTable(29-36)app/server/db/db.ts (1)
db(14-14)
app/server/modules/system/system.dto.ts (1)
app/client/api-client/types.gen.ts (1)
ExportFullConfigResponse(3373-3373)
🔇 Additional comments (7)
app/server/modules/system/__tests__/system.controller.test.ts (1)
124-145: Good security test coverage.This test properly validates that
passwordHashis never exposed in the export response, aligning with the security requirements discussed in the PR objectives. The iteration over all users ensures comprehensive coverage.app/server/modules/system/system.service.ts (3)
120-129: VerifysortOrderexclusion is intentional.
sortOrderis included inruntimeStatemetadata keys, but per the schema (backupSchedulesTable), it appears to be user-defined ordering for display purposes. Excluding it whenincludeMetadata: falsemay cause loss of custom ordering on import.If
sortOrderrepresents user-configured display order, consider moving it out ofruntimeStateor documenting that custom ordering is not preserved unlessincludeMetadata: true.
197-201: Password hash sanitization looks correct.The explicit removal of
passwordHashfrom exported users aligns with the security decision discussed in the PR objectives. The test atsystem.controller.test.ts:124-145validates this behavior.
176-186: Good parallel query execution.The
Promise.allpattern for independent database queries is appropriate and efficient. All seven tables are fetched concurrently without interdependencies.app/server/modules/system/system.dto.ts (2)
98-106: Schema correctly excludespasswordHash.The users schema appropriately omits
passwordHash, aligning with the service-side sanitization and the security decision in the PR objectives.
111-125: Route descriptor is consistent with existing patterns.The
fullExportDtofollows the same structure as other route descriptors in this file. While error responses (400, 401) aren't documented, this is consistent with the existingdownloadResticPasswordDtopattern.app/client/api-client/types.gen.ts (1)
3341-3349: LGTM!The request type structure is well-defined. The required password field aligns with the password-protected export feature, and the optional includeMetadata flag provides appropriate flexibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/server/modules/system/system.service.ts (1)
145-151: Consider: exportEntity doesn't need to be async.The function currently just calls the synchronous
filterMetadataOut. If future async operations are planned, the current design is fine; otherwise, it could be simplified to synchronous.🔎 Optional simplification if no async operations are planned
-async function exportEntity(entity: Record<string, unknown>, params: ExportParams) { +function exportEntity(entity: Record<string, unknown>, params: ExportParams) { return filterMetadataOut(entity, params.includeMetadata); } -async function exportEntities<T extends Record<string, unknown>>(entities: T[], params: ExportParams) { - return Promise.all(entities.map((e) => exportEntity(e, params))); +function exportEntities<T extends Record<string, unknown>>(entities: T[], params: ExportParams) { + return entities.map((e) => exportEntity(e, params)); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/server/modules/system/system.service.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/system/system.service.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/system/system.service.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/system/system.service.ts
🧠 Learnings (2)
📓 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-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/system/system.service.ts
🧬 Code graph analysis (1)
app/server/modules/system/system.service.ts (2)
app/server/db/schema.ts (9)
BackupScheduleNotification(166-166)BackupScheduleMirror(201-201)volumesTable(10-22)repositoriesTable(51-63)backupSchedulesTable(70-102)notificationDestinationsTable(122-130)backupScheduleNotificationsTable(139-155)backupScheduleMirrorsTable(172-189)usersTable(29-36)app/server/db/db.ts (1)
db(14-14)
🔇 Additional comments (4)
app/server/modules/system/system.service.ts (4)
7-23: LGTM!The imports and type definition are clean and appropriate for the export functionality.
110-143: Clean metadata filtering implementation.The metadata classification and filtering logic is correct. The use of
Partial<T>as the return type appropriately reflects that some keys may be removed.
153-174: Well-structured backup schedule transformation.The logic correctly associates notifications and mirrors with their parent schedules and consistently applies metadata filtering throughout.
218-218: LGTM!Correctly exposes the new export functionality through the service interface.
| const [exportVolumes, exportRepositories, exportNotifications, exportedUsersWithHash] = await Promise.all([ | ||
| exportEntities(volumes, params) as Promise<typeof volumes>, | ||
| exportEntities(repositories, params) as Promise<typeof repositories>, | ||
| exportEntities(notifications, params) as Promise<typeof notifications>, | ||
| exportEntities(users, params) as Promise<typeof users>, | ||
| ]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the ExportFullConfigResponse type and how exportConfig is called
ast-grep --pattern 'type ExportFullConfigResponse = $_'
echo "---"
# Find where exportConfig is called to see usage
rg -n -A5 "exportConfig\(" --type=tsRepository: nicotsx/zerobyte
Length of output: 602
🏁 Script executed:
#!/bin/bash
# Find the exportResponseSchema definition
rg -n "exportResponseSchema" --type=ts -B2 -A10
echo "---"
# Find the exportEntities function definition
ast-grep --pattern 'function exportEntities($_) { $$$ }'
echo "---"
# Also search for exportEntities as arrow function or method
rg -n -B3 "exportEntities\s*[=:]" --type=tsRepository: nicotsx/zerobyte
Length of output: 2296
🏁 Script executed:
#!/bin/bash
# Read the system.service.ts file to see exportConfig implementation and context
wc -l app/server/modules/system/system.service.ts
echo "---"
# Read around lines 190-195 with more context
sed -n '170,210p' app/server/modules/system/system.service.ts
echo "---"
# Search for exportEntities function definition
rg -n "const exportEntities|function exportEntities" --type=ts -A15Repository: nicotsx/zerobyte
Length of output: 3006
🏁 Script executed:
#!/bin/bash
# Find the exportEntity function definition
rg -n "function exportEntity|const exportEntity" --type=ts -A20Repository: nicotsx/zerobyte
Length of output: 1860
🏁 Script executed:
#!/bin/bash
# Find the filterMetadataOut function definition
rg -n "function filterMetadataOut|const filterMetadataOut" --type=ts -A15Repository: nicotsx/zerobyte
Length of output: 1352
Remove unsafe type assertions that claim full types when metadata is filtered.
The type assertions on lines 191-194 (as Promise<typeof volumes>, etc.) claim the exported entities have all original properties, but exportEntities returns Promise<Partial<Record<string, unknown>>[]> because filterMetadataOut explicitly returns Partial<T> when metadata is excluded. This masks the actual type from TypeScript and could cause issues if future code assumes all properties are present.
Either:
- Remove the assertions and update consuming code to handle
Partial<T>types, or - Ensure the export response type accurately reflects that some properties may be absent
🤖 Prompt for AI Agents
In app/server/modules/system/system.service.ts around lines 190 to 195, the
Promise.all calls use unsafe assertions (as Promise<typeof volumes> etc.) that
claim full entity shapes while exportEntities actually returns Partial<T> when
metadata is filtered; remove these incorrect type assertions and either change
the variable types to reflect Partial<Record<string, unknown>>[] (or
Partial<typeof volumes>[], Partial<typeof repositories>[], etc.) and update any
downstream handling to accept optional fields, or update exportEntities' return
type to a more precise generic (e.g., Promise<Partial<T>[]>) and propagate that
correct type through the consuming code so callers handle possibly-missing
properties rather than masking them with assertions.
| const exportUsers = exportedUsersWithHash.map((user) => { | ||
| const sanitizedUser = { ...user } as Record<string, unknown>; | ||
| delete sanitizedUser.passwordHash; | ||
| sanitizedUser.password = `\${USER_${user.username.toUpperCase()}_PASSWORD}`; | ||
| return sanitizedUser; | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sanitize usernames for environment variable placeholders.
Line 200 creates placeholders like ${USER_${username.toUpperCase()}_PASSWORD}, but usernames containing hyphens, dots, spaces, or other special characters will produce invalid environment variable names (e.g., ${USER_ADMIN-USER_PASSWORD}).
🔎 Suggested fix to sanitize username for env var format
const exportUsers = exportedUsersWithHash.map((user) => {
const sanitizedUser = { ...user } as Record<string, unknown>;
delete sanitizedUser.passwordHash;
- sanitizedUser.password = `\${USER_${user.username.toUpperCase()}_PASSWORD}`;
+ const sanitizedUsername = user.username.toUpperCase().replace(/[^A-Z0-9]/g, "_");
+ sanitizedUser.password = `\${USER_${sanitizedUsername}_PASSWORD}`;
return sanitizedUser;
});This replaces any non-alphanumeric characters with underscores to ensure valid environment variable names.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const exportUsers = exportedUsersWithHash.map((user) => { | |
| const sanitizedUser = { ...user } as Record<string, unknown>; | |
| delete sanitizedUser.passwordHash; | |
| sanitizedUser.password = `\${USER_${user.username.toUpperCase()}_PASSWORD}`; | |
| return sanitizedUser; | |
| }); | |
| const exportUsers = exportedUsersWithHash.map((user) => { | |
| const sanitizedUser = { ...user } as Record<string, unknown>; | |
| delete sanitizedUser.passwordHash; | |
| const sanitizedUsername = user.username.toUpperCase().replace(/[^A-Z0-9]/g, "_"); | |
| sanitizedUser.password = `\${USER_${sanitizedUsername}_PASSWORD}`; | |
| return sanitizedUser; | |
| }); |
🤖 Prompt for AI Agents
In app/server/modules/system/system.service.ts around lines 197 to 202, the
generated env placeholder uses raw username which can include invalid characters
(hyphens, dots, spaces) and produce invalid environment variable names; replace
any non-alphanumeric characters in user.username with underscores and then
uppercase the result before interpolating so placeholders become like
${USER_<SANITIZED_UPPER>_PASSWORD}; implement the sanitization by normalizing
the username (e.g., replace characters not in A-Z0-9 with '_') and use that
sanitizedUpper in the placeholder while leaving the original username intact in
other fields.
| version: 1, | ||
| exportedAt: new Date().toISOString(), | ||
| volumes: exportVolumes, | ||
| repositories: exportRepositories, | ||
| backupSchedules, | ||
| notificationDestinations: exportNotifications, | ||
| users: exportUsers, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add this if recoveryKey is not to be exported?
| version: 1, | |
| exportedAt: new Date().toISOString(), | |
| volumes: exportVolumes, | |
| repositories: exportRepositories, | |
| backupSchedules, | |
| notificationDestinations: exportNotifications, | |
| users: exportUsers, | |
| version: 1, | |
| exportedAt: new Date().toISOString(), | |
| recoveryKey: `\${RECOVERY_KEY}`, | |
| volumes: exportVolumes, | |
| repositories: exportRepositories, | |
| backupSchedules, | |
| notificationDestinations: exportNotifications, | |
| users: exportUsers, |
| const transformBackupSchedules = ( | ||
| schedules: BackupSchedule[], | ||
| scheduleNotifications: BackupScheduleNotification[], | ||
| scheduleMirrors: BackupScheduleMirror[], | ||
| params: ExportParams, | ||
| ) => { | ||
| return schedules.map((schedule) => { | ||
| const assignments = scheduleNotifications | ||
| .filter((sn) => sn.scheduleId === schedule.id) | ||
| .map((sn) => filterMetadataOut(sn, params.includeMetadata)); | ||
|
|
||
| const mirrors = scheduleMirrors | ||
| .filter((sm) => sm.scheduleId === schedule.id) | ||
| .map((sm) => filterMetadataOut(sm, params.includeMetadata)); | ||
|
|
||
| return { | ||
| ...filterMetadataOut(schedule, params.includeMetadata), | ||
| notifications: assignments, | ||
| mirrors, | ||
| }; | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
transformBackupSchedules function needs to translate repositoryId, volumeId and destinationId in notification assignemnts and repositoryId in mirrors into names as on import databse IDs can be different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import config will take care of the mapping. This is not what will be literally inserted in the new db, just an indicator for the import function to know where goes what
|
Why For all repository types, setting
Why local repos need explicit
Key insight: The createRepository for exisitng repositories on import needs This ensures export files can be re-imported on a new instance and reconnect to existing backup data instead of trying to create new repos. |
Since you have to give the restic password to re-use the repository, there is no need to mark this as imported. We will not run the init command in the import But anyway, this is not the responsibility of the export feature to tell how it will be imported |
Not sure if I understand. I am talking about having export json file ready for import to recover zerobyte instance. |
The import will not call the |
This branch includes changes from #92 and an extra refactor commit I did.
@tvarohohlavy I have simplified the process a lot and I need your feedback on my assumptions.
I think we don't need the options for plain text or deleted secrets. For two reasons:
Same for the user password hash, if we don't include it, what do we put in the db? A random password? I believe this is never something a user would want, if you export you just want your same password to work there.
Summary by CodeRabbit
New Features
Improvements
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.