Skip to content

fix(discord): bound server tool response reads - #55285

Closed
ooiuuii wants to merge 1 commit into
NousResearch:mainfrom
ooiuuii:fix/discord-tool-response-cap
Closed

fix(discord): bound server tool response reads#55285
ooiuuii wants to merge 1 commit into
NousResearch:mainfrom
ooiuuii:fix/discord-tool-response-cap

Conversation

@ooiuuii

@ooiuuii ooiuuii commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • cap tools.discord_tool._discord_request() success bodies with a bounded read(limit + 1)
  • cap Discord HTTP error bodies before building DiscordAPIError
  • add focused regression coverage for oversized success and error bodies

Fixes #55284.

This is separate from #54747 and #55209: those cover the Discord platform adapter / standalone send paths, while this PR only covers the Discord server tool in tools/discord_tool.py.

Test plan

  • C:\Users\Administrator\Documents\Codex\2026-06-29\hermes-main-latest-scan\.venv\Scripts\python.exe -m pytest tests\tools\test_discord_tool.py -q --basetemp .pytest-tmp-discord-tool-cap
  • C:\Users\Administrator\Documents\Codex\2026-06-29\hermes-main-latest-scan\.venv\Scripts\python.exe -m ruff check tools\discord_tool.py tests\tools\test_discord_tool.py
  • git diff --check

@alt-glitch alt-glitch added type/bug Something isn't working comp/tools Tool registry, model_tools, toolsets platform/discord Discord bot adapter P3 Low — cosmetic, nice to have labels Jun 30, 2026

@tonydwb tonydwb 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.

Code Review Summary

Verdict: LGTM

Discord server tool response bounding for security.

Looks Good

  • Caps success bodies with bounded read(limit + 1)
  • Caps error bodies before building DiscordAPIError
  • 36 lines of regression tests for oversized responses
  • Security fix: prevents unbounded memory consumption from large API responses

@teknium1

teknium1 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Thanks for the report and the fix — but this duplicates your own earlier PR #54747, which bounds the same reads in the same function (_discord_request() in tools/discord_tool.py) and was submitted a day earlier with a success/error cap split (4 MiB / 64 KiB). The issue's premise that #54747 "does not touch tools/discord_tool.py" appears to have been a mix-up.

#54747 has been salvaged onto current main via #60112 with your authorship preserved. Closing this one as the duplicate. Appreciate the thorough hardening work across these Discord surfaces!

@teknium1 teknium1 closed this Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have platform/discord Discord bot adapter type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Discord server tool reads REST success/error bodies without a cap

4 participants