-
-
Notifications
You must be signed in to change notification settings - Fork 251
Added documentation for the external label service feature. #1018
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…d the columns in the config page to make it easier to read.
WalkthroughAdds a new "External Label Service" documentation page, updates the Advanced menu (moves Import CSV under Advanced and adds External Label Service link), and restructures parts of the configuration docs (table column reorder, expanded HBOX_WEB_HOST/systemd examples and minor formatting). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Web as Homebox Web
participant Ext as External Label Service
User->>Web: Request label generation
Note over Web: Build GET request with query params and headers
Web->>Ext: GET /... (TitleText, DescriptionText, URL, etc.)
rect rgba(200,230,255,0.25)
Note over Ext: Generate label (PNG/SVG/PDF) and return binary
end
Ext-->>Web: Binary response (<= max size, valid Content-Type)
alt Success
Web-->>User: Return generated label
else Timeout / Invalid
Web-->>User: Error response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Security recommendations
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/en/configure/index.md (1)
163-166: Align HBOX_MODE defaults — docs vs code vs Dockerfiles
- Finding: config default is
development(backend/internal/sys/config/conf.go:21); Dockerfiles set ENV HBOX_MODE=production (Dockerfile, Dockerfile.rootless, Dockerfile.hardened); docs/en/configure/index.md listsproductionin the table but the CLI OPTIONS entry shows(default: development).- Action: either update docs/en/configure/index.md to show default
developmentand add a note that official Docker images set HBOX_MODE=production, or change backend default toproduction(and update tests) if production is the intended app default.- Locations: docs/en/configure/index.md (table + CLI options), backend/internal/sys/config/conf.go, Dockerfile, Dockerfile.rootless, Dockerfile.hardened.
- Security: ensure production mode in deployed images disables debug endpoints, reduces verbose logging, and that secrets are not baked into images or the repo.
🧹 Nitpick comments (14)
docs/en/advanced/external-label-service.md (7)
3-3: Capitalize brand nameCapitalize Homebox for consistency with the rest of the docs.
-You can use an external web service to generate asset and location labels in homebox. This is useful if you have custom requirements for your labels and are happy to spin up a web service that can accept incoming requests and return an image file for homebox to use. +You can use an external web service to generate asset and location labels in Homebox. This is useful if you have custom requirements for your labels and are happy to spin up a web service that can accept incoming requests and return an image file for Homebox to use.
5-9: Admonition syntax: remove quotes around titleVitePress admonitions generally don’t use quoted titles.
-::: info "Note" +::: info Note
13-13: Fix typos in configuration sentenceCorrect spelling and use “environment variable”.
-The extenal service is configured using the `HBOX_LABEL_MAKER_LABEL_SERVICE_URL` enviroment variable. +The external service is configured using the `HBOX_LABEL_MAKER_LABEL_SERVICE_URL` environment variable.
19-26: Heading level increment (MD001) and section structureUse h3 under an h2; switch “####” to “###”.
-#### Headers +### Headers @@ -#### Parameters +### Parameters
17-18: Query-string length risks with GETIf DescriptionText/AdditionalInformation can be long or multi‑line, GET URLs may exceed practical limits. Consider documenting max lengths, or allowing POST as an alternative.
Can you confirm whether the implementation enforces length limits or supports POST?
47-51: Units and header wording
- Use MB (megabytes) rather than Mb (megabits).
- Tighten Content-Type wording.
-- **Size:** Less than or equal to `HBOX_WEB_MAX_UPLOAD_SIZE` (Default: 10Mb) +- **Size:** Less than or equal to `HBOX_WEB_MAX_UPLOAD_SIZE` (Default: 10 MB) @@ -- **Content-Type**: Specified in the response header should be of the type image/* +- **Content-Type**: Must be an image/* type
43-53: Add a Security section (HTTPS, allowlist, size/time limits, content-type validation, auth)Document minimum operational safeguards for the external call.
## Response The external service should respond with the following specifications; - **Size:** Less than or equal to `HBOX_WEB_MAX_UPLOAD_SIZE` (Default: 10Mb) - **Content-Type**: Specified in the response header should be of the type image/* - **Time**: Within the time specified in `HBOX_LABEL_MAKER_LABEL_SERVICE_TIMEOUT` (Default 30s) + +### Security recommendations + +- Always use HTTPS for `HBOX_LABEL_MAKER_LABEL_SERVICE_URL`. +- Restrict the destination via allowlisting (avoid arbitrary egress to reduce SSRF risk). +- Enforce `HBOX_WEB_MAX_UPLOAD_SIZE` and fail closed on missing/invalid `Content-Type`. +- Prefer an authentication mechanism (API key, mTLS, or request signing). +- Log request IDs and timing; avoid logging sensitive content. +- Consider rate limiting and backoff on failures.docs/en/configure/index.md (7)
12-12: Tighten wording and punctuationMinor clarity/punctuation tweak.
-| HBOX_MODE | application mode used for runtime behavior can be one of: `development`, `production` | `production` | +| HBOX_MODE | Application mode for runtime behavior; one of: `development`, `production` | `production` |
17-21: Typo fixes: currencies, serverFix spelling and grammar.
-| HBOX_OPTIONS_CURRENCY_CONFIG | json configuration file containing additional currencie | | +| HBOX_OPTIONS_CURRENCY_CONFIG | JSON configuration file containing additional currencies | | @@ -| HBOX_OPTIONS_ALLOW_ANALYTICS | Allows the homebox team to view extremely basic information about the system that your running on. This helps make decisions regarding builds and other general decisions. | false | +| HBOX_OPTIONS_ALLOW_ANALYTICS | Allows the Homebox team to view extremely basic information about the system that you're running on. This helps make decisions regarding builds and other general decisions. | false | @@ -| HBOX_WEB_READ_TIMEOUT | Read timeout of HTTP sever | 10s | +| HBOX_WEB_READ_TIMEOUT | Read timeout of HTTP server | 10s |
46-50: Add missing environment variable: HBOX_LABEL_MAKER_MARGINMargin is referenced elsewhere (parameters table and CLI args) but absent here.
| HBOX_LABEL_MAKER_FONT_SIZE | font size for label text | 32.0 | +| HBOX_LABEL_MAKER_MARGIN | outer margin around the label in pixels | 32 | | HBOX_LABEL_MAKER_PRINT_COMMAND | the command to use for printing labels. if empty, label printing is disabled. <span v-pre>`{{.FileName}}`</span> in the command will be replaced with the png filename of the label | |
59-66: Wording consistency (US vs UK) and clarityMake phrasing clearer and consistent with US spelling used elsewhere.
-| 0.0.0.0 | Visible all interfaces (default behaviour) | +| 0.0.0.0 | Visible on all interfaces (default behavior) |
93-107: Systemd unit install path note (optional)Consider mentioning /etc/systemd/system as a common admin-managed location (besides /usr/local/lib/systemd/system).
-# /usr/local/lib/systemd/system/homebox.socket +# /usr/local/lib/systemd/system/homebox.socket +# Alternatively: /etc/systemd/system/homebox.socket
111-130: Systemd service: add optional enable/start guidanceConsider adding an enable note for persistence across reboots.
# /usr/local/lib/systemd/system/homebox.service @@ SystemCallFilter=@system-service-Usage:
+Usage:
@@
-systemctl start homebox.socket
+systemctl daemon-reload
+systemctl enable --now homebox.socket--- `156-204`: **CLI help block naming (“Usage: api …”)** If the binary is “homebox”, consider updating the Usage header; otherwise ignore. ```diff -Usage: api [options] [arguments] +Usage: homebox [options] [arguments]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/.vitepress/menus/en.mts(1 hunks)docs/en/advanced/external-label-service.md(1 hunks)docs/en/configure/index.md(4 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/en/advanced/external-label-service.md
19-19: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
🔇 Additional comments (2)
docs/.vitepress/menus/en.mts (1)
23-25: Menu updates look good — targets exist; no other locale menus detectedConfirmed: docs/en/advanced/import-csv.md and docs/en/advanced/external-label-service.md exist. No other locale menu files found under docs/.vitepress/menus/ — add/update locale menus only if translations are required.
Security: scan these docs/frontmatter for accidental secrets and verify external links point to trusted destinations.
docs/en/advanced/external-label-service.md (1)
27-42: Document label-maker defaults & external query keysThe server already sets defaults — update the docs table to list those sources/defaults and units (px, dpi, float).
- TitleFontSize: from config.LabelMaker.FontSize (backend/internal/sys/config/conf.go — default 32.0).
- DescriptionFontSize: computed as TitleFontSize * 0.8 (backend/pkgs/labelmaker/labelmaker.go — NewGenerateParams).
- Dpi: default = 72 (backend/pkgs/labelmaker/labelmaker.go — NewGenerateParams).
- QrSize: computed as Height - (Padding * 2) (backend/pkgs/labelmaker/labelmaker.go — NewGenerateParams).
- The service request uses these query keys: Width, Height, QrSize, Margin, ComponentPadding, TitleText, TitleFontSize, DescriptionText, DescriptionFontSize, Dpi, URL, DynamicLength, AdditionalInformation (backend/pkgs/labelmaker/labelmaker.go — fetchLabelFromURL).
Action: update docs/en/advanced/external-label-service.md to show the above defaults, units, and which values come from config (HBOX_LABEL_MAKER_*) vs which are derived or sent as query params.
Security (must): only call trusted HTTPS label services, enforce LabelServiceTimeout, validate response content-type/size, avoid following untrusted redirects, and whitelist allowed hosts.
Likely an incorrect or invalid review comment.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/en/configure/index.md (1)
155-205: HBOX_MODE default mismatch — code = development but docs table lists production
- backend/internal/sys/config/conf.go: Mode has
conf:"default:development"(authoritative).- docs/en/configure/index.md: variable table default is
production(incorrect); the CLI help block in the same doc shows(default: development)(correct).- Dockerfile, Dockerfile.rootless, Dockerfile.hardened set
ENV HBOX_MODE=production(image-level override).Action: update docs/en/configure/index.md table to show
development, or if the intended default isproductionchange backend/internal/sys/config/conf.go and align all Dockerfiles accordingly. Ensure Dockerfile ENV choices are intentional and documented.Security recommendation: for production images ensure debug flags are disabled, secrets aren’t baked into images, and TLS/DB credentials are configured securely; document that Dockerfile ENV overrides binary defaults.
🧹 Nitpick comments (8)
docs/en/configure/index.md (8)
17-21: Fix typos and tighten wordingMinor language fixes improve clarity and professionalism.
-| HBOX_OPTIONS_CURRENCY_CONFIG | json configuration file containing additional currencie | | +| HBOX_OPTIONS_CURRENCY_CONFIG | JSON configuration file containing additional currencies | | -| HBOX_OPTIONS_ALLOW_ANALYTICS | Allows the homebox team to view extremely basic information about the system that your running on. This helps make decisions regarding builds and other general decisions. | false | +| HBOX_OPTIONS_ALLOW_ANALYTICS | Allows the Homebox team to view extremely basic information about the system you're running. This helps inform build and other general decisions. | false | -| HBOX_WEB_READ_TIMEOUT | Read timeout of HTTP sever | 10s | +| HBOX_WEB_READ_TIMEOUT | Read timeout of HTTP server | 10s |
51-52: Label Service row now aligned with feature docs; add accepted MIME types and security noteGood correction to “image” vs “PDF.” Consider explicitly listing accepted types to avoid confusion (e.g., image/png, image/jpeg, image/webp).
- Security: Document that Homebox will perform an HTTP GET to this URL; advise operators to restrict egress and host the service on a trusted/allow‑listed network to reduce SSRF/exfiltration risk. Recommend TLS with proper certificate validation and timeouts consistent with HBOX_LABEL_MAKER_LABEL_SERVICE_TIMEOUT.
60-66: Polish example labelsMinor grammar tweak.
-| 0.0.0.0 | Visible all interfaces (default behaviour) | +| 0.0.0.0 | Visible on all interfaces (default behavior) |
93-107: Systemd socket example looks good; consider common install path noteLooks correct and secure (group-restricted 0660).
- Add a note that many distros prefer placing admin-managed units under /etc/systemd/system/ (with the same content), or keep it as-is if your packaging standardizes /usr/local/lib/systemd/system/.
111-131: Service unit solid; consider restart policyDynamicUser and hardening are good. Add a minimal restart policy for resilience.
[Service] DynamicUser=yes StateDirectory=homebox Environment=HBOX_WEB_HOST=sysd?name=homebox.socket WorkingDirectory=/var/lib/homebox ExecStart=/usr/local/bin/homebox +Restart=on-failure +RestartSec=3s NoNewPrivileges=yes CapabilityBoundingSet= RestrictNamespaces=true SystemCallFilter=@system-service
132-136: Usage: add enable for persistenceConsider including enable so the socket starts on boot.
systemctl start homebox.socket +systemctl enable homebox.socket
139-146: Fix capitalization and TLS terminologyMinor correctness in security guidance.
-For postgreSQL in production: +For PostgreSQL in production: -- Always use SSL (`sslmode=require` or `sslmode=verify-full`) +- Always use TLS (`sslmode=require` or `sslmode=verify-full`)
192-200: Add missing env var row for margin (present in CLI)CLI includes --label-maker-margin with default 32, but there is no HBOX_LABEL_MAKER_MARGIN row in the env table. Add it for parity.
| HBOX_LABEL_MAKER_PADDING | space between elements on label | 32 | +| HBOX_LABEL_MAKER_MARGIN | outer margin around the label (pixels) | 32 | | HBOX_LABEL_MAKER_FONT_SIZE | font size for label text | 32.0 |
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/en/configure/index.md(4 hunks)
🔇 Additional comments (1)
docs/en/configure/index.md (1)
10-12: Column reorder improves scanabilityThe (Variable, Description, Default) order is clearer when scrolling. Nice improvement.
Added documentation for the external label service feature, and re-ordered columns in the config page to make it easier to read due to scrolling.
What type of PR is this?
(REQUIRED)
What this PR does / why we need it:
This lets users know about the ability for homebox to reach out to an external web service to generate custom labels. This feature was added by @alhafoudh a few months ago in #913. Have also re-ordered the columns in the config page to make it easier to read.
Which issue(s) this PR fixes:
(REQUIRED)
Relates #913
Summary by CodeRabbit