diff --git a/.changeset/22218-chunk-progress-org-change-4xx.md b/.changeset/22218-chunk-progress-org-change-4xx.md new file mode 100644 index 00000000000..b80f0c5098f --- /dev/null +++ b/.changeset/22218-chunk-progress-org-change-4xx.md @@ -0,0 +1,14 @@ +--- +'@objectstack/service-storage': patch +--- + +fix(service-storage): sending a chunk, or polling an expired upload's progress, after switching active organization answers `409 RESOURCE_CONFLICT` naming the change, not a `500` that reads as a data-engine outage + +Clause-②: no + +`PUT /api/v1/storage/upload/chunked/:uploadId/chunk/:chunkIndex` records each chunk on the upload session, and `GET /api/v1/storage/upload/chunked/:uploadId/progress` stamps a session past its `expires_at` as `expired`. Both writes go by id under the caller's active organization, and they are scoped to it. When the uploader's active organization is no longer the one the upload was started in, the write cannot reach the session row. Both doors used to answer that with `500 INTERNAL` and a message diagnosing a data-engine outage ("Restore the data engine …"). + +- **Now:** each door answers `409 RESOURCE_CONFLICT` in the standard error envelope, with the same message the commit and chunked-completion doors give: the upload was started in a different organization than the active one, and switching back finishes it. The chunk door answers it before the chunk reaches the storage backend and before any write. The progress door answers it only when the expiry stamp is due, before stamping. The server log carries one warning naming the door, 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 chunk or a progress read from the starting organization; the progress of a live session, or of one already `expired`, which is a read and still answers `200` from any organization; a real data-engine failure, which still answers `500` with its consequence and fix; the resume-token check on the chunk door and the uploader-only rule on the progress door, which run first, so a caller who cannot act on the upload learns nothing about organizations; sessions stamped with no organization; routes with no session resolver; and a store with no data engine wired. + +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 index 3bd5525be1c..746fdadcf22 100644 --- a/packages/plugins/organizations/src/storage-upload-organization-change.wall.test.ts +++ b/packages/plugins/organizations/src/storage-upload-organization-change.wall.test.ts @@ -16,6 +16,15 @@ * organization an upload belongs to is NOT changed: switching back finishes it, * from where it started. * + * #22218 asks the same question at two more doors. The chunk door + * (`PUT …/upload/chunked/:uploadId/chunk/:chunkIndex`) writes the session's + * progress after every chunk, and the progress door + * (`GET …/upload/chunked/:uploadId/progress`) stamps a session past its + * `expires_at` as `expired`. Both writes are scoped the same way and used to + * answer the same `500`. The progress door asks only when that stamp is due: a + * live session's progress is a read, and it still answers the uploader from + * any organization. + * * 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 @@ -88,7 +97,7 @@ describe('the upload doors after the uploader switches organization, on a booted expect(res.status).toBe(200); return res.body.data.fileId; }; - const startChunked = async (): Promise<{ uploadId: string; fileId: string }> => { + const startChunked = async (): Promise<{ uploadId: string; fileId: string; resumeToken: string }> => { const res = await answer( await stack.apiAs(uploader, 'POST', '/storage/upload/chunked', { filename: 'org-change-chunked.bin', @@ -97,7 +106,40 @@ describe('the upload doors after the uploader switches organization, on a booted }), ); expect(res.status).toBe(200); - return { uploadId: res.body.data.uploadId, fileId: res.body.data.fileId }; + return { + uploadId: res.body.data.uploadId, + fileId: res.body.data.fileId, + resumeToken: res.body.data.resumeToken, + }; + }; + /** One binary chunk, with the resume token the start door handed back. */ + const putChunk = (uploadId: string, resumeToken: string) => + stack.api(`/storage/upload/chunked/${uploadId}/chunk/0`, { + method: 'PUT', + headers: { + Authorization: `Bearer ${uploader}`, + 'Content-Type': 'application/octet-stream', + 'x-resume-token': resumeToken, + }, + body: new TextEncoder().encode('chunk-0'), + }); + const progress = (uploadId: string) => + stack.apiAs(uploader, 'GET', `/storage/upload/chunked/${uploadId}/progress`); + /** + * Moves a session's own `expires_at` into the past, as SYSTEM and in the + * organization it was started in, so its expiry stamp is due. The row's + * organization is untouched. + */ + const pastDeadline = async (uploadId: string): Promise => { + await ql.update( + 'sys_upload_session', + { expires_at: new Date(Date.now() - 60_000).toISOString() }, + { where: { id: uploadId }, context: { isSystem: true, tenantId: startOrg } }, + ); + const row = await stored('sys_upload_session', uploadId); + expect(Date.parse(row.expires_at)).toBeLessThan(Date.now()); + expect(row.status).toBe('in_progress'); + expect(row.organization_id).toBe(startOrg); }; /** @@ -251,4 +293,64 @@ describe('the upload doors after the uploader switches organization, on a booted expect(progress.body.data.status).toBe('in_progress'); await switchTo(startOrg); }); + + describe('the chunk door (#22218)', () => { + it('ORGANIZATION CHANGED: 409 RESOURCE_CONFLICT naming the change, not 500; the session is unchanged; switching back lands the chunk', async () => { + await switchTo(startOrg); + const { uploadId, resumeToken } = await startChunked(); + const before = await stored('sys_upload_session', uploadId); + + await switchTo(otherOrg); + expectOrganizationChanged(await answer(await putChunk(uploadId, resumeToken)), 'chunk after switching'); + expect(await stored('sys_upload_session', uploadId)).toEqual(before); + + // The upload still belongs where it started, and takes its chunks there. + await switchTo(startOrg); + const home = await answer(await putChunk(uploadId, resumeToken)); + expect(home.status, JSON.stringify(home.body)).toBe(200); + const row = await stored('sys_upload_session', uploadId); + expect(row.uploaded_chunks).toBe(1); + expect(row.organization_id).toBe(startOrg); + }); + + it('CONTROL — SAME ORGANIZATION: the uploader sends a chunk exactly as before', async () => { + await switchTo(startOrg); + const { uploadId, resumeToken } = await startChunked(); + const own = await answer(await putChunk(uploadId, resumeToken)); + expect(own.status, JSON.stringify(own.body)).toBe(200); + expect(own.body.data.chunkIndex).toBe(0); + expect((await stored('sys_upload_session', uploadId)).uploaded_chunks).toBe(1); + }); + }); + + describe('the progress door, once the expiry stamp is due (#22218)', () => { + it('ORGANIZATION CHANGED: 409 RESOURCE_CONFLICT naming the change, not 500; the session is not stamped; switching back reports it expired', async () => { + await switchTo(startOrg); + const { uploadId } = await startChunked(); + await pastDeadline(uploadId); + const before = await stored('sys_upload_session', uploadId); + + await switchTo(otherOrg); + expectOrganizationChanged(await answer(await progress(uploadId)), 'progress past the deadline after switching'); + expect(await stored('sys_upload_session', uploadId)).toEqual(before); + + await switchTo(startOrg); + const home = await answer(await progress(uploadId)); + expect(home.status, JSON.stringify(home.body)).toBe(200); + expect(home.body.data.status).toBe('expired'); + const row = await stored('sys_upload_session', uploadId); + expect(row.status).toBe('expired'); + expect(row.organization_id).toBe(startOrg); + }); + + it('CONTROL — SAME ORGANIZATION: the uploader is told `expired`, and the session is stamped, exactly as before', async () => { + await switchTo(startOrg); + const { uploadId } = await startChunked(); + await pastDeadline(uploadId); + const own = await answer(await progress(uploadId)); + expect(own.status, JSON.stringify(own.body)).toBe(200); + expect(own.body.data.status).toBe('expired'); + expect((await stored('sys_upload_session', uploadId)).status).toBe('expired'); + }); + }); }); 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 153793078b0..0d134b24131 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 @@ -280,6 +280,10 @@ describe('Storage routes: a metadata write that never landed is not a 200 (#5216 // store's `organizationOutOfWriteReach` first and answer `409 // RESOURCE_CONFLICT`, naming the change; the operator log names the cause. // +// #22218 asks the same question at the two doors #22175 left: the chunk door, +// whose session write follows the chunk, and the progress door, whose one write +// is the expiry stamp — asked there only when the stamp is due. +// // 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 @@ -289,6 +293,7 @@ describe('Storage routes: a metadata write that never landed is not a 200 (#5216 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 CHUNK = '/api/v1/storage/upload/chunked/:uploadId/chunk/:chunkIndex'; 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.'; @@ -317,16 +322,23 @@ describe('Storage routes: an organization change is not an outage (#22175)', () }; 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); + await httpServer._getHandler(method, path)!( + createMockReq({ ...req, headers: { ...(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 — opened on the real adapter, so a completion * that gets past the doors' questions really assembles. Sets {@link uploadId}. + * + * `null` seeds the session with NO organization. Not `undefined`: an + * `undefined` argument takes the parameter's default, so it would seed + * `org_a` and a pin about an org-less session would pin nothing. */ let uploadId: string; - const seed = async (store: StorageMetadataStore, sessionOrganization: string | undefined = 'org_a') => { + const seed = async (store: StorageMetadataStore, sessionOrganization: string | null = 'org_a') => { uploadId = await adapter.initiateChunkedUpload('user/f1.bin', { contentType: 'application/octet-stream' }); await store.createFile({ id: 'f1', @@ -345,7 +357,7 @@ describe('Storage routes: an organization change is not an outage (#22175)', () chunk_size: 50, total_chunks: 2, status: 'in_progress', - ...(sessionOrganization ? { organization_id: sessionOrganization } : {}), + ...(sessionOrganization !== null ? { organization_id: sessionOrganization } : {}), }); }; const expectOrganizationChanged = (res: ReturnType, label: string) => { @@ -356,6 +368,16 @@ describe('Storage routes: an organization change is not an outage (#22175)', () expect(res._json.data, seen).toBeUndefined(); expect(String(res._json.error.message).startsWith(FIRST_SENTENCE), seen).toBe(true); }; + /** Moves the seeded session's own deadline into the past, so its expiry stamp is due. */ + const pastDeadline = (store: StorageMetadataStore) => + store.updateSession(uploadId, { expires_at: new Date(Date.now() - 60_000).toISOString() }); + /** One chunk of the seeded upload, sent as `as`. */ + const sendChunk = (as: string, headers: Record = {}) => + drive('PUT', CHUNK, as, { + params: { uploadId, chunkIndex: '0' }, + headers, + rawBody: async () => Buffer.from('chunk-0'), + }); beforeEach(async () => { rootDir = join(tmpdir(), `os-22175-${Date.now()}-${Math.random().toString(36).slice(2)}`); @@ -448,7 +470,8 @@ 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); + await seed(mount(engine), null); + expect(engine._rows('sys_upload_session')[0].organization_id).toBeUndefined(); 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' }); @@ -503,6 +526,162 @@ describe('Storage routes: an organization change is not an outage (#22175)', () }); }); + describe('the chunk door (#22218)', () => { + it('answers 409 RESOURCE_CONFLICT naming the change, sends no chunk to the backend, leaves the session, and logs the cause', async () => { + const engine = createFakeEngine(); + await seed(mount(engine)); + const before = { ...engine._rows('sys_upload_session')[0] }; + const uploadChunk = vi.spyOn(adapter, 'uploadChunk'); + const res = await sendChunk('alice@b'); + expectOrganizationChanged(res, 'chunk from org_b'); + expect(uploadChunk).not.toHaveBeenCalled(); + expect(engine._rows('sys_upload_session')[0]).toEqual(before); + expect(before).toMatchObject({ status: 'in_progress', organization_id: 'org_a', uploaded_chunks: 0 }); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain('chunk:'); + expect(warnings[0]).toContain(`'${uploadId}'`); + expect(warnings[0]).toContain("'org_a'"); + expect(warnings[0]).toContain("'org_b'"); + }); + + 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 pastDeadline(store); + expectOrganizationChanged(await sendChunk('alice@b'), 'expired, chunk 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 sendChunk('alice@a'); + expect(home._status).toBe(410); + expect(home._json.error.code).toBe('UPLOAD_SESSION_EXPIRED'); + expect(engine._rows('sys_upload_session')[0]).toMatchObject({ status: 'expired' }); + }); + + it('CONTROL — same organization: the chunk lands as before, and logs nothing', async () => { + const engine = createFakeEngine(); + await seed(mount(engine)); + const res = await sendChunk('alice@a'); + expect(res._status).toBe(200); + expect(res._json.data).toMatchObject({ chunkIndex: 0, bytesReceived: 7 }); + expect(engine._rows('sys_upload_session')[0]).toMatchObject({ status: 'in_progress', uploaded_chunks: 1 }); + expect(warnings).toEqual([]); + }); + + it('CONTROL — a real engine failure on a same-organization session still answers 500 with the outage text', async () => { + const engine = createFakeEngine(); + await seed(mount(engine)); + engine._setFailing('update', true); + const res = await sendChunk('alice@a'); + expect(res._status).toBe(500); + expect(res._json.error.code).toBe('INTERNAL'); + expect(res._json.error.message).toContain('persisted upload progress is stale'); + expect(res._json.error.message).toContain('Restore the data engine'); + expect(warnings).toEqual([]); + }); + + it('UNCHANGED — a session row with no organization stays in reach: this door writes no file row', async () => { + const engine = createFakeEngine(); + await seed(mount(engine), null); + expect(engine._rows('sys_upload_session')[0].organization_id).toBeUndefined(); + expect((await sendChunk('alice@b'))._status).toBe(200); + expect(engine._rows('sys_upload_session')[0]).toMatchObject({ uploaded_chunks: 1 }); + }); + + it('the resume token runs FIRST: a caller without it, acting in another organization, learns nothing of it', async () => { + const engine = createFakeEngine(); + const store = mount(engine); + await seed(store); + await store.updateSession(uploadId, { resume_token: 'token-a' }); + const res = await sendChunk('alice@b', { 'x-resume-token': 'wrong' }); + expect(res._status).toBe(403); + expect(res._json.error.code).toBe('INVALID_RESUME_TOKEN'); + expect(JSON.stringify(res._json)).not.toContain('organization'); + expect(warnings).toEqual([]); + // With the token, the same caller is told what changed. + expectOrganizationChanged(await sendChunk('alice@b', { 'x-resume-token': 'token-a' }), 'chunk with token'); + }); + }); + + describe('the progress door, once the expiry stamp is due (#22218)', () => { + it('answers 409 RESOURCE_CONFLICT naming the change, does not stamp the session, and logs the cause', async () => { + const engine = createFakeEngine(); + const store = mount(engine); + await seed(store); + await pastDeadline(store); + const res = await drive('GET', PROGRESS, 'alice@b', { params: { uploadId } }); + expectOrganizationChanged(res, 'progress past the deadline from org_b'); + expect(engine._rows('sys_upload_session')[0]).toMatchObject({ status: 'in_progress', organization_id: 'org_a' }); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain('progress:'); + expect(warnings[0]).toContain(`'${uploadId}'`); + expect(warnings[0]).toContain("'org_a'"); + expect(warnings[0]).toContain("'org_b'"); + }); + + it('CONTROL — same organization: reports and stamps `expired` as before, and logs nothing', async () => { + const engine = createFakeEngine(); + const store = mount(engine); + await seed(store); + await pastDeadline(store); + const res = await drive('GET', PROGRESS, 'alice@a', { params: { uploadId } }); + expect(res._status).toBe(200); + expect(res._json.data.status).toBe('expired'); + expect(engine._rows('sys_upload_session')[0]).toMatchObject({ status: 'expired' }); + expect(warnings).toEqual([]); + }); + + it('CONTROL — a real engine failure on the same-organization expiry stamp still answers 500 with the outage text', async () => { + const engine = createFakeEngine(); + const store = mount(engine); + await seed(store); + await pastDeadline(store); + engine._setFailing('update', true); + const res = await drive('GET', PROGRESS, 'alice@a', { params: { uploadId } }); + 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('asked only when the stamp is due: a session already `expired` is reported from the other organization, as a read', async () => { + const engine = createFakeEngine(); + const store = mount(engine); + await seed(store); + await pastDeadline(store); + await store.updateSession(uploadId, { status: 'expired' }); + const res = await drive('GET', PROGRESS, 'alice@b', { params: { uploadId } }); + expect(res._status).toBe(200); + expect(res._json.data.status).toBe('expired'); + expect(warnings).toEqual([]); + }); + + it('UNCHANGED — a session row with no organization stays in reach: stamped `expired` from any organization', async () => { + const engine = createFakeEngine(); + const store = mount(engine); + await seed(store, null); + expect(engine._rows('sys_upload_session')[0].organization_id).toBeUndefined(); + await pastDeadline(store); + const res = await drive('GET', PROGRESS, 'alice@b', { params: { uploadId } }); + expect(res._status).toBe(200); + expect(res._json.data.status).toBe('expired'); + expect(engine._rows('sys_upload_session')[0]).toMatchObject({ status: 'expired' }); + }); + + it('the ownership rule runs FIRST: a non-uploader acting in another organization learns nothing of it', async () => { + const engine = createFakeEngine(); + const store = mount(engine); + await seed(store); + await pastDeadline(store); + const res = await drive('GET', PROGRESS, 'carol@b', { params: { uploadId } }); + expect(res._status).toBe(403); + expect(res._json.error.code).toBe('PERMISSION_DENIED'); + expect(JSON.stringify(res._json)).not.toContain('organization'); + expect(engine._rows('sys_upload_session')[0]).toMatchObject({ status: 'in_progress' }); + expect(warnings).toEqual([]); + }); + }); + describe('unchanged', () => { it('the progress door still answers the uploader after the switch — it is a read', async () => { const engine = createFakeEngine(); @@ -523,5 +702,18 @@ describe('Storage routes: an organization change is not an outage (#22175)', () expect((await store.getSession(uploadId))?.status).toBe('completed'); expect(warnings).toEqual([]); }); + + it('the engine-absent stand-in: the chunk door and the progress door\'s expiry stamp answer from any organization as before (#22218)', async () => { + const store = mount(null); + await seed(store); + expect((await sendChunk('alice@b'))._status).toBe(200); + expect((await store.getSession(uploadId))?.uploaded_chunks).toBe(1); + await pastDeadline(store); + const progress = await drive('GET', PROGRESS, 'alice@b', { params: { uploadId } }); + expect(progress._status).toBe(200); + expect(progress._json.data.status).toBe('expired'); + expect((await store.getSession(uploadId))?.status).toBe('expired'); + expect(warnings).toEqual([]); + }); }); }); diff --git a/packages/services/service-storage/src/storage-routes.ts b/packages/services/service-storage/src/storage-routes.ts index fd950d45ec2..5fe0c1aed09 100644 --- a/packages/services/service-storage/src/storage-routes.ts +++ b/packages/services/service-storage/src/storage-routes.ts @@ -43,6 +43,8 @@ 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}. + * [#22218] The chunk door gives it too, and so does the progress door when the + * session's expiry stamp is due. * * `409` / `RESOURCE_CONFLICT`, the standard-catalog member HTTP 409 derives * (ADR-0112; no storage extension code is registered for this): the request @@ -368,20 +370,25 @@ 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. + // ── Starting-organization gate (#22175, #22218) ────────────────────── + // The commit, chunked-completion and chunk doors write by id under the + // ACTING organization (`StorageWriteContext`), and so does the progress + // door's expiry stamp. 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. + // Each door asks this right after the check that proves the caller may act + // on the upload — the ownership rule, or on the chunk door the resume token + // — so only that caller ever learns of it, and BEFORE its first write: the + // expiry stamp, and on the chunk door the backend chunk as well. The + // progress door asks only when the stamp is due, because a progress read + // writes nothing else and has no reach to miss. `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 @@ -450,27 +457,44 @@ export function registerStorageRoutes( const TERMINAL_SESSION_STATUSES = new Set(['completed', 'failed', 'expired']); /** - * Status an in-flight session `expired` once it is past its own `expires_at`, - * and hand back the row as it now stands. + * Whether an in-flight session is past its own `expires_at`, so that the + * `expired` stamp is due. * * A row with no `expires_at` (or an unparseable one) has no declared deadline * and is left alone: this enforces the deadline the session itself carries, - * it does not invent one. Terminal rows are returned untouched — a - * `completed` upload does not become `expired` by sitting around. + * it does not invent one. Terminal rows are never due — a `completed` upload + * does not become `expired` by sitting around. */ - const expireIfPastDeadline = async ( + const expiryStampDue = (session: UploadSessionRecord): boolean => { + if (TERMINAL_SESSION_STATUSES.has(session.status)) return false; + const deadline = session.expires_at ? Date.parse(session.expires_at) : NaN; + return Number.isFinite(deadline) && deadline <= Date.now(); + }; + + /** + * Status the session `expired` — the write {@link expiryStampDue} says is + * due — and hand back the row as it now stands. + */ + const stampExpired = async ( session: UploadSessionRecord, context?: StorageWriteContext, ): Promise => { - if (TERMINAL_SESSION_STATUSES.has(session.status)) return session; - const deadline = session.expires_at ? Date.parse(session.expires_at) : NaN; - if (!Number.isFinite(deadline) || deadline > Date.now()) return session; const updated = await store.updateSession(session.id, { status: 'expired' }, context); // `updateSession` answers null only when the row went away under us (the // TTL sweep, most likely) — the caller is refused either way. return updated ?? { ...session, status: 'expired' }; }; + /** + * Status an in-flight session `expired` once it is past its own `expires_at`, + * and hand back the row as it now stands. + */ + const expireIfPastDeadline = async ( + session: UploadSessionRecord, + context?: StorageWriteContext, + ): Promise => + expiryStampDue(session) ? stampExpired(session, context) : session; + /** * Best-effort `failed` stamp for a completion that threw. * @@ -742,6 +766,13 @@ export function registerStorageRoutes( return; } + // [#22218] This door writes the session row — the expiry stamp below, + // then the progress after the chunk — so that row must be in the scoped + // writes' reach. Asked after the resume token, for the same reason the + // expiry check is, and before the expiry stamp and the backend chunk, so + // a refused chunk lands nowhere. + if (!requireStartingOrganization(writeContext, [session], 'chunk', uploadId, res)) return; + // Expiry is checked AFTER the resume token: a caller who cannot prove it // owns the session learns nothing about its state (#7667). const live = await expireIfPastDeadline(session, writeContext); @@ -898,11 +929,19 @@ export function registerStorageRoutes( // expiry write below and before the progress answer discloses the row. if (!requireUploader(authSession, await store.getFile(stored.file_id), res)) return; + // [#22218] The expiry stamp is this door's only write, so the session + // row must be in its reach only when the stamp is due. A read with + // nothing to stamp has no reach to miss, and keeps answering the + // uploader from any organization. Decided ONCE, so the question and the + // stamp cannot disagree about the deadline. + const due = expiryStampDue(stored); + if (due && !requireStartingOrganization(writeContext, [stored], 'progress', uploadId, res)) return; + // Progress REPORTS the expiry rather than refusing it: `expired` is a // declared member of `UploadProgressSchema.status`, and a resuming client // (the SDK's `resumeUpload` polls this first) needs to be told the // session is gone, not handed a 410 it has to interpret (#7667). - const session = await expireIfPastDeadline(stored, writeContext); + const session = due ? await stampExpired(stored, writeContext) : stored; const uploadedChunks = session.uploaded_chunks ?? 0; const uploadedSize = session.uploaded_size ?? 0;