Skip to content

Commit e185f60

Browse files
committed
fix(settings): resolve issue #76 — unable to save settings (HTTP 500)
Root cause: masked api_bearer_token ('********') from GET was sent back in POST, overwriting the real token. Validation then failed on the 8-char masked value (min 32 chars required). Fix: - routes.py: strip masked/empty api_bearer_token before merge - settings.py: defense-in-depth guard for masked/empty tokens Closes #76 Bump version to 2.1.0
1 parent b1ce64c commit e185f60

File tree

7 files changed

+180
-19
lines changed

7 files changed

+180
-19
lines changed

RELEASE_NOTES.md

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,36 @@
1+
# Release v2.1.0
2+
3+
## Bug Fixes
4+
5+
### Issue #76 — Unable to save settings (HTTP 500)
6+
7+
**Root cause:** The web UI GET endpoint masks `api_bearer_token` as `'********'`
8+
before returning settings. When the user saves settings, this 8-character masked
9+
value is sent back in the POST payload and overwrites the real token during the
10+
settings merge. `save_settings()` then validates the masked string against a
11+
32-character minimum length requirement, causing the validation to fail and
12+
returning HTTP 500.
13+
14+
**Fix (defense-in-depth, two layers):**
15+
- **`modules/web/routes.py`**: Strip masked or empty `api_bearer_token` from
16+
incoming POST data before merging with current settings, preserving the real
17+
token already stored on disk.
18+
- **`modules/core/settings.py`**: Safety net in `save_settings()` — if the
19+
token is empty or the `'********'` placeholder, remove it from the dict
20+
instead of failing validation. The real token on disk remains untouched.
21+
22+
**Files changed:** `modules/web/routes.py`, `modules/core/settings.py`
23+
24+
## Test Suite
25+
26+
- 4 new unit tests in `tests/test_issue76_masked_token.py`:
27+
- Masked token (`'********'`) → save succeeds, placeholder NOT persisted
28+
- Empty token → save succeeds
29+
- Valid token → validated and persisted correctly
30+
- Invalid short token → still properly rejected
31+
32+
---
33+
134
# Release v2.0.3
235

336
## Bug Fixes

