Skip to content

fix(security): bulk 写谓词守卫按调用者谓词判定 + pre-image 读取去重(#3018 审查后续) - #3053

Merged
os-zhuang merged 1 commit into
mainfrom
claude/owner-id-security-fix-m36705
Jul 16, 2026
Merged

fix(security): bulk 写谓词守卫按调用者谓词判定 + pre-image 读取去重(#3018 审查后续)#3053
os-zhuang merged 1 commit into
mainfrom
claude/owner-id-security-fix-m36705

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

承接 PR #3018 那轮对抗式审查里记录、未在当时处理的两个加固点。

① bulk 写的反-oracle 谓词守卫改为顺序无关(潜在残留漏洞坐实并修复)

#2982 让 bulk update/delete 开始携带 opCtx.ast,于是它们首次进入 step 2.9 的反 filter-oracle 谓词守卫。该守卫的契约(见 predicate-guard.ts 文档注释)是对调用者自己的谓词判定——RLS / sharing 注入的过滤器合法地引用调用者不可读的字段(如 owner_id),不应被拒。

但对 bulk 写,守卫检查的是 opCtx.ast.where,而 plugin-sharing 的写分支可能已经owner_id 属主匹配 AND-合成进了 ast.where;两个中间件的注册顺序没有契约保证。核查结论:当前 verify 栈与生产栈都是 security 先于 sharing(security 外层),所以 2.9 在 sharing 注入前跑、目前是安全的(读路径同形、长期无 bug 也印证了这点)。但这是依赖注册顺序的隐性脆弱——一旦某对象把 owner_id 设为 FLS 不可读,且未来顺序被调整,该 private 对象上的所有 bulk 写会因注入的 owner_id 被 403。

修复:对 update/delete,守卫改看 opCtx.options.where(调用者未被改动的原始谓词),从此与中间件顺序无关,也绝不会把注入的属主/RLS 过滤器误判为探测。读路径不变(读的 seed 就是调用者查询原文,且本中间件在其自身 RLS 注入前运行)。

② pre-image 读取去重(维护性 + 少量性能)

同文件里“按 id 读目标行”的模式在 ~5 个门里各自内联、形态略有分歧(#3018 审查的 Reuse/Efficiency 项)。

  • readRowById(object, id, context) 统一读取形态(fail-closed:引擎缺失 / id 为空 / 读抛错 → null,null 一律 DENY,绝不放行),回填到几个 provenance 门(package-managed / system-row / controlled-by-parent)。
  • getCallerPreImage,把属主锚点回显检查(3.5)与 RLS check 后像(3.6)读取的同一(object, id, 调用者上下文) 行按操作记忆化为一次读取。安全:两门之间没有对该行的写入(驱动写在整个中间件链之后),pre-image 在本操作内稳定。

纯行为等价重构;读取形态从此不会在各站点漂移。

验证

  • 单测:plugin-security 463(+1 新:注入到 AST 的 owner_id 不再触发守卫;调用者自己过滤隐藏字段仍被拒)。既有全部门测试通过 → ② 去重行为等价。
  • dogfood:owner-anchor-and-bulk-writes / two-doors-permission / controlled-by-parent / showcase-invoice-cbp / showcase-private-owd 共 27 全绿。
  • 已加 changeset(plugin-security patch)。

关联

🤖 Generated with Claude Code


Generated by Claude Code

…dupe pre-image reads (#3018 review)
Two hardening follow-ups from the #3018 adversarial review.
① Predicate guard order-independence for writes. #2982 made bulk update/delete
carry opCtx.ast, first bringing them under the step-2.9 anti-oracle guard. That
guard is documented to inspect the CALLER's own predicate (RLS/sharing filters
legitimately reference caller-unreadable fields like owner_id), but for a bulk
write it inspected opCtx.ast.where — which plugin-sharing (a sibling middleware
of unguaranteed order) may already have composed an owner_id match into. On an
object whose owner_id is FLS-hidden that could 403 a legitimate bulk write. The
guard now inspects opCtx.options.where (the caller's untouched predicate) for
update/delete, making it independent of middleware order and unable to mistake
an injected filter for a probe. Reads unchanged.
② Pre-image read dedup. A single readRowById helper (fail-closed: missing
engine / null id / thrown read → null, which always denies) now backs the
provenance gates, and a memoized getCallerPreImage collapses the owner-anchor
echo (3.5) and RLS check post-image (3.6) — the identical (object, id,
caller-context) row — into one read per operation. Pure refactor; the ~5 inlined
read sites can no longer drift.
Tests: plugin-security 463 (+1 new: injected owner_id in the AST does not trip
the guard, a caller filtering a hidden field still does); dogfood owner-anchor /
two-doors / CBP / invoice-cbp / private-owd green.
@vercel

vercelBot commented Jul 16, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
specReadyReadyPreview, CommentJul 16, 2026 12:11pm

Request Review

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling size/m labels Jul 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-security.

11 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/getting-started/cli.mdx(via @objectstack/plugin-security)
  • content/docs/permissions/access-recipes.mdx(via packages/plugins/plugin-security)
  • content/docs/permissions/authorization.mdx(via packages/plugins/plugin-security)
  • content/docs/permissions/explain.mdx(via @objectstack/plugin-security)
  • content/docs/permissions/permissions-matrix.mdx(via packages/plugins/plugin-security)
  • content/docs/permissions/sharing-rules.mdx(via @objectstack/plugin-security)
  • content/docs/plugins/index.mdx(via @objectstack/plugin-security)
  • content/docs/plugins/packages.mdx(via @objectstack/plugin-security)
  • content/docs/releases/implementation-status.mdx(via @objectstack/plugin-security)
  • content/docs/ui/audience-based-interfaces.mdx(via packages/plugins/plugin-security)
  • content/docs/ui/dashboards.mdx(via @objectstack/plugin-security)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@os-zhuang
os-zhuang marked this pull request as ready for review July 16, 2026 12:13
@os-zhuang
os-zhuang merged commit dee7feb into mainJul 16, 2026
16 checks passed
@os-zhuang
os-zhuang deleted the claude/owner-id-security-fix-m36705 branch July 16, 2026 12:13
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@os-zhuang@claude