全量修复/补测试+UED改进 - #535
Merged
Merged
全量修复/补测试+UED改进#535
Conversation
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).
…m 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.
…tem 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.
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.
…urity 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.
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.
…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.
- 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).
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.
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.
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.
…nned 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.
…olicy 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.
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.
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.
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.
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.
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.
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.
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.
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.
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).
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.
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.
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.
|
@Marrrrrrrrry is attempting to deploy a commit to the vastsa's projects Team on Vercel. A member of the Team first needs to authorize it. |
Owner
|
感谢 pr，我 review 一下 |
Contributor
Author
|
1.下载 token 常量时间比较 /share/download 的 key 校验原为 set 成员判断，改为对当前与上一时间窗 token 2.attachment 响应头单一来源 Content-Disposition: attachment 是同源下载路径上防存储型 XSS 的关键， 3.JSON 404 修复 + 非 root 容器（可拆两个）
4.存储 endpoint 写入口校验 s3_endpoint_url / s3_hostname / webdav_url 会被服务端直接访问，被劫持的 5.S3 后端两个修复 + 测试
6.WebDAV 修复 + 测试 与 S3 同款的"缺失对象 200 坏流"之外，还发现 HEAD 连接错误被内层 7.OneDrive 404 前置 + OpenDAL 行为迭代 OneDrive 缺失对象经外层兜底返回 503，改为前置 404，与 local/S3/WebDAV |
Contributor
Author
|
前端稍等我一下 我前端因为在写新2026的样式 然后有点乱 因为UED改进涉及前端 让我整理一下再PR掉 |
Contributor
Author
|
前端推了 |
Contributor
Author
|
dependabot 这个批量pr 我这次pr前手工处理依赖升级了 流水线那块不一定是最佳实践（我好像还是忘了做bot报告合并）你正好修掉吧 然后那个收件箱那个不然我就先不做？不然容易撞 我本地有个分支已经做一半了 我开发思路写在那个PR里面了 |
vastsa
requested changes
Sep 19, 2026
vastsa
left a comment
vastsa
left a comment
Owner
There was a problem hiding this comment.
先说结论：先别合。 CI 是红的，还有一处可复现的 SSRF 绕过、一处 Docker 升级会炸。方向是对的，拆包/测试/安全加固都有价值，修完这几条再合。
我这边对照 master（2.7.0）看过 diff，本地跑了测试，也核对了 GitHub Actions。
必须改
1. CI 挂了：test_browser_accept_gets_theme_page
not_found_handler 对 Accept: text/html 直接 await index()，而 themes/ 在 .gitignore 里（只有 Docker 构建才有）。仓库 checkout / CI 都没有 index.html，于是再抛一次 HTTPException(404)。
Actions：1 failed, 263 passed，就是这条。测试里造一个假主题文件，或者 theme 缺失时回落 JSON 404，别再往上抛。
2. s3_hostname 的 SSRF 校验能被 @ 绕过（已复现）
validate_outbound_hostname("evil.com@127.0.0.1") → 放行
core/security.py 手写拆 host（只挡 :// 和 /），evil.com@127.0.0.1 当主机名 → getaddrinfo 失败 → fail-open。存储层拼的是 https://{s3_hostname}，urlsplit 会把 @ 前当 userinfo，实际连 127.0.0.1。
URL 档用了 urlsplit().hostname，不受影响。需要 admin token。改成拒绝 @ / userinfo，解析失败改为拒绝，不要当放行。
3. 非 root 入口会让 NAS / 只读卷起不来
docker-entrypoint.sh：set -e + 发现非 uid 10001 就 chown -R app:app /app/data。文档和 compose 注释里的 /app/data/local:ro、NFS root_squash 都会 EROFS → 容器起不来。changelog 里我们自己写过「为了卷兼容才用 root 跑」，这次等于又改回去，NAS 场景没覆盖到。
chown 失败应降级告警，只读挂载跳过。
升级兼容（给合入后的预期）
- 配置键、SQLite 表、提取码/JWT/下载 token 算法都没变，源码部署可以当 drop-in。
- 已写入的内网 MinIO/WebDAV 不会被新 SSRF 校验误杀（只校验有改动的字段），老用户保存其它设置不受影响。
- 分享下载缺文件仍是 HTTP 200 +
{"code":404}，前端这条契约没动。 - 默认 Docker named volume 能升（启动时 chown 成 10001）。挂了 NAS 只读目录或 bind mount 的先别升，等第 3 条修完。
- API 行为变化：未知路径 /
HTTPException(404)对 JSON 请求现在是真 404（以前一律 200 HTML）。分享流不受影响；curl/监控如果靠「乱打路径返回 200」会红。这是修复，但要心里有数。
建议改（不挡这次，别装没看见）
- 解析失败 fail-open + 只在写入时解析：DNS rebinding / Clash fake-ip（
198.18.0.0/15被is_private判死）下，生产会拒掉所有公网 S3。至少文档写清楚。 - WebDAV 默认跟随 3xx：公网 302 到
169.254.169.254仍会打到 metadata。 test_unknown_extension_with_known_signature_still_checked只有 docstring，空测试。- 附件头测试只断言
startswith("attachment;")，把quote()拿掉也绿。 conftest.py的rmtree(data/share)是老问题，这次放大了爆炸半径——别在有真实数据的 checkout 上跑 pytest。- PR 描述里的「游客上传提前拒绝」这 PR 没动前端，后端
share_required_login只是搬家。
没问题的部分
- storage 拆包：类名/方法签名与旧
core/storage.py一致，from core.storage import X还在。 - 认证搬到
apps.base.auth：权限语义没变，base→admin 依赖断了，旧 import 有 re-export。 - 下载 token 改
hmac.compare_digest(bytes)：非 ASCII 不再 500，调用点只有这一处。 - 五个后端都走
build_attachment_headers。 - S3 multipart ≥5MiB 缓冲、无 size 上传改
file.file.seek：对症。 - 测试主体（migration runner、moto、admin 写路径、token 边界）能当回归网。
先把 CI 那条 404、s3_hostname 的 @ 绕过、entrypoint 的 chown 修了。其余可以 follow-up。31 个 commit 塞了安全/重构/测试/Docker/升依赖，下次拆开更好审。
Contributor
Author
|
那我处理吗？还是直接你处理 我弄也行但是跟我讲一下 别等下撞了 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
改进了游客上传被拒的困扰（拒绝提前到选文件前）优化交互体验
然后拆了一些耦合和补测试（现在项目应该没有硬伤了）应该没有引入回归 我review了提交
但是依然可能有疏忽 虽然我本地构建了也跑了一些基本测试 但是还是建议你那也跑一下 尤其是老用户升级 我没这环境（我生产服务器性能一般）
以下是详细报告