Skip to content

Commit eb2db8f

Browse files
committed
fix: don't let a legacy background value block settings saves
#514 began validating `background` as an http(s) URL against the *merged* config, so a value stored by an older release (a relative path, or one containing a space/parenthesis — all legal before) made every subsequent settings save return 400, including saves that never touched `background`. Existing deployments could not change any setting at all. Validate only values that actually change: an untouched legacy value is kept as-is (it is still html-escaped on render), while any edit must pass validation. Regression tests cover both directions and fail without this change. Also treat OverflowError from hashlib.scrypt as a non-matching hash so a hand-crafted n/r/p cannot disturb the login path on runtimes that raise it.
1 parent 8215f91 commit eb2db8f

4 files changed

Lines changed: 76 additions & 5 deletions

File tree

‎apps/admin/services.py‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1567,10 +1567,16 @@ async def update_config(self, data: dict):
15671567
detail="storage_limit 不能小于 0",
15681568
)
15691569

1570-
try:
1571-
validate_background_url(next_config.get("background", ""))
1572-
except ValueError as exc:
1573-
raise HTTPException(status_code=400, detail=str(exc))
1570+
# 只校验"发生变化"的值:升级前存入的旧格式 background(相对路径、含空格
1571+
# 或括号)在旧版本是合法的,若每次保存都重新校验,存量部署会连无关设置项
1572+
# 都保存不了(一律 400)。渲染侧仍然 html 转义,而任何修改都必须通过校验。
1573+
current_background = str(settings.background or "")
1574+
candidate_background = str(next_config.get("background") or "")
1575+
if candidate_background != current_background:
1576+
try:
1577+
validate_background_url(candidate_background)
1578+
except ValueError as exc:
1579+
raise HTTPException(status_code=400, detail=str(exc))
15741580

15751581
if admin_password_changed:
15761582
next_config["jwt_secret"] = generate_jwt_secret()

‎core/utils.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,9 @@ def verify_password(password: str, hashed: str) -> bool:
174174
p=int(p),
175175
maxmem=_SCRYPT_MAXMEM,
176176
).hex()
177-
except (ValueError, TypeError):
177+
# 默认解释器对超范围的 n/r/p 抛 TypeError/ValueError;部分构建会抛
178+
# OverflowError,一并视为校验失败,避免坏掉的存量哈希影响登录路径。
179+
except (ValueError, TypeError, OverflowError):
178180
return False
179181
return hmac.compare_digest(password_hash, stored_hash)
180182

‎tests/test_background_url_validation.py‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,5 +73,63 @@ async def _scenario(self):
7373
await close_db()
7474

7575

76+
class LegacyBackgroundUpgradeTests(SettingsOverrideMixin, unittest.TestCase):
77+
"""存量部署升级:旧版本合法、新校验不接受的值,不得阻塞设置保存。"""
78+
79+
def test_unchanged_legacy_background_does_not_block_unrelated_save(self):
80+
asyncio.run(self._unchanged_legacy_allows_save())
81+
82+
async def _unchanged_legacy_allows_save(self):
83+
await init_memory_db()
84+
try:
85+
from apps.base.config import refresh_settings
86+
from apps.base.models import KeyValue
87+
88+
# 旧版本合法(相对路径 + 空格),新的 http(s) 校验会拒绝
89+
legacy = "/static/bg image.png"
90+
await KeyValue.create(
91+
key="settings", value={"background": legacy, "name": "old"}
92+
)
93+
await refresh_settings(force=True)
94+
95+
service = ConfigService()
96+
# 修复前这里会 400,导致存量部署连无关设置项都保存不了
97+
await service.update_config({"name": "new"})
98+
99+
record = await KeyValue.filter(key="settings").first()
100+
self.assertEqual(record.value.get("background"), legacy, "旧值应原样保留")
101+
self.assertEqual(record.value.get("name"), "new")
102+
finally:
103+
await close_db()
104+
105+
def test_changing_background_is_still_validated(self):
106+
asyncio.run(self._change_still_validated())
107+
108+
async def _change_still_validated(self):
109+
await init_memory_db()
110+
try:
111+
from fastapi import HTTPException
112+
113+
from apps.base.config import refresh_settings
114+
from apps.base.models import KeyValue
115+
116+
await KeyValue.create(key="settings", value={"background": "/legacy/bg.png"})
117+
await refresh_settings(force=True)
118+
119+
service = ConfigService()
120+
# 修改为恶意值仍然被拒
121+
with self.assertRaises(HTTPException) as ctx:
122+
await service.update_config(
123+
{"background": "x') ;background:url(https://evil.com/)"}
124+
)
125+
self.assertEqual(ctx.exception.status_code, 400)
126+
# 修改为合法值可以通过
127+
await service.update_config({"background": "https://ok.example/bg.png"})
128+
record = await KeyValue.filter(key="settings").first()
129+
self.assertEqual(record.value.get("background"), "https://ok.example/bg.png")
130+
finally:
131+
await close_db()
132+
133+
76134
if __name__ == "__main__":
77135
unittest.main()

‎tests/test_password_hashing.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ def test_malformed_hashes_are_rejected_not_crashing(self):
5353
self.assertFalse(verify_password("x", "scrypt$n$r$p$salt$zzz"))
5454
self.assertFalse(verify_password("x", "sha256$only-two"))
5555
self.assertFalse(verify_password("x", ""))
56+
# 超范围的 n/r/p:解释器抛 TypeError/ValueError(部分构建抛 OverflowError),
57+
# 都必须被吞掉并判为不匹配,而不是让登录及每个请求 500
58+
self.assertFalse(
59+
verify_password("x", "scrypt$99999999999999999999999999$8$1$aa$bb")
60+
)
5661

5762

5863
class TransparentRehashTests(SettingsOverrideMixin, unittest.TestCase):

0 commit comments

Comments
 (0)