Skip to content

fix(cursor): bound output guard reasoning quarantine - #347

Closed
luvs01 wants to merge 5 commits into
devfrom
codex/fix-unbounded-reasoning-quarantine-vulnerability
Closed

luvs01 wants to merge 5 commits into
devfrom
codex/fix-unbounded-reasoning-quarantine-vulnerability

Conversation

@luvs01

@luvs01 luvs01 commented Aug 28, 2026

Copy link
Copy Markdown
Owner

Motivation

  • Prevent unbounded memory growth when an external Cursor upstream streams many pre-text reasoning frames that were previously quarantined without any aggregate cap.
  • The quarantine must account for non-text thinking_delta events so an attacker or compromised provider cannot exhaust process memory during tool-result continuations.

Description

  • Introduce a shared aggregate hold cap CURSOR_OUTPUT_GUARD_MAX_HOLD_BYTES and reuse it in the envelope sniffers (src/adapters/cursor/envelope-echo.ts).
  • Meter every quarantined event by serialized size in createCursorAdapter by adding guardHeldBytes, guardEncoder, and holdGuardEvent(event) so reasoning frames contribute to the same 8 KiB cap and trigger a flush/disarm when exceeded (src/adapters/cursor.ts).
  • When the aggregate cap is exceeded the code now calls echoSniffer?.finish() / routingCommentarySniffer?.finish() and releaseGuardHeld() to flush and disarm the quarantine so held events are emitted instead of growing unbounded.
  • Add a regression test demonstrating a reasoning-only stream crosses the cap and flushes instead of retaining events indefinitely (tests/cursor-envelope-echo-retry.test.ts).

Testing

  • Ran the focused unit tests: bun test tests/cursor-envelope-echo-retry.test.ts and they passed (all tests in that file passed).
  • Ran bun run typecheck and bun run privacy:scan, both succeeded.
  • Attempted the full test suite (bun run test); repository-wide integration/Lab tests showed unrelated environment-dependent failures (sandbox proxy env checks, some long integration timeouts and CLI help differences) and therefore are outside the scope of this focused fix.

Codex Task


Devin Review

@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Repository: luvs01/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: b7de7244-a77e-42b5-8297-bab2f8dcc45c


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

Copy link
Copy Markdown

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Aug 28, 2026
@github-actions github-actions Bot changed the title fix(cursor): bound output guard reasoning quarantine [WRONG BRANCH] fix(cursor): bound output guard reasoning quarantine Aug 28, 2026
@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown

✅ READY

  • all PR quality gates passed.

Hygiene

Deterministic PR hygiene checks passed.

@github-actions
github-actions Bot marked this pull request as draft August 28, 2026 02:18

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 327635678e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/adapters/cursor.ts
Comment on lines +261 to +264
if (guardHeldBytes <= CURSOR_OUTPUT_GUARD_MAX_HOLD_BYTES) return true;
echoSniffer?.finish();
routingCommentarySniffer?.finish();
releaseGuardHeld();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Inspect cap-crossing text before disarming the quarantine

When retained reasoning leaves guardHeldBytes just below 8 KiB and the next first text_delta both crosses the cap and begins with [Tool Result] or matching routing-failure commentary, this branch settles both sniffers and emits the held events before the caller feeds that text to them. The invalid marker therefore reaches the client and sets emittedOutput, preventing the corrective retry even though nothing had escaped before this delta; evaluate a cap-crossing text event with the active sniffers before deciding to flush and disarm.

AGENTS.md reference: src/AGENTS.md:L17-L19

Useful? React with 👍 / 👎.

@luvs01 luvs01 changed the title [WRONG BRANCH] fix(cursor): bound output guard reasoning quarantine fix(cursor): bound output guard reasoning quarantine Sep 3, 2026
@luvs01
luvs01 changed the base branch from main to dev September 3, 2026 06:23
@github-actions
github-actions Bot marked this pull request as ready for review September 3, 2026 06:33
devin-ai-integration[bot]

This comment was marked as resolved.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

# Conflicts:
#	src/adapters/cursor.ts
#	tests/cursor-envelope-echo-retry.test.ts
devin-ai-integration[bot]

This comment was marked as resolved.

luvs01 and others added 2 commits September 21, 2026 08:36
Two Devin Review findings on the turn-start output quarantine:

- A single oversized event (Cursor accepts frames up to 16 MiB) was fully
  retained and re-encoded before the 8 KiB cap released it. The retained
  size is now projected from payload length before any serialized copy
  exists; a frame that cannot fit settles the sniffers, releases the held
  events, and passes through directly.
- The aggregate cap was checked before the armed sniffers saw the text,
  so a >8 KiB first delta could bypass echo/routing classification
  entirely. Text deltas are now classified first (on the bounded leading
  window each sniffer needs) and only then subject to the cap.

Adds regression tests for oversized single deltas carrying each guarded
pattern and one oversized reasoning frame, and documents the aggregate
limit/disarm contract in structure/providers/cursor.md.

Co-Authored-By: Epinephrine <luvs01@hanmail.net>
luvs01 added a commit that referenced this pull request Sep 21, 2026
… reasoning quarantine

Combines two fork PRs touching envelope-echo: retain adjacent mid-stream echo diagnostics (#380) and bound the output-guard reasoning quarantine (#347), rebased onto current dev.

bun test cursor-envelope-echo-retry + cursor-adapter + cursor-tool-continuation: 101 pass
@luvs01

luvs01 commented Sep 21, 2026

Copy link
Copy Markdown
Owner Author

Closing: superseded by combined PR #610, which carries this PR's bounded output-guard reasoning quarantine together with the echo-diagnostics retention change on current dev. Combined tests: 47 + 54 pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aardvark bug Something isn't working codex

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant