fix(openai): exclude user-owned alias overlays from canonical seed validation

[Flowershangfromthebranches]

Summary

Canonical OpenAI provider validation compares the persisted provider row against the built-in transport seed.

OpenCodex's dedicated alias APIs can legally persist:

• defaultAliases
• alias
• modelAliases

After one of those overlays exists, an unrelated general provider edit such as a context-window PATCH could fail with:

provider openai must equal the canonical built-in provider seed

This validation deadlock is real, but the general provider write path must not become an unvalidated alias writer in order to resolve it.

The revised fix keeps alias ownership at the dedicated APIs:

• A full provider POST may omit alias fields or round-trip values exactly equal to the stored provider. The latest stored alias overlays are restored before persistence.
• A full provider POST cannot create, clear, or change alias overlays.
• General provider PATCH rejects alias, modelAliases, and defaultAliases; callers must use the dedicated alias endpoints.
• Canonical OpenAI transport validation runs against a candidate from which only already-owned stored overlays were removed.

Safety

The canonical transport guard remains unchanged and continues rejecting changes to:

• baseUrl
• adapter
• authMode
• other canonical transport identity

The dedicated alias APIs remain the only writers that apply provider/combo/account collision checks, model-alias syntax and uniqueness rules, native-family reservations, and boolean shape validation.

Unrelated non-OpenAI provider edits retain their previous behavior and preserve stored aliases.

Regression coverage

Tests cover:

• full provider edits preserving existing provider, model, and default aliases;
• provider POST/PATCH being unable to introduce a provider alias collision;
• reserved, duplicate, and invalid model aliases remaining rejected;
• non-boolean defaultAliases and malformed alias maps returning bounded 4xx without persistence;
• canonical OpenAI with stored aliases successfully PATCHing modelContextWindows;
• baseUrl, adapter, and authMode tampering remaining rejected;
• unrelated non-OpenAI edits remaining unchanged;
• the dedicated alias API's case-insensitive collision rules;
• destination-policy test spies being restored so later tests cannot receive a false green result.

Verification

Exact publication head: dd60f05d73599991b12e11298f741d34ed84ccbf
Exact upstream baseline: 15b0f701eb69d37382880124be9010e412c20411
Environment: Bun 1.4.0, Node v23.11.0, identical local DNS/proxy/env.

• bun test tests/management-provider-validation.test.ts --test-name-pattern "full provider edit preserves aliases|general provider writes cannot introduce|general provider writes reject reserved|malformed alias overlays|canonical transport tampering|unrelated non-openai provider edits|canonical OpenAI with defaultAliases can still PATCH modelContextWindows" — 7 passed, 0 failed
• bun test tests/alias-management-api.test.ts tests/provider-model-aliases.test.ts — 9 passed, 0 failed
• bun test tests/routing-profile.test.ts --test-name-pattern "alias collision validation" — 1 passed, 0 failed
• bun run typecheck — passed
• bun run privacy:scan — passed
• git diff --check upstream/dev...HEAD — passed

Full focused diagnostic under the same environment:

Tree	Result
PR head dd60f05d7	73 pass / 18 fail
exact upstream/dev 15b0f701e	66 pass / 18 fail

The PR adds seven passing regression tests. The 18 failures have the same test names and semantically identical destination-policy errors on both trees; there are no branch-only failures.

Identical baseline/environment failure set

