Add support for setting a cacert or skipping certificate validation for self-signed certificates #182
Conversation
|
|
|
Warning Rate limit exceeded@nicotsx has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 14 minutes and 10 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
WalkthroughAdds TLS certificate config for REST repositories: schema fields 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 |
tvories
force-pushed
the
feat/rest-tls-support
branch
from
9284768 to
b7ba8fd
Compare
![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
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/volumes/volume.service.ts (1)
203-213: Use validated config backend instead of original input.Line 208 sets
type: volumeData.config?.backend, but should usenewConfig.backendfrom the validated config. This is inconsistent withnotifications.service.ts(line 198), which correctly usesnewConfig.type. Using the validated config ensures the persisted type matches the validated and encrypted config.🔎 Proposed fix
const [updated] = await db .update(volumesTable) .set({ name: newName, config: encryptedConfig, - type: volumeData.config?.backend, + type: newConfig.backend, autoRemount: volumeData.autoRemount, updatedAt: Date.now(), }) .where(eq(volumesTable.id, existing.id)) .returning();
🧹 Nitpick comments (4)
app/server/modules/backends/nfs/nfs-backend.ts (1)
116-120: Conditional logging reduces noise for expected scenarios.Suppressing logs for "Volume is not mounted" is sensible since this is an expected state, not an error condition worth alerting on.
Consider extracting the error message to a constant to avoid string coupling between lines 96, 102, and 117:
const NOT_MOUNTED_ERROR = "Volume is not mounted";This is a minor maintainability suggestion—the current implementation works correctly.
app/server/modules/backends/webdav/webdav-backend.ts (1)
141-143: Consider accepting additional FUSE-based WebDAV filesystem types.The check accepts
"fuse"and"davfs", but some WebDAV implementations report as"fuse.davfs2". The rclone backend usesincludes()for flexibility.🔎 Proposed fix
- if (mount.fstype !== "fuse" && mount.fstype !== "davfs") { + if (!mount.fstype.includes("fuse") && !mount.fstype.includes("davfs")) { throw new Error(`Path ${path} is not mounted as WebDAV (found ${mount.fstype}).`); }app/server/jobs/auto-remount.ts (1)
8-27: Job correctly isolates errors per-volume, preventing cascading failures.The implementation queries errored volumes, checks the
autoRemountflag, and uses try-catch error handling per volume—following the Job class pattern from coding guidelines. Error isolation ensures one volume failure doesn't block others.For volumes that fail repeatedly across multiple cron cycles, consider implementing a mechanism to track failure attempts or introduce exponential backoff delays when auto-remounting the same volume. This prevents excessive mount attempts on misconfigured or unavailable volumes that will continue to fail regularly.
app/client/modules/repositories/components/repository-forms/rest-repository-form.tsx (1)
79-98: Consider using actual newlines in the placeholder.The HTML entity
in the placeholder may not render as a newline in all browsers. Consider using a template literal with actual newlines or simplifying the placeholder.🔎 Suggested fix
<Textarea - placeholder="-----BEGIN CERTIFICATE----- ... -----END CERTIFICATE-----" + placeholder={"-----BEGIN CERTIFICATE-----\n...\n-----END CERTIFICATE-----"} rows={6} {...field} />
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
app/client/modules/notifications/components/create-notification-form.tsx(1 hunks)app/client/modules/repositories/components/create-repository-form.tsx(2 hunks)app/client/modules/repositories/components/repository-forms/rest-repository-form.tsx(2 hunks)app/client/modules/repositories/tabs/info.tsx(1 hunks)app/client/modules/volumes/components/create-volume-form.tsx(1 hunks)app/schemas/notifications.ts(2 hunks)app/schemas/restic.ts(3 hunks)app/schemas/volumes.ts(1 hunks)app/server/core/constants.ts(1 hunks)app/server/jobs/auto-remount.ts(1 hunks)app/server/jobs/repository-healthchecks.ts(0 hunks)app/server/modules/backends/nfs/nfs-backend.ts(4 hunks)app/server/modules/backends/rclone/rclone-backend.ts(4 hunks)app/server/modules/backends/smb/smb-backend.ts(4 hunks)app/server/modules/backends/webdav/webdav-backend.ts(5 hunks)app/server/modules/lifecycle/startup.ts(3 hunks)app/server/modules/notifications/notifications.service.ts(2 hunks)app/server/modules/repositories/repositories.service.ts(4 hunks)app/server/modules/volumes/volume.service.ts(2 hunks)app/server/utils/restic.ts(5 hunks)
💤 Files with no reviewable changes (1)
- app/server/jobs/repository-healthchecks.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/client/modules/repositories/tabs/info.tsx
- app/server/utils/restic.ts
🧰 Additional context used
📓 Path-based instructions (8)
app/client/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
app/client/**/*.{ts,tsx}: Use React Router v7 with React 19 for SSR frontend implementation
Implement Server-Sent Events for real-time updates using theuse-server-events.tshook
Use TanStack Query for server state management in the client
Files:
app/client/modules/notifications/components/create-notification-form.tsxapp/client/modules/volumes/components/create-volume-form.tsxapp/client/modules/repositories/components/repository-forms/rest-repository-form.tsxapp/client/modules/repositories/components/create-repository-form.tsx
app/client/**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Tailwind CSS v4 with Radix UI components for styling
Files:
app/client/modules/notifications/components/create-notification-form.tsxapp/client/modules/volumes/components/create-volume-form.tsxapp/client/modules/repositories/components/repository-forms/rest-repository-form.tsxapp/client/modules/repositories/components/create-repository-form.tsx
**/*.{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/client/modules/notifications/components/create-notification-form.tsxapp/schemas/volumes.tsapp/client/modules/volumes/components/create-volume-form.tsxapp/server/core/constants.tsapp/server/jobs/auto-remount.tsapp/server/modules/backends/nfs/nfs-backend.tsapp/server/modules/notifications/notifications.service.tsapp/client/modules/repositories/components/repository-forms/rest-repository-form.tsxapp/server/modules/backends/smb/smb-backend.tsapp/server/modules/volumes/volume.service.tsapp/schemas/notifications.tsapp/server/modules/backends/webdav/webdav-backend.tsapp/server/modules/repositories/repositories.service.tsapp/schemas/restic.tsapp/client/modules/repositories/components/create-repository-form.tsxapp/server/modules/backends/rclone/rclone-backend.tsapp/server/modules/lifecycle/startup.ts
app/schemas/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use ArkType for runtime schema validation instead of Zod
Files:
app/schemas/volumes.tsapp/schemas/notifications.tsapp/schemas/restic.ts
app/schemas/volumes.ts
📄 CodeRabbit inference engine (AGENTS.md)
Add new volume backend schemas to
app/schemas/volumes.tsand update thevolumeConfigSchemadiscriminated union
Files:
app/schemas/volumes.ts
app/server/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use Hono as the web framework with Bun runtime for server implementation
Files:
app/server/core/constants.tsapp/server/jobs/auto-remount.tsapp/server/modules/backends/nfs/nfs-backend.tsapp/server/modules/notifications/notifications.service.tsapp/server/modules/backends/smb/smb-backend.tsapp/server/modules/volumes/volume.service.tsapp/server/modules/backends/webdav/webdav-backend.tsapp/server/modules/repositories/repositories.service.tsapp/server/modules/backends/rclone/rclone-backend.tsapp/server/modules/lifecycle/startup.ts
app/server/jobs/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Create new scheduled jobs by extending the
Jobclass inapp/server/jobs/<job-name>.tsand registering them inapp/server/modules/lifecycle/startup.tswith a cron expression
Files:
app/server/jobs/auto-remount.ts
app/server/modules/backends/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Implement volume backends in
app/server/modules/backends/<backend>/withmount()andunmount()methods
Files:
app/server/modules/backends/nfs/nfs-backend.tsapp/server/modules/backends/smb/smb-backend.tsapp/server/modules/backends/webdav/webdav-backend.tsapp/server/modules/backends/rclone/rclone-backend.ts
🧠 Learnings (7)
📓 Common learnings
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`
📚 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/schemas/volumes.ts : Add new volume backend schemas to `app/schemas/volumes.ts` and update the `volumeConfigSchema` discriminated union
Applied to files:
app/schemas/volumes.tsapp/client/modules/volumes/components/create-volume-form.tsxapp/server/jobs/auto-remount.tsapp/server/modules/volumes/volume.service.tsapp/schemas/notifications.tsapp/schemas/restic.tsapp/client/modules/repositories/components/create-repository-form.tsxapp/server/modules/lifecycle/startup.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/modules/backends/**/*.ts : Implement volume backends in `app/server/modules/backends/<backend>/` with `mount()` and `unmount()` methods
Applied to files:
app/schemas/volumes.tsapp/server/jobs/auto-remount.tsapp/server/modules/backends/nfs/nfs-backend.tsapp/server/modules/backends/smb/smb-backend.tsapp/server/modules/volumes/volume.service.tsapp/server/modules/backends/webdav/webdav-backend.tsapp/server/modules/backends/rclone/rclone-backend.tsapp/server/modules/lifecycle/startup.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: For repository backends, add backend type to `app/schemas/restic.ts` and update `buildRepoUrl()` in `app/server/utils/restic.ts`
Applied to files:
app/schemas/volumes.tsapp/server/modules/repositories/repositories.service.tsapp/schemas/restic.tsapp/client/modules/repositories/components/create-repository-form.tsx
📚 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/schemas/**/*.ts : Use ArkType for runtime schema validation instead of Zod
Applied to files:
app/client/modules/volumes/components/create-volume-form.tsx
📚 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/core/constants.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/jobs/**/*.ts : Create new scheduled jobs by extending the `Job` class in `app/server/jobs/<job-name>.ts` and registering them in `app/server/modules/lifecycle/startup.ts` with a cron expression
Applied to files:
app/server/jobs/auto-remount.ts
🧬 Code graph analysis (10)
app/client/modules/notifications/components/create-notification-form.tsx (3)
app/client/modules/repositories/components/create-repository-form.tsx (1)
formSchema(36-39)app/client/modules/volumes/components/create-volume-form.tsx (1)
formSchema(27-29)app/schemas/notifications.ts (1)
notificationConfigSchemaBase(79-86)
app/server/jobs/auto-remount.ts (4)
app/server/utils/logger.ts (1)
logger(36-41)app/server/db/db.ts (1)
db(14-14)app/server/db/schema.ts (1)
volumesTable(10-22)app/server/modules/volumes/volume.service.ts (1)
volumeService(397-409)
app/server/modules/backends/nfs/nfs-backend.ts (4)
app/server/utils/logger.ts (1)
logger(36-41)app/schemas/volumes.ts (1)
BACKEND_STATUS(68-72)app/server/modules/backends/utils/backend-utils.ts (1)
executeUnmount(22-35)app/server/utils/errors.ts (1)
toMessage(16-19)
app/server/modules/notifications/notifications.service.ts (1)
app/schemas/notifications.ts (1)
notificationConfigSchema(88-88)
app/server/modules/backends/smb/smb-backend.ts (4)
app/server/utils/logger.ts (1)
logger(36-41)app/schemas/volumes.ts (1)
BACKEND_STATUS(68-72)app/server/modules/backends/utils/backend-utils.ts (1)
executeUnmount(22-35)app/server/utils/errors.ts (1)
toMessage(16-19)
app/server/modules/volumes/volume.service.ts (1)
app/schemas/volumes.ts (1)
volumeConfigSchema(64-64)
app/server/modules/backends/webdav/webdav-backend.ts (4)
app/server/utils/logger.ts (1)
logger(36-41)app/schemas/volumes.ts (1)
BACKEND_STATUS(68-72)app/server/modules/backends/utils/backend-utils.ts (1)
executeUnmount(22-35)app/server/utils/errors.ts (1)
toMessage(16-19)
app/server/modules/repositories/repositories.service.ts (3)
app/server/core/config.ts (1)
config(23-23)app/server/utils/crypto.ts (1)
cryptoUtils(186-189)app/schemas/restic.ts (1)
repositoryConfigSchema(93-93)
app/client/modules/repositories/components/create-repository-form.tsx (1)
app/schemas/restic.ts (1)
repositoryConfigSchemaBase(84-91)
app/server/modules/lifecycle/startup.ts (7)
app/server/db/db.ts (1)
db(14-14)app/server/modules/volumes/volume.service.ts (1)
volumeService(397-409)app/server/utils/logger.ts (1)
logger(36-41)app/server/modules/repositories/repositories.service.ts (1)
repositoriesService(476-489)app/server/modules/notifications/notifications.service.ts (1)
notificationsService(439-449)app/server/core/scheduler.ts (1)
Scheduler(52-52)app/server/jobs/auto-remount.ts (1)
VolumeAutoRemountJob(8-28)
🔇 Additional comments (31)
app/server/modules/backends/smb/smb-backend.ts (4)
31-34: LGTM!Good defensive improvement: only attempting unmount when health check returns an error state avoids unnecessary operations when the volume is simply unmounted. This aligns with the patterns in other backends.
85-93: LGTM!The refactor improves robustness:
- Using
getMountForPath()correctly validates mount points instead of relying on path existence.- The guarded
.catch(() => {})onrmdirprevents failures when the directory is already removed or inaccessible.
109-123: LGTM!The two-step health check validation is well-structured:
- Verifies path accessibility via
fs.access- Validates the mount point exists and matches the expected path
- Confirms the filesystem type is
cifsThis provides clear error differentiation between inaccessible paths and incorrect mount types.
132-136: LGTM!Suppressing logs for the expected "Volume is not mounted" case reduces noise while still logging genuine errors. Good use of
toMessage()for consistent error handling.app/server/modules/backends/nfs/nfs-backend.ts (4)
30-33: LGTM! Conditional unmount improves efficiency.The change to only unmount on error status (rather than unconditionally) avoids unnecessary cleanup operations when the volume is simply unmounted. The logic correctly handles all three states from
BACKEND_STATUS.
69-77: Good improvement using mount point validation.Using
getMountForPathis more reliable thanfs.accessfor determining if a path is actually a mount point. ThemountPoint !== pathcheck correctly prevents accidentally unmounting a parent mount.The silent
.catch(() => {})onrmdiris appropriate for cleanup, as failure to remove an empty directory shouldn't fail the unmount operation.
93-107: Robust health check validation.The layered validation approach is well-structured:
- Path accessibility check via
fs.access- Mount point equality validation ensures we're checking the exact mount, not a parent
- Filesystem type validation with
startsWith("nfs")correctly handles NFS variants (nfs3, nfs4, etc.)The detailed error message on line 106 including the actual
fstypeaids debugging.
124-128: LGTM!The public API via
makeNfsBackendremains unchanged, maintaining backward compatibility. The implementation correctly follows the backend pattern withmount(),unmount(), andcheckHealth()methods as per coding guidelines.app/server/core/constants.ts (1)
7-8: LGTM!Good addition of default exclusions for restic operations. Excluding the database, password file, and repository base directory prevents recursive backup issues and potential sensitive data exposure.
app/server/modules/backends/rclone/rclone-backend.ts (4)
31-34: LGTM!Good defensive pattern: only unmount before mount when health check reports an error. This prevents unnecessary unmount/remount cycles when the volume is already in a healthy or unmounted state.
80-88: LGTM!Proper mount-point validation before unmounting prevents errors when the path doesn't exist or isn't actually mounted. Swallowing
rmdirfailures is appropriate here since the directory may be non-empty or have permission restrictions.
104-118: LGTM!Health check improvements provide clearer error messaging:
- Distinguishes between inaccessible paths and non-rclone mounts
- Uses
includes("rclone")to handle FUSE variants of rclone mounts
127-131: LGTM!Suppressing logs for "Volume is not mounted" reduces noise in scheduled health checks while preserving visibility for actual errors.
app/server/modules/backends/webdav/webdav-backend.ts (3)
31-34: LGTM!Consistent with the rclone backend pattern: only unmount before mounting when health check reports an error state.
105-113: LGTM!Mount-point validation and silent
rmdirfailure handling are consistent with other backends (rclone, SMB, NFS).
152-156: LGTM!Consistent error logging suppression for "Volume is not mounted" with other backends.
app/client/modules/volumes/components/create-volume-form.tsx (1)
21-29: LGTM!Switching to
volumeConfigSchemaBasefor form validation is appropriate. The base schema provides lenient validation during editing, whiledeepCleanincleanSchemahandles sanitization. This aligns with the broader pattern in this PR where other forms also switched to*SchemaBasefor composition.app/client/modules/notifications/components/create-notification-form.tsx (1)
20-24: LGTM!The switch to
notificationConfigSchemaBaseis consistent with the pattern used in other form components (create-repository-form.tsx,create-volume-form.tsx), where base schemas are used for form composition while the full schema withonUndeclaredKey("delete")is applied server-side.app/client/modules/repositories/components/repository-forms/rest-repository-form.tsx (1)
99-115: LGTM!The Skip TLS checkbox implementation is well-designed with appropriate security warnings. The
field.value ?? falsepattern safely handles undefined values, and the UI layout with the bordered container makes the security implications visually prominent.app/schemas/volumes.ts (1)
58-64: LGTM!The introduction of
volumeConfigSchemaBaseand the restructuring ofvolumeConfigSchemato derive from it follows the same pattern established inapp/schemas/restic.ts. This enables consistent form composition across volumes and repositories while maintaining server-side undeclared key stripping. Based on learnings, volume backend schemas are correctly placed in this file.app/client/modules/repositories/components/create-repository-form.tsx (1)
23-23: LGTM!The switch to
repositoryConfigSchemaBasealigns with the pattern used in other form components. TheRestRepositoryFormcomponent (line 268) will properly capture the newcacertandinsecureTlsfields, which will flow through this base schema composition. Based on learnings, repository backend types are correctly sourced fromapp/schemas/restic.ts.Also applies to: 39-39
app/server/modules/repositories/repositories.service.ts (2)
51-53: LGTM!Correctly encrypts the
cacertfield for REST backend configurations, consistent with the handling of other sensitive fields likeusernameandpassword.
436-464: No action needed. The encryption handling in the repository update flow is already correct. The config secrets are protected from double-encryption by theisEncrypted()check in thesealSecret()function, which detects the "encv1:" prefix and returns encrypted values unchanged.app/server/modules/lifecycle/startup.ts (1)
71-71: LGTM!Scheduling
VolumeAutoRemountJobevery 5 minutes is appropriate for automatic recovery of errored volumes withautoRemountenabled. This complements the initial mount attempt at startup (lines 60-64).app/server/modules/notifications/notifications.service.ts (2)
14-16: LGTM: Schema-based validation imports added correctly.The imports for
notificationConfigSchemaand ArkType'stypeutility align with the new validation pattern. This supports runtime validation of notification configurations.
191-198: LGTM: Robust validation and type-safe config handling.The validation flow correctly:
- Validates with
notificationConfigSchemausing the merged config- Checks for ArkType errors via
instanceof type.errors- Throws a clear error on validation failure
- Encrypts only the validated
newConfig- Derives
updateData.typefrom the validated configThis ensures type safety and consistency before persisting.
app/server/modules/volumes/volume.service.ts (2)
19-20: LGTM: Schema-based validation imports added correctly.The imports for
volumeConfigSchemaand ArkType'stypeutility align with the new validation pattern, consistent with the approach innotifications.service.ts.
196-201: LGTM: Validation logic is correct and consistent.The validation flow properly validates, checks for errors, and encrypts the validated config before persisting.
app/schemas/notifications.ts (1)
79-88: LGTM: Clean schema refactoring for composability.Introducing
notificationConfigSchemaBaseallows forms and other consumers to extend the base schema, whilenotificationConfigSchemawith.onUndeclaredKey("delete")provides strict validation that strips unknown properties. This pattern aligns with the broader schema architecture across volumes and repositories.app/schemas/restic.ts (2)
71-72: LGTM: TLS configuration fields added for REST repositories.The new
cacertandinsecureTlsoptional fields enable users to work with self-signed certificates, directly supporting the PR's objective. Making them optional maintains backward compatibility.
84-93: LGTM: Consistent schema refactoring pattern.The introduction of
repositoryConfigSchemaBaseand the derivedrepositoryConfigSchemafollows the same composable pattern used innotifications.tsandvolumes.ts, promoting consistency across the codebase.
| const ensureLatestConfigurationSchema = async () => { | ||
| const volumes = await db.query.volumesTable.findMany({}); | ||
|
|
||
| for (const volume of volumes) { | ||
| await volumeService.updateVolume(volume.name, volume).catch((err) => { | ||
| logger.error(`Failed to update volume ${volume.name}: ${err}`); | ||
| }); | ||
| } | ||
|
|
||
| const repositories = await db.query.repositoriesTable.findMany({}); | ||
|
|
||
| for (const repo of repositories) { | ||
| await repositoriesService.updateRepository(repo.name, {}).catch((err) => { | ||
| logger.error(`Failed to update repository ${repo.name}: ${err}`); | ||
| }); | ||
| } | ||
|
|
||
| const notifications = await db.query.notificationDestinationsTable.findMany({}); | ||
|
|
||
| for (const notification of notifications) { | ||
| await notificationsService.updateDestination(notification.id, notification).catch((err) => { | ||
| logger.error(`Failed to update notification destination ${notification.id}: ${err}`); | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# Find and examine the service files for encryption handling
find app/server/modules -name "*.service.ts" -type f | head -20Repository: nicotsx/zerobyte
Length of output: 348
🏁 Script executed:
# Examine the volume service updateVolume method
sed -n '1,100p' app/server/modules/volumes/volume.service.ts | grep -A 30 "updateVolume"
# Examine the repositories service updateRepository method
sed -n '1,150p' app/server/modules/repositories/repositories.service.ts | grep -A 30 "updateRepository"
# Examine the notifications service updateDestination method
sed -n '1,150p' app/server/modules/notifications/notifications.service.ts | grep -A 30 "updateDestination"Repository: nicotsx/zerobyte
Length of output: 42
🏁 Script executed:
# First, let's get the file sizes and then read them appropriately
wc -l app/server/modules/volumes/volume.service.ts
wc -l app/server/modules/repositories/repositories.service.ts
wc -l app/server/modules/notifications/notifications.service.tsRepository: nicotsx/zerobyte
Length of output: 230
🏁 Script executed:
# Search for updateVolume method in volume service
rg -n "updateVolume" app/server/modules/volumes/volume.service.ts -A 25
# Search for updateRepository method in repositories service
rg -n "updateRepository" app/server/modules/repositories/repositories.service.ts -A 25
# Search for updateDestination method in notifications service
rg -n "updateDestination" app/server/modules/notifications/notifications.service.ts -A 25Repository: nicotsx/zerobyte
Length of output: 3257
🏁 Script executed:
# Get the full updateVolume implementation
sed -n '163,220p' app/server/modules/volumes/volume.service.ts
# Get the full updateRepository implementation
sed -n '418,480p' app/server/modules/repositories/repositories.service.ts
# Get the full updateDestination implementation
sed -n '160,230p' app/server/modules/notifications/notifications.service.tsRepository: nicotsx/zerobyte
Length of output: 5549
🏁 Script executed:
# Check what repositoryConfigSchema does - does it decrypt?
rg -n "repositoryConfigSchema" app/server/modules/repositories/repositories.service.ts -B 5 -A 10
# Check the schema definition
find app/server -name "*schema*" -o -name "*config*" | grep -i repositoryRepository: nicotsx/zerobyte
Length of output: 1194
🏁 Script executed:
# Find the repositoryConfigSchema definition
find app/server -name "*.ts" | xargs rg -l "repositoryConfigSchema" | head -5
# Check the restic schema file
cat app/server/schemas/restic.ts 2>/dev/null | head -100
# Also check encryptConfig implementation
rg -n "const encryptConfig|function encryptConfig" app/server -A 15Repository: nicotsx/zerobyte
Length of output: 1780
🏁 Script executed:
# Get the full encryptConfig implementation
sed -n '25,70p' app/server/modules/repositories/repositories.service.ts
# Check if repositoryConfigSchema does any decryption
rg -n "repositoryConfigSchema\s*=" app/server/schemas/restic.ts -A 20 | head -40Repository: nicotsx/zerobyte
Length of output: 1704
🏁 Script executed:
# Find restic schema
find app/server -name "restic*" -o -name "*restic*" | grep -E "\.(ts|tsx)$"
# Also check schemas directory
ls -la app/server/schemas/Repository: nicotsx/zerobyte
Length of output: 170
🏁 Script executed:
# Find the schemas directory
find app/server -type d -name "schemas" -o -type d -name "schema"
# Search for repositoryConfigSchema definition
rg -n "repositoryConfigSchema" app/server --type ts | head -20
# Search in all ts files for schema definitions
find app/server -name "*.ts" | xargs rg "export.*repositoryConfigSchema|const repositoryConfigSchema" | head -20Repository: nicotsx/zerobyte
Length of output: 790
🏁 Script executed:
# Look for where ~ is configured - check tsconfig
cat tsconfig.json 2>/dev/null | grep -A 5 "paths"
# Search for all files containing repositoryConfigSchema definition
rg "export const repositoryConfigSchema|export.*repositoryConfigSchema" app --type ts -A 5
# Alternative: search in all files
find . -name "*.ts" -type f | xargs grep -l "export.*repositoryConfigSchema" 2>/dev/null | head -10Repository: nicotsx/zerobyte
Length of output: 982
🏁 Script executed:
# Get the full schema definition for repositoryConfigSchema
cat app/schemas/restic.ts | head -100Repository: nicotsx/zerobyte
Length of output: 2574
🏁 Script executed:
# Check updateDestination for the same pattern
sed -n '160,230p' app/server/modules/notifications/notifications.service.ts | grep -A 30 "newConfig\|encryptSensitiveFields"
# Check the encryptSensitiveFields implementation
rg -n "const encryptSensitiveFields|export.*encryptSensitiveFields" app/server --type ts -A 20Repository: nicotsx/zerobyte
Length of output: 1230
🏁 Script executed:
# Search for encryptSensitiveFields definition more broadly
rg "encryptSensitiveFields" app/server --type ts -B 2 | head -50
# Look for where it's imported or defined
rg "encryptSensitiveFields" app --type ts | grep -E "(import|const|function|export)"Repository: nicotsx/zerobyte
Length of output: 2294
🏁 Script executed:
# Get the full encryptSensitiveFields implementation for volumes
rg -n "async function encryptSensitiveFields" app/server/modules/volumes/volume.service.ts -A 40
# Get the full encryptSensitiveFields implementation for notifications
rg -n "async function encryptSensitiveFields" app/server/modules/notifications/notifications.service.ts -A 40Repository: nicotsx/zerobyte
Length of output: 2386
🏁 Script executed:
# Now let me verify the startup.ts file to see exactly what's being called
sed -n '17,41p' app/server/modules/lifecycle/startup.ts
# Check if there's any decryption happening before the update calls
rg -n "decryptConfig\|decryptSensitiveFields" app/server/modules/repositories/repositories.service.ts | head -10Repository: nicotsx/zerobyte
Length of output: 942
Startup migration should conditionally apply schema updates based on configuration version to prevent redundant encryption.
The ensureLatestConfigurationSchema function correctly applies updates to all configurations, but it runs unconditionally on every startup. More critically, updateRepository(repo.name, {}) passes an empty updates object while the existing encrypted config gets re-encrypted by encryptConfig, causing double encryption of already-encrypted secrets. The same pattern occurs with updateDestination, where encrypted configurations are validated then re-encrypted without prior decryption.
Add a schema version marker to each configuration object. Check this version in ensureLatestConfigurationSchema and only call the update methods when the version differs from the current application schema version. This prevents unnecessary re-encryption cycles and ensures migrations run only when needed.
tvories
force-pushed
the
feat/rest-tls-support
branch
from
b7ba8fd to
b77bd2e
Compare
|
Hello @tvories thank you, these are nice additions you are suggesting. It seems you've made a mistake with a rebase? The diff is not the actual feature you're referring to |
tvories
force-pushed
the
feat/rest-tls-support
branch
2 times, most recently
from
9284768 to
15f66a9
Compare
yes, sorry, I was struggling mightily while trying to rebase my git username and email. It should be fixed now |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
app/client/modules/repositories/components/repository-forms/rest-repository-form.tsx (2)
79-115: Consider adding mutual exclusivity validation.The description at line 110 suggests that
cacertandinsecureTlsare alternative approaches: you either provide a CA certificate to trust it, OR skip verification entirely. However, the form currently allows both to be set simultaneously, which could lead to confusing behavior where the certificate is ignored because--insecure-tlsbypasses all verification.Consider adding validation to ensure only one approach is used at a time, or clearly document the precedence behavior.
Example validation approach
You could add a validation rule to the form schema or display a warning when both are set:
// In form validation or onChange handler if (insecureTls && cacert) { // Show warning or prevent submission }
93-93: Optional: Consider shortening the documentation URL.The FormDescription includes a full restic documentation URL that extends the line length. Consider using a shorter link text or moving it to a separate line for better readability.
app/schemas/restic.ts (1)
71-72: Consider enforcing mutual exclusivity between cacert and insecureTls.Both fields are optional, but their simultaneous use may not make semantic sense. When
insecureTlsis true, certificate validation is bypassed entirely, renderingcacertmeaningless. Consider adding validation to prevent this ambiguous configuration.Example constraint using ArkType
You can add a constraint to the schema:
export const restRepositoryConfigSchema = type({ backend: "'rest'", url: "string", username: "string?", password: "string?", path: "string?", cacert: "string?", insecureTls: "boolean?", }).and(baseRepositoryConfigSchema).narrow((config) => { // Prevent both cacert and insecureTls from being set if (config.cacert && config.insecureTls) { return "Cannot set both cacert and insecureTls - use cacert to trust a certificate or insecureTls to skip validation"; } return true; });Alternatively, document the precedence behavior if both are intentionally allowed.
app/server/utils/restic.ts (1)
152-160: Optimize: Skip cacert setup when insecureTls is enabled.When
config.insecureTlsis true, restic will bypass certificate validation entirely via the--insecure-tlsflag (added in lines 821-823). In this scenario, setting up the CA certificate file is unnecessary work. Consider skipping the cacert file creation when insecure TLS is enabled.Proposed optimization
case "rest": { if (config.username) { env.RESTIC_REST_USERNAME = await cryptoUtils.resolveSecret(config.username); } if (config.password) { env.RESTIC_REST_PASSWORD = await cryptoUtils.resolveSecret(config.password); } - if (config.cacert) { + if (config.cacert && !config.insecureTls) { const decryptedCert = await cryptoUtils.resolveSecret(config.cacert); const certPath = path.join("/tmp", `rest-server-cacert-${crypto.randomBytes(8).toString("hex")}.pem`); await fs.writeFile(certPath, decryptedCert, { mode: 0o600 }); env.RESTIC_CACERT = certPath; } if (config.insecureTls) { env._REST_INSECURE_TLS = "true"; } break; }This prevents unnecessary file I/O and cleanup when the certificate won't be used.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/client/modules/repositories/components/repository-forms/rest-repository-form.tsx(2 hunks)app/client/modules/repositories/tabs/info.tsx(1 hunks)app/schemas/restic.ts(1 hunks)app/server/modules/repositories/repositories.service.ts(1 hunks)app/server/utils/restic.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
app/client/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
app/client/**/*.{ts,tsx}: Use React Router v7 with React 19 for SSR frontend implementation
Implement Server-Sent Events for real-time updates using theuse-server-events.tshook
Use TanStack Query for server state management in the client
Files:
app/client/modules/repositories/components/repository-forms/rest-repository-form.tsxapp/client/modules/repositories/tabs/info.tsx
app/client/**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Tailwind CSS v4 with Radix UI components for styling
Files:
app/client/modules/repositories/components/repository-forms/rest-repository-form.tsxapp/client/modules/repositories/tabs/info.tsx
**/*.{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/client/modules/repositories/components/repository-forms/rest-repository-form.tsxapp/server/modules/repositories/repositories.service.tsapp/client/modules/repositories/tabs/info.tsxapp/schemas/restic.tsapp/server/utils/restic.ts
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.tsapp/server/utils/restic.ts
app/schemas/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use ArkType for runtime schema validation instead of Zod
Files:
app/schemas/restic.ts
app/server/utils/restic.ts
📄 CodeRabbit inference engine (AGENTS.md)
app/server/utils/restic.ts: Store Restic password file at/var/lib/zerobyte/restic/passwordwith 0600 permissions and never expose it
Restic password file requires 0600 permissions - never expose it in logs or error messages
Files:
app/server/utils/restic.ts
🧠 Learnings (5)
📓 Common learnings
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`
📚 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.tsapp/client/modules/repositories/tabs/info.tsxapp/schemas/restic.tsapp/server/utils/restic.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/schemas/volumes.ts : Add new volume backend schemas to `app/schemas/volumes.ts` and update the `volumeConfigSchema` discriminated union
Applied to files:
app/schemas/restic.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/utils/restic.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/utils/restic.ts
🧬 Code graph analysis (2)
app/server/modules/repositories/repositories.service.ts (2)
app/server/core/config.ts (1)
config(23-23)app/server/utils/crypto.ts (1)
cryptoUtils(186-189)
app/server/utils/restic.ts (2)
app/server/core/config.ts (1)
config(23-23)app/server/utils/crypto.ts (1)
cryptoUtils(186-189)
🔇 Additional comments (4)
app/server/modules/repositories/repositories.service.ts (1)
51-53: LGTM! Consistent secret encryption pattern.The cacert encryption follows the established pattern for other REST backend credentials and is properly guarded by a truthy check.
app/client/modules/repositories/tabs/info.tsx (1)
127-150: LGTM! Clear security status indicators.The REST-specific metadata display appropriately uses color coding to highlight security-relevant configuration: green for CA certificate configured and TLS validation enabled, red for disabled validation, and muted for unconfigured certificate. The conditional rendering is properly scoped to REST repositories.
app/server/utils/restic.ts (2)
809-811: LGTM! Proper cleanup for REST CA certificate.The cleanup logic correctly removes the temporary CA certificate file for REST backends and properly suppresses errors for missing files. This is consistent with the cleanup pattern used for other temporary credential files.
821-823: LGTM! Correct insecure TLS flag handling.The
--insecure-tlsflag is properly added to restic commands when the custom environment flag is set, following the same pattern as SFTP argument handling.
|
Thanks for the new features |
|
I added field validation and moved the fields closer to the rest server url: 
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/client/modules/repositories/components/repository-forms/rest-repository-form.tsx (1)
94-167: Consider using the library Tooltip component for consistency.The custom tooltip implementation with manual positioning and timing logic is complex and inconsistent with the library's Tooltip component used for the insecureTls field above (lines 52-75). This adds maintenance overhead and increases the chance of bugs (e.g., the memory leak from uncleaned timeouts).
The library's Tooltip component might support the same functionality with less code. If manual cursor tracking is truly required, consider extracting this pattern into a reusable component.
Additionally, line 100 forces the value to an empty string when
insecureTlsis true:value={insecureTls ? "" : field.value}This might be redundant since line 61 already clears
cacertwheninsecureTlsis enabled. The field is also already disabled viadisabled={insecureTls}, so the value override may be unnecessary.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/client/modules/repositories/components/create-repository-form.tsx(1 hunks)app/client/modules/repositories/components/repository-forms/rest-repository-form.tsx(3 hunks)app/schemas/restic.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/schemas/restic.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for code formatting and linting with tabs (not spaces), 120 character line width, and double quotes.
Files:
app/client/modules/repositories/components/create-repository-form.tsxapp/client/modules/repositories/components/repository-forms/rest-repository-form.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not auto-organize imports in Biome - imports organization is disabled.
Files:
app/client/modules/repositories/components/create-repository-form.tsxapp/client/modules/repositories/components/repository-forms/rest-repository-form.tsx
app/client/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
React client must use TanStack Query for server state management and auto-generated API client from OpenAPI spec.
Files:
app/client/modules/repositories/components/create-repository-form.tsxapp/client/modules/repositories/components/repository-forms/rest-repository-form.tsx
app/client/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use Server-Sent Events hook (
use-server-events.ts) for real-time updates in React components.
Files:
app/client/modules/repositories/components/create-repository-form.tsxapp/client/modules/repositories/components/repository-forms/rest-repository-form.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-19T18:25:52.583Z
Learning: When adding a new repository backend, update `buildRepoUrl()` in `app/server/utils/restic.ts` and `buildEnv()` to handle credentials/configuration.
🧬 Code graph analysis (1)
app/client/modules/repositories/components/repository-forms/rest-repository-form.tsx (2)
app/client/modules/repositories/components/create-repository-form.tsx (1)
RepositoryFormValues(42-42)app/client/modules/repositories/components/repository-forms/index.ts (1)
RestRepositoryForm(7-7)
🔇 Additional comments (3)
app/client/modules/repositories/components/create-repository-form.tsx (1)
75-75: LGTM! Good UX improvement.Setting validation mode to
"onTouched"improves the user experience by showing validation errors only after the user has interacted with a field, rather than on every keystroke or only on submit.app/client/modules/repositories/components/repository-forms/rest-repository-form.tsx (2)
46-86: LGTM! Good mutual exclusivity implementation.The insecureTls field correctly:
- Disables when a CA certificate is provided
- Clears the cacert field when enabled
- Shows a helpful tooltip explaining why it's disabled
- Includes an appropriate security warning
170-180: LGTM! Good security practices for external links.The external documentation link includes proper security attributes (
rel="noopener noreferrer") and provides helpful context for users configuring CA certificates.
| const tooltipTimeoutRef = useRef<NodeJS.Timeout | null>(null); | ||
| const tooltipHideTimeoutRef = useRef<NodeJS.Timeout | null>(null); |
There was a problem hiding this comment.
Clean up timeout refs on component unmount.
The timeout refs are not cleared when the component unmounts, which can lead to memory leaks or errors if timeouts fire after unmount.
🔎 Proposed fix to add cleanup effect
const tooltipTimeoutRef = useRef<NodeJS.Timeout | null>(null);
const tooltipHideTimeoutRef = useRef<NodeJS.Timeout | null>(null);
+
+ useEffect(() => {
+ return () => {
+ if (tooltipTimeoutRef.current) {
+ clearTimeout(tooltipTimeoutRef.current);
+ }
+ if (tooltipHideTimeoutRef.current) {
+ clearTimeout(tooltipHideTimeoutRef.current);
+ }
+ };
+ }, []);You'll also need to import useEffect:
-import { useState, useRef } from "react";
+import { useState, useRef, useEffect } from "react";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const tooltipTimeoutRef = useRef<NodeJS.Timeout | null>(null); | |
| const tooltipHideTimeoutRef = useRef<NodeJS.Timeout | null>(null); | |
| const tooltipTimeoutRef = useRef<NodeJS.Timeout | null>(null); | |
| const tooltipHideTimeoutRef = useRef<NodeJS.Timeout | null>(null); | |
| useEffect(() => { | |
| return () => { | |
| if (tooltipTimeoutRef.current) { | |
| clearTimeout(tooltipTimeoutRef.current); | |
| } | |
| if (tooltipHideTimeoutRef.current) { | |
| clearTimeout(tooltipHideTimeoutRef.current); | |
| } | |
| }; | |
| }, []); |
🤖 Prompt for AI Agents
In
app/client/modules/repositories/components/repository-forms/rest-repository-form.tsx
around lines 27 to 28, the tooltip timeout refs (tooltipTimeoutRef and
tooltipHideTimeoutRef) are not cleared on unmount; add an import for useEffect
and create a useEffect with an empty dependency array that on cleanup checks
each ref, calls clearTimeout(ref.current) if present, and then sets ref.current
to null to prevent timers firing after unmount and avoid memory leaks.
nicotsx
force-pushed
the
feat/rest-tls-support
branch
from
d5d354f to
b98646c
Compare
app/client/modules/repositories/components/create-repository-form.tsx
Outdated
Show resolved
Hide resolved
app/client/modules/repositories/components/repository-forms/rest-repository-form.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/client/modules/repositories/components/repository-forms/rest-repository-form.tsxapp/client/modules/repositories/tabs/info.tsxapp/schemas/restic.tsapp/server/modules/repositories/repositories.service.tsapp/server/utils/restic.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- app/server/modules/repositories/repositories.service.ts
- app/client/modules/repositories/components/repository-forms/rest-repository-form.tsx
- app/schemas/restic.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/client/modules/repositories/tabs/info.tsxapp/server/utils/restic.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use tabs (not spaces) for indentation with a line width of 120 characters
Use double quotes for strings
Do not auto-organize imports - imports organization is disabled in Biome
All imports must include file extensions when targeting Node/Bun, as the project uses"type": "module"
Files:
app/client/modules/repositories/tabs/info.tsxapp/server/utils/restic.ts
app/client/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
app/client/**/*.{ts,tsx}: Client uses TanStack Query for server state management
Client uses Radix UI primitives with custom Tailwind styling
Use Server-Sent Events hook (use-server-events.ts) for real-time updates in the client
Files:
app/client/modules/repositories/tabs/info.tsx
app/server/utils/restic.ts
📄 CodeRabbit inference engine (AGENTS.md)
app/server/utils/restic.ts: Restic password file must have 0600 permissions and should never be exposed
UpdatebuildRepoUrl()inapp/server/utils/restic.tswhen adding a new repository backend
UpdatebuildEnv()inapp/server/utils/restic.tsto handle credentials and configuration when adding a new repository backend
Restic password file is stored in/var/lib/zerobyte/restic/passwordand auto-generated on first run
Restic cache is stored in/var/lib/zerobyte/restic/cache
Files:
app/server/utils/restic.ts
🧠 Learnings (8)
📓 Common 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 : 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 `buildRepoUrl()` in `app/server/utils/restic.ts` when adding a new repository backend
Applied to files:
app/client/modules/repositories/tabs/info.tsxapp/server/utils/restic.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/client/modules/repositories/tabs/info.tsxapp/server/utils/restic.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/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/client/modules/repositories/tabs/info.tsx
📚 Learning: 2025-12-22T19:49:56.761Z
Learnt from: nicotsx
Repo: nicotsx/zerobyte PR: 197
File: app/client/modules/repositories/components/create-repository-form.tsx:112-112
Timestamp: 2025-12-22T19:49:56.761Z
Learning: During the repository import flow, when users provide a full path to an existing restic repository, do not implement server-side path sanitization for this flow. Rely on Restic to validate the path; the operation will fail if the path does not point to a valid restic repository. This guidance applies to repository import components under app/client/modules/repositories (not just the specified file).
Applied to files:
app/client/modules/repositories/tabs/info.tsx
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to app/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/utils/restic.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 cache is stored in `/var/lib/zerobyte/restic/cache`
Applied to files:
app/server/utils/restic.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 must have 0600 permissions and should never be exposed
Applied to files:
app/server/utils/restic.ts
🧬 Code graph analysis (1)
app/server/utils/restic.ts (2)
app/server/core/config.ts (1)
config(26-26)app/server/utils/crypto.ts (1)
cryptoUtils(186-189)
🔇 Additional comments (3)
app/server/utils/restic.ts (3)
815-818: LGTM! Addresses previous review feedback.The unconditional cleanup of
RESTIC_CACERTaddresses the maintainer's previous comment requesting cleanup in any case. This ensures the temporary certificate file is always removed after operations complete.
827-830: LGTM!The
--insecure-tlsflag is correctly added to all restic commands when the internal flag_REST_INSECURE_TLSis set. The implementation is consistent with the SFTP args pattern used above.
152-160: LGTM! The implementation correctly follows the established pattern for handling sensitive credentials:
- Decrypts the CA certificate using
cryptoUtils.resolveSecret- Writes to a temporary file with 0600 permissions (per coding guidelines)
- Sets the
RESTIC_CACERTenvironment variable for REST backend TLS verification- Uses an internal flag
_REST_INSECURE_TLSto control the--insecure-tlsargumentBoth
RESTIC_CACERTand--insecure-tlsare valid restic features documented in the official restic documentation for REST backend configuration.
| {repository.type === "rest" && "cacert" in repository.config && ( | ||
| <> | ||
| <div> | ||
| <div className="text-sm font-medium text-muted-foreground">CA Certificate</div> | ||
| <p className="mt-1 text-sm"> | ||
| {repository.config.cacert ? ( | ||
| <span className="text-green-500">configured</span> | ||
| ) : ( | ||
| <span className="text-muted-foreground">not configured</span> | ||
| )} | ||
| </p> | ||
| </div> | ||
| <div> | ||
| <div className="text-sm font-medium text-muted-foreground">TLS Certificate Validation</div> | ||
| <p className="mt-1 text-sm"> | ||
| {"insecureTls" in repository.config && repository.config.insecureTls ? ( | ||
| <span className="text-red-500">disabled</span> | ||
| ) : ( | ||
| <span className="text-green-500">enabled</span> | ||
| )} | ||
| </p> | ||
| </div> | ||
| </> | ||
| )} |
There was a problem hiding this comment.
Fix conditional rendering logic to show TLS validation status independently.
The current condition requires "cacert" in repository.config to show both the CA Certificate and TLS Certificate Validation sections. However, insecureTls can exist independently of cacert — a user might skip TLS validation without providing a CA certificate. This prevents the TLS validation status from being displayed in that scenario.
🔎 Proposed fix to separate the conditions
- {repository.type === "rest" && "cacert" in repository.config && (
+ {repository.type === "rest" && (
<>
- <div>
- <div className="text-sm font-medium text-muted-foreground">CA Certificate</div>
- <p className="mt-1 text-sm">
- {repository.config.cacert ? (
- <span className="text-green-500">configured</span>
- ) : (
- <span className="text-muted-foreground">not configured</span>
- )}
- </p>
- </div>
+ {"cacert" in repository.config && (
+ <div>
+ <div className="text-sm font-medium text-muted-foreground">CA Certificate</div>
+ <p className="mt-1 text-sm">
+ {repository.config.cacert ? (
+ <span className="text-green-500">configured</span>
+ ) : (
+ <span className="text-muted-foreground">not configured</span>
+ )}
+ </p>
+ </div>
+ )}
<div>
<div className="text-sm font-medium text-muted-foreground">TLS Certificate Validation</div>
<p className="mt-1 text-sm">
{"insecureTls" in repository.config && repository.config.insecureTls ? (
<span className="text-red-500">disabled</span>
) : (
<span className="text-green-500">enabled</span>
)}
</p>
</div>
</>
)}🤖 Prompt for AI Agents
In app/client/modules/repositories/tabs/info.tsx around lines 127 to 150, the
current JSX only renders both the CA Certificate and TLS Certificate Validation
blocks when "cacert" exists in repository.config, which hides the TLS status
when only "insecureTls" is present; split the conditional rendering so the CA
Certificate block is shown when "cacert" in repository.config, and the TLS
Certificate Validation block is shown when "insecureTls" in repository.config
(or otherwise render TLS status independently), keeping the same truthy checks
to choose "configured"/"not configured" for cacert and "disabled"/"enabled" for
insecureTls.
…user to skip tls validation
…s and ca certificate mutually exclusive with tooltips
nicotsx
force-pushed
the
feat/rest-tls-support
branch
from
b815903 to
08a19ed
Compare
3 participants
Overview
I use a self-signed certificate for my rest-server and there wasn't a way for me to use my existing restic repo without adding support for it.
I think it would be nice to be able to edit repositories once they are configured, but I figured I would try to add that in a separate PR if I have time.
What I've added:
One final note - the cacert is being encrypted, but it's really just a public certificate and is not sensitive data. I encrypted mostly to match the other fields, but it could just as easily be unencrypted.
Screenshots
Configured repository WITH the cacert defined:
Configured repository without the cacert but without TLS validation:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.