Skip to content

[codex] fix(copilot): preserve BYOK bearer auth through proxy#99955

Merged
clawsweeper[bot] merged 1 commit into
mainfrom
feature/copilot-byok-bearer-proxy
Jul 4, 2026
Merged

[codex] fix(copilot): preserve BYOK bearer auth through proxy#99955
clawsweeper[bot] merged 1 commit into
mainfrom
feature/copilot-byok-bearer-proxy

Conversation

@hxy91819

@hxy91819 hxy91819 commented Jul 4, 2026

Copy link
Copy Markdown
Member

What Problem This Solves

Fixes an issue where users routing OpenAI-compatible BYOK providers through the GitHub Copilot harness could hit upstream 401s when the Copilot SDK/runtime omitted the resolved bearer Authorization header before the loopback proxy forwarded the request.

This was reproduced with Tencent TokenPlan/TokenHub-shaped Chat Completions providers: direct OpenClaw provider calls succeeded, while the same provider/model through Copilot BYOK could fail at the upstream auth hop.

AI-assisted: yes.

Why This Change Was Made

The Copilot BYOK proxy now carries OpenClaw's resolved bearer auth intent to the final SSRF-guarded upstream hop, but only for nonce-protected proxy requests and only when the SDK did not already send authorization. Azure SDK nonce-less /openai/... proxy paths remain excluded so discovering the loopback port cannot turn the proxy into a bearer-token relay.

The package install proof also confirms the existing @github/copilot-sdk production dependency resolves the Copilot CLI package transitively, so this PR does not change the plugin dependency graph.

User Impact

Users can run Tencent TokenPlan/TokenHub and Ollama Cloud OpenAI-compatible BYOK models through the Copilot harness without losing bearer auth at the proxy hop. The PR does not change the plugin dependency graph; the existing SDK dependency continues to resolve the Copilot CLI package in production installs.

Evidence

Focused local tests:

  • node scripts/run-vitest.mjs extensions/copilot/src/byok-proxy.test.ts extensions/copilot/src/provider-bridge.test.ts test/scripts/plugin-package-dependencies.test.ts test/plugin-npm-package-manifest.test.ts
    • Passed: 4 files / 31 tests.
  • pnpm exec oxfmt --check --threads=1 extensions/copilot/src/byok-proxy.ts extensions/copilot/src/byok-proxy.test.ts extensions/copilot/src/provider-bridge.test.ts my_docs/04-cases/2026-07-04-copilot-byok-tencent-harness-bug/00-case.md
    • Passed.
  • git diff --check
    • Passed.
  • pnpm deadcode:dependencies
    • Passed after keeping @github/copilot as an SDK transitive dependency instead of a direct plugin dependency.
  • .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main
    • Passed: autoreview clean: no accepted/actionable findings reported.

Package/install proof:

  • npm pack --pack-destination /tmp for extensions/copilot.
  • Inspected the tarball:
    • package/package.json keeps @github/copilot-sdk: 1.0.0-beta.9 as the direct runtime dependency.
    • package/dist/... contains the proxy bearer fix symbols upstreamBearerAuthorization and isNonceProtectedProxyRequest.
  • Installed the tarball in a temporary npm project with npm install --package-lock-only --omit=dev --ignore-scripts <tgz> and confirmed node_modules/@github/copilot still resolves from @github/copilot-sdk's production dependency graph without adding a direct plugin dependency.

Live openclaw2 proof with real provider keys, redacted/no secret values printed:

  • Installed the final package through the trusted official path:
    • openclaw plugins install npm-pack:/tmp/openclaw-copilot-byok-final-20260704.tgz --force
    • Restarted gateway.
    • Confirmed loaded dist contains upstreamBearerAuthorization.
  • Gateway status after final install:
    • OpenClaw 2026.7.4 (80b40a8).
    • Gateway loopback 127.0.0.1:28443.
    • Connectivity probe: ok.

Final live model probes, all using:

openclaw infer model run --gateway --model <provider/model> \
  --prompt "Reply with exactly OK-LIVE-PROBE" --thinking off --json

