Skip to content

fix(security): require an exact SSE completion sentinel - #2422

Merged
HAYDEN-OAI merged 1 commit into
mainfrom
codex/sse-completion-sentinel-integrity-20260819-2049
Aug 19, 2026
Merged

HAYDEN-OAI merged 1 commit into
mainfrom
codex/sse-completion-sentinel-integrity-20260819-2049

Conversation

@HAYDEN-OAI

Copy link
Copy Markdown
Contributor

Summary

  • accept only the exact [DONE] SSE completion marker, preventing forged marker prefixes or multiline frames from silently truncating streams and suppressing provider errors
  • preserve canonical optional field spacing, CRLF framing, exact named sentinels, early response cancellation, genuine post-completion behavior, existing private diagnostics, and typed provider failures
  • cover the real public Chat Completions, Responses, and Assistants streaming clients, plus the exported SSE stream helper

Regression-first proof

On unchanged main, the new public-boundary suite produced 14 failures and 20 passing compatibility controls: malformed marker suffixes, sensitive appended payloads, multiline impersonation, hidden provider errors, and open-reader cleanup incorrectly completed successfully. The one-comparison fix makes all 34 tests pass.

Validation

  • focused streaming coverage: 10 files, 275 tests passed
  • complete handwritten suite: 123 files, 3,877 tests passed
  • generated API suite against an isolated local Steady server: 82 suites, 559 tests passed (one existing skipped test)
  • repository formatter, lint, and strict TypeScript passed
  • production build, published declarations under current TypeScript and TypeScript 4.9, and package lint passed
  • packed npm artifact exercised all three real public streaming clients plus Stream.fromSSEResponse, rejecting forged markers while preserving genuine completion

Changes are limited to one handwritten comparison and one dedicated regression file.

@HAYDEN-OAI
HAYDEN-OAI requested a review from a team as a code owner August 19, 2026 20:53
@openai-sdks

openai-sdks Bot commented Aug 19, 2026 •

Copy link
Copy Markdown
Contributor

OkTest Summary

✅ 237/237 SDK tests passed in 10.095s for Node SDK PR #2422.

Test results — 42 files
Test Result Time
tests/chat-completions-complex-body.test.ts ✅ Passed 126ms
tests/chat-completions-create.test.ts ✅ Passed 234ms
tests/chat-completions-stream.test.ts ✅ Passed 210ms
tests/files-content-binary.test.ts ✅ Passed 138ms
tests/files-create-multipart.test.ts ✅ Passed 128ms
tests/files-list-pagination.test.ts ✅ Passed 161ms
tests/initialize-config.test.ts ✅ Passed 162ms
tests/instance-isolation.test.ts ✅ Passed 110ms
tests/models-list.test.ts ✅ Passed 145ms
tests/responses-background-lifecycle.test.ts ✅ Passed 228ms
tests/responses-body-method-errors.test.ts ✅ Passed 276ms
tests/responses-cancel-timeout.test.ts ✅ Passed 234ms
tests/responses-cancel.test.ts ✅ Passed 243ms
tests/responses-compact-retries.test.ts ✅ Passed 293ms
tests/responses-compact.test.ts ✅ Passed 220ms
tests/responses-create-advanced-stream.test.ts ✅ Passed 131ms
tests/responses-create-advanced.test.ts ✅ Passed 166ms
tests/responses-create-disconnect.test.ts ✅ Passed 1.118s
tests/responses-create-errors.test.ts ✅ Passed 194ms
tests/responses-create-malformed-api-responses.test.ts ✅ Passed 192ms
tests/responses-create-retries.test.ts ✅ Passed 238ms
tests/responses-create-stream-failures.test.ts ✅ Passed 170ms
tests/responses-create-stream-timeout.test.ts ✅ Passed 2.147s
tests/responses-create-stream-wire.test.ts ✅ Passed 2.394s
tests/responses-create-stream.test.ts ✅ Passed 76ms
tests/responses-create-terminal-states.test.ts ✅ Passed 247ms
tests/responses-create-timeout.test.ts ✅ Passed 225ms
tests/responses-create.test.ts ✅ Passed 146ms
tests/responses-delete.test.ts ✅ Passed 137ms
tests/responses-input-items-errors.test.ts ✅ Passed 187ms
tests/responses-input-items-list.test.ts ✅ Passed 189ms
tests/responses-input-items-options.test.ts ✅ Passed 110ms
tests/responses-input-tokens-count-timeout.test.ts ✅ Passed 245ms
tests/responses-input-tokens-count.test.ts ✅ Passed 270ms
tests/responses-malformed-inputs.test.ts ✅ Passed 1.984s
tests/responses-not-found-errors.test.ts ✅ Passed 232ms
tests/responses-parse.test.ts ✅ Passed 214ms
tests/responses-retrieve-retries.test.ts ✅ Passed 222ms
tests/responses-retrieve.test.ts ✅ Passed 203ms
tests/responses-stored-method-errors.test.ts ✅ Passed 596ms
tests/retry-behavior.test.ts ✅ Passed 3.088s
tests/sdk-error-shape.test.ts ✅ Passed 239ms

View OkTest run #32301013564

SDK merge (f5cc8d471e00) · head (9a79e4d2b087) · base (dcc705c118fe) · OkTest (2b1bdfd25e98)

@jbeckwith-oai jbeckwith-oai 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.

Independent two-pass security review confirms SSE streams now terminate only on the exact [DONE] sentinel. Forged suffixed or multiline events no longer truncate streams, while valid protocol framing, UTF-8 fragmentation, cancellation, diagnostics, and Chat/Responses/Assistants behavior remain compatible. Comprehensive regressions and substantive CI are green; no actionable findings.

@HAYDEN-OAI
HAYDEN-OAI added this pull request to the merge queue Aug 19, 2026
Merged via the queue into main with commit a905bfb Aug 19, 2026
29 checks passed
@HAYDEN-OAI
HAYDEN-OAI deleted the codex/sse-completion-sentinel-integrity-20260819-2049 branch August 19, 2026 21:07
@openai-sdks openai-sdks Bot mentioned this pull request Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants