Skip to content
This repository was archived by the owner on Jul 16, 2026. It is now read-only.

feat(dm-result): multipart upload + shared send_allowlist (Phase 5.9) - #77

Merged
jgluck96 merged 1 commit into
dev-clientfrom
josh/phase-5.9-dm-result-upload
Jun 2, 2026
Merged

jgluck96 merged 1 commit into
dev-clientfrom
josh/phase-5.9-dm-result-upload

Conversation

@jgluck96

@jgluck96 jgluck96 commented Jun 2, 2026

Copy link
Copy Markdown

Summary

Phase 5.9 of the OSS→private sync. Three coordinated changes that close the REST-fallback file-delivery gap and remove the drift hazard between the WS-bridge and dm-result.py.

Change Shape Impact
src/send_allowlist.py (new) Verbatim port from OSS Single source of truth for SEND_ALLOWED_ROOTS / SEND_ALLOWED_PREFIXES / is_path_sendable
src/dm-result.py upload New _send_message_with_files + send_dm branch REST path now actually delivers files when voice is disconnected
src/discord-bridge.py adopt Replace inline allowlist with import + alias Removes drift hazard; zero behavior change at sink call sites

Pre-port behavior

Post-port behavior

  • Files emitted while voice is disconnected reach the DM via multipart/form-data POST to /channels/{id}/messages.
  • Both consumers reference the same SEND_ALLOWED_* tuples and the same is_path_sendable function — identity-checked in the new tests, so a future PR that re-inlines a copy fails CI immediately.
  • All four existing discord.File(fpath) sink call sites in the bridge are unchanged because the bridge keeps the local name _is_path_sendable as an alias to the shared function.

Minimal-divergence accounting

  • New file send_allowlist.py: zero conflict surface against future OSS pulls (file name + body byte-identical to OSS).
  • dm-result.py: net-additive — the marker-extraction guard already existed in stando; this PR fills in the multipart upload that the existing OSS comment block in stando flagged as "follow-up."
  • discord-bridge.py: replaces ~26 lines with ~16 lines (import + alias). The 4 sink call sites are unchanged.
  • _resolve_owner_id deliberately untouched — stando uses its own tier-map resolver, OSS uses discord_config.py. Mixing them is a separate phase.

Test plan

New tests

  • tests/send-allowlist.test.py — policy semantics (nonexistent rejects, directory rejects, allowed prefix matches, arbitrary rejects, symlink/realpath collapse defeats path-injection, dot-dot traversal rejects) + identity-level drift guards (both consumers reference the SAME tuple/function objects, not copies) + documented-prefix set guard.
  • tests/dm-result-upload.test.py — 5 regression guards for the new upload flow:
    1. Sendable file → text + multipart upload
    2. Disallowed file → text-only, no upload
    3. File-only body → multipart with empty content (no JSON message post)
    4. 10 files → 2 batches at Discord per-message cap

    5. Multipart body shape (boundary + payload_json + files[N] parts + filename sanitization defeats header-injection via CR/LF/quote)

Updated test

Regression sweep (all green)

  • 19 Python tests: discord-bridge-{allowlist, file-markers, file-send-allowlist, pending-replies, prose-marker, reply-chunks, state-prefetch, access-tier}, dm-result-send-dm, bridge-marker-no-leak, result-markers, discord-chunker, single-instance, agent-api-result-archive, health-check-phase-57, plus the 2 new files (+ the updated allowlist test).
  • 15 TypeScript tests: task-bridge-isvoice, web-server-presenter, client-dist-dir-override.
  • pnpm typecheck clean.

Manual verification

After merge, the following invocation will deliver an attached screenshot to the owner DM when voice is disconnected:

python3 src/dm-result.py "Here's the screenshot: [file: /tmp/sutando-test.png]"

Pre-port: the file silently dropped; only the text-without-marker landed.
Post-port: the file is uploaded as a real Discord attachment alongside the text.

Made with Cursor

Three coordinated changes that close the REST-fallback file-delivery
gap and remove the drift hazard between the WS-bridge and dm-result.

1. src/send_allowlist.py (new, verbatim port from OSS sutando)
   - Extracts SEND_ALLOWED_ROOTS, SEND_ALLOWED_PREFIXES, and the
     realpath-based is_path_sendable() sanitizer into one module.
   - Identical policy values to the prior inline copy in
     discord-bridge.py — this is a pure refactor, not a tightening
     or loosening.
   - Per the liususan091219 review on the OSS PR sonichi#1029: keeping the
     policy as a copy in each consumer drifts even with "keep in
     sync" comments. Pre-extract on stando, the dm-result inline
     copy was already missing the notes/Desktop/Documents roots
     that discord-bridge had.

2. src/dm-result.py: multipart upload path
   - Adds _send_message_with_files() (multipart/form-data POST to
     /channels/{id}/messages with payload_json + files[N] parts).
   - send_dm() now extracts [file:|send:|attach:] markers,
     allowlist-checks each path via the shared is_path_sendable,
     chunks the text as before, then uploads sendable files in
     batches of 10 (Discord per-message cap).
   - Pre-port: dm-result stripped markers and logged them as
     dropped because REST multipart wasn't implemented. Owner
     missed every file delivered when voice was disconnected.
   - Filename sanitization mirrors discord-bridge's
     _safe_attachment_basename (CR/LF/quote -> _) so a filename
     can't inject its own multipart headers.

3. src/discord-bridge.py: adopt the shared helper
   - Replaces inline SEND_ALLOWED_ROOTS / SEND_ALLOWED_PREFIXES /
     _is_path_sendable with `from send_allowlist import ...`.
   - Local name `_is_path_sendable` preserved as an alias to the
     shared function so every existing sink call site (4 sites
     across DM/voice/proactive paths) stays unchanged.

Tests
-----
- tests/send-allowlist.test.py (new): policy semantics (nonexistent
  rejects, directory rejects, allowed prefix matches, arbitrary
  rejects, symlink/realpath collapse defeats path-injection,
  dot-dot traversal rejects) + identity-level drift guards that
  both consumers reference the SAME tuple/function objects (a copy
  that equal-compared at module-load would silently drift later) +
  the documented-prefix set guard.
- tests/dm-result-upload.test.py (new): 5 regression guards for
  the new upload flow — sendable file -> text+multipart, disallowed
  file -> text-only no upload, file-only body -> multipart with
  empty content, >10 files -> 2 batches at Discord cap, multipart
  body shape (boundary + payload_json + files[N] parts + filename
  sanitization on the wire).
- tests/discord-bridge-allowlist.test.py (updated): structural
  sanitizer regression guard now inspects send_allowlist.py for
  the body (#1-#4) and verifies discord-bridge.py imports +
  aliases the helper (#5) before re-checking the discord.File()
  gate (#6). Same CodeQL-style guarantees; follows the
  implementation.

Verification
------------
- python3 -m py_compile on all three modified source files: OK
- New tests: 14 cases, all green.
- Existing regression sweep: discord-bridge-{allowlist,
  file-markers, file-send-allowlist, pending-replies, prose-marker,
  reply-chunks, state-prefetch, access-tier}, dm-result-send-dm,
  bridge-marker-no-leak, result-markers, discord-chunker,
  single-instance, agent-api-result-archive, health-check-phase-57,
  task-bridge-isvoice, web-server-presenter,
  client-dist-dir-override -> 19 Python + 15 TS, all green.
- `pnpm typecheck` clean.

Phase 5.9 of the OSS->private sync. No behavioral change to the
WS-connected bridge — the alias preserves every existing call
site. The user-visible win is on the REST-fallback path: files
emitted while voice is disconnected now actually reach the DM.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant