fix: Fix incorrect gating of ACCESS_TOKEN for template commands#1315
fix: Fix incorrect gating of ACCESS_TOKEN for template commands#1315
Conversation
🦋 Changeset detectedLatest commit: f9465f0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Package ArtifactsBuilt from 8d1fc52. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.19.6-wj-fix-api-key.0.tgzCLI ( npm install ./e2b-cli-2.10.2-wj-fix-api-key.0.tgzPython SDK ( pip install ./e2b-2.21.0+wj.fix.api.key-py3-none-any.whl |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9465f0e0e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const resolvedApiKey = resolveAPIKey() | ||
| const resolvedAccessToken = resolveAccessToken() |
There was a problem hiding this comment.
When E2B_API_KEY is set on a machine that also has a saved ~/.e2b/config.json, this still calls resolveAccessToken(), loads the saved access token, and returns both credentials. Callers such as template build then prefer accessToken for Docker auth, while template create sends both headers, so a stale or wrong local token can override the explicit API key and make the new API-key-only path fail or use the wrong account. Stop resolving the fallback token once an API key has been chosen, or otherwise preserve a single credential source.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| try { | ||
| child_process.execSync( | ||
| `echo "${accessToken}" | docker login docker.${connectionConfig.domain} -u _e2b_access_token --password-stdin`, | ||
| `echo "${dockerAuthToken}" | docker login docker.${connectionConfig.domain} -u _e2b_access_token --password-stdin`, |
There was a problem hiding this comment.
🔒 Agentic Security Review
Severity: MEDIUM
The Docker login command now interpolates dockerAuthToken directly into a shell command string (echo "${dockerAuthToken}" | ...). Because this value can now be E2B_API_KEY, the secret may be exposed through process inspection or command logging on shared hosts/CI runners while the command executes.
Impact: Team-scoped API credentials can leak to other local processes/users, enabling unauthorized access to protected template and registry operations.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| if (imageUriMask == undefined) { | ||
| try { | ||
| child_process.execSync( | ||
| `echo "${accessToken}" | docker login docker.${connectionConfig.domain} -u _e2b_access_token --password-stdin`, | ||
| `echo "${dockerAuthToken}" | docker login docker.${connectionConfig.domain} -u _e2b_access_token --password-stdin`, | ||
| { | ||
| stdio: 'inherit', | ||
| cwd: root, |
There was a problem hiding this comment.
🔴 After this PR, dockerAuthToken (build.ts:250) can be an API key when only E2B_API_KEY is set, but docker login (build.ts:381) and buildWithProxy.ts (the _e2b_access_token:... Basic-auth header at L42 and the ?account=_e2b_access_token query param at L127) still send the literal username _e2b_access_token. The README now explicitly markets E2B_API_KEY=e2b_... e2b template build as supported, so this is the exact path the PR is enabling. Please confirm server-side that the registry / token endpoint accepts API keys under _e2b_access_token (or make the username conditional, e.g. _e2b_api_key when sending an API key).
Extended reasoning...
What changed
Before this PR, build.ts called ensureAccessToken() so the credential passed to docker login and to buildWithProxy was always an access token. After the PR, that line is replaced with:
const credentials = ensureAPIKeyOrAccessToken()
const dockerAuthToken = credentials.accessToken ?? credentials.apiKey!So dockerAuthToken is the access token if present, otherwise the API key. dockerAuthToken is then plumbed into three places that were originally written assuming an access token:
build.ts:381–echo "${dockerAuthToken}" | docker login docker.${domain} -u _e2b_access_token --password-stdinbuildWithProxy.ts:42–Buffer.from(\_e2b_access_token:${dockerAuthToken}`).toString('base64')used as theBasicauth header against/v2/token` and as fallback in the proxy.buildWithProxy.ts:127–fetch(\https://docker.${domain}/v2/token?account=_e2b_access_token&scope=...\`, ...)– theaccount` query param is also hardcoded.
Why this is worth flagging
The whole point of the PR (per the changeset and the README diff) is to let users run E2B_API_KEY=e2b_... e2b template build. That command is the v1 build path which performs docker login against the E2B docker registry. If the registry/token endpoint only honours _e2b_access_token for actual access tokens, API-key-only users will fail at docker login (or the proxy token exchange) — silently nullifying the new flow that this PR is named after.
Step-by-step proof of the new code path
- User sets only
E2B_API_KEY=e2b_...(noE2B_ACCESS_TOKEN, no logged-in config), as the new README example instructs. ensureAPIKeyOrAccessToken()(api.ts) returns{ apiKey: 'e2b_...', accessToken: undefined }.build.ts:250resolvesdockerAuthToken = undefined ?? 'e2b_...'→ the API key.build.ts:381runsecho "e2b_..." | docker login docker.<domain> -u _e2b_access_token --password-stdin. The username sent to the registry is_e2b_access_tokenbut the password is an API key.- If
docker pushfalls back to the proxy,buildWithProxy.ts:42buildsBasic base64(_e2b_access_token:e2b_...)andbuildWithProxy.ts:127requests/v2/token?account=_e2b_access_token&scope=...with that header.
Addressing the refutation
A refuting verifier argued that _e2b_access_token is a Docker-registry convention like AWS ECR's AWS or GCR's _json_key, where the username is opaque and the password drives auth. That is plausible and may well be true here — but it's exactly what we cannot verify from the CLI source. The bug submitters and confirming verifiers are also explicit that this hinges on server behaviour: "Worth verifying server-side". The action being requested is small and concrete: the PR author can either (a) confirm in the PR description that the registry treats the username as opaque so an API key under _e2b_access_token works, or (b) make the username conditional (e.g. _e2b_api_key when credentials.accessToken is undefined) so the username matches the credential type and is robust to future server-side changes. Even if the server currently accepts both, the hardcoded label is now semantically wrong post-PR.
| export function ensureAPIKeyOrAccessToken() { | ||
| const resolvedApiKey = resolveAPIKey() | ||
| const resolvedAccessToken = resolveAccessToken() | ||
|
|
||
| if (resolvedApiKey || resolvedAccessToken) { | ||
| return { apiKey: resolvedApiKey, accessToken: resolvedAccessToken } | ||
| } | ||
|
|
||
| console.error(authErrorBox('E2B_API_KEY')) | ||
| process.exit(1) | ||
| } | ||
|
|
There was a problem hiding this comment.
🟡 When neither E2B_API_KEY nor E2B_ACCESS_TOKEN is set, ensureAPIKeyOrAccessToken() calls authErrorBox('E2B_API_KEY'), which only mentions the API key env var and the API-key dashboard URL. Since this function explicitly accepts either credential (and the README updated in this PR tells users they may use either), the error message is misleading for users who authenticate with an access token — consider extending authErrorBox to mention both options here.
Extended reasoning...
What the bug is
In packages/cli/src/api.ts (lines 98–109), the new ensureAPIKeyOrAccessToken() function accepts either E2B_API_KEY or E2B_ACCESS_TOKEN as valid authentication. However, when both are missing it falls through to console.error(authErrorBox('E2B_API_KEY')). The authErrorBox helper switches on keyName and only supports two single-key variants:
'E2B_API_KEY'→ produces "set the E2B_API_KEY environment variable. Visit https://e2b.dev/dashboard?tab=keys to get the API key."'E2B_ACCESS_TOKEN'→ produces the analogous message for the access token.
So a user who normally authenticates with E2B_ACCESS_TOKEN and forgets/typos it in CI will be told to set the wrong env var and pointed to the wrong dashboard tab.
Why the existing code doesn't prevent it
authErrorBox has no combined mode — its switch only knows about the two individual key names. The new ensureAPIKeyOrAccessToken reuses the API-key branch verbatim, which conflicts with this PR's own intent of accepting either credential. The README was updated in the same PR to advertise both env vars (packages/cli/README.md: "provide E2B_API_KEY or E2B_ACCESS_TOKEN as an environment variable"), so the error path is now inconsistent with the documented behavior.
Step-by-step proof
- User runs
e2b template listin CI with no~/.e2b/config.jsonand an unset/typo'dE2B_ACCESS_TOKEN(and noE2B_API_KEY). list.tscallsensureAPIKeyOrAccessToken().resolveAPIKey()returnsundefined(no env var, no user config).resolveAccessToken()returnsundefinedfor the same reason.- The
if (resolvedApiKey || resolvedAccessToken)branch is false. - Code reaches
console.error(authErrorBox('E2B_API_KEY'))(api.ts:106). - The user sees: "…set the E2B_API_KEY environment variable. Visit https://e2b.dev/dashboard?tab=keys to get the API key." — with no mention that
E2B_ACCESS_TOKENwould also have worked, and a link that is the wrong dashboard tab for their authentication method.
Impact
Purely a UX/diagnostic issue, not a functional break. The first line of the error ("Run e2b auth login") still points at a working remediation, so most interactive users recover. The degradation is concentrated on the CI/CD diagnostic path, where the env-var hint matters most. Because of that, nit severity is appropriate.
How to fix
Add a combined branch to authErrorBox (e.g. an 'E2B_API_KEY_OR_ACCESS_TOKEN' case) that lists both env vars and both dashboard URLs, and pass that key from ensureAPIKeyOrAccessToken. Alternatively, change authErrorBox to accept an array of key names and render a list. Either approach keeps the existing single-key callers (ensureAPIKey, ensureAccessToken) working unchanged.
1 participant
No description provided.