fix(oauth): restrict xAI discovery endpoint host and reject userinfo - #4065
fix(oauth): restrict xAI discovery endpoint host and reject userinfo#4065chilung-cgu wants to merge 3 commits into
Conversation
github-actions
Bot
added
the
intake: hygiene-blocked
|
1a12f25 to
638b396
Compare
⏳ DRAFT
What to do
Review readiness checklist
✅ 4/4 boxes ticked. This pull request was already a draft. Its draft status will be preserved after every issue above is resolved. |
리뷰 · 우선순위 47 / 80이 PR은 이슈 #4048을 닫는 xAI OAuth discovery 엔드포인트 검증 하드닝이다. 작성자는 chilung-cgu이고, 베이스는 왜 지금 이 PR이 하는 일은 딱 그 제안이다. 다만 머지 전에 게이트가 막고 있다. hygiene가 unsponsored_surface로 실패했고( 라인 47–54 (현 HEAD validateXaiEndpoint) - 이 PR이 정확히 고치는 느슨한 suffix/ 경로 TRUSTED_XAI_AUTH_HOSTS - 경로 tests/.../xai-endpoint-validation.test.ts - 단위·discovery 통합 가짜 fetch까지 있어 회귀 방지가 충분하다. 경로 hygiene unsponsored_surface - 인증 표면이라 외부 기여자는 경로 #4060 형제 PR - 같은 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🟡 Changes recommended
It introduces a new test file that is not registered in the repo’s enforced test-layout fixtures and also risks credential leakage by including the raw (possibly userinfo-bearing) URL in the thrown error message.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR hardens xAI OAuth/OIDC discovery endpoint validation to reduce the risk of refresh tokens being sent to untrusted endpoints by pinning discovery-derived URLs to a small trusted host allow-list and rejecting URLs containing userinfo.
Changes:
- Restrict
validateXaiEndpointtohttps://auth.x.aiandhttps://accounts.x.aionly, and reject embedded userinfo. - Add a new test suite covering trusted host allow-listing, scheme/domain rejection, and userinfo rejection.
File summaries
| File | Description |
|---|---|
| src/oauth/xai.ts | Tightens discovery endpoint validation (trusted hosts + no userinfo). |
| tests/providers/xai/xai-endpoint-validation.test.ts | Adds regression tests for the hardened endpoint validation and discovery rejection behavior. |
Review details
Suppressed comments (1)
tests/providers/xai/xai-endpoint-validation.test.ts:13
- New .test.ts files must resolve to a domain in scripts/test-layout/layout.json and be reflected in tests/fixtures/test-layout-expected.json (see tests/test-layout.test.ts and tests/test-layout-tooling.test.ts). This file’s basename (xai-endpoint-validation.test.ts) isn’t present in either explicit mapping/fixture and doesn’t match the providers/xai regex (which is grok-), so the full test suite will fail with an unresolved test file.
describe("xAI endpoint validation (#4048)", () => {
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| parsed.password || | ||
| !TRUSTED_XAI_AUTH_HOSTS.has(host) | ||
| ) { | ||
| throw new Error(`xAI OAuth discovery returned an unexpected endpoint: ${rawUrl}`); |
| import { | ||
| discoverXaiOAuthEndpoints, | ||
| validateXaiEndpoint, | ||
| XAI_OAUTH_DISCOVERY_URL, | ||
| } from "../../../src/oauth/xai"; |
Pin xAI OAuth discovery endpoint validation to known trusted hosts (auth.x.ai, accounts.x.ai) and reject endpoints containing embedded userinfo. Closes lidge-jun#4048
…unused test import
8adb5fa to
c48b802
Compare
Summary
auth.x.ai,accounts.x.ai) instead of accepting any*.x.aisubdomain.xai-endpoint-validation.test.tsin test layout maps (scripts/test-layout/layout.jsonandtests/fixtures/test-layout-expected.json).Verification
/home/ubuntu/.bun/bin/bun test tests/providers/xai/xai-endpoint-validation.test.ts— 5 pass / 0 fail./home/ubuntu/.bun/bin/bun test tests/test-layout.test.ts— 2 pass / 0 fail./home/ubuntu/.bun/bin/bun test tests/test-layout-tooling.test.ts— 15 pass / 0 fail./home/ubuntu/.bun/bin/bun test tests/server/server-xai-oauth-401-replay.test.ts— 8 pass / 0 fail./home/ubuntu/.bun/bin/bun run typecheck— exit 0./home/ubuntu/.bun/bin/bun run privacy:scan— passed.git diff --check— clean.Checklist
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.