diff --git a/apps/web/src/app/api/u/[slug]/flows/[id]/export/route.ts b/apps/web/src/app/api/u/[slug]/flows/[id]/export/route.ts index 1bd85311f..0763006c5 100644 --- a/apps/web/src/app/api/u/[slug]/flows/[id]/export/route.ts +++ b/apps/web/src/app/api/u/[slug]/flows/[id]/export/route.ts @@ -1,6 +1,7 @@ import { NextResponse } from 'next/server' import { auditEvent } from '@/lib/auth' +import { contentDispositionHeader } from '@/lib/workspace-attachments' import { resolveFlowRouteContext } from '@/lib/flows/api' import { createFlowActorScope } from '@/lib/flows/authorization' import { createFlowTemplate, type FlowTemplate } from '@/lib/flows/import-export' @@ -57,7 +58,7 @@ export const GET = withAuth( status: 200, headers: { 'Cache-Control': 'no-store', - 'Content-Disposition': `attachment; filename="${name}.zip"; filename*=UTF-8''${encodeURIComponent(`${name}.zip`)}`, + 'Content-Disposition': contentDispositionHeader(`${name}.zip`), 'Content-Type': 'application/zip', }, }) diff --git a/apps/web/src/app/api/w/[slug]/files/download/route.ts b/apps/web/src/app/api/w/[slug]/files/download/route.ts index 742874a3a..0e0438871 100644 --- a/apps/web/src/app/api/w/[slug]/files/download/route.ts +++ b/apps/web/src/app/api/w/[slug]/files/download/route.ts @@ -2,8 +2,8 @@ import { NextRequest } from "next/server" import { withAuth } from "@/lib/runtime/with-auth" import { + contentDispositionHeader, inferAttachmentMimeType, - sanitizeAttachmentFilename, } from "@/lib/workspace-attachments" import { isValidWorkspacePath, @@ -51,13 +51,12 @@ export const GET = withAuth<{ error: string }>( } const filename = normalizedPath.split("/").pop() ?? "download" - const safeName = sanitizeAttachmentFilename(filename) return new Response(new Uint8Array(content), { status: 200, headers: { "Cache-Control": "no-store", - "Content-Disposition": `attachment; filename="${safeName}"; filename*=UTF-8''${encodeURIComponent(filename)}`, + "Content-Disposition": contentDispositionHeader(filename), "Content-Type": inferAttachmentMimeType(filename), }, }) diff --git a/apps/web/src/lib/__tests__/workspace-attachments.test.ts b/apps/web/src/lib/__tests__/workspace-attachments.test.ts index 8f7e87e54..0f3659fc6 100644 --- a/apps/web/src/lib/__tests__/workspace-attachments.test.ts +++ b/apps/web/src/lib/__tests__/workspace-attachments.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest' import { MAX_ATTACHMENT_UPLOAD_MEGABYTES, + contentDispositionHeader, ensureUniqueAttachmentFilename, formatAttachmentSize, inferAttachmentMimeType, @@ -62,6 +63,21 @@ describe('workspace attachments helpers', () => { expect(isPresentationMimeType('application/vnd.ms-powerpoint')).toBe(false) }) + it('builds RFC 6266 Content-Disposition with sanitized ASCII and UTF-8 extended filename', () => { + expect(contentDispositionHeader('résumé.pdf')).toBe( + "attachment; filename=\"r_sum_.pdf\"; filename*=UTF-8''r%C3%A9sum%C3%A9.pdf", + ) + expect(contentDispositionHeader('report.csv')).toBe( + "attachment; filename=\"report.csv\"; filename*=UTF-8''report.csv", + ) + expect(contentDispositionHeader('')).toBe( + "attachment; filename=\"attachment\"; filename*=UTF-8''", + ) + expect(contentDispositionHeader("it's a report.pdf")).toBe( + "attachment; filename=\"it_s-a-report.pdf\"; filename*=UTF-8''it%27s%20a%20report.pdf", + ) + }) + it('sanitizes unsafe filenames', () => { const withControlChars = `${String.fromCharCode(0)}bad${String.fromCharCode(7)}name.txt` diff --git a/apps/web/src/lib/workspace-attachments.ts b/apps/web/src/lib/workspace-attachments.ts index 31cc31824..6c38fe5cc 100644 --- a/apps/web/src/lib/workspace-attachments.ts +++ b/apps/web/src/lib/workspace-attachments.ts @@ -62,6 +62,12 @@ export function inferAttachmentMimeType(filename: string): string { return MIME_BY_EXTENSION[ext] ?? FALLBACK_MIME } +export function contentDispositionHeader(filename: string): string { + const safeName = sanitizeAttachmentFilename(filename) + const encoded = encodeURIComponent(filename).replace(/[!'()*]/g, (c) => `%${c.charCodeAt(0).toString(16).toUpperCase()}`) + return `attachment; filename="${safeName}"; filename*=UTF-8''${encoded}` +} + export function sanitizeAttachmentFilename(filename: string): string { const cleaned = filename .replace(/[/\\]+/g, '-') diff --git a/openspec/changes/standardize-content-disposition/.openspec.yaml b/openspec/changes/standardize-content-disposition/.openspec.yaml new file mode 100644 index 000000000..7f2cf9bc0 --- /dev/null +++ b/openspec/changes/standardize-content-disposition/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-28 diff --git a/openspec/changes/standardize-content-disposition/design.md b/openspec/changes/standardize-content-disposition/design.md new file mode 100644 index 000000000..63dd2a2ff --- /dev/null +++ b/openspec/changes/standardize-content-disposition/design.md @@ -0,0 +1,52 @@ +## Context + +See `proposal.md` — Why for the three divergent constructions and the malformed-header risk. + +Current mechanics that shape this design (verified in code): + +- `workspace-attachments.ts` already owns attachment naming concerns for the files API: `sanitizeAttachmentFilename` strips path separators, control characters, and quoting hazards from a filename, and `inferAttachmentMimeType` picks the `Content-Type`. Download naming is therefore already centralized in this module; only the header assembly is ad hoc. +- The three routes each build the header inline: `files/download` sanitizes then concatenates both forms, `skills/export` concatenates the raw `[name]` path parameter, `flows/export` emits only the quoted form via `flowExportFileName`. +- RFC 5987 §3.2.1 defines `ext-value` as `charset "'" percent-encoded-value "'"`, where the value must percent-encode everything outside `attr-char`; `attr-char` excludes `*'()` among others. `encodeURIComponent` encodes none of those four. + +## Goals / Non-Goals + +**Goals:** + +- One code path produces every attachment `Content-Disposition` in the app. +- Non-ASCII names survive the round trip in browsers (extended form present everywhere). +- No route can embed an unsanitized value in the quoted filename again. + +**Non-Goals:** + +- Changing `Content-Type` inference, upload-side filename handling, or `flowExportFileName` itself (the flow-name-to-filename mapping stays as is; the helper consumes its output). +- Inline dispositions or non-attachment responses (none exist today). +- Encoding normalization beyond RFC 5987 (e.g. IRIs, percent-encoding uppercase/lowercase policy beyond the existing uppercase hex). + +## Decisions + +### D1: One helper in `workspace-attachments.ts`, both forms always emitted + +- `contentDispositionHeader(filename)` returns `attachment; filename=""; filename*=UTF-8''` unconditionally. Emitting both forms is the RFC 6266-recommended combination: legacy agents read `filename=`, RFC 5987-aware agents prefer `filename*`. +- The ASCII fallback reuses `sanitizeAttachmentFilename` — no second sanitizer to keep in sync. An empty input falls back to the ASCII name `attachment` with an empty extended value, matching the existing sanitize fallback behavior. + +### D2: Fix the RFC 5987 encoding, not just the duplication + +- The extended value is `encodeURIComponent(filename).replace(/[!'()*]/g, c => '%' + hex(c))`, the standard ES5-era RFC 3986 strict-encoding idiom. Without it, a name containing an apostrophe terminates or corrupts the `ext-value` quotes. + +### D3: Route call sites reduce to one expression + +- `files/download` passes the raw basename (the helper sanitizes; the pre-computed `safeName` variable disappears), `flows/export` passes `flowExportFileName(flow.name)`, `skills/export` passes `` `${name}.zip` ``. Each route keeps its own name derivation; none of them touches header assembly. + +## Risks / Trade-offs + +- [Downloaded name changes for flows with non-ASCII names] → Intended fix: names previously mangled by the ASCII-only fallback now decode correctly in modern browsers. Legacy-agent behavior is unchanged in shape (quoted form still present and sanitized). +- [`filename*` present but empty when the name is empty] → Accepted; the header stays syntactically valid and browsers fall back to the quoted `attachment`. + +## Migration Plan + +1. Single deploy; no DB, config, or API contract change beyond the header value. +2. Rollback: revert the deploy; routes return to their previous inline headers. + +## Open Questions + +- (none) diff --git a/openspec/changes/standardize-content-disposition/proposal.md b/openspec/changes/standardize-content-disposition/proposal.md new file mode 100644 index 000000000..a2b81a15a --- /dev/null +++ b/openspec/changes/standardize-content-disposition/proposal.md @@ -0,0 +1,28 @@ +## Why + +Three download/export routes construct `Content-Disposition` three different ways (issue #458). `flows/export` emits only the legacy quoted `filename=` with no RFC 5987 extended form, so non-ASCII flow names download with mangled names. `skills/export` embeds the raw URL path parameter into the quoted filename without sanitization, so a name containing `"` or `;` produces a malformed header. `files/download` is RFC 6266-correct but builds the value inline and re-implements the encoding. The RFC 5987 encoding shared by two of the routes is also wrong: `encodeURIComponent` leaves `'`, `*`, `(`, `)` unescaped although they are reserved in the `ext-value` production (RFC 5987 §3.2.1). + +## What Changes + +- Add `contentDispositionHeader()` to `apps/web/src/lib/workspace-attachments.ts`: the single helper that emits `attachment; filename=""; filename*=UTF-8''`, where the ASCII fallback reuses `sanitizeAttachmentFilename` and the extended form percent-encodes the RFC 5987 reserved characters `!'()*` on top of `encodeURIComponent`. +- Use the helper in `files/download`, `flows/[id]/export`, and `skills/[name]/export`, replacing all three inline constructions. `flows/export` gains the extended UTF-8 form; `skills/export` stops embedding the unsanitized path parameter in the quoted filename; `files/download` drops its separate pre-sanitization step. +- Cover the helper with unit tests for ASCII, non-ASCII, reserved-character, and empty names. + +No behavior change for `Content-Type`, upload naming, or inline (non-attachment) responses. + +## Capabilities + +### New Capabilities +- `file-downloads`: Behavioral contract for how workspace file download and export responses name attached files — one standardized RFC 6266/5987 `Content-Disposition` format produced by a single shared helper. + +### Modified Capabilities +- (none — no existing spec covers download/export response headers.) + +## Impact + +- `apps/web/src/lib/workspace-attachments.ts` — gains `contentDispositionHeader()` (existing `sanitizeAttachmentFilename` reused for the ASCII fallback). +- `apps/web/src/app/api/w/[slug]/files/download/route.ts` — header via helper; local `safeName` pre-sanitization removed. +- `apps/web/src/app/api/u/[slug]/flows/[id]/export/route.ts` — header via helper (gains `filename*`). +- `apps/web/src/app/api/u/[slug]/skills/[name]/export/route.ts` — header via helper (quoted filename now sanitized). +- Tests: `apps/web/src/lib/__tests__/workspace-attachments.test.ts` gains `contentDispositionHeader` cases. +- No DB, config, or dependency changes. diff --git a/openspec/changes/standardize-content-disposition/specs/file-downloads/spec.md b/openspec/changes/standardize-content-disposition/specs/file-downloads/spec.md new file mode 100644 index 000000000..389a3c0da --- /dev/null +++ b/openspec/changes/standardize-content-disposition/specs/file-downloads/spec.md @@ -0,0 +1,32 @@ +## Purpose + +Defines the behavioral contract for how workspace file download and export responses name attached files: a single standardized RFC 6266 `Content-Disposition` format, produced by one shared helper, that carries both a sanitized legacy `filename=` and the RFC 5987 extended `filename*=UTF-8''` form. + +## ADDED Requirements + +### Requirement: File download responses use one standardized Content-Disposition format +Every workspace route that responds with an attached file — workspace file download, flow template export, and skill archive export — SHALL emit exactly one `Content-Disposition` header of the form `attachment; filename=""; filename*=UTF-8''`. The ASCII fallback SHALL be produced by sanitizing the attachment filename, and the extended value SHALL be percent-encoded UTF-8 that also encodes the characters RFC 5987 reserves in `ext-value`. No route SHALL emit a header that omits the extended form or embeds an unsanitized path-derived name in the quoted filename. + +#### Scenario: ASCII-only attachment name +- **WHEN** a download or export route responds for a file named `report.csv` +- **THEN** the `Content-Disposition` header carries `filename="report.csv"` and `filename*=UTF-8''report.csv` + +#### Scenario: Non-ASCII attachment name +- **WHEN** a download or export route responds for a file named `résumé.pdf` +- **THEN** the quoted fallback contains the sanitized ASCII form of the name +- **AND** the extended form contains the percent-encoded UTF-8 representation `r%C3%A9sum%C3%A9.pdf` + +#### Scenario: Attachment name containing RFC 5987 reserved characters +- **WHEN** a download or export route responds for a file whose name contains `'`, `!`, `*`, `(`, or `)` +- **THEN** those characters are percent-encoded in the extended form and cannot terminate or corrupt the `ext-value` quoting + +#### Scenario: Empty attachment name +- **WHEN** a download route cannot derive a filename from the requested path +- **THEN** the header remains syntactically valid with the sanitized fallback ASCII name + +### Requirement: Content-Disposition construction is centralized +Download and export routes SHALL obtain their `Content-Disposition` value from the shared workspace attachment helper rather than constructing header strings inline, so that the attachment naming format has a single implementation and routes cannot diverge. + +#### Scenario: A new export route needs an attachment header +- **WHEN** an API route responds with an attached file +- **THEN** it passes its derived filename to the shared helper and does not assemble `Content-Disposition` parts itself diff --git a/openspec/changes/standardize-content-disposition/tasks.md b/openspec/changes/standardize-content-disposition/tasks.md new file mode 100644 index 000000000..2c870cebc --- /dev/null +++ b/openspec/changes/standardize-content-disposition/tasks.md @@ -0,0 +1,16 @@ +## 1. Shared header helper + +- [x] 1.1 Add `contentDispositionHeader(filename)` to `apps/web/src/lib/workspace-attachments.ts`: sanitize the ASCII fallback with `sanitizeAttachmentFilename`, build the extended form with `encodeURIComponent` plus escaping of the RFC 5987 reserved characters `!'()*`, and emit both forms in one `attachment` header. +- [x] 1.2 Cover the helper in `apps/web/src/lib/__tests__/workspace-attachments.test.ts`: ASCII-only name, non-ASCII name (`résumé.pdf`), reserved-character name (`it's a report.pdf`), and empty name. + +## 2. Route adoption + +- [x] 2.1 `apps/web/src/app/api/u/[slug]/flows/[id]/export/route.ts`: replace the quoted-only header with `contentDispositionHeader(flowExportFileName(flow.name))`. +- [x] 2.2 `apps/web/src/app/api/u/[slug]/skills/[name]/export/route.ts`: replace the inline concatenation (raw `name` path parameter in the quoted filename) with `contentDispositionHeader(\`${name}.zip\`)`. +- [x] 2.3 `apps/web/src/app/api/w/[slug]/files/download/route.ts`: replace the inline header and drop the separate `safeName` pre-sanitization; pass the raw basename to the helper. + +## 3. Final verification + +- [x] 3.1 Run `pnpm test` and `pnpm lint` from `apps/web/` — both green. +- [ ] 3.2 Run `bash scripts/check-podman-images.sh` from the repo root — images build. +- [x] 3.3 Run `openspec validate standardize-content-disposition --strict` — change validates.