Skip to content

fix(relay): stream non-JSON upstream error bodies safely - #106

Closed
luvs01 wants to merge 1 commit into
devfrom
codex/propose-fix-for-unbounded-logging-vulnerability
Closed

luvs01 wants to merge 1 commit into
devfrom
codex/propose-fix-for-unbounded-logging-vulnerability

Conversation

@luvs01

@luvs01 luvs01 commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Motivation

  • The deferred-request logging path previously called response.text() for any status >= 400, which buffers the entire upstream body and creates a DoS surface for large or never-ending non-JSON error responses.
  • The intent is to capture useful diagnostics while avoiding unbounded memory/time consumption and still forward the full response to the client.

Description

  • Stop using response.text() for all error statuses and restrict full-text inspection to application/json responses only by changing the JSON-branch guard to contentType.includes("application/json") and keeping JSON inspection unchanged.
  • Add a streaming fallback for non-JSON error responses that forwards chunks to the client immediately while retaining only an in-memory prefix (8 KiB) for logging, implemented with getReader() and a ReadableStream wrapper.
  • Introduce MAX_NON_JSON_ERROR_INSPECTION_BYTES = 8192 as the byte cap for retained diagnostic data and finalize logging on EOF, read error, or cancellation without buffering the whole body.
  • Add regression tests in tests/request-log.test.ts that assert non-JSON error bodies stream the first chunk immediately and that oversized error bodies are preserved downstream while only a bounded prefix is logged.

Testing

  • Ran bun run typecheck and it completed with no type errors.
  • Ran focused tests with OCX_TEST_NO_QUEUE=1 bun run test -- tests/request-log.test.ts and the modified request-log suite passed (47 tests).
  • Ran the full test harness (OCX_TEST_NO_QUEUE=1 bun run test) and observed the test run complete with tests passing in this run.
  • Ran bun run privacy:scan which passed with no privacy issues detected.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Aug 8, 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 7b03125f-e0be-46ad-b864-1174d6e75a25


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 Aug 8, 2026

Copy link
Copy Markdown

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Aug 8, 2026
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown

✅ READY

  • all PR quality gates passed.

Hygiene

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot changed the title fix(relay): stream non-JSON upstream error bodies safely [WRONG BRANCH] fix(relay): stream non-JSON upstream error bodies safely Aug 8, 2026
@github-actions
github-actions Bot marked this pull request as draft August 8, 2026 08:59
@luvs01 luvs01 closed this Aug 19, 2026
@luvs01 luvs01 reopened this Aug 19, 2026
@luvs01 luvs01 changed the title [WRONG BRANCH] fix(relay): stream non-JSON upstream error bodies safely fix(relay): stream non-JSON upstream error bodies safely Sep 3, 2026
@luvs01
luvs01 changed the base branch from main to dev September 3, 2026 06:26
@github-actions
github-actions Bot marked this pull request as ready for review September 3, 2026 06:35
@luvs01

luvs01 commented Sep 15, 2026

Copy link
Copy Markdown
Owner Author

Consolidated into #177 as the single main work item. Its checklist now carries this proposal’s requirements and relevant unresolved review concerns. Closing this duplicate tracking item does not mean the combined implementation is finished. The branch and review history are retained.

@luvs01 luvs01 closed this Sep 15, 2026
github-actions Bot pushed a commit that referenced this pull request Sep 16, 2026
Adapt #177 and #106 onto current dev. Preserve terminal ownership with bounded tee read-ahead, stream non-SSE bodies, and cover cancellation races.
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