Config generator for docker-compose.yml #623
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughA new Vue component, Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as ConfigEditor
participant Y as YAML Generator
U->>C: Open configuration interface
C->>C: Activate reactive state based on tab selection (Basic, Database, HTTPS, Storage)
C->>Y: Process inputs and generate Docker Compose YAML
Y-->>C: Return formatted YAML configuration
U->>C: Copy configuration or download YAML file
C-->>U: Perform copy/download action
Possibly related PRs
Suggested labels
Security Recommendations
✨ Finishing Touches🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
docs/en/installation.md:45
- Ensure that is correctly rendered in the documentation context and has been registered as a custom component by the markdown renderer.
<ConfigEditor />
docs/en/installation.md:87
- Confirm that the relative import path for ConfigEditor is accurate given the markdown file's location to avoid runtime issues in the docs.
import ConfigEditor from '../.vitepress/components/ConfigEditor.vue'
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
🧹 Nitpick comments (3)
docs/.vitepress/components/ConfigEditor.vue (3)
431-439: Security improvement: Strengthen password generation.The current password generation function could be improved to guarantee stronger passwords.
Consider enhancing the password generator to ensure it includes at least one character from each character class (uppercase, lowercase, number, symbol):
function generateRandomPassword(length = 16) { const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789!@#$%^&*()-_=+" - let password = "" - for (let i = 0; i < length; i++) { - const randomIndex = Math.floor(Math.random() * charset.length) - password += charset[randomIndex] - } - return password + const lowercase = "abcdefghijklmnopqrstuvwxyz" + const uppercase = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + const numbers = "0123456789" + const symbols = "!@#$%^&*()-_=+" + + // Ensure at least one character from each class + let password = + lowercase.charAt(Math.floor(Math.random() * lowercase.length)) + + uppercase.charAt(Math.floor(Math.random() * uppercase.length)) + + numbers.charAt(Math.floor(Math.random() * numbers.length)) + + symbols.charAt(Math.floor(Math.random() * symbols.length)) + + // Fill the rest randomly + for (let i = 4; i < length; i++) { + const randomIndex = Math.floor(Math.random() * charset.length) + password += charset[randomIndex] + } + + // Shuffle the password + return password.split('').sort(() => 0.5 - Math.random()).join('') }
487-633: Consider modularizing the Docker Compose generation function.The
generateDockerComposefunction is quite lengthy (146 lines) and handles multiple concerns.Breaking this down into smaller, focused functions would improve readability and maintainability:
// Add these helper functions +function generateHomeboxService(config) { + // Extract the homebox service generation logic + // Return the service object +} + +function generatePostgresService(config) { + // Extract the postgres service generation logic + // Return the service object +} + +function generateTraefikService(config) { + // Extract the traefik service generation logic + // Return the service object +} + +function formatComposeYaml(services, volumes) { + // Extract the YAML formatting logic + // Return the formatted YAML string +} function generateDockerCompose() { - const services = { - homebox: { - // ...lengthy homebox configuration - }, - } - - // ...lengthy configuration for other services - - // ...lengthy YAML generation - - return dockerCompose + const services = {} + + // Add homebox service + services.homebox = generateHomeboxService(config) + + // Add postgres service if needed + if (config.databaseType === "postgres") { + services.postgres = generatePostgresService(config) + } + + // Add traefik service if needed + if (config.useTraefik) { + services.traefik = generateTraefikService(config) + } + + // Generate volumes configuration + const volumeNames = getRequiredVolumes(config) + + // Format and return the YAML + return formatComposeYaml(services, volumeNames) }
636-639: Improve the user experience with better clipboard feedback.The current clipboard notification uses an alert dialog which is disruptive to user experience.
Replace the alert with a non-blocking notification:
function copyToClipboard() { navigator.clipboard.writeText(generateDockerCompose()) - alert('Docker Compose configuration has been copied to your clipboard.') + // Create and show a non-blocking notification + const notification = document.createElement('div') + notification.className = 'clipboard-notification' + notification.textContent = 'Docker Compose configuration copied to clipboard!' + notification.style.position = 'fixed' + notification.style.bottom = '20px' + notification.style.right = '20px' + notification.style.padding = '10px 15px' + notification.style.backgroundColor = 'var(--vp-c-brand)' + notification.style.color = 'white' + notification.style.borderRadius = '4px' + notification.style.zIndex = '1000' + document.body.appendChild(notification) + + // Remove after 3 seconds + setTimeout(() => { + notification.style.opacity = '0' + notification.style.transition = 'opacity 0.5s' + setTimeout(() => document.body.removeChild(notification), 500) + }, 3000) }
🛑 Comments failed to post (4)
docs/.vitepress/components/ConfigEditor.vue (4)
1-903: 🛠️ Refactor suggestion
Security note: Add environment variable validation.
Currently, there's no validation of user input that could lead to malformed Docker Compose files or potential injection issues.
Consider adding validation for important fields:
- For the port field, ensure it's a valid port number
- For directory paths, add basic validation to prevent command injection
- For domain names and emails, add format validation
Example validation function:
function validateConfig(config) { const errors = [] // Validate port is a number between 1-65535 const port = parseInt(config.port) if (isNaN(port) || port < 1 || port > 65535) { errors.push("Port must be a number between 1 and 65535") } // Validate domain format if Traefik is enabled if (config.useTraefik) { const domainRegex = /^[a-zA-Z0-9][a-zA-Z0-9-]{0,61}[a-zA-Z0-9](?:\.[a-zA-Z]{2,})+$/ if (!domainRegex.test(config.traefikDomain)) { errors.push("Please enter a valid domain name (e.g., homebox.example.com)") } // Validate email format const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/ if (!emailRegex.test(config.traefikEmail)) { errors.push("Please enter a valid email address for Let's Encrypt") } } // Validate directory paths if using host directories if (config.storageConfig.homeboxStorage.type === "directory") { // Basic validation to prevent command injection if (/[;&|]/.test(config.storageConfig.homeboxStorage.directory)) { errors.push("Directory path contains invalid characters") } } return errors }Then update the download and copy functions to check validation first:
function copyToClipboard() { + const errors = validateConfig(config) + if (errors.length > 0) { + alert('Please fix the following issues before copying:\n' + errors.join('\n')) + return + } navigator.clipboard.writeText(generateDockerCompose()) alert('Docker Compose configuration has been copied to your clipboard.') } function downloadConfig() { + const errors = validateConfig(config) + if (errors.length > 0) { + alert('Please fix the following issues before downloading:\n' + errors.join('\n')) + return + } const element = document.createElement("a") const file = new Blob([generateDockerCompose()], { type: "text/plain" }) element.href = URL.createObjectURL(file) element.download = "docker-compose.yml" document.body.appendChild(element) element.click() document.body.removeChild(element) }
569-579: 🛠️ Refactor suggestion
Security enhancement: Strengthen Traefik configuration.
The Traefik configuration could benefit from additional security hardening.
Consider adding these security-focused configurations:
command: [ "--api.insecure=false", "--providers.docker=true", "--providers.docker.exposedbydefault=false", "--entrypoints.web.address=:80", "--entrypoints.web.http.redirections.entrypoint.to=websecure", "--entrypoints.web.http.redirections.entrypoint.scheme=https", "--entrypoints.websecure.address=:443", + "--entrypoints.websecure.http.tls=true", + "--entrypoints.websecure.http.tls.options=modern@file", "--certificatesresolvers.letsencrypt.acme.tlschallenge=true", `--certificatesresolvers.letsencrypt.acme.email=${config.traefikEmail}`, "--certificatesresolvers.letsencrypt.acme.storage=/letsencrypt/acme.json", + // Add security headers + "--entrypoints.websecure.http.middlewares=secure-headers@file", ],Also, consider adding a volumes mount for a dynamic config file with security settings:
// Configure traefik volumes based on storage type if (config.storageConfig.traefikStorage.type === "volume") { - services.traefik.volumes.push(`${config.storageConfig.traefikStorage.volumeName}:/letsencrypt`) + services.traefik.volumes.push(`${config.storageConfig.traefikStorage.volumeName}:/letsencrypt`) + // Add dynamic configuration file for security options + services.traefik.volumes.push("./traefik_dynamic_conf.yml:/etc/traefik/dynamic_conf.yml:ro") } else { - services.traefik.volumes.push(`${config.storageConfig.traefikStorage.directory}:/letsencrypt`) + services.traefik.volumes.push(`${config.storageConfig.traefikStorage.directory}:/letsencrypt`) + // Add dynamic configuration file for security options + services.traefik.volumes.push("./traefik_dynamic_conf.yml:/etc/traefik/dynamic_conf.yml:ro") }Additionally, consider adding an option to download a sample dynamic configuration file with security headers.
📝 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.// Traefik command configuration with enhanced security options command: [ "--api.insecure=false", "--providers.docker=true", "--providers.docker.exposedbydefault=false", "--entrypoints.web.address=:80", "--entrypoints.web.http.redirections.entrypoint.to=websecure", "--entrypoints.web.http.redirections.entrypoint.scheme=https", "--entrypoints.websecure.address=:443", "--entrypoints.websecure.http.tls=true", "--entrypoints.websecure.http.tls.options=modern@file", "--certificatesresolvers.letsencrypt.acme.tlschallenge=true", `--certificatesresolvers.letsencrypt.acme.email=${config.traefikEmail}`, "--certificatesresolvers.letsencrypt.acme.storage=/letsencrypt/acme.json", // Add security headers "--entrypoints.websecure.http.middlewares=secure-headers@file", ], // Configure traefik volumes based on storage type and add dynamic security config if (config.storageConfig.traefikStorage.type === "volume") { services.traefik.volumes.push(`${config.storageConfig.traefikStorage.volumeName}:/letsencrypt`) // Add dynamic configuration file for security options services.traefik.volumes.push("./traefik_dynamic_conf.yml:/etc/traefik/dynamic_conf.yml:ro") } else { services.traefik.volumes.push(`${config.storageConfig.traefikStorage.directory}:/letsencrypt`) // Add dynamic configuration file for security options services.traefik.volumes.push("./traefik_dynamic_conf.yml:/etc/traefik/dynamic_conf.yml:ro") }
539-559: 🛠️ Refactor suggestion
Security concern: Add healthcheck for PostgreSQL service.
Adding a healthcheck to the PostgreSQL service will ensure that the application only tries to connect when the database is ready to accept connections.
Add a healthcheck configuration to improve stability and security:
// Add PostgreSQL service services["postgres"] = { image: "postgres:14", container_name: "homebox-postgres", restart: "always", + healthcheck: { + test: ["CMD", "pg_isready", "-U", config.postgresConfig.username], + interval: "10s", + timeout: "5s", + retries: 5 + }, environment: [ `POSTGRES_USER=${config.postgresConfig.username}`, `POSTGRES_PASSWORD=${config.postgresConfig.password}`, `POSTGRES_DB=${config.postgresConfig.database}`, ], volumes: [], }Then update the homebox service to depend on postgres health:
const services = { homebox: { image: config.rootless ? "ghcr.io/sysadminsmedia/homebox:latest-rootless" : "ghcr.io/sysadminsmedia/homebox:latest", container_name: "homebox", restart: "always", + depends_on: config.databaseType === "postgres" ? { + postgres: { + condition: "service_healthy" + } + } : undefined, environment: [📝 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.// Add PostgreSQL service services["postgres"] = { image: "postgres:14", container_name: "homebox-postgres", restart: "always", healthcheck: { test: ["CMD", "pg_isready", "-U", config.postgresConfig.username], interval: "10s", timeout: "5s", retries: 5 }, environment: [ `POSTGRES_USER=${config.postgresConfig.username}`, `POSTGRES_PASSWORD=${config.postgresConfig.password}`, `POSTGRES_DB=${config.postgresConfig.database}`, ], volumes: [], } // Configure postgres volumes based on storage type if (config.storageConfig.postgresStorage.type === "volume") { services.postgres.volumes.push(`${config.storageConfig.postgresStorage.volumeName}:/var/lib/postgresql/data`) } else { services.postgres.volumes.push(`${config.storageConfig.postgresStorage.directory}:/var/lib/postgresql/data`) } } // Later in the configuration file, update the homebox service configuration: const services = { homebox: { image: config.rootless ? "ghcr.io/sysadminsmedia/homebox:latest-rootless" : "ghcr.io/sysadminsmedia/homebox:latest", container_name: "homebox", restart: "always", depends_on: config.databaseType === "postgres" ? { postgres: { condition: "service_healthy" } } : undefined, environment: [ // ... other environment settings ] // ... any other configuration settings }, // ... possibly other services }
154-179: 🛠️ Refactor suggestion
Security concern: Password handling could be improved.
The PostgreSQL password implementation has some security aspects that should be addressed:
- The generated passwords are displayed in the final YAML output, which could be accidentally shared.
- Alerts are used to notify about password generation, which may not be ideal from a security perspective.
Consider these improvements:
- Add a note reminding users that the configuration contains sensitive information
- Use a non-blocking notification system instead of alerts
- Add a warning about storing the generated YAML securely
- alert('A new random password has been generated for the database.') + // Use a non-blocking notification instead + const notification = document.createElement('div') + notification.className = 'password-notification' + notification.textContent = 'A new random password has been generated for the database.' + document.body.appendChild(notification) + setTimeout(() => document.body.removeChild(notification), 3000)Also, consider adding a note below the config preview:
<div class="card-content"> <textarea class="code-preview" readonly :value="generateDockerCompose()" ></textarea> + <p class="security-warning">⚠️ This configuration contains sensitive information including passwords. Store securely and avoid sharing.</p> </div>Committable suggestion skipped: line range outside the PR's diff.
|
I think we should support multiple proxies, notably the ones that are already ACME ready (Caddy, Traefik, etc.) Letsencrypt maintains a list of ACME clients and what not https://letsencrypt.org/docs/client-options/ Nginx is kind of a PITA, but also doable if we really wanted to deal with that. |
|
We could support say the most 5 common, cloudflared would also be nice to have. |
|
Works for me |
Deploying homebox-docs with
|
| Latest commit: |
2b5d407
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://38a5adbb.homebox-docs.pages.dev |
| Branch Preview URL: | https://homebox-config-generator.homebox-docs.pages.dev |
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 13
🧹 Nitpick comments (17)
docs/.vitepress/components/BasicConfig.vue (1)
66-76: Add tooltip explaining Auto Increment Asset IDThe purpose of "Auto Increment Asset ID" may not be clear to users. Consider adding a tooltip or help text explaining what this setting does and its security implications.
<div class="form-row"> <label for="autoIncrementAssetId">Auto Increment Asset ID</label> <div class="toggle-switch"> <input type="checkbox" id="autoIncrementAssetId" v-model="config.autoIncrementAssetId" /> <label for="autoIncrementAssetId"></label> </div> + <p class="help-text">When enabled, new assets will receive sequential IDs, which may be more predictable but easier to manage</p> </div>docs/.vitepress/components/HttpsConfig.vue (6)
10-17: Encourage Input Validation for HTTPS Option
Selecting an HTTPS option can have wide implications. Consider validating or handling user input (e.g., checking spelling) whenconfig.httpsOptionis set. This helps guard against misconfiguration.
20-46: Validate Domain & Email for Traefik
For production environments, consider validating the domain/email fields. This can prevent certificate request failures and domain mismatch issues.
48-91: Include SSL Hardening for Nginx
When providing custom SSL certificates, remind users to employ secure cipher suites and recommended SSL settings (e.g., TLS 1.2+). This ensures improved security posture.
93-119: Optional Logging for Caddy
Adding a log file or console logs for Caddy’s HTTPS setup can help troubleshoot issues with certificate issuance or domain resolution.
121-156: Secure Storage of Cloudflare Tunnel Token
Since the token grants privileged access, consider using environment variables or secret storage to avoid embedding credentials directly in the config file.
162-169: Type Definitions for Props
You may want to define stricter type interfaces forconfigto ensure its shape remains consistent (e.g., via TypeScript or a validation library).docs/.vitepress/components/DatabaseConfig.vue (2)
10-14: Double-Check Database Defaults
SQLite as a default is reasonable. Ensure that the user has clarity on performance constraints and backup procedures, especially when switching from SQLite to PostgreSQL.
86-95: Prop Validation
Props are set up well. If you are constrained to plain JavaScript, consider addingvalidatorfunctions to ensureconfigandshowPasswordremain within expected types.docs/.vitepress/components/ConfigEditor.vue (2)
165-168: Visual Feedback on Password Regeneration
Alert-based feedback is acceptable, but consider a more integrated approach (e.g., displaying a toast). This can improve user experience.
170-173: Error Handling for Clipboard Access
navigator.clipboard.writeTextcan fail in certain browser contexts. Consider catching errors and providing fallback instructions for the user if copying fails.docs/.vitepress/components/dockerComposeGenerator.ts (5)
3-22: Consider using Docker secrets for sensitive data.
Your function places multiple secrets (like analytics flags) into environment variables. While this approach works, storing sensitive data in environment variables can create security risks if logs or container inspection are exposed. A more secure approach is to use Docker secrets or a secure vault.
43-81: Reinforce credentials security for PostgreSQL.
Environment variables for database credentials (especially passwords) can inadvertently be disclosed if the container or logs are inspected. Consider employing Docker secrets or a credentials manager for safer production security.
99-117: Optional: Specify Docker Compose version.
Some Docker Compose deployments may require an explicit “version” field (e.g.,version: "3.9") to avoid compatibility warnings. Adding it ensures consistent behavior across environments.+version: "3.9" services: homebox: image: ...
173-214: Be aware of Traefik’s Docker socket mounting.
Mounting/var/run/docker.sockread-only is common for Traefik, but still a security consideration. It grants container-level insights into Docker. Ensure that your environment accepts this risk or consider alternative architectures.
333-369: Cloudflared token handling.
Storing the Cloudflared token in environment variables may facilitate quick setup, but it risks exposure if logs or container configs are leaked. Consider using Docker secrets or environment variable injection from a secure store.docs/.vitepress/components/types.ts (1)
74-91: Interfaces for Docker services are cohesive.
The current definitions facilitate straightforward compose generation. Adding a short doc comment for each property can help maintain clarity as the project grows.
🛑 Comments failed to post (13)
docs/.vitepress/components/BasicConfig.vue (3)
31-38:
⚠️ Potential issueAdd validation for maximum file upload size
The max file upload size field should include validation to prevent potential security risks. Accepting arbitrary file sizes could lead to denial of service vulnerabilities if set too high.
<input type="text" id="maxFileUpload" + pattern="[0-9]+" + min="1" + max="1000" v-model="config.maxFileUpload" /> +<p class="help-text">Recommended maximum: 100MB to prevent resource exhaustion</p>📝 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.<div class="form-group"> <label for="maxFileUpload">Max File Upload (MB)</label> <input type="text" id="maxFileUpload" pattern="[0-9]+" min="1" max="1000" v-model="config.maxFileUpload" /> <p class="help-text">Recommended maximum: 100MB to prevent resource exhaustion</p> </div>
21-29:
⚠️ Potential issueInput validation needed for port field
The port input field currently accepts any value without validation. For better security and user experience, consider adding validation to ensure the port is a valid number within range (1-65535) and not a reserved system port.
<input type="text" id="port" + pattern="[0-9]+" + min="1024" + max="65535" v-model="config.port" />📝 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.<div class="form-group"> <label for="port">External Port</label> <input type="text" id="port" pattern="[0-9]+" min="1024" max="65535" v-model="config.port" /> <p class="help-text">Only used if HTTPS is not enabled</p> </div>
94-101:
⚠️ Potential issueInitialize config properties if undefined
The component assumes all required config properties exist, but doesn't handle cases where they might be missing. This could lead to runtime errors if the parent doesn't properly initialize the config object.
<script setup> +const props = defineProps({ -defineProps({ config: { type: Object, required: true } }) + +// Ensure all required properties exist with defaults +const ensureConfigDefaults = () => { + if (!props.config.rootless) props.config.rootless = false + if (!props.config.port) props.config.port = "3000" + if (!props.config.maxFileUpload) props.config.maxFileUpload = "100" + if (!props.config.allowAnalytics) props.config.allowAnalytics = false + if (!props.config.allowRegistration) props.config.allowRegistration = false + if (!props.config.autoIncrementAssetId) props.config.autoIncrementAssetId = false + if (!props.config.checkGithubRelease) props.config.checkGithubRelease = true +} + +ensureConfigDefaults() </script>📝 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.<script setup> const props = defineProps({ config: { type: Object, required: true } }) // Ensure all required properties exist with defaults const ensureConfigDefaults = () => { if (!props.config.rootless) props.config.rootless = false if (!props.config.port) props.config.port = "3000" if (!props.config.maxFileUpload) props.config.maxFileUpload = "100" if (!props.config.allowAnalytics) props.config.allowAnalytics = false if (!props.config.allowRegistration) props.config.allowRegistration = false if (!props.config.autoIncrementAssetId) props.config.autoIncrementAssetId = false if (!props.config.checkGithubRelease) props.config.checkGithubRelease = true } ensureConfigDefaults() </script>docs/.vitepress/components/ConfigPreview.vue (3)
18-24: 🛠️ Refactor suggestion
Consider security for the configuration content
The textarea displays configuration that might include sensitive information. Consider adding functionality to mask or warn about potential sensitive data.
<div class="card-content"> <textarea class="code-preview" readonly :value="config" + :class="{'has-sensitive': containsSensitiveData}" ></textarea> + <div v-if="containsSensitiveData" class="sensitive-data-warning"> + Warning: This configuration may contain sensitive information. Be careful when sharing. + </div> </div>Add the necessary script code:
import { ref, computed } from 'vue' // Check for potentially sensitive information patterns const containsSensitiveData = computed(() => { const sensitivePatterns = [ /password/i, /secret/i, /key/i, /token/i, /cert/i ] return sensitivePatterns.some(pattern => pattern.test(config.value)) })
8-13:
⚠️ Potential issueAdd security notice about sensitive data in the configuration
The copy and download actions for the configuration could expose sensitive information if the configuration contains secrets. Add a notice about this potential security risk.
<div class="card-actions"> + <div class="security-notice">⚠️ Remove any secrets before sharing</div> <button class="icon-button" @click="$emit('copy')" title="Copy to clipboard"> Copy </button> <button class="icon-button" @click="$emit('download')" title="Download as file"> Download </button> </div>Committable suggestion skipped: line range outside the PR's diff.
40-80: 🛠️ Refactor suggestion
Improve textarea accessibility and security
The current implementation lacks some accessibility features and could benefit from security enhancements for the displayed configuration.
<style scoped> @import './common.css'; .config-preview { height: 100%; } .card { height: 100%; display: flex; flex-direction: column; } .card-content { flex: 1; } .card-title-with-actions { display: flex; justify-content: space-between; align-items: center; } .card-actions { display: flex; gap: 0.5rem; + align-items: center; } +.security-notice { + color: var(--vp-c-danger); + font-size: 0.85rem; + margin-right: 0.5rem; +} .code-preview { width: 100%; height: 600px; font-family: monospace; padding: 1rem; border: 1px solid var(--vp-c-divider); border-radius: 4px; background-color: var(--vp-c-bg); color: var(--vp-c-text-1); resize: none; white-space: pre; overflow: auto; + scrollbar-width: thin; + tab-index: 0; /* Make it keyboard focusable */ } +.code-preview.has-sensitive { + border-color: var(--vp-c-danger); +} + +.sensitive-data-warning { + color: var(--vp-c-danger); + margin-top: 0.5rem; + font-weight: bold; +} </style>Committable suggestion skipped: line range outside the PR's diff.
docs/.vitepress/components/StorageConfig.vue (2)
63-72:
⚠️ Potential issueInitialize storage configuration with safe defaults
The component doesn't handle the case where
config.storageConfigmight be undefined or missing expected keys, which could lead to runtime errors and potential security issues.<script setup> import StorageTypeSelector from './StorageTypeSelector.vue' -defineProps({ +const props = defineProps({ config: { type: Object, required: true } }) +// Initialize storage config structure if missing +const initializeStorageConfig = () => { + if (!props.config.storageConfig) { + props.config.storageConfig = {} + } + + // Initialize storage configs with safe defaults + const storageKeys = ['homeboxStorage', 'postgresStorage', 'traefikStorage', 'nginxStorage', 'caddyStorage', 'cloudflaredStorage'] + + storageKeys.forEach(key => { + if (!props.config.storageConfig[key]) { + props.config.storageConfig[key] = { + type: 'volume', + volumeName: `homebox_${key.replace('Storage', '')}`, + directory: '' + } + } + }) +} + +initializeStorageConfig() </script>📝 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.<script setup> import StorageTypeSelector from './StorageTypeSelector.vue' const props = defineProps({ config: { type: Object, required: true } }) // Initialize storage config structure if missing const initializeStorageConfig = () => { if (!props.config.storageConfig) { props.config.storageConfig = {} } // Initialize storage configs with safe defaults const storageKeys = ['homeboxStorage', 'postgresStorage', 'traefikStorage', 'nginxStorage', 'caddyStorage', 'cloudflaredStorage'] storageKeys.forEach(key => { if (!props.config.storageConfig[key]) { props.config.storageConfig[key] = { type: 'volume', volumeName: `homebox_${key.replace('Storage', '')}`, directory: '' } } }) } initializeStorageConfig() </script>
26-58:
⚠️ Potential issueAdd security warnings for storage type configurations
Different storage types have different security implications. Add warnings about proper permissions and security considerations for each storage type, especially for host directories.
<!-- HTTPS Service Storage --> <StorageTypeSelector v-if="config.httpsOption === 'traefik'" storage-key="traefikStorage" label="Traefik Data Storage" description="Store Traefik certificates in a Docker volume or host directory" + securityInfo="Certificates contain sensitive information. Ensure proper permissions on host directories." :config="config" /> <StorageTypeSelector v-if="config.httpsOption === 'nginx'" storage-key="nginxStorage" label="Nginx Data Storage" description="Store Nginx configuration and certificates in a Docker volume or host directory" + securityInfo="Nginx configurations may contain sensitive information. Restrict access to storage locations." :config="config" /> <StorageTypeSelector v-if="config.httpsOption === 'caddy'" storage-key="caddyStorage" label="Caddy Data Storage" description="Store Caddy configuration and certificates in a Docker volume or host directory" + securityInfo="Certificates contain sensitive information. Ensure proper permissions on host directories." :config="config" /> <StorageTypeSelector v-if="config.httpsOption === 'cloudflared'" storage-key="cloudflaredStorage" label="Cloudflared Data Storage" description="Store Cloudflared configuration in a Docker volume or host directory" + securityInfo="Cloudflared configurations may contain tokens or secrets. Secure your storage location." :config="config" />📝 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.<!-- HTTPS Service Storage --> <StorageTypeSelector v-if="config.httpsOption === 'traefik'" storage-key="traefikStorage" label="Traefik Data Storage" description="Store Traefik certificates in a Docker volume or host directory" securityInfo="Certificates contain sensitive information. Ensure proper permissions on host directories." :config="config" /> <StorageTypeSelector v-if="config.httpsOption === 'nginx'" storage-key="nginxStorage" label="Nginx Data Storage" description="Store Nginx configuration and certificates in a Docker volume or host directory" securityInfo="Nginx configurations may contain sensitive information. Restrict access to storage locations." :config="config" /> <StorageTypeSelector v-if="config.httpsOption === 'caddy'" storage-key="caddyStorage" label="Caddy Data Storage" description="Store Caddy configuration and certificates in a Docker volume or host directory" securityInfo="Certificates contain sensitive information. Ensure proper permissions on host directories." :config="config" /> <StorageTypeSelector v-if="config.httpsOption === 'cloudflared'" storage-key="cloudflaredStorage" label="Cloudflared Data Storage" description="Store Cloudflared configuration in a Docker volume or host directory" securityInfo="Cloudflared configurations may contain tokens or secrets. Secure your storage location." :config="config" /> </div>docs/.vitepress/components/StorageTypeSelector.vue (3)
68-95: 🛠️ Refactor suggestion
Add styling for validation errors and security warnings
Add CSS for displaying validation errors and security warnings to make them visually distinct.
<style scoped> @import './common.css'; .storage-selector { margin-bottom: 2rem; padding: 1rem; border: 1px solid var(--vp-c-divider); border-radius: 4px; } .storage-selector h3 { font-size: 1rem; font-weight: 600; margin: 0 0 0.5rem; } .radio-group { display: flex; gap: 1.5rem; margin: 1rem 0; } .radio-option { display: flex; align-items: center; gap: 0.5rem; } +.error-text { + color: var(--vp-c-danger); + font-size: 0.875rem; + margin-top: 0.25rem; +} + +.security-warning { + background-color: rgba(255, 229, 100, 0.3); + border-left: 2px solid #e7c000; + padding: 0.5rem; + margin-top: 0.5rem; + font-size: 0.875rem; +} + +/* Highlight inputs with errors */ +input:invalid, +input.has-error { + border-color: var(--vp-c-danger); +} </style>📝 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.<style scoped> @import './common.css'; .storage-selector { margin-bottom: 2rem; padding: 1rem; border: 1px solid var(--vp-c-divider); border-radius: 4px; } .storage-selector h3 { font-size: 1rem; font-weight: 600; margin: 0 0 0.5rem; } .radio-group { display: flex; gap: 1.5rem; margin: 1rem 0; } .radio-option { display: flex; align-items: center; gap: 0.5rem; } .error-text { color: var(--vp-c-danger); font-size: 0.875rem; margin-top: 0.25rem; } .security-warning { background-color: rgba(255, 229, 100, 0.3); border-left: 2px solid #e7c000; padding: 0.5rem; margin-top: 0.5rem; font-size: 0.875rem; } /* Highlight inputs with errors */ input:invalid, input.has-error { border-color: var(--vp-c-danger); } </style>
47-66:
⚠️ Potential issueAdd validation and security information for storage paths
Add validation logic and display security information for storage paths to help users understand security implications.
<script setup> +import { ref, watch } from 'vue' -defineProps({ +const props = defineProps({ storageKey: { type: String, required: true }, label: { type: String, required: true }, description: { type: String, required: true }, + securityInfo: { + type: String, + default: '' + }, config: { type: Object, required: true } }) +const volumeNameError = ref('') +const directoryPathError = ref('') + +// Validate volume name follows Docker naming conventions +const validateVolumeName = () => { + const name = props.config.storageConfig[props.storageKey].volumeName + if (!name) { + volumeNameError.value = "Volume name is required" + return + } + + if (!/^[a-zA-Z0-9][a-zA-Z0-9_.-]+$/.test(name)) { + volumeNameError.value = "Volume name must start with alphanumeric character and contain only alphanumeric, underscore, dot, or hyphen" + return + } + + volumeNameError.value = "" +} + +// Validate directory path is secure and properly formatted +const validateDirectoryPath = () => { + const path = props.config.storageConfig[props.storageKey].directory + if (!path) { + directoryPathError.value = "Directory path is required" + return + } + + // Check if path is absolute + if (!/^\//.test(path) && !(/^[A-Z]:\\/.test(path))) { + directoryPathError.value = "Please use absolute paths for security (e.g., /home/user/data)" + return + } + + // Check for suspicious patterns + if (path.includes('..') || path.includes('//')) { + directoryPathError.value = "Path contains potentially unsafe patterns" + return + } + + directoryPathError.value = "" +} + +// Initialize config structure if needed +const ensureConfigStructure = () => { + if (!props.config.storageConfig) { + props.config.storageConfig = {} + } + + if (!props.config.storageConfig[props.storageKey]) { + props.config.storageConfig[props.storageKey] = { + type: 'volume', + volumeName: `homebox_${props.storageKey.replace('Storage', '')}`, + directory: '' + } + } +} + +ensureConfigStructure() </script>📝 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.<script setup> import { ref, watch } from 'vue' const props = defineProps({ storageKey: { type: String, required: true }, label: { type: String, required: true }, description: { type: String, required: true }, securityInfo: { type: String, default: '' }, config: { type: Object, required: true } }) const volumeNameError = ref('') const directoryPathError = ref('') // Validate volume name follows Docker naming conventions const validateVolumeName = () => { const name = props.config.storageConfig[props.storageKey].volumeName if (!name) { volumeNameError.value = "Volume name is required" return } if (!/^[a-zA-Z0-9][a-zA-Z0-9_.-]+$/.test(name)) { volumeNameError.value = "Volume name must start with alphanumeric character and contain only alphanumeric, underscore, dot, or hyphen" return } volumeNameError.value = "" } // Validate directory path is secure and properly formatted const validateDirectoryPath = () => { const path = props.config.storageConfig[props.storageKey].directory if (!path) { directoryPathError.value = "Directory path is required" return } // Check if path is absolute if (!/^\//.test(path) && !(/^[A-Z]:\\/.test(path))) { directoryPathError.value = "Please use absolute paths for security (e.g., /home/user/data)" return } // Check for suspicious patterns if (path.includes('..') || path.includes('//')) { directoryPathError.value = "Path contains potentially unsafe patterns" return } directoryPathError.value = "" } // Initialize config structure if needed const ensureConfigStructure = () => { if (!props.config.storageConfig) { props.config.storageConfig = {} } if (!props.config.storageConfig[props.storageKey]) { props.config.storageConfig[props.storageKey] = { type: 'volume', volumeName: `homebox_${props.storageKey.replace('Storage', '')}`, directory: '' } } } ensureConfigStructure() </script>
27-43:
⚠️ Potential issueAdd directory path validation for security
Host directory paths should be validated to prevent potential path traversal or permission issues. Add validation to ensure proper directory paths and display security warnings.
<div v-if="config.storageConfig[storageKey].type === 'volume'" class="form-group"> <label :for="`${storageKey}-volume-name`">Volume Name</label> <input type="text" :id="`${storageKey}-volume-name`" v-model="config.storageConfig[storageKey].volumeName" + pattern="[a-zA-Z0-9][a-zA-Z0-9_.-]+" + @input="validateVolumeName" /> + <p v-if="volumeNameError" class="error-text">{{ volumeNameError }}</p> </div> <div v-else class="form-group"> <label :for="`${storageKey}-directory-path`">Directory Path</label> <input type="text" :id="`${storageKey}-directory-path`" v-model="config.storageConfig[storageKey].directory" + @input="validateDirectoryPath" /> <p class="help-text">Absolute path recommended (e.g., /home/user/data)</p> + <p v-if="directoryPathError" class="error-text">{{ directoryPathError }}</p> + <p class="security-warning">Warning: Ensure proper permissions on host directories to prevent unauthorized access.</p> </div>📝 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.<div v-if="config.storageConfig[storageKey].type === 'volume'" class="form-group"> <label :for="`${storageKey}-volume-name`">Volume Name</label> <input type="text" :id="`${storageKey}-volume-name`" v-model="config.storageConfig[storageKey].volumeName" pattern="[a-zA-Z0-9][a-zA-Z0-9_.-]+" @input="validateVolumeName" /> <p v-if="volumeNameError" class="error-text">{{ volumeNameError }}</p> </div> <div v-else class="form-group"> <label :for="`${storageKey}-directory-path`">Directory Path</label> <input type="text" :id="`${storageKey}-directory-path`" v-model="config.storageConfig[storageKey].directory" @input="validateDirectoryPath" /> <p class="help-text">Absolute path recommended (e.g., /home/user/data)</p> <p v-if="directoryPathError" class="error-text">{{ directoryPathError }}</p> <p class="security-warning">Warning: Ensure proper permissions on host directories to prevent unauthorized access.</p> </div>docs/.vitepress/components/DatabaseConfig.vue (1)
45-70: 🛠️ Refactor suggestion
Secure PostgreSQL Password Handling
While you allow toggling and regenerating the password, consider storing it externally (e.g., environment variables or secrets) for better security.docs/.vitepress/components/ConfigEditor.vue (1)
71-79: 🛠️ Refactor suggestion
Use Cryptographically Secure RNG
Consider using a cryptographically secure random generator (such as the Web Crypto API) to produce truly random passwords for production scenarios.
|
@tonyaellie What's left on this? So far in testing and playing around it seems to work great. |
|
Need to test the different configs, i also think there was a bug but i forgot lol |
What type of PR is this?
What this PR does / why we need it:
As we add more features such a postgresql support users are likely to want more complex setups than the basic docker-compose.yml we currently offer, this is likely to lead to more requests for support if they make mistakes or dont know how to set this up. This pr adds a generator that creates a customised docker compose based on the users requirements.
This includes a proxy, database, storage settings and more.
Special notes for your reviewer:
This code was basically all ai generated based on the docs so this will need extensive testing. We also need to decide whether there are any options we want to add or remove and whether Traefik is the proxy we want to suggest (we could support multiple?).
Summary by CodeRabbit
New Features
Documentation