Skip to content

test(relay): give every media e2e upload its own blob hash - #9272

Merged
salman1993 merged 2 commits into
mainfrom
mtoohey/flake-relay-media-e2e
Nov 9, 2026
Merged

test(relay): give every media e2e upload its own blob hash#9272
salman1993 merged 2 commits into
mainfrom
mtoohey/flake-relay-media-e2e

Conversation

@matt2e

@matt2e matt2e commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Split out of the flakey-monday flake-fix series. Two Relay E2E media tests shared one deterministic blob hash across every test in their binaries. In enforce mode (BUZZ_MEDIA_READ_MODE=enforce, on for the Relay E2E lane since #9077), a rejected publish of that shared hash left a sticky publication marker that made ranged GETs from the other tests return 404. Every media e2e upload now has its own blob hash.

Commits (cherry-picked from flakey-monday, original messages kept):

  • test(media): give each video e2e test its own MP4 blob
  • test(media): publish images of their own in the media e2e tests

Root cause and fix per test

test_video_range_request_206 / test_video_range_request_416 (crates/buzz-test-client/tests/e2e_media_video.rs): a ranged GET returned 404 instead of 206/416 in 9 of 55 Relay E2E jobs.

  • Root cause (confirmed): build_test_mp4() was deterministic, so every test in the binary uploaded the same sha256, and the tests run in parallel against one relay. In enforce mode, validate_media_references records a sticky media_publications marker for every hash an event publishes, before dispatch, so a later rejection does not undo it. test_video_poster_imeta_rejects_video_as_poster published the shared hash and was then rejected by imeta validation. From then on media_access gave an uploader with no readable reference Restricted, served as 404, until the accepted poster test stored its event in an open channel.
  • Fix: build_test_mp4() appends a top-level free box holding a random UUID, so each call yields a distinct, still valid fast-start MP4. The upload validator allows free and does not inspect its payload. The relay is unchanged: the pre-dispatch marker is the documented fail-closed contract in docs/operator-private-media.md.

test_video_poster_imeta_accepted_via_ws and e2e_media_extended::test_ws_valid_imeta: hardening, not a fix for an observed failure. Both published a fixed 1x1 JPEG byte-identical to tiny_jpeg() in e2e_media.rs, which test_upload_and_get reads back through an uploader preview. That currently passes (a readable live reference wins before the marker matters), but a publishing test that shares a fixed blob with other media tests is one rejected or private event away from the same flake. Both now publish an 8x8 PNG of random pixels from a new unique_png() helper; test_ws_valid_imeta's imeta declares image/png and the real size; tiny_jpeg() is removed from e2e_media_video.rs.

Related issue

N/A (flake fixes; none found).

Testing

Human testing is still required. This PR does not carry buzz-review-completed; the author has not yet run these tests themselves on this branch.

Verification

These changes were verified locally on the flakey-monday branch before the split, against an enforce-mode relay with docker Postgres/MinIO:

  • Old code, fresh enforce-mode relay: reject test then range tests failed both range tests with 404 at the CI lines (reproduction).
  • Whole e2e_media_video binary, --test-threads=8: 20/20 passed.
  • Reject test then range tests against the polluted hash: 3/3 passed.
  • The CI "Media read-auth e2e" command (e2e_media, e2e_media_extended, e2e_media_video, --ignored), 5 back-to-back runs on the same scratch database (the persistent-relay case): 5/5 runs, 40/40 tests each time.
  • cargo fmt --check and clippy --tests: clean.

Nothing was re-run on this split branch: the workstation was overloaded, so no tests, builds or linters were executed after cherry-picking. CI on this PR is the verification for the split branch.

🤖 Generated with Claude Code

matt2e and others added 2 commits November 9, 2026 15:41
@matt2e @claude
test(media): give each video e2e test its own MP4 blob
a90da9a 
Fixes the Relay E2E flake in test_video_range_request_206 and
test_video_range_request_416 (crates/buzz-test-client/tests/
e2e_media_video.rs), where a ranged GET returned 404 instead of 206/416.
9 of 55 Relay E2E jobs failed this way after #9077 turned on
BUZZ_MEDIA_READ_MODE=enforce for that lane.

Root cause (confirmed): build_test_mp4() was deterministic, so every
test in the binary uploaded the same sha256, and the tests run in
parallel against one relay. In enforce mode, validate_media_references
records a sticky media_publications marker for every hash an event
publishes. It does this before dispatch, so a later rejection does not
undo it. test_video_poster_imeta_rejects_video_as_poster publishes the
shared hash and is then rejected by imeta validation. From then on,
media_access gives an uploader who has no readable reference Restricted
(referenced || published beats UploaderPreview), and the relay serves
that as 404. The window stays open until the accepted poster test
stores its event in an open channel.

Reproduced locally against a fresh enforce-mode relay with the old
code: running the reject test and then the range tests failed both
range tests with 404 at :397 and :448, the CI lines.

Fix: build_test_mp4() appends a top-level `free` box holding a random
UUID, so each call yields a distinct, still valid fast-start MP4. The
upload validator allows `free` and does not inspect its payload.

Product question: should a rejected event be able to consume the
uploader's preview? The current behaviour is the documented contract,
not an ordering bug. docs/operator-private-media.md says the marker
"consumes preview authority before publication dispatch" and "a later
rejected operation may conservatively consume a preview; reupload still
proves publication rights". Db::record_media_publications is documented
as "sticky ... before dispatch, even if the later command fails".
Recording the marker before dispatch keeps the boundary fail-closed
across every dispatch branch (commands, workflows, metadata commands,
storage) without a per-branch write that a crash or failed write could
skip. The uploader's publish rights are unaffected, and a successful
publish restores reads through the live reference. I left the relay
unchanged.

Verified locally (enforce-mode relay, docker Postgres/MinIO):
- the whole e2e_media_video binary, --test-threads=8: 20/20 passed
- reject test, then range tests, against the polluted hash: 3/3 passed
- the CI "Media read-auth e2e" step (e2e_media, e2e_media_extended,
  e2e_media_video): passed

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: mtoohey <mtoohey@squareup.com>
(cherry picked from commit 291ce14eb64d1eb0c31fea7b645d6e69ae97730f)
@matt2e @claude
test(media): publish images of their own in the media e2e tests
80db5f5 
Follow-up to 291ce14eb, from review. Two tests published a fixed
1x1 JPEG that is byte-identical to `tiny_jpeg()` in e2e_media.rs:
- test_video_poster_imeta_accepted_via_ws used it as its poster
- e2e_media_extended::test_ws_valid_imeta used it as its image
e2e_media::test_upload_and_get reads that same hash back through an
uploader preview.

The review expected that GET to 404 against a relay that outlives one
run, because publishing records a sticky publication marker. It does
not, and I checked: on a fresh enforce-mode database, the old poster
test followed by test_upload_and_get passes. media_access returns
Allowed for any readable live reference before the marker matters, and
both publishers post accepted events in open channels. The marker blocks
reads only while a hash has no readable reference: during dispatch, or
for good if the event is rejected or unreadable. That is how the
original video range flake happened.

So this is hardening, not a fix for an observed failure. A publishing
test that shares a fixed blob with other media tests is one rejected or
private event away from that flake again. Both tests now publish an 8x8
PNG of random pixels, from a new unique_png() helper built like
transparent_png() in e2e_media.rs. test_ws_valid_imeta's imeta now
declares image/png and the real size. tiny_jpeg() is gone from
e2e_media_video.rs, which no longer uses it.

Verified locally against an enforce-mode relay on a scratch database:
- the CI "Media read-auth e2e" command (e2e_media, e2e_media_extended,
  e2e_media_video, --ignored), 5 back-to-back runs on the same
  database: 5/5, 40/40 tests each time. Each run's e2e_media followed
  the previous run's publishers, the persistent-relay case.
- old code, fresh database, poster test then test_upload_and_get: both
  pass. That is the non-reproduction described above.
- cargo fmt --check and clippy --tests: clean

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: mtoohey <mtoohey@squareup.com>
(cherry picked from commit f42ac8766b36ca0ded409427f1a0569c2543cf53)
@matt2e
matt2e requested a review from a team as a code owner November 9, 2026 04:44
@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 9, 2026
@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown

🔐 Codex Security Review

Note: This is an automated, security-focused review generated by Codex.
Use it as a supplement to human review; false positives are possible.

Scope

  • Exact PR diff: 65e864301d2e51a93003816b45e77cae96be901f...80db5f58e7d059258657805d2b5b5a93e8b5b476
  • Model: gpt-6.1-sol

💡 Click "edited" above to see earlier reviews for this PR.

Review Summary

Overall Risk: NONE

No concrete findings in the authorized PR range. The changes isolate media test fixtures with unique hashes and preserve compatibility with existing validation and assertions.

Findings

No concrete security, correctness, or reliability findings were identified.

Notes

  • Review used read-only source inspection. Builds and tests were not executed, as requested.

Generated by Codex Security Review |
Requested by: @matt2e |
Workflow run

kazemek21
kazemek21 approved these changes Nov 9, 2026
View reviewed changes

@kazemek21 kazemek21 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kyle reviewed this PR and the Nexus review-tracker notes.

Posted by Codex (gpt-6.1-sol) on behalf of kylez

@salman1993
salman1993 merged commit b082b14 into main Nov 9, 2026
91 checks passed
@salman1993
salman1993 deleted the mtoohey/flake-relay-media-e2e branch November 9, 2026 18:57
fsola-sq added a commit that referenced this pull request Nov 9, 2026
@fsola-sq
Merge remote-tracking branch 'origin/main' into fsola/nip-fi-verified…
c40c3b9 
…-profile

* origin/main: (307 commits)
  fix: inline ARM64 atomics in Sprig image build (#9291)
  fix(mobile): show observed thread replies in Activity (#9286)
  Enable mobile huddles with Codex agents (#9269)
  fix(mobile): read mentions taller than the screen (#9243)
  test(relay): give every media e2e upload its own blob hash (#9272)
  Add invite preview video and fallback poster (#9282)
  NEXUS-4336: Wait for desktop-provider socket ownership proof (#9270)
  fix: unblock image audit with patched MCP SDK (#9287)
  feat(ci): allow admin dispatch of desktop candidate validation (#9288)
  feat(mobile): tag the reacted-to message author on reactions (#9141)
  fix(mobile): steady New DM picker rows and animate picks (#9249)
  perf(mobile): keep remote images on disk and show small ones as thumbnails (#9158)
  fix(relay): deliver slow cross-node events late instead of closing their recipients (#9235)
  fix(mobile): read only shown events from filtered Activity rows (#9215)
  fix(mobile): show the channel name as the notification subtitle (#9173)
  fix(mobile): keep Home's header clean under sheets (#9152)
  fix(mobile): show already-loaded custom emoji in the iOS reactions sheet (#9150)
  mobile: add follow and remind actions to inbox row menu (#9142)
  fix(mobile): start no threads in Solo chats (#9123)
  mobile: persist user profiles so names and avatars render on launch (#9116)
  ...

Signed-off-by: Franco Sola <fsola@squareup.com>

# Conflicts:
#	docs/nips/NIP-FA.md
#	docs/nips/NIP-FI.md
wpfleger96 added a commit that referenced this pull request Nov 9, 2026
@wpfleger96
Merge remote-tracking branch 'origin/main' into duncan/workflow-secur…
4bff63e 
…e-mode

* origin/main: (36 commits)
  feat(relay): let an operator mint invites for a community owner or admin (#9265)
  fix: inline ARM64 atomics in Sprig image build (#9291)
  fix(mobile): show observed thread replies in Activity (#9286)
  Enable mobile huddles with Codex agents (#9269)
  fix(mobile): read mentions taller than the screen (#9243)
  test(relay): give every media e2e upload its own blob hash (#9272)
  Add invite preview video and fallback poster (#9282)
  NEXUS-4336: Wait for desktop-provider socket ownership proof (#9270)
  fix: unblock image audit with patched MCP SDK (#9287)
  feat(ci): allow admin dispatch of desktop candidate validation (#9288)
  feat(mobile): tag the reacted-to message author on reactions (#9141)
  fix(mobile): steady New DM picker rows and animate picks (#9249)
  perf(mobile): keep remote images on disk and show small ones as thumbnails (#9158)
  fix(relay): deliver slow cross-node events late instead of closing their recipients (#9235)
  fix(mobile): read only shown events from filtered Activity rows (#9215)
  fix(mobile): show the channel name as the notification subtitle (#9173)
  fix(mobile): keep Home's header clean under sheets (#9152)
  fix(mobile): show already-loaded custom emoji in the iOS reactions sheet (#9150)
  mobile: add follow and remind actions to inbox row menu (#9142)
  fix(mobile): start no threads in Solo chats (#9123)
  ...

Signed-off-by: Will Pfleger <wpfleger@block.xyz>

This branch was previously deployed

1 inactive deployment
codex-review — 80db5f58 Deployed Nov 9, 2026 by matt2e via Run Codex Security Review #13949
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kazemek21 kazemek21 kazemek21 approved these changes

Assignees

No one assigned

Labels

codex-security-review-current The posted Codex security review matches its recorded range.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@matt2e @kazemek21 @salman1993