fix(kosong): forward full host headers only to first-party endpoints - #2581
fix(kosong): forward full host headers only to first-party endpoints#2581he-yufeng wants to merge 1 commit into
Conversation
resolveOutboundHeaders decided solely on the provider type, so a kimi-typed provider pointed at a third-party base_url received the complete X-Msh-* identity set, device id included. Gate the full forwarding on the resolved endpoint being a first-party Moonshot host; everything else gets the User-Agent only.
🦋 Changeset detectedLatest commit: 7e6a1c5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e6a1c5d6e
ℹ️ 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".
| return { ...parseKimiCodeCustomHeaders(), ...hostLayer, ...customHeaders }; | ||
| } | ||
|
|
||
| const FIRST_PARTY_HOSTS = new Set(['api.moonshot.ai', 'api.moonshot.cn']); |
There was a problem hiding this comment.
Include api.kimi.com here: the default managed endpoint is https://api.kimi.com/coding/v1 (packages/oauth/src/managed-usage.ts:29), and login provisions it as a type: 'kimi' provider (packages/oauth/src/managed-kimi-code.ts:590-594). With the current allowlist, every default managed/OAuth model request loses all X-Msh-* identity headers even though createKimiDefaultHeaders defines them for managed endpoints, so the primary logged-in flow no longer reports its platform and device identity.
Useful? React with 👍 / 👎.
| return true; | ||
| } | ||
| try { | ||
| return FIRST_PARTY_HOSTS.has(new URL(baseUrl).hostname); |
There was a problem hiding this comment.
Require url.protocol === 'https:' as well as an allowed hostname. With a configured URL such as http://api.moonshot.ai/v1, this hostname-only check forwards the device ID and the rest of the identity set over cleartext HTTP; Node's outbound client does not apply browser HSTS before the initial request, so an on-path party can observe the headers even if the server later redirects.
Useful? React with 👍 / 👎.
| const forwardsAll = | ||
| providerType !== undefined && | ||
| getProviderDefinition(providerType)?.hostHeaders === 'full'; | ||
| getProviderDefinition(providerType)?.hostHeaders === 'full' && | ||
| isFirstPartyBaseUrl(baseUrl); |
There was a problem hiding this comment.
Pass the endpoint decision through to attributeHeaders as well. For a type: 'kimi' provider on a foreign base URL, this new condition emits only User-Agent, but inspection.ts:479-495 still computes forwardsAll solely from the provider definition and labels that surviving header as coming from the full host-header set. Consequently catalog.inspect() reports provenance inconsistent with the exact resolution pass used by get().
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
| // A vendor's `hostHeaders: 'full'` contract is meant for the vendor's own | ||
| // endpoint. A provider that speaks the same protocol but points elsewhere | ||
| // must not receive the host identity set (device id included). |
There was a problem hiding this comment.
Move or incorporate this explanation into the existing top-of-file responsibility header rather than placing it beside the statement; the scoped guide requires comments in this package to live solely in the top /** */ block and explicitly forbids comments beside functions or statements. The newly added explanatory comment in catalog.test.ts:215-216 should be removed or handled consistently as part of the same cleanup.
AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L15-L19
Useful? React with 👍 / 👎.
|
CI note on the red shard: the failure is |
1 participant
Related Issue
Resolve #2576
Problem
See linked issue. A provider with
type = "kimi"and a third-partybase_url(a Kimi-compatible proxy or gateway) received the completeX-Msh-*host identity set, device id included, becauseresolveOutboundHeaderslooked at the vendor definition alone and never at the resolved endpoint.What changed
The full header forwarding now also requires the resolved base URL to be a first-party Moonshot host (
api.moonshot.ai/api.moonshot.cn). Third-party endpoints fall back to the User-Agent only, same as vendors without ahostHeaders: 'full'declaration. The vendor definition still decides the header contract; the endpoint decides whether that contract applies.Tests cover both directions: the existing dialect-path case (kimi type on
api.example.test) now asserts User-Agent only, and a new case confirms an explicit first-partybase_urlkeeps the full set. The default catalog test is unchanged.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.