feat: Add JSON configuration import for Infrastructure as Code support #93
Conversation
|
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 config-file import pipeline with validation and apply semantics, a CLI command to import or dry-run JSON configs (file or stdin), ArkType schemas for import formats, example docs and sample files, short-id validation/use across services, new id/error utilities, and a minor auth message text change. 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 |
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive JSON configuration file support to enable Infrastructure as Code (IaC) workflows for Zerobyte. The feature allows users to define volumes, repositories, backup schedules, notification destinations, and admin users in a zerobyte.config.json file with environment variable interpolation for secure secret injection.
Key Changes:
- Configuration file parsing with environment variable interpolation (
${VAR_NAME}syntax) - Create-only resource provisioning (existing resources are not overwritten)
- Support for pre-encrypted secrets from exports and automatic re-encryption of cleartext secrets
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
zerobyte.config.json |
Example configuration file demonstrating all supported resource types (volumes, repositories, schedules, notifications, admin) |
docker-compose.yml |
Added environment variables and volume mount for config file in production service |
app/server/utils/crypto.ts |
Exported isEncrypted() function to detect already-encrypted values |
app/server/modules/repositories/repositories.service.ts |
Added conditional encryption checks and special handling for existing repositories and local repo migrations |
app/server/modules/notifications/notifications.service.ts |
Added conditional encryption for notification secrets to support importing pre-encrypted values |
app/server/modules/lifecycle/startup.ts |
Core config import logic including file parsing, resource creation, recovery key setup, and admin user provisioning |
app/server/modules/backups/backups.service.ts |
Added duplicate schedule detection to prevent conflicts when importing configurations |
README.md |
Comprehensive documentation of config file format with examples for all resource types |
💡 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.
![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 (8)
docker-compose.yml (1)
39-45: Consider using a.envfile pattern instead of inline placeholder secrets.The placeholder credentials in the compose file could be accidentally replaced with real secrets and committed. Additionally, this environment block is incomplete—
zerobyte.config.jsonreferences additional variables likeSFTP_PASSWORD,SFTP_PRIVATE_KEY,SLACK_WEBHOOK_URL,EMAIL_PASSWORD,DISCORD_WEBHOOK_URL,GOTIFY_TOKEN,NTFY_PASSWORD,PUSHOVER_USER_KEY,PUSHOVER_API_TOKEN,TELEGRAM_BOT_TOKEN,SHOUTRRR_URL,ADMIN_PASSWORD, andRECOVERY_KEYthat are not defined here.Consider this approach:
environment: - - ACCESS_KEY_ID=your-access-key-id - - SECRET_ACCESS_KEY=your-secret-access-key - - SMB_PASSWORD=your-smb-password - - WEBDAV_PASSWORD=your-webdav-password - - GCS_CREDENTIALS=your-gcs-credentials-json - - AZURE_KEY=your-azure-key + - ACCESS_KEY_ID=${ACCESS_KEY_ID} + - SECRET_ACCESS_KEY=${SECRET_ACCESS_KEY} + - SMB_PASSWORD=${SMB_PASSWORD} + # ... additional vars from .env + env_file: + - .envThen provide a
.env.examplefile with all placeholders and add.envto.gitignore.zerobyte.config.json (1)
133-143: Consider simplifying the example backup schedule.The example schedule references all 8 notification destinations, which may overwhelm new users. A simpler example with 1-2 destinations would be more approachable while still demonstrating the feature.
- "notifications": ["slack-alerts", "email-admin", "discord-backups", "gotify-notify", "ntfy-notify", "pushover-notify", "telegram-notify", "custom-shoutrrr"] + "notifications": ["slack-alerts"]app/server/modules/repositories/repositories.service.ts (1)
75-76: Edge case: empty stringconfig.namewill bypass ID generation.If
config.nameis an empty string"", the conditionconfig.backend === "local" && config.nameevaluates tofalse(correct), but this relies on JavaScript's falsy coercion. Consider an explicit check for clarity.- const shortId = (config.backend === "local" && config.name) ? config.name : generateShortId(); + const shortId = (config.backend === "local" && config.name?.length) ? config.name : generateShortId();README.md (2)
40-75: Clarify top‑level config structure and JSON validityThe new structure example is helpful, but it currently omits the
adminblock that you document later. Also, the JSON snippets here use//comments, which are not valid in a realzerobyte.config.json.Consider:
- Including an
adminexample in the top‑level structure.- Adding a brief note that comments in the snippets are for documentation only and must be removed in actual JSON files.
337-397: Strengthen warning around recovery key reuse on migrationThe “Admin Setup (Automated)” and “About the Recovery Key” sections are clear, but given how critical the recovery key is, it may be worth explicitly stating that when migrating to an existing data directory, the
recoveryKeyin config must exactly match the one used by that instance, otherwise backups and stored credentials become unrecoverable.A one‑sentence, explicit “If this value differs from the existing instance’s recovery key, you will lose access to your data” under the migration example would help make the risk unmissable.
app/server/modules/lifecycle/startup.ts (3)
15-47: Config loading and env interpolation are solid; consider handling missing envs more explicitlyThe lazy imports and recursive
interpolatefunction are a clean way to support${VAR_NAME}in arbitrary nested config structures. Right now, missing environment variables silently become empty strings (process.env[v] || ""), which can be hard to diagnose for secrets mis‑spelled in the environment.Consider (optionally) logging a warning once per missing variable name, or allowing a mode that throws on unresolved
${…}placeholders, so misconfigurations fail fast instead of surfacing later as auth/connectivity errors.
177-207: Tighten admin config validation and logging behaviorThe admin auto‑setup block mostly matches the documented behavior (create/import admin only when no users exist, support
passwordHashfor migrations, and markhasDownloadedResticPasswordwhen a recoveryKey is present). A couple of minor robustness improvements:
- The
elsebranch logs"Admin config missing required fields..."even whenconfigFileAdminisnull(i.e., noadminblock in the config). That will emit a warning for users who intentionally don’t use config‑driven admin setup. Wrapping the warning inif (configFileAdmin)would avoid noisy logs.- If both
passwordandpasswordHashare provided, the code silently preferspasswordHash. Given the README says “Use eitherpasswordORpasswordHash, not both”, you may want to detect that case and log a warning or throw, so misconfigurations are caught early.These are small behavior tweaks but make the admin import path more predictable.
213-231: Existing remount logic remains compatible with config‑driven mountsThe final auto‑remount block still filters volumes by
statusandautoRemount, then callsmountVolumefor each. With the new config flow, some volumes may already have been mounted earlier when wiring backup schedules, somountVolumemight be called again for the same volume. AssumingmountVolumeis idempotent, this is harmless but slightly redundant.If
mountVolumeis not strictly idempotent, it may be worth short‑circuiting here for volumes you just mounted during schedule creation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
README.md(2 hunks)app/server/modules/backups/backups.service.ts(2 hunks)app/server/modules/lifecycle/startup.ts(2 hunks)app/server/modules/notifications/notifications.service.ts(1 hunks)app/server/modules/repositories/repositories.service.ts(3 hunks)app/server/utils/crypto.ts(2 hunks)docker-compose.yml(2 hunks)zerobyte.config.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-30T15:18:38.895Z
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 89
File: app/server/modules/notifications/builders/telegram.ts:3-8
Timestamp: 2025-11-30T15:18:38.895Z
Learning: In app/server/modules/notifications/builders/telegram.ts, the Telegram bot token in Shoutrrr URLs must NOT be URL-encoded. When passed to the Shoutrrr CLI, encoding the bot token (e.g., encoding the colon as %3A) causes "failed to initialize service" errors. The token should be used raw in the URL userinfo section. Chat IDs in the channels query parameter can optionally be encoded but work fine unencoded as well.
Applied to files:
app/server/modules/notifications/notifications.service.ts
🧬 Code graph analysis (4)
app/server/modules/backups/backups.service.ts (2)
app/server/db/db.ts (1)
db(13-13)app/server/db/schema.ts (1)
backupSchedulesTable(68-95)
app/server/modules/notifications/notifications.service.ts (1)
app/server/utils/crypto.ts (1)
cryptoUtils(62-66)
app/server/modules/repositories/repositories.service.ts (3)
app/server/utils/crypto.ts (1)
cryptoUtils(62-66)app/server/utils/id.ts (1)
generateShortId(3-6)app/server/utils/restic.ts (1)
restic(736-748)
app/server/modules/lifecycle/startup.ts (6)
app/server/core/scheduler.ts (1)
Scheduler(52-52)app/server/core/constants.ts (1)
RESTIC_PASS_FILE(5-5)app/server/utils/restic.ts (1)
restic(736-748)app/server/modules/volumes/volume.service.ts (1)
volumeService(414-427)app/server/db/schema.ts (4)
volumesTable(10-22)repositoriesTable(50-62)notificationDestinationsTable(112-120)usersTable(28-35)app/server/modules/auth/auth.service.ts (2)
hasUsers(146-149)authService(179-179)
⏰ 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 (12)
app/server/utils/crypto.ts (1)
8-10: LGTM! Clean extraction of encryption check logic.This helper function correctly encapsulates the encryption detection logic that was previously inline in
encrypt(). The type guard (typeof val === "string") handles undefined/null inputs safely.app/server/modules/notifications/notifications.service.ts (1)
36-81: LGTM! Encryption guards correctly prevent double-encryption.The conditional encryption pattern is consistently applied across all notification types. The
ntfy.passwordhandling at line 61 correctly preserves the optional field semantics with the nested check.app/server/modules/backups/backups.service.ts (1)
80-89: LGTM! Idempotent schedule creation with proper duplicate detection.The duplicate check correctly prevents creating multiple schedules for the same volume-repository pair, supporting the create-only semantics described in the PR objectives. The error message is descriptive and includes both entity names.
docker-compose.yml (1)
49-50: Mounting./mydataas read-only may conflict with backup restore operations.The
./mydata:/mydata:romount is read-only, which is appropriate for backup sources but will prevent restores to that volume. Verify this is intentional for the example configuration.zerobyte.config.json (1)
226-230: LGTM! Admin configuration with proper secret handling.The admin section correctly uses environment variable interpolation for sensitive values (
passwordandrecoveryKey), allowing secure configuration without hardcoding secrets.app/server/modules/repositories/repositories.service.ts (2)
21-23: LGTM! Encryption guard for customPassword.The optional check is correct—
customPasswordmay not exist on all configs.
114-124: Good resilience: graceful handling of pre-existing repositories.This fallback logic correctly handles the case where a repository already exists (e.g., during re-deployment or config re-import) by verifying accessibility via
snapshots()instead of failing immediately.README.md (2)
77-331: Config examples for volumes/repos/schedules/notifications look coherentThe examples for volume/repository types, backup schedules (including notification wiring), and notification destinations line up well with the described behavior and env‑var interpolation model. They give enough detail to author real configs for most backends without guessing.
394-417: docker‑compose snippet aligns with startup behaviorMounting
./zerobyte.config.jsoninto/app/zerobyte.config.jsonmatches the default path resolution instartup.ts(process.cwd()is/appin the container), so this example should “just work” without needingZEROBYTE_CONFIG_PATH. Looks good.app/server/modules/lifecycle/startup.ts (3)
99-175: Backup schedule creation and notification assignment flow is well‑structuredThe schedule import logic correctly:
- Resolves volumes and repositories by name (
volume/volumeName,repository/repositoryName).- Mounts volumes on demand if they are not yet mounted.
- Creates schedules via the backups service with the resolved IDs.
- Optionally resolves and assigns notification destinations by name, supporting both simple string names and detailed objects with
notifyOn*flags and sensible defaults.The per‑item try/catch blocks and “continue on failure” behavior give good resilience during partial imports.
68-98: Verify notification destination creation API and type handlingThe notification destination creation call may be missing the
typeargument:await notificationsServiceModule.notificationsService.createDestination(n.name, n.config);If the service API follows the same pattern as volume and repository creation (which pass
name,type, andconfigas separate arguments), this call should includen.type. Confirm the actualcreateDestinationmethod signature and whethertypeneeds to be passed explicitly or is inferred fromn.config.
52-62: Avoid unconditionally overwriting RESTIC_PASS_FILE when recoveryKey is providedThis block always writes
configFileAdmin.recoveryKeytoRESTIC_PASS_FILEwhenever it's present in the config, without checking whether the file already exists. On an existing instance with a prior recovery key, this could rotate the key and render existing backup data unreadable.Consider:
- Only write the file if it does not already exist, or
- Check for an existing file and either verify it matches the configured
recoveryKeyor log a clear error and skip writingRegarding the
await import("../../core/constants.js")path with the.jsextension: verify this is consistent with your module system and TypeScript configuration, as most other imports in the codebase appear to omit extensions.
|
Hello, I appreciate your contribution however I cannot afford to spend the time reviewing it as it has been AI generated. Were you able to test the implementation? |
Hi, Lots of the discrepancies cones from me splitting my code that was for both import and export at the same time, but it made sense to separate it into 2 PRs. I did a lot of tests importing and exporting, both with fresh instances and recovering old ones, but I did not test all possible repository and volume configurations, because its a lot and I dont have access to all at the moment. Yes, I use Github Copilot to help me and my main languages are Python, C++ and Golang however I spent on it several hours iterating a slowly bringing more features and fixing what AI has suggested. Also I did designed my changes and come up with the ideas and edge cases. I really like Zerobyte and see its wast potential, but configuration through UI only is definitely its downside, so I wanted to make it better. Thanks for all your good work! |
|
Okay thanks for the clarification. I'll give you my comments when I have some time |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/server/modules/repositories/repositories.service.ts (1)
53-57: Missing encryption guard for SFTP password field.The SFTP backend only encrypts
privateKey, but some SFTP configurations may use password authentication. If the schema supports apasswordfield for SFTP, it should also be guarded.Apply this diff to add password encryption for SFTP:
case "sftp": if (!cryptoUtils.isEncrypted(config.privateKey)) { encryptedConfig.privateKey = await cryptoUtils.encrypt(config.privateKey); } + if (config.password && !cryptoUtils.isEncrypted(config.password)) { + encryptedConfig.password = await cryptoUtils.encrypt(config.password); + } break;
🧹 Nitpick comments (1)
app/server/modules/lifecycle/startup.ts (1)
22-23: Duplicate dynamic imports offs.The
node:fs/promisesmodule is imported dynamically twice (lines 22-23 and 59-60). Consider importing it once at the top of the function or reusing the first import.try { - const fs = await import("node:fs/promises"); - const { RESTIC_PASS_FILE } = await import("../../core/constants.js"); + const { RESTIC_PASS_FILE } = await import("../../core/constants"); if (configFileAdmin && configFileAdmin.recoveryKey) {Note: The
fsvariable from line 22 can be reused here if moved to a broader scope, and the.jsextension on the constants import may be unnecessary depending on your TypeScript configuration.Also applies to: 59-60
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(2 hunks)app/server/modules/lifecycle/startup.ts(2 hunks)app/server/modules/repositories/repositories.service.ts(3 hunks)docker-compose.yml(2 hunks)zerobyte.config.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/server/modules/lifecycle/startup.ts (6)
app/server/core/scheduler.ts (1)
Scheduler(52-52)app/server/core/constants.ts (1)
RESTIC_PASS_FILE(5-5)app/server/utils/restic.ts (1)
restic(736-748)app/server/db/db.ts (1)
db(13-13)app/server/db/schema.ts (4)
volumesTable(10-22)repositoriesTable(50-62)notificationDestinationsTable(112-120)usersTable(28-35)app/server/modules/auth/auth.service.ts (1)
hasUsers(146-149)
app/server/modules/repositories/repositories.service.ts (5)
app/server/utils/crypto.ts (1)
cryptoUtils(62-66)app/server/db/db.ts (1)
db(13-13)app/server/db/schema.ts (1)
repositoriesTable(50-62)app/server/utils/id.ts (1)
generateShortId(3-6)app/server/utils/restic.ts (1)
restic(736-748)
🔇 Additional comments (9)
docker-compose.yml (1)
39-67: Environment and volume configuration looks complete.The environment block provides all variables referenced in the example
zerobyte.config.json, and the volume mappings correctly mount the config file read-only. The placeholder values are appropriate for an example template.zerobyte.config.json (1)
1-217: Well-structured example configuration.The configuration file correctly demonstrates all supported resource types with appropriate field structures. Previous review issues have been addressed:
- SFTP repository uses
userandprivateKey(lines 113-114)- Notification destinations have
typeinsideconfig(e.g., lines 134-135)- Environment variable interpolation syntax is consistent throughout
app/server/modules/lifecycle/startup.ts (2)
191-225: Admin setup logic properly handles migration scenarios.The implementation correctly:
- Validates mutual exclusivity of
passwordandpasswordHash(lines 194-197)- Only creates admin when no users exist (line 199)
- Supports password hash import for migrations (lines 201-208)
- Marks recovery key as acknowledged when provided via config (lines 215-217)
155-187: Flexible notification assignment design.The implementation elegantly handles both simple string references (
["slack-alerts"]) and detailed objects with granular control over notification triggers. Defaulting all triggers totruewhen using string references is a sensible user-friendly choice.app/server/modules/repositories/repositories.service.ts (2)
76-89: Good migration support with conflict detection.The shortId logic correctly allows users to specify an existing repository name for migration scenarios while preventing conflicts. The explicit error message on line 85 clearly explains the shortId's purpose as a subdirectory name.
127-137: Appropriate fallback for existing repository verification.When
restic initfails with "config file already exists", attempting verification viasnapshots()is a reasonable approach to determine if the repository is valid and accessible. This handles the edge case where a user re-imports a configuration.README.md (3)
40-56: Clear documentation of create-only semantics.The info box clearly explains that the config file serves as "initial setup" rather than "desired state sync," which is important for users to understand. This prevents confusion about config file behavior on subsequent restarts.
239-344: Notification examples correctly structured.The notification destination examples now show the
typefield inside theconfigobject (e.g., lines 246-252 for Slack), which matches the expected schema structure.
385-404: Excellent recovery key documentation.The warning box clearly explains the dual purpose of the recovery key (Restic password + database encryption), provides generation commands for multiple platforms, and shows how to retrieve it from an existing instance. This is critical information for data recovery.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/server/modules/repositories/repositories.service.ts (1)
25-48: MissingisEncrypted()guards will cause double-encryption of imported secrets.The PR objectives state support for "detection of encrypted values via cryptoUtils.isEncrypted()" and importing both encrypted and cleartext secrets. However,
encryptConfigunconditionally encrypts all sensitive fields without checking if they're already encrypted. When importing pre-encrypted values, this will corrupt the data by double-encrypting.Apply guards to all sensitive field encryption:
switch (config.backend) { case "s3": case "r2": - encryptedConfig.accessKeyId = await cryptoUtils.encrypt(config.accessKeyId); - encryptedConfig.secretAccessKey = await cryptoUtils.encrypt(config.secretAccessKey); + if (!cryptoUtils.isEncrypted(config.accessKeyId)) { + encryptedConfig.accessKeyId = await cryptoUtils.encrypt(config.accessKeyId); + } + if (!cryptoUtils.isEncrypted(config.secretAccessKey)) { + encryptedConfig.secretAccessKey = await cryptoUtils.encrypt(config.secretAccessKey); + } break; case "gcs": - encryptedConfig.credentialsJson = await cryptoUtils.encrypt(config.credentialsJson); + if (!cryptoUtils.isEncrypted(config.credentialsJson)) { + encryptedConfig.credentialsJson = await cryptoUtils.encrypt(config.credentialsJson); + } break; case "azure": - encryptedConfig.accountKey = await cryptoUtils.encrypt(config.accountKey); + if (!cryptoUtils.isEncrypted(config.accountKey)) { + encryptedConfig.accountKey = await cryptoUtils.encrypt(config.accountKey); + } break; case "rest": if (config.username) { - encryptedConfig.username = await cryptoUtils.encrypt(config.username); + if (!cryptoUtils.isEncrypted(config.username)) { + encryptedConfig.username = await cryptoUtils.encrypt(config.username); + } } if (config.password) { - encryptedConfig.password = await cryptoUtils.encrypt(config.password); + if (!cryptoUtils.isEncrypted(config.password)) { + encryptedConfig.password = await cryptoUtils.encrypt(config.password); + } } break; case "sftp": - encryptedConfig.privateKey = await cryptoUtils.encrypt(config.privateKey); + if (!cryptoUtils.isEncrypted(config.privateKey)) { + encryptedConfig.privateKey = await cryptoUtils.encrypt(config.privateKey); + } break; }Also apply the same pattern to
customPassword(lines 21-23):if (config.customPassword) { - encryptedConfig.customPassword = await cryptoUtils.encrypt(config.customPassword); + if (!cryptoUtils.isEncrypted(config.customPassword)) { + encryptedConfig.customPassword = await cryptoUtils.encrypt(config.customPassword); + } }
🧹 Nitpick comments (1)
app/server/modules/repositories/repositories.service.ts (1)
117-127: String-based error detection is fragile but acceptable.The error message matching (
"config file already exists") could break if restic changes its error messages in future versions. The verification fallback viasnapshots()is a reasonable approach to determine if the repository is actually usable.Consider extracting the error string to a constant for easier maintenance, or adding a comment noting the restic version this was tested against.
+// Error message from restic when repository already initialized +const RESTIC_CONFIG_EXISTS_ERROR = "config file already exists"; + if (errorStr.includes("config file already exists")) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/server/modules/repositories/repositories.service.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/server/modules/repositories/repositories.service.ts (5)
app/server/utils/crypto.ts (1)
cryptoUtils(58-61)app/server/db/db.ts (1)
db(13-13)app/server/db/schema.ts (1)
repositoriesTable(50-62)app/server/utils/id.ts (1)
generateShortId(3-6)app/server/utils/restic.ts (1)
restic(736-748)
🔇 Additional comments (1)
app/server/modules/repositories/repositories.service.ts (1)
65-79: LGTM!The conflict check for user-provided
shortIdvalues addresses the previous review concern. The logic properly validates uniqueness before usingconfig.namefor migration scenarios, with a clear error message explaining the constraint.
Re-encryption is not possible due to this in existing code: zerobyte/app/server/utils/crypto.ts Line 16 in b8e30e2 |
…and handle errors appropriately
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/server/modules/repositories/repositories.service.ts (1)
18-50: Avoid double-encrypting repository secrets by guarding withcryptoUtils.isEncrypted.As written,
encryptConfigwill attempt to encrypt all sensitive fields unconditionally. For JSON-config imports that may already contain encrypted values (migration scenarios), this will double-encrypt secrets, breaking access to existing repositories. This is the same issue maintainers already raised for this helper and needs to be fixed here, not just in higher-level import code.You can make
encryptConfigidempotent by checkingcryptoUtils.isEncryptedbefore encrypting each field:const encryptConfig = async (config: RepositoryConfig): Promise<RepositoryConfig> => { const encryptedConfig: Record<string, string | boolean | number> = { ...config }; - if (config.customPassword) { - encryptedConfig.customPassword = await cryptoUtils.encrypt(config.customPassword); - } + if (config.customPassword && !cryptoUtils.isEncrypted(config.customPassword)) { + encryptedConfig.customPassword = await cryptoUtils.encrypt(config.customPassword); + } switch (config.backend) { case "s3": case "r2": - encryptedConfig.accessKeyId = await cryptoUtils.encrypt(config.accessKeyId); - encryptedConfig.secretAccessKey = await cryptoUtils.encrypt(config.secretAccessKey); + if (!cryptoUtils.isEncrypted(config.accessKeyId)) { + encryptedConfig.accessKeyId = await cryptoUtils.encrypt(config.accessKeyId); + } + if (!cryptoUtils.isEncrypted(config.secretAccessKey)) { + encryptedConfig.secretAccessKey = await cryptoUtils.encrypt(config.secretAccessKey); + } break; case "gcs": - encryptedConfig.credentialsJson = await cryptoUtils.encrypt(config.credentialsJson); + if (!cryptoUtils.isEncrypted(config.credentialsJson)) { + encryptedConfig.credentialsJson = await cryptoUtils.encrypt(config.credentialsJson); + } break; case "azure": - encryptedConfig.accountKey = await cryptoUtils.encrypt(config.accountKey); + if (!cryptoUtils.isEncrypted(config.accountKey)) { + encryptedConfig.accountKey = await cryptoUtils.encrypt(config.accountKey); + } break; case "rest": - if (config.username) { - encryptedConfig.username = await cryptoUtils.encrypt(config.username); - } - if (config.password) { - encryptedConfig.password = await cryptoUtils.encrypt(config.password); - } + if (config.username && !cryptoUtils.isEncrypted(config.username)) { + encryptedConfig.username = await cryptoUtils.encrypt(config.username); + } + if (config.password && !cryptoUtils.isEncrypted(config.password)) { + encryptedConfig.password = await cryptoUtils.encrypt(config.password); + } break; case "sftp": - encryptedConfig.privateKey = await cryptoUtils.encrypt(config.privateKey); + if (!cryptoUtils.isEncrypted(config.privateKey)) { + encryptedConfig.privateKey = await cryptoUtils.encrypt(config.privateKey); + } break; } return encryptedConfig as RepositoryConfig; };This keeps
encryptConfigsafe to call for both fresh API-created repos and config-driven imports that may mix cleartext and already-encrypted secrets.
🧹 Nitpick comments (1)
app/server/modules/repositories/repositories.service.ts (1)
88-105: Repo-existence gating is sound; consider optionally surfacing snapshot errors.The
repoExistscheck viarestic.snapshots(encryptedConfig)combined with theisExistingRepositorygates correctly prevents accidental init over an existing repo and enforces that imports only succeed when the repository is actually accessible.One optional improvement: the
.catch(() => false)currently swallows all restic errors. Emitting a debug/log entry withtoMessage(error)here (or just before throwing the “Cannot access existing repository...” error) would make diagnosing misconfigured paths/credentials easier without changing behavior.Also applies to: 125-128
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/server/modules/repositories/repositories.service.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-03T19:48:55.097Z
Learnt from: nicotsx
Repo: nicotsx/zerobyte PR: 95
File: app/server/modules/backups/backups.service.ts:525-533
Timestamp: 2025-12-03T19:48:55.097Z
Learning: In app/server/modules/backups/backups.service.ts, the mirror copy operations use shared locks (acquireShared) for both source and mirror repositories because restic supports parallel write operations and concurrent backups with non-exclusive locks. Only maintenance operations like prune and rebuild-index require exclusive locks in restic.
Applied to files:
app/server/modules/repositories/repositories.service.ts
🧬 Code graph analysis (1)
app/server/modules/repositories/repositories.service.ts (3)
app/server/db/schema.ts (1)
repositoriesTable(50-62)app/server/utils/id.ts (1)
generateShortId(3-6)app/server/utils/restic.ts (1)
restic(736-748)
🔇 Additional comments (1)
app/server/modules/repositories/repositories.service.ts (1)
65-84: Local shortId /config.namehandling for migrations looks consistent.Using
config.nameasshortIdonly forbackend === "local"(with a DB uniqueness check onrepositoriesTable.shortId) and then normalizingprocessedConfigto{ ...config, name: shortId }for locals cleanly supports the “import existing local repo by directory name” scenario without disturbing other backends. I don’t see functional issues with this approach.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/server/modules/repositories/repositories.service.ts (1)
19-52: Critical: Missing encryption guards will cause double-encryption of already-encrypted values.The
encryptConfigfunction unconditionally encrypts sensitive fields without checking if they're already encrypted. When importing from an exported config (which already contains encrypted values), this will corrupt the credentials by double-encrypting them.This was explicitly flagged in the PR review comments. Each sensitive field needs an
isEncryptedguard:const encryptConfig = async (config: RepositoryConfig): Promise<RepositoryConfig> => { const encryptedConfig: Record<string, string | boolean | number> = { ...config }; - if (config.customPassword) { + if (config.customPassword && !cryptoUtils.isEncrypted(config.customPassword)) { encryptedConfig.customPassword = await cryptoUtils.encrypt(config.customPassword); } switch (config.backend) { case "s3": case "r2": - encryptedConfig.accessKeyId = await cryptoUtils.encrypt(config.accessKeyId); - encryptedConfig.secretAccessKey = await cryptoUtils.encrypt(config.secretAccessKey); + if (!cryptoUtils.isEncrypted(config.accessKeyId)) { + encryptedConfig.accessKeyId = await cryptoUtils.encrypt(config.accessKeyId); + } + if (!cryptoUtils.isEncrypted(config.secretAccessKey)) { + encryptedConfig.secretAccessKey = await cryptoUtils.encrypt(config.secretAccessKey); + } break; case "gcs": - encryptedConfig.credentialsJson = await cryptoUtils.encrypt(config.credentialsJson); + if (!cryptoUtils.isEncrypted(config.credentialsJson)) { + encryptedConfig.credentialsJson = await cryptoUtils.encrypt(config.credentialsJson); + } break; case "azure": - encryptedConfig.accountKey = await cryptoUtils.encrypt(config.accountKey); + if (!cryptoUtils.isEncrypted(config.accountKey)) { + encryptedConfig.accountKey = await cryptoUtils.encrypt(config.accountKey); + } break; case "rest": - if (config.username) { + if (config.username && !cryptoUtils.isEncrypted(config.username)) { encryptedConfig.username = await cryptoUtils.encrypt(config.username); } - if (config.password) { + if (config.password && !cryptoUtils.isEncrypted(config.password)) { encryptedConfig.password = await cryptoUtils.encrypt(config.password); } break; case "sftp": - encryptedConfig.privateKey = await cryptoUtils.encrypt(config.privateKey); + if (!cryptoUtils.isEncrypted(config.privateKey)) { + encryptedConfig.privateKey = await cryptoUtils.encrypt(config.privateKey); + } break; } return encryptedConfig as RepositoryConfig; };
🧹 Nitpick comments (4)
app/server/modules/backups/backups.service.ts (1)
90-99: LGTM! Duplicate schedule prevention correctly implemented.The check for existing schedules with the same volume and repository is a sensible safeguard for the config-driven startup flow.
Minor note: Line 98 exceeds the 120 character line width specified in coding guidelines.
if (existingSchedule) { - throw new ConflictError(`A backup schedule for volume '${volume.name}' and repository '${repository.name}' already exists`); + throw new ConflictError( + `A backup schedule for volume '${volume.name}' and repository '${repository.name}' already exists` + ); }docker-compose.yml (1)
62-62: Recovery key placeholder should indicate format requirements.The placeholder
your-64-char-hex-recovery-keyis helpful, but users might not realize this must be exactly 64 hex characters. Consider using a realistic example format:- - RECOVERY_KEY=your-64-char-hex-recovery-key + - RECOVERY_KEY=0000000000000000000000000000000000000000000000000000000000000000Or add a comment indicating how to generate one (as documented in README).
app/server/modules/lifecycle/startup.ts (2)
29-44: Add TypeScript type annotation to theinterpolatefunction.The
interpolatefunction lacks type annotations, which reduces type safety and IDE support:- function interpolate(obj) { + function interpolate<T>(obj: T): T { if (typeof obj === "string") { - return obj.replace(/\$\{([^}]+)\}/g, (_, v) => { + return obj.replace(/\$\{([^}]+)\}/g, (_, v: string) => { if (process.env[v] === undefined) { logger.warn(`Environment variable '${v}' is not defined. Replacing with empty string.`); return ""; } - return process.env[v]; + return process.env[v] as string; }); } else if (Array.isArray(obj)) { - return obj.map(interpolate); + return obj.map(interpolate) as T; } else if (obj && typeof obj === "object") { - return Object.fromEntries(Object.entries(obj).map(([k, v]) => [k, interpolate(v)])); + return Object.fromEntries(Object.entries(obj).map(([k, v]) => [k, interpolate(v)])) as T; } - return obj; + return obj as T; }
113-153: Consider validating schedule name uniqueness before creation.The schedule creation loop mounts volumes as needed, which is good. However, if a schedule fails due to name conflict (handled in
backupsService.createSchedule), the volume will have been mounted unnecessarily. This is minor but could be optimized by checking for existing schedules first.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(2 hunks)app/server/modules/backups/backups.service.ts(1 hunks)app/server/modules/lifecycle/startup.ts(2 hunks)app/server/modules/repositories/repositories.service.ts(3 hunks)docker-compose.yml(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use Biome for formatting and linting with configuration: tabs (not spaces), 120 character line width, double quotes
Do not auto-organize imports in Biome - imports organization is disabled in the codebase
Files:
app/server/modules/backups/backups.service.tsapp/server/modules/lifecycle/startup.tsapp/server/modules/repositories/repositories.service.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
All imports must include file extensions when targeting Node/Bun (TypeScript with
"type": "module")
Files:
app/server/modules/backups/backups.service.tsapp/server/modules/lifecycle/startup.tsapp/server/modules/repositories/repositories.service.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T17:31:30.248Z
Learning: Repository backend configurations must be defined with schemas in `app/schemas/restic.ts`, URL construction in `buildRepoUrl()`, and environment setup in `buildEnv()`
📚 Learning: 2025-12-04T17:31:30.248Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T17:31:30.248Z
Learning: Repository backend configurations must be defined with schemas in `app/schemas/restic.ts`, URL construction in `buildRepoUrl()`, and environment setup in `buildEnv()`
Applied to files:
app/server/modules/lifecycle/startup.tsapp/server/modules/repositories/repositories.service.ts
📚 Learning: 2025-12-04T17:31:30.248Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T17:31:30.248Z
Learning: Applies to app/server/modules/backends/*/!(backend).ts : Volume backend implementations must extend the base backend pattern with `mount()` and `unmount()` methods
Applied to files:
app/server/modules/lifecycle/startup.ts
📚 Learning: 2025-12-04T17:31:30.248Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T17:31:30.248Z
Learning: Applies to app/server/utils/restic.ts : Restic password file must have 0600 permissions and never be exposed in logs or error messages
Applied to files:
app/server/modules/lifecycle/startup.tsapp/server/modules/repositories/repositories.service.ts
📚 Learning: 2025-12-04T17:31:30.248Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T17:31:30.248Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use Biome for formatting and linting with configuration: tabs (not spaces), 120 character line width, double quotes
Applied to files:
app/server/modules/lifecycle/startup.ts
📚 Learning: 2025-12-03T19:48:55.097Z
Learnt from: nicotsx
Repo: nicotsx/zerobyte PR: 95
File: app/server/modules/backups/backups.service.ts:525-533
Timestamp: 2025-12-03T19:48:55.097Z
Learning: In app/server/modules/backups/backups.service.ts, the mirror copy operations use shared locks (acquireShared) for both source and mirror repositories because restic supports parallel write operations and concurrent backups with non-exclusive locks. Only maintenance operations like prune and rebuild-index require exclusive locks in restic.
Applied to files:
app/server/modules/repositories/repositories.service.ts
📚 Learning: 2025-12-04T17:31:30.248Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T17:31:30.248Z
Learning: Volume mount paths are located at `/var/lib/zerobyte/mounts/<volume-name>` and local repository paths at `/var/lib/zerobyte/repositories/<repo-name>`
Applied to files:
README.md
🧬 Code graph analysis (3)
app/server/modules/backups/backups.service.ts (2)
app/server/db/db.ts (1)
db(14-14)app/server/db/schema.ts (1)
backupSchedulesTable(68-97)
app/server/modules/lifecycle/startup.ts (3)
app/server/core/constants.ts (1)
RESTIC_PASS_FILE(5-5)app/server/db/schema.ts (4)
volumesTable(10-22)repositoriesTable(50-62)notificationDestinationsTable(116-124)usersTable(28-35)app/server/modules/auth/auth.service.ts (1)
hasUsers(146-149)
app/server/modules/repositories/repositories.service.ts (5)
app/server/core/config.ts (1)
config(21-21)app/server/db/db.ts (1)
db(14-14)app/server/db/schema.ts (1)
repositoriesTable(50-62)app/server/utils/id.ts (1)
generateShortId(3-6)app/server/utils/restic.ts (1)
restic(805-818)
🔇 Additional comments (10)
docker-compose.yml (1)
66-67: LGTM! Config file and data volume mounts are correctly configured.The read-only mount for the config file (
:ro) is appropriate since it should not be modified by the container.app/server/modules/repositories/repositories.service.ts (2)
66-80: LGTM! Local repository shortId handling with conflict detection.The logic correctly handles migration scenarios where
config.nameis provided for local backends, with proper conflict checking against existing repositories.
89-105: LGTM! Repository existence validation is well-implemented.The logic correctly:
- Detects if a restic repo already exists at the location
- Prevents accidental overwrite without explicit
isExistingRepositoryflag- Validates that imported repos actually exist and are accessible
app/server/modules/lifecycle/startup.ts (3)
58-76: LGTM! Recovery key validation is thorough and secure.The validation correctly:
- Checks type, length, and hex format
- Writes with restrictive permissions (0o600)
- Logs success without exposing the key value
82-111: LGTM! Resource creation with appropriate error isolation.Each resource type (volumes, repositories, notifications) is created independently with try-catch, preventing one failure from blocking others. Warnings are logged with helpful context.
191-225: LGTM! Admin setup with proper validation and migration support.The implementation correctly:
- Validates mutual exclusivity of
passwordandpasswordHash- Only creates admin when no users exist
- Supports migration with pre-hashed passwords
- Marks recovery key as downloaded when provided via config
README.md (4)
40-56: LGTM! Clear explanation of create-only semantics.The documentation clearly explains that the config file serves as "initial setup" rather than "desired state sync", which sets correct expectations for users.
154-168: LGTM! Local repository migration documentation is helpful.The note about importing existing local repositories with
config.nameandisExistingRepositoryis well-explained and aligns with the implementation inrepositories.service.ts.
385-405: LGTM! Recovery key documentation is comprehensive and security-focused.The warning about losing access to backups and credentials, along with generation commands for multiple platforms, is excellent user guidance.
430-430: LGTM! Config file mount instruction added to example.The read-only mount for the config file is correctly documented.
|
I need to check what changes will be needed regards the schedule naming change |
|
I was working earlier this week on the reset password feature and I started implementing a command pattern. I think this could be applied here as well. In order to avoid loading the stuff at startup and have all this runtime code the import feature would live only in 1 separated file and be executed like this: This would load automatically the file from /var/lib/zerobyte/data/import.json for example |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
README.md (1)
154-168: Clarify the relationship betweenconfig.nameand subdirectory naming.The documentation refers to
config.namebut this field is actually used as theshortId(subdirectory name) for local repositories. The distinction between the repository's display name (lines 159, 168) andconfig.namecould be clearer.Consider adding: "The
config.namevalue becomes the subdirectory name under the specified path, whilenameat the top level is the display name for the repository in the UI."docker-compose.yml (2)
42-65: Consider adding a security reminder comment.While placeholder values are appropriate for example configurations, it would be helpful to add a comment warning users not to commit real credentials to version control.
Consider adding above the environment section:
# WARNING: Replace placeholder values with real credentials # Do not commit real secrets to version control # Consider using .env files or Docker secrets for production environment:
69-70: Document that these are example paths.The mounted paths
./zerobyte.config.jsonand./mydataare examples and won't exist by default. Consider adding a comment to clarify these should be customized:- ./zerobyte.config.json:/app/zerobyte.config.json:ro # Create and customize this config file - ./mydata:/mydata:ro # Example: replace with your actual data pathapp/server/modules/repositories/repositories.service.ts (1)
101-105: Consider using a more specific error type.
InternalServerErrorsuggests a server-side bug, but "cannot access existing repository" is typically caused by incorrect user-provided credentials or paths. Consider usingBadRequestErroror a custom validation error to better indicate this is a client-side configuration issue.Apply this diff:
if (!repoExists && config.isExistingRepository) { - throw new InternalServerError( + throw new BadRequestError( `Cannot access existing repository. Verify the path/credentials are correct and the repository exists.` ); }Note: You'll need to import
BadRequestErrorfromhttp-errors-enhanced.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md(2 hunks)app/server/modules/backups/backups.service.ts(1 hunks)app/server/modules/repositories/repositories.service.ts(3 hunks)docker-compose.yml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/server/modules/backups/backups.service.ts
🧰 Additional context used
📓 Path-based instructions (2)
app/server/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use Hono as the web framework with Bun runtime for server implementation
Files:
app/server/modules/repositories/repositories.service.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Format and lint code using Biome with tabs (not spaces), 120 character line width, and double quotes
Do not auto-organize imports - imports organization is disabled in Biome
Use"type": "module"configuration - all imports must include extensions when targeting Node/Bun
Files:
app/server/modules/repositories/repositories.service.ts
🧠 Learnings (7)
📓 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.
📚 Learning: 2025-12-14T10:08:03.474Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T10:08:03.474Z
Learning: For repository backends, add backend type to `app/schemas/restic.ts` and update `buildRepoUrl()` in `app/server/utils/restic.ts`
Applied to files:
app/server/modules/repositories/repositories.service.ts
📚 Learning: 2025-12-03T19:48:55.119Z
Learnt from: nicotsx
Repo: nicotsx/zerobyte PR: 95
File: app/server/modules/backups/backups.service.ts:525-533
Timestamp: 2025-12-03T19:48:55.119Z
Learning: In app/server/modules/backups/backups.service.ts, the mirror copy operations use shared locks (acquireShared) for both source and mirror repositories because restic supports parallel write operations and concurrent backups with non-exclusive locks. Only maintenance operations like prune and rebuild-index require exclusive locks in restic.
Applied to files:
app/server/modules/repositories/repositories.service.ts
📚 Learning: 2025-12-14T10:08:03.474Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T10:08:03.474Z
Learning: Applies to app/server/utils/restic.ts : Restic password file requires 0600 permissions - never expose it in logs or error messages
Applied to files:
app/server/modules/repositories/repositories.service.ts
📚 Learning: 2025-12-14T10:08:03.474Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T10:08:03.474Z
Learning: Applies to app/server/utils/restic.ts : Store Restic password file at `/var/lib/zerobyte/restic/password` with 0600 permissions and never expose it
Applied to files:
app/server/modules/repositories/repositories.service.ts
📚 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:
app/server/modules/repositories/repositories.service.tsREADME.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
🧬 Code graph analysis (1)
app/server/modules/repositories/repositories.service.ts (4)
app/server/core/config.ts (1)
config(23-23)app/server/db/schema.ts (1)
repositoriesTable(50-62)app/server/utils/id.ts (1)
generateShortId(3-6)app/server/utils/restic.ts (1)
restic(805-818)
🔇 Additional comments (6)
README.md (3)
40-56: Clear documentation of config file behavior.The create-only semantics are well-explained with appropriate warnings about not syncing desired state. This sets correct expectations for users.
383-405: Excellent security documentation.The recovery key section clearly explains its dual purpose and provides practical generation commands. The prominent warning about data loss is appropriate given the criticality.
408-411: Good summary of key configuration points.Concise notes covering the essential requirements for configuration paths and credentials.
app/server/modules/repositories/repositories.service.ts (3)
67-80: Well-implemented migration support for local repositories.The shortId determination logic correctly:
- Allows user-provided subdirectory names for local repository migrations
- Validates uniqueness to prevent path collisions
- Falls back to generated IDs for new repositories
- Provides clear error messaging
This aligns with the migration documentation in README.md.
107-144: Solid error handling with rollback pattern.The repository creation flow correctly:
- Persists the encrypted configuration
- Conditionally initializes only new repositories
- Updates status on success
- Rolls back the database entry on failure
This transactional approach prevents orphaned database records.
19-52: ThesealSecretfunction inapp/server/utils/crypto.tsalready includes built-in double-encryption protection through the check:if (isEncrypted(value) || isSecretReference(value)) { return value; }. The code correctly usessealSecretwithout needing explicitisEncrypted()guards inencryptConfig, as the protection is handled internally.
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/server/modules/lifecycle/config-import.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-import.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-import.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-import.ts
🧠 Learnings (8)
📓 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.
📚 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-import.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/lifecycle/config-import.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-import.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-import.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 `buildRepoUrl()` in `app/server/utils/restic.ts` when adding a new repository backend
Applied to files:
app/server/modules/lifecycle/config-import.ts
📚 Learning: 2025-12-22T19:50:04.233Z
Learnt from: nicotsx
Repo: nicotsx/zerobyte PR: 197
File: app/client/modules/repositories/components/create-repository-form.tsx:112-112
Timestamp: 2025-12-22T19:50:04.233Z
Learning: When importing existing local repositories in the repositories service, users provide the full path (path + name) to an existing restic repository. Restic itself validates whether a valid repository exists at that location, so server-side path sanitization is not required for the import flow—the operation will fail if the path doesn't point to a valid restic repo.
Applied to files:
app/server/modules/lifecycle/config-import.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-import.ts
🔇 Additional comments (13)
app/server/modules/lifecycle/config-import.ts (13)
1-16: LGTM: Clean imports and type guards.The imports are well-organized and the helper functions
isRecordandasStringArrayprovide appropriate type narrowing for parsing untrusted config data.
18-41: LGTM: Well-structured type definitions.The type definitions clearly model the import configuration schema and result structure.
ImportResultis appropriately exported for use by the CLI command.
43-60: Environment variable handling already reviewed.This segment was previously reviewed regarding the handling of undefined environment variables. See existing review comments for details.
62-80: LGTM: Robust config file loading with graceful fallback.The function correctly handles missing files (ENOENT) by returning
null, allowing the application to proceed without configuration import. Other errors are logged as warnings without blocking startup, which aligns with the optional nature of config import.
82-101: LGTM: Flexible config parsing with safe defaults.The function gracefully handles both flat and nested config structures and applies environment variable interpolation. Safe defaults (empty arrays) prevent downstream null/undefined errors.
103-133: LGTM: Secure recovery key handling with proper validation.The recovery key validation (64-character hex) is appropriate for Restic keys, and the file mode
0o600ensures only the owner can read/write the passfile. The create-only semantics (skipping if passfile exists) prevents accidental overwrites.
135-160: LGTM: Volume import with appropriate error handling.The function validates volume entries before creation and properly handles the
autoRemountflag via a post-creation update. The create-only error handling (log warnings and continue) aligns with IaC semantics.
204-218: Repository existence check already reviewed.This segment was previously reviewed regarding potential false negatives when checking if a path is already a Restic repository. See existing review comments for details.
278-292: LGTM: Flexible field name handling.These helper functions gracefully handle field naming variations (
volumevsvolumeName,repositoryvsrepositoryName), improving config file compatibility.
402-413: Volume mount failure handling already reviewed.This segment was previously reviewed regarding the missing
result.warnings++increment in the catch block. See existing review comments for details.
456-501: LGTM: Robust mirror attachment with flexible field handling.The function handles field naming variations (
repositoryvsrepositoryName) and gracefully skips invalid or missing repositories with appropriate warnings.
519-571: LGTM: Well-structured initial user setup with clear constraints.The function properly handles the single-user limitation (warning users about extras), supports both plaintext password and pre-hashed password, and correctly derives the
hasDownloadedResticPasswordflag from the presence of a recovery key in the config.
573-634: LGTM: Clean orchestration and well-documented public APIs.The
runImportfunction executes imports in the correct order (recovery key → volumes → repositories → notifications → schedules → user), properly aggregates results, and provides appropriate logging. The exported functions are clearly documented and serve distinct use cases (CLI vs startup).
Improve error handling in repository existence check.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
app/server/modules/lifecycle/config-import.ts (6)
43-60: Existing concern about undefined environment variable handling.A past review comment already flagged that replacing undefined environment variables with empty strings could cause authentication failures or security misconfigurations for critical fields (passwords, API keys). Consider that feedback when finalizing.
164-165: Add.jsextensions to dynamic imports.These dynamic imports are missing the
.jsextension (flagged in a previous review). Update to:await import("../repositories/repositories.service.js") await import("../../utils/restic.js")
254-254: Add.jsextension to dynamic import.This dynamic import is missing the
.jsextension (flagged in a previous review). Update to:await import("../notifications/notifications.service.js")
367-367: Critical:destinationBySlugmap uses raw names but lookups use slugified names.This was flagged in a previous review. Line 318 slugifies
destNamefor lookup (slugify(destName, { lower: true, strict: true })), but this map is keyed by rawd.name. All notification lookups will fail.Fix by slugifying the keys:
const destinationBySlug = new Map( destinations.map((d) => [slugify(d.name, { lower: true, strict: true }), d] as const) );
358-359: Add.jsextensions to dynamic imports.These dynamic imports are missing the
.jsextension (flagged in a previous review). Update to:await import("../backups/backups.service.js") await import("../notifications/notifications.service.js")
511-511: Add.jsextension to dynamic import.This dynamic import is missing the
.jsextension (flagged in a previous review). Update to:await import("../auth/auth.service.js")
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/server/modules/lifecycle/config-import.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-import.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-import.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-import.ts
🧠 Learnings (12)
📓 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: 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
Learnt from: nicotsx
Repo: nicotsx/zerobyte PR: 197
File: app/client/modules/repositories/components/create-repository-form.tsx:112-112
Timestamp: 2025-12-22T19:50:04.233Z
Learning: When importing existing local repositories in the repositories service, users provide the full path (path + name) to an existing restic repository. Restic itself validates whether a valid repository exists at that location, so server-side path sanitization is not required for the import flow—the operation will fail if the path doesn't point to a valid restic repo.
📚 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-import.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/lifecycle/config-import.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-import.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-import.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 `buildRepoUrl()` in `app/server/utils/restic.ts` when adding a new repository backend
Applied to files:
app/server/modules/lifecycle/config-import.ts
📚 Learning: 2025-12-22T19:50:04.233Z
Learnt from: nicotsx
Repo: nicotsx/zerobyte PR: 197
File: app/client/modules/repositories/components/create-repository-form.tsx:112-112
Timestamp: 2025-12-22T19:50:04.233Z
Learning: When importing existing local repositories in the repositories service, users provide the full path (path + name) to an existing restic repository. Restic itself validates whether a valid repository exists at that location, so server-side path sanitization is not required for the import flow—the operation will fail if the path doesn't point to a valid restic repo.
Applied to files:
app/server/modules/lifecycle/config-import.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-import.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-import.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-import.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-import.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-import.ts
🧬 Code graph analysis (1)
app/server/modules/lifecycle/config-import.ts (11)
app/server/utils/logger.ts (1)
logger(36-41)app/server/core/config.ts (1)
config(28-28)app/server/core/constants.ts (1)
RESTIC_PASS_FILE(5-5)app/server/modules/volumes/volume.service.ts (1)
volumeService(403-415)app/schemas/volumes.ts (1)
BackendConfig(81-81)app/server/utils/restic.ts (2)
buildRepoUrl(75-106)restic(836-849)app/schemas/restic.ts (1)
RepositoryConfig(95-95)app/schemas/notifications.ts (1)
NotificationConfig(90-90)app/server/db/db.ts (1)
db(14-14)app/server/modules/auth/auth.service.ts (1)
hasUsers(146-149)app/server/db/schema.ts (1)
usersTable(29-36)
🪛 GitHub Actions: Checks
app/server/modules/lifecycle/config-import.ts
[error] 209-209: File content differs from formatting output. Formatting check failed during 'bun run lint:ci'.
🔇 Additional comments (8)
app/server/modules/lifecycle/config-import.ts (8)
11-41: LGTM!Type guards and type definitions are well-structured. The
isRecordguard correctly handles null, and theRetentionPolicy/ImportConfig/ImportResulttypes provide clear contracts for the import flow.
62-80: LGTM!Clean error handling: returns
nullon missing file (ENOENT) and logs a warning on parse errors, allowing callers to gracefully skip import when no config exists.
82-101: LGTM!Flexible parsing that supports both root-level and nested config structures. Proper coercion ensures type safety downstream.
103-133: LGTM!Recovery key handling is well-implemented: validates 64-char hex format, uses restrictive permissions (0o600), and respects create-only semantics by skipping if the passfile already exists.
135-160: LGTM!Volume import correctly validates entries, creates volumes, and handles the
autoRemountflag in a follow-up update. Error isolation ensures one failed volume doesn't block others.
369-458: LGTM!Backup schedule import is well-implemented: validates volume/repository references, mounts volumes when needed (with proper warning increment on failure), creates schedules with retention policies and patterns, and attaches notifications/mirrors.
507-575: LGTM!User setup correctly handles both pre-hashed passwords (direct DB insert for IaC) and plaintext passwords (via
authService.register). ThehasDownloadedResticPasswordflag is properly set based on recovery key presence, and the single-user constraint is enforced with a break after the first successful user.
577-638: LGTM!Clean orchestration:
runImportaggregates results from all import operations,logImportSummaryprovides appropriate log levels based on outcome, and the public API (applyConfigImport,applyConfigImportFromFile) offers both programmatic and file-based entry points.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
app/server/modules/lifecycle/config-import.ts (5)
3-6: Missing.jsextensions on relative imports.Per the coding guidelines, all relative imports must include file extensions when using
"type": "module". This issue was previously flagged and should be addressed.🔎 Proposed fix
-import { db } from "../../db/db"; -import { usersTable } from "../../db/schema"; -import { logger } from "../../utils/logger"; -import { volumeService } from "../volumes/volume.service"; +import { db } from "../../db/db.js"; +import { usersTable } from "../../db/schema.js"; +import { logger } from "../../utils/logger.js"; +import { volumeService } from "../volumes/volume.service.js";
164-165: Missing.jsextensions on dynamic imports.These dynamic imports require
.jsextensions for ESM compatibility. This was previously flagged.🔎 Proposed fix
- const repoServiceModule = await import("../repositories/repositories.service"); - const { buildRepoUrl, restic } = await import("../../utils/restic"); + const repoServiceModule = await import("../repositories/repositories.service.js"); + const { buildRepoUrl, restic } = await import("../../utils/restic.js");
254-254: Missing.jsextension on dynamic import.This dynamic import requires a
.jsextension. Previously flagged.🔎 Proposed fix
- const notificationsServiceModule = await import("../notifications/notifications.service"); + const notificationsServiceModule = await import("../notifications/notifications.service.js");
358-359: Missing.jsextensions on dynamic imports.These dynamic imports require
.jsextensions. Previously flagged.🔎 Proposed fix
- const backupServiceModule = await import("../backups/backups.service"); - const notificationsServiceModule = await import("../notifications/notifications.service"); + const backupServiceModule = await import("../backups/backups.service.js"); + const notificationsServiceModule = await import("../notifications/notifications.service.js");
511-511: Missing.jsextension on dynamic import.This dynamic import requires a
.jsextension. Previously flagged.🔎 Proposed fix
- const { authService } = await import("../auth/auth.service"); + const { authService } = await import("../auth/auth.service.js");
🧹 Nitpick comments (1)
app/server/modules/lifecycle/config-import.ts (1)
11-11: Type guardisRecordreturnstruefor arrays.
typeof value === "object"returnstruefor arrays. IfisRecordis called directly (not after anArray.isArraycheck), it will incorrectly returntruefor arrays. Consider adding an array exclusion for defensive correctness.🔎 Proposed fix
-const isRecord = (value: unknown): value is Record<string, unknown> => typeof value === "object" && value !== null; +const isRecord = (value: unknown): value is Record<string, unknown> => + typeof value === "object" && value !== null && !Array.isArray(value);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/server/modules/lifecycle/config-import.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-import.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-import.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-import.ts
🧠 Learnings (14)
📓 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: 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
📚 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-import.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/lifecycle/config-import.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 : Restic password file is stored in `/var/lib/zerobyte/restic/password` and auto-generated on first run
Applied to files:
app/server/modules/lifecycle/config-import.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-import.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-import.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 `buildRepoUrl()` in `app/server/utils/restic.ts` when adding a new repository backend
Applied to files:
app/server/modules/lifecycle/config-import.ts
📚 Learning: 2025-12-22T19:50:04.233Z
Learnt from: nicotsx
Repo: nicotsx/zerobyte PR: 197
File: app/client/modules/repositories/components/create-repository-form.tsx:112-112
Timestamp: 2025-12-22T19:50:04.233Z
Learning: When importing existing local repositories in the repositories service, users provide the full path (path + name) to an existing restic repository. Restic itself validates whether a valid repository exists at that location, so server-side path sanitization is not required for the import flow—the operation will fail if the path doesn't point to a valid restic repo.
Applied to files:
app/server/modules/lifecycle/config-import.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-import.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-import.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-import.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-import.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-import.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/drizzle/**/*.{ts,json} : If an automated migration is incorrect, remove all associated entries from `_journal.json` and newly created files in `app/drizzle/` before re-generating the migration
Applied to files:
app/server/modules/lifecycle/config-import.ts
🔇 Additional comments (6)
app/server/modules/lifecycle/config-import.ts (6)
62-80: LGTM!The file loading logic correctly distinguishes between "file not found" (returns null) and actual errors. The nested try-catch structure and ENOENT check are appropriate.
103-133: LGTM!The recovery key handling is well-implemented with proper validation (64-char hex), idempotent behavior (skips if file exists), and secure file permissions (
0o600).
135-160: LGTM!The volume import logic validates entry structure, handles
autoRemountconfiguration, and properly tracks results. Error handling follows create-only semantics.
282-352: LGTM!The schedule helper functions are well-structured with:
- Flexible field name support (
volume/volumeName,repository/repositoryName)- Sensible defaults for notification flags (all
true)- Proper error handling in
attachScheduleNotifications
460-505: LGTM!The mirror attachment logic follows the same robust pattern as notification attachment with proper validation, flexible field names, and comprehensive error handling.
577-638: LGTM!The orchestration layer is well-structured:
- Correct import order (recovery key → volumes → repos → notifications → schedules → users)
- Clean separation between file loading, parsing, execution, and logging
- Proper result aggregation from all import functions
…potent re-runs - Add `--json` flag for machine-readable JSON output - Add `--log-level` flag to control logging verbosity - Add `skipped` counter to ImportResult for idempotent operations - Change "already exists" conditions from warnings to skipped (info logs) - Recovery key mismatch now errors and stops import early - Pre-check volumes and notification destinations before creation - Attachment functions merge missing items instead of overwriting - Add toError() and mergeResults() helpers to reduce code duplication - Extract readConfigJson() and createOutput() for cleaner CLI code - Move fs/path imports to top level in config-import.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
app/server/modules/lifecycle/config-import.ts (6)
48-65: Consider stricter handling of undefined environment variables for critical fields.The function warns but proceeds with an empty string when an environment variable is undefined (lines 51-54). For sensitive or critical values (passwords, API keys, connection strings), this silent fallback could lead to authentication failures with non-obvious error messages or security misconfigurations.
While the resilient approach enables partial imports, consider adding validation for known-critical fields or providing a strict mode that fails fast on undefined variables.
🔎 Example: Pre-validate critical environment variables
Add validation before interpolation for known-sensitive patterns:
function validateCriticalEnvVars(config: unknown): void { const criticalPatterns = [ /password/i, /secret/i, /key/i, /token/i, ]; const configStr = JSON.stringify(config); const envVarMatches = configStr.matchAll(/\$\{([^}]+)\}/g); for (const match of envVarMatches) { const varName = match[1]; if (process.env[varName] === undefined && criticalPatterns.some(p => p.test(varName))) { throw new Error(`Critical environment variable '${varName}' is not defined`); } } }Call this before
interpolateEnvVarsinparseImportConfig.
5-8: Add.jsextensions to static imports.These static imports are missing the
.jsextension required when"type": "module"is set inpackage.json. This will cause runtime errors.🔎 Proposed fix
-import { db } from "../../db/db"; -import { usersTable } from "../../db/schema"; -import { logger } from "../../utils/logger"; -import { volumeService } from "../volumes/volume.service"; +import { db } from "../../db/db.js"; +import { usersTable } from "../../db/schema.js"; +import { logger } from "../../utils/logger.js"; +import { volumeService } from "../volumes/volume.service.js";As per coding guidelines, all imports must include file extensions when targeting Node/Bun.
189-190: Add.jsextensions to dynamic imports.These dynamic imports are missing the
.jsextension required when"type": "module"is set inpackage.json. This will cause runtime errors.🔎 Proposed fix
- const repoServiceModule = await import("../repositories/repositories.service"); - const { buildRepoUrl, restic } = await import("../../utils/restic"); + const repoServiceModule = await import("../repositories/repositories.service.js"); + const { buildRepoUrl, restic } = await import("../../utils/restic.js");
276-276: Add.jsextension to dynamic import.This dynamic import is missing the
.jsextension required when"type": "module"is set inpackage.json. This will cause a runtime error.🔎 Proposed fix
- const notificationsServiceModule = await import("../notifications/notifications.service"); + const notificationsServiceModule = await import("../notifications/notifications.service.js");
436-437: Add.jsextensions to dynamic imports.These dynamic imports are missing the
.jsextension required when"type": "module"is set inpackage.json. This will cause runtime errors.🔎 Proposed fix
- const backupServiceModule = await import("../backups/backups.service"); - const notificationsServiceModule = await import("../notifications/notifications.service"); + const backupServiceModule = await import("../backups/backups.service.js"); + const notificationsServiceModule = await import("../notifications/notifications.service.js");
641-641: Add.jsextension to dynamic import.This dynamic import is missing the
.jsextension required when"type": "module"is set inpackage.json. This will cause a runtime error.🔎 Proposed fix
- const { authService } = await import("../auth/auth.service"); + const { authService } = await import("../auth/auth.service.js");
🧹 Nitpick comments (2)
app/server/modules/lifecycle/config-import.ts (2)
228-234: Consider discriminating error types in repository existence check.The
restic.snapshots()catch-all returnsfalsefor any error (line 231-234), which could mask real failures like permission errors or network issues for remote backends. While the debug logging helps, treating all failures as "not a repo" could lead to attempting initialization over a temporarily inaccessible repository.🔎 Proposed improvement
Discriminate between "not a repository" and other errors:
const isAlreadyRepo = await restic .snapshots({ ...r.config, isExistingRepository: true } as RepositoryConfig) .then(() => true) .catch((e) => { - logger.debug(`Repo existence check for '${r.name}': ${toError(e).message}`); - return false; + const err = toError(e); + const msg = err.message.toLowerCase(); + // Restic returns specific errors for "not a repository" + if (msg.includes("is not a repository") || msg.includes("wrong password")) { + logger.debug(`'${r.name}' is not an existing repo: ${err.message}`); + return false; + } + // Other errors (permission, network) should be logged as warnings + logger.warn(`Could not check if '${r.name}' is a repo: ${err.message}`); + return true; // Assume it might exist to prevent accidental overwrite });
671-687: Consider validating passwordHash format.Lines 673-678 insert a
passwordHashdirectly into the database without validation. If the hash is malformed or not a valid bcrypt hash, authentication will fail later with unclear errors. While this flow supports migration scenarios where hashes come from exports, basic format validation (e.g., checking for bcrypt prefix$2a$,$2b$, or$2y$) could catch configuration errors earlier.🔎 Proposed validation
Add validation before the insert:
if (typeof u.passwordHash === "string" && u.passwordHash.length > 0) { // Basic bcrypt format check if (!/^\$2[aby]\$\d{2}\$[./A-Za-z0-9]{53}$/.test(u.passwordHash)) { logger.warn(`User '${u.username}' has invalid passwordHash format; skipping`); result.warnings++; continue; } try { await db.insert(usersTable).values({ // ... rest of the code
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/server/modules/lifecycle/config-import.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-import.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-import.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-import.ts
🧠 Learnings (14)
📓 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.
📚 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-import.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-import.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-import.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-import.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-import.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-import.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 `buildRepoUrl()` in `app/server/utils/restic.ts` when adding a new repository backend
Applied to files:
app/server/modules/lifecycle/config-import.ts
📚 Learning: 2025-12-22T19:50:04.233Z
Learnt from: nicotsx
Repo: nicotsx/zerobyte PR: 197
File: app/client/modules/repositories/components/create-repository-form.tsx:112-112
Timestamp: 2025-12-22T19:50:04.233Z
Learning: When importing existing local repositories in the repositories service, users provide the full path (path + name) to an existing restic repository. Restic itself validates whether a valid repository exists at that location, so server-side path sanitization is not required for the import flow—the operation will fail if the path doesn't point to a valid restic repo.
Applied to files:
app/server/modules/lifecycle/config-import.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-import.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-import.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-import.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/drizzle/**/*.{ts,json} : If an automated migration is incorrect, remove all associated entries from `_journal.json` and newly created files in `app/drizzle/` before re-generating the migration
Applied to files:
app/server/modules/lifecycle/config-import.ts
📚 Learning: 2025-11-30T15:18:38.913Z
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 89
File: app/server/modules/notifications/builders/telegram.ts:3-8
Timestamp: 2025-11-30T15:18:38.913Z
Learning: In app/server/modules/notifications/builders/telegram.ts, the Telegram bot token in Shoutrrr URLs must NOT be URL-encoded. When passed to the Shoutrrr CLI, encoding the bot token (e.g., encoding the colon as %3A) causes "failed to initialize service" errors. The token should be used raw in the URL userinfo section. Chat IDs in the channels query parameter can optionally be encoded but work fine unencoded as well.
Applied to files:
app/server/modules/lifecycle/config-import.ts
🔇 Additional comments (1)
app/server/modules/lifecycle/config-import.ts (1)
116-116: Add.jsextension to dynamic import.This dynamic import is missing the
.jsextension required when"type": "module"is set inpackage.json. This will cause a runtime error.🔎 Proposed fix
- const { RESTIC_PASS_FILE } = await import("../../core/constants.js"); + const { RESTIC_PASS_FILE } = await import("../../core/constants.js");Wait, looking at the code again - it already has
.js! Let me re-read line 116...Actually, looking at the past review comments, line 116 was flagged but I need to check the actual code. The annotated code shows:
This already has the .js extension! So this is actually correct. Let me skip this.
⛔ Skipped due to learnings
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 : Restic password file is stored in `/var/lib/zerobyte/restic/password` and auto-generated on first runLearnt 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 backendLearnt 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 : Restic password file must have 0600 permissions and should never be exposedLearnt 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 : Restic cache is stored in `/var/lib/zerobyte/restic/cache`
…up and pass shortIds to service creation from config import
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/server/modules/backups/backups.service.tsapp/server/modules/lifecycle/config-import.tsapp/server/modules/repositories/repositories.service.tsapp/server/modules/volumes/volume.service.tsapp/server/utils/id.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/server/modules/lifecycle/config-import.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/volumes/volume.service.tsapp/server/modules/backups/backups.service.tsapp/server/utils/id.tsapp/server/modules/repositories/repositories.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/volumes/volume.service.tsapp/server/modules/backups/backups.service.tsapp/server/utils/id.tsapp/server/modules/repositories/repositories.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/volumes/volume.service.tsapp/server/modules/backups/backups.service.tsapp/server/modules/repositories/repositories.service.ts
app/server/modules/repositories/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Local repositories are stored in
/var/lib/zerobyte/repositories/<repo-name>
Files:
app/server/modules/repositories/repositories.service.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.
📚 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/volumes/volume.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/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/volumes/volume.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/backends/**/*.ts : Each volume backend (NFS, SMB, WebDAV, SFTP, directory) must implement mounting logic using system tools
Applied to files:
app/server/modules/volumes/volume.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/backends/**/*.ts : Volume mounts are stored in `/var/lib/zerobyte/mounts/<volume-name>`
Applied to files:
app/server/modules/volumes/volume.service.ts
📚 Learning: 2025-12-03T19:48:55.119Z
Learnt from: nicotsx
Repo: nicotsx/zerobyte PR: 95
File: app/server/modules/backups/backups.service.ts:525-533
Timestamp: 2025-12-03T19:48:55.119Z
Learning: In app/server/modules/backups/backups.service.ts, the mirror copy operations use shared locks (acquireShared) for both source and mirror repositories because restic supports parallel write operations and concurrent backups with non-exclusive locks. Only maintenance operations like prune and rebuild-index require exclusive locks in restic.
Applied to files:
app/server/modules/backups/backups.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 : Update `buildRepoUrl()` in `app/server/utils/restic.ts` when adding a new repository backend
Applied to files:
app/server/modules/backups/backups.service.tsapp/server/modules/repositories/repositories.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 : 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/backups/backups.service.tsapp/server/modules/repositories/repositories.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/repositories/repositories.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/**/*.ts : Local repositories are stored in `/var/lib/zerobyte/repositories/<repo-name>`
Applied to files:
app/server/modules/repositories/repositories.service.ts
📚 Learning: 2025-12-22T19:50:04.233Z
Learnt from: nicotsx
Repo: nicotsx/zerobyte PR: 197
File: app/client/modules/repositories/components/create-repository-form.tsx:112-112
Timestamp: 2025-12-22T19:50:04.233Z
Learning: When importing existing local repositories in the repositories service, users provide the full path (path + name) to an existing restic repository. Restic itself validates whether a valid repository exists at that location, so server-side path sanitization is not required for the import flow—the operation will fail if the path doesn't point to a valid restic repo.
Applied to files:
app/server/modules/repositories/repositories.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/repositories/repositories.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 is stored in `/var/lib/zerobyte/restic/password` and auto-generated on first run
Applied to files:
app/server/modules/repositories/repositories.service.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/repositories/repositories.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 **/*.{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/repositories/repositories.service.ts
🧬 Code graph analysis (2)
app/server/modules/backups/backups.service.ts (2)
app/server/utils/id.ts (2)
isValidShortId(10-13)generateShortId(5-8)app/server/db/schema.ts (1)
backupSchedulesTable(70-102)
app/server/modules/repositories/repositories.service.ts (4)
app/schemas/restic.ts (2)
RepositoryConfig(95-95)CompressionMode(103-103)app/server/utils/id.ts (2)
isValidShortId(10-13)generateShortId(5-8)app/server/db/schema.ts (1)
repositoriesTable(51-63)app/server/utils/restic.ts (1)
restic(836-849)
🪛 ast-grep (0.40.3)
app/server/utils/id.ts
[warning] 10-10: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^[A-Za-z0-9_-]{${length}}$)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (2)
app/server/utils/id.ts (1)
10-13: Static analysis false positive: regex is safe from ReDoS.The static analysis tool flagged the
RegExpconstructor as a potential ReDoS risk. However, this is a false positive because:
- The
lengthparameter is numeric (defaults toSHORT_ID_LENGTH = 8), not user-supplied string input- The pattern
^[A-Za-z0-9_-]{n}$is a simple character class with a fixed quantifier—no nested quantifiers or backtracking- Time complexity is O(n) where n is the string length being tested
The regex safely validates that a string contains exactly
lengthbase64url characters.app/server/modules/backups/backups.service.ts (1)
98-115: UseBadRequestErrorfor consistent error handling.Line 102 throws a generic
Errorfor invalid shortId format, while line 108 correctly usesConflictErrorfor duplicates. For consistency with the rest of the service (e.g., lines 87, 170), invalid input should throwBadRequestError.🔎 Proposed fix
if (providedShortId) { if (!isValidShortId(providedShortId)) { - throw new Error(`Invalid shortId format: '${providedShortId}'. Must be 8 base64url characters.`); + throw new BadRequestError(`Invalid shortId format: '${providedShortId}'. Must be 8 base64url characters.`); } const shortIdInUse = await db.query.backupSchedulesTable.findFirst({Likely an incorrect or invalid review comment.
…revent overwrite if database is not empty
… repository and volume creation
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
app/server/cli/commands/import-config.ts (1)
134-137: Add.jsextensions to dynamic imports for ESM compatibility.Per the coding guidelines, imports must include file extensions when targeting Node/Bun with
"type": "module". Dynamic imports should also follow this convention.🔎 Proposed fix
- const { runDbMigrations } = await import("../../db/db"); + const { runDbMigrations } = await import("../../db/db.js"); runDbMigrations(); - const { applyConfigImport } = await import("../../modules/lifecycle/config-import"); + const { applyConfigImport } = await import("../../modules/lifecycle/config-import.js");app/server/modules/lifecycle/config-import.ts (5)
4-14: Add.jsextensions to static imports.Static imports from relative paths require the
.jsextension when"type": "module"is set. These will cause runtime errors.🔎 Proposed fix
-import { db } from "../../db/db"; +import { db } from "../../db/db.js"; import { usersTable, volumesTable, repositoriesTable, backupSchedulesTable, notificationDestinationsTable, -} from "../../db/schema"; -import { logger } from "../../utils/logger"; -import { toError } from "../../utils/errors"; -import { volumeService } from "../volumes/volume.service"; +} from "../../db/schema.js"; +import { logger } from "../../utils/logger.js"; +import { toError } from "../../utils/errors.js"; +import { volumeService } from "../volumes/volume.service.js";
222-223: Add.jsextensions to dynamic imports.🔎 Proposed fix
- const repoServiceModule = await import("../repositories/repositories.service"); - const { buildRepoUrl, restic } = await import("../../utils/restic"); + const repoServiceModule = await import("../repositories/repositories.service.js"); + const { buildRepoUrl, restic } = await import("../../utils/restic.js");
313-313: Add.jsextension to dynamic import.🔎 Proposed fix
- const notificationsServiceModule = await import("../notifications/notifications.service"); + const notificationsServiceModule = await import("../notifications/notifications.service.js");
474-475: Add.jsextensions to dynamic imports.🔎 Proposed fix
- const backupServiceModule = await import("../backups/backups.service"); - const notificationsServiceModule = await import("../notifications/notifications.service"); + const backupServiceModule = await import("../backups/backups.service.js"); + const notificationsServiceModule = await import("../notifications/notifications.service.js");
692-692: Add.jsextension to dynamic import.🔎 Proposed fix
- const { authService } = await import("../auth/auth.service"); + const { authService } = await import("../auth/auth.service.js");
🧹 Nitpick comments (1)
app/server/cli/commands/import-config.ts (1)
106-130: Consider enhancing dry-run validation feedback.The dry-run mode validates JSON structure and counts items, but doesn't validate individual item schemas (e.g., required fields, valid backends). This is fine for a quick check, but users might expect deeper validation.
Consider adding a note in the output or documentation clarifying that dry-run only validates JSON syntax and structure, not the semantic correctness of each item.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/server/cli/commands/import-config.tsapp/server/modules/lifecycle/config-import.tsapp/server/utils/errors.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/utils/errors.tsapp/server/cli/commands/import-config.tsapp/server/modules/lifecycle/config-import.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/utils/errors.tsapp/server/cli/commands/import-config.tsapp/server/modules/lifecycle/config-import.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-import.ts
🧠 Learnings (14)
📓 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.
📚 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 **/*.{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/cli/commands/import-config.tsapp/server/modules/lifecycle/config-import.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 **/*.{ts,tsx,js,jsx} : Do not auto-organize imports - imports organization is disabled in Biome
Applied to files:
app/server/cli/commands/import-config.tsapp/server/modules/lifecycle/config-import.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 **/*.{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/cli/commands/import-config.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-import.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/server/modules/lifecycle/config-import.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-import.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/server/modules/lifecycle/config-import.ts
📚 Learning: 2025-12-22T19:50:04.233Z
Learnt from: nicotsx
Repo: nicotsx/zerobyte PR: 197
File: app/client/modules/repositories/components/create-repository-form.tsx:112-112
Timestamp: 2025-12-22T19:50:04.233Z
Learning: When importing existing local repositories in the repositories service, users provide the full path (path + name) to an existing restic repository. Restic itself validates whether a valid repository exists at that location, so server-side path sanitization is not required for the import flow—the operation will fail if the path doesn't point to a valid restic repo.
Applied to files:
app/server/modules/lifecycle/config-import.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/server/modules/lifecycle/config-import.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/lifecycle/config-import.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/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-import.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/drizzle/**/*.{ts,json} : If an automated migration is incorrect, remove all associated entries from `_journal.json` and newly created files in `app/drizzle/` before re-generating the migration
Applied to files:
app/server/modules/lifecycle/config-import.ts
📚 Learning: 2025-11-30T15:18:38.913Z
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 89
File: app/server/modules/notifications/builders/telegram.ts:3-8
Timestamp: 2025-11-30T15:18:38.913Z
Learning: In app/server/modules/notifications/builders/telegram.ts, the Telegram bot token in Shoutrrr URLs must NOT be URL-encoded. When passed to the Shoutrrr CLI, encoding the bot token (e.g., encoding the colon as %3A) causes "failed to initialize service" errors. The token should be used raw in the URL userinfo section. Chat IDs in the channels query parameter can optionally be encoded but work fine unencoded as well.
Applied to files:
app/server/modules/lifecycle/config-import.ts
🧬 Code graph analysis (1)
app/server/cli/commands/import-config.ts (4)
app/server/utils/errors.ts (1)
toError(21-21)app/server/core/config.ts (1)
config(32-32)app/server/db/db.ts (1)
runDbMigrations(16-30)app/server/modules/lifecycle/config-import.ts (1)
applyConfigImport(815-821)
🔇 Additional comments (17)
app/server/utils/errors.ts (1)
21-22: LGTM!Clean utility function that complements the existing
toMessagehelper. The implementation correctly handles the unknown type and provides consistent error normalization across the codebase.app/server/cli/commands/import-config.ts (3)
8-14: LGTM!Clean stdin reading implementation using async iteration.
16-37: LGTM!Well-structured output abstraction that cleanly handles both JSON and human-readable modes. The
neverreturn type onerrorcorrectly indicates non-returning behavior.
39-62: LGTM!The function correctly handles both stdin and file input paths. The precondition checks at lines 83-94 ensure either
--configor--stdinis provided before this function is called.app/server/modules/lifecycle/config-import.ts (13)
19-24: LGTM!Clean type guard and array coercion utilities.
52-69: Environment variable interpolation works correctly.The function handles missing env vars by warning and substituting empty strings. This is a reasonable default for partial imports, though sensitive fields (passwords, keys) may benefit from stricter validation in critical deployments.
103-118: LGTM!Efficient parallel query approach for checking database emptiness. Good safety mechanism for recovery key overwrite protection.
120-176: LGTM!Robust recovery key handling with proper validation (64-char hex), secure file permissions (0o600), and comprehensive safety checks preventing accidental overwrites of existing keys when data exists.
178-218: LGTM!Clean implementation with proper duplicate detection using slugified names, shortId support for IaC reproducibility, and appropriate error handling.
258-277: Good defensive check for existing restic repositories.The logic correctly detects when a location is already initialized as a restic repository and warns the user to set
isExistingRepository: trueif they want to import it. The debug logging on line 265 aids troubleshooting.
355-413: LGTM!Flexible schema handling that accepts both
volume/volumeNameandrepository/repositoryNamekeys improves config file ergonomics. The notification assignment builder correctly uses slugified lookups.
415-468: LGTM!Correctly implements idempotent notification attachment with proper deduplication and merge semantics.
527-576: Good idempotent schedule handling.The logic correctly skips existing schedules while still attempting to attach notifications and mirrors. Volume mount failures now properly increment warnings (line 544).
606-686: LGTM!Mirrors attachment logic correctly follows the same idempotent pattern as notifications, with proper deduplication and merge semantics.
710-762: LGTM!User import correctly handles both pre-hashed passwords (for migration/backup restore) and plaintext passwords (for IaC), with proper single-user enforcement via
breakafter successful creation.
776-793: LGTM!Clean orchestration with appropriate early termination on recovery key errors. The import order (recovery key → volumes → repositories → notifications → schedules → users) ensures dependencies are available when needed.
795-821: LGTM!Good summary logging with appropriate severity levels based on the result type (errors → error level, warnings → warn level, success → info level).
- Add app/schemas/config-import.ts with ArkType schemas for upfront validation - Replace runtime type checks with schema-based validation in config-import.ts - Return validation errors early with detailed path and message info - Export retentionPolicySchema from backups.dto.ts for reuse - Update CLI to handle validation errors in both dry-run and import modes - Fix README mirrors examples to use existing repository names
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
app/server/cli/commands/import-config.ts (1)
53-58: Potential UX issue: empty config path resolves to current directory.When
options.configis undefined (which shouldn't happen given the check at line 83-89),path.resolve(process.cwd(), "")resolves to the current working directory. While the subsequentfs.accesscheck will fail with a directory, the error message would be confusing.Consider adding explicit handling:
🔎 Proposed fix
- const configPath = path.resolve(process.cwd(), options.config ?? ""); + if (!options.config) { + out.error("Config path is required"); + } + const configPath = path.resolve(process.cwd(), options.config);app/server/modules/lifecycle/config-import.ts (2)
40-57: Consider documenting the lenient env var interpolation behavior.The function warns and substitutes empty strings for undefined variables. While this enables partial imports, users may not realize a typo in
${VAR_NAME}silently becomes an empty value. The warning log helps, but consider adding a note in the README about this behavior.
541-605: Type mismatch inrepoByNamemap definition.The map is typed as
Map<string, { id: string; name: string; config: RepositoryConfig }>at line 545, butrepoByNameis created fromdb.query.repositoriesTable.findMany()at line 437 which returns database rows whereconfigis stored as JSON (likelyunknownor a broader type). The actual usage only accessesidandname, so this works at runtime but the type annotation is misleading.🔎 Proposed fix
async function attachScheduleMirrors( scheduleId: number, scheduleName: string, mirrors: ScheduleMirrorImport[], - repoByName: Map<string, { id: string; name: string; config: RepositoryConfig }>, + repoByName: Map<string, { id: string; name: string }>, backupServiceModule: typeof import("../backups/backups.service"), ): Promise<ImportResult> {app/schemas/config-import.ts (1)
77-82: Schema allows bothpasswordandpasswordHashsimultaneously.The user import schema accepts both
passwordandpasswordHashas optional fields without enforcing mutual exclusivity. While the import logic atconfig-import.ts:630-664correctly prioritizespasswordHashand then falls back topassword, the schema could be stricter.This is a low-priority improvement since the runtime logic handles it correctly.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/schemas/config-import.tsapp/server/cli/commands/import-config.tsapp/server/modules/backups/backups.dto.tsapp/server/modules/lifecycle/config-import.tsexamples/config-file-import/README.md
🧰 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/schemas/config-import.tsapp/server/modules/backups/backups.dto.tsapp/server/cli/commands/import-config.tsapp/server/modules/lifecycle/config-import.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/schemas/config-import.tsapp/server/modules/backups/backups.dto.tsapp/server/cli/commands/import-config.tsapp/server/modules/lifecycle/config-import.ts
app/schemas/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Prefer ArkType over Zod for runtime schema validation
Files:
app/schemas/config-import.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/backups/backups.dto.tsapp/server/modules/lifecycle/config-import.ts
🧠 Learnings (18)
📓 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.
📚 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/schemas/config-import.tsapp/server/modules/backups/backups.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/**/*.ts : Prefer ArkType over Zod for runtime schema validation
Applied to files:
app/schemas/config-import.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/schemas/config-import.tsapp/server/modules/backups/backups.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 **/*.{ts,tsx,js,jsx} : Do not auto-organize imports - imports organization is disabled in Biome
Applied to files:
app/server/cli/commands/import-config.tsapp/server/modules/lifecycle/config-import.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 **/*.{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/cli/commands/import-config.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/cli/commands/import-config.tsexamples/config-file-import/README.md
📚 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/server/modules/lifecycle/config-import.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-import.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-import.tsexamples/config-file-import/README.md
📚 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/server/modules/lifecycle/config-import.ts
📚 Learning: 2025-12-22T19:50:04.233Z
Learnt from: nicotsx
Repo: nicotsx/zerobyte PR: 197
File: app/client/modules/repositories/components/create-repository-form.tsx:112-112
Timestamp: 2025-12-22T19:50:04.233Z
Learning: When importing existing local repositories in the repositories service, users provide the full path (path + name) to an existing restic repository. Restic itself validates whether a valid repository exists at that location, so server-side path sanitization is not required for the import flow—the operation will fail if the path doesn't point to a valid restic repo.
Applied to files:
app/server/modules/lifecycle/config-import.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/server/modules/lifecycle/config-import.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/lifecycle/config-import.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/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-import.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/drizzle/**/*.{ts,json} : If an automated migration is incorrect, remove all associated entries from `_journal.json` and newly created files in `app/drizzle/` before re-generating the migration
Applied to files:
app/server/modules/lifecycle/config-import.ts
📚 Learning: 2025-11-30T15:18:38.913Z
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 89
File: app/server/modules/notifications/builders/telegram.ts:3-8
Timestamp: 2025-11-30T15:18:38.913Z
Learning: In app/server/modules/notifications/builders/telegram.ts, the Telegram bot token in Shoutrrr URLs must NOT be URL-encoded. When passed to the Shoutrrr CLI, encoding the bot token (e.g., encoding the colon as %3A) causes "failed to initialize service" errors. The token should be used raw in the URL userinfo section. Chat IDs in the channels query parameter can optionally be encoded but work fine unencoded as well.
Applied to files:
app/server/modules/lifecycle/config-import.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:
examples/config-file-import/README.md
🧬 Code graph analysis (2)
app/schemas/config-import.ts (4)
app/schemas/volumes.ts (1)
volumeConfigSchema(79-79)app/schemas/restic.ts (1)
repositoryConfigSchema(93-93)app/schemas/notifications.ts (1)
notificationConfigSchema(88-88)app/server/modules/backups/backups.dto.ts (1)
retentionPolicySchema(6-14)
app/server/cli/commands/import-config.ts (3)
app/server/utils/errors.ts (1)
toError(21-21)app/server/modules/lifecycle/config-import.ts (2)
validateConfig(746-748)applyConfigImport(726-740)app/server/db/db.ts (1)
runDbMigrations(16-30)
🪛 LanguageTool
examples/config-file-import/README.md
[style] ~440-~440: To form a complete sentence, be sure to include a subject or ‘there’.
Context: ...tifications (detailed) notifications can be strings (destination names) or objec...
(MISSING_IT_THERE)
🔇 Additional comments (22)
app/server/modules/backups/backups.dto.ts (1)
6-14: LGTM! Exporting the schema enables proper reuse.Exporting
retentionPolicySchemaallows the config-import module to reference the canonical retention policy definition, avoiding schema duplication.examples/config-file-import/README.md (1)
1-519: Comprehensive and well-structured documentation.The README provides clear examples for all supported resource types, explains both interpolation mechanisms (
${VAR}vsenv://), and documents CLI options thoroughly. The create-only semantics are well-explained.app/server/cli/commands/import-config.ts (5)
8-14: LGTM! Clean stdin reading implementation.The async iterator pattern for reading stdin chunks is idiomatic and handles the buffer concatenation correctly.
16-37: LGTM! Well-designed output abstraction.The
createOutputhelper cleanly separates JSON and human-readable output modes, with theerrormethod correctly exiting with code 1.
64-94: LGTM! Clear command definition with appropriate options.The command properly defines mutually exclusive input sources and provides helpful usage guidance when neither is specified.
106-144: LGTM! Dry-run mode provides useful validation feedback.The dry-run implementation correctly validates the config schema and reports item counts without making any changes.
146-175: LGTM! Import execution with proper error handling.The import flow correctly runs DB migrations before applying the config, handles validation errors distinctly from runtime errors, and exits with appropriate codes.
app/server/modules/lifecycle/config-import.ts (10)
1-29: LGTM! Well-organized imports and type definitions.The imports are logically grouped and the module correctly uses ArkType schemas for validation as per coding guidelines.
72-92: LGTM! Clean schema validation with helpful error paths.The
parseImportConfigfunction properly handles wrapped config format, applies env interpolation before validation, and provides detailed error paths for debugging.
122-175: LGTM! Robust recovery key handling with safety checks.The function correctly:
- Skips if key already matches
- Requires explicit
--overwrite-recovery-keyflag for replacement- Verifies database is empty before allowing overwrite (preventing data loss)
- Sets proper file permissions (0o600)
177-211: LGTM! Volume import with proper duplicate detection.The function correctly slugifies names before checking for duplicates, passes optional
shortIdfor IaC reproducibility, and handlesautoRemountas a post-creation update.
249-266: Improved: Now logs repo existence check failures for debugging.The debug logging at line 254 addresses the earlier review concern about silently swallowing errors during repository existence checks.
343-372: LGTM! Notification assignment builder handles both string and object formats.The function correctly processes both simple destination names and detailed notification objects with per-event settings.
429-539: LGTM! Schedule import with proper dependency resolution.The function correctly:
- Validates volume and repository existence before creating schedules
- Generates default schedule names from volume+repository if not provided
- Skips existing schedules but still processes notification/mirror attachments
- Mounts unmounted volumes before backup schedule creation
607-676: LGTM! User import with proper single-user handling.The function correctly:
- Skips if users already exist
- Warns when multiple users are provided (Zerobyte limitation)
- Supports both plaintext passwords and pre-hashed passwords
- Sets
hasDownloadedResticPasswordbased on recovery key presence
682-699: LGTM! Import orchestration with proper sequencing.The
runImportfunction correctly stops early if recovery key has errors (preventing orphaned resources) and follows the documented import order.
726-748: LGTM! Clean public API for config import.The
applyConfigImportandvalidateConfigexports provide a well-designed interface for the CLI and potential future integrations.app/schemas/config-import.ts (5)
1-5: LGTM! Proper schema imports following coding guidelines.Using ArkType and importing existing validated schemas (volumeConfigSchema, repositoryConfigSchema, etc.) ensures consistency with the rest of the codebase.
12-13: LGTM! ShortId schema matches expected format.The regex
/^[A-Za-z0-9_-]{8}$/correctly validates 8-character base64url strings used for reproducible IaC identifiers.
16-36: LGTM! Resource import schemas are well-defined.Volume, repository, and notification schemas properly:
- Require non-empty names (
string>=1)- Make shortId optional for IaC flexibility
- Strip undeclared keys for forward compatibility
59-74: LGTM! Backup schedule schema with comprehensive options.The schema supports all schedule configuration options including retention policy, patterns, notifications, and mirrors.
94-114: LGTM! Root config schema and type exports.The
importConfigSchemaproperly aggregates all resource arrays as optional (supporting partial imports) and exports all necessary types for consumers.
|
@nicotsx |
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @app/server/cli/index.ts:
- Line 2: Update the import statement that brings in importConfigCommand in
index.ts to use a Node-compatible module specifier by adding the .js extension
to the imported module path; locate the line importing importConfigCommand and
change its module specifier to include the .js extension so the import conforms
to "type: module" ESM rules.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/server/modules/repositories/repositories.service.ts (1)
31-68: CRITICAL: Missing encryption guards cause double-encryption.The
encryptConfigfunction encrypts sensitive fields unconditionally without checking if they're already encrypted. When importing configurations (e.g., from exports or migrations), fields may already be encrypted, leading to double-encryption that corrupts the values and renders repositories unusable.As noted in the PR objectives, the maintainer flagged this as a critical issue and recommended adding guards using
cryptoUtils.isEncrypted()for each sensitive field before encryption.🔎 Proposed fix adding encryption guards
const encryptConfig = async (config: RepositoryConfig): Promise<RepositoryConfig> => { const encryptedConfig: Record<string, string | boolean | number> = { ...config }; - if (config.customPassword) { + if (config.customPassword && !cryptoUtils.isEncrypted(config.customPassword)) { encryptedConfig.customPassword = await cryptoUtils.sealSecret(config.customPassword); } - if (config.cacert) { + if (config.cacert && !cryptoUtils.isEncrypted(config.cacert)) { encryptedConfig.cacert = await cryptoUtils.sealSecret(config.cacert); } switch (config.backend) { case "s3": case "r2": - encryptedConfig.accessKeyId = await cryptoUtils.sealSecret(config.accessKeyId); - encryptedConfig.secretAccessKey = await cryptoUtils.sealSecret(config.secretAccessKey); + if (!cryptoUtils.isEncrypted(config.accessKeyId)) { + encryptedConfig.accessKeyId = await cryptoUtils.sealSecret(config.accessKeyId); + } + if (!cryptoUtils.isEncrypted(config.secretAccessKey)) { + encryptedConfig.secretAccessKey = await cryptoUtils.sealSecret(config.secretAccessKey); + } break; case "gcs": - encryptedConfig.credentialsJson = await cryptoUtils.sealSecret(config.credentialsJson); + if (!cryptoUtils.isEncrypted(config.credentialsJson)) { + encryptedConfig.credentialsJson = await cryptoUtils.sealSecret(config.credentialsJson); + } break; case "azure": - encryptedConfig.accountKey = await cryptoUtils.sealSecret(config.accountKey); + if (!cryptoUtils.isEncrypted(config.accountKey)) { + encryptedConfig.accountKey = await cryptoUtils.sealSecret(config.accountKey); + } break; case "rest": - if (config.username) { + if (config.username && !cryptoUtils.isEncrypted(config.username)) { encryptedConfig.username = await cryptoUtils.sealSecret(config.username); } - if (config.password) { + if (config.password && !cryptoUtils.isEncrypted(config.password)) { encryptedConfig.password = await cryptoUtils.sealSecret(config.password); } break; case "sftp": - encryptedConfig.privateKey = await cryptoUtils.sealSecret(config.privateKey); + if (!cryptoUtils.isEncrypted(config.privateKey)) { + encryptedConfig.privateKey = await cryptoUtils.sealSecret(config.privateKey); + } break; } return encryptedConfig as RepositoryConfig; };Based on PR objectives, this was flagged as critical by the maintainer.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.mdapp/server/cli/index.tsapp/server/modules/backups/backups.service.tsapp/server/modules/repositories/repositories.service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
🧰 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/cli/index.tsapp/server/modules/backups/backups.service.tsapp/server/modules/repositories/repositories.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/cli/index.tsapp/server/modules/backups/backups.service.tsapp/server/modules/repositories/repositories.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/backups/backups.service.tsapp/server/modules/repositories/repositories.service.ts
app/server/modules/repositories/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Local repositories are stored in
/var/lib/zerobyte/repositories/<repo-name>
Files:
app/server/modules/repositories/repositories.service.ts
🧠 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.
📚 Learning: 2025-12-03T19:48:55.119Z
Learnt from: nicotsx
Repo: nicotsx/zerobyte PR: 95
File: app/server/modules/backups/backups.service.ts:525-533
Timestamp: 2025-12-03T19:48:55.119Z
Learning: In app/server/modules/backups/backups.service.ts, the mirror copy operations use shared locks (acquireShared) for both source and mirror repositories because restic supports parallel write operations and concurrent backups with non-exclusive locks. Only maintenance operations like prune and rebuild-index require exclusive locks in restic.
Applied to files:
app/server/modules/backups/backups.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 : Update `buildRepoUrl()` in `app/server/utils/restic.ts` when adding a new repository backend
Applied to files:
app/server/modules/backups/backups.service.tsapp/server/modules/repositories/repositories.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 : 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/backups/backups.service.tsapp/server/modules/repositories/repositories.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/repositories/repositories.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/**/*.ts : Local repositories are stored in `/var/lib/zerobyte/repositories/<repo-name>`
Applied to files:
app/server/modules/repositories/repositories.service.ts
📚 Learning: 2025-12-22T19:50:04.233Z
Learnt from: nicotsx
Repo: nicotsx/zerobyte PR: 197
File: app/client/modules/repositories/components/create-repository-form.tsx:112-112
Timestamp: 2025-12-22T19:50:04.233Z
Learning: When importing existing local repositories in the repositories service, users provide the full path (path + name) to an existing restic repository. Restic itself validates whether a valid repository exists at that location, so server-side path sanitization is not required for the import flow—the operation will fail if the path doesn't point to a valid restic repo.
Applied to files:
app/server/modules/repositories/repositories.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/repositories/repositories.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 is stored in `/var/lib/zerobyte/restic/password` and auto-generated on first run
Applied to files:
app/server/modules/repositories/repositories.service.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/repositories/repositories.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 **/*.{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/repositories/repositories.service.ts
🧬 Code graph analysis (2)
app/server/cli/index.ts (1)
app/server/cli/commands/import-config.ts (1)
importConfigCommand(64-176)
app/server/modules/repositories/repositories.service.ts (4)
app/schemas/restic.ts (2)
RepositoryConfig(97-97)CompressionMode(105-105)app/server/utils/id.ts (2)
isValidShortId(10-13)generateShortId(5-8)app/server/db/schema.ts (1)
repositoriesTable(51-63)app/server/utils/restic.ts (1)
restic(915-930)
🔇 Additional comments (7)
app/server/modules/backups/backups.service.ts (2)
18-18: LGTM: Import addition is clean.The addition of
isValidShortIdto the imports is appropriate for the new validation logic.
86-86: LGTM: ShortId handling is well-implemented.The validation and collision detection logic for
providedShortIdis correct and provides clear error messages. A unique constraint exists on theshortIdcolumn (shortId: text("short_id").notNull().unique()in the schema), which prevents race conditions between the collision check and insert at the database level. The implementation follows a consistent pattern with other modules.app/server/modules/repositories/repositories.service.ts (4)
78-95: LGTM! ShortId validation logic is well-structured.The shortId handling correctly:
- Validates format using
isValidShortIdbefore use- Checks uniqueness against the database
- Uses appropriate error types (
BadRequestErrorfor invalid format,ConflictErrorfor duplicates)- Provides clear, actionable error messages
- Falls back to generating a new shortId when none is provided
This supports the config import use case while maintaining data integrity.
104-120: LGTM! Repository existence validation is robust.The pre-insertion validation correctly distinguishes between creating new repositories and importing existing ones:
- Uses
restic.snapshots()to verify both existence and accessibility- Throws
ConflictErrorwhen attempting to create over an existing repo (with guidance to setisExistingRepository: true)- Throws
BadRequestErrorwhen attempting to import a non-existent repo- Provides clear, actionable error messages to guide configuration fixes
This prevents data loss from accidental overwrites and catches misconfiguration early.
141-144: LGTM! Initialization condition is clearer.The change from checking
!config.isExistingRepositoryto!repoExistsis more explicit and correct. Since the validation at lines 109-120 ensures consistency between the flag and the configuration, using therepoExistsvariable directly makes the initialization logic clearer and easier to understand.
98-100: Local repository directory naming is correct.For new local repositories, using
shortIdas the repository name ensures unique directory paths at/var/lib/zerobyte/repositories/<shortId>. The check for!config.isExistingRepositorycorrectly preserves the original name when importing existing repositories.Based on learnings, local repositories are stored in
/var/lib/zerobyte/repositories/<repo-name>.app/server/cli/index.ts (1)
8-8: LGTM!The command registration follows the established pattern and correctly integrates the new import-config command into the CLI.
| @@ -1,9 +1,11 @@ | |||
| import { Command } from "commander"; | |||
| import { importConfigCommand } from "./commands/import-config"; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Checking package.json for type: module ==="
if [ -f package.json ]; then
cat package.json | jq '.type // "not set"'
else
echo "package.json not found"
fi
echo -e "\n=== Checking tsconfig.json module resolution ==="
if [ -f tsconfig.json ]; then
cat tsconfig.json | jq '.compilerOptions.module, .compilerOptions.moduleResolution'
else
echo "tsconfig.json not found"
fi
echo -e "\n=== Checking import patterns in TypeScript files (sample) ==="
rg -n 'from\s+["\x27]\./[^"\']*["\x27]' --type ts --max-count=20
echo -e "\n=== Checking app/server/cli/index.ts specifically ==="
cat -n app/server/cli/index.ts | head -15Repository: nicotsx/zerobyte
Length of output: 334
🏁 Script executed:
#!/bin/bash
echo "=== Checking app/server/cli/index.ts ==="
cat -n app/server/cli/index.ts
echo -e "\n=== Checking import patterns (first 30 lines of TS files with imports) ==="
rg -n 'from.*["\x27]\.' --type ts --max-count=15
echo -e "\n=== Checking if any imports use file extensions ==="
rg -n 'from.*\.(ts|js)["\x27]' --type ts --max-count=15Repository: nicotsx/zerobyte
Length of output: 50372
Add file extension to import statement.
The import on line 2 must include a file extension per the coding guideline: "All imports must include file extensions when targeting Node/Bun, as the project uses "type": "module"". Change to import { importConfigCommand } from "./commands/import-config.js";
Note: While the rest of the codebase omits extensions, this guideline is explicit and one file (openapi-ts.config.ts) demonstrates compliance by including .js.
🤖 Prompt for AI Agents
In @app/server/cli/index.ts around line 2, Update the import statement that
brings in importConfigCommand in index.ts to use a Node-compatible module
specifier by adding the .js extension to the imported module path; locate the
line importing importConfigCommand and change its module specifier to include
the .js extension so the import conforms to "type: module" ESM rules.
|
I would love this feature as i try to do everything as IaC especially the backup and restore solution should be a one click thing. Can i support here in any way? I am not that experienced with javascript frameworks. |
|
@Robsmon I'm focusing on adding oidc currently, as it changes the data model by a lot, I don't want to merge yet the import/export PRs as it would render the previous exports useless with the data missing info |
4 participants
feat: Add JSON configuration import for Infrastructure as Code support
Summary
Adds the ability to configure Zerobyte entirely via a JSON configuration file, enabling Infrastructure as Code (IaC) workflows, Docker Compose deployments, and migration between instances.
Features
Configuration File Support
${VAR_NAME}syntax for secure secret injectionCLI Command
Options:
--config <path>--stdin--dry-run--json--log-level <level>--overwrite-recovery-keySupported Resources
Import Order & Dependencies
Repository Import Safeguards
isExistingRepository: trueto import pre-existing restic repositoriesMigration Support
passwordHashfield) for seamless admin migrationChanges
New Files
Modified Files
retentionPolicySchemafor reuseimport-configCLI commandExample Configuration
{ "recoveryKey": "${RECOVERY_KEY}", "volumes": [ { "name": "my-data", "config": { "backend": "directory", "path": "/data" } } ], "repositories": [ { "name": "s3-backup", "config": { "backend": "s3", "bucket": "backups", "accessKeyId": "${AWS_KEY}", "secretAccessKey": "${AWS_SECRET}" } } ], "backupSchedules": [ { "volume": "my-data", "repository": "s3-backup", "cronExpression": "0 2 * * *", "retentionPolicy": { "keepLast": 7, "keepDaily": 7 }, "notifications": ["slack-alerts"], "mirrors": [{ "repository": "local-mirror" }] } ], "notificationDestinations": [ { "name": "slack-alerts", "config": { "type": "slack", "webhookUrl": "${SLACK_WEBHOOK}" } } ], "users": [ { "username": "admin", "password": "${ADMIN_PASSWORD}" } ] }CLI Usage
Future Possible Enhancements
Related Issues
#13
Related PRs
Import compatible with export files from #92
Summary by CodeRabbit
New Features
Documentation
Validation & Errors
✏️ Tip: You can customize this high-level summary in your review settings.