Skip to content

fix(cli): preserve attachments across successive edits - #9226

Open
bradseiler wants to merge 5 commits into
mainfrom
honor/media-edit-attachments
Open

bradseiler wants to merge 5 commits into
mainfrom
honor/media-edit-attachments

Conversation

@bradseiler

Copy link
Copy Markdown
Contributor

Summary

Editing attachment messages through buzz messages edit can remove their files from the effective message. Preserve the latest live imeta set across successive edits and add --clear-attachments for explicit removal. Add an SDK builder for complete replacement attachment sets while preserving the existing API.

The relay provides a versioned effective-edit lookup on /query. It uses the existing normalized-target index, fresh channel authorization, and a primary snapshot. Generic #e matching is unchanged. Mixed-case historical edits cannot restore removed files, including when another reference would allow republication.

Related issue

Fixes #3022. Follows #9077. Related open attempts: #3062 and #3065.

Testing

No manual testing. Automated coverage invokes the real CLI entrypoint against local HTTP/ingest/media handlers in enforce mode. It checks successive edits, GET/HEAD, explicit and partial removal, and the mixed-case target witness. A database regression seeds 10,001 unrelated edits and checks the target index plan.

Draft validation is still in progress. The expanded local PostgreSQL run currently has two test-fixture failures: a parent-versus-child index name assertion and a test-thread stack overflow. Corrections are prepared. All 3,030 non-ignored SDK/CLI/DB/relay tests passed at e8b1cf0527. Mandatory push hooks are running; Tauri clippy has passed. Independent review is in progress.

Rollout

Deploy the relay first, then release the CLI. The new CLI aborts without publishing if an older relay ignores the lookup extension. Old clients keep their previous behavior against the new relay. This change has no feature flag and needs no new schema beyond migration 0086.

Enforce remains blocked for the separate block/buzz-app MCP edit producer and unaudited direct SDK publishers. The compatibility build_edit helper still removes attachments; integrations must adopt build_edit_with_media and effective state. Agent ownership grants no private-channel media access. A retaining edit is rejected if any carried file is not publishable by its signer.

Observe buzz_media_publication_access_total{mode="shadow",reason="denied"} before enforcement. Roll back the CLI release while retaining the additive lookup. Concurrent message edits remain ordered by (created_at,id); the lookup adds no compare-and-swap precondition.

Generated with Codex

bradseiler and others added 4 commits November 6, 2026 18:10
Carry the latest live replacement metadata and provide explicit clearing. Order successive edits after their observed head and fail on incomplete history.

Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz>
Signed-off-by: Brad Seiler <seiler@squareup.com>
Resolve live originals and the latest edit in a primary snapshot. Require a versioned response so older relays cannot silently provide incomplete attachment state.

Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz>
Signed-off-by: Brad Seiler <seiler@squareup.com>
Recognize partition child indexes, box large CLI and ingest futures, and verify fresh reader authorization through HTTP. Include both journeys in the media acceptance recipe.

Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz>
Signed-off-by: Brad Seiler <seiler@squareup.com>
Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz>
Signed-off-by: Brad Seiler <seiler@squareup.com>
@bradseiler
bradseiler marked this pull request as ready for review November 7, 2026 00:00
@bradseiler
bradseiler requested a review from a team as a code owner November 7, 2026 00:00
@bradseiler
bradseiler deployed to codex-review November 7, 2026 00:00 — with GitHub Actions Active
@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 7, 2026
@github-actions

github-actions Bot commented Nov 7, 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: 96af488d968a1ae9d961c2132bea27822735f8da...dc2ca42990b91f08a647ab1bcc1876c91cded373
  • Model: gpt-6.1-sol

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


Review Summary

Overall Risk: NONE

No concrete security, correctness, or reliability findings were identified in the authorized PR range.

Findings

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

Notes

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

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

@wpfleger96 wpfleger96 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤖 Approving. I checked the parts that carry the risk:

  • Read authorization on the new effective_edit lookup. It runs after relay-member admission and the existing /query gates. Its SQL rule (channel not deleted, and visibility='open' or an active channel_members row) matches what get_readable_named_only_channels grants a filter that names the channel. It then applies event_visible_to_reader to both the original and the edit, so a reader gets nothing here that the normal explicit-channel read wouldn't give them. Missing and inaccessible both return the same 404.
  • Edit selection. The latest-edit SQL doesn't filter by pubkey. That's fine because validate_edit_ownership only accepts 40003s from the effective author or the owning human, so every stored live edit already comes from an authorized editor. Carried files are still publication-checked per signer.
  • CLI. It refuses to publish unless it gets the exact v1 envelope back, so a new CLI against an old relay fails closed. It re-checks the returned original's id, kind and channel. Stamping max(now, head+1) with a 60s ceiling stays well inside the relay's 900s future bound.
  • SDK. Rejecting non-imeta tags in build_edit_with_media closes off injecting a target, channel or auth tag.

Nonblocking:

  • The PostgreSQL and E2E lanes were still running when I looked, and the description mentions two Postgres fixture fixes that haven't been pushed yet. Once they land, the Testing paragraph's in-progress status can come out.
  • buzz-relay now has a dev-dependency on buzz-cli just for the CLI end-to-end test. That's reasonable, but it does grow the relay's test build graph.

Share the attachment-governed kind set across the CLI, lookup and media reads. Preserve attachments through edits and keep explicit clear effective for diff and forum messages. Add isolated access journeys and a historical exposure preflight.

Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz>
Signed-off-by: Brad Seiler <seiler@squareup.com>
@github-actions github-actions Bot removed the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 7, 2026
@bradseiler
bradseiler deployed to codex-review November 7, 2026 01:03 — with GitHub Actions Active
@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 7, 2026

This branch was successfully deployed

1 active deployment
codex-review — dc2ca429 Deployed Nov 7, 2026 by bradseiler via Run Codex Security Review #13702
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

buzz messages edit silently deletes the message's attachments

2 participants