Skip to content

Commit 2ff01cf

Browse files
Elon Muskclaude
andauthored
fix(service-storage): stamp sys_upload_session.organization_id from the acting session (#13180)
* fix(service-storage): stamp sys_upload_session.organization_id from the acting session (#12928) WIP checkpoint before the build/test lap. * chore: record the new sys_upload_session engine doubles in the pinned ledger (#12928) --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent fa9018a commit 2ff01cf

6 files changed

Lines changed: 686 additions & 27 deletions

File tree

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
"@objectstack/service-storage": patch
3+
---
4+
5+
fix(service-storage): stamp `sys_upload_session.organization_id` from the acting session (#12928)
6+
7+
`StorageMetadataStore.createSession` inserted into `sys_upload_session` with no
8+
execution context, so the SQL driver's `injectTenantOnInsert` had no `tenantId`
9+
to stamp from and every chunked-upload session row landed with
10+
`organization_id` NULL — on a tenancy-ENABLED object (the declaration carries no
11+
`tenancy` key, so `applySystemFields` provisions the column unconditionally).
12+
This is the `sys_upload_session` sibling of the `sys_file` gap fixed in #12745,
13+
and the chunked-upload door already held the value: it threads the identical
14+
`session?.organizationId` into the `createFile` immediately above.
15+
16+
`createSession` now takes the same optional `StorageWriteContext` as
17+
`createFile` and hands the engine `{ context: { tenantId } }`, so the platform's
18+
existing insert-side chokepoint decides the rest — whether the object has a
19+
tenant column at all, and whether an explicit value on the row wins. A caller
20+
with no organization passes no options and the row lands unstamped exactly as
21+
before.
22+
23+
Maintainer ruling 2026-08-29, verbatim and untranslated: 「同意」 — forward stamp
24+
only. There is deliberately **no backfill**: rows already NULL age out through
25+
this object's own ADR-0057 TTL sweep. That premise is verified rather than
26+
assumed — `sys-upload-session-ttl-sweep.test.ts` drives the shipped declaration
27+
through the real `LifecycleService` against live SQL and pins that an expired
28+
NULL-organization row is reaped, that a stamped row is reaped by the same
29+
sweep, that a live session survives it, and that a run with no declaration reaps
30+
nothing.
31+
32+
Why an unstamped row mattered even without a cross-tenant read: both walled
33+
Layer 0 predicates are exclusive (`{ organization_id: <id> }` under `isolated`,
34+
`{ $in: [...] }` under `group`), and neither matches NULL — so on a walled
35+
deployment an unstamped session row was invisible to its own tenant, the same
36+
silent-empty class `sys_api_key` was renamed to avoid.

packages/services/service-storage/src/metadata-store.ts

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ export interface FileRecord {
5050
}
5151

5252
/**
53-
* The acting session's organization, threaded into a `sys_file` write (#12745).
53+
* The acting session's organization, threaded into a `sys_file` write (#12745)
54+
* or a `sys_upload_session` write (#12928).
5455
*
5556
* ## Why a context and not a column on the payload
5657
*
@@ -125,6 +126,21 @@ export interface UploadSessionRecord {
125126
started_at?: string;
126127
expires_at?: string;
127128
updated_at?: string;
129+
/**
130+
* The registry-injected tenant column (#12928) — the `sys_upload_session`
131+
* sibling of the `sys_file` declaration above.
132+
*
133+
* `sys_upload_session` declares no `tenancy` key either, so
134+
* `isTenancyDisabled()` reads `false` and `applySystemFields` provisions
135+
* `organization_id` on every install, walled deployment or not.
136+
*
137+
* Same division of labour as {@link FileRecord.organization_id}: the store
138+
* does NOT put this in the engine payload (see {@link StorageWriteContext}),
139+
* and it is declared here because the column is real — a caller may read it
140+
* back off a row, an admin cross-organization write may set it explicitly,
141+
* and the engine-absent stand-in records it.
142+
*/
143+
organization_id?: string | null;
128144
}
129145

130146
/** The `IDataEngine` operations this store issues. */
@@ -331,7 +347,29 @@ export class StorageMetadataStore {
331347
// Upload sessions
332348
// ---------------------------------------------------------------------------
333349

334-
async createSession(rec: UploadSessionRecord): Promise<UploadSessionRecord> {
350+
/**
351+
* Insert one `sys_upload_session` row.
352+
*
353+
* `context` carries the acting organization (#12928). It is the same channel,
354+
* the same chokepoint and the same reasoning as {@link createFile} (#12745):
355+
* with it the insert reaches the engine as `{ context: { tenantId } }`, which
356+
* is how `sys_upload_session.organization_id` gets written at all — see
357+
* {@link StorageWriteContext} for the chain and for why the column is not
358+
* stamped onto the payload here. Without it the call behaves exactly as it
359+
* did before: the row lands unstamped.
360+
*
361+
* ⚠️ Ruled scope (maintainer, 2026-08-29, verbatim and untranslated: 「同意」):
362+
* FORWARD STAMP ONLY. Rows that are already NULL are deliberately not
363+
* repaired — they age out through this object's own ADR-0057 TTL sweep
364+
* (`lifecycle.ttl`, keyed on `expires_at`). That premise is not assumed here:
365+
* `sys-upload-session-ttl-sweep.test.ts` drives the real declaration through
366+
* the real Reaper against live SQL, because if the sweep were dead the
367+
* no-backfill half of the ruling would fall with it.
368+
*/
369+
async createSession(
370+
rec: UploadSessionRecord,
371+
context?: StorageWriteContext,
372+
): Promise<UploadSessionRecord> {
335373
const now = new Date().toISOString();
336374
const full: UploadSessionRecord = {
337375
uploaded_chunks: 0,
@@ -341,12 +379,20 @@ export class StorageMetadataStore {
341379
updated_at: now,
342380
...rec,
343381
};
382+
const options = writeOptionsFor(context);
344383
if (!this.engine) {
345-
this.sessions.set(full.id, full);
346-
return full;
384+
// Mirrors `createFile`'s stand-in exactly: no schema to ask, so it
385+
// records what it was told, and an explicit value on the record wins
386+
// just as `injectTenantOnInsert` lets it.
387+
const stamped: UploadSessionRecord =
388+
options && full.organization_id == null
389+
? { ...full, organization_id: options.context.tenantId }
390+
: full;
391+
this.sessions.set(stamped.id, stamped);
392+
return stamped;
347393
}
348394
await this.engineOp('sys_upload_session', 'insert', SESSION_INSERT_CONSEQUENCE, (engine) =>
349-
engine.insert('sys_upload_session', full),
395+
engine.insert('sys_upload_session', full, options),
350396
);
351397
return full;
352398
}

packages/services/service-storage/src/storage-routes.ts

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,13 @@ export type FileReadVerdict = 'allow' | 'deny' | 'unauthenticated';
2020
*
2121
* `organizationId` is the session's ACTIVE organization — the scope the upload
2222
* is happening in, and the value threaded into `createFile` so the new
23-
* `sys_file` row is stamped rather than landing NULL. It is optional in both
24-
* directions on purpose: a resolver that only knows the user (every
25-
* pre-#12745 implementation, and every single-tenant deployment) keeps
26-
* type-checking and keeps working, and a session with no active organization
27-
* resolves to `undefined` rather than to a guess.
23+
* `sys_file` row is stamped rather than landing NULL, and (since #12928) into
24+
* `createSession` so the chunked door's `sys_upload_session` row is stamped
25+
* for the same reason. It is optional in both directions on purpose: a
26+
* resolver that only knows the user (every pre-#12745 implementation, and
27+
* every single-tenant deployment) keeps type-checking and keeps working, and a
28+
* session with no active organization resolves to `undefined` rather than to a
29+
* guess.
2830
*/
2931
export interface StorageUploadSession {
3032
userId?: string;
@@ -478,23 +480,31 @@ export function registerStorageRoutes(
478480
const resumeToken = randomUUID();
479481
const expiresAt = new Date(Date.now() + sessionTtl * 1000).toISOString();
480482

481-
await store.createSession({
482-
id: uploadId,
483-
file_id: fileId,
484-
key,
485-
filename,
486-
mime_type: mimeType,
487-
total_size: totalSize,
488-
chunk_size: chunkSize,
489-
total_chunks: totalChunks,
490-
resume_token: resumeToken,
491-
backend_upload_id: backendUploadId,
492-
scope: scope ?? 'user',
493-
bucket,
494-
metadata: metadata ? JSON.stringify(metadata) : undefined,
495-
status: 'in_progress',
496-
expires_at: expiresAt,
497-
});
483+
// The session row is stamped from the SAME session value the
484+
// `createFile` above already threads (#12928). `sys_upload_session` is a
485+
// tenancy-enabled object too, so an insert with no context lands
486+
// `organization_id` NULL — invisible to its own tenant under a walled
487+
// posture, since both walled Layer 0 predicates exclude NULL.
488+
await store.createSession(
489+
{
490+
id: uploadId,
491+
file_id: fileId,
492+
key,
493+
filename,
494+
mime_type: mimeType,
495+
total_size: totalSize,
496+
chunk_size: chunkSize,
497+
total_chunks: totalChunks,
498+
resume_token: resumeToken,
499+
backend_upload_id: backendUploadId,
500+
scope: scope ?? 'user',
501+
bucket,
502+
metadata: metadata ? JSON.stringify(metadata) : undefined,
503+
status: 'in_progress',
504+
expires_at: expiresAt,
505+
},
506+
{ organizationId: session?.organizationId },
507+
);
498508

499509
sendOk(res, {
500510
uploadId,

0 commit comments

Comments
 (0)