Skip to content

Slack: ground DOCX attachments and govern uploads - #3

Merged
chungty merged 3 commits into
mainfrom
codex/mer-266-hermes-files
Jul 30, 2026
Merged

Slack: ground DOCX attachments and govern uploads#3
chungty merged 3 commits into
mainfrom
codex/mer-266-hermes-files

Conversation

@chungty

@chungty chungty commented Jul 29, 2026

Copy link
Copy Markdown

Implements MER-251 and MER-267, plus the Hermes substrate for MER-141. Adds bounded current-message and prior-thread document ingestion, local DOCX extraction, actual transfer accounting, retry handling, and default-off governed local uploads that send the exact validated byte snapshot.\n\nVerification: 293 focused Slack/document tests pass; ruff, compileall, and diff checks pass. The broader Hermes suite has a pre-existing local-keychain isolation failure in 14 Anthropic credential tests, unrelated to this diff.\n\nDeployment/acceptance gates remain in MER-266.


Note

Medium Risk
Changes security-sensitive Slack upload paths and injects untrusted attachment text into agent prompts, but limits are explicit, uploads are default-off, and behavior is heavily tested.

Overview
Tightens Slack file handling on both directions: inbound documents are ingested with limits and optional DOCX text grounding; outbound local uploads only work through a default-off governed artifact workflow.

Inbound: Adds gateway/document_extract.py for bounded, local-only DOCX → Markdown extraction (headings, tables, size caps, typed errors). The Slack adapter centralizes attachment handling via _resolve_slack_file_object, _ingest_slack_document, and streaming downloads with byte budgets. Current-message attachments inject untrusted extracted DOCX text into the prompt; prior-thread fetches now include document provenance and extracted content under count/byte/char budgets.

Outbound: Local MEDIA: / file uploads go through _prepare_local_upload (opt-in via HERMES_SLACK_LOCAL_UPLOADS_ENABLED, paths under HERMES_SLACK_ARTIFACT_ROOT or ~/.hermes/artifacts/slack, symlink-safe containment, magic-byte checks, credential/archive scans). Uploads send validated byte snapshots via content= instead of arbitrary filesystem paths. Slack platform hints and send_message_tool docs steer the model toward approved artifact paths only.

Tests: New/expanded coverage for DOCX extraction, upload policy denials, DOCX injection, thread attachment budgets, and download retry/stream limits.

Reviewed by Cursor Bugbot for commit f117d3674dad3b4cd64bce85e300870fc15ad16f. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • New Features
    • Added DOCX attachment extraction with formatted output (headings, paragraphs, and tables) and truncation metadata when limits are reached.
    • Enhanced Slack attachment handling: downloads, caching, and inclusion in message and thread context with provenance.
    • Introduced bounded handling for attachment count, downloaded size, and rendered content length.
  • Bug Fixes
    • Improved resilience for transient Slack download and file-info failures with consistent retry behavior.
    • Tightened Slack uploads to only allow governed artifacts; strengthened rejection of unsafe or malformed files.
  • Documentation
    • Updated Slack-specific messaging instructions to require MEDIA:<approved_path> from a trusted artifact tool.

@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@linear-code

linear-code Bot commented Jul 29, 2026

Copy link
Copy Markdown

MER-251

MER-267

MER-266

MER-141

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@chungty, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 37 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b0b88725-6442-4677-af6a-7f584830ae87

📥 Commits

Reviewing files that changed from the base of the PR and between 92ba6f8 and bf2a7dd.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (6)
  • gateway/document_extract.py
  • gateway/platforms/slack.py
  • pyproject.toml
  • tests/gateway/test_document_extract.py
  • tests/gateway/test_send_image_file.py
  • tests/gateway/test_slack.py
📝 Walkthrough

Walkthrough

Adds bounded DOCX extraction and integrates governed Slack uploads, Slack Connect attachment resolution, document ingestion, retry-limited downloads, and deduplicated thread attachment context with expanded tests.

Changes

Slack artifact and attachment handling

