Skip to content

fix(assistants): reject run-step identity hijacking before tool callbacks - #2476

Open
HAYDEN-OAI wants to merge 3 commits into
mainfrom
codex/assistant-run-step-identity-20260825-2109-7fb0cf32
Open

fix(assistants): reject run-step identity hijacking before tool callbacks#2476
HAYDEN-OAI wants to merge 3 commits into
mainfrom
codex/assistant-run-step-identity-20260825-2109-7fb0cf32

Conversation

@HAYDEN-OAI

Copy link
Copy Markdown
Contributor

Summary

  • Validate canonical assistant run-step identities before emitting raw events, mutating snapshots, or finalizing tool calls.
  • Reject duplicate run-step creation and foreign lifecycle events while preserving standalone terminal events, reserved identifiers, sequential steps, listener mutations, and descriptor-safe proxy handling.
  • Add 27 regression cases through the public AssistantStream.fromReadableStream() NDJSON transport and the existing streaming helper.

Validation

  • Red-first: 19 run-step identity/security assertions failed on current main; all 27 new assertions now pass.
  • Focused AssistantStream suites: 176 tests passed.
  • Full handwritten suites: 6,303 tests across 149 files passed.
  • Generated API suites against an isolated private mock: 556 tests across 82 suites passed.
  • Repository lint, strict TypeScript, production build, published TypeScript 4.9 and current TypeScript checks, publint, and packed-package verification passed.
  • The packed CommonJS and ESM public AssistantStream exports both reject the original duplicate-ID attack before exposing its attacker-controlled callback or snapshot.

@HAYDEN-OAI
HAYDEN-OAI requested a review from a team as a code owner August 25, 2026 21:26
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-08-25T22:00:13.348913Z 3253164 New commits
🔒 Security Review Completed 2026-08-25T22:00:38.393899Z 3253164 New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@openai-sdks

openai-sdks Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

OkTest Summary

237/237 SDK tests passed in 10.421s for Node SDK PR #2476.

Test results — 42 files
Test Result Time
tests/chat-completions-complex-body.test.ts ✅ Passed 174ms
tests/chat-completions-create.test.ts ✅ Passed 241ms
tests/chat-completions-stream.test.ts ✅ Passed 158ms
tests/files-content-binary.test.ts ✅ Passed 161ms
tests/files-create-multipart.test.ts ✅ Passed 146ms
tests/files-list-pagination.test.ts ✅ Passed 152ms
tests/initialize-config.test.ts ✅ Passed 170ms
tests/instance-isolation.test.ts ✅ Passed 97ms
tests/models-list.test.ts ✅ Passed 148ms
tests/responses-background-lifecycle.test.ts ✅ Passed 207ms
tests/responses-body-method-errors.test.ts ✅ Passed 284ms
tests/responses-cancel-timeout.test.ts ✅ Passed 198ms
tests/responses-cancel.test.ts ✅ Passed 278ms
tests/responses-compact-retries.test.ts ✅ Passed 294ms
tests/responses-compact.test.ts ✅ Passed 193ms
tests/responses-create-advanced-stream.test.ts ✅ Passed 125ms
tests/responses-create-advanced.test.ts ✅ Passed 183ms
tests/responses-create-disconnect.test.ts ✅ Passed 1.177s
tests/responses-create-errors.test.ts ✅ Passed 181ms
tests/responses-create-malformed-api-responses.test.ts ✅ Passed 137ms
tests/responses-create-retries.test.ts ✅ Passed 260ms
tests/responses-create-stream-failures.test.ts ✅ Passed 248ms
tests/responses-create-stream-timeout.test.ts ✅ Passed 2.165s
tests/responses-create-stream-wire.test.ts ✅ Passed 2.557s
tests/responses-create-stream.test.ts ✅ Passed 111ms
tests/responses-create-terminal-states.test.ts ✅ Passed 252ms
tests/responses-create-timeout.test.ts ✅ Passed 220ms
tests/responses-create.test.ts ✅ Passed 178ms
tests/responses-delete.test.ts ✅ Passed 191ms
tests/responses-input-items-errors.test.ts ✅ Passed 177ms
tests/responses-input-items-list.test.ts ✅ Passed 186ms
tests/responses-input-items-options.test.ts ✅ Passed 108ms
tests/responses-input-tokens-count-timeout.test.ts ✅ Passed 236ms
tests/responses-input-tokens-count.test.ts ✅ Passed 312ms
tests/responses-malformed-inputs.test.ts ✅ Passed 2.127s
tests/responses-not-found-errors.test.ts ✅ Passed 238ms
tests/responses-parse.test.ts ✅ Passed 205ms
tests/responses-retrieve-retries.test.ts ✅ Passed 245ms
tests/responses-retrieve.test.ts ✅ Passed 248ms
tests/responses-stored-method-errors.test.ts ✅ Passed 535ms
tests/retry-behavior.test.ts ✅ Passed 3.195s
tests/sdk-error-shape.test.ts ✅ Passed 257ms

