fix(images): allow managed pool candidates to serve image generation with admission bearer auth

[rrmlima]

Summary

When clients (such as the standard OpenAI SDK, LangChain, or external agents) connect to OpenCodex using the conventional proxy admission bearer (Authorization: Bearer <proxy-secret>), /v1/images/generations previously flagged skipOpenAiForwardForAdmissionBearer = true and discarded all OpenAI forward candidates.

As a result:

• Calls failed to reach active, healthy managed OpenAI pool accounts stored locally (~/.codex/auth.json), even though pool accounts never forward the caller's bearer (they substitute it with proxy-managed OAuth tokens).
• Requests either silently fell back to Google Antigravity (gemini-3.1-flash-image) or failed with 400 invalid_request_error ("Built-in image generation needs an OpenAI upstream...").
• Callers were forced to use the nonstandard x-opencodex-api-key header to bypass this guard.

This PR fixes the routing by making candidate eligibility granular:

1. When caller admission uses a proxy bearer secret (callerBearerMayBeForwarded === false), only candidates in direct mode (which would attempt to forward caller credentials) are skipped. Candidates in managed pool modes remain eligible.
2. Injects a post-assembly validation asserting that assembled outbound headers never carry a proxy admission secret before reaching fetch(), wrapped in try/catch to safely release probe leases and return a sanitized 500 error on unexpected validation failure.
3. Adds automated regressions in tests/server/server-images.test.ts verifying:
   • A caller with a proxy admission bearer successfully routes to OpenAI in pool mode, replacing the bearer with the managed pool token and never leaking the proxy secret.
   • A caller with a proxy admission bearer encountering a pool auth failure surfaces the honest 401 error rather than masking it behind a separately billed keyed provider.

Fixes #4213 (partially: built-in image generation routing through proxy).

Verification

bun test tests/server/server-images.test.ts tests/server/api-key-scope-images.test.ts
# 91 pass, 0 fail, 333 expect() calls

bun run typecheck
# Strict TypeScript validation passed

bun run privacy:scan
# Privacy scan passed

bun run structure:check
# structure/ SSOT checks passed

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed.
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

• Required local validation passed; commands, results, and any full-suite exception are documented.

• I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).

• I resolved all correct Codex and CodeRabbit findings.

• My PR is ready for review.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 0d7f3259-3813-447e-847c-ea82ed5e003e

📥 Commits

Reviewing files that changed from the base of the PR and between f32f9aa and 7c1e862.

📒 Files selected for processing (2)

• src/server/images.ts
• tests/server/server-images.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

---

📝 Walkthrough

Walkthrough

Image forwarding now retains eligible OpenAI pool candidates when the proxy admission credential cannot be forwarded. The handler resolves credentials from those candidates and validates completed headers before forwarding. Tests cover pool-account forwarding and missing pool credentials.

Changes

Image pool forwarding

Layer / File(s)	Summary
Candidate selection and pool forwarding src/server/images.ts, tests/server/server-images.test.ts	The handler filters direct OpenAI candidates when the admission credential cannot be forwarded and passes eligible candidates to credential resolution. Tests check that pool credentials and the account ID are used without forwarding the proxy secret, and that missing pool credentials return 401 without falling back to a keyed provider.
Forward header validation src/server/images.ts	The handler validates assembled headers before forwarding. If validation fails, it releases the probe lease and returns a 500 response.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 7c1e8

Managed pool image forwarding has no identified issue that should block merging after normal checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 7c1e8

Bearer-authenticated image requests can now use managed pool accounts without forwarding the caller’s secret. An interrupted request may, however, leave a pool recovery reservation held and delay service for other callers.

Retained concerns

• Medium · security · inferred: Newly eligible proxy-bearer pool requests can be cancelled after acquiring a recovery probe lease. The cancellation path neither records an outcome nor releases the lease, so subsequent recovery probes for that account can be blocked until the cooldown state changes. The cleanup gap predates this PR, but this change expands its reachability.

Security review details

Security Blast Radius

• inferred — The expanded authority is available to callers already admitted to the public image routes and affects configured managed OpenAI pool accounts; it does not itself grant unauthenticated access.

Security Findings and Attack Paths

• inferred — A bearer-authenticated caller can cancel a newly reachable pool image request while it owns a recovery probe. The cancellation return leaves the lease unsettled, potentially delaying recovery traffic for other callers sharing that account.

Trust Boundaries and Controls

• observed — Ingress authentication and destination-scope checks precede forwarding. Managed pool credentials replace the admission bearer, and the completed forward headers receive an additional admission-secret check.

Resilience and Maintainability Implications

• observed — The handler releases a probe lease on header-validation failure and records normal upstream outcomes, but its client-cancellation branch only returns an error and performs transport cleanup.

Hardening Proposals

