feat: API-key credential support + bulk import - #137
Open
ngh1105 wants to merge 70 commits into
Open
Conversation
…login
Kiro-Go could not sign in Microsoft 365 / Entra ID (Azure AD) tenant accounts:
such an account is neither an AWS Builder ID nor an AWS IAM Identity Center
account, and there was no Kiro hosted-portal flow at all. This adds the external
IdP path end to end.
Core token-path support (makes Azure-tenant tokens actually work):
- config.Account gains TokenEndpoint / IssuerURL / Scopes for external IdP creds.
- auth/oidc.go: external_idp refresh branch (refreshExternalIdpToken + shared
postExternalIdpToken) — refreshes against the IdP token endpoint (OAuth2
refresh_token grant, public client), not AWS SSO OIDC.
- proxy/kiro_headers.go: send TokenType: EXTERNAL_IDP on CodeWhisperer calls for
external_idp accounts. Without it CodeWhisperer silently returns an empty
profile list and rejects data-plane calls.
Login flow (auth/kiro_sso.go, new):
- Kiro hosted-portal PKCE flow with a loopback listener on 127.0.0.1:3128 (+[::1]).
- Drives both legs: social (Cognito) and the enterprise external-IdP leg (portal
descriptor -> OIDC-discover the Azure issuer -> 302 the browser to Azure ->
/oauth/callback -> exchange at the IdP token endpoint).
- SSRF/open-redirect controls: https-only allow-list of *.microsoftonline.com
(.us/.cn), no IP literals, applied to issuer + both discovered endpoints;
OIDC discovery refuses redirects; anti-CSRF state matched on both legs;
single-shot enterprise leg; deadline self-teardown frees the loopback port.
Admin wiring:
- proxy/handler.go: /auth/kiro-sso/{start,poll,cancel} (Start/Poll session
pattern mirroring Builder ID); profileArn resolved lazily on first use.
- web/app.js + locales: 7th "Enterprise SSO - Microsoft 365" Add-Account card.
Tests: external-IdP refresh (form encoding, response mapping, refresh-token
retention, precondition guards), issuer allow-list validation, authorize-URL
construction, JWT email/preferred_username extraction, and the EXTERNAL_IDP
header gating (set for external_idp, omitted for idc/social).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Azure-tenant (external_idp) logins hardcoded the account region to us-east-1,
so a Kiro profile provisioned in another region (e.g. eu-central-1) never
resolved: the external_idp refresh path returns no profileArn, and the
ListAvailableProfiles probe that would supply one targeted the wrong region.
- proxy/kiro_api.go: probe ListAvailableProfiles across candidate regions
(account region first, then fallbacks) and derive the authoritative region
from the returned profile ARN (arn:aws:codewhisperer:REGION:...). Persist it
via config.UpdateAccountRegion and apply it before caching the ARN so the
same request's data-plane calls target the detected region.
- Fallback probing is gated to external_idp / region-unset accounts; idc,
social and Builder ID keep their prior single-region behavior (no extra
upstream calls, no chance of flipping an established region).
- Candidate fallbacks default to us-east-1,eu-central-1 and are overridable
with KIRO_PROFILE_REGIONS.
- regionalizeURL refactored to regionalizeURLForRegion; the non-us-east-1
invariant (both q.us-east-1 and codewhisperer.us-east-1 collapse to
q.{region}; there is never a codewhisperer.{region} host) is pinned by tests.
Docker: make `docker-compose up -d` complete the Enterprise SSO login without
manual edits — publish 3128 host-side on loopback, bind the in-container
callback to 0.0.0.0 (KIRO_SSO_CALLBACK_BIND=0.0.0.0), EXPOSE 3128. Removes the
need for a local docker-compose.override.yml.
Tests: parseRegionFromProfileArn, the regionalizeURLForRegion invariant, and
candidate-region ordering/gating/env-override (24 region tests).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reconcile region handling with upstream feb3437 ("use profile ARN region for data-plane calls"). Upstream derives the data-plane region from the profile ARN (account.Region is the auth/OIDC region and can differ from the profile region); this branch adds cross-region ListAvailableProfiles probing so an Azure-tenant (external_idp) account whose profile lives outside its configured region (e.g. eu-central-1) can discover that profile in the first place. Resolution in proxy/kiro_api.go: - Keep upstream regionFromProfileArn / kiroRegionForProfile / regionalizeURLForProfile (data-plane region derived from the ARN). - regionalizeURLForProfile now delegates to this branch's regionalizeURLForRegion primitive, which also backs the region-targeted profile probe (listAvailableProfilesInRegion). - Drop the now-redundant parseRegionFromProfileArn, applyDetectedRegion and config.UpdateAccountRegion: the cached profile ARN is the single source of the data-plane region, so persisting a separate region is unnecessary and would wrongly overwrite the auth region for idc / Builder ID accounts. - Cross-region probing stays gated to external_idp / region-unset accounts.
The operator no longer enters an AWS region to start the Microsoft 365 / Kiro hosted-portal sign-in. The data-plane region is derived from the profile ARN returned by SSO (social exchange) or discovered via the cross-region profile probe (external_idp / Azure), so a region the user would have to know up front is no longer required. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The 'Add credential JSON' import path (apiImportCredentials handler + importCredentials frontend) never learned the external_idp account type, so pasting an Azure AD / Microsoft 365 credential errors with 'Token refresh failed' (authMethod collapses to 'social' → wrong endpoint). Design covers backend + frontend adapter changes, SSRF validation of the user-supplied tokenEndpoint, and a test seam for the IdP allow-list.
apiImportCredentials now recognizes external_idp (via normalizeImportAuthMethod + tokenEndpoint inference), validates the user-supplied IdP endpoint against the allow-list (SSRF guard), carries TokenEndpoint/Scopes into the refresh, and persists them with Provider defaulting to AzureAD.
A pasted full record's id is reused (config.AccountIDExists guards collisions), email falls back to the provided value when GetUserInfo can't resolve it, and profileArn is preserved since external_idp refresh returns none — so re-importing a backup no longer duplicates accounts.
importCredentials now reads tokenEndpoint/issuerUrl/scopes (+ id/email/profileArn), recognizes external_idp (alias or tokenEndpoint) before the idc/social default, and extends the payload so external_idp accounts round-trip through the new backend.
Kiro Account Manager exports carry refreshToken + clientId (+ userId at account level) but omit tokenEndpoint/issuerUrl/scopes. DeriveExternalIdpEndpoints reconstructs them from userId's embedded Azure tenant + the Kiro client ID, so the export format imports instead of failing 'requires clientId and tokenEndpoint'. The derived tokenEndpoint is still re-validated against the IdP allow-list.
Bare credential blobs (just clientId+accessToken+refreshToken, no authMethod/userId/tokenEndpoint) had no marker, so normalizeImportAuthMethod classified them as idc and refresh hit the wrong endpoint. Decode the access token JWT issuer: a microsoftonline iss classifies the credential as external_idp and yields the Azure tenant to derive tokenEndpoint/issuerUrl/scopes from (DeriveExternalIdpEndpoints now falls back to the JWT iss when userId is absent). The reclassify only fires when the derived endpoint clears the IdP allow-list, so idc/social tokens (AWS/non-microsoftonline iss) are never misclassified.
…n JWT When a pasted external_idp credential carries an Azure AD access token (a JWT with a real exp), persist it directly WITHOUT a live refresh round-trip. The JSON can then be imported repeatedly / into multiple instances without each import consuming (rotating) the refresh token, and without requiring egress to Microsoft at import time — the runtime background refresh renews it later. Falls back to refresh-at-import for idc/social and external_idp creds without a usable access token, so the regression gate still holds there.
…mport The json.accounts.map() in both importCredentials functions never read c.accessToken, so pasting a Kiro Account Manager export dropped the access token before it reached the backend. Trust-on-import (which skips the Microsoft refresh round-trip by persisting the JWT's exp as ExpiresAt) requires accessToken; without it the import fell back to refresh-at-import and failed with AADSTS9002313 on stale tokens. Read c.accessToken/a.accessToken in the map for both UIs.
…/dup guards Post-merge hardening from a security review of the external_idp credential import feature. - auth/oidc.go: re-validate tokenEndpoint against the IdP allow-list inside postExternalIdpToken, at the outbound-POST boundary. Validation was only enforced at import time, so a runtime/background refresh of an account whose TokenEndpoint was set out-of-band (backup restore, external file write) could POST the refresh token to a non-allow-listed host. Uses the exported ValidateExternalIdpEndpoint so the SetExternalIdpValidatorForTest seam still relaxes it for httptest. - proxy/handler.go: cap apiImportCredentials body with http.MaxBytesReader(1MiB). accessToken is attacker-influenced input that gets base64+JSON decoded twice (issuerFromAccessTokenJWT / ExpFromAccessTokenJWT); an oversized token was a memory-amplification DoS. - config/config.go: reject a duplicate account id inside AddAccount under the write lock, closing the TOCTOU between the import path's AccountIDExists pre-check (RLock) and the append. - auth/kiro_sso_test.go: install the validator seam in the two refreshExternalIdpToken unit tests now that the POST boundary validates. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove per-account isolation from the fingerprint store so N accounts in a pool share cache entries (they're in the same Anthropic org). Hit rate jumps from ~(1/N)*70% to ~70%.
Replace the hardcoded 0.85 cache-read cap with a config value (promptCacheMaxRatio, default 0.85). Operators can raise it to 0.95 for continue-heavy workloads where >85% of input is genuinely from cache.
Load data/prompt_cache.json on startup and debounce-write every 30s so cache entries survive restart. Expired entries are dropped on load. The file already existed but was unused — now it's read and written.
Port two ideas from the kiro-rs reference into the Go prompt-cache tracker: - splitAgainstTotal: rescale the estimate-domain cache split onto the real upstream input total (contextUsage) so input+creation+read == total, instead of reporting estimate-based creation/read against a different real total. Wired into both Claude handler paths once realInputTokens is known. - Bounded LRU: cap the entries map at maxPromptCacheEntries (4096) and evict least-recently-hit entries (LastHit) instead of relying on TTL pruning only. Also wires pool.RecordLatency from all 4 handler success paths, completing the A1 latency signal from the previous commit. Tests: cache_tracker_hardening_test.go (LRU eviction, proportional-split identity + no-coverage). Two pre-existing translator image tests remain failing (unrelated to this change).
…==creation) Compute() derives creation from lastTokens capped at 85% of total, but computePromptCacheTTLBreakdown sums raw breakpoint deltas capped only at TotalInputTokens. When the 0.85 cap bites, cache5m+cache1h > creation, violating the Anthropic invariant cache_creation_input_tokens == ephemeral_5m + ephemeral_1h in the message_start SSE event. Add clampCacheBreakdownToCreation (ratio-preserving downscale to creation) and apply at both call sites: the empty-cache path (effectiveCreation) and the matched path (creation).
…rty on hit - Stop(): guard close(stopChan) with sync.Once so a second Stop (test cleanup + main shutdown) no longer panics on close-of-closed-channel. - Compute(): set t.dirty = true on a cache hit so the extended TTL/LastHit is persisted even if the save loop flushes between Compute and the next Update. TDD: TestStopIsIdempotent + TestComputeSetsDirtyOnCacheHit, RED before fix (Stop panicked; dirty stayed false) -> GREEN after. go test ./proxy/... clean.
TestComputeBreakdownClampedToCreationEmptyCacheBelowMin covers the empty-cache (first-request) clamp at cache_tracker.go:255. When lastTokens < minTokens (1024), effectiveCreation is zeroed but computePromptCacheTTLBreakdown(profile,0) still returns lastTokens > 0; the clamp forces 5m/1h to 0 to preserve the Anthropic invariant cache_creation == ephemeral_5m + 1h. The matched-path TestComputeBreakdownClampedToCreation (always lastTokens >= minTokens) never hits this branch. Corrects an earlier 'empty-path clamp is a no-op' read (no-op only when lastTokens >= minTokens; bites on the below-min sub-case, reachable via a short first request). Verified by toggle: comment the empty-cache clamp -> test RED (5m=0 1h=100 creation=0) -> restore -> GREEN.
- entries: value -> *pointer + container/list recency list (front=MRU) - drop LastHit; list position is authoritative; evictOverflowLocked pops back in O(1) (replaces O(n log n) sort-under-lock evictLRULocked) - two constructors: newPromptCacheTracker delegates to config default; newPromptCacheTrackerWithCapacity for tests (any capacity) - capacity clamp (>=256) lives in config.GetPromptCacheMaxEntries (prod safety), not the constructor (which must trust callers for testability) - 5 tests rewritten to compile against the new pointer/list shape
feat(hardening): P0 production hardening ported from kiro-tutu (zero-dep) Five stability/correctness/security gaps closed without adding any dependency (stays on github.com/google/uuid only): - Dynamic account retry budget + exponential backoff resolveAccountRetryBudget() scales the per-request retry count with the account pool size (min(totalAccounts,64)) instead of a hardcoded 3, so a request can now try every account instead of giving up after the first three under a 429 storm. accountRetryBackoff() adds 200ms->2s exp + jitter between attempts. Fixes the >3-account bug. All 6 retry loops wired. - Atomic config write Save() now writes a sibling .tmp then os.Rename over config.json, so a crash mid-write can no longer truncate/corrupt the single source of truth (all account tokens, API keys, admin password). - Production hardening middleware (proxy/prod_middleware.go) recoverMiddleware turns panics into clean 500s (no hung SSE clients), requestIDMiddleware propagates/generates X-Request-Id, securityHeaders sets nosniff/DENY/no-referrer, bodyCapMiddleware bounds every request at 64 MiB (closes the unlimited io.ReadAll OOM on /v1/messages). - Graceful shutdown main.go now catches SIGINT/SIGTERM and drains in-flight requests via srv.Shutdown (55s) instead of hard-killing mid-SSE. The serve/shutdown loop is factored into runServerWithDrain so the path is unit-testable (Windows taskkill cannot deliver SIGINT to a detached console process). Verification: go build/vet/test all green; security + x-request-id headers confirmed live on the rebuilt binary; dynamic budget confirmed (1 account -> 20/50 throttle, 15 accounts -> 50/50 clean). Constant-time admin password already present (handler.go subtle.ConstantTimeCompare), so not touched. Metrics/ratelimit/audit/warmup intentionally deferred (need deps). @
feat(dispatch): exponential error cooldown + jitter (P1 from kiro-tutu) RecordError previously set a fixed 1-minute cooldown at >=3 consecutive errors, so every co-failing account un-cools at the identical instant and they stampede upstream together (thundering herd) — observed in testing where a transient blip cascade-disabled 13 accounts that then all re-enabled at once. errorCooldownDuration() now backoffs exponentially (1m -> 2m -> 4m -> ..., capped at 8m) starting at the threshold, plus +-10% jitter so recoveries stagger. Quota (1h) and threshold (<3, no cooldown) behaviour unchanged. Tested via pool/cooldown_test.go: below-threshold == base, respects 8m cap, never non-positive, grows monotonically before cap. go build/vet/test green. @
A 400 "Improperly formed request" is inherent to the request payload: no endpoint or account can fix it. Previously it hit handleAccountFailure's default branch -> RecordError, penalising the relaying account's health, cooling it down, and rotating to other accounts that would fail identically. Under a malformed-request storm this cascade-disabled healthy accounts and scattered cache affinity (observed in 100/500-concurrent testing where a single bad account disabled the whole pool). Three layers now cooperate via a single typed error: - kiro.go: HTTP 400 with "improperly formed request" body is wrapped as upstreamPermanentError and returned immediately (no other endpoint tried), detected before any event-stream bytes are read so streaming handlers have not yet sent anything to the client. - handleAccountFailure: isUpstreamPermanentError is neutral -> returns without recording success or failure (account left at full health). - All 6 account retry loops (handler x4, responses_handler x2): break on isUpstreamPermanentError so no other account is tried; lastErr reaches the client. ImproperlyFormedClientMessage helper removed (no callers). go build/vet green; full suite ok. TestResponsesStreamSSE TempDir-cleanup flake confirmed pre-existing (reproduced on main 4f079f4), not a regression.
Anthropic's real prompt-cache minimum is ~1024 tokens for the Claude 4 / Opus family. The previous 4096 excluded many valid Opus breakpoints, so an Opus conversation reported far fewer cache hits than it actually earned (shorter Opus prefixes that should cache were silently skipped in both Compute and Update). Now matches the non-Opus default and tutu. Evaluated but NOT ported: tutu's localCacheEntryTTL=1h decoupled from the billing 5m TTL. The current repo sets ExpiresAt = breakpoint.TTL (correct). A 1h local ledger would let a request at minute 6 HIT locally (reporting cache_read>0) while Anthropic's real 5m cache had already expired — over-reporting cache_read versus reality. Rejected as it degrades outward-facing billing-number accuracy. Kept current repo's on-disk persistence + LRU + configurable 0.85 cap + clampCacheBreakdownToCreation (all of which tutu lacks). go build/vet/test green; no test depends on the 4096 constant (the 4096s in tests are max_tokens/context-window values, unrelated).
…p (zero-dep P1)
Ports the remaining zero-dependency observability/hardening pieces from
kiro-tutu. No new dependencies (stays on github.com/google/uuid only).
- Rate limiting (proxy/ratelimit.go)
Token-bucket limiter with a global tier and a per-API-key tier, both OFF
by default (rate 0 == unlimited) so enabling never regresses existing
deployments. Operators opt in via KIRO_RATE_LIMIT_RPM,
KIRO_RATE_LIMIT_PER_KEY_RPM, KIRO_RATE_LIMIT_BURST_SECONDS. Health/admin/
static paths are exempt; per-key buckets idle >10m are swept.
- Prometheus /metrics (proxy/prod_metrics.go + prod_gauges.go)
Hand-written Prometheus text exposition (no metrics client dep):
kiro_http_requests_total{method,code,path}, duration histogram, plus
counters for ratelimit rejections, recovered panics, and token refreshes.
Pool gauges: kiro_accounts_total, kiro_accounts_available,
kiro_credential_quota_remaining{account}. kiro_account_inflight omitted
(this repo has no in-flight tracking; gaugeHelp has no entry for it).
- Token warm-up + refresh counter (proxy/warmup.go + token_refresh.go)
backgroundWarmup runs every 2 min with a 15-min look-ahead, refreshing
tokens nearing expiry off the request path (re-checks under tokenRefreshMu
so it never races or double-refreshes the request path). authRefreshToken
wraps auth.RefreshToken so every refresh (request path, background sweep,
admin verify/import) is counted in kiro_token_refresh_total from one choke
point. kiro_api.go's now-unused auth import removed.
- Startup security warnings (main.go)
Warns once at boot if the admin password is still the default 'changeme',
and if binding 0.0.0.0 with no API-key requirement (the two most common
foot-guns for accidental public exposure). Behavior unchanged.
prod_middleware.go rewritten to fold the 64 MiB body cap into
instrumentMiddleware (was a separate layer) and add rateLimitMiddleware,
statusRecorder, metricsRouter, pathClass, clientIP. WrapHardening now
composes recover -> request-id -> /metrics -> security headers ->
instrument(status+latency+counters+body cap) -> rate limit -> handler.
go build/vet green. TestClaudeNonStreamRetriesNextAccountAfterPreResponseFailure
TempDir-cleanup flake confirmed pre-existing (reproduced on main 5eb78de in a
worktree: 2/5 FAIL identical), not a regression from the warmup goroutine.
…(P1)
Replaces pure weighted round-robin with a smart scheduler: among accounts
passing the hard filters (model support, not cooling down, token valid,
quota available), pick the one least likely to fail or stall:
lowest in-flight -> fewest recent failures -> fewest recent selections
-> fewest recent successes
in-flight is incremented on selection and decremented on every finish path
(RecordSuccess/RecordError/RecordPermanentRejection/DisableAccount/
MarkOverLimit), so under concurrency the scheduler spreads load instead of
piling onto one account. Every finish clamps at 0, so a missed/double
finish can never drive the counter negative; with the default concurrency
cap of 0 (unlimited) a leaked in-flight only skews the smart tie-break,
never starves an account.
Two opt-in knobs (default = existing behaviour):
KIRO_MAX_CONCURRENT_PER_ACCOUNT hard per-account in-flight cap (0=unlimited)
KIRO_SELECTION_STRATEGY smart|round_robin|random|least_used|most_used
Deliberate differences from kiro-tutu:
- This repo's least-cooldown fallback is retained: when no account passes
all hard filters, the earliest-cooldown eligible account is returned
(rather than nil -> 503), preserving existing availability behaviour.
- endpoint429 / affinity machinery NOT ported (no cache-affinity layer here;
endpoint-429 signals only fed affinity circuit-breaking).
Also: handleAccountFailure now calls RecordPermanentRejection on the
permanent-error path (fixes an in-flight leak — previously it returned early
without releasing the selection slot). prod_gauges.go now emits
kiro_account_inflight{account} from GetRuntimeStatsSnapshot.
New: pool/dispatch.go (selection + runtime stats), pool/tuning.go (cap +
strategy), pool/dispatch_test.go (7 tests: lower-in-flight preference,
in-flight decrement, neutral permanent-rejection, clamp-at-zero, disable
releases slot, least-cooldown fallback, selection spans both accounts).
pool/account.go trimmed (selection logic moved to dispatch.go). go
build/vet/test green.
Ports the durable request audit / access log from kiro-tutu. Records one structured entry per HTTP request (request id, SHA-256 of the provided API key, method, path, status, latency, response size). Writes are async and non-blocking — a full buffer drops the entry and counts it (kiro_audit_dropped_total) rather than stalling the proxied request. Backend: append-only JSON-lines file (zero dependency). tutu defaults to SQLite (modernc.org/sqlite); this repo keeps a single dependency (github.com/google/uuid), so only the JSONL backend is compiled in. KIRO_AUDIT_BACKEND is accepted for compatibility and any non-"jsonl" value falls back to JSONL with a one-time info note. KIRO_AUDIT_DISABLE turns audit off entirely. Each entry is flushed immediately so a crash loses at most the single in-flight record. Wiring: - proxy/persist.go: AuditEntry, jsonlAudit backend, async AuditStore (buffered chan 4096 + 1 writer goroutine), InitAuditStore/CloseAuditStore, recordAudit, hashAPIKey, AuditRecent (tail-read). - proxy/prod_middleware.go: recordAudit called in instrumentMiddleware (one entry per request); /debug/audit-log route in metricsRouter guarded by adminPasswordOK (constant-time subtle compare); serveAuditLog. - proxy/prod_metrics.go: kiro_audit_dropped_total counter + help. - main.go: InitAuditStore(dir) + defer CloseAuditStore(). The API key is SHA-256 hashed, never stored, so the audit log is safe to expose to an authenticated admin. go build/vet green. The 2 proxy test "failures" (TestClaudeNonStreamRetriesNextAccountAfterPreResponseFailure, TestResponsesStreamSSE) are Windows TempDir-cleanup flakes reproduced indistinguishably on main c59ef41 (8/10 FAIL) vs this branch (9/10 FAIL); no test calls InitAuditStore so globalAudit stays nil and audit cannot affect the TempDir lifecycle.
…oring slices Three comment blocks lagged behind slices already merged into main and contradicted the code sitting next to them: - prod_gauges.go header claimed kiro_account_inflight was omitted because "the health-scoring+inFlight slice was not ported." That slice merged in c59ef41, and collectPoolGauges already emits the gauge from GetRuntimeStatsSnapshot. Reword to match reality. - prod_metrics.go gaugeHelp had no entry for kiro_account_inflight, so /metrics rendered it with a generic "Gauge." help line. Add the entry. - prod_metrics.go and prod_middleware.go package docs claimed the audit store and kiro_audit_dropped_total were "intentionally omitted (no audit store)." The JSONL audit store merged in a2ddc08 and the counter is incremented from persist.go. Reword both; only cluster drain remains genuinely excluded. No behaviour change. Comments-only, so build/vet stay green.
…roperly formed request")
Ported from kiro-tutu (zero-dep: stdlib + kiro-go/logger only).
Motivation: when a request carries a large tools array, the raw serialized
payload can exceed Kiro's accepted size and the upstream answers with a
400 "Improperly formed request." This repo already DETECTS that rejection
after the fact (upstream_error.go isImproperlyFormedRejection, wired at
kiro.go:393) and permanently short-circuits the request. This slice adds
the complementary PREVENTION: compress oversized tool definitions before
send, so fewer requests get rejected in the first place.
compressToolsIfNeeded does two progressive stages, each stop-early on
success:
1. Recursively simplify each tool's inputSchema — keep the structural
skeleton (type, required, enum, $ref, anyOf/oneOf/allOf, properties,
items) and strip purely descriptive fields (description, examples,
title, default). These are usually the bulk of the bytes and are not
needed for the model to understand parameter structure.
2. If still over threshold, ratio-truncate each description (UTF-8 safe,
keep at least minToolDescChars so the tool stays identifiable).
Threshold default 20 KiB (KIRO_TOOLS_COMPRESS_THRESHOLD_BYTES); 0 disables.
Correctness notes baked into the schema simplifier (locked by tests):
- required / enum / nested-object required are preserved — dropping them
would change parameter semantics and turn an accepted request into a
rejection (worse than no compression).
- A node with only $ref (no type) keeps $ref so it does not collapse to
an empty {} the model/upstream cannot interpret.
- anyOf/oneOf/allOf subschemas are recursively simplified.
Wired into both converter exits (complements existing per-tool
maxToolDescLen truncation + cleanSchema):
- convertClaudeTools (translator.go)
- convertOpenAITools (translator.go)
Tests (9): no-op under threshold, schema-only suffices, description
truncation fallback + UTF-8 safety, threshold=0 disable, env resolution
table, end-to-end via convertClaudeTools, constraint-preservation
(type/required/enum/$ref/nested/anyOf), CJK-safe ratio truncation.
build/vet/test green: proxy 5.573s, pool 0.388s.
…dep P1) Ported from kiro-tutu (zero-dep: stdlib sync/time + kiro-go/config/logger). Motivation: this repo returns a SIMULATED cache_read to downstream clients (handler.go resp.Usage.CacheReadInputTokens = cacheUsage). The higher the reported hit rate, the less the downstream pays and the more this gateway subsidises. cache_metrics.go adds two missing capabilities: 1. Windowed observability: aggregate per-dispatch cache diagnostics into rolling 5m/15m/1h windows (by source / model / account), exposed via /v1/stats under "cacheWindows" — so cache coverage is auditable without digging through info logs. Previously only point-in-time PromptCacheStats (cumulative hits/misses) was surfaced. 2. Hit-rate safety valve: when the aggregate cache hit ratio over a rolling 5m window exceeds the agreed 88% ceiling (cacheHitRatioWarnThreshold) with enough samples, emit a WARN so runaway subsidy is detectable rather than only governed by a code comment. Warn-only — never mutates the numbers or affects billing. Denominator correctness (locked by test, reproduces a real prod regression): computeCacheMetricRatios uses max(InputTokens, cacheRead+cacheCreation) as the denominator. cache_read/creation come from the local prompt-cache simulator while InputTokens comes from the upstream report/estimate — different bases. On long high-hit sessions cache_read routinely exceeds InputTokens, so using InputTokens as denominator produced ratios > 1 (prod saw cacheHitRatio=1.4). The max() denominator keeps ratios in [0,1] and matches Anthropic's official read/(read+create+uncached) basis. Wiring: 4 dispatch-completion hooks in both Claude paths (recordClaudeCacheDispatch): stream success (handler.go:1281), stream failure (1227), nonstream success (1564), nonstream failure (1525). Failures record 0 tokens so a failed dispatch cannot inflate the simulated cache_read subsidy. Source dimension is "stream"/"nonstream" (this repo has no cache affinity, so the tutu affinity-source classification does not apply). Tests (9): totals+ratio aggregation, dimension grouping, window boundary exclusion, retain-based prune, reset, empty-state ratios, the >1-ratio regression guard, and the safety-valve (threshold trigger + throttle + min-sample + below-threshold silence). build/vet/test green: proxy 5.587s, pool 0.382s, config 0.542s.
…p P1)
Ported from kiro-tutu (zero-dep: stdlib sync/time + fmt).
Motivation: this repo had removed its total stream timeout (letting very
long opus thinking run unbounded), but emitted nothing to the client while
the upstream was silent — not before first byte, and not between data
chunks. Cloudflare's ~120s Proxy Read Timeout then closed the connection
mid-stream, surfacing to users as "the reply cuts off halfway". This repo
only set the Connection: keep-alive header; it never sent a heartbeat.
streamKeepaliveInterval (default 10s) drives a goroutine that emits a
': keepalive\n\n' SSE comment line whenever the connection has been idle
for that long. Comment lines are ignored by Anthropic/OpenAI SDKs and
Claude Code, so they keep the client/CDN connection alive without
polluting the response. It covers both stall cases: slow prefill before
first byte, and silent gaps between data chunks during heavy thinking.
Concurrency: the heartbeat goroutine and the main goroutine both write to
the same ResponseWriter, so every real write is funnelled through a mutex-
guarded emitter (emit / emitRaw) that shares hbMu with the heartbeat.
lastWriteNano gates the heartbeat so it only fires on genuine silence, not
on top of active data. hbStopped + hbDone shut the goroutine down cleanly.
Committed-flag error path: once the heartbeat has flushed a 200 header
(committed), the HTTP status can no longer change — the client is parsing
SSE. On the loop-exit error path (no account / all failed before any data),
if committed the handler closes the stream with a valid SSE error sequence
(Claude: error+message_delta+message_stop; OpenAI: error chunk+[DONE])
instead of sendClaudeError/sendOpenAIError's WriteHeader+JSON, which would
trigger a superfluous WriteHeader and corrupt the stream. If not committed,
the original HTTP error response is preserved. stopHeartbeat is called
before deciding the form so the heartbeat cannot race the final write.
Wired into both streaming handlers in handler.go:
- handleClaudeStream (emit replaces every h.sendSSE on the hot path)
- handleOpenAIStream (emitRaw replaces the three direct fmt.Fprintf
writes + the tail)
Tests (handler_keepalive_test.go): a stalled upstream server emits one
text frame, sleeps long enough to cross multiple heartbeat intervals, then
emits a second frame. Asserts ': keepalive' appears between the two real
frames, both real frames are delivered intact, and the protocol terminator
(message_stop / [DONE]) is present. Uses a lockedFlushRecorder so a -race
run catches any handler-internal contention; the real mutual exclusion is
hbMu by construction. -race needs cgo (unavailable in this env), so
correctness is verified by stable isolation runs (3/3 green, both paths)
plus the full suite.
build/vet/test green: proxy 6.110s, pool 0.431s, config 0.528s. One FAIL
in the full run (TestResponsesContinuationKeepsNewInstructions → unlinkat
... directory is not empty) is the pre-existing Windows TempDir cleanup
flake, reproducible on main before this slice; passes in isolation.
The legacy single-key path compared the provided key with a plain `provided != expected`. A byte-by-byte early-exit string compare leaks the length of the matching prefix through response timing, giving a network observer a side-channel to recover the configured API key one prefix at a time. Replace with crypto/subtle.ConstantTimeCompare, matching the constant-time checks already used for the admin password (handler.go) and the audit-log password (prod_middleware.go). The multi-key path (FindApiKeyByValue) is out of scope — it maps key→entry by hash, not by direct comparison. No new dependency (crypto/subtle is stdlib). build/vet green.
…ro-dep P1)
Ported from kiro-tutu (zero-dep: stdlib crypto/sha256 + encoding/hex).
Motivation: when an account has an empty MachineId, buildKiroHeaderValues
appended no device suffix, so the User-Agent ended at the KiroIDE version
and was byte-identical for every empty-id account. A shared, stable UA
across many accounts is the strongest cross-account association signal
upstream can correlate on — the single most reliable way to detect that
these are one operator's pool rather than independent users.
config.DeriveMachineId(accountID) deterministically maps an account ID to a
stable 64-hex device id (sha256("kiro-device-"+id)). The empty-MachineId
fallback now uses it, so every account's UA always carries a unique, fixed
device fingerprint — same value per account across requests and restarts,
distinct across accounts. An explicitly configured MachineId still wins;
a nil account (no ID to derive from) keeps the suffix-less UA.
Only the device-id dimension is diversified here (the dominant signal).
The tutu reference additionally derives a per-account desktop platform
(OS/version) triple via GetKiroClientConfig(accountID); that slice is a
larger port touching the config API surface and is intentionally left for
a follow-up — the device suffix alone removes the strongest correlation
signal.
Test (TestBuildKiroHeaderValuesFallsBackToDerivedMachineId): empty-id
fallback is stable across calls, carries the derived suffix, is distinct
across accounts, yields to an explicit MachineId, and is absent for a nil
account. build/vet/test green: proxy 6.457s, pool 0.545s, config 0.820s.
…int, zero-dep P1)
Ported from kiro-tutu (zero-dep: stdlib crypto/sha256 + encoding/hex).
Motivation: GetKiroClientConfig() chose the {systemVersion, kiroVersion,
nodeVersion} UA triple via runtime.GOOS. A Linux server deploy therefore
advertised os/linux#6.6.87 in every request's User-Agent. A Linux kernel in
a Kiro IDE UA is the single strongest "this is not a real Kiro IDE user"
signal — it directly contradicts the one platform Kiro IDE actually ships on
(mac/win), and it appeared identically on every account. This is the
highest-value remaining anti-fingerprint gap.
Replaces runtime.GOOS selection with a deterministic per-account profile:
- clientProfilePool: a weighted, verified-real set of {platform, version}
triples (macOS-weighted 85%, Windows minority 15%). Every systemVersion
is a real shipped build (darwin#23.6.0/24.5.0/24.6.0, win32#10.0.22631/
19045). NEVER contains Linux — the pool guarantees the output platform is
always mac/win regardless of host OS.
- DeriveClientProfile(accountID): sha256("kiro-profile-"+accountID) selects
from the pool by cumulative weight. Pure + deterministic, so an account
looks like one stable desktop install across requests and restarts, and
distinct accounts spread across the distribution. The "kiro-profile-"
prefix differs from DeriveMachineId's "kiro-device-" so platform and
device selection are independent while both stay bound to the account.
- GetKiroClientConfig(accountID): derives the triple from the account
(never Linux); operator config overrides still win per-field, preserving
the manual-override path. defaultSystemVersion() + runtime.GOOS path
removed (and the runtime import with it).
Pairs with the MachineId fallback (be43c7f): device id + platform both derive
from the same accountID, so one account presents one consistent mac/win
desktop install.
Tests (config/client_profile_test.go): never-Linux over many IDs incl. empty;
deterministic stability; distribution matches weights (5000 samples);
verified KiroVersion/NodeVersion preserved; operator override wins per-field.
build/vet/test green: proxy 6.457s, pool 0.545s, config 0.820s. The one FAIL
in the full proxy run (TestOpenAIStreamEmitsKeepaliveDuringUpstreamStall →
unlinkat ... directory is not empty) is the pre-existing Windows TempDir
cleanup flake, reproducible on main before this slice; passes in isolation.
…hint (zero-dep P1)
Ported from kiro-tutu (zero-dep: stdlib errors + strings, reuses the
existing isImproperlyFormedRejection predicate).
Motivation: when upstream rejects a request with "Improperly formed
request", the raw text reaches the client and reads like a gateway bug.
In practice the request is usually structurally valid but the tools array
is too large / payload oversized (upstream size limits). Users have no
actionable hint.
improperlyFormedClientMessage(err) rewrites that one opaque message into a
readable, actionable hint ("...try reducing the number of tools or
shortening the context"); every other error passes through unchanged, so
wiring is safe at every client-facing terminal error site. It complements
tool_compression.go (b27686c): compression PREVENTS the rejection when it
can; when it cannot, this surfaces a hint instead of the raw text.
Wired into all 10 client-facing terminal-error sites across the three
protocols (Claude/OpenAI/Responses × stream/nonstream): the post-loop
500 sendClaudeError/sendOpenAIError tails, the committed-header SSE error
sequences (stream paths), the mid-stream error events, and the
response.failed events. The one remaining err.Error() in responses_handler
(line 63) is a 400 input-validation error — intentionally left raw since it
is a genuine client input problem, not an upstream rejection.
Tests (upstream_error_test.go): translates the opaque rejection
case-insensitively, passes other errors through unchanged, nil-safe.
build/vet/test green: proxy 6.241s, pool 0.489s, config 0.706s, auth 2.234s.
float64 + omitempty treated an explicit temperature of 0 as a zero value and stripped it, silently reverting deterministic-decoding clients to the default temperature. Switch Temperature to *float64 in InferenceConfig, ClaudeRequest, and OpenAIRequest, and change the translator guards from `> 0` to `!= nil` so an explicitly-set 0 propagates to Kiro while an unset temperature is still omitted.
…kups + smoke output /test.py holds a live sk- API key used for local smoke runs and must never be committed. *~ covers editor backups like kiro-go.exe~ (not matched by the existing *.exe rule). .smoke/ is local dispatch/CLI smoke-test output.
…o port)
Ports the two remaining cache-semantics items from kiro-rs
cache_metering.rs (zero-dep). Both are billing-number-sensitive, so
behavior is locked by TDD tests mirroring the Rust cases.
(3) Structural skip of leading system blocks before the first cache_control
(mirrors extract_segments:444-462). Claude Code injects a per-turn DYNAMIC
block at system[0] WITHOUT cache_control, followed by the stable cached
block(s). Fingerprinting from that volatile head drifts every downstream
prefix fingerprint across turns, collapsing cross-turn hits to zero.
appendSystemCacheBlocks []interface{} branch now computes skipUntil =
index of the first system block carrying cache_control (unwrap_or(0)
when none) and iterates v[skipUntil:]. system_index is a stripped
position key so the index shift does not affect fingerprints.
isAnthropicBillingHeaderBlock string-match drop stays as a backstop for a
non-leading billing block (Rust has no such guard — strictly an
improvement).
(4) Auto-prefix breakpoints without explicit cache_control (mirrors
prefix_chain_works_without_any_cache_control + detect_max_ttl). A request
with zero cache_control markers now still produces message-end breakpoints
at the default TTL, reproducing Anthropic's automatic prompt caching (a
stable history prefix hits across turns even though the client never set
cache_control). Removed the claudeRequestHasCacheControl fast-path
nil-return in BuildClaudeProfile; activeTTL is now seeded with
detectMaxTTL(req) (scans system+message blocks, max ttl 1h>5m, default
5m) so implicit message-end breakpoints fire from the first request.
Tests: TestPromptCacheStructuralSkipOfLeadingDynamicSystemBlock +
TestBuildClaudeProfileAutoPrefixWithoutCacheControl (replaces
...ReturnsNilWithoutCacheControl). RED→GREEN; go build/vet
./proxy/... ./pool/... ./config/... clean. The Windows TempDir cleanup
flake (TestClaudeStreamEmitsKeepaliveDuringUpstreamStall) is pre-existing
and reproduced on main, not a regression.
ngh1105
force-pushed
the
fix/temperature-zero-omitempty-drop
branch
from
July 13, 2026 15:30
3e475a6 to
ade1621
Compare
Three api_key robustness fixes surfaced by the refresh-ban investigation
(a valid-feeling api_key kept getting BANNED):
1. Region unification (REST <-> chat path inconsistency):
kiroRegionForProfile now branches on IsApiKeyCredential ->
EffectiveApiRegion(), the same fallback chain the chat path
(CallKiroAPI) uses. Previously the REST path (getUsageLimits,
ListAvailableModels, overage) saw only account.Region, so ApiRegion +
the global region were silently ignored and REST calls targeted a
different region than chat. The chat path in kiro.go is simplified back
to a single regionalizeURLForProfile call (kiroRegionForProfile now
encapsulates the api_key branch); this also fixes the nonexistent
codewhisperer.{region} host for non-us-east-1 api_key accounts.
2. Ban guard: RefreshAccountInfo no longer BANs an api_key account on a
usage-limit 403. api_key accounts authenticate directly and have no
profile-ARN cross-region probe, so a usage-limit 403 is not
authoritative (could be region/config). The chat path
(handleAccountFailure) is the source of truth and bans a
genuinely-invalid key on a real request. Without this guard, the region
mismatch in (1) permanently disabled a valid key over a spurious 403.
3. Log masking: accountEmailForLog falls back to maskKey(KiroApiKey) when
an api_key account has no email yet (email is only populated after the
first successful refresh). All RefreshAccountInfo / model-cache / import
log sites reachable for api_key accounts now use accountEmailForLog
instead of the bare (empty) account.Email, so refresh/ban/cache lines
are no longer blind ("for : ...").
Tests: TestRegionalizeURLApiKeyUsesEffectiveApiRegion,
TestRefreshAccountInfoDoesNotBanApiKeyOnUsageLimit403,
TestAccountEmailForLogMasksApiKey. 3 full-suite failures (env-proxy cache,
Windows TempDir cleanup race) are environmental and pass in isolation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ports
api_keyauth-method support and bulk API-key import fromhian699/Kiro-Go(0dccff1) into this repo. A Kiro API key is used directly as the bearer token (Authorization: Bearer <key>+tokentype: API_KEY), needs no OAuth refresh token and no profile ARN. The key is mirrored into the existingAccessTokenfield for pool/dispatch/metrics compatibility.Design spec:
docs/superpowers/specs/2026-07-13-api-key-credential-import-design.mdImplementation plan:
docs/superpowers/plans/2026-07-13-api-key-credential-import.mdWhat's included (10 TDD tasks)
config:Account.{KiroApiKey,AuthRegion,ApiRegion}+IsApiKeyCredential+Effective{Auth,Api}Region+MaxPayloadBytes13344fdtranslator: runtimeconfig.GetMaxPayloadBytes()cap (replaces hard-coded900*1024)b3eb835kiro_headers: api_key bearer branch (Bearer <KiroApiKey>+tokentype: API_KEY),external_idpbranch preserved661ae32kiro_api:ResolveProfileArnskips api_key accounts (no HTTP probe)00b39e3kiro:CallKiroAPIregion rebuild guarded byIsApiKeyCredential()— OAuth keepsregionalizeURLForProfile758f4e5handler:apiAddAccountapi_key branch,fetchAndCacheAccountModelsgate,maxPayloadBytessettings GET/POST25170d4handler:apiImportCredentialsapi_key branch (best-effortRefreshAccountInfo, no OAuth refresh)a5a439eapikey_batch.go(NEW) +POST /auth/apikeys-batchrouted683112web:apikey/apikeybatchmethod cards + modals + import handlers +maxPayloadBytesselector + locale keys (en/zh)3e475a63f33937Scope
In:
api_keyauth method, region-aware endpoints,MaxPayloadBytescap, bulk API-key import, web UI for all of the above.Out (separate features, deliberately not ported): bulk proxy import (
proxy_import.go,/proxy/import), sticky routing / proxy pool failover.Spec-vs-source divergences (intentional)
0dccff1appliesEffectiveApiRegion()rebuild to ALL accounts. This PR guards it withIsApiKeyCredential()so OAuth accounts keepregionalizeURLForProfile(they derive region from profile ARN).ResolveProfileArnentry — one point covers bothCallKiroAPIandensureRestProfileArn.Testing
go test ./config/andgo test ./proxy/pass. 3 full-suite tests (TestBuildKiroTransportFallsBackToEnvironmentProxy,TestResponsesContinuationKeepsNewInstructions,TestResponsesStreamSSE) fail under parallel load but pass in isolation — environmental flakiness (env-proxy cache + Windows TempDir cleanup race), unrelated to these changes./v1/messages→ HTTP 200 with real model output.Backward compatibility
AccessToken = KiroApiKey, so all code readingAccessTokenworks unchanged.ExpiresAt=0+RefreshToken=""; all refresh call sites already guard on those, so no refresh round-trip ever fires (pinned by the Task 10 characterization test).Note
This branch also carries 3 predecessor commits (
93c79e5temperature fix,6213fb9gitignore,f04e8b1cache) that were already on the branch before the api-key work. Happy to split them into a separate PR if preferred.Files: 21 changed, +3040 / −34.