fix(album): address remaining issues #3-#16 - #18
Merged
Merged
Conversation
- 删除 app/loading.tsx 与 components/skeletons/page-skeleton.tsx:根级 loading 会为 整个路由提前 flush 外壳,使 redirect()/notFound() 退化成 200 + 客户端跳转——私有 相册与无效分享 token 不再返回 404、未登录访问 /files 不再返回 307。分段骨架由各页 内层 <Suspense> 负责,根级边界既多余又有害。 - lib/share-auth.ts:Secure 改为按 NEXTAUTH_URL 的协议判定,而非看 NODE_ENV。 next start 在生产模式下 NODE_ENV 恒为 production,用 http 提供服务时浏览器会丢弃 Secure cookie,表现为密码正确却仍停在门后;用 curl 手工回传 cookie 会掩盖这一点。 两处都由 10.7.89.132 上的真机验证发现,并在同一环境验证已修复。
mediaType 与 mimeType 原来由两处独立推导(`file.type.startsWith('image/')`
与 `file.type || (isImage ? 'image/jpeg' : 'video/mp4')`),同一个值被两处
决定就意味着它们可以互相矛盾。现在只有 resolveUploadMedia 产出这两个字段。
关闭 issue #11 / #5 的缺陷 G,但实际后果与报告不同:报告称"缺 MIME 的图片会被
存成 mediaType=video、mimeType=video/mp4",而 persistVideo 的白名单在写库之前
就会拒掉空 type,那条污染路径不可达。真实后果是这类上传被以「仅支持 MP4 /
WebM / MOV 视频」这个误导性 400 拒绝 —— 修复把它变成能传,或在真的判断不出来
时给一个说清问题的 400(AmbiguousMediaError,且不留数据库行)。
不做 magic byte 嗅探:覆盖 MP4/MOV 需要解析偏移 4 处的 ftyp box,代价是要么引
依赖要么手写几十行,而"猜不出"的正确响应是一个诚实的 400,不是约 95% 命中的启发式。
ResolvedMedia.source 保留 'sniffed' 的扩展位。
顺带修掉三处同源的账:
- guessMimeFromFilename 的表里没有任何视频条目(.mp4 → octet-stream),已扩充,
并与 ALLOWED_*_MIME、私有的 getExtensionFromMime 合并成 lib/media-type.ts 里
的两张表(此前仓库有 4 份 mime/扩展名知识,现在 2 张表互为一个方向的视图)。
- persistVideo 在空 type 时两头都推不出扩展名,键名会没有扩展名,任何按扩展名
反推类型的读取永远失败。
- persistImage 原本先发原图上传、再做缩略图解码:一张坏图片会在桶里留下没有
数据库行引用、也没有任何代码能找到的孤儿。解码验证提到写入之前。
新增 35 个表驱动用例(含 1 个针对 G 的回归用例)。route handler 本身在本机测不了
(lib/storage.ts import 了未安装的 server-only),所以上线前需要带桶手工验收。
———— issue 对应 ————
#11 Bug 2 上传接口 MIME 类型判断逻辑错误
#5 Bug 4 上传接口 mimeType 回退逻辑
Fixes #11
Fixes #5
issue #6 的 Bug 3 只列了 7 处缺 .positive() 的 id,实际是 9 处(users/password 的 userId 没人提到,users DELETE 的 id 与 transferToUserId 是两条而不是三条)。现在 统一成 lib/validation.ts 导出的 idSchema / optionalIdSchema,与既有的 visibilitySchema 同处一地。 .max() 挡住的是 2^31-1 到安全整数上限之间那段值:zod v4 的 .int() 要求"安全整数", 所以 1e21 它自己就拒了,但 3000000000 是合法安全整数、却是 MySQL 有符号 INT 溢出, 放行它只是把 400 换成驱动层的 500。 新增 lib/prisma-errors.ts:P2025→404、P2003/P2002→409、其余 500。P2025/P2003 此前在整个仓库里一次都没出现过,所以"格式合法但不存在"的 id 一路冒成裸 500, 而前端一律读 body.error / json.message,500 的响应体里没有这两个键,管理员只会 看到一句通用兜底。categories PUT 与 users PUT/PATCH 因此补上 try/catch。 两处刻意的设计约束,都有测试钉住: - 识别用鸭子类型而非 instanceof,因为本机与 CI 都没有生成的 Prisma 客户端(运行时 是 scripts/create-prisma-stub.cjs,里面没有 PrismaClientKnownRequestError 这个类)。 但 TosServerError 同样带一个字符串 code,所以必须 name 与 P 形码同时成立,否则一次 对象存储故障会被洗成 404/409,把可重试的上游错误报成客户端错误。 - 响应工厂 prismaErrorResponse 把包络差异收成一个人参:相册与用户域是 {error}、 云盘域是 {message},统一它要动 11 个 route 加 6 个组件去关一个不会坏的不一致。 保留现状,但不让这份差异以复制样板的形式长在每个文件里。 categories DELETE 与 users DELETE 的 id 校验与错误处理都不在这里改:它们分别在 缺陷 B 与缺陷 D 的改动里被整体重写,分两次改同几行只会让审查看不出行为差异。 新增 25 个用例(validation 9 + prisma-errors 16)。 ———— issue 对应 ———— #6 Bug 3 多处 Zod schema 的 ID 字段缺少 .positive() (另附 lib/prisma-errors.ts:为缺陷 D/F 把 P2025/P2003 从裸 500 变成 404/409) Fixes #6
关闭 issue #4 Bug 3(删分类不清存储)、#13/#12/#8/#7/#6/#5/#4 反复报的删除顺序、 以及 #4 Bug 3 提到的 allSettled 静默失败。 根因不是"某处忘了加判断",而是删除动作没有一个归属地:同一个"这批行要动哪些对象" 的判断散在三处且各不相同——photos 现场用 mediaType 三元重推、filesets 硬编码 deleteFileAsset、categories 与 users 干脆什么都没做。 分成两层,沿用 lib/access-rules.ts / lib/access.ts 已有的分层理由(判断要能脱离 数据库单测,执行才碰 I/O): - lib/asset-deletion.ts:纯。CleanupUnit 的 kind 与 domain 绑在同一个联合成员上, 于是"云盘文件被路由到 uploads/ 前缀"在这个类型下不可表示。这不是洁癖:TOS 删一个 不存在的键返回 204 而非 NoSuchKey,所以 deleteUploadObject(云盘 filename) 不只是 空操作,它会报告成功、泄漏真正的对象而没有任何调用方能察觉。唯一可靠的防线是让它 写不出来。 - lib/asset-cleanup.ts:TOS 适配 + 必须走数据库的子项枚举。 删除顺序从散文约定变成结构约束:唯一的破坏性入口 deleteAssetsThenRows 只能"对象先、 行后",且没有任何导出函数能单独删行,所以 photos 原来那种"先 deleteMany、再删存储、 失败返回 500"(行已没了才发觉配置不对——这两个事件最坏的一种排列)写不出来。 反过来时的镜像状态(对象没了、行还在)是可追溯且自愈的:行仍带着 filename,重试会 删一个已不存在的键并得到 204。 编排本身以注入执行器的形式留在纯层,因此这四条策略有机器验证,不需要真桶: 空计划直通删行(不预检配置,否则没照片的分类删不掉)/预检失败时一个对象都不发起/ allSettled 结果必须汇总(缺陷 F 的成因就是从不查看)/任何失败都不写行。 部分失败返回 502 + code=storage_cleanup_failed 而不是成功:行还在,客户端若当成功, 用户以为删掉了而对象与行都活着。 unitForPhoto 对未知 mediaType 偏向超集(按图片处理),与旧实现方向相反:多删一个 不存在的缩略图只是 204,漏删真实缩略图是永久且无人可寻的泄漏。 顺带:categories 与 filesets 的 DELETE 补上存在性检查,不存在的 id 从误导性 500 变成 404(用 commit 2 的映射器);filesets 里两个永不成立的 catch 分支(requireAdmin 返回对象、不抛 'Unauthorized'/'Forbidden')随之消失。 route handler 的接线本身在本机测不了(lib/storage.ts import 了未安装的 server-only), 所以 115 个用例覆盖的是判定与编排;桶侧行为需要带真 TOS 手工验收。 ———— issue 对应 ———— #4 Bug 2 删除照片时先删库再删存储 #4 Bug 3 删除分类时未清理对象存储中的图片文件 #13 Bug 2 删除顺序错误导致文件孤儿 #12 Bug 3 DELETE 先删数据库记录再清理存储 #8 Bug 2 先删库记录再删对象存储,数据不一致 #7 Bug 3 删除操作顺序错误 #6 Bug 2 删除顺序 #5 Bug 2 数据库删除先于存储清理 Fixes #4 Fixes #13 Fixes #12 Fixes #8 Fixes #7 Fixes #6 Fixes #5
关闭 issue #10 Bug 2 / #4 Bug 4-5(归属关系无 onDelete)与 #15 / #5 Bug 3(删用户 照片不清存储)。实际后果比任何一份报告都严重:那个接口会**先把照片行销毁**,再在 prisma.user.delete 上撞它从没问过的云盘外键 P2003,返回一个裸 500——用户还在, 照片已经没了,它们的对象永远留在桶里且再无记录可寻。有云盘但零照片的用户则是直接 500、什么都没删。根因是整个分支只由 `_count.photos` 驱动,那个文件从不提 File/FileSet。 不给三个归属关系加 onDelete: Cascade,这与直觉相反但理由是硬的:Prisma 会把 Cascade 下推成真正的 MySQL 外键级联,子行在库内消失、应用永远读不到 filename,于是泄漏变成 永久且不可追溯——那正是缺陷 B 的成因(Photo.category 已有 Cascade)。把同一个坏机制 复制进用户关系,是伪装成修复的倒退。schema 侧只加注释记录不变量,因此 `prisma db push` 应当是无操作。 改成转移-only:删除用户不再销毁任何资产,所以那条"删了行没删对象"的泄漏路径由构造 消失,issue #15 报的那条也随之关闭。File 不区分所在文件集归谁、一律跟着转移,因为 uploaderId 除了"谁传的"还兼着"谁能改/删它"(app/api/files/[id]/route.ts:95,157), 留在一个已不存在的行上就等于那些文件从此只能由管理员处置。 决策做成纯函数 planUserDelete(lib/asset-deletion.ts),于是这批最危险的分支能脱离 数据库被真值表测住,并且**一次返回全部违规**:旧实现逐条揭示,管理员修完它问的那条 才撞上它没问的那条。顺带补上两个从未存在的守卫——不能删自己、不能删唯一的管理员。 契约变更:{id, transferToUserId?, deletePhotos?} → {id, photoDecision?, driveDecision?, transferToUserId?}。deletePhotos 直接移除而不做兼容:唯一调用方在本次一起改,而旧 客户端发来会得到一句说明缺哪个决定的 400,那本身就是正确行为。'delete' 取值现在是 写出来就被拒的占位——将来放开"连资产一并销毁"是在决策表里加规则,销毁路径可直接复用 deleteAssetsThenRowsWith。 UI 按同一取向塌缩:对话框不再有"直接删除/转移"两个单选(后端会拒绝其中一个),改成 目标选择器 + 后果摘要,并把内联在 487 行组件里的 Dialog 抽成 components/admin/user-delete-dialog.tsx(admin-users-tab.tsx 486 → 418 行)。 新增 schema-invariants 测试一类:本环境 Prisma 委托是 [key:string]: any,where 与 _count 的字段名拼错 tsc 和 CI 双盲,所以把 model User 的反向关系名单与 USER_ASSET_COUNT_SELECT 逐字对齐来测,并附解析器自检(找不到就抛)——否则这类测试 会绿着什么都没检查。注意 schema.prisma 工作副本是 CRLF,不归一化则任何 $ 锚定正则 静默失配。146 个用例通过,其中本次新增 31 个。 ———— issue 对应 ———— #15 删除用户照片时未清理对象存储,导致存储泄漏(改为只转移、不销毁资产,泄漏路径由构造消失) #10 Bug 2 Photo/File 与 User 的关系缺少 onDelete #4 Bug 4 Photo.uploader 外键缺少 onDelete 策略 #4 Bug 5 File.uploader 外键缺少 onDelete 策略 #5 Bug 3 删除用户时不清理 TOS 存储文件 Fixes #15 Fixes #10 Fixes #4 Fixes #5
关闭 issue #10 Bug 3 / #9 Bug 3(账号状态接口无鉴权,可枚举用户名与 pending/active/rejected)。 删掉整个 app/api/users/check 而不是给它加防护,因为它唯一的存在价值是重复一条已经 工作的路径:全仓库唯一的调用方是 components/login-form.tsx,它只读 status === 'pending' 一个值来省一次 signIn 往返;而 lib/auth.ts 的 authorize() 在 bcrypt 校验通过之后就会 抛出待审核/已拒绝,NextAuth 把消息原样带回 response.error,注册成功提示则来自 POST /api/users 的响应体,与这个端点无关。删掉它就删掉了整个枚举 oracle,而且是净减 代码。 不加限流:仓库里没有 middleware.ts、没有 429、没有任何 limiter,三条路都不划算—— 进程内 Map 在扩容那天静默失效,DB 计数表在这个只有 db push、没有 sweeper 的项目里是 一笔新账,而限流也只是给 oracle 减速、并不关闭它。 同一 commit 必须一起修的:探测一走,login-form.tsx 里那句 `includes('待审核') || includes('审核')` 就成了唯一的分类路径,而它本来就是错的——「账户已被拒绝,无法登录」 两个特征词都不含,于是密码正确但被拒绝的用户被告知"用户名或密码错误",会一直重试 自己的正确密码。分类逻辑落到 lib/login-feedback.ts 的纯函数并由表驱动测试覆盖。 文案只有一个来源:LOGIN_FEEDBACK_TEXT 同时供 authorize() 抛出与登录页匹配,依赖方向 与 lib/access.ts → access-rules.ts 一致。测试直接读 lib/auth.ts 断言它引用常量、且 不再存在手写文案,所以改一句话不会再静默把某个分支降级成 invalid。 161 个用例通过。注意删路由后 .next/types/ 里的陈旧产物会让 tsc --noEmit 报错, 需 next build 重新生成(CI 是全量构建,不受影响)。 ———— issue 对应 ———— #10 Bug 3 POST /api/users/check 向未认证用户泄露账号状态 #9 Bug 3 用户名状态枚举接口无鉴权 Fixes #10 Fixes #9
两件事,都是收尾而非新设计,合计净减 73 行。
一、把剩下 11 处 `if ('error' in authCheck)` 统一成 `if (!ok)`(本批前五个 commit
又消掉了 2 处,原报告说的 13 处全部落地)。这是 issue #14 Bug 2 / #13 Bug 3 / #9 Bug 2
反复报的那条,但要说清严重程度:**运行时今天完全等价**,因为 AuthOk = {ok, session,
viewer} 确实没有 error 键,所以这是一笔一致性债、不是活 bug。值得做的非表面的理由只有一
个:lib/auth-guards.ts:10 那个联合是按 ok 判别的,`!x.ok` 是 TypeScript 设计上会收窄
的形式,而 `'error' in x` 只是"成功分支永远不会长出一个 error 键"这个巧合——给它加一个
`error?: undefined` 就会静默走错分支。这一步实际是收尾 2630649
「统一认证检查返回格式为 ok/error 结构」,那个 commit 转了 files/filesets/photos/search,
把 categories/users/users/password/profile/upload/share 落在了后面。
本 commit 的行为差异应当为零,故与前面的功能修复分开。
二、删除 GET /api/share(约 88 行,连同 SharedPhoto / ShareLinkWithCategory /
shareAccessSchema 三个只服务它的类型)。它把密码读自 ?password= 查询串,无鉴权也无限流,
一次返回整册照片的元数据与 fileUrl,而仓库里已经没有任何调用方:分享落地页直接查 Prisma
并由 lib/share-auth.ts 的服务端 HMAC cookie 门禁,admin-share-tab 只发 POST 与 DELETE。
兄弟路由 app/api/share/unlock/route.ts:15 的注释早就写明了本项目的政策("密码走请求体
而不是查询串"),所以留着这个 GET 不是保留一个被权衡过的设计,而是同一族重构做了一半。
选择删除而不是"只去掉 ?password=":后者会留下一份没人用的重复实现继续要维护。
风险已知:外部若有集成在用 /api/share?token=… 会开始收到 405。
CLAUDE.md 三处修正:那条 API 清单里的 GET /api/share 已不存在,补上 unlock 与
DELETE /api/users 的真实语义;「No test suite: Manual testing required」从 2630649 起
就是错的(现在 7 个文件 161 个用例);并把这轮摸清的三条约束写进去——route handler
因 server-only 未安装而测不了、Prisma 的 [key: string]: any 让字段名拼错对 tsc 与 CI
双盲、以及 {error}/{message} 两套包络是刻意保留的债及其理由。
———— issue 对应 ————
#14 Bug 2 auth check 模式不一致('error' in authCheck vs !ok)
#13 Bug 3 鉴权判断方式与类型定义不符
#9 Bug 2 认证守卫检查模式不一致
#8 Bug 3 分享链接密码通过 GET query string 明文传输(整个 GET 端点已删)
#16 安全漏洞:已认证用户可绕过 visibility 访问私有内容
——该 issue 的两条(搜索越权、批量下载越权)均由 d7953d9 完成,本批六个 commit
没有触碰那部分代码;在此登记只为留下一条可追溯的关闭指针。
本批六个 commit 合计关闭 #4 #5 #6 #7 #8 #9 #10 #11 #12 #13 #14 #15,
加上 #16 后,14 个 open issue 中 13 个可关;#3(presigned 直传)仍是未实现的 feature。
Fixes #14
Fixes #13
Fixes #9
Fixes #8
Closes #16
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
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.
Summary
Verification
Deployment notes
Closes #3
Closes #4
Closes #5
Closes #6
Closes #7
Closes #8
Closes #9
Closes #10
Closes #11
Closes #12
Closes #13
Closes #14
Closes #15
Closes #16