From 4f96ff89691cc9a09227db4e74cfb45e8b0e245a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 06:06:32 +0000 Subject: [PATCH 1/2] fix(service-storage): the commit and chunked-completion doors answer an organization change with 409, not an outage 500 An uploader whose active organization changed after starting an upload names a row the doors' scoped by-id write cannot reach. The engine's miss came back as 500 INTERNAL carrying the store's "Restore the data engine" text. Both doors now ask the store (organizationOutOfWriteReach) before any write and answer 409 RESOURCE_CONFLICT naming the change, with one operator warning naming the cause. The engine-absent stand-in does not scope and keeps its answers. Claude-Session: https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ Co-authored-by: Claude --- .changeset/22175-upload-org-change-4xx.md | 14 + ...ge-upload-organization-change.wall.test.ts | 254 ++++++++++++++++++ .../src/error-envelope.conformance.test.ts | 25 ++ .../src/metadata-store.test.ts | 45 ++++ .../service-storage/src/metadata-store.ts | 54 +++- .../storage-routes.metadata-outage.test.ts | 252 +++++++++++++++++ .../service-storage/src/storage-routes.ts | 91 ++++++- 7 files changed, 732 insertions(+), 3 deletions(-) create mode 100644 .changeset/22175-upload-org-change-4xx.md create mode 100644 packages/plugins/organizations/src/storage-upload-organization-change.wall.test.ts diff --git a/.changeset/22175-upload-org-change-4xx.md b/.changeset/22175-upload-org-change-4xx.md new file mode 100644 index 00000000000..0303c0a3783 --- /dev/null +++ b/.changeset/22175-upload-org-change-4xx.md @@ -0,0 +1,14 @@ +--- +'@objectstack/service-storage': patch +--- + +fix(service-storage): finishing an upload after switching active organization answers `409 RESOURCE_CONFLICT` naming the change, not a `500` that reads as a data-engine outage + +Clause-②: no + +`POST /api/v1/storage/upload/complete` and `POST /api/v1/storage/upload/chunked/:uploadId/complete` write by id under the caller's active organization, and that write is scoped to it. When the uploader's active organization is no longer the one the upload was started in, the write cannot reach the upload's row. Both doors used to answer that with `500 INTERNAL` and a message diagnosing a data-engine outage ("Restore the data engine …"). + +- **Now:** before writing anything, each door answers `409 RESOURCE_CONFLICT` in the standard error envelope. The message says the upload was started in a different organization than the active one, and that switching back finishes it. The server log carries one warning naming the upload, the organization it was started in and the active one. +- **Unchanged:** which organization an upload belongs to (switching back finishes it there); a commit or completion from the starting organization; a real data-engine failure, which still answers `500` with its consequence and fix; the uploader-only rule, which runs first, so a caller who did not start the upload is still refused `403 PERMISSION_DENIED` and learns nothing about organizations; uploads stamped with no organization; routes with no session resolver; a store with no data engine wired; and the progress door. + +What changes for you: when either door answers `409 RESOURCE_CONFLICT`, switch the active organization back to the one the upload was started in and retry the same call. diff --git a/packages/plugins/organizations/src/storage-upload-organization-change.wall.test.ts b/packages/plugins/organizations/src/storage-upload-organization-change.wall.test.ts new file mode 100644 index 00000000000..3bd5525be1c --- /dev/null +++ b/packages/plugins/organizations/src/storage-upload-organization-change.wall.test.ts @@ -0,0 +1,254 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #22175 — an uploader whose ACTIVE organization changed after starting an + * upload, measured on a BOOTED organization wall. + * + * The commit door (`POST …/upload/complete`) and the chunked-completion door + * (`POST …/upload/chunked/:uploadId/complete`) write by id, and those writes + * are scoped to the caller's active organization. A row stamped for the + * organization the upload started in is out of that scope's reach once the + * uploader switches, and the doors used to answer the scoped miss as + * `500 INTERNAL` with a message diagnosing a data-engine outage. They now ask + * the store whether that write can reach the row (`organizationOutOfWriteReach`) + * before any write and answer `409 RESOURCE_CONFLICT`, in the ADR-0112 + * envelope, with a message that names the organization change. Which + * organization an upload belongs to is NOT changed: switching back finishes it, + * from where it started. + * + * Same boot as `storage-upload-door-ownership.wall.test.ts` (and the same + * reason this file lives in this package: the wall is raised by THIS + * package's `OrganizationsPlugin`, which ADR-0132 forbids every other + * workspace package from mounting). One caller: the seeded platform owner, + * who holds two organizations and switches between them through the + * product's own `set-active` door. Nothing is stubbed. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { StorageServicePlugin } from '@objectstack/service-storage'; +import { OrganizationsPlugin } from './organizations-plugin.js'; + +const APP = { + manifest: { + id: 'com.example.uploadorgchange', + namespace: 'uploadorgchange', + version: '0.0.1', + type: 'app', + name: 'Upload Organization Change Fixture', + }, + objects: [], +}; + +const SYS = { context: { isSystem: true } } as const; + +interface Answer { + status: number; + body: any; +} + +describe('the upload doors after the uploader switches organization, on a booted wall (#22175)', () => { + const restoreEnv: Array<[string, string | undefined]> = []; + let rootDir: string; + let stack: VerifyStack; + let ql: any; + /** The seeded platform owner — the uploader throughout. */ + let uploader: string; + /** The organization every upload below is started in. */ + let startOrg: string; + /** The uploader's second organization — where they switch to. */ + let otherOrg: string; + + const answer = async (res: Response): Promise => ({ + status: res.status, + body: await res.json().catch(() => undefined), + }); + const activeOrganization = async (): Promise => { + const res = await stack.apiAs(uploader, 'GET', '/auth/get-session'); + return ((await res.json()) as any)?.session?.activeOrganizationId ?? null; + }; + const switchTo = async (organizationId: string): Promise => { + const res = await stack.apiAs(uploader, 'POST', '/auth/organization/set-active', { organizationId }); + expect(res.status).toBe(200); + expect(await activeOrganization()).toBe(organizationId); + }; + const stored = (object: string, id: string) => ql.findOne(object, { where: { id } }, SYS); + + const presign = async (): Promise => { + const res = await answer( + await stack.apiAs(uploader, 'POST', '/storage/upload/presigned', { + filename: 'org-change-plan.txt', + mimeType: 'text/plain', + size: 5, + }), + ); + expect(res.status).toBe(200); + return res.body.data.fileId; + }; + const startChunked = async (): Promise<{ uploadId: string; fileId: string }> => { + const res = await answer( + await stack.apiAs(uploader, 'POST', '/storage/upload/chunked', { + filename: 'org-change-chunked.bin', + mimeType: 'application/octet-stream', + totalSize: 10, + }), + ); + expect(res.status).toBe(200); + return { uploadId: res.body.data.uploadId, fileId: res.body.data.fileId }; + }; + + /** + * The organization-change answer: a 4xx in the declared envelope, its code, + * and a message that names the change — never the `500` an engine outage + * answers, nor that outage's prescription. + */ + const expectOrganizationChanged = (res: Answer, label: string) => { + const seen = `${label}: ${JSON.stringify(res.body)}`; + expect(res.status, seen).toBe(409); + expect(res.body?.success, seen).toBe(false); + expect(res.body?.error?.code, seen).toBe('RESOURCE_CONFLICT'); + expect(res.body?.data, seen).toBeUndefined(); + expect(res.body?.error?.message, seen).toMatch(/started in a different organization/); + expect(res.body?.error?.message, seen).not.toMatch(/data engine/); + }; + + beforeAll(async () => { + for (const [name, value] of [ + ['OS_TENANCY_POSTURE', 'isolated'], + // A walled deployment must declare what a new user joins, or this + // package's own boot gate refuses it. + ['OS_AUTH_MEMBERSHIP_POLICY', 'invite-only'], + ] as const) { + restoreEnv.push([name, process.env[name]]); + process.env[name] = value; + } + rootDir = mkdtempSync(join(tmpdir(), 'upload-org-change-')); + stack = await bootStack(APP as never, { + extraPlugins: [ + new OrganizationsPlugin(), + new StorageServicePlugin({ adapter: 'local', local: { rootDir }, bindToSettings: false }), + ], + }); + ql = await stack.kernel.getServiceAsync('objectql'); + + uploader = await stack.signIn(); + startOrg = (await activeOrganization())!; + const created = await answer( + await stack.apiAs(uploader, 'POST', '/auth/organization/create', { + name: 'Upload Org Change Second', + slug: 'upload-org-change-second', + }), + ); + expect(created.status).toBe(200); + otherOrg = created.body.id; + await switchTo(startOrg); + }, 120_000); + + afterAll(async () => { + await stack?.stop(); + if (rootDir) rmSync(rootDir, { recursive: true, force: true }); + for (const [name, value] of restoreEnv.reverse()) { + if (value === undefined) delete process.env[name]; + else process.env[name] = value; + } + }); + + it('PREMISE: the wall is up, the uploader holds two organizations, and uploads are stamped where they start', async () => { + const tenancy: any = await stack.kernel.getServiceAsync('tenancy'); + expect(tenancy.posture).toBe('isolated'); + expect(tenancy.isolationActive).toBe(true); + expect(tenancy.degraded).toBe(false); + + expect(startOrg).toBeTruthy(); + expect(otherOrg).toBeTruthy(); + expect(otherOrg).not.toBe(startOrg); + + await switchTo(startOrg); + const fileId = await presign(); + expect((await stored('sys_file', fileId)).organization_id).toBe(startOrg); + const { uploadId } = await startChunked(); + expect((await stored('sys_upload_session', uploadId)).organization_id).toBe(startOrg); + }); + + describe('the commit door', () => { + const commit = (fileId: string, eTag: string) => + stack.apiAs(uploader, 'POST', '/storage/upload/complete', { fileId, eTag }); + + it('ORGANIZATION CHANGED: 409 RESOURCE_CONFLICT naming the change, not 500; the row is unchanged; switching back commits it', async () => { + await switchTo(startOrg); + const fileId = await presign(); + const before = await stored('sys_file', fileId); + + await switchTo(otherOrg); + expectOrganizationChanged(await answer(await commit(fileId, 'etag-elsewhere')), 'commit after switching'); + expect(await stored('sys_file', fileId)).toEqual(before); + + // The upload still belongs where it started, and finishes there. + await switchTo(startOrg); + const home = await answer(await commit(fileId, 'etag-home')); + expect(home.status, JSON.stringify(home.body)).toBe(200); + const row = await stored('sys_file', fileId); + expect(row.status).toBe('committed'); + expect(row.etag).toBe('etag-home'); + expect(row.organization_id).toBe(startOrg); + }); + + it('CONTROL — SAME ORGANIZATION: the uploader commits exactly as before', async () => { + await switchTo(startOrg); + const fileId = await presign(); + const own = await answer(await commit(fileId, 'etag-same')); + expect(own.status, JSON.stringify(own.body)).toBe(200); + expect(own.body.data.fileId).toBe(fileId); + expect((await stored('sys_file', fileId)).status).toBe('committed'); + }); + }); + + describe('the chunked-completion door', () => { + const complete = (uploadId: string) => + stack.apiAs(uploader, 'POST', `/storage/upload/chunked/${uploadId}/complete`, { parts: [] }); + + it('ORGANIZATION CHANGED: 409 RESOURCE_CONFLICT naming the change, not 500; neither row moves; switching back completes it', async () => { + await switchTo(startOrg); + const { uploadId, fileId } = await startChunked(); + const sessionBefore = await stored('sys_upload_session', uploadId); + const fileBefore = await stored('sys_file', fileId); + + await switchTo(otherOrg); + expectOrganizationChanged(await answer(await complete(uploadId)), 'chunked completion after switching'); + expect(await stored('sys_upload_session', uploadId)).toEqual(sessionBefore); + expect(await stored('sys_file', fileId)).toEqual(fileBefore); + + await switchTo(startOrg); + const home = await answer(await complete(uploadId)); + expect(home.status, JSON.stringify(home.body)).toBe(200); + expect((await stored('sys_upload_session', uploadId)).status).toBe('completed'); + const file = await stored('sys_file', fileId); + expect(file.status).toBe('committed'); + expect(file.organization_id).toBe(startOrg); + }); + + it('CONTROL — SAME ORGANIZATION: the uploader completes exactly as before', async () => { + await switchTo(startOrg); + const { uploadId, fileId } = await startChunked(); + const own = await answer(await complete(uploadId)); + expect(own.status, JSON.stringify(own.body)).toBe(200); + expect(own.body.data.fileId).toBe(fileId); + expect((await stored('sys_upload_session', uploadId)).status).toBe('completed'); + expect((await stored('sys_file', fileId)).status).toBe('committed'); + }); + }); + + it('UNCHANGED — the progress door still answers the uploader after the switch (a read; no write is due)', async () => { + await switchTo(startOrg); + const { uploadId, fileId } = await startChunked(); + await switchTo(otherOrg); + const progress = await answer(await stack.apiAs(uploader, 'GET', `/storage/upload/chunked/${uploadId}/progress`)); + expect(progress.status, JSON.stringify(progress.body)).toBe(200); + expect(progress.body.data.fileId).toBe(fileId); + expect(progress.body.data.status).toBe('in_progress'); + await switchTo(startOrg); + }); +}); diff --git a/packages/services/service-storage/src/error-envelope.conformance.test.ts b/packages/services/service-storage/src/error-envelope.conformance.test.ts index 4559a01b5ed..5819f04a7ef 100644 --- a/packages/services/service-storage/src/error-envelope.conformance.test.ts +++ b/packages/services/service-storage/src/error-envelope.conformance.test.ts @@ -222,6 +222,31 @@ describe('storage error envelope (#3675)', () => { return drive(routes, 'POST', `${BASE}/upload/complete`, { body: { fileId: 'f-owned' } }); }, }, + { + // #22175: the uploader, acting in another organization than the one the + // upload was started in. + name: 'committing an upload after switching active organization', + status: 409, + code: 'RESOURCE_CONFLICT', + run: async () => { + // An engine-backed store (the stand-in does not scope its writes, so + // it never answers this); the door refuses before any write, so the + // engine only ever answers the by-id read. + const row = { + id: 'f-started', + key: 'user/f-started.bin', + name: 'f.bin', + status: 'pending', + owner_id: 'u-uploader', + organization_id: 'org-start', + }; + const store = new StorageMetadataStore({ findOne: async () => ({ ...row }) } as any); + const routes = mount(await tmpAdapter(), store, { + resolveSession: async () => ({ userId: 'u-uploader', organizationId: 'org-elsewhere' }), + }); + return drive(routes, 'POST', `${BASE}/upload/complete`, { body: { fileId: 'f-started' } }); + }, + }, { name: 'anonymous upload when a session resolver is wired', status: 401, diff --git a/packages/services/service-storage/src/metadata-store.test.ts b/packages/services/service-storage/src/metadata-store.test.ts index 844521a5fc7..75410da863c 100644 --- a/packages/services/service-storage/src/metadata-store.test.ts +++ b/packages/services/service-storage/src/metadata-store.test.ts @@ -22,6 +22,7 @@ import type { IDataEngine } from '@objectstack/spec/contracts'; import { StorageMetadataStore, StorageMetadataStoreError, + organizationOutOfWriteReach, type FileRecord, type UploadSessionRecord, } from './metadata-store.js'; @@ -359,3 +360,47 @@ describe('StorageMetadataStore: engine wired and failing (#5216)', () => { ); }); }); + +// --------------------------------------------------------------------------- +// 4. #22175 — does the scoped by-id write reach the row? Asked before writing. +// --------------------------------------------------------------------------- + +describe('organizationOutOfWriteReach (#22175)', () => { + const inOrgA = { organization_id: 'org_a' }; + + it('names the starting organization when the acting one differs, on an engine-backed store', () => { + const store = new StorageMetadataStore(createFakeEngine()); + expect(organizationOutOfWriteReach(store, inOrgA, { organizationId: 'org_b' })).toBe('org_a'); + }); + + it('answers null when the row is in reach: same organization', () => { + const store = new StorageMetadataStore(createFakeEngine()); + expect(organizationOutOfWriteReach(store, inOrgA, { organizationId: 'org_a' })).toBeNull(); + }); + + it('answers null when no organization is threaded — the write is unscoped', () => { + const store = new StorageMetadataStore(createFakeEngine()); + expect(organizationOutOfWriteReach(store, inOrgA)).toBeNull(); + expect(organizationOutOfWriteReach(store, inOrgA, {})).toBeNull(); + expect(organizationOutOfWriteReach(store, inOrgA, { organizationId: '' })).toBeNull(); + expect(organizationOutOfWriteReach(store, inOrgA, { organizationId: null })).toBeNull(); + }); + + it('answers null for a row stamped with no organization — it stays in reach', () => { + const store = new StorageMetadataStore(createFakeEngine()); + for (const row of [{}, { organization_id: null }, { organization_id: '' }]) { + expect(organizationOutOfWriteReach(store, row, { organizationId: 'org_b' })).toBeNull(); + } + }); + + it('answers null for a missing row — there is nothing for it to refuse', () => { + const store = new StorageMetadataStore(createFakeEngine()); + expect(organizationOutOfWriteReach(store, null, { organizationId: 'org_b' })).toBeNull(); + expect(organizationOutOfWriteReach(store, undefined, { organizationId: 'org_b' })).toBeNull(); + }); + + it('answers null on the engine-absent stand-in, which does not scope its writes', () => { + const store = new StorageMetadataStore(null); + expect(organizationOutOfWriteReach(store, inOrgA, { organizationId: 'org_b' })).toBeNull(); + }); +}); diff --git a/packages/services/service-storage/src/metadata-store.ts b/packages/services/service-storage/src/metadata-store.ts index ff99779bb75..065ddd1d0ca 100644 --- a/packages/services/service-storage/src/metadata-store.ts +++ b/packages/services/service-storage/src/metadata-store.ts @@ -350,6 +350,18 @@ const SESSION_DELETE_CONSEQUENCE = const SESSION_READ_CONSEQUENCE = 'the row could not be read at all, which is NOT the same as it being absent — answering "upload session not found" here would abort a live upload.'; +/** + * [#22175] Every store constructed over a wired data engine — the stores whose + * by-id writes are SCOPED by the acting organization ({@link StorageWriteContext}). + * The engine-absent stand-in is never in it: it does not scope (see the stand-in + * branch of {@link StorageMetadataStore.updateFile}). + * + * A module-private set rather than a class member, so the one fact the upload + * doors need — "does this store's write have a reach to miss?" — is answered by + * {@link organizationOutOfWriteReach} without widening the class's public face. + */ +const ENGINE_BACKED_STORES = new WeakSet(); + /** * Storage metadata persistence. * @@ -379,7 +391,9 @@ export class StorageMetadataStore { private readonly files = new Map(); private readonly sessions = new Map(); - constructor(private readonly engine: IDataEngine | null) {} + constructor(private readonly engine: IDataEngine | null) { + if (engine) ENGINE_BACKED_STORES.add(this); + } /** * Run one engine call, converting any failure into a @@ -652,3 +666,41 @@ export class StorageMetadataStore { ); } } + +/** + * [#22175] The organization `row` was started in, when the by-id write `store` + * would issue under `context` cannot reach it — `null` when it can. + * + * The commit and chunked-completion doors ask this BEFORE they write. An + * uploader whose active organization changed after starting an upload names a + * row the scoped write cannot reach, and the engine raises that miss as an + * error {@link StorageMetadataStore}'s `engineOp` can only wrap as a + * {@link StorageMetadataStoreError} — an outage, by its text. Asked first, the + * door can answer what actually happened instead. + * + * It reads only what decides the reach of the write this store issues: + * - the engine-absent stand-in does not scope → `null`, always; + * - no organization threaded (`writeOptionsFor` → `undefined`) → the write is + * unscoped → `null`; + * - a row stamped with no organization → `null`: the driver's scope keeps + * org-less rows in reach (#2734's global-row arm); + * - otherwise the row is in reach exactly when its organization is the acting + * one. + * + * ⛔ It composes no predicate and decides no tenancy rule — the statement's + * tenant term stays the driver's (`applyTenantScope`); this reads, in advance, + * the reach {@link StorageWriteContext} already documents. ⛔ Not re-exported + * from the package entry: it is the doors' question, not API. + */ +export function organizationOutOfWriteReach( + store: StorageMetadataStore, + row: { organization_id?: string | null } | null | undefined, + context?: StorageWriteContext, +): string | null { + if (!ENGINE_BACKED_STORES.has(store)) return null; + const acting = writeOptionsFor(context)?.context.tenantId; + if (acting === undefined) return null; + const started = row?.organization_id; + if (typeof started !== 'string' || started.length === 0) return null; + return started === acting ? null : started; +} diff --git a/packages/services/service-storage/src/storage-routes.metadata-outage.test.ts b/packages/services/service-storage/src/storage-routes.metadata-outage.test.ts index f441ff2f787..70cfd095bcd 100644 --- a/packages/services/service-storage/src/storage-routes.metadata-outage.test.ts +++ b/packages/services/service-storage/src/storage-routes.metadata-outage.test.ts @@ -267,3 +267,255 @@ describe('Storage routes: a metadata write that never landed is not a 200 (#5216 expect(await store.getFile(res._json.data.fileId)).toMatchObject({ status: 'pending' }); }); }); + +// --------------------------------------------------------------------------- +// #22175 — an organization change is not an outage +// --------------------------------------------------------------------------- +// +// The commit door and the chunked-completion door write by id under the +// uploader's ACTIVE organization, and an engine-backed store scopes that write +// to it. An uploader who switched organization after starting an upload names +// a row the scoped write cannot reach, and the engine's miss used to come back +// through this file's `500` path with the outage text. The doors now ask the +// store's `organizationOutOfWriteReach` first and answer `409 +// RESOURCE_CONFLICT`, naming the change; the operator log names the cause. +// +// The double above does NOT scope its writes, so a door that skipped the +// question would visibly change the row — which is what makes every "row +// unchanged" assertion below able to fail. The same pins on a booted +// organization wall, where the write really is scoped, live in +// `packages/plugins/organizations`. + +describe('Storage routes: an organization change is not an outage (#22175)', () => { + const COMMIT = '/api/v1/storage/upload/complete'; + const CHUNKED_COMPLETE = '/api/v1/storage/upload/chunked/:uploadId/complete'; + const PROGRESS = '/api/v1/storage/upload/chunked/:uploadId/progress'; + /** The first sentence of the answer — its wording is the ruling's contract. */ + const FIRST_SENTENCE = 'This upload was started in a different organization than your active one.'; + + /** The uploader in three active organizations, and a second user acting elsewhere. */ + const SESSIONS: Record = { + 'alice@a': { userId: 'u_alice', organizationId: 'org_a' }, + 'alice@b': { userId: 'u_alice', organizationId: 'org_b' }, + 'alice@none': { userId: 'u_alice' }, + 'carol@b': { userId: 'u_carol', organizationId: 'org_b' }, + }; + + let rootDir: string; + let adapter: LocalStorageAdapter; + let httpServer: ReturnType; + let warnings: string[]; + + const mount = (engine: IDataEngine | null) => { + const store = new StorageMetadataStore(engine); + registerStorageRoutes(httpServer as any, adapter, store, { + basePath: '/api/v1/storage', + resolveSession: async (req) => SESSIONS[String(req.headers['x-test-as'] ?? '')] ?? null, + logger: { info: () => {}, warn: (msg: string) => { warnings.push(msg); } }, + }); + return store; + }; + const drive = async (method: string, path: string, as: string, req: Partial = {}) => { + const res = createMockRes(); + await httpServer._getHandler(method, path)!(createMockReq({ ...req, headers: { 'x-test-as': as } }), res); + return res; + }; + /** A pending upload Alice started in `org_a`: its file row and, for the chunked door, its session. */ + const seed = async (store: StorageMetadataStore, sessionOrganization: string | undefined = 'org_a') => { + await store.createFile({ + id: 'f1', + key: 'user/f1.bin', + name: 'f1.bin', + status: 'pending', + owner_id: 'u_alice', + organization_id: 'org_a', + }); + await store.createSession({ + id: 'u1', + file_id: 'f1', + key: 'user/f1.bin', + filename: 'f1.bin', + total_size: 100, + chunk_size: 50, + total_chunks: 2, + status: 'in_progress', + ...(sessionOrganization ? { organization_id: sessionOrganization } : {}), + }); + }; + const expectOrganizationChanged = (res: ReturnType, label: string) => { + const seen = `${label}: ${JSON.stringify(res._json)}`; + expect(res._status, seen).toBe(409); + expect(res._json.success, seen).toBe(false); + expect(res._json.error.code, seen).toBe('RESOURCE_CONFLICT'); + expect(res._json.data, seen).toBeUndefined(); + expect(String(res._json.error.message).startsWith(FIRST_SENTENCE), seen).toBe(true); + }; + + beforeEach(async () => { + rootDir = join(tmpdir(), `os-22175-${Date.now()}-${Math.random().toString(36).slice(2)}`); + await fs.mkdir(rootDir, { recursive: true }); + adapter = new LocalStorageAdapter({ rootDir, signingSecret: 'test-secret' }); + httpServer = createMockHttpServer(); + warnings = []; + }); + + afterEach(async () => { + if (rootDir) await fs.rm(rootDir, { recursive: true, force: true }); + }); + + describe('the commit door', () => { + it('answers 409 RESOURCE_CONFLICT naming the change, writes nothing, and logs the cause', async () => { + const engine = createFakeEngine(); + await seed(mount(engine)); + const res = await drive('POST', COMMIT, 'alice@b', { body: { fileId: 'f1', eTag: 'etag-b' } }); + expectOrganizationChanged(res, 'commit from org_b'); + expect(engine._rows('sys_file')[0]).toMatchObject({ status: 'pending', organization_id: 'org_a' }); + expect(engine._rows('sys_file')[0].etag).toBeUndefined(); + // The operator log names the cause: which upload, where it started, where the uploader acts now. + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain("'f1'"); + expect(warnings[0]).toContain("'org_a'"); + expect(warnings[0]).toContain("'org_b'"); + }); + + it('CONTROL — same organization: commits as before, and logs nothing', async () => { + const engine = createFakeEngine(); + await seed(mount(engine)); + const res = await drive('POST', COMMIT, 'alice@a', { body: { fileId: 'f1', eTag: 'etag-a' } }); + expect(res._status).toBe(200); + expect(res._json.data.fileId).toBe('f1'); + expect(engine._rows('sys_file')[0]).toMatchObject({ status: 'committed', etag: 'etag-a' }); + expect(warnings).toEqual([]); + }); + + it('CONTROL — a real engine failure on a same-organization row still answers 500 with the outage text', async () => { + const engine = createFakeEngine(); + await seed(mount(engine)); + engine._setFailing('update', true); + const res = await drive('POST', COMMIT, 'alice@a', { body: { fileId: 'f1', eTag: 'etag-a' } }); + expect(res._status).toBe(500); + expect(res._json.error.code).toBe('INTERNAL'); + expect(res._json.error.message).toContain('did not land'); + expect(res._json.error.message).toContain('Restore the data engine'); + expect(engine._rows('sys_file')[0]).toMatchObject({ status: 'pending' }); + }); + + it('UNCHANGED — no active organization: the write carries no scope, and commits as before', async () => { + const engine = createFakeEngine(); + await seed(mount(engine)); + expect((await drive('POST', COMMIT, 'alice@none', { body: { fileId: 'f1' } }))._status).toBe(200); + expect(engine._rows('sys_file')[0]).toMatchObject({ status: 'committed' }); + }); + + it('UNCHANGED — a row stamped with no organization stays in reach from any organization', async () => { + const engine = createFakeEngine(); + const store = mount(engine); + await store.createFile({ id: 'f0', key: 'user/f0.txt', name: 'f0.txt', status: 'pending', owner_id: 'u_alice' }); + expect((await drive('POST', COMMIT, 'alice@b', { body: { fileId: 'f0' } }))._status).toBe(200); + expect(engine._rows('sys_file')[0]).toMatchObject({ status: 'committed' }); + }); + + it('the ownership rule runs FIRST: a non-uploader acting in another organization learns nothing of it', async () => { + const engine = createFakeEngine(); + await seed(mount(engine)); + const res = await drive('POST', COMMIT, 'carol@b', { body: { fileId: 'f1' } }); + expect(res._status).toBe(403); + expect(res._json.error.code).toBe('PERMISSION_DENIED'); + expect(JSON.stringify(res._json)).not.toContain('organization'); + expect(warnings).toEqual([]); + }); + }); + + describe('the chunked-completion door', () => { + it('answers 409 RESOURCE_CONFLICT naming the change, moves neither row, and logs the cause', async () => { + const engine = createFakeEngine(); + await seed(mount(engine)); + const res = await drive('POST', CHUNKED_COMPLETE, 'alice@b', { params: { uploadId: 'u1' }, body: { parts: [] } }); + expectOrganizationChanged(res, 'chunked completion from org_b'); + expect(engine._rows('sys_upload_session')[0]).toMatchObject({ status: 'in_progress' }); + expect(engine._rows('sys_file')[0]).toMatchObject({ status: 'pending' }); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain("'u1'"); + expect(warnings[0]).toContain("'org_a'"); + expect(warnings[0]).toContain("'org_b'"); + }); + + it('a session row with no organization still answers 409 when its FILE row is out of reach — that write would miss', async () => { + const engine = createFakeEngine(); + await seed(mount(engine), undefined); + const res = await drive('POST', CHUNKED_COMPLETE, 'alice@b', { params: { uploadId: 'u1' }, body: { parts: [] } }); + expectOrganizationChanged(res, 'org-less session, file in org_a'); + expect(engine._rows('sys_upload_session')[0]).toMatchObject({ status: 'in_progress' }); + expect(engine._rows('sys_file')[0]).toMatchObject({ status: 'pending' }); + }); + + it('asked BEFORE the expiry write: an expired session is not stamped from the other organization', async () => { + const engine = createFakeEngine(); + const store = mount(engine); + await seed(store); + await store.updateSession('u1', { expires_at: new Date(Date.now() - 60_000).toISOString() }); + expectOrganizationChanged( + await drive('POST', CHUNKED_COMPLETE, 'alice@b', { params: { uploadId: 'u1' }, body: { parts: [] } }), + 'expired, from org_b', + ); + expect(engine._rows('sys_upload_session')[0]).toMatchObject({ status: 'in_progress' }); + // Control: from its own organization the uploader reaches the expiry branch, which does write. + const home = await drive('POST', CHUNKED_COMPLETE, 'alice@a', { params: { uploadId: 'u1' }, body: { parts: [] } }); + expect(home._status).toBe(410); + expect(engine._rows('sys_upload_session')[0]).toMatchObject({ status: 'expired' }); + }); + + it('CONTROL — same organization: completes as before, and logs nothing', async () => { + const engine = createFakeEngine(); + await seed(mount(engine)); + const res = await drive('POST', CHUNKED_COMPLETE, 'alice@a', { params: { uploadId: 'u1' }, body: { parts: [] } }); + expect(res._status).toBe(200); + expect(engine._rows('sys_upload_session')[0]).toMatchObject({ status: 'completed' }); + expect(engine._rows('sys_file')[0]).toMatchObject({ status: 'committed' }); + expect(warnings).toEqual([]); + }); + + it('CONTROL — a real engine failure on same-organization rows still answers 500 with the outage text', async () => { + const engine = createFakeEngine(); + await seed(mount(engine)); + engine._setFailing('update', true); + const res = await drive('POST', CHUNKED_COMPLETE, 'alice@a', { params: { uploadId: 'u1' }, body: { parts: [] } }); + expect(res._status).toBe(500); + expect(res._json.error.code).toBe('INTERNAL'); + expect(res._json.error.message).toContain('Restore the data engine'); + expect(engine._rows('sys_upload_session')[0]).toMatchObject({ status: 'in_progress' }); + }); + + it('the ownership rule runs FIRST: a non-uploader acting in another organization learns nothing of it', async () => { + const engine = createFakeEngine(); + await seed(mount(engine)); + const res = await drive('POST', CHUNKED_COMPLETE, 'carol@b', { params: { uploadId: 'u1' }, body: { parts: [] } }); + expect(res._status).toBe(403); + expect(res._json.error.code).toBe('PERMISSION_DENIED'); + expect(JSON.stringify(res._json)).not.toContain('organization'); + expect(warnings).toEqual([]); + }); + }); + + describe('unchanged', () => { + it('the progress door still answers the uploader after the switch — it is a read', async () => { + const engine = createFakeEngine(); + await seed(mount(engine)); + const res = await drive('GET', PROGRESS, 'alice@b', { params: { uploadId: 'u1' } }); + expect(res._status).toBe(200); + expect(res._json.data.fileId).toBe('f1'); + expect(warnings).toEqual([]); + }); + + it('the engine-absent stand-in does not scope, so both doors finish the upload from any organization as before', async () => { + const store = mount(null); + await seed(store); + expect((await drive('POST', COMMIT, 'alice@b', { body: { fileId: 'f1' } }))._status).toBe(200); + expect((await store.getFile('f1'))?.status).toBe('committed'); + const chunked = await drive('POST', CHUNKED_COMPLETE, 'alice@b', { params: { uploadId: 'u1' }, body: { parts: [] } }); + expect(chunked._status).toBe(200); + expect((await store.getSession('u1'))?.status).toBe('completed'); + expect(warnings).toEqual([]); + }); + }); +}); diff --git a/packages/services/service-storage/src/storage-routes.ts b/packages/services/service-storage/src/storage-routes.ts index 3ea485cd37f..fd950d45ec2 100644 --- a/packages/services/service-storage/src/storage-routes.ts +++ b/packages/services/service-storage/src/storage-routes.ts @@ -2,6 +2,7 @@ import { randomUUID } from 'node:crypto'; import type { IHttpServer, IHttpRequest, IHttpResponse, IStorageService } from '@objectstack/spec/contracts'; +import type { StandardErrorCode } from '@objectstack/spec/api'; // The declared envelope is written in ONE place for the whole platform (#3973). import { sendOk, sendError } from '@objectstack/types'; // [#15999] The BRAND predicate, never `instanceof` — this error crosses package @@ -16,6 +17,9 @@ import type { UploadSessionRecord, StorageWriteContext, } from './metadata-store.js'; +// [#22175] Whether a door's scoped by-id write can reach the row it names — +// the store's own answer, asked before the write. +import { organizationOutOfWriteReach } from './metadata-store.js'; import type { LocalStorageAdapter } from './local-storage-adapter.js'; // [#22046] The ONE upload ownership rule. Declared in its own module and only // CALLED here — the three doors below share it rather than each carrying a @@ -35,6 +39,27 @@ import { contentDispositionValue } from './content-disposition.js'; /** Authorization verdict for an attachments-scope download (#2970 item 2). */ export type FileReadVerdict = 'allow' | 'deny' | 'unauthenticated'; +/** + * [#22175] The answer the commit and chunked-completion doors give an uploader + * whose active organization is no longer the one the upload was started in — + * see `requireStartingOrganization` inside {@link registerStorageRoutes}. + * + * `409` / `RESOURCE_CONFLICT`, the standard-catalog member HTTP 409 derives + * (ADR-0112; no storage extension code is registered for this): the request + * conflicts with where the upload stands, and the caller can resolve it — the + * same call succeeds once they switch back. Not `403 PERMISSION_DENIED`: that + * code is the ownership rule's one refusal, and this caller IS the uploader. + */ +const ORGANIZATION_CHANGED_STATUS = 409; +const ORGANIZATION_CHANGED_CODE: StandardErrorCode = 'RESOURCE_CONFLICT'; +/** + * Constant, and naming no organization: it tells the uploader what changed and + * what finishes the upload. The organization ids go to the operator log only. + */ +const ORGANIZATION_CHANGED_MESSAGE = + 'This upload was started in a different organization than your active one. ' + + 'Switch your active organization back to the one the upload was started in, then finish the upload.'; + /** * What the upload routes need to know about the caller (#2755, widened #12745). * @@ -343,6 +368,60 @@ export function registerStorageRoutes( return false; }; + // ── Starting-organization gate (#22175) ────────────────────────────── + // The commit and chunked-completion doors write by id under the ACTING + // organization (`StorageWriteContext`), and on an engine-backed store that + // write is SCOPED to it: the driver's statement reaches a row stamped for + // the acting organization or for none, and no other. An uploader who + // switched their active organization after starting an upload therefore + // names a row the write cannot reach, and that scoped miss used to surface + // as `500 INTERNAL` carrying the store's outage text ("Restore the data + // engine…") — an organization change diagnosed, to the caller and to the + // operator, as a data-engine fault. + // + // Each door now asks this right after the ownership rule (so only the + // uploader ever learns of it) and BEFORE any write, the expiry stamp + // included. `false` ⇒ the 409 was already sent and the handler must stop. + // + // The question is the write's own reach and nothing wider, and the store + // answers it (`organizationOutOfWriteReach`) for the write context the door + // is about to pass: open mode and a session with no active organization + // thread no scope; a row stamped with no organization stays in reach; the + // engine-absent stand-in does not scope at all, so it never refuses here and + // its answers are unchanged. Every row the door is about to write is asked — + // the chunked door writes the upload session AND its file, and either one + // out of reach means the write misses. + // + // ⛔ It changes nothing about where an upload belongs: it asks, it never + // re-stamps, and the write context the doors pass is untouched — switching + // back finishes the upload in the organization it was started in. + // ⛔ It is not a `catch`: a real engine failure on a row in the caller's own + // organization still reaches the door's outer `catch` and its `500`. + const requireStartingOrganization = ( + writeContext: StorageWriteContext, + rows: ReadonlyArray<{ organization_id?: string | null } | null>, + door: string, + upload: string, + res: IHttpResponse, + ): boolean => { + for (const row of rows) { + const started = organizationOutOfWriteReach(store, row, writeContext); + if (started === null) continue; + // The operator log names the cause — a `4xx` is otherwise quiet in the + // server-fault log, which used to carry the misleading outage line. + opts.logger?.warn( + `[storage] ${door}: refused upload '${upload}' with ${ORGANIZATION_CHANGED_STATUS} ` + + `${ORGANIZATION_CHANGED_CODE} — it was started in organization '${started}', and the ` + + `uploader's active organization is now '${writeContext.organizationId}'. The door writes by ` + + 'id under the active organization, which cannot reach a row stamped for another one: an ' + + `organization change, not a data-engine fault. Switching back to '${started}' finishes the upload.`, + ); + sendError(res, ORGANIZATION_CHANGED_STATUS, ORGANIZATION_CHANGED_CODE, ORGANIZATION_CHANGED_MESSAGE); + return false; + } + return true; + }; + // ── Terminal upload-session statuses (#7667) ───────────────────────────── // `sys_upload_session.status` declares `failed` and `expired`, the retention // backstop reaps on them (`onlyWhen: { status: { $in: ['completed', @@ -509,6 +588,10 @@ export function registerStorageRoutes( // [#22046] Only the uploader commits — before the write below and // before anything about the row is answered. if (!requireUploader(session, file, res)) return; + // [#22175] …and only while the scoped write below can reach the row — + // asked of the SAME write context that write is about to carry. + const writeContext: StorageWriteContext = { organizationId: session?.organizationId }; + if (!requireStartingOrganization(writeContext, [file], 'commit', fileId, res)) return; const updated = await store.updateFile( fileId, @@ -516,7 +599,7 @@ export function registerStorageRoutes( status: 'committed', etag: eTag ?? undefined, }, - { organizationId: session?.organizationId }, + writeContext, ); sendOk(res, { @@ -734,7 +817,11 @@ export function registerStorageRoutes( // [#22046] The session row names no user; it reaches its uploader // through `file_id`. Asked before the expiry stamp, the status writes // and the completion answer below. - if (!requireUploader(authSession, await store.getFile(session.file_id), res)) return; + const file = await store.getFile(session.file_id); + if (!requireUploader(authSession, file, res)) return; + // [#22175] Both rows this door writes must be in the scoped writes' + // reach — asked before the expiry stamp, which is the first of them. + if (!requireStartingOrganization(writeContext, [session, file], 'chunked completion', uploadId, res)) return; const live = await expireIfPastDeadline(session, writeContext); if (live.status === 'expired') { From af2811c51c814a5cca6ce315b3ff8fe0ca01c88e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 06:09:08 +0000 Subject: [PATCH 2/2] test(service-storage): open the seeded chunked sessions on the real adapter so a same-organization completion assembles Claude-Session: https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ Co-authored-by: Claude --- .../storage-routes.metadata-outage.test.ts | 34 +++++++++++-------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/packages/services/service-storage/src/storage-routes.metadata-outage.test.ts b/packages/services/service-storage/src/storage-routes.metadata-outage.test.ts index 70cfd095bcd..153793078b0 100644 --- a/packages/services/service-storage/src/storage-routes.metadata-outage.test.ts +++ b/packages/services/service-storage/src/storage-routes.metadata-outage.test.ts @@ -320,8 +320,14 @@ describe('Storage routes: an organization change is not an outage (#22175)', () await httpServer._getHandler(method, path)!(createMockReq({ ...req, headers: { 'x-test-as': as } }), res); return res; }; - /** A pending upload Alice started in `org_a`: its file row and, for the chunked door, its session. */ + /** + * A pending upload Alice started in `org_a`: its file row and, for the + * chunked door, its session — opened on the real adapter, so a completion + * that gets past the doors' questions really assembles. Sets {@link uploadId}. + */ + let uploadId: string; const seed = async (store: StorageMetadataStore, sessionOrganization: string | undefined = 'org_a') => { + uploadId = await adapter.initiateChunkedUpload('user/f1.bin', { contentType: 'application/octet-stream' }); await store.createFile({ id: 'f1', key: 'user/f1.bin', @@ -331,7 +337,7 @@ describe('Storage routes: an organization change is not an outage (#22175)', () organization_id: 'org_a', }); await store.createSession({ - id: 'u1', + id: uploadId, file_id: 'f1', key: 'user/f1.bin', filename: 'f1.bin', @@ -430,12 +436,12 @@ describe('Storage routes: an organization change is not an outage (#22175)', () it('answers 409 RESOURCE_CONFLICT naming the change, moves neither row, and logs the cause', async () => { const engine = createFakeEngine(); await seed(mount(engine)); - const res = await drive('POST', CHUNKED_COMPLETE, 'alice@b', { params: { uploadId: 'u1' }, body: { parts: [] } }); + const res = await drive('POST', CHUNKED_COMPLETE, 'alice@b', { params: { uploadId }, body: { parts: [] } }); expectOrganizationChanged(res, 'chunked completion from org_b'); expect(engine._rows('sys_upload_session')[0]).toMatchObject({ status: 'in_progress' }); expect(engine._rows('sys_file')[0]).toMatchObject({ status: 'pending' }); expect(warnings).toHaveLength(1); - expect(warnings[0]).toContain("'u1'"); + expect(warnings[0]).toContain(`'${uploadId}'`); expect(warnings[0]).toContain("'org_a'"); expect(warnings[0]).toContain("'org_b'"); }); @@ -443,7 +449,7 @@ describe('Storage routes: an organization change is not an outage (#22175)', () it('a session row with no organization still answers 409 when its FILE row is out of reach — that write would miss', async () => { const engine = createFakeEngine(); await seed(mount(engine), undefined); - const res = await drive('POST', CHUNKED_COMPLETE, 'alice@b', { params: { uploadId: 'u1' }, body: { parts: [] } }); + const res = await drive('POST', CHUNKED_COMPLETE, 'alice@b', { params: { uploadId }, body: { parts: [] } }); expectOrganizationChanged(res, 'org-less session, file in org_a'); expect(engine._rows('sys_upload_session')[0]).toMatchObject({ status: 'in_progress' }); expect(engine._rows('sys_file')[0]).toMatchObject({ status: 'pending' }); @@ -453,14 +459,14 @@ describe('Storage routes: an organization change is not an outage (#22175)', () const engine = createFakeEngine(); const store = mount(engine); await seed(store); - await store.updateSession('u1', { expires_at: new Date(Date.now() - 60_000).toISOString() }); + await store.updateSession(uploadId, { expires_at: new Date(Date.now() - 60_000).toISOString() }); expectOrganizationChanged( - await drive('POST', CHUNKED_COMPLETE, 'alice@b', { params: { uploadId: 'u1' }, body: { parts: [] } }), + await drive('POST', CHUNKED_COMPLETE, 'alice@b', { params: { uploadId }, body: { parts: [] } }), 'expired, from org_b', ); expect(engine._rows('sys_upload_session')[0]).toMatchObject({ status: 'in_progress' }); // Control: from its own organization the uploader reaches the expiry branch, which does write. - const home = await drive('POST', CHUNKED_COMPLETE, 'alice@a', { params: { uploadId: 'u1' }, body: { parts: [] } }); + const home = await drive('POST', CHUNKED_COMPLETE, 'alice@a', { params: { uploadId }, body: { parts: [] } }); expect(home._status).toBe(410); expect(engine._rows('sys_upload_session')[0]).toMatchObject({ status: 'expired' }); }); @@ -468,7 +474,7 @@ describe('Storage routes: an organization change is not an outage (#22175)', () it('CONTROL — same organization: completes as before, and logs nothing', async () => { const engine = createFakeEngine(); await seed(mount(engine)); - const res = await drive('POST', CHUNKED_COMPLETE, 'alice@a', { params: { uploadId: 'u1' }, body: { parts: [] } }); + const res = await drive('POST', CHUNKED_COMPLETE, 'alice@a', { params: { uploadId }, body: { parts: [] } }); expect(res._status).toBe(200); expect(engine._rows('sys_upload_session')[0]).toMatchObject({ status: 'completed' }); expect(engine._rows('sys_file')[0]).toMatchObject({ status: 'committed' }); @@ -479,7 +485,7 @@ describe('Storage routes: an organization change is not an outage (#22175)', () const engine = createFakeEngine(); await seed(mount(engine)); engine._setFailing('update', true); - const res = await drive('POST', CHUNKED_COMPLETE, 'alice@a', { params: { uploadId: 'u1' }, body: { parts: [] } }); + const res = await drive('POST', CHUNKED_COMPLETE, 'alice@a', { params: { uploadId }, body: { parts: [] } }); expect(res._status).toBe(500); expect(res._json.error.code).toBe('INTERNAL'); expect(res._json.error.message).toContain('Restore the data engine'); @@ -489,7 +495,7 @@ describe('Storage routes: an organization change is not an outage (#22175)', () it('the ownership rule runs FIRST: a non-uploader acting in another organization learns nothing of it', async () => { const engine = createFakeEngine(); await seed(mount(engine)); - const res = await drive('POST', CHUNKED_COMPLETE, 'carol@b', { params: { uploadId: 'u1' }, body: { parts: [] } }); + const res = await drive('POST', CHUNKED_COMPLETE, 'carol@b', { params: { uploadId }, body: { parts: [] } }); expect(res._status).toBe(403); expect(res._json.error.code).toBe('PERMISSION_DENIED'); expect(JSON.stringify(res._json)).not.toContain('organization'); @@ -501,7 +507,7 @@ describe('Storage routes: an organization change is not an outage (#22175)', () it('the progress door still answers the uploader after the switch — it is a read', async () => { const engine = createFakeEngine(); await seed(mount(engine)); - const res = await drive('GET', PROGRESS, 'alice@b', { params: { uploadId: 'u1' } }); + const res = await drive('GET', PROGRESS, 'alice@b', { params: { uploadId } }); expect(res._status).toBe(200); expect(res._json.data.fileId).toBe('f1'); expect(warnings).toEqual([]); @@ -512,9 +518,9 @@ describe('Storage routes: an organization change is not an outage (#22175)', () await seed(store); expect((await drive('POST', COMMIT, 'alice@b', { body: { fileId: 'f1' } }))._status).toBe(200); expect((await store.getFile('f1'))?.status).toBe('committed'); - const chunked = await drive('POST', CHUNKED_COMPLETE, 'alice@b', { params: { uploadId: 'u1' }, body: { parts: [] } }); + const chunked = await drive('POST', CHUNKED_COMPLETE, 'alice@b', { params: { uploadId }, body: { parts: [] } }); expect(chunked._status).toBe(200); - expect((await store.getSession('u1'))?.status).toBe('completed'); + expect((await store.getSession(uploadId))?.status).toBe('completed'); expect(warnings).toEqual([]); }); });