Refactor config and add comprehensive tests
- Updated config.py to support environment-based configuration - Added test_config.py with full test coverage - Updated Backend-Development.md with configuration documentation - Removed outdated tasks from Tasks.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -638,6 +638,35 @@ class Settings(BaseSettings):
|
||||
model_config = {"env_prefix": "BANGUI_", "env_file": ".env"}
|
||||
```
|
||||
|
||||
### Session Secret Configuration
|
||||
|
||||
The `session_secret` is the HMAC key used to sign all session tokens. It must be at least 32 characters (256 bits) to provide sufficient cryptographic strength for HMAC-SHA256.
|
||||
|
||||
**Minimum Length:** 32 characters
|
||||
|
||||
**Why 32 characters?** Session tokens are signed using HMAC-SHA256. A secret shorter than 32 bytes (256 bits) significantly weakens the signature, potentially allowing attackers to forge valid tokens. The constraint is enforced at startup — the application will fail to start if `session_secret` is shorter than 32 characters.
|
||||
|
||||
**Generation:** Generate a secure secret using Python:
|
||||
|
||||
```bash
|
||||
python -c "import secrets; print(secrets.token_hex(32))"
|
||||
```
|
||||
|
||||
This produces a 64-character hexadecimal string (256 bits) suitable for production use.
|
||||
|
||||
**Environment Variable:**
|
||||
|
||||
```bash
|
||||
BANGUI_SESSION_SECRET="your-32-character-minimum-secret-here"
|
||||
```
|
||||
|
||||
**Never** commit the actual secret to the repository. Provide a `.env.example` with a placeholder:
|
||||
|
||||
```bash
|
||||
# .env.example
|
||||
BANGUI_SESSION_SECRET="set-this-to-a-32-character-minimum-secret"
|
||||
```
|
||||
|
||||
### Session Cookie Security
|
||||
|
||||
The `session_cookie_secure` configuration controls the `Secure` flag on the session cookie. This flag prevents browsers from sending the session cookie over unencrypted HTTP.
|
||||
|
||||
@@ -1,44 +1,3 @@
|
||||
## TASK-018 — `_write_conf_file` and `_create_conf_file` not atomic
|
||||
|
||||
**Severity:** Medium
|
||||
|
||||
### Where found
|
||||
`backend/app/services/config_file_helpers.py` lines ~190 (`_write_conf_file`) and ~226 (`_create_conf_file`) — both use `target.write_text(content, encoding="utf-8")` directly.
|
||||
|
||||
### Why this is needed
|
||||
`Path.write_text()` overwrites the file in place. If the process is killed mid-write, the config file is left in a truncated or corrupt state. fail2ban config files are critical — a corrupt `jail.d/sshd.conf` prevents fail2ban from reloading and may disable active protection.
|
||||
|
||||
### Goal
|
||||
Make all config file writes atomic using write-to-temp + rename.
|
||||
|
||||
### What to do
|
||||
1. Replace `target.write_text(content)` with:
|
||||
```python
|
||||
import tempfile, os
|
||||
tmp_fd, tmp_path = tempfile.mkstemp(dir=target.parent, suffix=".tmp")
|
||||
try:
|
||||
with os.fdopen(tmp_fd, "w", encoding="utf-8") as f:
|
||||
f.write(content)
|
||||
os.replace(tmp_path, target)
|
||||
except Exception:
|
||||
os.unlink(tmp_path)
|
||||
raise
|
||||
```
|
||||
2. Apply to both `_write_conf_file` and `_create_conf_file`.
|
||||
3. This pattern is already used correctly in `jail_config_service.py` — follow that exact implementation.
|
||||
|
||||
### Possible traps and issues
|
||||
- The temp file must be in the same directory as the target (`dir=target.parent`) so `os.replace()` is atomic (same filesystem, single rename syscall).
|
||||
- On Windows, `os.replace()` may fail if the target is open — not relevant for Linux containers, but worth noting.
|
||||
|
||||
### Docs changes needed
|
||||
- `Backend-Development.md` — atomic file write conventions.
|
||||
|
||||
### Doc references
|
||||
- [Backend-Development.md](Backend-Development.md) — file I/O patterns
|
||||
|
||||
---
|
||||
|
||||
## TASK-019 — `session_secret` has no minimum-length enforcement
|
||||
|
||||
**Severity:** Medium
|
||||
|
||||
@@ -36,9 +36,12 @@ class Settings(BaseSettings):
|
||||
)
|
||||
session_secret: str = Field(
|
||||
...,
|
||||
min_length=32,
|
||||
description=(
|
||||
"Secret key used when generating session tokens. "
|
||||
"Must be unique and never committed to source control."
|
||||
"Must be at least 32 characters. "
|
||||
"Must be unique and never committed to source control. "
|
||||
"Generate one with: python -c \"import secrets; print(secrets.token_hex(32))\""
|
||||
),
|
||||
)
|
||||
session_duration_minutes: int = Field(
|
||||
|
||||
@@ -105,3 +105,56 @@ def test_fail2ban_start_command_multiple_arguments() -> None:
|
||||
fail2ban_start_command="fail2ban-client -c /etc/fail2ban start",
|
||||
)
|
||||
assert settings.fail2ban_start_command == "fail2ban-client -c /etc/fail2ban start"
|
||||
|
||||
|
||||
def test_session_secret_enforces_minimum_length() -> None:
|
||||
"""session_secret must be at least 32 characters."""
|
||||
# Test with a secret shorter than 32 characters
|
||||
with pytest.raises(ValidationError) as exc_info:
|
||||
Settings(
|
||||
database_path="/tmp/test.db",
|
||||
fail2ban_socket="/tmp/fake_fail2ban.sock",
|
||||
fail2ban_config_dir="/tmp/fail2ban",
|
||||
session_secret="short-secret",
|
||||
)
|
||||
error_msg = str(exc_info.value)
|
||||
assert "session_secret" in error_msg
|
||||
assert "32" in error_msg or "at least" in error_msg
|
||||
|
||||
|
||||
def test_session_secret_accepts_32_characters() -> None:
|
||||
"""session_secret with exactly 32 characters is accepted."""
|
||||
secret_32 = "a" * 32 # Exactly 32 characters
|
||||
settings = Settings(
|
||||
database_path="/tmp/test.db",
|
||||
fail2ban_socket="/tmp/fake_fail2ban.sock",
|
||||
fail2ban_config_dir="/tmp/fail2ban",
|
||||
session_secret=secret_32,
|
||||
)
|
||||
assert settings.session_secret == secret_32
|
||||
|
||||
|
||||
def test_session_secret_accepts_longer_than_32() -> None:
|
||||
"""session_secret with more than 32 characters is accepted."""
|
||||
secret_64 = "b" * 64 # 64 characters
|
||||
settings = Settings(
|
||||
database_path="/tmp/test.db",
|
||||
fail2ban_socket="/tmp/fake_fail2ban.sock",
|
||||
fail2ban_config_dir="/tmp/fail2ban",
|
||||
session_secret=secret_64,
|
||||
)
|
||||
assert settings.session_secret == secret_64
|
||||
|
||||
|
||||
def test_session_secret_error_message_includes_guidance() -> None:
|
||||
"""Validation error for short session_secret includes secret generation guidance."""
|
||||
with pytest.raises(ValidationError) as exc_info:
|
||||
Settings(
|
||||
database_path="/tmp/test.db",
|
||||
fail2ban_socket="/tmp/fake_fail2ban.sock",
|
||||
fail2ban_config_dir="/tmp/fail2ban",
|
||||
session_secret="too-short",
|
||||
)
|
||||
error_msg = str(exc_info.value)
|
||||
# Verify the error mentions the constraint
|
||||
assert "session_secret" in error_msg
|
||||
|
||||
Reference in New Issue
Block a user