From e535d42b17c210be5ec658303069a7d5f0ad97d8 Mon Sep 17 00:00:00 2001 From: jgluck96 Date: Mon, 1 Jun 2026 20:06:43 -0400 Subject: [PATCH] feat(dm-result): multipart upload + shared send_allowlist (Phase 5.9) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #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 --- src/discord-bridge.py | 59 ++-- src/dm-result.py | 164 ++++++++-- src/send_allowlist.py | 109 +++++++ tests/discord-bridge-allowlist.test.py | 111 +++++-- tests/dm-result-upload.test.py | 404 +++++++++++++++++++++++++ tests/send-allowlist.test.py | 236 +++++++++++++++ 6 files changed, 985 insertions(+), 98 deletions(-) create mode 100644 src/send_allowlist.py create mode 100644 tests/dm-result-upload.test.py create mode 100644 tests/send-allowlist.test.py diff --git a/src/discord-bridge.py b/src/discord-bridge.py index 4709542e0a..d00bfce6eb 100644 --- a/src/discord-bridge.py +++ b/src/discord-bridge.py @@ -138,26 +138,18 @@ def _push_vision_image(path: str, source: str = "discord") -> bool: # type: ign ARCHIVE_RESULTS_DIR = REPO / "results" / "archive" OWNER_ACTIVITY_FILE = STATE_DIR / "last-owner-activity.json" -# Allowlist for paths that may be attached to outgoing Discord messages. -# Result text can embed `[file: /path]` / `[send: /path]` / `[attach: /path]` -# markers; we only forward paths that resolve under one of these roots. -# Fail-closed: a non-matching path is reported inline rather than sent. -SEND_ALLOWED_ROOTS = ( - str(REPO / "results"), - str(REPO / "notes"), - # Notes canonical home (private dir) — once saved by save_note, paths - # reference the private location. Both old and new paths allowed during - # the transition; the resolver picks whichever exists. - str(shared_personal_path("notes", REPO)), - str(REPO / "docs"), - str(Path.home() / "Desktop" / "iclr-backups"), - str(Path.home() / "Documents" / "sutando-launch-assets"), -) -SEND_ALLOWED_PREFIXES = ( - "/tmp/sutando-", - "/private/tmp/sutando-", - "/tmp/echo-", - "/private/tmp/echo-", +# Allowlist for paths attached via `[file:|send:|attach:]` markers. +# Single source of truth is `src/send_allowlist.py` — shared with +# `src/dm-result.py`'s REST-fallback delivery path. Per the OSS +# liususan091219 review on PR #1029: keeping the policy as a copy in +# each file will drift even with "keep in sync" comments. The extract +# removes that hazard at the boundary. The policy values are byte- +# identical to the pre-extract inline copy here (this is a pure +# refactor, not a tightening/loosening of allowed roots). +from send_allowlist import ( # noqa: E402 + SEND_ALLOWED_PREFIXES, + SEND_ALLOWED_ROOTS, + is_path_sendable as _is_path_sendable_shared, ) @@ -347,27 +339,12 @@ def _split_file_markers(text: str) -> tuple[str, list[str]]: return clean_text, files -def _is_path_sendable(fpath: str) -> bool: - """True iff `fpath` is a real file AND resolves under an allowed root. - - Uses os.path.realpath to collapse symlinks / `..` segments before the - prefix comparison, matching the sanitizer pattern used across the - codebase for CodeQL py/path-injection resolution. - """ - if not os.path.isfile(fpath): - return False - try: - real = os.path.realpath(fpath) - except OSError: - return False - for root in SEND_ALLOWED_ROOTS: - root_real = os.path.realpath(root) - if real == root_real or real.startswith(root_real + os.sep): - return True - for prefix in SEND_ALLOWED_PREFIXES: - if real.startswith(prefix): - return True - return False +# Public name preserved (`_is_path_sendable`) so all existing call sites +# in this bridge (write_owner_activity downstream + the four send paths) +# continue to work unchanged. The shared helper lives in +# `src/send_allowlist.py`; this alias is the single boundary between the +# bridge and the shared policy module. +_is_path_sendable = _is_path_sendable_shared def write_owner_activity(channel: str, summary: str) -> None: diff --git a/src/dm-result.py b/src/dm-result.py index eefd61eb9f..ff3c1545e2 100644 --- a/src/dm-result.py +++ b/src/dm-result.py @@ -38,18 +38,32 @@ ACCESS_JSON = Path.home() / ".claude" / "channels" / "discord" / "access.json" SSE_STATUS_URL = "http://localhost:8080/sse-status" +# Path allowlist for `[file:|send:|attach:]` markers — sourced from +# `src/send_allowlist.py` so this REST-fallback path uses the SAME +# policy as the WS-connected live bridge (`src/discord-bridge.py`). +# Per @liususan091219 review on the OSS PR #1029: a copied allowlist +# will drift even with a comment claiming they're in sync — the +# extract removes that hazard at the boundary. Ported in stando +# Phase 5.9 of the OSS→private sync. +from send_allowlist import ( # noqa: E402 + is_path_sendable as _is_path_sendable, + SEND_ALLOWED_PREFIXES as _SEND_ALLOWED_PREFIXES, # re-exported for tests + SEND_ALLOWED_ROOTS as _SEND_ALLOWED_ROOTS, # re-exported for tests +) + _FENCE_LINE = re.compile(r"^\s{0,3}(`{3,}|~{3,})\s*([^\s`~][^`~]*)?\s*$") # Mirror of discord-bridge.py's `_FILE_MARKER_RE` — agent-emitted file # attachment markers embedded in result bodies. dm-result.py is the -# REST-only fallback delivery path used when voice isn't connected; -# without parsing these markers it would deliver the literal text -# `[file: /tmp/sutando-x.png]` in the DM and silently drop the -# attachment. PR limitation: REST multipart upload for actual file -# delivery is a follow-up — this commit strips the markers from the -# body so the user doesn't see the literal text. See dm-result_TODO. +# REST-only fallback delivery path used when voice isn't connected. +# The WS-connected live bridge attaches files via `discord.File(path)`; +# this REST path builds the equivalent multipart upload (see +# `_send_message_with_files`). Each marker path is allowlist-checked +# against `_is_path_sendable` — same policy as discord-bridge.py via +# the shared `send_allowlist` import — to bound exfil if an attacker- +# controlled marker ever reaches a result body. _FILE_MARKER_RE = re.compile(r'\[(?:file|send|attach):\s*((?:/|~/)[^\]:]+)\]') @@ -196,6 +210,79 @@ def _discord_api(method, path, token, body=None): return json.loads(raw) if raw else None +def _send_message_with_files(channel_id: str, token: str, content: str, + file_paths: "list[str]"): + """POST a message to a channel as multipart/form-data with attached + files. Used by `send_dm()` when an agent-emitted result body + contains `[file:|send:|attach:]` markers — the WS-connected live + bridge calls `discord.File(path)` directly; this REST path needs + to assemble the same multipart payload by hand. + + `content` may be empty (file-only message). `file_paths` are the + already-allowlisted absolute paths. Raises on non-2xx; caller + handles per-file errors. + + Discord docs: POST /channels/{id}/messages with + multipart/form-data, parts named `payload_json` (the message body) + and `files[0]`, `files[1]`, ... each with a `filename=` in the + `Content-Disposition`. + """ + # Random boundary that won't appear in any payload we send. + import uuid + boundary = f"----SutandoBoundary{uuid.uuid4().hex}" + parts: list[bytes] = [] + # payload_json part + payload = {"content": content} if content else {} + parts.append( + f"--{boundary}\r\n" + f'Content-Disposition: form-data; name="payload_json"\r\n' + f"Content-Type: application/json\r\n\r\n".encode() + ) + parts.append(json.dumps(payload).encode()) + parts.append(b"\r\n") + # files[N] parts + for i, fpath in enumerate(file_paths): + filename = os.path.basename(fpath) + # Sanitize filename header — same shape as discord-bridge.py's + # `_safe_attachment_basename`. A filename containing CR/LF + # here would let the file inject its own headers into the + # multipart envelope. + safe_name = ( + filename + .replace("\r", "_") + .replace("\n", "_") + .replace('"', "_") + )[:80] or f"file-{i}" + parts.append( + f"--{boundary}\r\n" + f'Content-Disposition: form-data; name="files[{i}]"; ' + f'filename="{safe_name}"\r\n' + f"Content-Type: application/octet-stream\r\n\r\n".encode() + ) + with open(fpath, "rb") as fh: + parts.append(fh.read()) + parts.append(b"\r\n") + parts.append(f"--{boundary}--\r\n".encode()) + body = b"".join(parts) + url = f"https://discord.com/api/v10/channels/{channel_id}/messages" + req = urllib.request.Request( + url, + data=body, + headers={ + "Authorization": f"Bot {token}", + "Content-Type": f"multipart/form-data; boundary={boundary}", + "Content-Length": str(len(body)), + "User-Agent": "Sutando/1.0", + }, + method="POST", + ) + # 30s timeout — multipart uploads can be slower than JSON; cap to + # bound the dm-result.py invocation time. + with urllib.request.urlopen(req, timeout=30) as resp: + raw = resp.read() + return json.loads(raw) if raw else None + + def _resolve_owner_id(token): """Return the Discord user ID for the human owner. @@ -284,32 +371,42 @@ def send_dm(text: str) -> bool: print(f"dm-result: failed to open DM channel with {owner_id}: {e}", file=sys.stderr) return False - # Strip [file:|send:|attach:] markers from the body — the bridge's - # WS-connected client would attach the file directly, but dm-result - # is REST-only and has no multipart upload path yet, so the markers - # would otherwise deliver as literal text in the DM. We log the - # dropped files so the lossy delivery is visible in the dm-result - # output rather than silent. - clean_text, dropped_files = _split_file_markers(text) - if dropped_files: + # Extract [file:|send:|attach:] markers. The WS-connected live + # bridge calls `discord.File(path)` for each marker; this REST + # path now builds the equivalent multipart upload (see + # `_send_message_with_files`). Each marker path is allowlist- + # checked against `_is_path_sendable` — same policy as + # discord-bridge.py via the shared `send_allowlist` import — to + # bound exfil if an attacker-controlled marker ever reaches a + # result body. + clean_text, marker_files = _split_file_markers(text) + expanded_files = [os.path.expanduser(p.strip()) for p in marker_files] + sendable_files = [p for p in expanded_files if _is_path_sendable(p)] + rejected_files = [p for p in expanded_files if not _is_path_sendable(p)] + if rejected_files: + # Same security signal as discord-bridge: rejected paths log + # but don't leak the failure to the user. print( - f"dm-result: {len(dropped_files)} file marker(s) stripped from body " - f"(REST multipart upload not implemented): {dropped_files}", + f"dm-result: {len(rejected_files)} file marker(s) rejected by " + f"allowlist (would deliver via [file:] but path is outside " + f"_SEND_ALLOWED_ROOTS / _SEND_ALLOWED_PREFIXES): {rejected_files}", file=sys.stderr, ) # An all-marker / all-whitespace body becomes empty after strip. # Sending `""` to Discord returns 400 ("Cannot send an empty - # message"); skip the send and report the no-op instead. - if not clean_text: + # message") for a text-only request. For a multipart upload with + # files, an empty `content` is valid. + if not clean_text and not sendable_files: print( - f"dm-result: body is empty after marker-strip; nothing to send " - f"(channel {channel_id})" + f"dm-result: body is empty after marker-strip and no sendable " + f"files; nothing to send (channel {channel_id})" ) - return True # not an error — the input had no deliverable text + return True # not an error — the input had no deliverable payload - # Chunk into Discord-safe pieces, preserving code fences across boundaries. - chunks = list(_chunk_for_discord(clean_text)) + # Chunk text into Discord-safe pieces, preserving code fences + # across boundaries. + chunks = list(_chunk_for_discord(clean_text)) if clean_text else [] for i, chunk in enumerate(chunks): try: _discord_api("POST", f"/channels/{channel_id}/messages", token, {"content": chunk}) @@ -320,8 +417,27 @@ def send_dm(text: str) -> bool: ) return False + # Attach files in batches of 10 (Discord per-message attachment + # cap). Each batch goes as a separate multipart message with + # empty content — the text was already delivered as chunks above. + DISCORD_FILES_PER_MESSAGE = 10 + for batch_start in range(0, len(sendable_files), DISCORD_FILES_PER_MESSAGE): + batch = sendable_files[batch_start : batch_start + DISCORD_FILES_PER_MESSAGE] + try: + _send_message_with_files(channel_id, token, "", batch) + except Exception as e: + print( + f"dm-result: failed to upload file batch " + f"{batch_start // DISCORD_FILES_PER_MESSAGE + 1} " + f"({len(batch)} file(s)) to channel {channel_id}: {e}", + file=sys.stderr, + ) + return False + + file_summary = f", {len(sendable_files)} file(s)" if sendable_files else "" print( - f"dm-result: sent to DM ({len(clean_text)} chars in {len(chunks)} chunk(s)) via channel {channel_id}" + f"dm-result: sent to DM ({len(clean_text)} chars in {len(chunks)} chunk(s)" + f"{file_summary}) via channel {channel_id}" ) return True diff --git a/src/send_allowlist.py b/src/send_allowlist.py new file mode 100644 index 0000000000..9597836faf --- /dev/null +++ b/src/send_allowlist.py @@ -0,0 +1,109 @@ +"""Shared file-attachment allowlist for `[file:|send:|attach:]` markers. + +Single source of truth for the policy that decides whether an agent- +emitted file marker can be delivered to the owner's Discord DM / +channel. Used by: + + - ``src/discord-bridge.py`` — live WS-connected bridge + (``discord.File(path)``). + - ``src/dm-result.py`` — REST-only fallback when the bridge isn't + available (``multipart/form-data`` upload, see PR #1029). + +Per @liususan091219 review on PR #1029: keeping the policy as a copy +in each file *will* drift even with the "keep in sync" comment, so +extracting here gives both consumers a shared import. A tightening +of the allowlist now lands in both paths automatically; a broadening +is a single edit. + +Ported from OSS sutando in Phase 5.9 of the OSS→private sync. The +policy values are byte-identical to the pre-port inline copy in +``src/discord-bridge.py`` — this extract is a pure refactor that +removes the drift hazard between the bridge and ``dm-result.py``. + +The policy: + + * Files must be regular files (`os.path.isfile`) so symlinks + pointing at non-existent destinations don't qualify. + * `realpath` collapses the path before the prefix check — defeats + symlink-to-`/etc/passwd` and `..` escapes. + * Either the realpath equals an allowed-root or starts with + ``/`` (the trailing-slash check prevents + ``/private/tmp/sutando-xyz`` from being treated as a child of + ``/private/tmp/sutando``). + * Prefix matches use `startswith` against the realpath so the + same anti-symlink/anti-traversal property holds. + +Per-machine paths (`~/Desktop/iclr-backups`, +`~/Documents/sutando-launch-assets`) resolve at module-load time +based on the current `Path.home()`; if the user's home dir changes +between processes, the resolved root differs accordingly — that's +the same shape the discord-bridge copy already had and matches +expectations for owner-local file roots. +""" + +from __future__ import annotations + +import os +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) +from workspace_default import resolve_workspace # noqa: E402 +from util_paths import shared_personal_path # noqa: E402 + +_REPO = resolve_workspace() + +# Owner-relative + machine-local roots. Files under these roots are +# delivered to Discord as attachments without further checks. +SEND_ALLOWED_ROOTS: tuple[str, ...] = ( + str(_REPO / "results"), + str(_REPO / "notes"), + # Notes canonical home (private dir) — once saved by save_note, + # paths reference the private location. Both old and new paths + # allowed during the transition; resolver picks whichever exists. + str(shared_personal_path("notes", _REPO)), + str(_REPO / "docs"), + str(Path.home() / "Desktop" / "iclr-backups"), + str(Path.home() / "Documents" / "sutando-launch-assets"), +) + +# Prefix forms — files whose realpath starts with any of these strings +# are deliverable. Covers temp-file artifacts the agent generates +# (`/tmp/sutando-recording-*.mov`, `/tmp/echo-screenshot-*.png`, etc.) +# without needing to enumerate every filename. +SEND_ALLOWED_PREFIXES: tuple[str, ...] = ( + "/tmp/sutando-", + "/private/tmp/sutando-", + "/tmp/echo-", + "/private/tmp/echo-", +) + + +def is_path_sendable(fpath: str) -> bool: + """True iff `fpath` is a regular file AND its `realpath` resolves + under one of ``SEND_ALLOWED_ROOTS`` or starts with one of + ``SEND_ALLOWED_PREFIXES``. + + Single source of truth for the file-attachment-delivery policy. + Mirrors the shape used by ``_is_path_sendable`` in both call sites + pre-extract. + + A non-existent path, a symlink to outside the allowlist (caught by + realpath collapse), or a path that simply doesn't match any + allowed root/prefix all return False. Callers should fail-closed + on False (log + skip; never deliver the file). + """ + if not os.path.isfile(fpath): + return False + try: + real = os.path.realpath(fpath) + except OSError: + return False + for root in SEND_ALLOWED_ROOTS: + root_real = os.path.realpath(root) + if real == root_real or real.startswith(root_real + os.sep): + return True + for prefix in SEND_ALLOWED_PREFIXES: + if real.startswith(prefix): + return True + return False diff --git a/tests/discord-bridge-allowlist.test.py b/tests/discord-bridge-allowlist.test.py index 567ee5f9e1..bb481e2712 100755 --- a/tests/discord-bridge-allowlist.test.py +++ b/tests/discord-bridge-allowlist.test.py @@ -5,25 +5,33 @@ pattern or the fail-closed allowlist behavior. The CodeQL py/path-injection rule relies on the realpath+startswith -sanitizer being inline at the sink. If a refactor moves that logic into -a helper whose return is used inconsistently, or drops the allowlist in -favor of bare `discord.File(fpath)`, we want to catch it at test time — -not after a talk demo leaks an attacker-supplied path. +sanitizer pattern existing at the sink path. If a refactor moves that +logic into a helper whose return is used inconsistently, or drops the +allowlist in favor of bare `discord.File(fpath)`, we want to catch it +at test time — not after a talk demo leaks an attacker-supplied path. -Scope: STRUCTURAL — regex-matches the source file. Does NOT import the +Scope: STRUCTURAL — regex-matches the source files. Does NOT import the bridge (discord.py dep weight is huge). Mirrors the style of `discord-bridge-access-tier.test.py`. +Architecture note (Phase 5.9 OSS sync): the sanitizer body now lives in +`src/send_allowlist.py` (shared with `src/dm-result.py`). The discord +bridge imports the function and aliases it as `_is_path_sendable` so +every existing sink call site stays unchanged. This test follows the +implementation: checks #1-#4 inspect the helper module, check #5 +verifies the import + alias wiring in the bridge, and check #6 +verifies every `discord.File()` sink is gated by the alias. + Guards: - 1. `_is_path_sendable` function is defined. - 2. Inside `_is_path_sendable`, `os.path.realpath` is used (NOT - replaceable with `Path.resolve()` without re-proving to CodeQL). - 3. `_is_path_sendable` checks both SEND_ALLOWED_ROOTS and - SEND_ALLOWED_PREFIXES before returning True. - 4. Fail-closed default: the helper returns False when no allowlist - entry matches. - 5. `discord.File(fpath)` is always gated by a call to - `_is_path_sendable` — no bare sends. + 1. `is_path_sendable` is defined in `src/send_allowlist.py`. + 2. The helper uses `os.path.realpath` (CodeQL sanitizer pattern — + NOT replaceable with `Path.resolve()` without re-proving). + 3. The helper checks both SEND_ALLOWED_ROOTS and SEND_ALLOWED_PREFIXES. + 4. Fail-closed default: helper returns False when no entry matches. + 5. `discord-bridge.py` imports the helper and binds it as the local + `_is_path_sendable` name. (Drift guard — a future PR that + re-inlines a copy would break this assertion.) + 6. `discord.File(fpath)` sinks are always gated by `_is_path_sendable`. Run: python3 tests/discord-bridge-allowlist.test.py Exit: 0 on pass, 1 on fail. @@ -35,6 +43,7 @@ REPO = Path(__file__).resolve().parent.parent BRIDGE = REPO / "src" / "discord-bridge.py" +HELPER = REPO / "src" / "send_allowlist.py" def fail(msg: str, context: str = "") -> int: @@ -48,62 +57,98 @@ def fail(msg: str, context: str = "") -> int: def main() -> int: if not BRIDGE.exists(): return fail(f"{BRIDGE} not found") + if not HELPER.exists(): + return fail(f"{HELPER} not found — shared allowlist module is missing") - src = BRIDGE.read_text() + bridge_src = BRIDGE.read_text() + helper_src = HELPER.read_text() - # 1. Helper defined + # 1. Helper defined in send_allowlist.py helper_match = re.search( - r"def _is_path_sendable\(fpath:\s*str\)\s*->\s*bool:\s*\n([\s\S]{0,2000}?)(?=\n\ndef |\n\n[A-Z]|\Z)", - src, + r"def is_path_sendable\(fpath:\s*str\)\s*->\s*bool:\s*\n([\s\S]{0,2000}?)(?=\n\ndef |\n\n[A-Z]|\Z)", + helper_src, ) if not helper_match: - return fail("`_is_path_sendable` function not found") + return fail( + "`is_path_sendable` function not found in send_allowlist.py — " + "shared sanitizer body is missing" + ) helper_body = helper_match.group(1) # 2. realpath used (CodeQL sanitizer pattern) if "os.path.realpath" not in helper_body: return fail( - "_is_path_sendable must use os.path.realpath (CodeQL py/path-injection sanitizer)", + "is_path_sendable must use os.path.realpath " + "(CodeQL py/path-injection sanitizer)", helper_body, ) - # 3. Both ROOTS and PREFIXES consulted + # 3. Both ROOTS and PREFIXES consulted in the helper if "SEND_ALLOWED_ROOTS" not in helper_body or "SEND_ALLOWED_PREFIXES" not in helper_body: return fail( - "_is_path_sendable must check both SEND_ALLOWED_ROOTS and SEND_ALLOWED_PREFIXES", + "is_path_sendable must check both SEND_ALLOWED_ROOTS and " + "SEND_ALLOWED_PREFIXES", helper_body, ) # 4. Fail-closed default: final `return False` after the loops - if not re.search(r"for prefix in SEND_ALLOWED_PREFIXES:[\s\S]+?return\s+False", helper_body): + if not re.search( + r"for prefix in SEND_ALLOWED_PREFIXES:[\s\S]+?return\s+False", helper_body + ): return fail( - "_is_path_sendable must return False after iterating both allowlists (fail-closed)", + "is_path_sendable must return False after iterating both " + "allowlists (fail-closed)", helper_body, ) - # 5. Every discord.File(fpath) send call must be gated by _is_path_sendable. + # 5. discord-bridge.py must import the helper AND alias it locally + # as `_is_path_sendable` so existing sink call sites stay gated. + # Match the multi-line `from send_allowlist import (...)` block AND + # the explicit alias assignment. + if not re.search( + r"from\s+send_allowlist\s+import\s+\(?[\s\S]{0,400}?is_path_sendable\b", + bridge_src, + ): + return fail( + "discord-bridge.py must import `is_path_sendable` from " + "`send_allowlist` — drift hazard if a copy gets inlined" + ) + if not re.search( + r"_is_path_sendable\s*=\s*_is_path_sendable_shared\b", bridge_src + ): + return fail( + "discord-bridge.py must alias `_is_path_sendable = " + "_is_path_sendable_shared` so existing sink call sites " + "resolve to the shared helper" + ) + + # 6. Every discord.File(fpath) send call must be gated by _is_path_sendable. # Find all `discord.File(...)` sink calls; for each, check that the # enclosing 6-line window above contains an `_is_path_sendable` guard. - for match in re.finditer(r"discord\.File\(\s*(\w+)\s*\)", src): + for match in re.finditer(r"discord\.File\(\s*(\w+)\s*\)", bridge_src): arg = match.group(1) - # Find the 6-line window ending at this match - start = src.rfind("\n", 0, match.start()) + start = bridge_src.rfind("\n", 0, match.start()) for _ in range(6): - prev = src.rfind("\n", 0, start) + prev = bridge_src.rfind("\n", 0, start) if prev < 0: break start = prev - window = src[start:match.end()] - if f"_is_path_sendable({arg})" not in window and f"_is_path_sendable( {arg}" not in window: + window = bridge_src[start:match.end()] + if ( + f"_is_path_sendable({arg})" not in window + and f"_is_path_sendable( {arg}" not in window + ): return fail( - f"discord.File({arg}) sink found without preceding _is_path_sendable gate", + f"discord.File({arg}) sink found without preceding " + f"_is_path_sendable gate", window, ) print("PASS: discord-bridge.py file-send allowlist looks correct.") - print(" - _is_path_sendable uses os.path.realpath (CodeQL sanitizer)") + print(" - is_path_sendable (in send_allowlist.py) uses os.path.realpath") print(" - checks both SEND_ALLOWED_ROOTS and SEND_ALLOWED_PREFIXES") print(" - fail-closed default (returns False if no allowlist match)") + print(" - discord-bridge.py imports + aliases the helper (no inline copy)") print(" - all discord.File() sinks gated by _is_path_sendable") return 0 diff --git a/tests/dm-result-upload.test.py b/tests/dm-result-upload.test.py new file mode 100644 index 0000000000..f781cba2bc --- /dev/null +++ b/tests/dm-result-upload.test.py @@ -0,0 +1,404 @@ +#!/usr/bin/env python3 +"""Tests for `send_dm` REST-multipart upload path in src/dm-result.py. + +Phase 5.9 of the OSS→private sync added multipart upload to the +REST-fallback delivery path. Pre-port, dm-result stripped +`[file:|send:|attach:]` markers and logged them as dropped because the +WS-connected bridge was the only path that could actually attach files. +Post-port, dm-result builds the equivalent multipart payload itself so +file delivery survives the bridge being offline. + +Five regression guards: + + 1. Marker with sendable file → text + file upload, allowlist passes. + 2. Marker with disallowed file → text-only delivered, file rejected + (logged), no multipart POST issued. + 3. File-only body (text empty, file sendable) → multipart POST with + empty `content`, no JSON-message POST. + 4. >10 files → batched into chunks of 10 (Discord per-message cap). + 5. Multipart body shape: boundary present, payload_json part + present, files[N] parts present, filename sanitized. + +Test uses a recording fake urllib transport — same shape as +`tests/dm-result-send-dm.test.py` — but extended to handle +multipart/form-data requests (those use `request.data` as bytes, +not JSON, so the existing fake's `json.loads(request.data)` would +choke). +""" + +import importlib.util +import json +import os +import sys +import tempfile +from pathlib import Path + +REPO = Path(__file__).resolve().parent.parent + +os.environ.setdefault("DISCORD_BOT_TOKEN", "test-token-not-real") + +_channels_env = Path.home() / ".claude" / "channels" / "discord" / ".env" +if not _channels_env.exists(): + _channels_env.parent.mkdir(parents=True, exist_ok=True) + _channels_env.write_text("DISCORD_BOT_TOKEN=test-token-not-real\n") + + +def _load(name: str, path: Path): + spec = importlib.util.spec_from_file_location(name, path) + m = importlib.util.module_from_spec(spec) + spec.loader.exec_module(m) + return m + + +dm = _load("dm_result", REPO / "src" / "dm-result.py") + + +class _FakeResponse: + def __init__(self, body_bytes: bytes): + self._body = body_bytes + + def read(self): + return self._body + + def __enter__(self): + return self + + def __exit__(self, *a): + return False + + +class _FakeTransport: + """Records every request and replies with canned responses. Unlike + the JSON-only fake in `dm-result-send-dm.test.py`, this one keeps + the raw bytes when content-type is multipart so the test can + introspect the assembled body.""" + + def __init__(self, responses): + self.calls: list[dict] = [] + self._responses = dict(responses) + + def urlopen(self, request, timeout=None): + method = getattr(request, "method", None) or ( + "POST" if request.data is not None else "GET" + ) + url = request.full_url + ct = request.headers.get("Content-type", "") or request.headers.get( + "Content-Type", "" + ) + raw = request.data + body = None + if raw is not None and ct.startswith("application/json"): + body = json.loads(raw.decode()) + self.calls.append({ + "method": method, + "url": url, + "body": body, + "raw": raw, + "content_type": ct, + }) + for (m, suffix), reply in self._responses.items(): + if m == method and url.endswith(suffix): + return _FakeResponse(json.dumps(reply).encode()) + raise AssertionError(f"unmocked request: {method} {url}") + + +def _install_transport(transport): + dm.urllib.request.urlopen = transport.urlopen + + +def _restore_transport(original): + dm.urllib.request.urlopen = original + + +def _with_access_json(content, fn): + original = dm.ACCESS_JSON + tmp = Path(tempfile.mkdtemp(prefix="sutando-dm-upload-test-")) / "access.json" + tmp.write_text(json.dumps(content)) + dm.ACCESS_JSON = tmp + try: + fn() + finally: + dm.ACCESS_JSON = original + tmp.unlink() + tmp.parent.rmdir() + + +def _make_sutando_temp_file(name: str = "asset.png", content: bytes = b"PNGDATA") -> str: + """Materialize a real file under the `/tmp/sutando-` prefix so + `_is_path_sendable` accepts it. Returns the absolute path. + + Forces `dir="/tmp"` because macOS's default TMPDIR is + `/var/folders/.../T/`, which doesn't match the allowlist + prefix — we want this fixture to live under the real allowed + root regardless of host.""" + fd, path = tempfile.mkstemp( + prefix="sutando-upload-test-", suffix=f"-{name}", dir="/tmp" + ) + os.close(fd) + Path(path).write_bytes(content) + return path + + +# ----------------------------------------------------------------------- +# 1. Marker with sendable file → text + multipart upload +# ----------------------------------------------------------------------- + + +def test_sendable_file_marker_triggers_multipart_upload(): + """Marker path is real, under allowed prefix → file IS uploaded. + Pre-port behavior was to silently drop with a stderr log; the + upload path makes the file deliverable again.""" + fpath = _make_sutando_temp_file() + try: + transport = _FakeTransport({ + ("POST", "/users/@me/channels"): {"id": "dm-up-1"}, + ("POST", "/channels/dm-up-1/messages"): {"id": "msg"}, + }) + original = dm.urllib.request.urlopen + + def run(): + _install_transport(transport) + try: + ok = dm.send_dm(f"Here's the asset: [file: {fpath}]") + finally: + _restore_transport(original) + assert ok is True + json_msgs = [c for c in transport.calls if "/messages" in c["url"] + and c["content_type"].startswith("application/json")] + multipart_msgs = [c for c in transport.calls if "/messages" in c["url"] + and c["content_type"].startswith("multipart/form-data")] + assert len(json_msgs) == 1, ( + f"expected exactly one JSON text post; got {len(json_msgs)}" + ) + assert "Here's the asset:" in json_msgs[0]["body"]["content"] + assert "[file:" not in json_msgs[0]["body"]["content"] + assert len(multipart_msgs) == 1, ( + f"expected exactly one multipart upload; got {len(multipart_msgs)}" + ) + assert os.path.basename(fpath).encode() in multipart_msgs[0]["raw"], ( + "filename missing from multipart body" + ) + + _with_access_json( + {"allowFrom": ["human"], "tierMap": {"human": "owner"}}, + run, + ) + finally: + os.unlink(fpath) + + +# ----------------------------------------------------------------------- +# 2. Disallowed file marker → text-only, no upload +# ----------------------------------------------------------------------- + + +def test_disallowed_file_marker_rejected_without_upload(): + """Marker path doesn't resolve under any allowed root → file is + rejected by `_is_path_sendable`. Text still posts; NO multipart + upload happens. Same security gate the WS-bridge enforces.""" + # /etc/hosts: real file, NOT on the allowlist. + transport = _FakeTransport({ + ("POST", "/users/@me/channels"): {"id": "dm-up-2"}, + ("POST", "/channels/dm-up-2/messages"): {"id": "msg"}, + }) + original = dm.urllib.request.urlopen + + def run(): + _install_transport(transport) + try: + ok = dm.send_dm("Look at this: [file: /etc/hosts]") + finally: + _restore_transport(original) + assert ok is True + multipart_msgs = [c for c in transport.calls + if c["content_type"].startswith("multipart/form-data")] + assert multipart_msgs == [], ( + f"disallowed file leaked into multipart upload: {multipart_msgs}" + ) + json_msgs = [c for c in transport.calls if "/messages" in c["url"] + and c["content_type"].startswith("application/json")] + assert len(json_msgs) == 1 + assert "[file:" not in json_msgs[0]["body"]["content"] + assert "Look at this:" in json_msgs[0]["body"]["content"] + + _with_access_json( + {"allowFrom": ["human"], "tierMap": {"human": "owner"}}, + run, + ) + + +# ----------------------------------------------------------------------- +# 3. File-only body → multipart upload only, no JSON message +# ----------------------------------------------------------------------- + + +def test_file_only_body_uploads_without_json_message(): + """Body is JUST a marker → no text to chunk; multipart upload + happens with empty `content`. Pre-port this case fell through the + empty-body guard and returned `True` without delivering anything.""" + fpath = _make_sutando_temp_file(content=b"ONLY_FILE") + try: + transport = _FakeTransport({ + ("POST", "/users/@me/channels"): {"id": "dm-up-3"}, + ("POST", "/channels/dm-up-3/messages"): {"id": "msg"}, + }) + original = dm.urllib.request.urlopen + + def run(): + _install_transport(transport) + try: + ok = dm.send_dm(f"[file: {fpath}]") + finally: + _restore_transport(original) + assert ok is True + json_msgs = [c for c in transport.calls if "/messages" in c["url"] + and c["content_type"].startswith("application/json")] + multipart_msgs = [c for c in transport.calls if "/messages" in c["url"] + and c["content_type"].startswith("multipart/form-data")] + assert json_msgs == [], ( + f"file-only body should NOT post a JSON text message; " + f"got {json_msgs}" + ) + assert len(multipart_msgs) == 1 + # And the file content actually rode along + assert b"ONLY_FILE" in multipart_msgs[0]["raw"] + + _with_access_json( + {"allowFrom": ["human"], "tierMap": {"human": "owner"}}, + run, + ) + finally: + os.unlink(fpath) + + +# ----------------------------------------------------------------------- +# 4. >10 files batched at the Discord per-message cap +# ----------------------------------------------------------------------- + + +def test_more_than_ten_files_batched_at_discord_cap(): + """Discord caps attachments at 10 per message. send_dm must split + into ceil(N/10) multipart POSTs. Pin the batching so a future + `DISCORD_FILES_PER_MESSAGE` change doesn't silently bin-pack 50 + files into a single 413-payload request.""" + files = [_make_sutando_temp_file(name=f"batch-{i}.bin") for i in range(11)] + try: + transport = _FakeTransport({ + ("POST", "/users/@me/channels"): {"id": "dm-up-4"}, + ("POST", "/channels/dm-up-4/messages"): {"id": "msg"}, + }) + original = dm.urllib.request.urlopen + markers = " ".join(f"[file: {p}]" for p in files) + + def run(): + _install_transport(transport) + try: + ok = dm.send_dm(f"Eleven files: {markers}") + finally: + _restore_transport(original) + assert ok is True + multipart_msgs = [c for c in transport.calls + if c["content_type"].startswith("multipart/form-data")] + assert len(multipart_msgs) == 2, ( + f"expected 2 batches (10 + 1); got {len(multipart_msgs)} " + f"multipart messages" + ) + # First batch should carry 10 file parts, second carries 1. + batch_1_parts = multipart_msgs[0]["raw"].count(b'name="files[') + batch_2_parts = multipart_msgs[1]["raw"].count(b'name="files[') + assert batch_1_parts == 10, ( + f"batch 1 should have 10 files; got {batch_1_parts}" + ) + assert batch_2_parts == 1, ( + f"batch 2 should have 1 file; got {batch_2_parts}" + ) + + _with_access_json( + {"allowFrom": ["human"], "tierMap": {"human": "owner"}}, + run, + ) + finally: + for p in files: + os.unlink(p) + + +# ----------------------------------------------------------------------- +# 5. Multipart body shape and filename sanitization +# ----------------------------------------------------------------------- + + +def test_multipart_body_shape_and_filename_sanitization(): + """The multipart envelope must include: + - a boundary in Content-Type + - a `payload_json` part + - one `files[N]` part per attachment, with sanitized filename + + Sanitization: filenames containing CR/LF/quote chars must not + let the file inject its own headers. Pin the same sanitization + the WS-bridge has (`_safe_attachment_basename`).""" + # Create a file whose name contains a quote — when Discord's + # filename header is `filename="..."` an unescaped quote would + # terminate the value and let the file smuggle arbitrary header + # fields into the multipart envelope. + fd, raw_path = tempfile.mkstemp( + prefix='sutando-upload-shape-test-evil"-name-', + suffix=".txt", + dir="/tmp", + ) + os.close(fd) + Path(raw_path).write_text("shape-test-body") + try: + transport = _FakeTransport({ + ("POST", "/users/@me/channels"): {"id": "dm-up-5"}, + ("POST", "/channels/dm-up-5/messages"): {"id": "msg"}, + }) + original = dm.urllib.request.urlopen + + def run(): + _install_transport(transport) + try: + ok = dm.send_dm(f"[file: {raw_path}]") + finally: + _restore_transport(original) + assert ok is True + multipart_msgs = [c for c in transport.calls + if c["content_type"].startswith("multipart/form-data")] + assert len(multipart_msgs) == 1 + ct = multipart_msgs[0]["content_type"] + assert "boundary=" in ct + raw = multipart_msgs[0]["raw"] + assert b'name="payload_json"' in raw + assert b'name="files[0]"' in raw + assert b"shape-test-body" in raw + # Sanitization: the raw quote char in the filename must + # have been replaced before landing in the Content-Disposition. + # Find the filename="...evil_-..." line — should NOT have + # `evil"-` (the unsanitized quote) on the wire. + disposition_lines = [ + line for line in raw.split(b"\r\n") + if line.startswith(b"Content-Disposition: form-data; name=\"files[") + ] + assert len(disposition_lines) == 1 + assert b'evil"-name' not in disposition_lines[0], ( + "unsanitized quote leaked into Content-Disposition header" + ) + + _with_access_json( + {"allowFrom": ["human"], "tierMap": {"human": "owner"}}, + run, + ) + finally: + os.unlink(raw_path) + + +def main(): + test_sendable_file_marker_triggers_multipart_upload() + test_disallowed_file_marker_rejected_without_upload() + test_file_only_body_uploads_without_json_message() + test_more_than_ten_files_batched_at_discord_cap() + test_multipart_body_shape_and_filename_sanitization() + print("All dm-result upload tests passed.") + + +if __name__ == "__main__": + main() diff --git a/tests/send-allowlist.test.py b/tests/send-allowlist.test.py new file mode 100644 index 0000000000..b0275a937a --- /dev/null +++ b/tests/send-allowlist.test.py @@ -0,0 +1,236 @@ +#!/usr/bin/env python3 +"""Tests for `src/send_allowlist.py` — the shared file-attachment policy. + +`send_allowlist.is_path_sendable` is the single source of truth for the +gate between an agent-emitted `[file: /path]` marker and any outbound +Discord delivery (both the WS-connected `discord-bridge.py` and the +REST-fallback `dm-result.py`). If a future refactor weakens this +function — or if either consumer drifts away from importing it — +exfil paths reopen. + +Tests cover three concerns: + + 1. Policy semantics (same shape as the pre-extract test in + `tests/discord-bridge-file-send-allowlist.test.py`, but applied + directly to the helper module): regular-file gate, allowlist + match, symlink/path-traversal rejection, fail-closed default. + + 2. Drift guards: both consumers (`discord-bridge.py` and + `dm-result.py`) must reference the SAME object as the helper, + not their own copies. Identity, not just equality, because a + copy would equal at module-load time but drift on later edits. + + 3. Documented-set guard: the prefix set stays the deliberate four + entries. A future PR widening it to e.g. `/tmp/` forces an + explicit test update. +""" + +import importlib.util +import os +import sys +import tempfile +import types +from pathlib import Path + +REPO = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(REPO / "src")) + +# Stub `discord` so loading discord-bridge.py doesn't require the lib. +if "discord" not in sys.modules: + stub = types.ModuleType("discord") + stub.Intents = type( + "Intents", + (), + {"default": staticmethod(lambda: type("I", (), {"message_content": False})())}, + ) + stub.Client = type( + "Client", + (), + {"__init__": lambda self, **kw: None, "event": staticmethod(lambda fn: fn)}, + ) + stub.File = type("File", (), {}) + stub.Message = type("Message", (), {}) + sys.modules["discord"] = stub + +# Materialize a placeholder .env so `_load_token()` succeeds during +# discord-bridge module load. +_channels_env = Path.home() / ".claude" / "channels" / "discord" / ".env" +if not _channels_env.exists(): + _channels_env.parent.mkdir(parents=True, exist_ok=True) + _channels_env.write_text("DISCORD_BOT_TOKEN=test-token-not-real\n") + + +def _load(name: str, path: Path): + spec = importlib.util.spec_from_file_location(name, path) + m = importlib.util.module_from_spec(spec) + spec.loader.exec_module(m) + return m + + +import send_allowlist # noqa: E402 + +# Both consumers — load with module-loader so hyphenated filenames work. +bridge = _load("dbridge", REPO / "src" / "discord-bridge.py") +dm = _load("dm_result", REPO / "src" / "dm-result.py") + + +# ----------------------------------------------------------------------- +# 1. Policy semantics +# ----------------------------------------------------------------------- + + +def test_nonexistent_path_is_not_sendable(): + """Fail-closed on missing files.""" + assert not send_allowlist.is_path_sendable( + "/tmp/sutando-helper-does-not-exist-99999.png" + ) + + +def test_directory_is_not_sendable(): + """`os.path.isfile` is the type gate — directories must reject.""" + tmp = Path(tempfile.mkdtemp(prefix="/tmp/sutando-test-helper-dir-")) + try: + assert not send_allowlist.is_path_sendable(str(tmp)) + finally: + tmp.rmdir() + + +def test_allowed_prefix_match(): + """Files under `/tmp/sutando-*` are explicitly allowed.""" + with tempfile.NamedTemporaryFile( + prefix="sutando-helper-test-", suffix=".png", dir="/tmp", delete=False + ) as f: + f.write(b"x") + path = f.name + try: + assert send_allowlist.is_path_sendable(path), \ + f"expected allowed prefix to match {path}" + finally: + os.unlink(path) + + +def test_arbitrary_path_rejected(): + """A regular file outside the allowlist must reject.""" + assert not send_allowlist.is_path_sendable("/etc/hosts") + + +def test_symlink_outside_allowed_root_rejected(): + """Symlink-escape regression guard. A symlink at an allowed prefix + that points at an out-of-list file must reject — `realpath` + collapses before the prefix comparison.""" + link = Path("/tmp/sutando-helper-symlink-pointer.txt") + outside = Path("/tmp/escaped-not-sutando-helper.txt") + outside.write_text("would be exfil") + if link.exists() or link.is_symlink(): + link.unlink() + try: + os.symlink(outside, link) + assert not send_allowlist.is_path_sendable(str(link)), ( + f"symlink {link} → {outside} bypassed the allowlist — " + "realpath collapse is broken" + ) + finally: + if link.exists() or link.is_symlink(): + link.unlink() + outside.unlink() + + +def test_path_traversal_dotdot_rejected(): + """`..` segments that escape the allowlist must reject.""" + rogue = Path("/tmp/passwd-not-sutando-helper-no-write") + if not rogue.exists(): + rogue.write_text("not actual passwd") + try: + traversal = "/tmp/sutando-x/../passwd-not-sutando-helper-no-write" + assert not send_allowlist.is_path_sendable(traversal), ( + "path traversal via .. bypassed the allowlist — " + "realpath collapse is broken" + ) + finally: + if rogue.exists(): + rogue.unlink() + + +# ----------------------------------------------------------------------- +# 2. Drift guards: consumers must reference the helper, not copies +# ----------------------------------------------------------------------- + + +def test_discord_bridge_imports_helper_constants_by_identity(): + """`discord-bridge.py` must reference the SAME tuple object as the + helper — not a copy. A future PR that re-inlines the constants + would equal-compare at first but drift on the next edit. Identity + check is the only honest drift guard.""" + assert bridge.SEND_ALLOWED_ROOTS is send_allowlist.SEND_ALLOWED_ROOTS, ( + "discord-bridge.SEND_ALLOWED_ROOTS is no longer the helper's " + "object — drift hazard reintroduced." + ) + assert bridge.SEND_ALLOWED_PREFIXES is send_allowlist.SEND_ALLOWED_PREFIXES, ( + "discord-bridge.SEND_ALLOWED_PREFIXES is no longer the helper's " + "object — drift hazard reintroduced." + ) + # The function alias must point at the helper's function. (We + # check by calling — identity comparison through `is` on + # functions works in CPython but is implementation-defined for + # wrapped/decorated functions, so we use a function-call probe.) + assert bridge._is_path_sendable is send_allowlist.is_path_sendable, ( + "discord-bridge._is_path_sendable is no longer the helper — " + "drift hazard reintroduced." + ) + + +def test_dm_result_imports_helper_constants_by_identity(): + """Same drift guard for dm-result.py.""" + assert dm._SEND_ALLOWED_ROOTS is send_allowlist.SEND_ALLOWED_ROOTS, ( + "dm-result._SEND_ALLOWED_ROOTS is no longer the helper's " + "object — drift hazard reintroduced." + ) + assert dm._SEND_ALLOWED_PREFIXES is send_allowlist.SEND_ALLOWED_PREFIXES, ( + "dm-result._SEND_ALLOWED_PREFIXES is no longer the helper's " + "object — drift hazard reintroduced." + ) + assert dm._is_path_sendable is send_allowlist.is_path_sendable, ( + "dm-result._is_path_sendable is no longer the helper — " + "drift hazard reintroduced." + ) + + +# ----------------------------------------------------------------------- +# 3. Documented-set guard +# ----------------------------------------------------------------------- + + +def test_allowed_prefixes_are_the_documented_set(): + """Architectural assertion: the allowed prefixes must stay the + deliberate four-entry set. A future refactor that adds `/tmp/` + (no suffix) or `/Users/` would massively widen exfil; this test + forces a deliberate update of the documented set.""" + documented = { + "/tmp/sutando-", + "/private/tmp/sutando-", + "/tmp/echo-", + "/private/tmp/echo-", + } + actual = set(send_allowlist.SEND_ALLOWED_PREFIXES) + assert actual == documented, ( + f"SEND_ALLOWED_PREFIXES has changed unexpectedly. " + f"Removed: {documented - actual}, Added: {actual - documented}. " + "Update this test deliberately to confirm the new exposure is intended." + ) + + +def main(): + test_nonexistent_path_is_not_sendable() + test_directory_is_not_sendable() + test_allowed_prefix_match() + test_arbitrary_path_rejected() + test_symlink_outside_allowed_root_rejected() + test_path_traversal_dotdot_rejected() + test_discord_bridge_imports_helper_constants_by_identity() + test_dm_result_imports_helper_constants_by_identity() + test_allowed_prefixes_are_the_documented_set() + print("All send_allowlist tests passed.") + + +if __name__ == "__main__": + main()