Skip to content

fix: bound sys.stdin.read() to prevent OOM on oversized payloads - #3903

Open
Quratulain-bilal wants to merge 2 commits into
github:mainfrom
Quratulain-bilal:fix/bounded-stdin-read
Open

fix: bound sys.stdin.read() to prevent OOM on oversized payloads#3903
Quratulain-bilal wants to merge 2 commits into
github:mainfrom
Quratulain-bilal:fix/bounded-stdin-read

Conversation

@Quratulain-bilal

Copy link
Copy Markdown
Contributor

Problem

sys.stdin.read() loads all of stdin into memory with no size limit. A malfunctioning agent hook or malicious process can pipe gigabytes of data into stdin, exhausting process memory before the payload is ever consumed.

This affects both the CLI event runner (commands/event.py) and the standalone events entry point (events.py).

Fix

Cap stdin reads at 10 MiB using a bounded chunked read loop. The function reads in 64 KiB chunks and stops once the limit is reached, preventing unbounded memory allocation.

Testing

  • Verified that normal payloads (well under 10 MiB) pass through unchanged
  • Verified that the bounded reader stops at the limit instead of reading all input

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Attempts to cap event-runner stdin payloads at 10 MiB to reduce memory-exhaustion risk.

Changes:

  • Adds chunked stdin readers.
  • Applies them to CLI and generated event dispatchers.
Show a summary per file
File Description
src/specify_cli/events.py Bounds generated dispatcher input.
src/specify_cli/commands/event.py Bounds CLI event input.

Review details

Tip

Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Suppressed comments (2)

src/specify_cli/events.py:317

  • This call is emitted inside _EVENTS_DISPATCHER_TEMPLATE, but _read_stdin_bounded was defined in the host module before the template starts. The generated .specify/events.py therefore has no such function and every standalone event invocation raises NameError before dispatch. Move the constant and helper into the generated template (and remove the unused host-level copy).
    payload = _read_stdin_bounded()

src/specify_cli/commands/event.py:19

  • sys.stdin.read(n) and len(str) count characters rather than encoded bytes, so this does not enforce _MAX_STDIN_BYTES; UTF-8 non-ASCII payloads may read and retain substantially more than 10 MiB. Read bounded chunks from sys.stdin.buffer and decode afterward with a defined truncation/error policy.
        chunk = sys.stdin.read(min(max_bytes - total, 65536))
  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread src/specify_cli/events.py Outdated
Comment thread src/specify_cli/commands/event.py
…te limiting

Address Copilot review feedback on PR github#3903:

- Use sys.stdin.buffer.read() instead of sys.stdin.read() so the 10 MiB
  limit is enforced on raw bytes rather than Unicode code points (a 4-byte
  UTF-8 sequence now counts as 4 bytes, not 1 character).
- Add regression tests for both the events module and CLI event runner:
  below-limit, exact-limit, oversized, multibyte UTF-8, empty stdin,
  TTY, and invalid UTF-8 replacement.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread src/specify_cli/events.py
# Gemini/Tabnine/Devin which derive from the same protocol).
native_event = sys.argv[5] if len(sys.argv) >= 6 else ""
payload = sys.stdin.read() if not sys.stdin.isatty() else "{}"
payload = _read_stdin_bounded()
The generated .specify/events.py files cannot import _read_stdin_bounded
from the host module. Embed the bounded-read implementation directly in
the template so it is self-contained.
@Quratulain-bilal
Quratulain-bilal force-pushed the fix/bounded-stdin-read branch 2 times, most recently from 0f4f3d5 to 2967161 Compare August 17, 2026 20:36
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.

3 participants