View OkTest run #32903783257

SDK merge (cda11eb7d9e0) · head (3253164713e4) · base (9dd86484d9d5) · OkTest (2b1bdfd25e98)

@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Castiron custom code

✅ No new custom-code files detected.

32 mixed files remain; 0 existing customizations changed.

Compared 9dd86484d9d53253164713e4. Generated baselines verified.

32 existing customizations unchanged
  • api.md
  • scripts/castiron/README.md
  • scripts/castiron/custom_code_report.py
  • scripts/castiron/test_custom_code_report.py
  • src/client.ts
  • src/resources/audio/transcriptions.ts
  • src/resources/audio/translations.ts
  • src/resources/beta/assistants.ts
  • src/resources/beta/beta.ts
  • src/resources/beta/index.ts
  • src/resources/beta/responses/internal-base.ts
  • src/resources/beta/responses/responses.ts
  • src/resources/beta/threads/index.ts
  • src/resources/beta/threads/runs/index.ts
  • src/resources/beta/threads/runs/runs.ts
  • src/resources/beta/threads/threads.ts
  • src/resources/chat/completions/completions.ts
  • src/resources/chat/completions/index.ts
  • src/resources/conversations/index.ts
  • src/resources/embeddings.ts
  • src/resources/files.ts
  • src/resources/fine-tuning/checkpoints/permissions.ts
  • src/resources/images.ts
  • src/resources/responses/internal-base.ts
  • src/resources/responses/responses.ts
  • src/resources/skills/skills.ts
  • src/resources/skills/versions/versions.ts
  • src/resources/vector-stores/file-batches.ts
  • src/resources/vector-stores/files.ts
  • src/resources/webhooks/index.ts
  • src/resources/webhooks/webhooks.ts
  • tests/lib/data-residency.test.ts

A changed generated baseline means this report cannot reliably identify which handwritten lines changed.

Inspect the custom-code diff

Download the exact patch produced by this run (requires repository access):

gh run download 32903817290 --repo openai/openai-node \
  --name castiron-custom-code-32903817290-1 --dir /tmp/castiron-custom-code-32903817290-1
git apply --stat /tmp/castiron-custom-code-32903817290-1/custom-code.patch
cat /tmp/castiron-custom-code-32903817290-1/custom-code.patch

Or reproduce it from an SDK checkout containing the vendored reporter:

git fetch --no-tags origin 9dd86484d9d5a2212ea8ec472fad6f76c747ba14 3253164713e440d83b09d634a43c1527c5246e70
python3 scripts/castiron/custom_code_report.py report \
  --base 9dd86484d9d5a2212ea8ec472fad6f76c747ba14 \
  --head 3253164713e440d83b09d634a43c1527c5246e70 --fetch --require-head-hash --public \
  --out /tmp/castiron-custom-code-3253164713e4
cat /tmp/castiron-custom-code-3253164713e4/custom-code.patch

This is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR.

Full report and patch

@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: bb42bbc222

ℹ️ 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/lib/AssistantStream.ts

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

Requesting changes for the existing P1 discussion at src/lib/AssistantStream.ts:463. A public generic event listener can mutate the retained currentRunStepSnapshot().id from active step B to an already-completed step A after event.data has been validated. #handleRunStep then emits privileged toolCallCreated/toolCallDelta/runStepDelta callbacks against the hijacked identity, and the retained-snapshot ownership check rejects only after those irreversible callbacks have already run. Revalidate the retained active snapshot and its canonical owner immediately after generic event emission and before any callback-producing handler; add a two-step A/B mutating-listener regression proving that no tool callback executes. Existing inline discussion already captures the blocker.

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