Results:

  • custom-tokenplan-hy3/hy3 through Copilot BYOK: returned OK-LIVE-PROBE.
  • custom-tokenhub-hy3/hy3 through Copilot BYOK: returned OK-LIVE-PROBE.
  • ollama-cloud/deepseek-v4-flash direct gateway path: returned OK-LIVE-PROBE.
  • Temporary custom-ollama-cloud-copilot/deepseek-v4-flash through Copilot BYOK: returned OK-LIVE-PROBE.

Cleanup proof:

  • The temporary Ollama Copilot BYOK provider and agents.defaults.models["custom-ollama-cloud-copilot/deepseek-v4-flash"] allowlist entry were removed after the live probe.
  • openclaw config validate passed after cleanup.
  • Gateway was restarted and remained healthy.
  • Confirmed hasTempProvider=false and hasTempAllowlist=false.

Known unrelated environment noise on openclaw2:

  • Plugin version drift: 6 active official plugins not on gateway 2026.7.4.
  • Existing legacy state migration warning for memory index conflicts.
  • Neither affected the package install, gateway connectivity, config validation, or live provider probes above.

@clawsweeper

clawsweeper Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 4, 2026, 9:42 AM ET / 13:42 UTC.

Summary
The branch adds nonce-gated BYOK bearer Authorization injection in the Copilot proxy when SDK requests omit it, plus regression tests for OpenAI-compatible bearer and Azure nonce-less paths.

PR surface: Source +40, Tests +100. Total +140 across 3 files.

Reproducibility: yes. Source inspection shows current main resolves authHeader into bearerToken, then the proxy forwards only incoming SDK headers, so a missing SDK Authorization header drops bearer auth before the guarded upstream fetch; the PR body also includes live after-fix provider probes.

Review metrics: 2 noteworthy metrics.

  • Credential Relay Path: 1 added. The new nonce-gated bearer injection path is auth-provider and security-boundary sensitive even though the patch is small.
  • Package Graph Changes: 0 package, lockfile, or shrinkwrap files changed. The current head no longer changes the plugin dependency graph, so review can focus on the credential relay behavior.

Stored data model
Persistent data-model change detected: serialized state: extensions/copilot/src/byok-proxy.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Get Copilot/runtime or security owner sign-off on the nonce-gated bearer forwarding behavior before merge.

Risk before merge

  • [P1] Merging changes how resolved BYOK bearer credentials are relayed through a loopback proxy; a Copilot/runtime or security owner should confirm the nonce gate, fixed upstream target scoping, and no-overwrite behavior before merge.

Maintainer options:

  1. Confirm Credential Boundary (recommended)
    Have the Copilot/runtime or security owner confirm that nonce-gated bearer forwarding to the fixed SSRF-guarded target is intentional before merge.
  2. Tighten The Relay Contract
    If owner review wants a narrower boundary, adjust the proxy guard or tests before merge without changing provider selection.
  3. Pause If Proxy Synthesis Is Rejected
    If maintainers do not want the proxy to synthesize bearer credentials at all, pause this PR and choose a different SDK/provider integration path.

Next step before merge

  • [P2] Manual review is required because this protected maintainer PR has a human-review label and changes BYOK bearer relay behavior across an auth/security boundary; there is no narrow automated repair to queue.

Security
Cleared: No concrete security defect was found in the diff, but the intentional bearer-forwarding boundary remains a maintainer-visible merge risk.

Review details

Best possible solution:

Land the focused nonce-gated proxy fix after Copilot/runtime or security owner sign-off confirms the credential boundary and exact-head proof remains valid.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection shows current main resolves authHeader into bearerToken, then the proxy forwards only incoming SDK headers, so a missing SDK Authorization header drops bearer auth before the guarded upstream fetch; the PR body also includes live after-fix provider probes.

Is this the best way to solve the issue?

Yes, pending owner sign-off on the sensitive boundary. Fixing the proxy's final guarded hop is narrower than forking SDK behavior or duplicating provider auth logic, and the tests cover both the missing-header and Azure nonce-less paths.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 787f3a2ce557.

Label changes

Label changes:

  • add status: 🚀 automerge armed: This PR is in ClawSweeper's automerge lane. Sufficient (live_output): The PR body includes after-fix live OpenClaw gateway probes through real BYOK providers plus npm-pack/install proof, with secret values redacted.
  • remove status: 👀 ready for maintainer look: Current PR status label is status: 🚀 automerge armed.

Label justifications:

  • P1: The PR fixes a broken Copilot BYOK provider workflow that can produce upstream 401s for real configured providers.
  • merge-risk: 🚨 auth-provider: Merging changes how resolved BYOK bearer credentials reach upstream Copilot provider requests.
  • merge-risk: 🚨 security-boundary: Merging changes secret-bearing loopback proxy behavior, so the nonce and target-scoping boundary needs maintainer confirmation.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 🚀 automerge armed: This PR is in ClawSweeper's automerge lane. Sufficient (live_output): The PR body includes after-fix live OpenClaw gateway probes through real BYOK providers plus npm-pack/install proof, with secret values redacted.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live OpenClaw gateway probes through real BYOK providers plus npm-pack/install proof, with secret values redacted.
Evidence reviewed

PR surface:

Source +40, Tests +100. Total +140 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 1 44 4 +40
Tests 2 100 0 +100
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 144 4 +140

What I checked:

  • Repository policy applied: Root AGENTS.md and the scoped extensions policy were read; plugin boundary, dependency-contract, auth-provider, and security-boundary guidance affected this review. (AGENTS.md:1, 787f3a2ce557)
  • Current main drops missing SDK bearer headers: On current main, the BYOK proxy sends only normalized incoming request headers to fetchWithSsrFGuard, so an SDK request that omits Authorization does not recover the already-resolved bearer token. (extensions/copilot/src/byok-proxy.ts:111, 787f3a2ce557)
  • Bearer intent exists before proxying: Current main maps authHeader plus a resolved API key to ProviderConfig.bearerToken, so the missing behavior is at the proxy's final guarded upstream hop rather than provider selection. (extensions/copilot/src/provider-bridge.ts:108, 787f3a2ce557)
  • Branch fix is nonce-gated and non-overwriting: The PR computes upstreamBearerAuthorization, passes it only to nonce-protected proxy requests, and injects it only when the SDK did not already provide an Authorization header. (extensions/copilot/src/byok-proxy.ts:120, ef753a992939)
  • Regression tests cover the auth split: The added tests cover injecting resolved bearer auth when Authorization is missing and not injecting bearer auth on nonce-less Azure SDK paths. (extensions/copilot/src/byok-proxy.test.ts:85, ef753a992939)
  • Provider mapping test covers Chat Completions bearer shape: The PR adds a provider bridge test proving OpenAI Chat Completions BYOK maps authHeader to bearerToken with provider-local model ids. (extensions/copilot/src/provider-bridge.test.ts:76, ef753a992939)

