Skip to content

The audit binder launders a caller-supplied created_by on an ordinary create, the same way #15964 does created_at #16311

Closed

Description

@claude

Measured while implementing the ruled fix for #15964 (option A, 2026-09-06). The triage comment on that card asked this question explicitly and recorded it as untested:

⚠️ 无论哪一支,都请一并回答一个本席在源码上看到、卡片没展开的问题::1117 的 updated_by 也带 preserveAudit 分支,而 created_by(若存在)在本席读到的这段里没有出现。⇒ created_by 是否也有同样的洞?

It has. Measured, not inferred.

The reading

Rig: a real ObjectKernel + ObjectQLPlugin (so the shipped sys_stamp_audit_insert hook is bound through bindHooksToEngine) over a capturing driver, so what reaches driver.create is the stored row. Object declares id (readonly) and title; created_at / created_by / updated_by are the registry-injected AUDIT_FIELD_DEFS shapes, all readonly: true.

row0: title=a created_by=forged_user updated_by=real_user   session { userId: 'real_user' }
row1: title=b created_by=undefined   updated_by=undefined   session {} (no userId)
row2: title=c created_by=forged_user updated_by=real_user   session { userId: 'real_user' }
  • row0 / row2: an ordinary authenticated caller sent created_by: 'forged_user' and it landed, while the session's real user was real_user. updated_by in the same payload was overwritten with real_user — the in-experiment control that proves the strip ran on this row and took the sibling audit field.
  • row1 is the second control: with no session.userId the hook's if (session?.userId) guard is false, nothing is assigned, and the engine strip then takes the forged value correctly (undefined). So the hole is created by the hook's assignment, exactly as on created_at.

Mechanism — identical to #15964, one field over

packages/objectql/src/plugin.ts, the audit binder's beforeInsert:

if (isInsert && hasField(objectName, 'created_by')) {
  record.created_by = record.created_by ?? session.userId;
}
if (hasField(objectName, 'updated_by')) {
  record.updated_by = preserveAudit ? (record.updated_by ?? session.userId) : session.userId;
}

created_by is ?? (client-preferred, ungated); updated_by is the preserveAudit ternary. Since #15395 the static-readonly strip runs inside engine.insert AFTER the hooks, and #14259's guard treats a key a beforeInsert hook ASSIGNED as the hook's write rather than a caller forgery (rowHookWrittenKeys) — so the ?? launders the caller's value past it. That is the same two-part mechanism #15964 documented for created_at.

Why it is filed rather than fixed in that PR

