Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions backend/app/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,13 @@
from __future__ import annotations

from functools import lru_cache
from typing import Literal

from pydantic import Field, SecretStr, field_validator
from pydantic_settings import BaseSettings, SettingsConfigDict

LogLevel = Literal["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"]


class Settings(BaseSettings):
"""Application-wide runtime configuration.
Expand Down Expand Up @@ -79,7 +82,10 @@ class Settings(BaseSettings):

# Server
server_port: int = 8090
log_level: str = "INFO"
log_level: LogLevel = "INFO"
"""Python-Loglevel — auf die fünf dokumentierten Werte beschränkt
(Issue #46). Ungültige Werte (Tippfehler, lowercase) erzeugen einen
ValidationError beim Startup statt lautlos defaulten."""

# SSE EventBus — configurable resource limits
sse_queue_size: int = Field(default=32, gt=0)
Expand Down Expand Up @@ -130,15 +136,24 @@ class Settings(BaseSettings):
@field_validator("webhook_api_key")
@classmethod
def validate_api_key_length(cls, v: SecretStr) -> SecretStr:
"""Reject keys shorter than 32 characters.
"""Reject keys shorter than 32 characters or whitespace-only keys.

An empty string is accepted so that the hub can start without
webhook authentication configured (the webhook endpoint will
refuse all requests at runtime, but startup succeeds).

Issue #46: whitespace-only keys (e.g. accidentally pasted as
``\" \"``) sind faktisch leere Keys —
sie schützen nichts und sollten beim Startup fehlschlagen statt
eine trügerische Sicherheit zu suggerieren.
"""
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

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.

Suggested change
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)



Expand Down
67 changes: 67 additions & 0 deletions backend/tests/unit/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,3 +163,70 @@ def test_settings_printers_config_env_override(monkeypatch: pytest.MonkeyPatch)
monkeypatch.setenv("PRINTER_HUB_PRINTERS_CONFIG", "/custom/path/printers.yaml")
s = Settings(_env_file=None)
assert s.printers_config == "/custom/path/printers.yaml"


# ---------------------------------------------------------------------------
# Issue #46 — Stricter validation for log_level and webhook_api_key
# ---------------------------------------------------------------------------


@pytest.mark.parametrize(
"level",
["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"],
)
def test_settings_log_level_accepts_valid(level: str) -> None:
"""Alle dokumentierten Python-Loglevels muessen akzeptiert werden."""
s = Settings(log_level=level, _env_file=None) # type: ignore[call-arg]
assert s.log_level == level


@pytest.mark.parametrize(
"invalid",
["debug", "info", "TRACE", "VERBOSE", "", "INFOO"],
)
def test_settings_log_level_rejects_invalid(invalid: str) -> None:
"""log_level=Literal: ungueltige Werte muessen einen ValidationError werfen."""
from pydantic import ValidationError

with pytest.raises(ValidationError):
Settings(log_level=invalid, _env_file=None) # type: ignore[call-arg]


def test_settings_log_level_default_is_info() -> None:
"""Default-Loglevel bleibt INFO (keine Regression)."""
s = Settings(_env_file=None)
assert s.log_level == "INFO"


@pytest.mark.parametrize(
"whitespace_key",
[
" " * 32,
"\t" * 33,
" \t\n" * 12 + " ",
],
)
def test_settings_webhook_api_key_rejects_whitespace_only(whitespace_key: str) -> None:
"""Whitespace-only Keys mit >=32 Zeichen muessen abgelehnt werden.

Ohne Validation wuerden Tippfehler wie ein versehentlicher Space-Wall
im .env durchgehen — und der Webhook-Endpunkt akzeptiert dann jedes
Header-Token das whitespace ist.
"""
from pydantic import ValidationError

with pytest.raises(ValidationError, match="whitespace"):
Settings(webhook_api_key=whitespace_key, _env_file=None) # type: ignore[call-arg]


def test_settings_webhook_api_key_accepts_real_key() -> None:
"""Echte Keys (32+ Zeichen, kein whitespace-only) bleiben akzeptiert."""
real_key = "a" * 32
s = Settings(webhook_api_key=real_key, _env_file=None) # type: ignore[call-arg]
assert s.webhook_api_key.get_secret_value() == real_key


def test_settings_webhook_api_key_empty_still_accepted() -> None:
"""Leerer Key (Phase 1-Default) bleibt erlaubt fuer Bootstrap ohne Webhook-Auth."""
s = Settings(webhook_api_key="", _env_file=None) # type: ignore[call-arg]
assert s.webhook_api_key.get_secret_value() == ""
Loading