• proposed — Ensure cancellation settles or releases an owned pool recovery lease without misclassifying a client abort as an upstream failure, and cover interruption after lease acquisition in a regression test.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	For the image-generation portion of [#4213], src/server/images.ts now excludes only direct OpenAI forward candidates when the request uses the proxy admission bearer. Managed pool candidates rem…
Out of Scope Changes check	✅ Passed	The reviewed changes are limited to src/server/images.ts and tests/server/server-images.test.ts. The source changes implement admission-bearer image routing, outbound credential validation, lease …
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: allowing managed pool candidates to serve image generation requests authenticated with the proxy admission bearer.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ Deterministic PR hygiene checks passed.

[github-actions]

⏳ DRAFT

• review readiness checklist open (0/4 boxes ticked).

What to do

• Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

• ⬜ Required local validation passed; commands, results, and any full-suite exception are documented.
• ⬜ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
• ⬜ I resolved all correct Codex and CodeRabbit findings.
• ⬜ My PR is ready for review.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

[lidge-jun]

리뷰 · 우선순위 64 / 80

그림 그리기 요청이 프록시 입장 비밀번호만 들고 오면, 저장해 둔 ChatGPT 계정을 쓰게 고칩니다. 바탕은 dev입니다.

/v1/images/generations는 Authorization: Bearer에 이 프록시의 입장 비밀번호가 있으면, OpenAI로 넘길 후보를 전부 버렸습니다. 그 비밀번호가 바깥으로 나가면 안 됩니다. 모아 둔 계정은 그 비밀번호를 그대로 보내지 않습니다. 이 프로그램이 저장해 둔 로그인 토큰으로 바꿉니다. 후보를 전부 버려서, 쓸 수 있는 계정이 있어도 그림이 안 나가거나 구글 쪽으로 넘어갔습니다. 보통 클라이언트는 x-opencodex-api-key라는 별도 헤더를 써야 했습니다.

이제는 손님이 준 값을 그대로 올릴 수 있는 direct 후보만 뺍니다. 모아 둔 pool 후보는 남습니다. 헤더를 다 만든 뒤에 입장 비밀번호가 Authorization에 남아 있으면, 보내기 전에 막습니다. 이슈 #4213에서 그림 생성만 해당합니다. types.ts와 config.ts를 나누는 일과는 겹치지 않습니다. 같은 주제로 열린 다른 PR은 없습니다.

라인 - src/server/images.ts:749 validateForwardAdmissionCredential. 이 호출은 811줄 try 밖에 있습니다. 예외가 나면 이 함수 밖으로 나갑니다. 손님에게 정리된 오류 응답이 가지 않습니다. 풀 계정을 고르며 잡아 둔 잠금은 741줄 거절에서만 releaseProbeLease로 돌려줍니다. 이 예외 길에는 돌려주는 코드가 없습니다.

라인 - tests/server/server-images.test.ts:1734. 1650줄에 같은 경우가 이미 있습니다. direct만 있고 API 키가 있으면, 입장 비밀번호를 올리지 않고 그 키로 그림을 그립니다. 새 테스트는 그 경우를 한 번 더 확인합니다. 풀이 식었을 때 API 키로 가는지, 풀 오류를 그대로 돌려주는지는 없습니다.

메인테이너의 판단이 필요한 지점

입장 비밀번호로 온 그림 요청은 이제 풀을 먼저 탑니다. 풀이 되면 저장 토큰으로 바꿉니다. 새 테스트가 그 성공을 확인합니다. 풀이 식었거나 로그인이 깨지면, 지금 코드는 API 키가 있어도 그 오류를 그대로 돌려줍니다. 어제는 이 손님이 풀을 타지 않아서 API 키로 그림이 나갔습니다. 696줄 주석은 풀이 식어도 API 키가 놀면 안 된다고 적습니다. 750줄은 풀 오류를 그대로 돌려줍니다. 이 손님에게 어느 쪽을 쓸지 정해 주세요.

이 PR은 아직 초안입니다. 본문의 준비 체크 네 칸이 비어 있습니다.

너의 추천

풀로 그림을 보내는 수정은 두세요. 749줄 검사는 try 안에 넣으세요. 예외가 나면 잠금을 돌려주고, 비밀이 나가지 않았다는 오류 응답으로 끝내면 됩니다. 1734줄 테스트는 1650줄과 겹칩니다. 그 자리에 풀이 식은 계정과 API 키가 같이 있을 때의 응답을 하나 두면 됩니다. 다른 열린 PR은 닫지 마세요. 초안 체크를 채운 뒤에 머지하면 됩니다.

이 댓글은 grok-bot이 작성했습니다

[lidge-jun]

Thank you, @rrmlima. Your fix landed on dev through #6097 (squash commit 468b954), and the commit credits you with a Co-authored-by trailer.

It was carried rather than merged directly because dev had moved and the target test file had reached its size cap. The core of your change is intact: a proxy admission bearer can now use a managed Pool account for Images, while Direct still never forwards it. Review added two things on top: a scoped key's provider/model scope is now checked before any Pool credential is resolved, and the recovery probe lease is released when the client cancels mid-send. Closing this in favour of #6097.