app.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
CertMate - Modular SSL Certificate Management Application
33
Main application entry point with modular architecture
44
"""
5-
__version__ = '2.0.4'
5+
__version__ = '2.1.0'
66
import os
77
import sys
88
import tempfile

modules/core/file_operations.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ def create_unified_backup(self, settings_data, backup_reason="manual"):
140140
"backup_id": backup_id,
141141
"timestamp": datetime.now().isoformat(),
142142
"backup_reason": backup_reason,
143-
"version": "2.0.4", # New unified format
143+
"version": "2.1.0", # New unified format
144144
"type": "unified",
145145
"domains": domains,
146146
"settings_domains": [d.get('domain') if isinstance(d, dict) else d for d in settings_data.get('domains', [])],

modules/core/settings.py

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -241,21 +241,29 @@ def save_settings(self, settings, backup_reason="auto_save"):
241241
settings['email'] = email_or_error
242242

243243
if 'api_bearer_token' in settings:
244-
# Use compatibility layer for validation
245-
try:
246-
import app
247-
if hasattr(app, 'validate_api_token'):
248-
is_valid, token_or_error = app.validate_api_token(settings['api_bearer_token'])
249-
else:
244+
token = settings['api_bearer_token']
245+
# Skip validation for masked/placeholder tokens — the real
246+
# token is preserved in the file; callers should strip these
247+
# before calling save_settings, but this is a safety net.
248+
if not token or token == '********':
249+
settings.pop('api_bearer_token')
250+
logger.info("Stripped masked/empty api_bearer_token from settings before save")
251+
else:
252+
# Use compatibility layer for validation
253+
try:
254+
import app
255+
if hasattr(app, 'validate_api_token'):
256+
is_valid, token_or_error = app.validate_api_token(token)
257+
else:
258+
from modules.core.utils import validate_api_token
259+
is_valid, token_or_error = validate_api_token(token)
260+
except ImportError:
250261
from modules.core.utils import validate_api_token
251-
is_valid, token_or_error = validate_api_token(settings['api_bearer_token'])
252-
except ImportError:
253-
from modules.core.utils import validate_api_token
254-
is_valid, token_or_error = validate_api_token(settings['api_bearer_token'])
255-
256-
if not is_valid:
257-
logger.error(f"Invalid API token: {token_or_error}")
258-
return False
262+
is_valid, token_or_error = validate_api_token(token)
263+
264+
if not is_valid:
265+
logger.error(f"Invalid API token: {token_or_error}")
266+
return False
259267

260268
# Validate dns_provider against supported set
261269
supported_providers = {'cloudflare','route53','azure','google','powerdns','digitalocean','linode','gandi','ovh','namecheap','vultr','dnsmadeeasy','nsone','rfc2136','hetzner','porkbun','godaddy','he-ddns','dynudns','arvancloud','infomaniak','acme-dns'}
@@ -594,7 +602,7 @@ def _ensure_certificate_metadata(self):
594602
"domain": domain,
595603
"dns_provider": dns_provider,
596604
"created_at": "unknown",
597-
"version": "2.0.4",
605+
"version": "2.1.0",
598606
"migrated": True
599607
}
600608

modules/web/routes.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -928,6 +928,13 @@ def web_settings():
928928
logger.warning(f"Setup attempt from non-local IP: {client_ip}")
929929
return jsonify({'error': 'Initial setup only allowed from localhost'}), 403
930930

931+
# Strip masked/sentinel api_bearer_token from incoming data.
932+
# The GET endpoint masks it as '********', so if the UI sends
933+
# it back unchanged we must preserve the real token.
934+
incoming_token = new_settings.get('api_bearer_token', '')
935+
if not incoming_token or incoming_token == '********':
936+
new_settings.pop('api_bearer_token', None)
937+
931938
# Merge with existing settings
932939
merged_settings = {**current_settings, **new_settings}
933940

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"private": true,
33
"name": "certmate",
4-
"version": "2.0.4",
4+
"version": "2.1.0",
55
"description": "CertMate SSL Certificate Manager - frontend assets",
66
"scripts": {
77
"css:build": "npx tailwindcss -i static/css/input.css -o static/css/tailwind.min.css --minify",
@@ -10,4 +10,4 @@
1010
"devDependencies": {
1111
"tailwindcss": "^3.4.17"
1212
}
13-
}
13+
}

tests/test_issue76_masked_token.py

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
"""
2+
Regression test for issue #76 — saving settings must not fail when the
3+
masked api_bearer_token placeholder '********' is present.
4+
5+
These are fast unit tests that do NOT require Docker.
6+
"""
7+
8+
import json
9+
from pathlib import Path
10+
from unittest.mock import patch
11+
12+
import pytest
13+
14+
pytestmark = [pytest.mark.unit]
15+
16+
17+
@pytest.fixture
18+
def settings_env(tmp_path):
19+
"""Create a minimal SettingsManager with a temp settings file,
20+
bypassing the compat layer so writes go to the temp directory."""
21+
from modules.core.file_operations import FileOperations
22+
from modules.core.settings import SettingsManager
23+
24+
data_dir = tmp_path / "data"
25+
data_dir.mkdir()
26+
cert_dir = tmp_path / "certificates"
27+
cert_dir.mkdir()
28+
backup_dir = tmp_path / "backups"
29+
backup_dir.mkdir()
30+
logs_dir = tmp_path / "logs"
31+
logs_dir.mkdir()
32+
33+
file_ops = FileOperations(
34+
data_dir=data_dir,
35+
cert_dir=cert_dir,
36+
backup_dir=backup_dir,
37+
logs_dir=logs_dir,
38+
)
39+
settings_file = data_dir / "settings.json"
40+
mgr = SettingsManager(file_ops, settings_file)
41+
42+
# Bypass the compat wrappers so we use our local file_ops directly
43+
mgr._safe_file_write_compat = file_ops.safe_file_write
44+
mgr._safe_file_read_compat = file_ops.safe_file_read
45+
mgr._settings_file_exists_compat = lambda: settings_file.exists()
46+
mgr._save_settings_compat = lambda s, r="auto": mgr.save_settings(s, r)
47+
48+
return mgr, settings_file
49+
50+
51+
def _seed(settings_file):
52+
"""Write a valid baseline settings file and return the dict."""
53+
from modules.core.utils import generate_secure_token
54+
base = {
55+
"email": "user@example.com",
56+
"dns_provider": "cloudflare",
57+
"domains": [],
58+
"auto_renew": True,
59+
"api_bearer_token": generate_secure_token(),
60+
"setup_completed": True,
61+
}
62+
settings_file.write_text(json.dumps(base))
63+
return base
64+
65+
66+
class TestMaskedTokenSave:
67+
"""Issue #76: save_settings must handle masked api_bearer_token."""
68+
69+
def test_save_with_masked_token_succeeds(self, settings_env):
70+
"""Saving settings that contain '********' must not fail."""
71+
mgr, settings_file = settings_env
72+
initial = _seed(settings_file)
73+
74+
updated = dict(initial)
75+
updated["api_bearer_token"] = "********"
76+
result = mgr.save_settings(updated, "test")
77+
assert result is True, "save_settings should succeed with masked token"
78+
79+
saved = json.loads(settings_file.read_text())
80+
assert saved.get("api_bearer_token") != "********", \
81+
"Masked placeholder must not be persisted"
82+
83+
def test_save_with_empty_token_succeeds(self, settings_env):
84+
"""Saving settings with an empty api_bearer_token must not fail."""
85+
mgr, settings_file = settings_env
86+
_seed(settings_file)
87+
88+
updated = json.loads(settings_file.read_text())
89+
updated["api_bearer_token"] = ""
90+
result = mgr.save_settings(updated, "test")
91+
assert result is True, "save_settings should succeed with empty token"
92+
93+
def test_save_with_valid_token_still_validates(self, settings_env):
94+
"""A real, valid token must still pass through validation."""
95+
mgr, settings_file = settings_env
96+
initial = _seed(settings_file)
97+
real_token = initial["api_bearer_token"]
98+
99+
result = mgr.save_settings(dict(initial), "test")
100+
assert result is True
101+
102+
saved = json.loads(settings_file.read_text())
103+
assert saved.get("api_bearer_token") == real_token
104+
105+
def test_save_with_invalid_token_still_rejected(self, settings_env):
106+
"""A truly invalid (short, non-masked) token must still be rejected."""
107+
mgr, settings_file = settings_env
108+
_seed(settings_file)
109+
110+
bad_settings = json.loads(settings_file.read_text())
111+
bad_settings["api_bearer_token"] = "tooshort"
112+
result = mgr.save_settings(bad_settings, "test")
113+
assert result is False, "Short invalid tokens must still be rejected"

0 commit comments

Comments
 (0)