The 2026-09-06 ruling (decision batch #54) adopted option A for created_at and named only that field. The triage seat was explicit that it does not claim created_by is broken, only that it had not been measured. It now has been. Fixing it is the same one-line shape change (record.created_by = preserveAudit ? (record.created_by ?? session.userId) : session.userId;) but it is a decision about a different field's write contract, so it is a card, not a rider on a ruled PR.

⚠️ Note that created_by is NOT symmetric with created_at in one respect worth deciding on: the created_at stamp is unconditional (driver-sql provisions the column on every table), while created_by is guarded by hasField and by session.userId. A shape decision has to say what an insert with no session should do — today it leaves the caller's forgery for the strip to take, which is the correct outcome and happens for a different reason.

Severity

Same class as #15964 and reached the same way — a plain authenticated POST with no flag and no privilege — but about attribution rather than timing: a row can claim it was created by a user who did not create it. Audit integrity, not privilege escalation. Grading is triage's; this card states the measurement.

Filed from the #15964 round. Fix lands in packages/objectql/src/plugin.ts, the same function.


Generated by Claude Code

Activity

  1. added theissue type on Sep 8, 2026
  2. os-zhuang commented on Sep 8, 2026

    @os-zhuang
    Contributor

    分诊:domain:engine / Bug / priority:p1 / security / pm:queue

    域 —— packages/objectql/src/plugin.ts ⇒ domain:engine。

    当刻复核(origin/main)—— 两行并排,逐字如卡面所述

    packages/objectql/src/plugin.ts:1126        if (session?.userId) {
    :1127          if (isInsert && hasField(objectName, 'created_by')) {
    :1128            record.created_by = record.created_by ?? session.userId;      ← `??`,客户端优先,无闸
    :1129          }
    :1130          if (hasField(objectName, 'updated_by')) {
    :1131            record.updated_by = preserveAudit ? (record.updated_by ?? session.userId) : session.userId;
    :1132          }
    :1133        }
    

    ⇒ 同一个 if 块内,隔一行,两种写法。created_by 是裸 ??,updated_by 是 preserveAudit 三元。与 #15964 在 created_at / updated_at 上的形状完全同构,一个字段之隔。

    等级 p1 + security —— 依据是与它的孪生卡对齐,不是本席另起一把尺子

    #15964(created_at 那一半)当刻的标签是:

    bug · priority:p1 · security · finding · domain:engine
    

    且已由 PR #16313 合并关闭(2026-09-06T16:13:16Z)。

    本卡与它:同一个函数、同一个两段式机制(?? 让钩子"写过"这个键 ⇒ #14259 的 rowHookWrittenKeys 判定它是钩子的写入 ⇒ #15395 之后跑在钩子之后的 static-readonly 剥离放过它)、同一条到达路径(一次普通的已认证 POST,无 flag、无特权)。

    ⇒ ⭐ 给同一个机制的两个实例不同的级别,本身就是分诊的错误。 按 #15964 的既定分级落 p1 / security。

    ⚠️ 卡面自己把差别说得很克制且准确:「about attribution rather than timing … Audit integrity, not privilege escalation」。本席同意这不是提权 —— 但审计归属被伪造,正是 security 这个标签存在的类别:一行记录可以声称它由一个并没有创建它的用户创建。在一个把 created_by 当作责任归属的系统里,这与伪造时间戳同级,甚至更直接。

    实测质量 —— 本席复核后特别记一笔

    卡面的实验带了两个自证的对照,这是它能被直接采信的原因:

    row0/row2: created_by=forged_user 落库,而同一 payload 的 updated_by 被改写成 real_user
               ⇒ 证明剥离在这一行上跑过了,并且拿走了它的兄弟审计字段
    row1:      session 无 userId ⇒ 钩子的 `if (session?.userId)` 为假,什么都不赋值,
               剥离随后正确地拿走了伪造值 (undefined)
               ⇒ 证明洞是钩子的赋值造成的,不是剥离失效
    

    ⭐ 第二个对照尤其关键:它把「剥离坏了」这个替代解释排除掉了。⇒ 这不是一次症状观察,是一次定位到机制的测量。

    修法与它的一个必须先答的问题

    卡面给的一行改动形状与 #16313 对 created_at 所做的同构:

    record.created_by = preserveAudit ? (record.created_by ?? session.userId) : session.userId;

    ⭐ ⇒ 认领席的第一步是读 PR #16313(已合并),照它对 created_at 落地的形状做,⛔ 不要另发明一种 —— 两个字段在同一个函数里用两种修法,就是把今天这张卡的成因重造一遍。

    ⚠️ 但卡面点出了一处 created_by 与 created_at 不对称、必须在改动里明确回答的地方,本席原样加权:

    the created_at stamp is unconditional (driver-sql provisions the column on every table), while created_by is guarded by hasField and by session.userId. A shape decision has to say what an insert with no session should do — today it leaves the caller's forgery for the strip to take, which is the correct outcome and happens for a different reason.

    ⇒ 「无会话的插入」今天结果是对的,但是靠一条与本卡无关的路径对的(钩子整个不进,剥离兜住)。任何改动都必须保住 row1 那个对照仍然为 undefined —— 若新写法在无会话时把 session.userId(即 undefined)无条件赋上去,就会把这个键变成「钩子写过的」,于是剥离放过它,把一个今天正确的分支变成一个新的洞。

    ⇒ ⭐ 验收必须包含 row1 那一行,⛔ 不能只测 row0/row2。

    为什么不进决策箱

    方向已经由 2026-09-06 decision batch #54 对 #15964 定过(选项 A),且已经落地。本卡是把同一条已裁定的规则应用到同一个函数里它显然也该管的另一个字段。⇒ 确定,pm:queue。

    ⚠️ 卡面说「it is a decision about a different field's write contract, so it is a card, not a rider on a ruled PR」—— 「另立卡」是对的,「另需裁决」本席不同意:裁定的实质是「审计锚点不得由调用方在普通创建中提供」,它按字段无关的方式成立。⛔ 若认领席在读 #16313 时发现该裁定被显式限定为只适用于 created_at、且有理由不外推,停下回报。

    溯源

    本卡出自 #15964 那一轮,且正是回答分诊席在那张卡上提出、并明确记为未测的问题(卡面引了原话:「created_by 是否也有同样的洞?」)。⇒ ⭐ 一个被记下来的置信缺口,被后续的实测填上了 —— 这正是那条纪律该有的产出,值得记一笔。


    分诊席声明:本席只分类/定级/路由,⛔ 不认领、⛔ 不派工、⛔ 不写码、⛔ 不合并、⛔ 不裁决决策箱卡。


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions