Skip to content

multi-org: sys_upload_session is never org-stamped either — StorageMetadataStore.createSession inserts without a context #12928

Closed
Bug
Closed
multi-org: sys_upload_session is never org-stamped either — StorageMetadataStore.createSession inserts without a context#12928
Bug
Labels
domain:servicespm:dispatchedpriority:p1High: required for production / M2

Description

@os-litant
os-litant
opened on Aug 28, 2026

Found while implementing the #12745 ruling (sys_file forward stamping + backfill). ⛔ Deliberately NOT fixed there: the maintainer ruling that authorises the sys_file work is scoped to sys_file alone, and the backfill precedent (plugin-approvals/src/backfill-platform-row-organizations.ts) requires a maintainer order per table. This card records the sibling so the asymmetry is visible rather than discovered later.

⛔ Not graded, not routed — recorded only.

Measured, on origin/main at aef1b7e64

1. Same "tenancy-enabled, never written" shape as #12745. packages/services/service-storage/src/objects/system-upload-session.object.ts declares no tenancy key at all:

grep -n "tenancy" packages/services/service-storage/src/objects/system-upload-session.object.ts   # exit 1, no match

so isTenancyDisabled() reads false and applySystemFields provisions organization_id on the object exactly as it does for sys_file.

2. The insert carries no context. packages/services/service-storage/src/metadata-store.ts:349:

await this.engineOp('sys_upload_session', 'insert', SESSION_INSERT_CONSEQUENCE, (engine) =>
  engine.insert('sys_upload_session', full),
);

No third argument, so the SQL driver's injectTenantOnInsert has no options.tenantId to stamp from and the column stays NULL — the same mechanism #12745 measured on sys_file.

3. The caller holds the session. storage-routes.ts's chunked-upload door calls requireUploadSession(...) and then store.createSession({...}) in the same handler that now threads the organization into store.createFile(...). The value is in hand at the call site; only the createSession signature has nowhere to put it.

What is different from #12745, and why that may change the answer

⚠️ These are NOT the same card, and the difference runs toward "maybe leave it":

  • Lifetime. sys_upload_session rows are transient — the object declares a retention/TTL sweep and rows are reaped after a session completes, fails or expires. sys_file is mostly-permanent business truth with compliance value. A read-visibility split on a table that reaps itself is a different cost from one on a table that grows with every upload.
  • Backfill. If the forward stamp is ruled in, whether a backfill is owed at all is a separate question here for the same reason: the existing NULL population ages out on its own, where sys_file's does not.
  • Subject. A sys_upload_session row names a file_id, so its organization could be derived from the sys_file row it belongs to. That derivation only exists AFTER multi-org: sys_file is never org-stamped — StorageMetadataStore.createFile inserts without a context while both callers hold the session #12745's work lands, which is another reason this could not have been folded in.

Not established here

Re-check

grep -c "tenancy" packages/services/service-storage/src/objects/system-upload-session.object.ts
grep -n "engine.insert('sys_upload_session'" packages/services/service-storage/src/metadata-store.ts

Dedup

Searched the storage + org-scoping family. #12745 is the sys_file sibling and is being implemented, not duplicated by this. #7667 (failed/expired status producers) and #5202 (audit rows per chunk) touch the same table on unrelated concerns. No match.

Activity

  1. os-litant
    added
    finding
    on Aug 28, 2026
  2. huangyiirene
    added
    priority:p1High: required for production / M2
    needs-user-decision
    domain:services
    and removed
    finding
    on Aug 28, 2026
  3. huangyiirene
    added the
    Bug
    issue type on Aug 28, 2026

  4. huangyiirene commented on Aug 28, 2026

    @huangyiirene
    huangyiirene
    on Aug 28, 2026 – with Claude
    Collaborator

    <!-- os-decision-facets -->

    定级：Bug · priority:p1 · domain:services · needs-user-decision

    分诊席位，session session_01Aujz2zykf5LXt3T98gRsGe。domain:services 按补丁落地的包定（packages/services/service-storage/**）。needs-user-decision：租户/权限边界属人工地板，⛔ 永不代裁；且 #12745 的同一个岔口已经走过维护者。

    卡的两条 re-check 在当前 main（2b4178a）复现 ✅

    git grep -c 'tenancy' … system-upload-session.object.ts   → exit 1（无 tenancy 键）
正对照 同 ref 同文件 'sys_upload_session'                  → 1        ✅ 零是真读数
git grep -n "engine.insert('sys_upload_session'" … metadata-store.ts:349   → 命中，仍无第三参

    ⭐ 但本卡有一处方向性错误，它把优先级压低了

    本卡写：「No severity grade — nothing here malfunctions today.」

    这一句不成立。 未打戳的行不是"归属不明"，而是被读侧当作平台全局行，对每一个租户可见。读实现是唯一的读侧收口 SqlDriver.applyTenantScope（packages/drivers/driver-sql/src/sql-driver.ts:11722），谓词是：

    // `(field = :tenantId OR field IS NULL)` — a NULL tenant column marks a
// GLOBAL/platform row … Rows stamped with a DIFFERENT tenant stay invisible
// exactly as before; authorization on global rows remains the job of the
// layers above (RBAC, RLS, managed_by gates).
return builder.where((b) => {
  void b.where(field, String(tenantId)).orWhereNull(field);
});

    ⇒ 这是 #2734 刻意的 fail-open（否则新部署上每个租户管理员看到零条 RBAC 行）。⛔ 它没有错 —— 错的是让一张业务表的行掉进"平台全局"这个类别。

    掉进去的行携带什么（system-upload-session.object.ts，146 行，我读了全部字段声明）：filename、key（对象存储路径）、file_id、mime_type、total_size、chunk_size、uploaded_chunks / total_chunks、status、expires_at。⚠️ 其中 key 是存储侧真实路径。

    而它是一等公民元数据对象，不是内部表：声明了 label / pluralLabel / icon / nameField: 'filename' / titleFormat / highlightFields —— 即会出现在列表 UI 里。

    对象级访问守卫：一个都没有。 实测该文件不含 permissions、managed_by、hidden、internal、tenancy 任何一个键（正对照：同文件确实声明了 name/label/fields/lifecycle 等 8 个顶层键）。⚠️ sys_file 同样一个都没有 —— 所以上面那段 docstring 点名的三道补偿控制里，managed_by 这一道实测缺席。

    四棱

    ① 项目长远合理性

    读侧 NULL⇒全局是一条平台级语义：它把"没打戳"和"属于所有人"合并成同一个状态。只要有对象既启用租户隔离又存在不带 context 的写入路径，就会持续生产全局行。⇒ 长远正解不是逐表补戳，是让这一类在写入时就不可能发生（见棱③）。逐表补戳是必须做的止血，⛔ 但不要当成结构修复。

    ② 实际业务拉动

    负向拉动，且是最难挽回的那种。 多租户 SaaS 的第一条承诺是"我的数据只有我看得见"。一个客户在列表里看到另一个客户的文件名和存储路径，损失的不是一个功能，是这条承诺本身。⚠️ 行是 TTL 回收的 —— 这限制窗口，不改变性质：任意时刻的在飞上传都在窗口内。

    ③ 防 AI 犯错

    ⭐ 这一棱最重，因为本卡自己就是证据：一个把这条链路读得非常仔细的席位（测了 tenancy 键、测了 insert 缺参、测了调用方手里有 org），仍然把结论写成了"今天不出故障" —— 因为它没有读读侧的 NULL 语义。⇒ 缺的不是细心，是"未打戳意味着什么"这条事实不在读代码的路径上：写侧看不到它，只有读侧一段注释里有。
    ⚠️ 同一个坑会继续吃人。injectTenantOnInsert 下面紧接着就有一个"surface writes that target a tenant-scoped object but don't carry a tenantId"的机制 —— 它为什么没有对这次 insert 报警，我没有测（下节列了命令）。若它本该报而没报，那才是这一类的真正修复点。

    ④ 创业阶段不扩散

    ⛔ 不要借本卡开一个"全表租户审计"工程。范围应当锁死在 service-storage 的这一处 insert，外加一条把这类写入变成不可能的机制（若上一段那个报警机制已存在，就是修它，不是新建）。⚠️ 全仓审计是正确的长期动作，但它是另一张卡，且需要维护者点头 —— 那属于机群形态/开支。

    选项 × 真实客户可感成本

    做法 客户可感 风险
    A ⭐ createSession 前向打戳（签名加 org，调用方手里已有） 跨租户可见立即消失；在飞上传不受影响 低。⚠️ 存量 NULL 行仍全局可见，直到 TTL 到期
    B A + 回填存量 同上，且立即收干净 需维护者按表下令（backfill-platform-row-organizations.ts 先例）。⚠️ 本卡指出 file_id 可反查 org，但该反查要等 #12745 落地后才存在
    C 声明 tenancy: { enabled: false } ⛔ 不消除暴露 —— 只是把"全局可见"从事故改名为设计 ⚠️ 除非同时加对象级守卫，否则这是把问题写进规格
    D 什么都不做 跨租户可见持续存在 本卡原倾向（因判其"不出故障"）—— ⭐ 该前提已被推翻

    建议：A 立即，B 由维护者按 #12745 先例单独下令

    理由是不对称的：A 的代价是一个签名加一个参数（调用方 storage-routes.ts 手里已经有 org，本卡自己测过），收益是止住新增暴露。⛔ 不要等 B 一起做 —— 回填要按表授权，而止血不需要。

    ⛔ C 不能单独使用。 若维护者倾向 C（这张表确实不该按租户隔离），它必须与一道对象级守卫同时落地，否则等于把跨租户可见性合法化。

    退路

    若判定 sys_upload_session 根本不该对租户可读（它是服务内部状态） ⇒ 走 C + managed_by 或等价守卫，并把同一判断适用于 sys_file 之外的其余 service 内部表 —— ⚠️ 那是比 A 更大的范围，需要单独立卡。

    ⚠️ 置信缺口（必填，本卡有三条，第 1 条决定它是不是 p0）

    1. ⭐ 我没有测 RBAC/RLS 是否在驱动之上把它关上了。 driver 层对 NULL 行是敞开的，而那段 docstring 自己把补偿控制点名为「RBAC, RLS, managed_by gates」——其中 managed_by 我实测缺席，另外两道未测。⇒ 若默认权限集不授予租户用户读 sys_upload_session，实际暴露面显著小于本卡描述，应降到 p2；若授予，这是 p0。取卡者第一步必须是这一测，⛔ 不要跳过。
    2. 我没有测那道"无 tenantId 写入"的报警机制为何没响。 下一步命令： 
      git grep -n 'warnUnscoped\|unscoped write\|without a tenantId' -- packages/drivers/driver-sql/src/sql-driver.ts
      若它本应覆盖这条 insert，则这一类的修复点在那里，而不在 service-storage。
    3. TTL 的实际时长我没读（只见 expires_at 字段与 lifecycle 段）。窗口大小影响 B（回填）值不值得，⛔ 不影响 A。

    裁后执行段

    Generated by Claude Code

  5. zhuangjianguo
    added a commit that references this issue on Aug 30, 2026

    fix(service-storage): scope the sys_file / sys_upload_session update …

    e02648e
  6. akarma-synetal
    added 2 commits that reference this issue on Sep 1, 2026

    fix(service-storage): stamp `sys_upload_session.organization_id` from…

    2ff01cf

    fix(service-storage): scope the sys_file / sys_upload_session update …

    f087c37
  7. akarma-synetal
    added a commit that references this issue on Oct 7, 2026

    docs(service-storage): re-anchor the dead tracker citations to the co…

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    domain:servicespm:dispatchedpriority:p1High: required for production / M2

    Type

    Bug

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions