fix(config): validate provider send paths before management writes

[luvs01]

Current author follow-up

Published head c8f7030ff114b98753ecd7d4a206a590b54c65d8 is based on the fixed dev snapshot 43f4450a538d729f353144c029aa97de1c4f2483. This supersedes older head/behind/CI status statements below; older verification remains historical evidence. Required conflicts were resolved without dropping upstream contracts. No repeated tip chasing was performed.

• bun run typecheck: passed.
• bun run structure:check: passed.
• bun run privacy:scan: passed.
• git diff --check: passed.
• bun test tests/server/management-provider-validation.test.ts: passed (137 pass; 0 fail; 1095 expect() calls).

Exact-head hosted verification is not yet complete; no older-head matrix is substituted.

Maintainer sponsorship, where required, remains a separate hold. Existing resolved review findings were not reopened.

Summary

Provider registration and replacement could accept a malformed responsesPath or chatCompletionsPath even though loading the same configuration from disk rejected it. Export the existing shared relative-path validator with an unknown-input check and call it at the management write boundary before DNS validation, state adoption or persistence.

Editor PATCH operations that revalidate the resulting provider also check its retained paths; existing pacing-only and other validation bypasses are unchanged. This does not add a new PATCH field or change the existing path rules. All mapped structure owners link to the contract, and the eight provider-reference translations describe the write-time validation.

Verification

• Before the fix, invalid create/replace POST cases returned 200 rather than 400. New tests exercise the actual management handler and compare live configuration and file bytes, with no DNS or catalog-refresh call on rejection. A valid-path persistence/reload control also passes.
• New focused group: 9 pass, 0 fail, 139 assertions. Complete tests/server/management-provider-validation.test.ts: 137 pass, 0 fail, 1095 assertions.
• bun run typecheck, bun run structure:check, bun run privacy:scan, and git diff --check: passed.
• Documentation: frozen-lockfile install unchanged; 441-page build passed, with the new validation paragraph checked in all eight rendered provider references. Generated output was removed afterward.
• Full cross-platform CI has not been verified for this new head. Local checks above do not claim an observed credential leak or a full-suite pass.

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 evidence

Rebased onto dev at 15fbd49f3 and pushed; the branch was 0 commits behind at push time. Published head is 3dc6d474d.

Local verification on that head: bun run typecheck, bun run structure:check, bun run privacy:scan and git diff --check all pass. bun test tests/server/management-provider-validation.test.ts reports 137 pass / 0 fail.

Hosted cross-platform CI has not been dispatched on this head. On current dev the windows shard fails independently of this pull request: tests/clients/desktop-app-restart-posix.test.ts fails 9 of 17 on Windows because isUnderRoot builds its boundary from path.sep alone and because the darwin/linux currentUid() helpers call process.getuid(), which does not exist on Windows. That is a dev regression, not a defect of this change, and it is fixed separately in #4564. The first box is ticked on the local run recorded above, which is what its wording asks for.

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.

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

---

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 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]

⏳ DRAFT

• hygiene: unsponsored_surface.

What to do

• Fix 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.
• Tick all four boxes in the PR description once you're done (currently 2/4).

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.

2/4 boxes ticked.

This pull request was already a draft. Its draft status will be preserved after every issue above is resolved.
@luvs01 Tick the boxes once your local CI is green, your branch is on the latest dev commit, and every correct Codex and CodeRabbit finding is resolved.

[lidge-jun]

리뷰 · 우선순위 71 / 80

이 PR은 지금 dev(HEAD df7dc1be5, package 2.54.0)에서 관리 API로 provider를 만들거나 바꿀 때 responsesPath / chatCompletionsPath가 깨진 값이어도 통과하던 구멍을 막습니다. 디스크에서 config를 읽을 때는 이미 src/config.ts의 providerRelativeSendPathConfigError가 상대 경로만 허용하는데, POST /api/providers 쪽 관리 write 경계에서는 그 검사가 빠져 있어서 같은 값이 메모리·파일에 먼저 들어간 뒤 DNS/카탈로그 refresh까지 갈 수 있었습니다. 보고대로면 예전에는 잘못된 create/replace가 200을 돌려줬습니다.

고치는 방식은 새 규칙을 만들지 않고 이미 있는 검사기를 공유하는 쪽입니다. providerRelativeSendPathConfigError를 export하고, 입력이 문자열이 아니면 ``field must be a string으로 거절하게 넓힌 뒤, `src/server/auth-cors.ts`의 `providerManagementConfigError`가 destination 검사 직후·headers 검사 전에 두 필드를 돌립니다. Editor PATCH가 결과 provider를 다시 검증할 때도 남아 있는 잘못된 경로를 잡습니다. pacing-only 같은 기존 bypass는 그대로 두고, send-path를 바꾸는 새 PATCH 필드는 추가하지 않습니다.

테스트가 이 PR의 핵심입니다. tests/server/management-provider-validation.test.ts에 management write 경계 그룹을 넣어서 잘못된 값이 메모리/디스크를 바꾸기 전에 400이 나고, DNS·catalog refresh가 호출되지 않는지, 올바른 /custom/... 경로는 persist/reload 되는지까지 실제 handleManagementAPI로 확인합니다. 문서도 8개 언어 providers 참고서와 structure/config.md의 Provider relative send paths 절, 여러 structure 맵 링크를 같이 맞췄습니다. types.ts/config.ts 분할 캠페인과 충돌하지 않는 독립 수정입니다.

지금 dev 방향(웹검색 패스스루 브릿지, desktop restart, reasoning ladder)과는 축이 다르지만, 관리 API로 잘못된 send path가 살아 남는 건 운영·보안 경계 문제라 우선순위는 높습니다. 구조 맵에 같은 한 줄을 여러 파일 맨 위에 반복한 부분은 약간 시끄럽지만, 계약 추적용으로는 이해됩니다.

라인 - src/config.ts providerRelativeSendPathConfigError - 예전 시그니처는 string | undefined만 받았고 관리 JSON의 number/null/object는 타입 밖으로 새어 들어갈 수 있었다. 이번 export + unknown 가드가 그 구멍을 막는다.
라인 - src/server/auth-cors.ts providerManagementConfigError - destination 검사 뒤에 send-path를 넣었다. 순서는 맞지만, destination DNS가 비싸다면 send-path를 더 앞에 두는 편이 실패 빠르게 끝날 수 있다(지금은 테스트가 DNS mock을 안 부르게 막아 둔 상태).
경로/심볼 - PATCH + 이미 깨진 live row - 본문이 말한 대로 send-path setter는 없고, 다른 필드 PATCH 때 retained path를 재검증해 400으로 막는다. 레거시 깨진 값을 GUI에서 고치는 UX는 이 PR 범위 밖이다.
경로/심볼 - structure/*.md 상단 반복 링크 - catalog/images/runtime 등 관련 약한 맵까지 같은 문장이 들어갔다. 계약 추적은 되지만 노이즈가 크다.
경로/심볼 - CI - 로컬 typecheck/structure/privacy와 focused 9+137 테스트는 통과했다고 했으나 full cross-platform CI는 이 head에서 아직이다.

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

• 이미 디스크에 남아 있는 잘못된 send-path row를 마이그레이션/경고로 드러낼지, PATCH 거절만으로 충분할지
• structure 맵 전면 링크를 유지할지, structure/config.md + gui-and-management-api만 남길지
• send-path 검사를 destination보다 앞으로 당겨 실패 비용을 줄일지

너의 추천
tip CI 초록 확인 후 merge 후보로 본다. 범위가 작고 회귀 테스트가 실제 management handler를 치므로 독립 랜딩이 맞다. structure 노이즈는 후속 정리로 미뤄도 된다.

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

[luvs01]

Author follow-up on the Cross-platform CI failure for this head.

The failing shard is Windows 5/6, and all nine failures come from tests/clients/desktop-app-restart-posix.test.ts. None of them touch this PR: the change here is confined to provider send-path validation, its management-boundary test, and documentation. The Ubuntu and macOS shards passed.

The failure reproduces on a Windows host at current dev with no local changes, so it is a pre-existing condition on the branch rather than a regression introduced here. Two independent causes: isUnderRoot derives its boundary from path.sep alone, so a member path written with forward slashes reads as outside the install root on Windows; and the macOS and Linux adapters scope enumeration through process.getuid(), which a Windows host does not provide, so the probe reports that it could not run.

I opened a separate fix for that so it is not mixed into this PR. Once it lands, this head should be re-run before any readiness claim. This PR is also beyond the latest-dev tolerance, so it stays Draft until a coordinated rebase.

[luvs01]

Rebased onto latest dev and updated the readiness checklist. Two follow-ups on the gate state.

Hygiene gate needs a maintainer decision, not an author change. PR hygiene fails with unsponsored_surface because this PR touches src/server/auth-cors.ts, which .github/scripts/pr-sponsored-surface.cjs lists as a restricted authentication surface. The touch there is three lines: it imports providerRelativeSendPathConfigError and calls it for responsesPath and chatCompletionsPath inside the existing providerManagementConfigError validation chain, immediately after the destination check. No authentication, credential, CORS, or admission logic is read or modified, and the provider name is passed through the existing redactSecretString before it reaches a message. The validator itself lives in src/config.ts.

The reason it has to be called from that file is the point of the change: providerManagementConfigError is the single boundary the management write path runs, so a send-path override that src/config.ts would reject on load could previously be stored through the management API and only fail later at send time. Moving the call elsewhere would reintroduce that gap.

Could a maintainer review the auth-surface touch and apply maintainer-sponsored if it looks right? I cannot apply that label as the author, and it is the only thing keeping this PR in Draft.

Superseding my earlier CI note. The comment above about the Windows shard and the latest-dev tolerance is resolved. This head is rebased onto dev at 15fbd49f3 with no conflicts, and the Windows failures I reported there were the dev regression in tests/clients/desktop-app-restart-posix.test.ts, fixed separately in #4564.

Local verification on the published head 3dc6d474d: bun run typecheck, bun run structure:check, bun run privacy:scan and git diff --check pass; bun test tests/server/management-provider-validation.test.ts reports 137 pass / 0 fail; the docs-site build completes 441 pages and the generated docs-site/dist was removed afterward.