fix(config): stricter validation for log_level + webhook_api_key (#46)#116
Conversation
- log_level: Literal["DEBUG","INFO","WARNING","ERROR","CRITICAL"] statt str
→ Tippfehler ("debug", "TRACE") werfen jetzt ValidationError beim Startup
- webhook_api_key: whitespace-only Keys (>=32 Zeichen Spaces/Tabs) werden
abgelehnt — sonst trügerische Sicherheit, da der Webhook-Check nur die
Länge prüft.
Issue #46 Teil "ql820_port/pt750w_port range validation" ist obsolet:
Phase 1i CA-1 hat diese Felder durch printers.yaml ersetzt.
Tests:
- test_settings_log_level_accepts_valid (5 Werte parametrisiert)
- test_settings_log_level_rejects_invalid (6 Werte parametrisiert)
- test_settings_log_level_default_is_info (Regression)
- test_settings_webhook_api_key_rejects_whitespace_only (3 Patterns)
- test_settings_webhook_api_key_accepts_real_key
- test_settings_webhook_api_key_empty_still_accepted (Bootstrap-Pfad)
Closes #46
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! Dieser Pull Request verbessert die Konfigurationsvalidierung der Anwendung, um die Robustheit beim Startup zu erhöhen. Durch die Einschränkung des log_level-Feldes auf definierte Literale und die zusätzliche Prüfung des webhook_api_key auf Whitespace-Inhalte werden potenzielle Fehlkonfigurationen frühzeitig erkannt und verhindert. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces stricter validation for configuration settings in backend/app/config.py by restricting log_level to a literal of standard Python log levels and rejecting whitespace-only values for webhook_api_key, accompanied by comprehensive unit tests. The review feedback highlights a security risk where leading or trailing whitespace can bypass the minimum length requirement of 32 characters (e.g., 31 spaces followed by a single character). It is recommended to validate the length of the stripped key and return the cleaned SecretStr to prevent this vulnerability.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| secret = v.get_secret_value() | ||
| if secret and len(secret) < 32: | ||
| if not secret: | ||
| return v | ||
| if len(secret) < 32: | ||
| raise ValueError("PRINTER_HUB_WEBHOOK_API_KEY must be at least 32 characters") | ||
| if secret.strip() == "": | ||
| raise ValueError("PRINTER_HUB_WEBHOOK_API_KEY must not be whitespace-only") | ||
| return v |
There was a problem hiding this comment.
Security Risk / Validation Bypass:
If an API key contains leading or trailing whitespace (e.g., " " * 31 + "a"), the total length is 32 characters. This causes the len(secret) < 32 check to incorrectly pass. Since the key is not completely empty, the secret.strip() == "" check also does not trigger. However, the actual key after stripping has only a single significant character ("a"), which completely bypasses the required minimum length of 32 characters for entropy.
Recommendation:
Validate the length of the cleaned (stripped) key. Additionally, it is advisable to return the cleaned value directly as a SecretStr. This automatically cleans up accidentally copied whitespace or line breaks (e.g., from .env files), increasing robustness.
| secret = v.get_secret_value() | |
| if secret and len(secret) < 32: | |
| if not secret: | |
| return v | |
| if len(secret) < 32: | |
| raise ValueError("PRINTER_HUB_WEBHOOK_API_KEY must be at least 32 characters") | |
| if secret.strip() == "": | |
| raise ValueError("PRINTER_HUB_WEBHOOK_API_KEY must not be whitespace-only") | |
| return v | |
| secret = v.get_secret_value() | |
| if not secret: | |
| return v | |
| stripped = secret.strip() | |
| if not stripped: | |
| raise ValueError("PRINTER_HUB_WEBHOOK_API_KEY must not be whitespace-only") | |
| if len(stripped) < 32: | |
| raise ValueError("PRINTER_HUB_WEBHOOK_API_KEY must be at least 32 characters (excluding leading/trailing whitespace)") | |
| return SecretStr(stripped) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #116 +/- ##
==========================================
+ Coverage 89.22% 89.26% +0.03%
==========================================
Files 91 91
Lines 4251 4257 +6
Branches 368 369 +1
==========================================
+ Hits 3793 3800 +7
+ Misses 360 359 -1
Partials 98 98
... and 2 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Gemini-Review-Feedback auf PR #116: die Length-Check verglich gegen die Rohlänge des Secrets — das ließ ``" " * 31 + "a"`` durchgehen (32 Zeichen formal, aber nur 1 Zeichen echtes Auth-Material). - `len(stripped) < 32` statt `len(secret) < 32` - Validator gibt jetzt `SecretStr(stripped)` zurück, damit später konsistent gegen den effektiven Key verglichen wird (kein Trailing-Whitespace-Drift) Tests: - `test_settings_webhook_api_key_rejects_short_effective_length` mit 3 Patterns (Gemini-Pattern, Whitespace-Padding, Tab+Newline) - `test_settings_webhook_api_key_strips_leading_trailing_whitespace` 40 Tests grün, mypy + ruff sauber. Refs PR #116 Review-Comment 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Round-1 addressed (Gemini): |
## <small>0.10.2 (2026-06-15)</small> * refactor(main): _resolve_model_id_from_config nimmt PrinterYAMLConfig statt Any (#67) (#123) ([2ff51d2](2ff51d2)), closes [#67](#67) [#123](#123) [#67](#67) [#59](#59) [#67](#67) [#59](#59) * refactor(snipeit): typisiere response payload als TypedDict statt dict[str, Any] (#67) (#122) ([e88601b](e88601b)), closes [#67](#67) [#122](#122) [#67](#67) [#67](#67) [#51](#51) * perf(lifespan): plugin-discovery in Thread auslagern (Audit #67) (#121) ([48215b4](48215b4)), closes [#67](#67) [#121](#121) [#67](#67) [#59](#59) * fix(config): stricter validation for log_level + webhook_api_key (#46) (#116) ([6b8ac30](6b8ac30)), closes [#46](#46) [#116](#116) [#46](#46) [#46](#46) [#46](#46) [#46](#46) [#116](#116) * fix(print): copies-replication erzeugt UNIQUE job_ids (Round 2 #hangar-109) (#120) ([bd781ae](bd781ae)), closes [#hangar-109](https://github.com/strausmann/label-printer-hub/issues/hangar-109) [#120](#120) [#115](#115) * fix(print): PrintOptions.copies replicates images for hardware print (#hangar-109) (#115) ([d27bbcc](d27bbcc)), closes [#hangar-109](https://github.com/strausmann/label-printer-hub/issues/hangar-109) [#115](#115) [strausmann/hangar#109](https://github.com/strausmann/hangar/issues/109) * docs(metrics): präzisiere sse_events_published_total Beschreibung (Audit #67) (#119) ([ab8c4c4](ab8c4c4)), closes [#67](#67) [#119](#119) [#67](#67) [#67](#67) [#65](#65) * docs(ptouch): korrigiere Tape-Klassen-Namen in ptouch-integration.md (Audit #67) (#118) ([0faab99](0faab99)), closes [#67](#67) [#118](#118) [#67](#67) [#59](#59) * chore(readme): codecov badge ohne embedded Token (Audit #67) (#117) ([4bb5423](4bb5423)), closes [#67](#67) [#117](#117) [#67](#67) [#59](#59) [skip ci]
Summary
log_level: Literal["DEBUG","INFO","WARNING","ERROR","CRITICAL"]stattstr— Tippfehler werfen jetztValidationErrorbeim Startup statt lautlos defaultenwebhook_api_key: whitespace-only Keys (≥32 Zeichen Spaces/Tabs/Newlines) werden abgelehnt — sonst trügerische Sicherheit, da der Webhook-Check nur die Länge prüftHinweis zu Issue #46 Scope
Issue #46 fordert auch
Field(ge=1, le=65535)fürql820_port+pt750w_port. Diese Felder wurden in Phase 1i CA-1 (PR #95) entfernt und durchprinters.yamlersetzt. Das Port-Range-Constraint wandert dorthin und wird mit Phase 1k.1b/c separat addressiert.Tests
Neu in
backend/tests/unit/test_config.py:test_settings_log_level_accepts_validDEBUG–CRITICAL) — parametrisierttest_settings_log_level_rejects_invaliddebug,TRACE,"",INFOO) — parametrisierttest_settings_log_level_default_is_infoINFOtest_settings_webhook_api_key_rejects_whitespace_onlytest_settings_webhook_api_key_accepts_real_key"a"wird akzeptierttest_settings_webhook_api_key_empty_still_acceptedmypy ✓ ruff check ✓ ruff format ✓
Closes #46