diff --git a/.changeset/approval-attachment-descriptors.md b/.changeset/approval-attachment-descriptors.md new file mode 100644 index 0000000000..544b79ce8a --- /dev/null +++ b/.changeset/approval-attachment-descriptors.md @@ -0,0 +1,29 @@ +--- +"@objectstack/spec": patch +"@objectstack/plugin-approvals": patch +--- + +fix(approvals): return decision attachments as file values, not "[object Object]" (#3504) + +`sys_approval_action.attachments` is a `Field.file`, so the column **stores an +opaque `sys_file` id** (ADR-0104 D3 — the stored form of every media field). The +ObjectQL read path resolves that id into its expanded +`{ id, name, size, mimeType, url }` form on the way out. But `rowFromAction` +mapped the column with `.map(String)`, collapsing each expanded value to the +literal string `"[object Object]"`. Every `listActions` consumer (the approval +inbox timeline) then received garbage: the attachment chip had no filename and +its id was `"[object Object]"`, so opening it 404'd. + +- `ApprovalActionRow.attachments` is now `ApprovalActionAttachment[]` — the + expanded file value plus its id, so a consumer can label and open an + attachment without needing read access to the system `sys_file` object (which + regular approvers do not have). +- Three read forms are accepted: the expanded value (the normal case), a bare id + (nothing to expand it into — storage service absent, file not committed), and + a legacy inline blob written before file-as-reference (`file_id` / + `mime_type`), until the backfill converts it. The id test reuses the + platform's `isFileIdToken`, so this and the engine's read resolver cannot + disagree about what counts as an id. +- The decision *input* (`ApprovalDecisionInput.attachments`) is unchanged — it + still takes fileId strings, which is also exactly what the column stores. Only + the read shape changed. diff --git a/.changeset/storage-download-filename.md b/.changeset/storage-download-filename.md new file mode 100644 index 0000000000..08466bf709 --- /dev/null +++ b/.changeset/storage-download-filename.md @@ -0,0 +1,22 @@ +--- +"@objectstack/spec": patch +"@objectstack/service-storage": patch +--- + +fix(storage): downloads carry the real filename + content-type, not the URL token (#3504) + +A presigned download served the bytes as `application/octet-stream` with no +`Content-Disposition`, so a browser saved the file under the opaque URL token +(e.g. `eyJrIjoiYXR0YWNo…`) instead of its real name — an approval's +`signed-contract.pdf` downloaded as a nameless blob. + +- `IStorageService.getSignedUrl` / `getPresignedDownload` take an optional + `PresignedDownloadOptions` (`filename`, `contentType`, `disposition`). +- The REST download routes (`GET /storage/files/:id/url` and `/:id`) pass the + `sys_file` record's `name` + `mime_type`. +- The local adapter carries them in the signed token; the `_local/raw` route + emits `Content-Type` + an RFC 5987 `Content-Disposition` (ASCII fallback + + `filename*=UTF-8''…` for non-ASCII names). The S3 adapter bakes the same into + the signed URL via `ResponseContentType` / `ResponseContentDisposition`. +- Default disposition is `inline`, so previewable types (PDF, images) still open + in the browser — now with the correct name when saved. diff --git a/.claude/launch.json b/.claude/launch.json index f2ec1bcf39..6341418fd0 100644 --- a/.claude/launch.json +++ b/.claude/launch.json @@ -18,6 +18,24 @@ "file:/tmp/showcase-dogfood-3777/data.db" ], "port": 3777 + }, + { + "name": "pr3505-attach-3477", + "runtimeExecutable": "pnpm", + "runtimeArgs": [ + "-C", + "/home/user/objectstack-pr3505/examples/app-showcase", + "exec", + "objectstack", + "dev", + "--ui", + "--seed-admin", + "-p", + "3477", + "-d", + "file:/tmp/pr3505-attach/data.db" + ], + "port": 3477 } ] } diff --git a/content/docs/kernel/contracts/storage-service.mdx b/content/docs/kernel/contracts/storage-service.mdx index be8f95bda8..e7e0a4cc65 100644 --- a/content/docs/kernel/contracts/storage-service.mdx +++ b/content/docs/kernel/contracts/storage-service.mdx @@ -29,7 +29,7 @@ export interface IStorageService { list?(prefix: string): Promise; // Signed URL (optional) - getSignedUrl?(key: string, expiresIn: number): Promise; + getSignedUrl?(key: string, expiresIn: number, options?: PresignedDownloadOptions): Promise; // Presigned browser-direct upload / download (optional) getPresignedUpload?( @@ -37,7 +37,11 @@ export interface IStorageService { expiresIn: number, options?: StorageUploadOptions, ): Promise; - getPresignedDownload?(key: string, expiresIn: number): Promise; + getPresignedDownload?( + key: string, + expiresIn: number, + options?: PresignedDownloadOptions, + ): Promise; // Chunked / multipart upload (optional) initiateChunkedUpload?(key: string, options?: StorageUploadOptions): Promise; diff --git a/content/docs/kernel/runtime-services/storage-service.mdx b/content/docs/kernel/runtime-services/storage-service.mdx index bd29137bdd..0a78a2ce25 100644 --- a/content/docs/kernel/runtime-services/storage-service.mdx +++ b/content/docs/kernel/runtime-services/storage-service.mdx @@ -17,14 +17,14 @@ services.storage.delete(key: string): Promise services.storage.exists(key: string): Promise services.storage.getInfo(key: string): Promise services.storage.list?(prefix: string): Promise -services.storage.getSignedUrl?(key: string, expiresIn: number): Promise +services.storage.getSignedUrl?(key: string, expiresIn: number, options?: PresignedDownloadOptions): Promise ``` ## Presigned / Chunked (optional) ```ts services.storage.getPresignedUpload?(key: string, expiresIn: number, options?: StorageUploadOptions): Promise -services.storage.getPresignedDownload?(key: string, expiresIn: number): Promise +services.storage.getPresignedDownload?(key: string, expiresIn: number, options?: PresignedDownloadOptions): Promise services.storage.initiateChunkedUpload?(key: string, options?: StorageUploadOptions): Promise services.storage.uploadChunk?(uploadId: string, partNumber: number, data: Buffer): Promise services.storage.completeChunkedUpload?(uploadId: string, parts: Array<{ partNumber: number; eTag: string }>): Promise diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts index 2ef565cfa7..4c6accbcaa 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts @@ -1948,7 +1948,7 @@ describe('ApprovalService — decision_progress & deep links (#2678 P1.5)', () = // listActions must surface decision attachments through the contract mapping // (#3266 — the column existed but rowFromAction dropped it; caught in browser). describe('ApprovalService — listActions attachments mapping (#3266)', () => { - it('returns the attachments recorded on a decision', async () => { + it('normalizes a bare fileId string into an attachment descriptor', async () => { const engine = makeFakeEngine(); let n = 0; const svc = new ApprovalService({ engine: engine as any, clock: { now: () => new Date(1757000000000 + (n++) * 1000) } }); @@ -1956,6 +1956,49 @@ describe('ApprovalService — listActions attachments mapping (#3266)', () => { await svc.decideNode(req.id, { decision: 'approve', actorId: 'u9', attachments: ['file_a'] }, SYS); const acts = await svc.listActions(req.id, SYS); const approve = acts.find(a => a.action === 'approve'); - expect(approve?.attachments).toEqual(['file_a']); + expect(approve?.attachments).toEqual([{ id: 'file_a' }]); + }); + + // The normal case. The column STORES an opaque sys_file id (ADR-0104 D3); + // the ObjectQL read path resolves it into the expanded + // `{ id, name, size, mimeType, url }` form on the way out. The old + // `.map(String)` turned that object into "[object Object]", so the inbox chip + // had no name and 404'd on open. The mapping must pass it through unmangled. + it('passes the engine-expanded file value through with its name and url', async () => { + const engine = makeFakeEngine(); + let n = 0; + const svc = new ApprovalService({ engine: engine as any, clock: { now: () => new Date(1757000000000 + (n++) * 1000) } }); + const req = await svc.openNodeRequest(openInput(['u9']), CTX); + await svc.decideNode(req.id, { decision: 'approve', actorId: 'u9' }, SYS); + // Simulate what the read path hands back after resolving the stored id. + const row = engine._tables['sys_approval_action'].find((a: any) => a.action === 'approve'); + row.attachments = [ + { id: 'file_a', name: 'signed-contract.pdf', mimeType: 'application/pdf', size: 24, url: '/api/v1/storage/files/file_a' }, + ]; + const acts = await svc.listActions(req.id, SYS); + const approve = acts.find(a => a.action === 'approve'); + expect(approve?.attachments).toEqual([ + { id: 'file_a', name: 'signed-contract.pdf', mimeType: 'application/pdf', size: 24, url: '/api/v1/storage/files/file_a' }, + ]); + }); + + // Rows written before file-as-reference hold an inline blob whose keys are + // snake_case (`file_id`, `mime_type`). They stay readable until the backfill + // converts them, so both casings must map — the same drift that made objectui + // stop recognising images when the expanded form arrived. + it('maps a legacy inline blob (file_id / mime_type) written before the cutover', async () => { + const engine = makeFakeEngine(); + let n = 0; + const svc = new ApprovalService({ engine: engine as any, clock: { now: () => new Date(1757000000000 + (n++) * 1000) } }); + const req = await svc.openNodeRequest(openInput(['u9']), CTX); + await svc.decideNode(req.id, { decision: 'approve', actorId: 'u9' }, SYS); + const row = engine._tables['sys_approval_action'].find((a: any) => a.action === 'approve'); + row.attachments = [ + { file_id: 'file_b', name: 'old.pdf', mime_type: 'application/pdf', size: 12, url: 'https://cdn/old.pdf' }, + ]; + const acts = await svc.listActions(req.id, SYS); + expect(acts.find(a => a.action === 'approve')?.attachments).toEqual([ + { id: 'file_b', name: 'old.pdf', mimeType: 'application/pdf', size: 12, url: 'https://cdn/old.pdf' }, + ]); }); }); diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index 671f8a8331..c75354c28c 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -19,6 +19,7 @@ import type { IApprovalService, ApprovalRequestRow, ApprovalActionRow, + ApprovalActionAttachment, ApprovalDecisionInput, ApprovalDecisionResult, ApprovalRecallInput, @@ -30,6 +31,7 @@ import type { ApprovalStatus, SharingExecutionContext, } from '@objectstack/spec/contracts'; +import { isFileIdToken } from '@objectstack/spec/data'; import { isGrantActive } from '@objectstack/core'; /** @@ -255,7 +257,58 @@ function slaDueAt(createdAt: unknown, cfg: any): string | undefined { return new Date(t + hours * 3600_000).toISOString(); } +/** + * Normalize one raw `attachments` entry into an {@link ApprovalActionAttachment}. + * + * `sys_approval_action.attachments` is a `Field.file` (multiple), so the column + * **stores opaque `sys_file` ids** — that is the stored form of every media + * field (ADR-0104 D3). What arrives here is whichever of three forms the read + * path produced: + * + * 1. the **expanded** `{ id, name, size, mimeType, url }` the ObjectQL read + * path resolves a stored id into — the normal case; + * 2. a **bare id**, when there was nothing to expand it into (storage service + * absent, file not committed); + * 3. a **legacy inline blob** (`{ file_id, name, mime_type, url, … }`) written + * before file-as-reference, until the backfill converts it. + * + * The original mapping did `String(entry)`, which turned form 1 into the + * literal `"[object Object]"` — so the inbox timeline showed a nameless, + * un-openable attachment chip (#3266 follow-up; caught by browser verification). + * + * Note the casing: the expanded form carries `mimeType`, the legacy blob + * `mime_type`. Both are accepted for the duration of the migration window. + */ +function normalizeActionAttachment(entry: any): ApprovalActionAttachment | undefined { + if (entry == null) return undefined; + // Form 2 — a bare reference. `isFileIdToken` is the platform's single arbiter + // of "is this string an opaque file id, or a URL?", shared with the engine's + // read resolver, so the two cannot disagree about what counts as an id. + if (typeof entry === 'string') { + const id = entry.trim(); + if (!id) return undefined; + return isFileIdToken(id) ? { id } : { id, url: id }; + } + if (typeof entry === 'object') { + // Forms 1 and 3 — `file_id` is the legacy blob's key for the same thing. + const id = entry.id ?? entry.file_id; + if (id == null || String(id) === '') return undefined; + const mimeType = entry.mimeType ?? entry.mime_type; + return { + id: String(id), + name: typeof entry.name === 'string' ? entry.name : undefined, + url: typeof entry.url === 'string' ? entry.url : undefined, + mimeType: typeof mimeType === 'string' ? mimeType : undefined, + size: typeof entry.size === 'number' ? entry.size : undefined, + }; + } + return undefined; +} + function rowFromAction(row: any): ApprovalActionRow { + const attachments = Array.isArray(row.attachments) + ? row.attachments.map(normalizeActionAttachment).filter((a: ApprovalActionAttachment | undefined): a is ApprovalActionAttachment => !!a) + : []; return { id: String(row.id), request_id: String(row.request_id), @@ -264,10 +317,9 @@ function rowFromAction(row: any): ApprovalActionRow { action: row.action, actor_id: row.actor_id ?? undefined, comment: row.comment ?? undefined, - // Decision attachments (#3266). The column shipped in #3268 but this - // contract mapping didn't — the raw engine row carried the fileIds while - // every consumer of listActions saw none (caught by browser verification). - attachments: Array.isArray(row.attachments) && row.attachments.length ? row.attachments.map(String) : undefined, + // Decision attachments (#3266): rich descriptors carrying the display name + + // download URL, so consumers label/open them without reading `sys_file`. + attachments: attachments.length ? attachments : undefined, created_at: row.created_at ?? undefined, }; } diff --git a/packages/services/service-storage/src/content-disposition.test.ts b/packages/services/service-storage/src/content-disposition.test.ts new file mode 100644 index 0000000000..a60cc7eeb9 --- /dev/null +++ b/packages/services/service-storage/src/content-disposition.test.ts @@ -0,0 +1,30 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { contentDispositionValue } from './content-disposition.js'; + +describe('contentDispositionValue', () => { + it('defaults to inline and emits both filename and filename*', () => { + expect(contentDispositionValue('signed-contract.pdf')).toBe( + "inline; filename=\"signed-contract.pdf\"; filename*=UTF-8''signed-contract.pdf", + ); + }); + + it('honors an attachment disposition', () => { + expect(contentDispositionValue('report.csv', 'attachment')).toBe( + "attachment; filename=\"report.csv\"; filename*=UTF-8''report.csv", + ); + }); + + it('keeps a non-ASCII name in filename* and sanitizes the ASCII fallback', () => { + const v = contentDispositionValue('季度报告.pdf'); + // ASCII fallback replaces each of the 4 non-ascii chars; filename* preserves them. + expect(v).toContain('filename="____.pdf"'); + expect(v).toContain("filename*=UTF-8''%E5%AD%A3%E5%BA%A6%E6%8A%A5%E5%91%8A.pdf"); + }); + + it('neutralizes quotes/backslashes in the ASCII fallback (no header injection)', () => { + const v = contentDispositionValue('a"b\\c.txt'); + expect(v).toContain('filename="a_b_c.txt"'); + }); +}); diff --git a/packages/services/service-storage/src/content-disposition.ts b/packages/services/service-storage/src/content-disposition.ts new file mode 100644 index 0000000000..c330511435 --- /dev/null +++ b/packages/services/service-storage/src/content-disposition.ts @@ -0,0 +1,22 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Build a `Content-Disposition` header value that survives non-ASCII filenames. + * + * Emits both a sanitized ASCII `filename=` (legacy fallback) and the RFC 5987 + * `filename*=UTF-8''…` form that modern browsers prefer — so a download saves + * under its real name instead of the opaque URL token. Used by the local + * adapter's `_local/raw` route (header) and the S3 adapter's + * `ResponseContentDisposition` (baked into the signed URL). + */ +export function contentDispositionValue( + filename: string, + type: 'inline' | 'attachment' = 'inline', +): string { + const asciiFallback = filename.replace(/[^\x20-\x7e]/g, '_').replace(/["\\]/g, '_'); + const encoded = encodeURIComponent(filename).replace( + /['()*]/g, + (c) => '%' + c.charCodeAt(0).toString(16).toUpperCase(), + ); + return `${type}; filename="${asciiFallback}"; filename*=UTF-8''${encoded}`; +} diff --git a/packages/services/service-storage/src/local-storage-adapter.test.ts b/packages/services/service-storage/src/local-storage-adapter.test.ts index e93ee1472c..184aece397 100644 --- a/packages/services/service-storage/src/local-storage-adapter.test.ts +++ b/packages/services/service-storage/src/local-storage-adapter.test.ts @@ -151,6 +151,21 @@ describe('LocalStorageAdapter', () => { expect(desc.downloadUrl).toContain('/_local/raw/'); expect(desc.expiresIn).toBe(60); }); + + it('carries filename + content-type into the download token (so the browser saves the real name)', async () => { + await createTempDir(); + await adapter.upload('attachments/abc.pdf', Buffer.from('%PDF-')); + const desc = await adapter.getPresignedDownload!('attachments/abc.pdf', 60, { + filename: 'signed-contract.pdf', + contentType: 'application/pdf', + disposition: 'inline', + }); + const token = desc.downloadUrl.split('/_local/raw/')[1]; + const payload = adapter.verifyToken(token, 'get'); + expect(payload.n).toBe('signed-contract.pdf'); + expect(payload.ct).toBe('application/pdf'); + expect(payload.d).toBe('inline'); + }); }); // ========================================================================= diff --git a/packages/services/service-storage/src/local-storage-adapter.ts b/packages/services/service-storage/src/local-storage-adapter.ts index 19271cccc0..8c09abc32d 100644 --- a/packages/services/service-storage/src/local-storage-adapter.ts +++ b/packages/services/service-storage/src/local-storage-adapter.ts @@ -14,6 +14,7 @@ import type { StorageFileInfo, PresignedUploadDescriptor, PresignedDownloadDescriptor, + PresignedDownloadOptions, } from '@objectstack/spec/contracts'; /** @@ -48,6 +49,8 @@ export interface LocalStorageAdapterOptions { interface PresignTokenPayload { k: string; // storage key ct?: string; // content-type + n?: string; // download filename (Content-Disposition) + d?: 'inline' | 'attachment'; // disposition type (default 'inline') exp: number; // expiry epoch seconds op: 'put' | 'get'; } @@ -268,17 +271,31 @@ export class LocalStorageAdapter implements IStorageService { }; } - async getPresignedDownload(key: string, expiresIn: number): Promise { + async getPresignedDownload( + key: string, + expiresIn: number, + options?: PresignedDownloadOptions, + ): Promise { const exp = Math.floor(Date.now() / 1000) + Math.max(1, expiresIn); - const token = this.signToken({ k: key, exp, op: 'get' }); + // Carry filename + content-type in the token so the `_local/raw` route can + // emit a real Content-Disposition / Content-Type (else the browser saves + // the file under the opaque token, as `application/octet-stream`). + const token = this.signToken({ + k: key, + exp, + op: 'get', + ct: options?.contentType, + n: options?.filename, + d: options?.disposition, + }); return { downloadUrl: `${this.baseUrl}${this.basePath}/_local/raw/${token}`, expiresIn, }; } - async getSignedUrl(key: string, expiresIn: number): Promise { - const desc = await this.getPresignedDownload(key, expiresIn); + async getSignedUrl(key: string, expiresIn: number, options?: PresignedDownloadOptions): Promise { + const desc = await this.getPresignedDownload(key, expiresIn, options); return desc.downloadUrl; } diff --git a/packages/services/service-storage/src/s3-storage-adapter.ts b/packages/services/service-storage/src/s3-storage-adapter.ts index 840c32f8fd..29d5fcd221 100644 --- a/packages/services/service-storage/src/s3-storage-adapter.ts +++ b/packages/services/service-storage/src/s3-storage-adapter.ts @@ -11,7 +11,9 @@ import type { StorageFileInfo, PresignedUploadDescriptor, PresignedDownloadDescriptor, + PresignedDownloadOptions, } from '@objectstack/spec/contracts'; +import { contentDispositionValue } from './content-disposition.js'; /** * Configuration for the S3 storage adapter. @@ -227,8 +229,8 @@ export class S3StorageAdapter implements IStorageService { // Presigned URLs // --------------------------------------------------------------------------- - async getSignedUrl(key: string, expiresIn: number): Promise { - const desc = await this.getPresignedDownload(key, expiresIn); + async getSignedUrl(key: string, expiresIn: number, options?: PresignedDownloadOptions): Promise { + const desc = await this.getPresignedDownload(key, expiresIn, options); return desc.downloadUrl; } @@ -256,11 +258,24 @@ export class S3StorageAdapter implements IStorageService { }; } - async getPresignedDownload(key: string, expiresIn: number): Promise { + async getPresignedDownload( + key: string, + expiresIn: number, + options?: PresignedDownloadOptions, + ): Promise { const client = await this.getClient(); const s3 = await this.s3Mod(); const { getSignedUrl } = await this.presignerMod(); - const cmd = new s3.GetObjectCommand({ Bucket: this.bucket, Key: key }); + // S3 bakes these response overrides into the signed URL, so the download + // carries the real filename + type instead of the object key + octet-stream. + const cmd = new s3.GetObjectCommand({ + Bucket: this.bucket, + Key: key, + ...(options?.contentType ? { ResponseContentType: options.contentType } : {}), + ...(options?.filename + ? { ResponseContentDisposition: contentDispositionValue(options.filename, options.disposition ?? 'inline') } + : {}), + }); const url = await getSignedUrl(client, cmd, { expiresIn }); return { downloadUrl: url, expiresIn }; } diff --git a/packages/services/service-storage/src/storage-routes.ts b/packages/services/service-storage/src/storage-routes.ts index f9a66db99f..a57db99528 100644 --- a/packages/services/service-storage/src/storage-routes.ts +++ b/packages/services/service-storage/src/storage-routes.ts @@ -4,6 +4,7 @@ import { randomUUID } from 'node:crypto'; import type { IHttpServer, IHttpRequest, IHttpResponse, IStorageService } from '@objectstack/spec/contracts'; import type { StorageMetadataStore, FileRecord } from './metadata-store.js'; import type { LocalStorageAdapter } from './local-storage-adapter.js'; +import { contentDispositionValue } from './content-disposition.js'; /** Authorization verdict for an attachments-scope download (#2970 item 2). */ export type FileReadVerdict = 'allow' | 'deny' | 'unauthenticated'; @@ -518,12 +519,13 @@ export function registerStorageRoutes( const ttl = await authorizeDownload(file, req, res); if (ttl === false) return; + const downloadOpts = { filename: file.name, contentType: file.mime_type }; let url: string; if (storage.getPresignedDownload) { - const desc = await storage.getPresignedDownload(file.key, ttl); + const desc = await storage.getPresignedDownload(file.key, ttl, downloadOpts); url = desc.downloadUrl; } else if (storage.getSignedUrl) { - url = await storage.getSignedUrl(file.key, ttl); + url = await storage.getSignedUrl(file.key, ttl, downloadOpts); } else { url = `${basePath}/_local/file/${encodeURIComponent(file.key)}`; } @@ -557,12 +559,13 @@ export function registerStorageRoutes( const ttl = await authorizeDownload(file, req, res); if (ttl === false) return; + const downloadOpts = { filename: file.name, contentType: file.mime_type }; let url: string; if (storage.getPresignedDownload) { - const desc = await storage.getPresignedDownload(file.key, ttl); + const desc = await storage.getPresignedDownload(file.key, ttl, downloadOpts); url = desc.downloadUrl; } else if (storage.getSignedUrl) { - url = await storage.getSignedUrl(file.key, ttl); + url = await storage.getSignedUrl(file.key, ttl, downloadOpts); } else { url = `${basePath}/_local/file/${encodeURIComponent(file.key)}`; } @@ -621,6 +624,11 @@ export function registerStorageRoutes( res.header('content-type', payload.ct ?? 'application/octet-stream'); res.header('content-length', String(data.byteLength)); + // When the token carries the original filename, advertise it so the + // browser saves the file under its real name (not the opaque URL token). + if (payload.n) { + res.header('content-disposition', contentDispositionValue(payload.n, payload.d ?? 'inline')); + } res.send(data); } catch (err: any) { const statusCode = err.message?.includes('expired') || err.message?.includes('signature') ? 403 : 500; diff --git a/packages/services/service-storage/src/swappable-storage-service.ts b/packages/services/service-storage/src/swappable-storage-service.ts index 670e290b92..f4f1af7d0a 100644 --- a/packages/services/service-storage/src/swappable-storage-service.ts +++ b/packages/services/service-storage/src/swappable-storage-service.ts @@ -6,6 +6,7 @@ import type { StorageUploadOptions, PresignedUploadDescriptor, PresignedDownloadDescriptor, + PresignedDownloadOptions, } from '@objectstack/spec/contracts'; /** @@ -77,11 +78,11 @@ export class SwappableStorageService implements IStorageService { return this.inner.list(prefix); } - getSignedUrl(key: string, expiresIn: number): Promise { + getSignedUrl(key: string, expiresIn: number, options?: PresignedDownloadOptions): Promise { if (typeof this.inner.getSignedUrl !== 'function') { return Promise.reject(new Error('Active storage adapter does not support getSignedUrl()')); } - return this.inner.getSignedUrl(key, expiresIn); + return this.inner.getSignedUrl(key, expiresIn, options); } getPresignedUpload( @@ -95,11 +96,11 @@ export class SwappableStorageService implements IStorageService { return this.inner.getPresignedUpload(key, expiresIn, options); } - getPresignedDownload(key: string, expiresIn: number): Promise { + getPresignedDownload(key: string, expiresIn: number, options?: PresignedDownloadOptions): Promise { if (typeof this.inner.getPresignedDownload !== 'function') { return Promise.reject(new Error('Active storage adapter does not support getPresignedDownload()')); } - return this.inner.getPresignedDownload(key, expiresIn); + return this.inner.getPresignedDownload(key, expiresIn, options); } initiateChunkedUpload(key: string, options?: StorageUploadOptions): Promise { diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index 108820987c..52a309416d 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -3434,6 +3434,7 @@ "AnalyticsQuery (interface)", "AnalyticsResult (interface)", "AnalyticsStrategy (interface)", + "ApprovalActionAttachment (interface)", "ApprovalActionKind (type)", "ApprovalActionRow (interface)", "ApprovalDecisionInput (interface)", @@ -3590,6 +3591,7 @@ "Plugin (interface)", "PluginStartupResult (interface)", "PresignedDownloadDescriptor (interface)", + "PresignedDownloadOptions (interface)", "PresignedUploadDescriptor (interface)", "ProposePendingActionInput (interface)", "PubSubHandler (type)", diff --git a/packages/spec/src/contracts/approval-service.ts b/packages/spec/src/contracts/approval-service.ts index 698de94bda..26ac8bf78b 100644 --- a/packages/spec/src/contracts/approval-service.ts +++ b/packages/spec/src/contracts/approval-service.ts @@ -199,6 +199,31 @@ export type ApprovalActionKind = /** #1322 M1: an out-of-office approver's slot was auto-rerouted to their delegate at resolution time. */ | 'ooo_substitute'; +/** + * A file attached to a decision action (#3266) — the READ shape of one + * `sys_approval_action.attachments` entry. + * + * The column **stores an opaque `sys_file` id** (ADR-0104 D3: that is the + * stored form of every media field). The name, size, MIME type and URL are not + * stored alongside it — the ObjectQL read path resolves the id into its + * expanded `FileValueSchema` form on the way out, and this interface is that + * form plus the id. So a consumer gets everything it needs to label and open an + * attachment without read access to the system `sys_file` object, while the + * write side stays a plain id (see `ApprovalDecisionInput.attachments`). + * + * Field names follow the expanded form — `mimeType`, not `mime_type`. + */ +export interface ApprovalActionAttachment { + /** The `sys_file` id — pass to `GET /storage/files/:id/url` for a signed URL. */ + id: string; + /** Original filename, for the chip label. */ + name?: string; + /** Stable download URL (`/api/v1/storage/files/:id`); may be relative. */ + url?: string; + mimeType?: string; + size?: number; +} + /** Audit row. */ export interface ApprovalActionRow { id: string; @@ -208,8 +233,8 @@ export interface ApprovalActionRow { action: ApprovalActionKind; actor_id?: string; comment?: string; - /** File references attached to this action (decision attachments, #3266). */ - attachments?: string[]; + /** Files attached to this action (decision attachments, #3266). */ + attachments?: ApprovalActionAttachment[]; created_at?: string; /** Display name of the actor (`sys_user.name`), when resolvable. */ actor_name?: string; diff --git a/packages/spec/src/contracts/storage-service.ts b/packages/spec/src/contracts/storage-service.ts index d06e73d950..db493c9e04 100644 --- a/packages/spec/src/contracts/storage-service.ts +++ b/packages/spec/src/contracts/storage-service.ts @@ -71,6 +71,22 @@ export interface PresignedDownloadDescriptor { expiresIn: number; } +/** + * Presentation hints for a presigned download. Without these the served bytes + * default to `application/octet-stream` with no filename, so a browser saves + * the file under the opaque URL token instead of its real name. Callers that + * know the file's metadata (e.g. the REST download routes, which have the + * `sys_file` record) should pass it so the download carries a real name + type. + */ +export interface PresignedDownloadOptions { + /** Original filename → `Content-Disposition` (the browser's save-as name). */ + filename?: string; + /** Content type → `Content-Type` (defaults to `application/octet-stream`). */ + contentType?: string; + /** `inline` previews in the browser (default); `attachment` forces a download. */ + disposition?: 'inline' | 'attachment'; +} + export interface IStorageService { /** * Upload a file to storage @@ -120,7 +136,7 @@ export interface IStorageService { * @param expiresIn - URL expiration time in seconds * @returns Pre-signed URL string */ - getSignedUrl?(key: string, expiresIn: number): Promise; + getSignedUrl?(key: string, expiresIn: number, options?: PresignedDownloadOptions): Promise; // ========================================== // Presigned Upload / Download (browser-direct) @@ -155,7 +171,7 @@ export interface IStorageService { * @param key - Storage key/path * @param expiresIn - URL expiration time in seconds */ - getPresignedDownload?(key: string, expiresIn: number): Promise; + getPresignedDownload?(key: string, expiresIn: number, options?: PresignedDownloadOptions): Promise; // ========================================== // Chunked / Multipart Upload Methods