Layer / File(s) Summary
Bounded DOCX extraction and validation
gateway/document_extract.py, tests/gateway/test_document_extract.py
Adds bounded ordered DOCX text and table extraction with heading conversion, truncation metadata, structured errors, and coverage for malformed and oversized packages.
Governed local media uploads
gateway/platforms/slack.py, agent/prompt_builder.py, tools/send_message_tool.py, tests/gateway/test_slack.py, tests/gateway/test_send_multiple_images.py
Restricts Slack local uploads to approved artifact paths, validates bytes and archive formats, uploads content snapshots, and documents governed MEDIA paths.
Slack attachment resolution and ingestion
gateway/platforms/slack.py, tests/gateway/test_slack.py, tests/gateway/test_media_download_retry.py
Resolves Slack Connect files, bounds and retries downloads, extracts supported documents, injects untrusted attachment content, and reports attachment failures.
Bounded thread attachment context
gateway/platforms/slack.py, tests/gateway/test_slack_approval_buttons.py
Adds provenance-rich attachment blocks with deduplication, count/byte/character budgets, access notices, and retry coverage.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SlackEvent
  participant SlackAdapter
  participant SlackFiles
  participant DocumentExtractor
  participant AgentPrompt

  SlackEvent->>SlackAdapter: receive attachment metadata
  SlackAdapter->>SlackFiles: resolve files_info and download bounded bytes
  SlackFiles-->>SlackAdapter: attachment bytes
  SlackAdapter->>DocumentExtractor: extract DOCX text
  DocumentExtractor-->>SlackAdapter: text and truncation metadata
  SlackAdapter->>AgentPrompt: add provenance and untrusted attachment content
Loading

Suggested reviewers: teknium1

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 23.26% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately reflects the main changes: grounding Slack DOCX attachments and governing uploads.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Fix failing CI checks
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/mer-266-hermes-files

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown

🔎 Lint report: codex/mer-266-hermes-files vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 9279 on HEAD, 9276 on base (🆕 +3)

🆕 New issues (4):

Rule Count
unresolved-import 3
unresolved-attribute 1
First entries
tests/gateway/test_document_extract.py:6: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
gateway/document_extract.py:22: [unresolved-import] unresolved-import: Cannot resolve imported module `defusedxml`
tests/gateway/test_slack.py:629: [unresolved-attribute] unresolved-attribute: Attribute `kwargs` is not defined on `None` in union `_Call | None`
gateway/document_extract.py:23: [unresolved-import] unresolved-import: Cannot resolve imported module `defusedxml.common`

✅ Fixed issues (1):

Rule Count
invalid-return-type 1
First entries
gateway/platforms/slack.py:2933: [invalid-return-type] invalid-return-type: Function can implicitly return `None`, which is not assignable to return type `bytes`

Unchanged: 5061 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.

Fix All in Cursor

Bugbot Autofix prepared fixes for all 3 issues found in the latest run.

  • ✅ Fixed: Retry bytes trip size limit
    • Changed size limit check to only use current attempt bytes and return actual file size instead of cumulative transfer across retries.
  • ✅ Fixed: Char budget blocks all files
    • Separated char budget check from hard limits and only apply it to DOCX files since they're the only type with text extraction.
  • ✅ Fixed: XML bound trusts zip headers
    • Changed to stream-read with a hard byte limit per chunk instead of trusting the declared file_size in zip headers.

Create PR

Or push these changes by commenting:

