Skip to content

fix(bb-file-bucket): harden dev file server against stored XSS and token forgery - #292

Merged
osama-rizk merged 2 commits into
mainfrom
fix/file-bucket-dev-server-xss
Aug 5, 2026
Merged

fix(bb-file-bucket): harden dev file server against stored XSS and token forgery#292
osama-rizk merged 2 commits into
mainfrom
fix/file-bucket-dev-server-xss

Conversation

@osama-rizk

Copy link
Copy Markdown
Contributor

What

Harden the local dev file server (packages/bb-file-bucket/src/file-server.ts) against two issues in the same code path:

  1. Stored XSS. GET served the stored object body inline under an attacker-controlled Content-Type. Any client with a presigned PUT can upload a text/html/SVG payload; served inline it executes in the local app's origin. Now every download sends:
    • X-Content-Type-Options: nosniff
    • Content-Disposition: attachment
  2. Forgeable signing secret. Presigned-URL tokens were HMAC-signed with a hardcoded, source-visible literal ('__blocks_file_bucket_dev_secret__'), so anyone could mint a valid token offline for any fullId/path/method without ever calling getUrl()/putUrl(). LOCAL_FILE_SECRET is now a per-process random value (randomBytes(32)). The token-minting mock (index.mock.ts) and the validating dev server (file-server.ts) import the same in-process module instance, so local round-trips are unaffected; only offline forgery is closed.

LOCAL_FILE_SECRET and tokens.ts are internal (not in the package exports map, not in API.md), so this is not a public API change.

Why

Board bug-bash card (P0 / Security): bb-file-bucket: dev mock signs uploads with hardcoded secret + serves text/htmlhttps://github.com/orgs/aws-amplify/projects/141/views/15?pane=issue&itemId=195528277

Keeps local dev no weaker than an S3 object served behind CloudFront.

Testing

Two regression tests added to file-server.test.ts, both verified red on the old code, green with the fix:

  • GET sends nosniff + Content-Disposition: attachment after uploading an HTML payload.

  • A token forged with the former hardcoded secret is rejected (403).

  • npm run build (full monorepo) — passes

  • npm test in packages/bb-file-bucket — 92 tests, 0 failures

  • biome lint --changed --since=origin/main — clean

  • DESIGN.md updated (token-secret + download-hardening notes)

Changeset

Included (@aws-blocks/bb-file-bucket patch).

Scope note

The card mentions the mock also "serves whatever was uploaded as text/html". This PR addresses that via response hardening (nosniff + attachment) rather than rewriting content-type handling, and closes the hardcoded-secret half. Both halves of the card are covered.

…ken forgery

The local dev file server served attacker-controlled bodies inline under an
attacker-controlled Content-Type (stored-XSS primitive), and signed presigned-URL
tokens with a hardcoded, source-visible secret (forgeable offline).

- GET responses now send X-Content-Type-Options: nosniff and
  Content-Disposition: attachment so an uploaded text/html/SVG payload can't
  execute in the app origin.
- LOCAL_FILE_SECRET is now a per-process random value instead of a fixed literal.
  The token-minting mock and the validating dev server share one in-process
  module instance, so local presigned-URL round-trips still work.

Adds regression tests (both verified red on the old code): nosniff+attachment on
GET, and rejection of a token forged with the former hardcoded secret.
@osama-rizk
osama-rizk requested a review from a team as a code owner August 3, 2026 10:11
@changeset-bot

changeset-bot Bot commented Aug 3, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: f7c59c8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@aws-blocks/bb-file-bucket Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@osama-rizk
osama-rizk merged commit bd59e60 into main Aug 5, 2026
45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants