Support env-backed CLI server headers - #1323
Closed
ramarivera wants to merge 1 commit into
Closed
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for server-profile HTTP headers whose values are resolved from environment variables, primarily to enable Cloudflare Access service token headers for executor CLI usage against Access-protected self-hosted deployments. The SDK gains header configuration and resolution utilities, and the CLI applies those resolved headers across device-login discovery and API requests while scoping header injection to the Executor server origin for OAuth endpoints.
Changes:
- Extend
ExecutorServerConnectionto include optional env-backed header mappings, plus resolution helpers and a typed resolution error in@executor-js/sdk. - Add
executor server add --header-env Header-Name=ENV_VAR, persist mappings in server profiles, and include header counts inserver list. - Apply resolved headers to CLI login discovery, device-code requests, token polling, token refresh, and typed API client requests, with same-origin scoping for OAuth endpoints.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/core/sdk/src/shared.ts | Re-export new server-header types and resolution helpers from the shared SDK surface. |
| packages/core/sdk/src/server-connection.ts | Add header mapping types, normalization, env resolution, and JSON schema support for persisted connections/manifests. |
| packages/core/sdk/src/server-connection.test.ts | Add unit coverage for header normalization, env resolution, and manifest round-trip persistence. |
| packages/core/sdk/src/index.ts | Export the new header types and resolution helpers from the SDK entrypoint. |
| e2e/local/server-profile-header-env.test.ts | E2E scenario verifying header-env persistence without persisting or printing secret values. |
| apps/cli/src/server-profile.ts | Extend persisted server profile schema to include optional header mappings. |
| apps/cli/src/server-profile.test.ts | Verify CLI profile persistence and parsing includes header-env mappings. |
| apps/cli/src/main.ts | Add --header-env CLI surface, apply resolved headers to login and API client calls, and preserve mappings across login/logout/refresh. |
| apps/cli/src/device-login.ts | Add request options and same-origin header scoping for device flow discovery, device-code, token polling, and refresh. |
| apps/cli/src/device-login.test.ts | Add tests asserting headers are sent for discovery and only same-origin device endpoints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
752
to
+767
| @@ -759,8 +762,19 @@ const refreshOAuthConnection = ( | |||
| const { refreshToken, tokenEndpoint, clientId } = auth; | |||
| if (!refreshToken || !tokenEndpoint || !clientId) return connection; | |||
|
|
|||
| const headers = yield* resolveExecutorServerConfiguredHeaders(connection, process.env).pipe( | |||
| Effect.mapError(toError), | |||
| ); | |||
Collaborator
|
Superseded by #1539, which replayed this branch's commit (preserving authorship) onto current main and has now merged. Thanks @ramarivera for the original implementation. |
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
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
executor server add --header-env Header-Name=ENV_VARfor Cloudflare Access service token style deployments.Fixes #1322
Testing
bun run format:checkbun run lintbun run typecheckbun run --cwd packages/core/sdk test src/server-connection.test.tsbun run --cwd apps/cli test src/server-profile.test.ts src/device-login.test.tsbun run --cwd e2e vitest run --project local local/server-profile-header-env.test.tsbun run testwas also attempted. It failed in the existing@executor-js/host-mcpstdio integration because this machine already has local processes listening onlocalhost:4788; the test connected to that ambient daemon and gotUnauthorized. The changed SDK and CLI tests passed, and full typecheck passed.Live smoke
Against
https://executor.ai.roxasroot.net/api/auth/cli-login:/api/auth/cli-login.