@cursor push 1f13adad32
Preview (1f13adad32)
diff --git a/gateway/document_extract.py b/gateway/document_extract.py
--- a/gateway/document_extract.py
+++ b/gateway/document_extract.py
@@ -132,24 +132,29 @@ def extract_docx_text(
                 raise DocumentExtractionError(
                     "malformed_docx", "DOCX package is missing word/document.xml"
                 ) from exc
-            if info.file_size > max_xml_bytes:
-                raise DocumentExtractionError(
-                    "docx_too_large",
-                    f"DOCX main document XML exceeds the {max_xml_bytes}-byte extraction limit",
-                )
-            document_xml = archive.read(info)
+            # Stream-read with a hard byte limit to defend against zip bombs
+            # (malicious files with false declared sizes in headers).
+            chunks: list[bytes] = []
+            bytes_read = 0
+            with archive.open(info) as entry:
+                while True:
+                    chunk = entry.read(65536)
+                    if not chunk:
+                        break
+                    bytes_read += len(chunk)
+                    if bytes_read > max_xml_bytes:
+                        raise DocumentExtractionError(
+                            "docx_too_large",
+                            f"DOCX main document XML exceeds the {max_xml_bytes}-byte extraction limit",
+                        )
+                    chunks.append(chunk)
+            document_xml = b"".join(chunks)
     except DocumentExtractionError:
         raise
     except (BadZipFile, OSError, RuntimeError, ValueError) as exc:
         raise DocumentExtractionError(
             "malformed_docx", "DOCX package is malformed or unreadable"
         ) from exc
-
-    if len(document_xml) > max_xml_bytes:
-        raise DocumentExtractionError(
-            "docx_too_large",
-            f"DOCX main document XML exceeds the {max_xml_bytes}-byte extraction limit",
-        )
     try:
         root = ElementTree.fromstring(document_xml)
     except ElementTree.ParseError as exc:

@@ -132,24 +132,29 @@ def extract_docx_text(
                 raise DocumentExtractionError(
                     "malformed_docx", "DOCX package is missing word/document.xml"
                 ) from exc
-            if info.file_size > max_xml_bytes:
-                raise DocumentExtractionError(
-                    "docx_too_large",
-                    f"DOCX main document XML exceeds the {max_xml_bytes}-byte extraction limit",
-                )
-            document_xml = archive.read(info)
+            # Stream-read with a hard byte limit to defend against zip bombs
+            # (malicious files with false declared sizes in headers).
+            chunks: list[bytes] = []
+            bytes_read = 0
+            with archive.open(info) as entry:
+                while True:
+                    chunk = entry.read(65536)
+                    if not chunk:
+                        break
+                    bytes_read += len(chunk)
+                    if bytes_read > max_xml_bytes:
+                        raise DocumentExtractionError(
+                            "docx_too_large",
+                            f"DOCX main document XML exceeds the {max_xml_bytes}-byte extraction limit",
+                        )
+                    chunks.append(chunk)
+            document_xml = b"".join(chunks)
     except DocumentExtractionError:
         raise
     except (BadZipFile, OSError, RuntimeError, ValueError) as exc:
         raise DocumentExtractionError(
             "malformed_docx", "DOCX package is malformed or unreadable"
         ) from exc
-
-    if len(document_xml) > max_xml_bytes:
-        raise DocumentExtractionError(
-            "docx_too_large",
-            f"DOCX main document XML exceeds the {max_xml_bytes}-byte extraction limit",
-        )
     try:
         root = ElementTree.fromstring(document_xml)
     except ElementTree.ParseError as exc:

diff --git a/gateway/platforms/slack.py b/gateway/platforms/slack.py
--- a/gateway/platforms/slack.py
+++ b/gateway/platforms/slack.py
@@ -3021,10 +3021,11 @@ async def _fetch_thread_context(
                         continue
                     remaining_bytes = MAX_THREAD_ATTACHMENT_BYTES - attachment_bytes
                     remaining_chars = MAX_THREAD_EXTRACTED_CHARS - extracted_chars
+
+                    # Hard budget limits (file count, total bytes)
                     if (
                         attachment_count >= MAX_THREAD_ATTACHMENT_FILES
                         or remaining_bytes <= 0
-                        or remaining_chars <= 0
                     ):
                         attachment_parts.append(
                             "[Slack thread attachment notice]\n"

@@ -3021,10 +3021,11 @@ async def _fetch_thread_context(
                         continue
                     remaining_bytes = MAX_THREAD_ATTACHMENT_BYTES - attachment_bytes
                     remaining_chars = MAX_THREAD_EXTRACTED_CHARS - extracted_chars
+
+                    # Hard budget limits (file count, total bytes)
                     if (
                         attachment_count >= MAX_THREAD_ATTACHMENT_FILES
                         or remaining_bytes <= 0
-                        or remaining_chars <= 0
                     ):
                         attachment_parts.append(
                             "[Slack thread attachment notice]\n"
@@ -3033,6 +3034,21 @@ async def _fetch_thread_context(
                         )
                         attachment_budget_exhausted = True
                         continue
+
+                    # Char budget only applies to DOCX (text extraction)
+                    filename = str(file_obj.get("name") or "")
+                    _, ext = os.path.splitext(filename)
+                    ext = ext.lower()
+                    is_docx = (
+                        ext == ".docx"
+                        or mimetype == "application/vnd.openxmlformats-officedocument.wordprocessingml.document"
+                    )
+                    if is_docx and remaining_chars <= 0:
+                        attachment_parts.append(
+                            "[Slack thread attachment notice]\n"
+                            "DOCX attachment skipped: text-extraction budget exhausted."
+                        )
+                        continue
                     attachment_count += 1
                     try:
                         block, downloaded_bytes, document_chars = (

@@ -3033,6 +3034,21 @@ async def _fetch_thread_context(
                         )
                         attachment_budget_exhausted = True
                         continue
+
+                    # Char budget only applies to DOCX (text extraction)
+                    filename = str(file_obj.get("name") or "")
+                    _, ext = os.path.splitext(filename)
+                    ext = ext.lower()
+                    is_docx = (
+                        ext == ".docx"
+                        or mimetype == "application/vnd.openxmlformats-officedocument.wordprocessingml.document"
+                    )
+                    if is_docx and remaining_chars <= 0:
+                        attachment_parts.append(
+                            "[Slack thread attachment notice]\n"
+                            "DOCX attachment skipped: text-extraction budget exhausted."
+                        )
+                        continue
                     attachment_count += 1
                     try:
                         block, downloaded_bytes, document_chars = (
@@ -3371,7 +3387,7 @@ async def _download_slack_file_bytes(
                             async for chunk in response.aiter_bytes():
                                 data.extend(chunk)
                                 attempt_bytes_consumed += len(chunk)
-                                if retry_bytes_consumed + attempt_bytes_consumed > max_bytes:
+                                if attempt_bytes_consumed > max_bytes:
                                     raise _SlackAttachmentError(
                                         "Slack attachment download exceeded the "
                                         f"{max_bytes}-byte limit.",

@@ -3371,7 +3387,7 @@ async def _download_slack_file_bytes(
                             async for chunk in response.aiter_bytes():
                                 data.extend(chunk)
                                 attempt_bytes_consumed += len(chunk)
-                                if retry_bytes_consumed + attempt_bytes_consumed > max_bytes:
+                                if attempt_bytes_consumed > max_bytes:
                                     raise _SlackAttachmentError(
                                         "Slack attachment download exceeded the "
                                         f"{max_bytes}-byte limit.",
@@ -3380,8 +3396,7 @@ async def _download_slack_file_bytes(
                                         ),
                                     )
                             payload = bytes(data)
-                            consumed = retry_bytes_consumed + attempt_bytes_consumed
-                            return (payload, consumed) if return_consumed else payload
+                            return (payload, attempt_bytes_consumed) if return_consumed else payload
                     response = await client.get(
                         url,
                         headers={"Authorization": f"Bearer {bot_token}"},

@@ -3380,8 +3396,7 @@ async def _download_slack_file_bytes(
                                         ),
                                     )
                             payload = bytes(data)
-                            consumed = retry_bytes_consumed + attempt_bytes_consumed
-                            return (payload, consumed) if return_consumed else payload
+                            return (payload, attempt_bytes_consumed) if return_consumed else payload
                     response = await client.get(
                         url,
                         headers={"Authorization": f"Bearer {bot_token}"},

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit f117d3674dad3b4cd64bce85e300870fc15ad16f. Configure here.

Comment thread gateway/platforms/slack.py
Comment thread gateway/platforms/slack.py
Comment thread gateway/document_extract.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (3)
gateway/platforms/slack.py (2)

3352-3384: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Cumulative retry_bytes_consumed shrinks the per-attempt budget, so a retried near-limit download fails with a misleading message.

After a partial attempt consumes N bytes, the retry only has max_bytes - N of headroom and aborts with "download exceeded the {max_bytes}-byte limit" even though the file itself is within the limit. If the aggregate transfer budget is intentional (it appears to be, per the thread accounting), the message should say the transfer budget was exhausted rather than implying the file is oversized.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gateway/platforms/slack.py` around lines 3352 - 3384, The max_bytes overflow
handling in the retry loop around retry_bytes_consumed should distinguish
aggregate transfer-budget exhaustion from an oversized attachment. Preserve the
cumulative accounting and limit check, but update the _SlackAttachmentError
message to clearly state that the transfer budget was exceeded when
retry_bytes_consumed contributes to the limit breach.

1285-1311: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoff

Archive scan reads and decodes every member in full.

For a 20 MB archive this decompresses each entry into memory and runs the full redaction regex suite over payload.decode(...) twice (once on the container bytes, once per member) on the request path. The aggregate file_size cap bounds it, but consider streaming member reads with an early exit, or capping per-member scan length, if upload latency matters here.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gateway/platforms/slack.py` around lines 1285 - 1311, Update the archive scan
around ZipFile and _contains_secret to avoid reading and decoding each member in
full on the request path. Stream member contents in bounded chunks through the
secret detection logic, stopping immediately when a match is found, while
preserving the aggregate expansion limit and rejecting any archive containing
credentials.
tests/gateway/test_slack_approval_buttons.py (1)

349-417: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Budget tests hardcode literal constant values — change-detector risk.

Both tests bake in the exact numeric values of MAX_THREAD_ATTACHMENT_FILES (implied 10, via range(11) + await_count == 10) and MAX_THREAD_ATTACHMENT_BYTES (implied via two 20*1024*1024 chunks) instead of deriving expectations from the constants themselves. If either budget is retuned, these tests break even though the enforcement logic is still correct.

♻️ Suggested refactor: derive expectations from the actual constants
+from gateway.platforms.slack import MAX_THREAD_ATTACHMENT_FILES, MAX_THREAD_ATTACHMENT_BYTES
+
     async def test_thread_attachment_count_budget_skips_excess_files(self):
         adapter = _make_adapter()
         mock_client = adapter._team_clients["T1"]
         files = [
             {
                 "id": f"F_{index}",
                 "name": f"file-{index}.pdf",
                 "mimetype": "application/pdf",
                 "size": 4,
                 "url_private_download": f"https://files.slack.com/file-{index}.pdf",
             }
-            for index in range(11)
+            for index in range(MAX_THREAD_ATTACHMENT_FILES + 1)
         ]
         ...
-        assert adapter._download_slack_file_bytes.await_count == 10
+        assert adapter._download_slack_file_bytes.await_count == MAX_THREAD_ATTACHMENT_FILES
         assert "bounded attachment-context budget was reached" in context
-        assert "file_id=F_9" in context
-        assert "file_id=F_10" not in context
+        assert f"file_id=F_{MAX_THREAD_ATTACHMENT_FILES - 1}" in context
+        assert f"file_id=F_{MAX_THREAD_ATTACHMENT_FILES}" not in context

A similar derivation (e.g., bytes_consumed=MAX_THREAD_ATTACHMENT_BYTES // 2 + 1) would decouple the byte-budget test from the literal 20MB constant.

As per coding guidelines, "Do not write change-detector tests that fail whenever data expected to change (model catalogs, config version numbers, enumeration counts, hardcoded lists) gets updated. Write tests that assert relationships and invariants instead."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/gateway/test_slack_approval_buttons.py` around lines 349 - 417, Update
test_thread_attachment_count_budget_skips_excess_files to build
MAX_THREAD_ATTACHMENT_FILES + 1 files and assert the download count against
MAX_THREAD_ATTACHMENT_FILES instead of hardcoded 11 and 10. Update
test_thread_byte_budget_counts_rejected_misreported_downloads to derive
bytes_consumed from MAX_THREAD_ATTACHMENT_BYTES, such as half the budget plus
one, while preserving the assertion that enforcement stops after the budget is
reached.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@gateway/document_extract.py`:
- Around line 153-158: Update the DOCX XML parsing in the document extraction
flow around ElementTree.fromstring to use defusedxml’s safe parser for
user-supplied document_xml, while preserving the existing
DocumentExtractionError handling for malformed XML. Do not rely solely on the
size limit; ensure DTD and entity expansion attacks are blocked.

In `@gateway/platforms/slack.py`:
- Around line 2382-2387: Update the attachment condition in the inbound
processing flow around `_ingest_slack_document` so a URL alone does not select
the document path. Exclude attachments whose mimetype starts with `image/`,
`audio/`, or `video/`, matching `_fetch_thread_context`’s filtering, while
preserving processing for supported document mimetypes and recognized file
extensions.
- Around line 1215-1249: Update _prepare_local_upload to catch genuine OSError
failures from artifact-root setup and candidate.read_bytes(), while preserving
existing _SlackUploadPolicyError and FileNotFoundError behavior. Normalize these
filesystem failures into the adapter’s handled upload error type so send_video
and send_document continue returning SendResult instead of propagating
unexpected exceptions.

---

Nitpick comments:
In `@gateway/platforms/slack.py`:
- Around line 3352-3384: The max_bytes overflow handling in the retry loop
around retry_bytes_consumed should distinguish aggregate transfer-budget
exhaustion from an oversized attachment. Preserve the cumulative accounting and
limit check, but update the _SlackAttachmentError message to clearly state that
the transfer budget was exceeded when retry_bytes_consumed contributes to the
limit breach.
- Around line 1285-1311: Update the archive scan around ZipFile and
_contains_secret to avoid reading and decoding each member in full on the
request path. Stream member contents in bounded chunks through the secret
detection logic, stopping immediately when a match is found, while preserving
the aggregate expansion limit and rejecting any archive containing credentials.

In `@tests/gateway/test_slack_approval_buttons.py`:
- Around line 349-417: Update
test_thread_attachment_count_budget_skips_excess_files to build
MAX_THREAD_ATTACHMENT_FILES + 1 files and assert the download count against
MAX_THREAD_ATTACHMENT_FILES instead of hardcoded 11 and 10. Update
test_thread_byte_budget_counts_rejected_misreported_downloads to derive
bytes_consumed from MAX_THREAD_ATTACHMENT_BYTES, such as half the budget plus
one, while preserving the assertion that enforcement stops after the budget is
reached.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b19b619b-de19-4553-a5ff-cf5378ec7a2e

📥 Commits

Reviewing files that changed from the base of the PR and between f532c6b and f117d3674dad3b4cd64bce85e300870fc15ad16f.

📒 Files selected for processing (9)
  • agent/prompt_builder.py
  • gateway/document_extract.py
  • gateway/platforms/slack.py
  • tests/gateway/test_document_extract.py
  • tests/gateway/test_media_download_retry.py
  • tests/gateway/test_send_multiple_images.py
  • tests/gateway/test_slack.py
  • tests/gateway/test_slack_approval_buttons.py
  • tools/send_message_tool.py

Comment thread gateway/document_extract.py
Comment thread gateway/platforms/slack.py
Comment thread gateway/platforms/slack.py
@chungty
chungty force-pushed the codex/mer-266-hermes-files branch 2 times, most recently from 15ea071 to 20c41e4 Compare July 29, 2026 05:45
@cursor

cursor Bot commented Jul 29, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@chungty
chungty force-pushed the codex/mer-266-hermes-files branch from 20c41e4 to c548749 Compare July 29, 2026 05:51
@cursor

cursor Bot commented Jul 29, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

Add bounded DOCX ingestion for current messages and prior thread attachments. Upload only exact validated snapshots from an explicitly enabled artifact workflow, with aggregate transfer budgets and retry accounting.\n\nRefs MER-141, MER-251, MER-267.
@chungty
chungty force-pushed the codex/mer-266-hermes-files branch from c548749 to 92ba6f8 Compare July 29, 2026 05:54
@cursor

cursor Bot commented Jul 29, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@chungty

chungty commented Jul 29, 2026

Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@gateway/platforms/slack.py`:
- Around line 1279-1339: Update _assert_upload_content_safe to validate the
provided data snapshot for archive checks instead of reopening candidate from
disk: wrap data with BytesIO and pass it to ZipFile. Add the required io.BytesIO
import, while preserving the existing archive limits, Office member checks, and
credential scanning behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e59ff3f0-bb1e-4acd-a1c6-cb74fc16e385

📥 Commits

Reviewing files that changed from the base of the PR and between f117d3674dad3b4cd64bce85e300870fc15ad16f and 92ba6f8.

📒 Files selected for processing (9)
  • agent/prompt_builder.py
  • gateway/document_extract.py
  • gateway/platforms/slack.py
  • tests/gateway/test_document_extract.py
  • tests/gateway/test_media_download_retry.py
  • tests/gateway/test_send_multiple_images.py
  • tests/gateway/test_slack.py
  • tests/gateway/test_slack_approval_buttons.py
  • tools/send_message_tool.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • tools/send_message_tool.py
  • tests/gateway/test_media_download_retry.py

Comment thread gateway/platforms/slack.py
@chungty
chungty merged commit 44a5a46 into main Jul 30, 2026
19 checks passed
@chungty
chungty deleted the codex/mer-266-hermes-files branch July 30, 2026 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant