Skip to content

feat: support VIP_CLI_TOKEN environment variable for keychain-free authentication - #2946

Open
spenserhale wants to merge 1 commit into
Automattic:trunkfrom
spenserhale:add/vip-cli-token-env-var
Open

feat: support VIP_CLI_TOKEN environment variable for keychain-free authentication#2946
spenserhale wants to merge 1 commit into
Automattic:trunkfrom
spenserhale:add/vip-cli-token-env-var

Conversation

@spenserhale

@spenserhale spenserhale commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Description

Adds support for a VIP_CLI_TOKEN environment variable that supplies the Personal Access Token directly, bypassing OS keychain storage entirely.

Use case: authenticated VIP-CLI usage in environments where the OS keychain is unavailable — SSH sessions on macOS (the keychain ACL authorization prompt requires a GUI and cannot be shown, so token reads fail), CI pipelines, containers, and scripting. This follows the pattern established by gh's GH_TOKEN and this repo's own WPVIP_DEPLOY_TOKEN.

Behavior:

  • When VIP_CLI_TOKEN is set (non-empty after trimming), Token.get() builds the token from the environment and never touches the keychain. It takes precedence over stored credentials.
  • A malformed env token fails with an actionable error naming the variable and the token URL (a new EnvTokenError), rather than silently falling back to the keychain or crashing with an opaque message.
  • vip login prints a note when the env var is set (the newly stored token won't be used until the variable is unset).
  • vip logout does not invalidate a user-managed VIP_CLI_TOKEN server-side (matching gh semantics); it purges stored credentials as usual and prints a note that the env var continues to authenticate until unset.

Known behavior: if VIP_CLI_TOKEN is set but malformed, vip login also exits with the actionable error rather than starting the login flow. This is intentional — the env var takes precedence, so a broken value should be fixed or unset first rather than silently ignored.

Related: pairs with #2947, which skips the token read for commands that don't require auth; the two are independent and address different use cases.

Changelog Description

Added

  • Added support for the VIP_CLI_TOKEN environment variable to authenticate directly without OS keychain storage, for use in SSH sessions, CI, and other non-graphical environments.

Pull request checklist

Steps to Test

  1. Check out PR.
  2. Run npm run build.
  3. Get a Personal Access Token from https://dashboard.wpvip.com/me/cli/token
  4. Run VIP_CLI_TOKEN=<token> node ./dist/bin/vip.js whoami — authenticates using the env token; the keychain read for auth is bypassed. (Note: with DEBUG=@automattic/vip:keychain you may still see one keychain line from the analytics UUID lookup — that is unrelated to authentication and its errors are swallowed by the tracker.)
  5. Run VIP_CLI_TOKEN=not-a-jwt node ./dist/bin/vip.js app list — prints an actionable error naming VIP_CLI_TOKEN instead of an unhandled crash.
  6. With VIP_CLI_TOKEN set, run node ./dist/bin/vip.js logout — stored credentials are purged, and a note explains the env var still authenticates until unset.
  7. Best exercised over SSH to a macOS machine, where keychain-based auth fails but VIP_CLI_TOKEN works.

@sonarqubecloud

Copy link
Copy Markdown

@spenserhale

Copy link
Copy Markdown
Contributor Author

Hey @sjinks

Could you review this PR?

@sjinks
sjinks marked this pull request as ready for review August 11, 2026 14:42
Copilot AI lite review requested due to automatic review settings August 11, 2026 14:42
…thentication

Add a VIP_CLI_TOKEN environment variable that, when set (non-empty after
trimming), authenticates VIP-CLI directly and bypasses the OS keychain
entirely. This unblocks headless, SSH, and CI sessions where the keychain
is unavailable, and makes scripting easier. It mirrors the existing
WPVIP_DEPLOY_TOKEN precedent.

The env token takes precedence over stored credentials: a malformed value
throws an EnvTokenError with an actionable message naming the variable and
token URL, surfaced cleanly instead of crashing. The login flow warns that
the env var overrides any token you log in with, and logout skips the
server-side call so it never invalidates a user-managed token, while still
purging local state and reminding the user to unset the variable.
@sonarqubecloud

Copy link
Copy Markdown

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.

Pull request overview

Adds support for authenticating VIP-CLI using a VIP_CLI_TOKEN environment variable (bypassing OS keychain reads), improving usability in SSH/CI/container contexts while aligning logout/login messaging and documentation.

Changes:

  • Add VIP_CLI_TOKEN support in Token.get() with a dedicated EnvTokenError for malformed env tokens.
  • Update vip login and vip logout behavior/messaging when an env token is set (including skipping server-side logout invalidation).
  • Document VIP_CLI_TOKEN and add/extend unit tests around env-token behavior.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/lib/token.ts Introduces env-token precedence (VIP_CLI_TOKEN) plus EnvTokenError and shared token URL constant.
src/lib/logout.ts Skips server-side logout when env token is set; prints user guidance about unsetting the env var.
src/bin/vip.js Surfaces EnvTokenError via exit.withError and adds login note when env token is set.
docs/SETUP.md Documents the new VIP_CLI_TOKEN environment variable.
tests/lib/token.js Adds unit tests verifying env-token precedence, trimming, and malformed-token error behavior.
tests/lib/logout.test.ts Adds test coverage for logout behavior when env token is set.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/lib/token.ts
Comment on lines 132 to +137
public static async get(): Promise< Token > {
const envToken = process.env[ ENV_TOKEN_NAME ];
if ( envToken?.trim() ) {
// The env var supplies the token directly; never touch the keychain.
try {
return new Token( envToken );
Comment thread src/bin/vip.js
Comment on lines +208 to +218
try {
token = await Token.get();
} catch ( err ) {
// A malformed VIP_CLI_TOKEN already carries an actionable message that
// names the env var; surface it directly instead of crashing.
if ( err instanceof EnvTokenError ) {
exit.withError( err.message );
}

throw err;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

An expired or claim-invalid but decodable environment token enters the login flow. The replacement is stored in the keychain, but subsequent requests reread and send the still-authoritative environment token.

We must treat every invalid environment token as an actionable configuration error rather than offering keychain login.

Acceptance criteria: Expired and claim-invalid VIP_CLI_TOKEN values exit without prompting, name the variable, and explain how to replace or unset it.

Comment thread src/lib/logout.ts
try {
await http( '/logout', { method: 'post' } );
// VIP_CLI_TOKEN is user-managed: logout must not invalidate it server-side.
if ( ! Token.isEnvTokenSet() ) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When both stored token A and environment token B exist, logout skips server revocation but deletes A from the keychain (see the finally block). Token A remains valid server-side while its local handle is lost.

AC: Mixed-source logout neither revokes B nor silently deletes an unrevoked A.

@sjinks

sjinks commented Aug 11, 2026

Copy link
Copy Markdown
Member

There are two more issues:

  1. With environment token A active, vip login stores and aliases analytics to the entered token B, even though requests continue to authenticate as A (src/bin/vip.js:176).

  2. Elevated tokens are cached only by API host and operation scope. Switching VIP_CLI_TOKEN between users does not perform the cache clear that interactive login performs, so user B can receive user A’s cached elevated header (src/lib/rechallenge/token-cache.ts:12).

@sjinks

sjinks commented Aug 11, 2026

Copy link
Copy Markdown
Member

What do you think if we instead add an env var, say VIP_DISABLE_SECURE_TOKEN, that controls which token storage is used?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants