Uh oh!
There was an error while loading. Please reload this page.
fix(metadata-protocol): findData must not take its execution context from the request (#3960) - #3961
Merged
Merged
Conversation
…from the request (#3960) From the #3946 sweep's leftover question — whether expand's advanced usage (a caller-supplied Record<string, QueryAST> whose sub-ASTs carry their own `object`) is a cross-object read channel. It is NOT: expandRelatedRecords takes its target from the parent schema (the expand key must be a real `reference` field; the sub-AST's `object` is never read), re-enters engine.find so the referenced object's RLS + FLS both run, $and-merges a nested `where` instead of spreading it over the id filter, and caps depth. No change needed there. One layer down is the problem. findData built `{ ...request.query }` and then assigned `context` from `request.context` CONDITIONALLY. `request.query` is the caller's raw bag (the REST POST /data/:object/query route passes req.body straight in), and `context` is in the known-params set so it was not swept into the implicit-filter bucket — it survived, and became the execution context whenever no server context resolved. Everything hangs off that value: plugin-security opens with `if (opCtx.context?.isSystem) return next()`, skipping the whole RLS/FLS/CRUD chain, and `__expandRead` collects the #2850 CRUD-gate waiver. Neither is schema-stripped on the read path — ExecutionContextSchema.parse runs only in createContext. enforceAuth is what kept it unreachable (anonymous data requests need `requireAuth: false`), so this is a fail-open default, not a live exploit — and not something the protocol should delegate upward. Any inbound `context` is now dropped unconditionally. Verified at the protocol layer: a forged { isSystem, userId, __expandRead } reached engine.find verbatim before, is dropped after. The anonymous HTTP reachability half is NOT verified — #3960 records what was and wasn't reproduced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TzLE9cw4gZKNyPN2ZP4iTt
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 1 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
Closes#3960。查的是 #3946 留下的那条尾巴。
先给 expand 的结论:干净的,不用改
原本的怀疑是:
expand的"高级用法"允许调用者直传Record<string, QueryAST>,每个子 AST 自带object—— 会不会是跨对象越权读?不是,expandRelatedRecords四道防线都在:reference、类型为lookup/master_detail/user的字段;读的目标取fieldDef.reference,从 schema 来。子 AST 里那个object键根本没人读(和 全仓扫查"body 展开在受信任值之后":runtime 的 /data/:object/query 也中了,其余 8 处已核实无问题 #3946 排除ast.object同理)。this.find(Security: $expand bypasses RLS/FLS on the referenced object (data leak on lookups) #2850 有意如此)→ 被引用对象的 RLS + FLS 照跑。where用显式$and合并,注释里还专门写了"浅展开会 clobber 掉 id 约束"—— 正是本系列那个坑,这里早就防住了。__expandRead豁免面很窄(只find、只非 private,只免对象级 CRUD 闸门,RLS 注入和 FLS 掩码照跑)。这条结论是承重的,所以值得写下来:下次再有人怀疑 expand,可以直接看这段。
但第 4 条依赖的那句前提不成立
__expandRead的安全性写在注释里:"a server-set marker;executionContextis never client-built"。而protocol.ts:2661:request.query在每个入口上都是调用者的原始 bag(RESTPOST /data/:object/query直接把req.body当query传进来);context在knownParams里,所以也不会被扫进隐式过滤桶,原样留着;context就是这次操作的执行上下文。而它承载的是全部:
__expandRead: true同样能拿到 #2850 的豁免。两者在读路径上都不会被 schema 剥掉 ——ExecutionContextSchema.parse只在engine.createContext()里用,读写热路径不走它。可达性:一半实测,一半没做成,如实说明
ObjectStackProtocolImplementation+ 假引擎):按匿名形状调用,伪造的{ isSystem, userId, __expandRead }原样到达engine.find;isSystem短路整条中间件;读路径无 schema 剥离;requireAuth: false。我在examples/app-crm上加了api: { requireAuth: false }起服务器实打,没生效 ——dev命令这条路径没读 stack 的api键(serve.ts:1724读它,但dev不走那段),四个探测仍全是 401。所以准确的说法是:一个被上层闸门挡住的 fail-open 默认,不是当场可打的洞。但协议层不该把这个不变量委托给上面那道闸门 —— 它自己就拥有这个不变量。
修法
赋值之前无条件
delete options.context。执行上下文只能来自request.context。已核对不破坏任何合法调用方:仓库里唯一构造这些参数的地方(
rest/src/import-runner.ts:245的findArgsBase)把context放顶层、不在query里。getData等兄弟方法从零构造 options,不受影响 —— 只有findData是"调用者 bag 就是 options bag"。验证
expected { isSystem: true } to be undefined。metadata-protocol99 条、rest440 条、runtime837 条、objectql1163 条全绿;改动文件 ESLint 干净。🤖 Generated with Claude Code
https://claude.ai/code/session_01TzLE9cw4gZKNyPN2ZP4iTt
Generated by Claude Code