feat(grant): store keys for Anthropic-compatible gateways - #457
Conversation
📦 Previous Review (superseded by newer review)Claude finished @dpup's task in 11m 35s —— View job Review: feat(grant): store keys for Anthropic-compatible gateways
Note: this branch is stacked on #456;
The core design is solid: the security-sensitive part — Posted 4 inline comments:
One thing I couldn't leave as an inline comment (it's on unchanged code, so GitHub rejected the anchor) but wanted to flag: Everything else — credential metadata plumbing, |
📦 Previous Review (superseded by newer review)Claude finished @dpup's task in 4m 25s —— View job Claude finished reviewing this PR
Scope reviewedPer the PR description, this PR is stacked on #456; I scoped the review to this PR's own commits ( Verification
FindingsNo bugs, security issues, or missing test coverage found. Specifically checked:
This is a clean, well-scoped PR — the security reasoning is spelled out in code comments where it matters ( |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| - Scheme must be `http` or `https` | ||
|
|
||
| Moat routes traffic through a relay endpoint on the Moat proxy, which forwards requests to the configured URL with credentials injected. This works transparently with `localhost` URLs because the relay runs on the host where `localhost` resolves correctly. Credentials from the `anthropic` or `claude` grant are injected for the base URL host in addition to the standard `api.anthropic.com` injection. | ||
| Traffic goes through the Moat proxy, which injects the `anthropic` or `claude` grant's credential for the endpoint's host in addition to the standard `api.anthropic.com` injection. The key stays on the host; the container only ever holds a placeholder. |
There was a problem hiding this comment.
This line is now stale for the case this PR adds. It says:
Traffic goes through the Moat proxy, which injects the
anthropicorclaudegrant's credential for the endpoint's host in addition to the standardapi.anthropic.cominjection.
But AnthropicProvider.ConfigureProxy (internal/providers/claude/provider.go:99-104) now returns early — skipping the api.anthropic.com injection — whenever the credential has Metadata[base_url] set (a gateway key from moat grant anthropic --base-url). That's the whole point of this PR's security fix, but this reference page (not touched by the diff) still describes the old "both hosts get the key" behavior unconditionally.
Worth a one-line carve-out here, similar to what docs/content/guides/01-claude-code.md and docs/content/reference/01-cli.md already got in this PR.
There was a problem hiding this comment.
Fixed in 9f8a449. The page now says the credential is injected for the endpoint's host, then carves out the gateway case explicitly:
An ordinary Anthropic key is injected for
api.anthropic.comas well. A gateway key — one granted withmoat grant anthropic --base-url— is deliberately not: Claude Code contactsapi.anthropic.comregardless ofANTHROPIC_BASE_URL, and a third-party key has no business being sent there.
| // --base-url marks the key as belonging to an Anthropic-compatible gateway | ||
| // rather than to Anthropic. Only the anthropic provider knows what to do | ||
| // with it; anywhere else it would be silently ignored. | ||
| if grantBaseURL != "" { |
There was a problem hiding this comment.
The core gateway-key logic (ValidateBaseURL, ConfigureProxy's skip, resolveClaudeBaseURL) is well covered in internal/providers/claude/grant_baseurl_test.go, provider_test.go, and internal/run/baseurl_test.go. But this CLI wiring — the --base-url-on-wrong-provider error at line 142, the ValidateBaseURL error wrap, and the new gateway-display branches in grant_list.go's credType and grant_show.go's showProviderMetadata — has no test coverage (grant_test.go/grant_show_test.go don't exercise the gateway path, and there's no grant_list_test.go at all).
Per this repo's CLAUDE.md invariant #1 ("test the companion case"), it'd be good to add at least:
- a
runGrant-level test asserting--base-urlwith a non-anthropicprovider errors - a
credType/showProviderMetadatatest asserting the gateway credential renders asapi-key (gateway)/ printsEndpoint:
Not a blocker given the underlying logic is solid, but worth adding before this pattern gets copied elsewhere.
There was a problem hiding this comment.
Added in 9f8a449, all with companion cases:
TestGrantBaseURLWrongProvider—--base-urlagainst github/claude/openai/npm all error with guidance naming the anthropic grant.TestGrantBaseURLInvalidURL—ftp://, no scheme,http://,http://:8080all error, and the message names the flag.TestCredTypeGateway— gateway rendersapi-key (gateway); plain key and empty-base_urlmetadata both stayapi-key.TestShowProviderMetadataGatewayEndpoint— printsEndpoint:; companion asserts a plain key prints no empty label.
| // | ||
| // The returned URL has any trailing slash removed, since the path is joined | ||
| // onto it later. | ||
| func ValidateBaseURL(raw string) (string, error) { |
There was a problem hiding this comment.
Minor: the "scheme must be http/https, host non-empty" check is now implemented three times independently — here, in internal/config/config.go's claude.base_url validation, and again in internal/run/baseurl.go's resolveClaudeBaseURL (which re-checks on purpose since it also validates a credential-sourced URL that bypasses the other two — see the comment at internal/run/baseurl.go:114-117, which is a legitimate reason for that one to exist independently).
The doc comment here even says it "mirrors the moat.yaml claude.base_url rules so the two sources cannot disagree" — but that's only true as long as someone remembers to keep both copies in sync by hand. A small shared helper (e.g. config.ValidateHTTPURL(raw string) (*url.URL, error)) that all three call would make that guarantee structural instead of a comment. Not urgent, just flagging since CLAUDE.md's review checklist calls out "duplicated code that should be shared or consolidated."
There was a problem hiding this comment.
| // The key is sent as x-api-key, the same header moat injects at runtime for an | ||
| // anthropic credential, so a gateway that only accepts Bearer tokens fails here | ||
| // rather than at the first real request. | ||
| func (a *anthropicAuth) ValidateGatewayKey(ctx context.Context, apiKey, baseURL string) error { |
There was a problem hiding this comment.
ValidateGatewayKey duplicates the request-building block from ValidateKey right above it (same reqBody template, Content-Type, x-api-key, anthropic-version headers — only the endpoint and status handling differ). If the validation payload changes later (new header, different probe model), it's easy to update one and miss the other, and gateway-key validation would silently drift from Anthropic-key validation. Consider factoring the request construction into a shared helper that both call.
There was a problem hiding this comment.
Fixed in 9f8a449 — both now build the probe through newAPIKeyProbe(ctx, endpoint, apiKey), so the payload and headers can't drift between Anthropic-key and gateway-key validation.
72ec721 to
9f8a449
Compare
📦 Previous Review (superseded by newer review)Claude finished @dpup's task in 9m 26s —— View job Review complete
Prior review round — all four resolvedCommit New findings (posted inline)
Additional findings (can't inline — outside the diff)
Minor / optional (not blocking)
Nothing here is a blocker on the core gateway-key security property for the primary documented flow ( |
| // AddExtraHeader, etc.) validate via isValidHost which rejects colons. The | ||
| // proxy's own lookup falls back from host:port to host, so a bare host | ||
| // still matches a request to a non-default port. | ||
| host := baseURLHost |
There was a problem hiding this comment.
ConfigureBaseURLProxy has no guard against baseURLHost resolving to api.anthropic.com, unlike AnthropicProvider.ConfigureProxy a few lines above (line 99-102 of this same diff), which explicitly skips injection for a gateway credential (cred.Metadata[MetaKeyBaseURL] != "").
This PR's whole security argument is "the gateway key is deliberately not registered for api.anthropic.com" (per the doc comment above and the CHANGELOG). But claudeBaseURLSource (internal/run/baseurl.go:106-114) lets moat.yaml's claude.base_url win over the credential's own recorded endpoint. If a project's moat.yaml sets claude.base_url: https://api.anthropic.com (by mistake, or via a shared/templated config) while a gateway-flavored anthropic credential is active, resolveClaudeBaseURL resolves the host to api.anthropic.com, and this function injects the gateway key there via x-api-key — exactly the outcome the PR's security section says can't happen.
Worth mirroring the same check here, e.g.:
if cred.Metadata[credential.MetaKeyBaseURL] != "" && host == "api.anthropic.com" {
return
}There was a problem hiding this comment.
Confirmed and fixed in 055bb1c — you're right, the guarantee had a hole exactly where you said.
ConfigureBaseURLProxy now refuses api.anthropic.com for a gateway credential, mirroring the guard in ConfigureProxy, with the host as a named constant (claude.APIHost) so the two can't drift. The run also warns rather than leaving an opaque 401, since pointing a gateway key at Anthropic is a config mistake worth naming:
Warning: claude.base_url points at api.anthropic.com, but the active anthropic grant is a gateway key for https://gw.lunaroute.com — it will not be sent to Anthropic.
Remove claude.base_url to use the gateway, or run without this profile to use an Anthropic key
Three tests, so the guard can't be over- or under-broad: gateway key + Anthropic host is refused; gateway key + its own host is still injected; plain Anthropic key + Anthropic host is still injected. The reference doc now states it unconditionally instead of implying the credential path is the only route.
|
|
||
| host := u.Hostname() | ||
| if !isLoopbackHost(host) { | ||
| return claudeBaseURL{ContainerURL: raw, CredentialHost: host}, nil |
There was a problem hiding this comment.
Trailing-slash normalization is applied on only one of the two claude.base_url entry points, so functionally-identical input produces different ANTHROPIC_BASE_URL values depending on where it came from.
ValidateBaseURL (internal/providers/claude/grant_baseurl.go:37-42, the --base-url grant path) does strings.TrimSuffix(raw, "/") before storing the credential's endpoint — its own doc comment says this matters "since the path is joined onto it later." But config.Load's claude.base_url validation (internal/config/config.go:742-746) calls ValidateHTTPURL and never trims, and here the non-loopback branch returns raw unchanged as ContainerURL.
So moat.yaml's claude.base_url: https://gw.example.com/ (trailing slash) ends up as ANTHROPIC_BASE_URL=https://gw.example.com/ in the container — if Claude Code or the gateway joins /v1/messages naively, that's a double slash — while the same endpoint granted via moat grant anthropic --base-url https://gw.example.com/ is normalized first. Given the trim already exists and the reasoning for it (ValidateHTTPURL's doc comment: "the three [sources] cannot drift into disagreeing about what a usable endpoint is"), it'd be more robust to fold the trim into config.ValidateHTTPURL itself (or have resolveClaudeBaseURL apply it to raw) so all three paths agree structurally rather than only two of three.
There was a problem hiding this comment.
Fixed in 055bb1c, folded into config.ValidateHTTPURL as you suggested rather than patched at the third site.
It now returns the normalized string alongside the parsed URL, and all three entry points use it: config.Load normalizes cfg.Claude.BaseURL in place, ValidateBaseURL returns the normalized form (its own TrimSuffix is gone), and resolveClaudeBaseURL builds ContainerURL from it. strings.TrimRight rather than TrimSuffix, so https://gw.example.com/// also collapses.
Covered in both places: TestValidateHTTPURL asserts the normalized form for four trailing-slash inputs, and TestResolveClaudeBaseURLTrimsTrailingSlash checks the remote and loopback branches (the loopback one rebuilds the URL, so it needed its own case).
📦 Previous Review (superseded by newer review)Claude finished @dpup's task in 2m 45s —— View job Review: feat(grant): store keys for Anthropic-compatible gateways
This is a re-review after From this round's most important finding (
From the earlier round (
Verification performed:
One low-confidence note, not blocking: the new guard in No other issues found. The PR is in good shape — the security-critical property (a gateway key never reaching |
A gateway that serves the Anthropic Messages API (LunaRoute, an internal
router) issues its own keys, so `moat grant anthropic` rejected them: the
sk-ant- prefix check fails, and validation went to api.anthropic.com,
which knows nothing about them. The only way to use such a gateway was to
export its key into the container via env or secrets, where the agent can
read it and the proxy logs it in cleartext.
`moat grant anthropic --base-url <url>` stores one properly:
- skips the sk-ant- prefix check
- validates against <url>/v1/messages, failing only on 401/403 (a
gateway serves its own catalog, so the validation model is usually
unknown to it and a 400/404 still proves the key authenticated)
- sends the key as x-api-key, the header moat injects at runtime, so a
Bearer-only gateway fails at grant time rather than first request
- records the endpoint on the credential
Runs then resolve their endpoint from the credential when moat.yaml does
not name one, so a gateway key works in any project with no per-project
config. moat.yaml's claude.base_url still wins.
The key is deliberately NOT registered for api.anthropic.com. Claude Code
contacts that host regardless of ANTHROPIC_BASE_URL — telemetry, the
bootstrap check, the MCP registry — and injecting a third-party key into
those requests would hand it to Anthropic. Verified against a local
gateway: those requests carry only the placeholder.
Pair with --profile to keep a gateway key separate from a real Anthropic
key. `moat grant list` marks it "api-key (gateway)" and `moat grant show`
prints the endpoint, since the two are not interchangeable.
Four review findings on the gateway grant.
The "http or https, with a host" rule was implemented three times —
moat.yaml's claude.base_url, --base-url, and the run-side resolver that
re-checks a credential-sourced endpoint. A comment promised they stayed
in sync by hand. They now all call config.ValidateHTTPURL, so that
guarantee is structural, and the empty-hostname case ("http://:8080")
is fixed everywhere at once rather than in two of three places.
ValidateGatewayKey duplicated ValidateKey's request building, so a
change to the probe payload or headers could silently apply to Anthropic
keys but not gateway keys. Both now build it through newAPIKeyProbe.
The CLI wiring had no tests: --base-url on a non-anthropic provider,
--base-url naming an unusable endpoint, and the two display branches
that keep a gateway key distinguishable from an Anthropic one in
`moat grant list` and `moat grant show`. Each is covered with its
companion case.
Finally, docs/content/reference/02-moat-yaml.md still said the
credential is injected "in addition to the standard api.anthropic.com
injection" — exactly what this branch stops doing for a gateway key.
Review found a hole in this branch's own security claim. The key is not registered for api.anthropic.com by AnthropicProvider.ConfigureProxy — but moat.yaml's claude.base_url wins over the endpoint recorded on the credential, so a config naming api.anthropic.com while a gateway key is active routed that key straight back to Anthropic through ConfigureBaseURLProxy. ConfigureBaseURLProxy now refuses that host for a gateway credential, mirroring the guard above it, and the run warns instead of leaving an opaque 401 — pointing at Anthropic with a gateway key is a configuration mistake worth naming. A plain Anthropic key aimed at api.anthropic.com is still injected; that is a legitimate setup. Also from review: trailing-slash normalization ran on only one of the three endpoint sources. --base-url trimmed, moat.yaml did not, so the same gateway written with a trailing slash produced a different ANTHROPIC_BASE_URL depending on where it came from, and a naive join would yield "//v1/messages". The trim moved into config.ValidateHTTPURL, which now returns the normalized form, so all three sources agree structurally rather than two of three by hand.
055bb1c to
869b8fa
Compare
|
Claude finished @dpup's task in 6m 42s —— View job Review in progress
|
1 participant
Summary
A gateway that serves the Anthropic Messages API (LunaRoute, an internal router) issues its own keys, so
moat grant anthropicrejected them outright: thesk-ant-prefix check fails, and validation went toapi.anthropic.com, which knows nothing about them. The only way to use such a gateway was to export its key into the container viaenv/secrets— where the agent can read it and the proxy logs it in cleartext.moat grant anthropic --base-url https://gw.lunaroute.com --profile lunaroute moat run --profile lunaroute -- claude # in any projectWhat
--base-urldoessk-ant-prefix check — gateways issue keys in their own format.<url>/v1/messages, failing only on 401/403. A gateway serves its own model catalog, so the fixed validation model is usually unknown to it and the request comes back 400/404 — which still proves the key authenticated. Being stricter would reject working keys on every gateway that doesn't happen to serve Anthropic's model ids.x-api-key, the same header moat injects at runtime, so a Bearer-only gateway fails at grant time rather than at the first real request.Metadata["base_url"]), so runs resolve it with no per-project config.moat.yaml'sclaude.base_urlstill wins.The security bit worth reviewing
The gateway key is deliberately not registered for
api.anthropic.com. Claude Code contacts that host regardless ofANTHROPIC_BASE_URL— telemetry, the bootstrap check, the MCP registry — so injecting a third-party key into those requests would hand it to Anthropic.AnthropicProvider.ConfigureProxynow returns early for a gateway credential, andTestAnthropicConfigureProxy_GatewayKeyNotSentToAnthropicguards it (with the companion case asserting a plain key is still injected).Verified end to end against a local gateway stand-in that only accepts one key. From the run's
network.jsonl:The gateway got the real key (
hello from the gatewaycame back through the agent); Anthropic's own API only ever saw the placeholder. The injected header is redacted in the log, which also sidesteps the cleartext-logging exposure thesecrets:route has.Also verified: wrong key →
401 … rejected the key;--base-urlon another provider → actionable error;ftp://→ rejected;moat grant listshowsapi-key (gateway);moat grant showprints the endpoint.Confirmed LunaRoute accepts the header moat injects — with no auth it answers
Provide your LunaRoute API key via the LUNAROUTE-API-KEY header, Authorization: Bearer, or x-api-key.Also included
examples/lunaroute/— working example using the grant + profile flow.docs/content/guides/01-claude-code.mdgains an "Anthropic-compatible gateway" section.make lintclean,make test-unitgreen.Out of scope
moat doctor claudestill diagnoses againstapi.anthropic.com, so it will report no LLM traffic for a gateway run. Worth a follow-up.envin~/.moat/config.yaml(so the model names live with the profile instead of eachmoat.yaml) is the next PR in this series.🤖 Generated with Claude Code