Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion apps/web/src/app/api/u/[slug]/flows/[id]/export/route.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -57,7 +58,7 @@ export const GET = withAuth<FlowTemplate | { error: string }, FlowExportRoutePar
timezone: flow.timezone,
}), {
headers: {
'Content-Disposition': `attachment; filename="${flowExportFileName(flow.name)}"`,
'Content-Disposition': contentDispositionHeader(flowExportFileName(flow.name)),
},
})
},
Expand Down
5 changes: 3 additions & 2 deletions apps/web/src/app/api/u/[slug]/skills/[name]/export/route.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { NextResponse } from 'next/server'

import { auditEvent } from '@/lib/auth'
import { withAuth } from '@/lib/runtime/with-auth'
import { readSkillBundle } from '@/lib/skills/skill-store'
import { createSkillArchive } from '@/lib/skills/skill-zip'
import { withAuth } from '@/lib/runtime/with-auth'
import { contentDispositionHeader } from '@/lib/workspace-attachments'

export const runtime = 'nodejs'
export const dynamic = 'force-dynamic'
Expand Down Expand Up @@ -35,7 +36,7 @@ export const GET = withAuth<{ error: string }, SkillExportRouteParams>(
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',
},
})
Expand Down
5 changes: 2 additions & 3 deletions apps/web/src/app/api/w/[slug]/files/download/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
},
})
Expand Down
16 changes: 16 additions & 0 deletions apps/web/src/lib/__tests__/workspace-attachments.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest'

import {
MAX_ATTACHMENT_UPLOAD_MEGABYTES,
contentDispositionHeader,
ensureUniqueAttachmentFilename,
formatAttachmentSize,
inferAttachmentMimeType,
Expand Down Expand Up @@ -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`

Expand Down
6 changes: 6 additions & 0 deletions apps/web/src/lib/workspace-attachments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, '-')
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-08-28
52 changes: 52 additions & 0 deletions openspec/changes/standardize-content-disposition/design.md
Original file line number Diff line number Diff line change
@@ -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="<sanitized>"; filename*=UTF-8''<encoded>` 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)
28 changes: 28 additions & 0 deletions openspec/changes/standardize-content-disposition/proposal.md
Original file line number Diff line number Diff line change
@@ -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="<sanitized ASCII>"; filename*=UTF-8''<encoded>`, 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.
Original file line number Diff line number Diff line change
@@ -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="<sanitized ASCII name>"; filename*=UTF-8''<RFC 5987 percent-encoded name>`. 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
16 changes: 16 additions & 0 deletions openspec/changes/standardize-content-disposition/tasks.md
Original file line number Diff line number Diff line change
@@ -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.
Loading