downloadFile (src/download/index.ts:71) falls back to file.metadata.title as the output
path when no outPath is given, and downloadThumbnail (:84) uses it with a thumb_
prefix. file.metadata.title is decrypted from server-supplied metadata; it is fully
attacker-controlled if the account is compromised or the server is hostile, and quak's entire
premise is that the server is not trusted with plaintext.
A title of ../../.ssh/authorized_keys writes outside the intended directory. bin/quak.ts
passes opts.out straight through, which is undefined unless --out was given, so quak get <id> in a shell hits this path directly.
src/backup.ts has a sanitizePath helper for the backup tree, but the download layer does
not use it and sanitizePath itself is not applied to path separators in a way that has been
tested against traversal payloads.
src/model/decrypt.ts:100 compounds this: the decrypted metadata JSON is parsed as implicit any and every field is read unchecked, so a non-string title propagates as-is.
Definition of done
When no explicit output path is supplied, the filename derived from server metadata is
sanitized so it can only ever name a file in the intended directory: no path separators, no .. components, no absolute paths, no leading dots, no NUL bytes, no reserved device names.
An explicitly supplied outPath is honoured verbatim — the caller is trusted, the server
is not. This distinction is documented in a code comment.
A title that sanitizes to the empty string falls back to a deterministic safe name derived
from the file ID.
decryptFile validates the shape of the decrypted metadata JSON rather than trusting it:
a non-string title, or a missing one, produces a well-defined result instead of
propagating an arbitrary value.
Sanitization lives in one place and is shared with src/backup.ts rather than duplicated.
Tests cover, as literate documentation of the threat model: ../ traversal, absolute path,
embedded separators, NUL byte, empty title, non-string title, and a normal title passing
through unchanged.
make check green.
TODO.md updated in the same commit.
## Problem
`downloadFile` (`src/download/index.ts:71`) falls back to `file.metadata.title` as the output
path when no `outPath` is given, and `downloadThumbnail` (`:84`) uses it with a `thumb_`
prefix. `file.metadata.title` is decrypted from server-supplied metadata; it is fully
attacker-controlled if the account is compromised or the server is hostile, and quak's entire
premise is that the server is not trusted with plaintext.
A title of `../../.ssh/authorized_keys` writes outside the intended directory. `bin/quak.ts`
passes `opts.out` straight through, which is `undefined` unless `--out` was given, so
`quak get <id>` in a shell hits this path directly.
`src/backup.ts` has a `sanitizePath` helper for the backup tree, but the download layer does
not use it and `sanitizePath` itself is not applied to path separators in a way that has been
tested against traversal payloads.
`src/model/decrypt.ts:100` compounds this: the decrypted metadata JSON is parsed as implicit
`any` and every field is read unchecked, so a non-string `title` propagates as-is.
## Definition of done
1. When no explicit output path is supplied, the filename derived from server metadata is
sanitized so it can only ever name a file in the intended directory: no path separators, no
`..` components, no absolute paths, no leading dots, no NUL bytes, no reserved device names.
2. An explicitly supplied `outPath` is honoured verbatim — the caller is trusted, the server
is not. This distinction is documented in a code comment.
3. A title that sanitizes to the empty string falls back to a deterministic safe name derived
from the file ID.
4. `decryptFile` validates the shape of the decrypted metadata JSON rather than trusting it:
a non-string `title`, or a missing one, produces a well-defined result instead of
propagating an arbitrary value.
5. Sanitization lives in one place and is shared with `src/backup.ts` rather than duplicated.
6. Tests cover, as literate documentation of the threat model: `../` traversal, absolute path,
embedded separators, NUL byte, empty title, non-string title, and a normal title passing
through unchanged.
7. `make check` green.
8. `TODO.md` updated in the same commit.
clawbot
added this to the 1.0.0 milestone 2026-08-09 03:44:48 +02:00
clawbot
self-assigned this 2026-08-09 03:44:48 +02:00
The line numbers in the issue body predate the library rewrite. Where a server-supplied title reaches a filesystem path today:
quak get / quak get-thumb: bin/quak.ts:288 and :323 use originalName / thumbnailName from src/cli-output.ts:36-40, which return the raw title (and thumb_ + title) when --out is absent.
downloadFile / downloadThumbnail: src/download/index.ts:289 and :308 fall back to the raw title when outPath is absent.
Two identical sanitizePath copies: src/backup.ts:111 and src/metadata-backup.ts:17. Neither handles .., NUL bytes, or reserved device names.
The file names in the originals cache take their extension from the title: src/backup.ts:117 and src/library/content.ts:213 (extname(title)). The extension must also be restricted to safe characters, falling back to .bin.
Metadata parse without validation: src/model/decrypt.ts:100.
Definition of done: as in the issue body, applied to every site above. Also:
One shared sanitizer module used by the CLI, download, backup, metadata-backup and the extension derivation.
A normal title still produces exactly the same name as today. The CLI output must stay byte-identical for safe titles (see the comment at the top of src/cli-output.ts).
A caller-supplied --out / outPath is used verbatim.
Tests at the sanitizer level and at least one at each call site, covering quak get without --out.
Out of scope: any other hardening of metadata fields.
Model: opus-5-5
## Implementer brief (current tree, branch `next2`)
The line numbers in the issue body predate the library rewrite. Where a server-supplied title reaches a filesystem path today:
- `quak get` / `quak get-thumb`: `bin/quak.ts:288` and `:323` use `originalName` / `thumbnailName` from `src/cli-output.ts:36-40`, which return the raw title (and `thumb_` + title) when `--out` is absent.
- `downloadFile` / `downloadThumbnail`: `src/download/index.ts:289` and `:308` fall back to the raw title when `outPath` is absent.
- Two identical `sanitizePath` copies: `src/backup.ts:111` and `src/metadata-backup.ts:17`. Neither handles `..`, NUL bytes, or reserved device names.
- The file names in the originals cache take their extension from the title: `src/backup.ts:117` and `src/library/content.ts:213` (`extname(title)`). The extension must also be restricted to safe characters, falling back to `.bin`.
- Metadata parse without validation: `src/model/decrypt.ts:100`.
Definition of done: as in the issue body, applied to every site above. Also:
- One shared sanitizer module used by the CLI, download, backup, metadata-backup and the extension derivation.
- A normal title still produces exactly the same name as today. The CLI output must stay byte-identical for safe titles (see the comment at the top of `src/cli-output.ts`).
- A caller-supplied `--out` / `outPath` is used verbatim.
- Tests at the sanitizer level and at least one at each call site, covering `quak get` without `--out`.
Out of scope: any other hardening of metadata fields.
Model: opus-5-5
Built in #78: every file name taken from server metadata now goes through one sanitizer, and a user-supplied --out/outPath is still used as given.
Model: opus-5-5
Built in https://git.eeqj.de/sneak/quak/pulls/78: every file name taken from server metadata now goes through one sanitizer, and a user-supplied `--out`/`outPath` is still used as given.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Problem
downloadFile(src/download/index.ts:71) falls back tofile.metadata.titleas the outputpath when no
outPathis given, anddownloadThumbnail(:84) uses it with athumb_prefix.
file.metadata.titleis decrypted from server-supplied metadata; it is fullyattacker-controlled if the account is compromised or the server is hostile, and quak's entire
premise is that the server is not trusted with plaintext.
A title of
../../.ssh/authorized_keyswrites outside the intended directory.bin/quak.tspasses
opts.outstraight through, which isundefinedunless--outwas given, soquak get <id>in a shell hits this path directly.src/backup.tshas asanitizePathhelper for the backup tree, but the download layer doesnot use it and
sanitizePathitself is not applied to path separators in a way that has beentested against traversal payloads.
src/model/decrypt.ts:100compounds this: the decrypted metadata JSON is parsed as implicitanyand every field is read unchecked, so a non-stringtitlepropagates as-is.Definition of done
sanitized so it can only ever name a file in the intended directory: no path separators, no
..components, no absolute paths, no leading dots, no NUL bytes, no reserved device names.outPathis honoured verbatim — the caller is trusted, the serveris not. This distinction is documented in a code comment.
from the file ID.
decryptFilevalidates the shape of the decrypted metadata JSON rather than trusting it:a non-string
title, or a missing one, produces a well-defined result instead ofpropagating an arbitrary value.
src/backup.tsrather than duplicated.../traversal, absolute path,embedded separators, NUL byte, empty title, non-string title, and a normal title passing
through unchanged.
make checkgreen.TODO.mdupdated in the same commit.clawbot referenced this issue2026-09-23 00:30:14 +02:00
Implementer brief (current tree, branch
next2)The line numbers in the issue body predate the library rewrite. Where a server-supplied title reaches a filesystem path today:
quak get/quak get-thumb:bin/quak.ts:288and:323useoriginalName/thumbnailNamefromsrc/cli-output.ts:36-40, which return the raw title (andthumb_+ title) when--outis absent.downloadFile/downloadThumbnail:src/download/index.ts:289and:308fall back to the raw title whenoutPathis absent.sanitizePathcopies:src/backup.ts:111andsrc/metadata-backup.ts:17. Neither handles.., NUL bytes, or reserved device names.src/backup.ts:117andsrc/library/content.ts:213(extname(title)). The extension must also be restricted to safe characters, falling back to.bin.src/model/decrypt.ts:100.Definition of done: as in the issue body, applied to every site above. Also:
src/cli-output.ts).--out/outPathis used verbatim.quak getwithout--out.Out of scope: any other hardening of metadata fields.
Model: opus-5-5
Built in #78: every file name taken from server metadata now goes through one sanitizer, and a user-supplied
--out/outPathis still used as given.Model: opus-5-5