• provider PATCH sets, clears, and rejects annotateEmptyToolOutputs
• provider POST overwrite preserves modelCosts when the payload omits it
• provider POST overwrite preserves the account-failover opt-out when the payload omits it (#2568d)
• provider POST overwrite preserves an explicit annotateEmptyToolOutputs: false
• provider POST overwrite preserves hand-edited context windows (#1409) > an omitted modelContextWindows keeps the user's map, without registry seed keys
• provider POST overwrite preserves hand-edited context windows (#1409) > a submitted modelContextWindows updates that key and keeps the others
• provider POST overwrite preserves hand-edited context windows (#1409) > an omitted contextWindow keeps the user's scalar
• provider POST overwrite preserves hand-edited context windows (#1409) > a submitted contextWindow still wins
• provider POST overwrite preserves hand-edited context windows (#1409) > a brand-new provider still receives the registry seed
• provider POST overwrite preserves hand-edited context windows (#1409) > PATCH can still delete a key with an explicit null
• provider management rejects runtime metadata and accepts only canonical OpenAI option seeds
• provider management does not persist registry-only static auth headers for opencode-free
• provider PATCH can enable allowPrivateNetwork and then change baseUrl to localhost
• provider PATCH persists liveModels and provider metadata exposes the normalized state
• provider PATCH persists and clears structured-output model opt-outs
• provider management rejects POST setDefault for a disabled provider
• provider PATCH field-mask edits non-reserved providers and rejects unsafe fields (WP040)
• provider PATCH manages custom headers with merge and clear semantics

All failures originate from this host's Clash/Mihomo fake-IP DNS mapping .example / .example.test fixtures into 198.18.0.0/15, after which the existing destination policy correctly returns 400 instead of the tests' expected 200. Evidence from the same shell:

relay.example       198.18.0.235
api.example.test    198.18.0.236
extra2.example.test 198.18.0.240
agw.example.test    198.18.0.241

This is an exact upstream baseline condition, not a PR regression.

Review readiness checklist

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

• All CI tests are green on my local testing.

• I pushed my PR to the latest dev commit.

• I resolved all correct Codex and CodeRabbit findings.

• My PR is ready for review.

Summary by CodeRabbit

• Bug Fixes

  • Preserved configured provider aliases during provider updates and reloads.
  • Prevented unauthorized or malformed alias changes from being saved.
  • Improved validation for provider submissions, including invalid request bodies and alias conflicts.
  • Protected canonical provider connection settings while allowing supported model configuration updates.
  • Ensured failed provider changes return an error without partially saving data.

• Tests

  • Added coverage for alias preservation, validation failures, collision handling, and model context-window updates.

[github-actions]

⚠️ Deterministic hygiene checks failed.

• unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: src/server/auth-cors.ts.

[github-actions]

✅ READY

• all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

• ✅ All CI tests are green on my local testing.
• ✅ I pushed my PR to the latest dev commit.
• ✅ I resolved all correct Codex and CodeRabbit findings.
• ✅ My PR is ready for review.

✅ 4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers: @lidge-jun @Ingwannu

Hygiene

✅ Deterministic PR hygiene checks passed.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 501b94ec-f30e-416f-9dde-74e6b98a35b7

📥 Commits

Reviewing files that changed from the base of the PR and between 15b0f70 and dd60f05.

📒 Files selected for processing (2)

• src/server/management/provider-routes.ts
• tests/management-provider-validation.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

Provider management routes now treat alias, modelAliases, and defaultAliases as dedicated overlay fields. POST preserves persisted overlays, PATCH rejects direct overlay updates, reload and PATCH validation omit overlays, and tests cover ownership and persistence behavior.

Changes

Provider alias ownership

Layer / File(s)	Summary
Alias ownership and transport validation src/server/management/provider-routes.ts:108-164, src/server/management/provider-routes.ts:561-564	The route module defines alias ownership checks, removes overlay fields from transport validation candidates, restores persisted overlays, and applies the candidate during local provider reload.
POST and PATCH route integration src/server/management/provider-routes.ts:624-634, src/server/management/provider-routes.ts:723-726, src/server/management/provider-routes.ts:748-749, src/server/management/provider-routes.ts:819-822, src/server/management/provider-routes.ts:862-865	POST requires a plain provider object, rejects unowned overlays, validates transport fields without overlays, and restores persisted values after DNS validation. PATCH rejects alias overlay fields and excludes them from both validation paths.
Provider management validation coverage tests/management-provider-validation.test.ts:917-1181	Tests cover overlay preservation, collisions, malformed aliases, canonical OpenAI protection, unrelated provider edits, and model context-window patches. Failed writes are checked for absent memory and disk changes.

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

Merge Risk: 🔵 Low · up to dd60f

The PR separates alias updates from general provider edits while preserving stored aliases and transport validation. It is mergeable with owner awareness that persistence and recovery ordering for alias restoration is not fully evidenced and could affect alias retention during failure or recovery paths.

Suggested reviewers: lidge-jun

Sequence Diagram(s)

sequenceDiagram
  participant ProviderClient
  participant ProviderRoutes
  participant AliasOwnershipCheck
  participant TransportValidation
  participant DNSValidation
  participant PersistedProvider
  ProviderClient->>ProviderRoutes: POST /api/providers
  ProviderRoutes->>AliasOwnershipCheck: Check submitted alias overlays
  ProviderRoutes->>TransportValidation: Validate provider without overlays
  ProviderRoutes->>DNSValidation: Await DNS validation
  DNSValidation-->>ProviderRoutes: Return validation result
  ProviderRoutes->>PersistedProvider: Restore authoritative alias overlays

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: excluding user-owned alias overlays from canonical OpenAI provider validation.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[Ingwannu]

Requesting changes on exact head d72a3213dc10c7fc7e3f85a9250eed34e0d89621.

The reported validation deadlock is real: alias overlays already written by the dedicated alias APIs must not make an otherwise canonical OpenAI row fail transport-seed validation. Deleting the three fields from the comparison candidate is not sufficient at the management boundary, because it also makes the general provider write path an unvalidated alias writer.

PUT /api/providers/{name}/alias, /model-aliases, and /api/default-aliases enforce provider/combo/account namespace collisions, model-alias syntax and uniqueness, native OpenAI-family reservations, and boolean shape. A full /api/providers provider object can now carry arbitrary alias, modelAliases, or defaultAliases; once these fields are discarded before the seed comparison, those dedicated checks are bypassed. The current tests cover transport tampering but not alias tampering or collisions.

Keep alias ownership at the dedicated endpoints. Ordinary provider PATCH/POST should preserve already-persisted alias overlays while validating the transport object, but must not create or alter those overlays without running the same collision/shape rules. Add regressions for:

• a full provider edit preserving existing aliases;
• a general write being unable to introduce a provider alias collision;
• reserved/duplicate/invalid model aliases remaining rejected;
• non-boolean defaultAliases and malformed alias maps returning a bounded 4xx rather than reaching config persistence.

This remains Draft with hygiene/target red and no hosted CI. Resolve the ownership boundary and focused tests before requesting workflow approval or Ready review.

[Flowershangfromthebranches]

Thanks for the ownership-boundary review. I reworked the fix on head f34aaa47354a1e701a06a105bd543399dee7dd97:

• restored strict canonical seed validation instead of unconditionally excluding incoming alias fields;
• full provider POST now accepts alias overlays only when they exactly match the stored provider, and restores the latest stored values before persistence;
• general provider PATCH rejects alias, modelAliases, and defaultAliases, keeping all mutations in the dedicated alias APIs;
• canonical transport validation still rejects baseUrl, adapter, and authMode tampering.

Added the requested regressions for full-edit preservation, provider-alias collision attempts, reserved/duplicate/invalid model aliases, malformed/non-boolean payloads with no persistence, the original modelContextWindows deadlock, transport tampering, and non-OpenAI preservation.

Tests run:

• bun test tests/management-provider-validation.test.ts --test-name-pattern "full provider edit preserves aliases|general provider writes cannot introduce|general provider writes reject reserved|malformed alias overlays|canonical transport tampering|unrelated non-openai provider edits|canonical OpenAI with defaultAliases can still PATCH modelContextWindows" — 7 passed, 0 failed
• bun test tests/alias-management-api.test.ts — 2 passed, 0 failed
• bun run typecheck — passed
• bun run privacy:scan — passed
• git diff --check upstream/dev...HEAD — passed

I also ran the full management-provider file: 88 passed and 5 unrelated destination-policy cases failed because this host's fake-IP DNS resolves .example fixtures into 198.18.0.0/15. The PR remains Draft.

[Ingwannu]

Incremental review completed on exact head f34aaa4. The previous ownership bypass is addressed in the current code: full provider POST can only omit alias overlays or round-trip values exactly equal to the stored row, general PATCH rejects all three alias-owned fields, canonical transport validation receives a candidate stripped only after ownership admission, and the latest live alias values are restored synchronously after the DNS await before persistence. The requested collision, reservation, malformed-shape, no-persistence, transport-tampering, and non-OpenAI regressions are present. I did not find a remaining static ownership/transport blocker in this revision.

I am not applying maintainer-sponsored or approving this draft yet. The PR body still has all four readiness boxes unchecked, reports five failures in the full management-provider diagnostic, and the branch is now four commits behind current dev. Please rebase onto current dev, resolve or baseline-classify the full focused-suite failures on that head, tick the checklist only after the stated validation is current, and request security review again. At that point the exact-head workflows can run and sponsorship can be decided from green evidence.

[Flowershangfromthebranches]

@codex security review

@Ingwannu — requesting the exact-head maintainer security review again on dd60f05d73599991b12e11298f741d34ed84ccbf, rebased onto current dev 15b0f701eb69d37382880124be9010e412c20411.

The ownership architecture from your incremental review is unchanged. The only follow-up code change restores the destination-policy spy in the canonical alias regression so it cannot mask later tests.

Under the same Bun 1.4.0 / Node v23.11.0 / DNS environment:

• branch full management-provider file: 73 pass / 18 fail;
• exact upstream/dev baseline: 66 pass / 18 fail;
• the 18 failure names and 400-vs-200 destination-policy errors are identical;
• branch-only failures: none.

dig confirms the affected .example fixtures resolve into Clash/Mihomo's 198.18.0.0/15 fake-IP range. All 17 focused alias/ownership/routing tests, typecheck, privacy scan, and diff check pass on the published head. The PR body is being updated with the exact comparison and readiness evidence.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: dd60f05d73

ℹ️ 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".

[Ingwannu]

The exact-head macOS failure is now traced to the current dev fixture rather than this PR's alias changes.

Job 99778561688 failed at tests/server-auth.test.ts:2302: the first WebSocket turn observed a refreshed credential instead of the expected old credential. #3121 does not touch that file. The follow-up merged in #3139 moved the fake clock before startServer, but still called updateAccountQuota under the real clock; the startup prime can therefore compare a future fake time with a real-time updatedAt, classify the fresh quota row as stale, and rotate before the first turn.

I opened #3143 with the minimal fixture correction. On exact head 1e50927f5, the two affected cases pass five consecutive runs under Bun 1.4.0 and typecheck is green. I am keeping #3121 unapproved until that upstream CI fix lands, this branch rebases onto it, and exact-head CI is fully green; no new blocker was found in the alias ownership implementation itself.

[lidge-jun]

리뷰 · 우선순위 62 / 80

설명

이 PR은 canonical OpenAI provider 검증이 사용자 alias 오버레이(defaultAliases/alias/modelAliases) 때문에 막히던 교착을 풉니다. 전용 alias API가 합법적으로 저장한 뒤, 무관한 context-window PATCH 같은 일반 provider 편집이 provider openai must equal the canonical built-in provider seed 로 실패했습니다. 수정은 일반 provider 쓰기 경로를 열린 alias writer로 만들지 않습니다. POST는 alias를 생략하거나 저장된 값과 정확히 같을 때만 round-trip하고, 최신 저장 오버레이를 복원한 뒤 persist합니다. PATCH는 세 키를 거절하고 전용 alias 엔드포인트로 보냅니다. canonical transport 검증은 이미 소유한 저장 오버레이만 제거한 candidate로 돌립니다.

baseUrl/adapter/authMode 등 transport identity 가드는 그대로입니다. 전용 alias API만 collision·문법·native-family·boolean shape를 적용합니다. 파일은 src/server/management/provider-routes.ts 와 tests/management-provider-validation.test.ts, +349/−8, MERGEABLE 입니다. types/config 분할과 직접 충돌하는 대규모 재작성은 아닙니다. management validation 수리입니다. 점수는 62입니다.

라인 PROVIDER_ALIAS_OVERLAY_FIELDS - alias/modelAliases/defaultAliases 소유권을 한상수로 모읍니다. PATCH 거절과 POST round-trip이 같은 목록을 쓰는지 보세요
경로 providerAliasOverlayOwnershipError - POST가 오버레이를 만들/지우/바꾸지 못하게 합니다. 테스트 271줄 추가가 그 표를 잠급니다
경로 canonical seed 검증 - 오버레이만 벗긴 candidate로 비교합니다. transport 필드 변경은 계속 거절되어야 합니다
경로 비-OpenAI provider - 본문은 무관 제공자에 영향을 줄이지 않는다고 합니다. 테스트에 한 케이스 있으면 좋습니다

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

• GUI가 alias 필드를 일반 provider PATCH에 넣어 보내는지. 보내면 새 400을 보게 됩니다(의도)
• 문서에 "alias는 전용 API만" 한 줄이 필요한지

너의 추천
머지 후보입니다. management 교착을 풀고 transport 가드는 유지합니다. 라벨은 바꾸지 않습니다.

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

[Ingwannu]

The new Grok summary does not supersede the exact-head gate. Current head dd60f05d73599991b12e11298f741d34ed84ccbf still has a failed macOS job, so it is not merge-ready yet. That failure is the already identified fake-clock/startup-prime fixture race at tests/server-auth.test.ts:2302; the owner replacement fix landed later in #3147, after this PR's CI run.\n\nPlease rebase this branch onto current dev e40245e4cbb1e1e70322170369ac528a610dbe76 (the head is currently 98 dev commit(s) behind) and let the exact-head matrix rerun. The only branch-local change since my last implementation review is the destination-policy spy restoration, which is correct and prevents the canonical alias test from contaminating later cases. I still see no new alias-ownership blocker, but successful current-head CI remains required before approval.

[lidge-jun]

Landed via maintainer rebase #3189. All three commits cherry-picked onto current dev with author credit preserved, no conflicts. management-provider-validation reports 91 pass, 0 fail on the rebase head, most of it your own new coverage.

Counting user-owned alias overlays as canonical seed entries was a subtle one — the symptom (unrelated provider writes start failing) points nowhere near the cause. Thanks for the fix and for the 265 lines of coverage that come with it.