Skip to content

fix(adapters): bound inline image decoding - #523

Open
luvs01 wants to merge 31 commits into
devfrom
codex/propose-fix-for-image-normalization-vulnerability
Open

luvs01 wants to merge 31 commits into
devfrom
codex/propose-fix-for-image-normalization-vulnerability

Conversation

@luvs01

@luvs01 luvs01 commented Sep 14, 2026

Copy link
Copy Markdown
Owner

Motivation

  • The openai-chat image normalization path decoded attacker-supplied inline images without a safe decoded-memory cap, allowing a small number of requests to force large native decodes and exhaust process memory/CPU.
  • The normalization flow also did not observe the request abort signal, so cancelled clients could not stop in-flight native work.

Description

  • Lower per-image decoded-pixel ceiling from 100_000_000 to 16_000_000 and reject inputs that exceed it by checking Bun image metadata before a full decode in bunImageEncode and bunImageValidate (src/adapters/anthropic-image-codec.ts).
  • Serialize native decode work with a process-wide admission gate sized to IMAGE_NORMALIZE_CONCURRENCY so decoded-image work is limited across requests and not per-request (src/adapters/anthropic-image-normalize.ts).
  • Propagate abort signals into adapter request construction and translated-chat normalization by passing abortSignal to buildRequest and normalizeOpenAIChatImages, and make the normalizer respect cancellation so queued decode work stops when the client disconnects (src/server/responses/core.ts, src/adapters/openai-chat.ts, src/adapters/openai-chat-images.ts, src/adapters/anthropic-image-normalize.ts).
  • Add regression tests that assert the decoder gate is shared across concurrent requests, that highly-compressed huge images do not reach the decoder, and that already-cancelled oversized builds do not start normalization (tests/adapters/anthropic/anthropic-image-normalize.test.ts, tests/adapters/openai/openai-chat-image-normalization.test.ts).
  • Update structure/docs to record the process-wide decoder admission, the metadata-confirmed pixel ceiling, and cancellation semantics (structure/* updates).

Testing

  • Ran the focused adapter tests with ./node_modules/bun/bin/bun.exe test tests/adapters/openai/openai-chat-image-normalization.test.ts tests/adapters/anthropic/anthropic-image-normalize.test.ts, which completed with 42 passed, 0 failed.
  • Ran bun run typecheck, bun run privacy:scan, and bun run structure:check, all of which passed in this environment.
  • Ran the repository full test driver (bun run test) in this container; the image-normalization changes passed locally but the full parallel suite reported unrelated failures in tests/server/api-key-attribution.test.ts that are not caused by the image-normalization changes (environmental/cross-test server interactions observed in this CI container).

Codex Task

lidge-jun and others added 30 commits September 6, 2026 01:08
…43-01a07240

[WRONG BRANCH] chore(release): promote verified candidate to 2.43.0
…in-07c0

chore(release): promote validated 2.44.0 to main
Promote frozen dev source cf9f662; no new runtime changes. Candidate CI34061274315 and service34061276621 are the validation references. Publication waits for successful validation and the final main push CI at the exact release SHA.
Promote frozen dev source cf9f662 as 2.45.0.

The repository owner explicitly authorized this main/preview release promotion and admin PR-only merge. This is a release-specific owner decision, not an independent approval or the dev-only maintainer exception.

Frozen candidate full CI34061274315 passed all25jobs after one unchanged-source rerun of Windows5; the initial holder busy assertion remains recorded without a root-cause resolution claim. Service lifecycle34061276621 passed Linux/macOS/Windows. Dev version pre-move3812 is merged. Publication still requires this actual main merge SHA's own successful push CI and Service lifecycle. No local suites were run.
[WRONG BRANCH] chore(release): promote verified 2.46.0 to main
[WRONG BRANCH] release: promote 2.47.0 to main
[WRONG BRANCH] release: apply final roster correction to main
[WRONG BRANCH] release: promote 2.48.0 to main
…in-01a08498

release: promote verified 2.49.0 product tree to main
Promotes the audited freeze SHA 12c248f unchanged. The resulting tree is byte-identical to that commit's tree, d8f5a71, so what ships is exactly what the six-lane regression audit read.
…in-01a08a81

[WRONG BRANCH] release: promote verified 2.50.0 product tree to main
[WRONG BRANCH] release: promote verified 2.51.0 product tree to main
Product tree is dev at 7bfb2ad. Only package.json differs, carrying the
stable channel version 2.52.0.

This promotion follows a CI repair and a regression sweep: dev had drifted 79
commits past its last finished run and the one run allowed to complete had
failed. lidge-jun#4390 fixed an integration test that never denied anything and had been
red on dev since lidge-jun#4342; lidge-jun#4396/lidge-jun#4397/lidge-jun#4398 closed four gaps in the quota
avoidance contract lidge-jun#4368 introduced; lidge-jun#4403 made a port fixture deterministic.
release: promote verified 2.52.0 product tree to main
Product tree is dev at eb81eaa, byte-identical (dev already carries the
2.53.0 version line).

This promotion follows a 364-commit regression audit (origin/main..981b53e)
by 15 parallel subagent lanes plus two audit-spawned fixes reviewed and merged
to dev: lidge-jun#4496 (live sideband connect watchdog) and lidge-jun#4497 (chat image part
shapes). Zero unresolved P0/P1 at promotion time. Exact-head hosted CI green
on eb81eaa (run 34750934849). Local suite NOT RUN per task rule; hosted
exact-head CI is the gate.
[WRONG BRANCH] release: promote verified 2.53.0 product tree to main
Product tree is dev at a84e6e8, byte-identical (dev already carries the
2.54.0 version line).

Headed by the Codex model-picker incident fix (lidge-jun#4531). On Codex
0.154.0-alpha.6.2 a paginated-history preflight vetoed the entire Codex config
write, so model_catalog_json never reached config.toml and both the Codex app
and the CLI fell back to their six built-in models, while ocx sync reported that
failure as success. The refusal is now scoped to the conversation-history
relabel unit on the apply direction only, narrowed to the one permanent reason,
and a provider table the home already published survives the write.

Exact-head hosted CI green on a84e6e8 via lidge-jun#4531 (run 34772128322): all four
Linux test shards, both macOS shards, gates, hygiene, storage policy, api usage,
keyring x3, npm-global x3, docker smoke, react-doctor. Local suite NOT RUN per
task rule; hosted exact-head CI is the gate.
[WRONG BRANCH] release: promote verified 2.54.0 product tree to main
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 14, 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-09-14T01:01:00.222261Z 673d542 PR opened
ℹ️ 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.

@coderabbitai

coderabbitai Bot commented Sep 14, 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: Advanced

Run ID: e1e7c234-54e9-4f69-9482-9e2ab2f29088


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.

@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: 673d542aed

ℹ️ 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 on lines 112 to +114
export interface NormalizeTargetsOptions extends NormalizeOptions {
/** Cancels queued native decode work and stops pulling more images. */
abortSignal?: AbortSignal;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Pass abort signals through every normalizer wrapper

The new process-wide queue is cancellable only when abortSignal reaches this option, but the Anthropic production paths do not expose or pass it: normalizeAnthropicImages still accepts only NormalizeOptions, createAnthropicAdapter().buildRequest omits incoming.abortSignal, and native Claude normalization omits req.signal. When all four slots are occupied, disconnected Anthropic requests therefore remain in imageDecodeWaiters, retain their image payloads, and later consume decode capacity, allowing cancelled traffic to build a CPU/memory backlog that delays live requests. Extend the wrapper options and thread the request signal through these callers.

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

Useful? React with 👍 / 👎.

Comment on lines +7527 to +7531
initialRequest = await activeAdapter.buildRequest(parsed, {
headers: selectedForwardHeaders,
translatorBudget,
abortSignal: upstream.signal,
});

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 Include the abort signal in continuation rebuilds

This propagates cancellation for the initial build and the recovery rebuild, but fetchContinuation later calls activeAdapter.buildRequest without abortSignal around core.ts:8206. For an oversized OpenAI-chat image request that reaches terminal-guard continuation, disconnecting while normalization is queued will not remove that work from the new process-wide gate, so the request continues retaining and decoding images after cancellation. Pass upstream.signal to that continuation build as well.

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

Useful? React with 👍 / 👎.

@github-actions

Copy link
Copy Markdown

Deterministic PR hygiene checks passed.

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

github-actions Bot commented Sep 14, 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(adapters): bound inline image decoding [WRONG BRANCH] fix(adapters): bound inline image decoding Sep 14, 2026
@github-actions
github-actions Bot marked this pull request as draft September 14, 2026 01:40
@luvs01 luvs01 changed the title [WRONG BRANCH] fix(adapters): bound inline image decoding fix(adapters): bound inline image decoding Sep 16, 2026
@luvs01
luvs01 changed the base branch from main to dev September 16, 2026 10:02
@github-actions
github-actions Bot marked this pull request as ready for review September 16, 2026 11:06
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.

2 participants