返回 slidev
016-confine-export-output-path.md
根目录 / plans / 016-confine-export-output-path.md
1 # Plan 016: Confine the export output path derived from deck `exportFilename`
2
3 > **Executor instructions**: Follow this plan step by step. Run every
4 > verification command and confirm the expected result. If anything in "STOP
5 > conditions" occurs, stop and report. When done, update the status row in
6 > `plans/README.md`. Security-hardening change: code + tests only.
7 >
8 > **Drift check (run first)**: `git diff --stat c63cb120..HEAD -- packages/slidev/node/commands/export.ts packages/slidev/node/commands/build.ts`
9 > On a mismatch with the excerpts below, treat it as a STOP condition.
10
11 ## Status
12
13 - **Priority**: P2
14 - **Effort**: S
15 - **Risk**: LOW
16 - **Depends on**: none
17 - **Category**: security
18 - **Planned at**: commit `c63cb120`, 2026-07-10
19
20 ## Why this matters
21
22 The export output filename can come from **deck config** (`exportFilename`),
23 which is attacker-controlled if the deck is untrusted. It is written after only
24 appending an extension, so a traversing value (e.g. escaping the intended output
25 directory) causes `slidev export` / `slidev build --download` to write the
26 generated artifact outside where the operator expects. An explicit CLI
27 `--output` is operator-supplied and trusted; the *deck-config* fallback is what
28 needs constraining to a basename.
29
30 ## Current state
31
32 `packages/slidev/node/commands/export.ts:603` (in `getExportOptions`):
33 ```ts
34 outFilename = output || outFilename || options.data.config.exportFilename || `${path.basename(entry, '.md')}-export`
35 return { output: outFilename, /* ... */ }
36 ```
37 Here `output` is the CLI `--output` arg (trusted) and `exportFilename` is deck
38 config (untrusted). The returned `output` is later written by the `gen*`
39 functions (`export.ts:388,417,442,498,538`) and by `commands/build.ts:149-153`:
40 ```ts
41 const filename = options.data.config.exportFilename || 'slidev-exported'
42 await exportSlides({ port, base: config.base, ...getExportOptions(args, options, join(outDir, `${filename}.pdf`)) })
43 ```
44
45 ## Commands you will need
46
47 | Purpose | Command | Expected |
48 |---------|---------|----------|
49 | Install | `pnpm install` | exit 0 |
50 | Build | `pnpm build` | exit 0 |
51 | Test | `pnpm test -- export` (new) | pass |
52 | Typecheck | `pnpm typecheck` | exit 0 |
53
54 ## Scope
55
56 **In scope**:
57 - `packages/slidev/node/commands/export.ts` (sanitize the deck-config filename)
58 - `packages/slidev/node/commands/build.ts` (sanitize the download filename)
59 - A small unit test for the sanitizer
60
61 **Out of scope**:
62 - The CLI `--output` path (operator-supplied, trusted — leave it able to target
63 any directory the operator chooses).
64 - Browser teardown / temp-server port (plans 007/013).
65
66 ## Git workflow
67
68 - Branch: `fix/confine-export-filename`.
69 - Conventional commit: `fix(security): treat deck exportFilename as a basename`.
70 - Do NOT push/PR unless instructed.
71
72 ## Steps
73
74 ### Step 1: Add a filename sanitizer
75
76 Add a small exported helper (e.g. in `export.ts` or `node/utils.ts`):
77 ```ts
78 import path from 'node:path'
79 // Deck-controlled filenames must not contain directory components.
80 export function sanitizeExportBasename(name: string): string {
81 return path.basename(name)
82 }
83 ```
84 `path.basename` strips any directory portion (`../../x` → `x`,
85 `/etc/foo` → `foo`), which is the correct constraint for a deck-provided name.
86
87 ### Step 2: Apply to the deck-config fallback in `getExportOptions`
88
89 Only sanitize the **deck-config** source, not the CLI `--output`:
90 ```ts
91 const deckName = options.data.config.exportFilename
92 ? sanitizeExportBasename(options.data.config.exportFilename)
93 : undefined
94 outFilename = output || outFilename || deckName || `${path.basename(entry, '.md')}-export`
95 ```
96
97 ### Step 3: Apply to `build.ts --download`
98
99 ```ts
100 const filename = options.data.config.exportFilename
101 ? sanitizeExportBasename(options.data.config.exportFilename)
102 : 'slidev-exported'
103 ```
104 (The `join(outDir, ...)` then keeps it inside `outDir`.)
105
106 ### Step 4: Unit test
107
108 Add `packages/slidev/node/commands/export.test.ts` (or extend an existing test):
109 ```ts
110 import { describe, expect, it } from 'vitest'
111 import { sanitizeExportBasename } from './export'
112
113 describe('sanitizeExportBasename', () => {
114 it('keeps a plain name', () => expect(sanitizeExportBasename('talk')).toBe('talk'))
115 it('strips directory traversal', () => expect(sanitizeExportBasename('../../talk')).toBe('talk'))
116 it('strips absolute dirs', () => expect(sanitizeExportBasename('/etc/talk')).toBe('talk'))
117 })
118 ```
119
120 **Verify**: `pnpm build && pnpm test -- export` passes.
121
122 ## Test plan
123
124 - Unit-test the sanitizer (deterministic, no fs).
125 - Confirm a normal `exportFilename: my-talk` still yields `my-talk.pdf` in the
126 expected location (no behavior change on the happy path).
127 - Confirm CLI `--output ./some/dir/name` still works (operator path untouched).
128
129 ## Done criteria
130
131 - [ ] Deck-config `exportFilename` is reduced to a basename before use in both `export.ts` and `build.ts`
132 - [ ] CLI `--output` behavior is unchanged (can still target any directory)
133 - [ ] Sanitizer is unit-tested
134 - [ ] `pnpm build && pnpm typecheck` exit 0
135 - [ ] Only in-scope files modified (`git status`)
136 - [ ] `plans/README.md` status row updated
137
138 ## STOP conditions
139
140 Stop and report if:
141
142 - A documented feature relies on `exportFilename` containing a subdirectory
143 (search docs/tests) — if so, the fix should resolve-and-assert-within-outDir
144 instead of basename-stripping; report before changing approach.
145
146 ## Maintenance notes
147
148 - Keep the trust distinction explicit in code comments: CLI args = operator
149 (trusted), deck config = untrusted.
150 - Reviewer: confirm no other deck-config value feeds a write path unsanitized.
151
151 lines MARKDOWN