Skip to content

Commit 57c75a7

Browse files
authored
全量修复/补测试+UED改进 (#535)
* fix: constant-time comparison for download tokens (security item 5) Replace the set-membership key check in /share/download with hmac.compare_digest over the two valid window tokens; add negative-path tests (wrong key 403, both windows accepted, foreign-code token 403). * fix: single source for attachment headers + guard tests (security item 2) All six Content-Disposition construction sites across the five storage backends now go through build_attachment_headers; the header is what neutralizes stored XSS on the same-origin download path. Guard tests assert every get_file_response uses the builder and no hand-built disposition reappears. * fix: reject internal-network endpoints for storage config (security item 7) s3_endpoint_url/s3_hostname/webdav_url are fetched server-side; with APP_ENV=production the write entry now enforces http(s) schemes and a loopback/private/link-local/metadata host blacklist. Development env stays open for local minio/webdav. Already-stored values are never re-validated, so existing deployments are unaffected. * fix: JSON 404 for handler-raised 404s; negative-path test batch Starlette status-code handlers take precedence over the HTTPException class handler, so registering the theme index as the 404 handler turned every HTTPException(404) app-wide into a 200 HTML page (found by the new negative-path tests). The new not_found_handler serves the theme page only to browsers (Accept: text/html) and JSON 404 to API clients. Negative-path batch: download count exhaustion, missing chunk session 404s, expired presign session deletion, admin update_file uniqueness/ existence. * fix: run container as non-root app user with compat volume chown (security item 3) Entrypoint starts as root only to fix the data-dir ownership (skipped when nothing needs changing or when the deployer sets an explicit user), then drops to uid 10001 via gosu. Verified in-container: uvicorn runs as app (uid 10001), setup and share round-trip work against a root-owned volume. * fix: compare download tokens as bytes (non-ASCII key caused 500) * test: lock changed-only semantics for endpoint config saves Integration regression for the background lesson: a stored internal endpoint must not block unrelated settings saves in production mode; changing to an internal endpoint (URL or bare-hostname form) is 400. * fix: resolve-based SSRF check — deny IP shorthands, hex/decimal IPs, DNS maps Probe found the static blacklist bypassed by glibc shorthands (127.1, 10.1), decimal (2130706433) and hex IP forms, *.localhost, and loopback-mapping DNS services. Both validators now resolve the host and re-check every resolved address; hostname tier extracts the host part correctly for [v6]/host:port/bare-v6 forms. 44 tests. * test: pin 404 handler dual-branch behavior (browser theme page vs API JSON) * fix: write back normalized endpoint values; hostname dev-gate test * test: header-injection negative paths for attachment disposition * fix: S3 merge buffers parts to >=5MiB; missing object 404 upfront - S3 multipart rejects parts <5MiB (EntityTooSmall) except the last; chunk size is client-controlled (commonly 2-4MB) so multi-chunk merges always failed with 500. Merge now buffers chunks into >=5MiB parts (memory bound: 5MB + one chunk). - get_file_response: a 404 head_object now raises StorageError(404) upfront instead of signing a doomed presigned URL and failing mid-stream (aligns with local backend semantics from the M2 behavior unification). * test: S3 backend coverage via in-process moto server (13 cases) mock_aws cannot intercept aioboto3's aiohttp stack, so tests run against a real in-process ThreadedMotoServer endpoint. Covers roundtrips, missing object 404, merge failure modes (missing chunk / hash mismatch -> abort, no leftover object), cleanup scoping, presign shapes, proxy dispatch. Also adds moto[s3,server] to the dev group and CI. * fix: WebDAV missing object 404 upfront; connection errors map to 503 HEAD 404 was silently swallowed (signed a 200 bad stream that failed mid-download, same family as the S3 bug), and HEAD connection errors were also swallowed so the 503 mapping never fired. Both now surface properly; the aiohttp session is reclaimed on pre-stream exception paths. * test: WebDAV backend coverage via in-process WebDAV server (9 cases) Real HTTP (HEAD/GET/PUT/DELETE/MKCOL/PROPFIND) against a minimal aiohttp WebDAV server over a temp dir: roundtrips, missing object 404, connection error 503, delete with empty-parent cleanup, merge failure modes, chunk cleanup scoping. * fix: OneDrive missing object maps to 404 upfront; OpenDAL behavior pinned OneDrive get_file_response translated graph itemNotFound into the outer 503 catch-all; now raises StorageError(404) like local/S3/WebDAV. OpenDAL already mapped missing objects to 404 via its outer catch — pinned with fake-operator tests (SDK not in runtime deps, instances built via __new__). Also tightens the S3 presign URL assertion. * test: admin write-path coverage (batch/single delete, batch update, policy actions) 13 negative-path cases for the previously untested data-modifying admin endpoints: mixed-id aggregation with duplicates and missing records, empty-list rejections, clear_expired_at permanence semantics, policy action boundaries (zero limit 400, unknown action 400, missing 404), and the DoesNotExist->404 mapping pinned for single delete. * test: admin read-path and view-preset CRUD coverage (22 cases) Detail/metadata/preview/admin-download/activities/local-lists+delete/ verify: missing-record 404s, note/tag truncation limits, preview max_chars truncation, activity filtering and the 80-event clamp, local path traversal rejection. Presets: update-vs-create by id, name truncation, filter normalization clamps, 24-preset cap, delete-missing 404. * refactor: move auth primitives to apps/base/auth, kill base->admin dep JWT create/verify, bearer extraction and the share-upload gate move to apps.base.auth (consumed by both surfaces); admin.dependencies keeps admin session gating and service providers, re-exporting the primitives for import-path compatibility. base.views no longer imports from apps.admin — the last cross-app reverse dependency is gone. * refactor: split admin services god-module (ConfigService/LocalFile out) apps/admin/services.py (1697 lines) becomes FileService (~1200 lines) + a compatibility facade re-exporting ConfigService, LocalFileService, LocalFileClass and keyvalue_write_lock from their new homes (config_service.py holds the D5 KeyValue lock; one-way imports, no cycle). test_admin_security patch targets follow ConfigService to its new module. Behavior unchanged; 247 tests green. * fix: size-less uploads no longer crash (upstream seek bug) validate_file_size's size-None branch called UploadFile.seek(0, 2), which raises TypeError (UploadFile.seek takes one arg); the underlying SpooledTemporaryFile.seek is sync, so awaiting it also fails. Use the underlying file object with sync seek(0, SEEK_END). Surface discovered by the mypy pilot run; regression test covers the size-None branch. * chore: bump direct deps (patch/minor) fastapi 0.139.2->0.141.1, pydantic 2.12.5->2.13.5, uvicorn 0.51.0->0.53.0, aiohttp 3.14.2->3.14.3; lockfile regenerated. tortoise-orm 0.x->1.x major deliberately deferred pending API-change review. * ci: gradual mypy adoption via baseline ratchet scripts/mypy_ratchet.py compares mypy output against scripts/mypy-baseline.txt (line numbers stripped for edit stability): new type errors fail CI, fixes are folded back via --regenerate. Baseline starts at 30 known errors; the seek bug found in the pilot was fixed before baselining. mypy joins the dev group and CI. * fix: migrate WebDAV auth off aiohttp.BasicAuth (deprecated in 4.0) Dependency-upgrade probe on aiohttp 3.14.3 surfaced the BasicAuth deprecation; switch to aiohttp.encode_basic_auth() headers so the aiohttp 4.0 upgrade does not break the WebDAV backend. * chore: drop dead code (WebDAV _instance stub, unused LocalFileClass.write) * test: file-validation negative paths (13 cases) Magic-bytes spoofing (text-as-png, exe-as-pdf, mp4/webp box detection), whitelist rule semantics (star/image-wildcard/content mismatch), chunk-0 header validation parity, and end-to-end UploadFile rejection. * test: migration runner coverage (5 cases) Full chain executes 001-007 in filename order and registers; rerun is idempotent; pre-registered entries are skipped (DDL not re-executed); the resulting schema matches deployment; a failing migration propagates and is not registered. Uses a fresh :memory: DB without generate_schemas so the migrations themselves build the schema (the real deployment path). * refactor: split core/storage.py into a package (per-backend modules) core/storage.py (1400+ lines, six backends) becomes core/storage/ with _base.py (data contracts, interface, shared header builder), and local/s3/onedrive/opendal/webdav modules. The package __init__ re-exports the full historical public surface; test patch targets move to the precise backend modules (core.storage.local.data_root etc.) and the attachment guard scans the package. Import-only change; 264 tests green. Storage-layer DI deferred: current tests construct instances against patched settings, so the added surface isn't justified yet. * refactor: TypedDict for IPRateLimit records The rate-limit ledger mixed int and datetime under a loose Union typing, producing 5 mypy noise errors. A TypedDict gives precise per-key types; setdefault replaces the get-then-assign pattern. One mypy baseline error family resolved; 264 tests green. * chore: tighten mypy baseline after IPRateLimit TypedDict * refactor: resolve all 26 baseline mypy errors; precise test assertions Per-error treatment (no bulk script): honest annotations for StoredDownload (Path/bytes/Callable), KeyValue.value (Any for JSONField), dict[str, Any] for form/config dicts; signature split in create_token; str() normalization in config int(); platform/type-ignore only where the checker cannot follow (msvcrt, FastAPI Request injection). Baseline file and ratchet compare now redundant at zero errors but kept as the guard against regressions. Tests: assertions tightened to status_code == 403 with HTTPException raises; _create_share explicit parameters replace **extra passthrough; seek-fix pointer-reset assertion persisted.
1 parent 82555c0 commit 57c75a7

48 files changed

Lines changed: 4562 additions & 2089 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.github/workflows/ci.yml‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,15 @@ jobs:
3535
# --require-hashes so CI fails on lockfile drift, exactly like the
3636
# Docker build does.
3737
pip install --require-hashes -r requirements.lock.txt
38-
pip install pytest pytest-asyncio httpx
38+
pip install pytest pytest-asyncio httpx 'moto[s3,server]' mypy
3939
4040
- name: Ruff
4141
run: pipx run ruff==0.16.6 check .
4242

43+
- name: Mypy ratchet (new type errors are rejected; fix and run
44+
scripts/mypy_ratchet.py --regenerate to tighten the baseline)
45+
run: python scripts/mypy_ratchet.py
46+
4347
- name: Verify lockfile matches requirements.txt
4448
# Dependabot bumps requirements.txt but cannot regenerate the hashed
4549
# lockfile; without this check a stale lockfile would silently keep

‎Dockerfile‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,17 @@ COPY --from=frontend-builder /build/fronted-2023/dist ./themes/2023
5959

6060
# 安装系统安全更新 + Python 依赖
6161
# 依赖从带哈希的锁定文件安装(--require-hashes),保证构建可复现、防供应链篡改。
62-
# 清理 apt 缓存,降低镜像噪音与扫描面
62+
# gosu 用于入口脚本的数据卷属主修正后降权;清理 apt 缓存,降低镜像噪音与扫描面
6363
RUN apt-get update \
6464
&& apt-get upgrade -y --no-install-recommends \
65+
&& apt-get install -y --no-install-recommends gosu \
6566
&& rm -rf /var/lib/apt/lists/* \
6667
&& pip install --no-cache-dir --require-hashes -r requirements.lock.txt \
6768
&& pip cache purge || true
6869

70+
# 非 root 运行用户;数据卷属主由 docker-entrypoint.sh 按需修正(兼容存量 root 卷)
71+
RUN useradd --system --uid 10001 --home-dir /app app
72+
6973
# 环境变量配置
7074
ENV HOST="0.0.0.0" \
7175
PORT=12345 \
@@ -77,6 +81,10 @@ ENV HOST="0.0.0.0" \
7781

7882
EXPOSE 12345
7983

84+
COPY docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh
85+
RUN chmod +x /usr/local/bin/docker-entrypoint.sh
86+
ENTRYPOINT ["/usr/local/bin/docker-entrypoint.sh"]
87+
8088
# 生产环境启动命令
8189
# FORWARDED_ALLOW_IPS 默认为空:仅信任直连 IP,避免任意客户端伪造 X-Forwarded-*。
8290
# 若前面有反向代理,请显式设置为代理网段,例如 "10.0.0.0/8,172.16.0.0/12"。

‎apps/admin/config_service.py‎

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
"""System config write path (ConfigService) + the KeyValue JSON lock.
2+
3+
keyvalue_write_lock lives here because ConfigService.update_config is its
4+
primary consumer; FileService imports it (one-way, no cycle).
5+
"""
6+
import asyncio
7+
8+
from fastapi import HTTPException
9+
10+
from core.settings import (
11+
ADMIN_SESSION_EXPIRE_MAX,
12+
ADMIN_SESSION_EXPIRE_MIN,
13+
settings,
14+
)
15+
from apps.base.config import refresh_settings
16+
from core.security import (
17+
INTERNAL_CONFIG_KEYS,
18+
OUTBOUND_ENDPOINT_CONFIG_KEYS,
19+
generate_jwt_secret,
20+
validate_outbound_endpoint,
21+
validate_outbound_hostname,
22+
)
23+
from apps.base.models import KeyValue
24+
from core.utils import hash_password, is_password_hashed, validate_background_url
25+
26+
# KeyValue 里的 settings/activities/presets 都是整块 JSON 读-改-写;
27+
# 进程内写锁串行化这三个写路径,避免并发管理操作互相覆盖(last-writer-wins)。
28+
# 多进程部署下锁不跨进程——文档已锁定单 worker 部署。
29+
keyvalue_write_lock = asyncio.Lock()
30+
31+
32+
class ConfigService:
33+
INT_FIELDS = {
34+
"admin_session_expire",
35+
"enable_chunk",
36+
"error_count",
37+
"error_minute",
38+
"login_count",
39+
"login_minute",
40+
"max_save_seconds",
41+
"onedrive_proxy",
42+
"open_upload",
43+
"port",
44+
"s3_proxy",
45+
"server_port",
46+
"server_workers",
47+
"show_admin_addr",
48+
"storage_limit",
49+
"upload_count",
50+
"upload_minute",
51+
"upload_size",
52+
"webdav_proxy",
53+
}
54+
FLOAT_FIELDS = {"opacity"}
55+
56+
def get_config(self):
57+
config = dict(settings.items())
58+
config["admin_token"] = ""
59+
for key in INTERNAL_CONFIG_KEYS:
60+
config.pop(key, None)
61+
return config
62+
63+
async def update_config(self, data: dict):
64+
current_config = dict(settings.items())
65+
next_config = dict(current_config)
66+
update_data = {
67+
key: value
68+
for key, value in data.items()
69+
if key in settings.default_config and key not in INTERNAL_CONFIG_KEYS
70+
}
71+
72+
admin_token = update_data.get("admin_token")
73+
admin_password_changed = False
74+
if admin_token is None or admin_token == "":
75+
update_data.pop("admin_token", None)
76+
elif not is_password_hashed(admin_token):
77+
update_data["admin_token"] = hash_password(admin_token)
78+
admin_password_changed = True
79+
else:
80+
admin_password_changed = True
81+
82+
for key, value in update_data.items():
83+
if value == "" and key in self.INT_FIELDS | self.FLOAT_FIELDS:
84+
continue
85+
86+
try:
87+
if key in self.INT_FIELDS:
88+
next_config[key] = int(value)
89+
elif key in self.FLOAT_FIELDS:
90+
next_config[key] = float(value)
91+
else:
92+
next_config[key] = value
93+
except (TypeError, ValueError):
94+
raise HTTPException(status_code=400, detail=f"{key} 配置值格式错误")
95+
96+
try:
97+
session_expire = int(str(next_config.get("admin_session_expire")))
98+
except (TypeError, ValueError):
99+
raise HTTPException(
100+
status_code=400,
101+
detail="admin_session_expire 配置值格式错误",
102+
)
103+
if (
104+
not ADMIN_SESSION_EXPIRE_MIN <= session_expire <= ADMIN_SESSION_EXPIRE_MAX
105+
or session_expire % ADMIN_SESSION_EXPIRE_MIN != 0
106+
):
107+
raise HTTPException(
108+
status_code=400,
109+
detail="admin_session_expire 必须是 1 到 365 个整天",
110+
)
111+
next_config["admin_session_expire"] = session_expire
112+
113+
if int(next_config.get("storage_limit", 0)) < 0:
114+
raise HTTPException(
115+
status_code=400,
116+
detail="storage_limit 不能小于 0",
117+
)
118+
119+
# 只校验"发生变化"的值:升级前存入的旧格式 background(相对路径、含空格
120+
# 或括号)在旧版本是合法的,若每次保存都重新校验,存量部署会连无关设置项
121+
# 都保存不了(一律 400)。渲染侧仍然 html 转义,而任何修改都必须通过校验。
122+
current_background = str(settings.background or "")
123+
candidate_background = str(next_config.get("background") or "")
124+
if candidate_background != current_background:
125+
try:
126+
validate_background_url(candidate_background)
127+
except ValueError as exc:
128+
raise HTTPException(status_code=400, detail=str(exc))
129+
130+
# 只校验"发生变化"的值:企业内网 minio/webdav 是正当场景,存量部署历史
131+
# 合法写入的内网 endpoint 若每次保存都重新校验,会连无关设置都保存不了
132+
# (background 曾有同款回归,上游 #528 修复过——本处沿用同一语义)。
133+
# s3_hostname 是裸主机名(存储层按 https://{hostname} 拼接),单独分档校验。
134+
for endpoint_key in OUTBOUND_ENDPOINT_CONFIG_KEYS:
135+
if endpoint_key not in next_config:
136+
continue
137+
candidate = str(next_config[endpoint_key] or "")
138+
current = str(getattr(settings, endpoint_key, "") or "")
139+
if candidate == current:
140+
continue
141+
validator = (
142+
validate_outbound_hostname
143+
if endpoint_key == "s3_hostname"
144+
else validate_outbound_endpoint
145+
)
146+
try:
147+
# 写回规范化值(validator 去除首尾空白):校验通过但入库脏值
148+
# 会让存储层在连接期才报错,应在校验点归一。
149+
next_config[endpoint_key] = validator(candidate)
150+
except ValueError as exc:
151+
raise HTTPException(status_code=400, detail=str(exc))
152+
153+
if admin_password_changed:
154+
next_config["jwt_secret"] = generate_jwt_secret()
155+
156+
async with keyvalue_write_lock:
157+
await KeyValue.update_or_create(key="settings", defaults={"value": next_config})
158+
await refresh_settings(force=True)
159+

0 commit comments

Comments
 (0)