Skip to content

fix(sdk-kotlin): keep an explicit API key header ahead of the configured key - #2375

Merged
Pangjiping merged 1 commit into
opensandbox-group:mainfrom
xxwtiancai:fix/kotlin-apikey-header-precedence
Oct 25, 2026
Merged

Pangjiping merged 1 commit into
opensandbox-group:mainfrom
xxwtiancai:fix/kotlin-apikey-header-precedence

Conversation

@xxwtiancai

Copy link
Copy Markdown
Contributor

Summary

An explicit OPEN-SANDBOX-API-KEY supplied through ConnectionConfig.headers should take precedence over the configured apiKey and its environment fallback — the same contract #2348 established for the JavaScript SDK and #2372 carries to C#. The Kotlin authenticated client violated it:

  • HttpClientProvider's AuthenticationInterceptor runs after ExtraHeadersInterceptor (which applies user-supplied headers) and unconditionally replaces the API key header — OkHttp matches header names case-insensitively — so a caller's explicitly supplied credential was always overwritten with the configured key, whatever casing was used.
  • With no key configured, the interceptor still attached an empty OPEN-SANDBOX-API-KEY header value to every authenticated request, unlike the JS/C# SDKs which only attach the header when a key exists. (LifecycleMetricsReporter already handled both aspects correctly.)

The fix skips the injection when the request already carries an API key header in any casing (the same guard the neighboring ClientIpInterceptor uses — its comment states the intent that a user-supplied value is preserved), and when the key is blank. Default injection is unchanged when the header is absent and a key is configured.

Testing

  • Unit tests: 3 new behavioral regression cases fail on the base commit (explicit canonical-casing header, explicit lowercase header, and no header when unconfigured) and pass after the fix, plus one control case for default injection. New ApiKeyHeaderTest uses MockWebServer end-to-end against authenticatedClient, following the existing ClientIpTest pattern.
  • ./gradlew spotlessCheck :sandbox:test :code-interpreter:test — 531 tests, 0 failures (JDK 8 toolchain provisioned locally for :code-interpreter).
  • pnpm docs:build was not run (no pnpm available locally); the docs change is one prose paragraph in docs/sdks/kotlin.md.
  • Live sandbox / cross-language integration tests were not run.

Breaking Changes

  • None: an explicitly supplied header is now honored per HTTP's case-insensitive header semantics; the only other behavior change is that authenticated requests no longer carry an empty API key header when no key is configured.

Checklist

  • Clearly described motivation
  • Added/updated docs (a short note in docs/sdks/kotlin.md, mirroring the JavaScript/C# SDK notes)
  • Added regression tests
  • Security impact considered: preserves the caller's selected credential instead of silently substituting it; no new credential exposure
  • Backward compatibility considered

Prepared with AI assistance. Checked the open PR queue for in-flight Kotlin header-precedence work before submitting.

…red key

An explicit OPEN-SANDBOX-API-KEY supplied through ConnectionConfig
headers should take precedence over the configured apiKey and its
environment fallback, matching the JavaScript (opensandbox-group#2348) and C# header
precedence contract. The authenticated client violated it:
AuthenticationInterceptor runs after ExtraHeadersInterceptor and
unconditionally replaces the API key header — OkHttp matches names
case-insensitively — so the caller's explicitly supplied credential
was always overwritten with the configured key.

Skip the injection when the request already carries an API key header
in any casing, mirroring the neighboring ClientIpInterceptor guard,
and also skip it when no key is configured so authenticated requests
no longer send an empty header value.

Adds four MockWebServer regression tests: configured key carried,
explicit canonical and lowercase headers preserved exactly once, and
no header when unconfigured. Three fail on the base commit and pass
after the fix; the full :sandbox and :code-interpreter suites (531
tests) and spotlessCheck pass.
Copilot AI balanced review requested due to automatic review settings October 23, 2026 18:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@github-actions github-actions Bot added documentation Improvements or additions to documentation sdk/java size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 23, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-23T18:20:31.247335Z bedbaab PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@Pangjiping Pangjiping left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Pangjiping
Pangjiping merged commit 5050784 into opensandbox-group:main Oct 25, 2026
60 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation sdk/java size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants