fix(storage): 对象可把文件读权限委托给自己的服务 — 修复 #3534 引入的审批附件 403#3580
Merged
Conversation
…ervice Fixes a regression from the governed-download change (ADR-0104 D3 wave 2, #3534), found by driving app-showcase in a browser as a real non-admin approver: a LEGITIMATE APPROVER COULD SEE A DECISION ATTACHMENT'S FILENAME BUT GOT 403 OPENING IT. The gate authorized a field-owned file by testing whether the caller can READ the owning row. For an ordinary business object that is right — row readability IS the access rule. For sys_approval_action it asks the wrong authority: the audit table is deliberately closed to ordinary approver positions ("operation 'find' on object 'sys_approval_action' is not permitted for positions [auditor, everyone]"), so the test denied exactly the person the attachment was filed for. The approvals service has always held the real rule — which is why the timeline listing the attachment returned 200 while the bytes returned 403. An object may now name a service to answer instead: - ObjectSchema.fileAccessDelegate — the kernel service that authorizes downloads of files owned by this object's media fields - IFileAccessDelegate.authorizeFileRead(recordId, context) — the contract - sys_approval_action declares 'approvals'; ApprovalService.authorizeFileRead reuses the SAME gate listActions applies (visibility of the parent request) rather than inventing a second, looser rule for the bytes Fails closed: a declared delegate that is missing or does not implement the method denies, rather than silently reverting to the raw read it was declared to replace. Objects that declare nothing are unchanged. Verified in the browser on both sides of the gate, not just the happy one. The approver now downloads the real PDF (200, %PDF-1.4 bytes); an anonymous request is still refused (401), so the anonymous capability URL #3534 set out to close stays closed. Checking the permissive side mattered: the delegate is exactly as open as listActions and no more, because approval history is tenant-scoped by design. The net invariant is that a decision attachment is exactly as readable as the decision it hangs off — never more, and no longer less. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SHpGw3GBA9aFpfwVArRWfd
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 106 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
os-zhuang
marked this pull request as ready for review
July 27, 2026 08:35
This was referenced Jul 27, 2026
os-zhuang
added a commit
that referenced
this pull request
Jul 27, 2026
…he tenant (#3590) (#3599) * fix(approvals)!: scope approval requests to their participants, not the tenant (#3590) getRequest / listRequests / countRequests deliberately query with SYSTEM_CTX to bypass RLS — as their own comments say, the approver-visibility rule spans identity forms RLS cannot model cleanly, so it has to be expressed in the service. Only the TENANT half of that rule was ever written. The comment names the participant half; the code never applied it. So any authenticated user could read any approval request in their tenant: its payload snapshot, its full decision history, and — once decision attachments derived their access from the request (#3580) — its files. `approverId` on listRequests is a FILTER, not authorization. Omitting it returned the whole tenant. A caller now sees a request when they participate in it: the submitter, a current approver (via the normalized approver index, so every identity form the write path recorded is covered), or someone who already acted on it — a past approver whose slot has moved on, a commenter. Override admins keep the unrestricted view the "all requests" console surface depends on; a tokenless context sees nothing. Keying on the concrete user id is sufficient rather than an approximation, and that is worth stating because under-granting here would silently hide approvals someone must act on: position/team/manager/field approvers are resolved to concrete user ids at open time, and the `type:value` literal is only the fallback for a spec that resolved to NOBODY — a slot no one can act on either way, since can_act is a plain membership test over the resolved ids. A write path's own result is NOT re-gated. Every operation echoes back the request it just changed via getRequest; the operation authorized itself before writing, and re-asking answers a settled question — wrongly, for a context that carries no userId (a flow-driven resume, a service-to-service call), turning a successful write into a null result. Four existing tests caught exactly that, which is why the echo now goes through readBackRequest instead. Verified in the browser on app-showcase: a non-admin approver's unfiltered list drops from 3 requests to the 1 she is on, an admin still sees all 3, her decision attachment still downloads (200, real PDF bytes — #3580 composes), and a request she has nothing to do with reads back empty. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SHpGw3GBA9aFpfwVArRWfd * docs(approvals): state who can see a request, now that it is participant-scoped The drift check flagged approvals.mdx, and rightly: it documented `approverId` as one filter among several and never said who may see what — so a reader would reasonably assume omitting it returns the whole tenant, which is exactly what this PR stops being true. Also records that a decision's attachments inherit the same rule, since that is the non-obvious half: the file is as readable as the decision, never more. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SHpGw3GBA9aFpfwVArRWfd --------- Co-authored-by: Claude <noreply@anthropic.com>
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.
修复 #3534(受管下载)引入的回归。在浏览器里以真实非管理员审批人身份驱动 app-showcase 时发现的——单测发现不了。
症状
合法的待办审批人 Ada(
role: null,positions[auditor, everyone]):listActionssigned-contract.pdf这个名字FILE_DOWNLOAD_DENIED名字看得见,点不开。
根因:问错了权威
#3534 判定字段拥有的文件时,问的是"调用者能不能 READ 拥有者那一行"。对普通业务对象这是对的——行可读性本身就是访问规则。
但对
sys_approval_action就问错了人。这张审计表对普通审批人 position 是故意关闭的:于是这个判定恰好拒绝了这份附件本来就是给他看的那个人。真正的规则一直在审批服务手里——这正是为什么列出该附件的时间线返回 200,而字节返回 403。
修法:对象可声明授权委托
ObjectSchema.fileAccessDelegate—— 由哪个 kernel 服务来回答"能不能下载本对象媒体字段拥有的文件"IFileAccessDelegate.authorizeFileRead(recordId, context)—— 契约sys_approval_action声明'approvals';ApprovalService.authorizeFileRead复用listActions用的那同一道闸(父请求的可见性),而不是为字节另发明一条更松的规则fail closed:声明了但服务缺失/没实现该方法 → 拒绝,而不是悄悄退回它本来要替换掉的那个 raw read。没有声明的对象行为完全不变。
浏览器验证(两侧都验,不只验顺的那侧)
在真实 Chromium 会话 + 真实 better-auth cookie 下,走真实决策端点
POST /approvals/requests/{id}/approve:%PDF-1.4真实字节listActions完全一致(见下)第三行我特意查了:Ada 对那个请求的
listActions同样是 200。审批历史是按租户可见的(listActions自己的注释就说这道闸是防跨租户泄漏),所以委托的宽松度和既有时间线规则完全相同,没有更松。净不变量:一份决策附件的可读性,恰好等于它所挂的那条决策本身——不会更多,也不再更少。
过程中的一次自我纠错
第一次跑出的 403 其实是我用
admin/set-user-password留下的PASSWORD_EXPIRED让授权探测 fail-closed 造成的假象,错误码却显示成FILE_DOWNLOAD_DENIED。把密码状态清干净重跑,才拿到上面这个真结果。先证伪再下结论在这里救了一次误报。测试
approval-service.test.ts+5:可见父请求则放行、不可见则拒绝、未知/空 action id 拒绝、查询抛错则 fail closed、以及断言sys_approval_action确实声明了委托。与 #3505 的关系
独立、互补。#3505 修的是
.map(String)把展开值压成"[object Object]"(本分支不含它,所以上面的验证里 Ada 的 chip 仍显示[object Object]—— 顺带独立复现了 #3505 要修的 bug)。两个都合并之后,审批人才能既看到正确文件名、又点得开。🤖 Generated with Claude Code
https://claude.ai/code/session_01SHpGw3GBA9aFpfwVArRWfd
Generated by Claude Code