Likely related people:

  • vincentkoc: Authored the merged BYOK provider parity change that added the Copilot BYOK provider/proxy surface now being repaired. (role: recent feature owner; confidence: high; commits: bfffc77bfc86, d3019e612705, 21d67b168a8b; files: extensions/copilot/src/byok-proxy.ts, extensions/copilot/src/provider-bridge.ts, extensions/copilot/src/attempt.ts)
  • obviyus: Recently changed adjacent Copilot attempt header behavior on the same provider/request-header surface. (role: recent adjacent contributor; confidence: medium; commits: 3380d3b34505; files: extensions/copilot/src/attempt.ts)
  • steipete: Recent history includes Copilot runtime externalization and install/package boundary work relevant to the dependency proof for this PR. (role: adjacent package/runtime contributor; confidence: medium; commits: 99b24a80fbe9; files: extensions/copilot/package.json, src/commands/copilot-sdk-install.ts)
  • Ramrajprabu: Authored the merged change that introduced the GitHub Copilot agent runtime plugin surface this BYOK repair builds on. (role: original feature introducer; confidence: medium; commits: f3cfd752d330; files: extensions/copilot, src/commands/copilot-sdk-install.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (5 earlier review cycles)
  • reviewed 2026-07-04T12:47:55.960Z sha 06bcb68 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-04T12:54:22.712Z sha 06bcb68 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-04T13:00:18.500Z sha 06bcb68 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-04T13:31:48.590Z sha ef753a9 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-04T13:37:21.572Z sha ef753a9 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels Jul 4, 2026
@hxy91819
hxy91819 marked this pull request as ready for review July 4, 2026 12:52
@hxy91819
hxy91819 requested a review from a team as a code owner July 4, 2026 12:52
@chatgpt-codex-connector

Copy link
Copy Markdown
Contributor

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@github-actions github-actions Bot added the dependencies-changed PR changes dependency-related files label Jul 4, 2026
@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Dependency graph guard cleared

This PR no longer has blocked dependency graph changes. A future dependency graph change requires a fresh /allow-dependencies-change comment after the guard blocks that new head SHA.

  • Current SHA: ef753a992939fe3e06a6e253dd052eb812a85800

@hxy91819
hxy91819 force-pushed the feature/copilot-byok-bearer-proxy branch from 06bcb68 to ef753a9 Compare July 4, 2026 13:23
@github-actions github-actions Bot removed the dependencies-changed PR changes dependency-related files label Jul 4, 2026
@hxy91819

hxy91819 commented Jul 4, 2026

Copy link
Copy Markdown
Member Author

@clawsweeper automerge

@clawsweeper

clawsweeper Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

🦞✅
Maintainer-approved ClawSweeper automerge is complete.

Approver: hxy91819
Head: ef753a992939
Merge status: merged by ClawSweeper automerge
Merged at: 2026-07-04T13:48:28Z
Merge commit: 4287d2629fb3

What merged:

  • Merged [codex] fix(copilot): preserve BYOK bearer auth through proxy after ClawSweeper review.

Automerge notes:

  • No ClawSweeper repair was needed after automerge opt-in.

The automerge loop is complete.

Automerge progress:

  • 2026-07-04 13:37:33 UTC review queued ef753a992939 (queued)
  • 2026-07-04 13:48:29 UTC merged ef753a992939 (merged by ClawSweeper automerge)

@clawsweeper clawsweeper Bot added clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge clawsweeper:human-review Needs maintainer review before ClawSweeper can continue labels Jul 4, 2026
@clawsweeper

clawsweeper Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

🦞✅
ClawSweeper is pausing this repair loop for human review.

Source: clawsweeper[bot]
Reason: - [P2] Manual review is required because this protected maintainer PR has a human-review label and changes BYOK bearer relay behavior across an auth/security boundary; there is no narrow automated repair to queue.; Cleared: No concrete security defect was found in the diff, but the intentional bearer-forwarding boundary remains a maintainer-visible merge risk. (sha=ef753a992939fe3e06a6e253dd052eb812a85800)

Why human review is needed:
This item touches a maintainer-protected decision point. ClawSweeper is keeping the item open so a maintainer can own the final validation choice.

What the maintainer can do as a next step:
If the maintainer accepts the current risk and wants ClawSweeper to continue merge gates, comment @clawsweeper approve. If the flagged decision needs more evidence first, add the missing proof or resolve the maintainer-owned blocker, then comment @clawsweeper automerge. If the maintainer wants to own landing outside automation, merge manually after repository gates pass.

I added clawsweeper:human-review and left the final call with a maintainer.

@clawsweeper clawsweeper Bot added status: 🚀 automerge armed This PR is in ClawSweeper's automerge lane. and removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 4, 2026
@hxy91819

hxy91819 commented Jul 4, 2026

Copy link
Copy Markdown
Member Author

@clawsweeper approve

@clawsweeper clawsweeper Bot removed the clawsweeper:human-review Needs maintainer review before ClawSweeper can continue label Jul 4, 2026
@clawsweeper
clawsweeper Bot merged commit 4287d26 into main Jul 4, 2026
124 of 126 checks passed
@clawsweeper
clawsweeper Bot deleted the feature/copilot-byok-bearer-proxy branch July 4, 2026 13:48
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 5, 2026
…aw#99955)

Summary:
- Merged [codex] fix(copilot): preserve BYOK bearer auth through proxy after ClawSweeper review.

Automerge notes:
- No ClawSweeper repair was needed after automerge opt-in.

Validation:
- ClawSweeper review passed for head ef753a9.
- Required merge gates passed before the squash merge.

Prepared head SHA: ef753a9
Review: openclaw#99955 (comment)

Co-authored-by: Mason Huang <[email protected]>
Approved-by: hxy91819
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge extensions: copilot maintainer Maintainer-authored PR merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 🚀 automerge armed This PR is in ClawSweeper's automerge lane.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant