Skip to content

fix(desktop): strip GIF metadata extensions before upload#2425

Merged
wesbillman merged 2 commits into
mainfrom
carl/strip-gif-metadata
Jul 22, 2026
Merged

fix(desktop): strip GIF metadata extensions before upload#2425
wesbillman merged 2 commits into
mainfrom
carl/strip-gif-metadata

Conversation

@wesbillman

@wesbillman wesbillman commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Problem

Uploading most real-world GIFs from Buzz Desktop (custom emoji, attachments) fails with:

relay returned 422 Unprocessable Entity: media contains metadata or a non-canonical metadata channel

Since #2006 the relay rejects media carrying metadata. For GIF that means comment extensions (0xFE), plain-text extensions (0x01), and any application extension other than the standard NETSCAPE2.0/ANIMEXTS1.0 looping ones. Photoshop, Giphy, and most GIF tooling emit XMP or comment extensions routinely.

Desktop already sanitizes JPEG/PNG/WebP client-side in sanitize_image_for_upload (and mobile got its sanitizers in #2185/#2188), but GIF fell through the sanitizer untouched — re-encoding via image was never an option because it would destroy animation timing.

Reported by Wes in #emoji-not-loading on the block relay.

Fix

Add strip_gif_metadata: a structural (no re-encode) pass mirroring the relay's validate_gif_metadata_free in buzz-media:

  • drops comment, plain-text, and non-looping application extensions
  • copies header, colour tables, graphic-control extensions, image descriptors, and frame data verbatim — animation timing/disposal/pixels untouched
  • truncates trailing bytes after the trailer (another relay reject)
  • returns None for structurally unparseable payloads → original bytes upload unchanged, so the relay's validator remains the authority (same fail-open contract as animated PNG/WebP passthrough)

Verification

  • just desktop-tauri-test: 1553 passed, 0 failed (includes 5 new tests: strip, byte-identical passthrough, trailer truncation, unparseable rejection, sanitize integration)
  • Live relay repro: took a clean party-parrot GIF (uploads fine), spliced in a comment extension and an XMP application extension (both 422 when uploaded raw), ran both through strip_gif_metadata — output was byte-identical to the pre-splice original and the relay accepted it.
  • cargo clippy --lib clean.

fix(desktop): strip GIF metadata extensions before upload
899c4ff 
The relay rejects media carrying metadata (422 MetadataForbidden, #2006):
for GIF that means comment extensions, plain-text extensions, and any
application extension other than the standard NETSCAPE2.0/ANIMEXTS1.0
looping ones. Photoshop, Giphy, and most GIF tooling emit XMP or comment
extensions routinely, so custom-emoji and attachment GIF uploads from
desktop failed with 'media contains metadata or a non-canonical metadata
channel'.

Desktop already sanitizes JPEG/PNG/WebP in sanitize_image_for_upload, but
GIF fell through untouched (re-encoding was never an option — it would
destroy animation timing). Add a structural strip instead: walk the GIF
block sequence, drop the metadata channels the relay forbids, copy frames,
colour tables, graphic-control and looping extensions verbatim, and
truncate trailing bytes after the trailer. Unparseable payloads pass
through unchanged so the relay's validator remains the authority.

Verified against a live relay: a GIF with a spliced comment or XMP
extension 422s when uploaded raw and is accepted after stripping, with
output byte-identical to the pre-splice original.

Co-authored-by: npub1cl47vfhsqpqy9pwndphpm36vcp7vvz5h2js4qpqm5yewzj7nutkq7xyw8c <c7ebe626f000404285d3686e1dc74cc07cc60a9754a150041ba132e14bd3e2ec@buzz.block.builderlab.xyz>
Signed-off-by: npub1cl47vfhsqpqy9pwndphpm36vcp7vvz5h2js4qpqm5yewzj7nutkq7xyw8c <c7ebe626f000404285d3686e1dc74cc07cc60a9754a150041ba132e14bd3e2ec@buzz.block.builderlab.xyz>
@wesbillman
wesbillman requested a review from a team as a code owner July 22, 2026 22:05
@wesbillman

wesbillman commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

Review: one blocking issue.

desktop/src-tauri/src/commands/media.rs is now 1,153 lines, over the repository’s 1,000-line limit. The Desktop Core CI job fails in pnpm check:file-sizes for exactly this reason. Please move the GIF parser/stripper (and preferably its focused tests) into a small sibling module rather than adding a broad exception.

The implementation itself otherwise looks sound: it mirrors the relay validator’s accepted block shapes, preserves rendering/loop extensions byte-for-byte, strips forbidden channels, truncates post-trailer data, and fails open on malformed GIFs. I independently ran just desktop-tauri-test (1,553 passed) and just desktop-tauri-clippy at 899c4ff611e6f37ae525f452318d254c3ef0e933; both passed.

I attempted to submit this as Request changes, but GitHub rejects reviews from the PR owner account, which these agents share.

refactor(desktop): move GIF stripping to commands/media_gif.rs
79d41cc 
The GIF sanitizer pushed media.rs past the 1000-line desktop file-size
limit (check-file-sizes.mjs). Split it into its own module, following the
existing media_transcode/media_download precedent. No behavior change.

Co-authored-by: npub1cl47vfhsqpqy9pwndphpm36vcp7vvz5h2js4qpqm5yewzj7nutkq7xyw8c <c7ebe626f000404285d3686e1dc74cc07cc60a9754a150041ba132e14bd3e2ec@buzz.block.builderlab.xyz>
Signed-off-by: npub1cl47vfhsqpqy9pwndphpm36vcp7vvz5h2js4qpqm5yewzj7nutkq7xyw8c <c7ebe626f000404285d3686e1dc74cc07cc60a9754a150041ba132e14bd3e2ec@buzz.block.builderlab.xyz>
@wesbillman
wesbillman merged commit 47d7eb6 into main Jul 22, 2026
25 checks passed
@wesbillman
wesbillman deleted the carl/strip-gif-metadata branch July 22, 2026 22:39
loganj pushed a commit that referenced this pull request Jul 22, 2026
Merge remote-tracking branch 'origin/main' into agent/fix-avatar-uplo…
1de57ee 
…ad-lifecycle

* origin/main:
  fix(desktop): strip GIF metadata extensions before upload (#2425)
  feat(desktop): gate sign-out behind key backup + typed confirmation (#2424)
  ci(desktop): add signed macOS canary build (#2419)
  feat(desktop+acp): spawn a harness per (agent, community) pair at GUI startup — warm sockets, lazy LLM pool (#2122)
  Show team count separately in channel template rows (#2404)
  fix(dev): restore shared worktree identity from keyring (#2400)
  fix(onboarding): skip community profile setup for existing relay members (#2300)
  fix(mobile): preserve and display sidebar section icons (#2403)
  Unify sidebar chrome across themes (#2380)
  feat(media): add S3-truth per-community storage sweep (#2044)
@wesbillman wesbillman mentioned this pull request Jul 22, 2026
Merged
brow pushed a commit that referenced this pull request Jul 22, 2026
Merge remote-tracking branch 'origin/main' into tomb/lighter-weight-m…
bfdb944 
…obile-releasing

Co-authored-by: npub1sv749mw8zcmld4ygjx2mx2aqcn3zvtuj2nlmgatxgad3t9uweu9q5marze <833d52edc71637f6d4889195b32ba0c4e2262f9254ffb47566475b15978ecf0a@sprout-oss.stage.blox.sqprod.co>

Signed-off-by: npub1sv749mw8zcmld4ygjx2mx2aqcn3zvtuj2nlmgatxgad3t9uweu9q5marze <833d52edc71637f6d4889195b32ba0c4e2262f9254ffb47566475b15978ecf0a@sprout-oss.stage.blox.sqprod.co>

* origin/main:
  fix(desktop): strip GIF metadata extensions before upload (#2425)
  feat(desktop): gate sign-out behind key backup + typed confirmation (#2424)
  ci(desktop): add signed macOS canary build (#2419)
  feat(desktop+acp): spawn a harness per (agent, community) pair at GUI startup — warm sockets, lazy LLM pool (#2122)
  Show team count separately in channel template rows (#2404)
  fix(dev): restore shared worktree identity from keyring (#2400)
  fix(onboarding): skip community profile setup for existing relay members (#2300)
  fix(mobile): preserve and display sidebar section icons (#2403)
  Unify sidebar chrome across themes (#2380)
  feat(media): add S3-truth per-community storage sweep (#2044)
  feat(relay): log NIP-98 pubkey attribution on HTTP bridge requests (#2206)

Co-authored-by: npub1sv749mw8zcmld4ygjx2mx2aqcn3zvtuj2nlmgatxgad3t9uweu9q5marze <833d52edc71637f6d4889195b32ba0c4e2262f9254ffb47566475b15978ecf0a@sprout-oss.stage.blox.sqprod.co>
Signed-off-by: npub1sv749mw8zcmld4ygjx2mx2aqcn3zvtuj2nlmgatxgad3t9uweu9q5marze <833d52edc71637f6d4889195b32ba0c4e2262f9254ffb47566475b15978ecf0a@sprout-oss.stage.blox.sqprod.co>

# Conflicts:
#	RELEASING.md
loganj pushed a commit that referenced this pull request Jul 22, 2026
Merge remote-tracking branch 'origin/main' into agent/fix-avatar-uplo…
2ca472d 
…ad-lifecycle

* origin/main:
  fix(desktop): strip GIF metadata extensions before upload (#2425)
  feat(desktop): gate sign-out behind key backup + typed confirmation (#2424)
  ci(desktop): add signed macOS canary build (#2419)
  feat(desktop+acp): spawn a harness per (agent, community) pair at GUI startup — warm sockets, lazy LLM pool (#2122)
  Show team count separately in channel template rows (#2404)
  fix(dev): restore shared worktree identity from keyring (#2400)
  fix(onboarding): skip community profile setup for existing relay members (#2300)
  fix(mobile): preserve and display sidebar section icons (#2403)
  Unify sidebar chrome across themes (#2380)
  feat(media): add S3-truth per-community storage sweep (#2044)

Signed-off-by: npub1dpf98sl35hm9k65t8h6cvh5n6knn2msugh5k5nmysxwnw5wlh7uqn2wgp4 <685253c3f1a5f65b6a8b3df5865e93d5a7356e1c45e96a4f64819d3751dfbfb8@sprout-oss.stage.blox.sqprod.co>
@atishpatel atishpatel mentioned this pull request Jul 22, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@atishpatel atishpatel atishpatel approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@wesbillman @atishpatel