Skip to content

feat(article): add cover image upload - #209

Merged
aquie00t merged 1 commit into
mainfrom
feature/article-cover-upload
Aug 24, 2026
Merged

feat(article): add cover image upload#209
aquie00t merged 1 commit into
mainfrom
feature/article-cover-upload

Conversation

@aquie00t

Copy link
Copy Markdown
Collaborator

What does this PR do?

Stage 4 of the article feature — cover image upload.

POST /api/v1/articles/cover    auth required, SENSITIVE, one file, 5 MB max
→ 200 { data: { coverImageKey, coverImageUrl }, meta: { timestamp } }

The endpoint returns a key, not a URL

The article body accepts only a storage key, validated in stage 2 against the caller's own prefix. Keeping URLs out of the request is what closes javascript: and data: stored XSS, external tracking pixels, and one user attaching another user's upload — one rule instead of a sanitizer. This is the surface create-post.schema.ts leaves open by accepting arbitrary format: uri media URLs.

The type comes from the bytes, never from the client

The multipart part's mimetype and filename are never consulted and never forwarded. Both are attacker-controlled: a request can claim image/png while carrying an SVG, and a name like cover.png.html reads as an image to an extension check. Magic bytes are the only statement about a file the uploader cannot forge.

Format Signature
JPEG FF D8 FF
PNG 89 50 4E 47 0D 0A 1A 0A
GIF GIF8
WEBP RIFF at 0 and WEBP at 8
AVIF ftypavif at 4

SVG has no signature to match and is rejected for free — the intended outcome, since it is a scriptable document rather than a raster image and serving one from the CDN would be stored XSS. The WEBP check requires both markers, so a .wav file (also a RIFF container) does not slip through.

The stored file name is generated, so a path traversal in the client's name cannot reach a storage key. Size is checked twice — byte length and the transport's truncation flag, because the multipart limit truncates silently on some paths.

CryptoPort gains generateUuid() so the use case stays free of node imports, in keeping with how the rest of core reaches randomness.

Verification

721 unit tests pass (708 existing + 13 new), lint, format:check, build clean. Booting the app confirms DI resolution, the route, and its OpenAPI entry.

The 13 new unit tests cover each accepted format, SVG and HTML rejection, empty and truncated buffers, the RIFF-but-not-WEBP case, and that the generated key matches the validator from stage 2 — so the two halves cannot drift apart.

The e2e suite deliberately omits the happy path: a real upload needs a live R2 connection, matching how tests/e2e/post/upload-media.test.ts is written. Everything it does cover is rejected before storage is reached — an SVG sent as image/png, HTML sent as image/jpeg, an empty file, and a filename carrying ../../../etc/passwd.png — which is exactly where the security-relevant behaviour lives.

Note

While verifying this branch I reproduced a pre-existing flake unrelated to these changes: the three argon2 tests in tests/unit/infrastructure/security/password.service.test.ts exceed the 5 s testTimeout when the machine is under load (reproducible by running two suites concurrently — 3 failures, every time). argon2 is deliberately CPU-hard, so a busy CI runner can hit it. Fixing it separately so it does not ride along with a feature.


Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • Chore

Checklist

  • My branch follows the naming convention (feature/, fix/, chore/, docs/)
  • My commits follow Conventional Commits
  • I have tested my changes locally
  • I have not introduced any breaking changes
  • I have updated relevant documentation if needed

🤖 Generated with Claude Code

Stage 4 of the article feature.

  POST /api/v1/articles/cover   auth required, SENSITIVE, one file, 5 MB max

The endpoint returns a storage key, not a URL, and the article body accepts
only that key - validated in stage 2 against the caller's own prefix. Keeping
URLs out of the request is what closes javascript: and data: stored XSS,
external tracking pixels, and one user attaching another user's upload, all
with one rule instead of a sanitizer.

The file type is decided by reading magic bytes. The multipart part's mimetype
and filename are never consulted and never forwarded: both are attacker
controlled, so a request can claim image/png while carrying an SVG, and a name
like cover.png.html reads as an image to an extension check. The bytes are the
only statement about a file the uploader cannot forge.

SVG has no signature to match and is rejected for free, which is the intended
outcome - it is a scriptable document rather than a raster image, and serving
one from the CDN would be stored XSS.

The stored file name is generated rather than derived from the upload, so a
path traversal in the client name cannot reach a storage key. Size is checked
twice: the byte length, and the transport truncation flag, because the
multipart limit truncates silently on some paths.

CryptoPort gains generateUuid so the use case stays free of node imports, in
keeping with how the rest of core reaches randomness.

The happy path is not covered by e2e: a real upload needs a live R2
connection, matching how the post media suite is written. Every case that is
covered is rejected before storage is reached, which is where the
security-relevant behaviour lives.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@aquie00t
aquie00t merged commit 91de09a into main Aug 24, 2026
10 checks passed
@aquie00t
aquie00t deleted the feature/article-cover-upload branch August 24, 2026 23:22
github-actions Bot pushed a commit that referenced this pull request Aug 24, 2026
# [1.4.0](v1.3.0...v1.4.0) (2026-08-24)

### Features

* **article:** add cover image upload ([#209](#209)) ([91de09a](91de09a))
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 1.4.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant