Add Step-up rechallenge implementation by rinatkhaziev · Pull Request #2869 · Automattic/vip-cli

rinatkhaziev · 2026-06-04T20:59:25Z

Description

This pull request adds a comprehensive suite of unit tests for the new "rechallenge" (elevated permissions) flow and related features. The tests cover the rechallenge client, flow orchestration, Apollo GraphQL link integration, defensive mode API, and logout logic, ensuring that the elevated permissions workflow is robust, correctly integrated, and well-covered by automated tests.

Rechallenge/Elevated Permissions Flow Testing:

Adds unit tests for the rechallenge client covering session creation, status polling, and token exchange, including error handling for non-2xx responses (__tests__/lib/rechallenge/client.test.ts).
Implements tests for the rechallenge flow orchestration, including polling, abort handling, caching, telemetry, and interactive context detection (__tests__/lib/rechallenge/flow.test.ts).
Introduces tests for the Apollo rechallenge link, verifying correct header attachment, token caching, retry logic on elevated-permission-required errors, and error propagation (__tests__/lib/rechallenge/link.test.ts).

Defensive Mode API Testing:

Adds tests for updateDefensiveModeStatus and updateDefensiveModeConfig API calls, checking input formatting and omission of optional fields (__tests__/lib/defensive-mode/api.test.ts).

Logout Flow Testing:

Adds a test to verify that logout purges the primary token, clears the elevated token cache, and emits a telemetry event (__tests__/lib/logout.test.ts).

Changelog Description

Added

Removed

Fixed

Changed

Pull request checklist

Update SETUP.md with any new environmental variables.
Update the documentation.
Manually test the relevant changes.
Follow the pull request checklist
Add/update automated tests as needed.

New release checklist

Automated tests pass.
The Preparing for release checklist is completed.

Steps to Test

Outline the steps to test and verify the PR here.

Example:

Check out PR.
Run npm run build
Run ./dist/bin/vip-cookies.js nom
Verify cookies are delicious.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…n errors

Also extracts API_HOST/API_URL/PRODUCTION_API_HOST into a leaf constants module and lazy-requires the rechallenge link inside API() to break a circular-dependency cycle that caused Jest mocks to misfire in the rechallenge test suite.

…ions

Implements `vip defensive-mode configure` with full flag validation, interactive prompting for missing required flags, and non-interactive hard-error mode.

…e guards, telemetry order, teardown race) - configure: log current effective config and proposed input before mutating (Fix 1) - enable/disable: add --non-interactive option and guard; error on production mutation attempted non-interactively without --skip-confirmation (Fix 2) - flow: add clientType=cli to rechallenge_required event (Fix 3) - flow: fire rechallenge_verified before rechallenge_exchanged to match spec order (Fix 4) - link: split innerSub into firstSub/retrySub to eliminate teardown race during async gap (Fix 5) - enable/disable/configure: use console.error for error-path chalk.red messages (Fix 6) - token-cache: document single-blob keychain strategy (Fix 7) - tests: assert proposed config logged in configure; add non-interactive-production exit tests for enable/disable; assert rechallenge_verified fires before rechallenge_exchanged Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

github-actions · 2026-06-04T20:59:45Z

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Copilot

Pull request overview

This PR introduces a new “step-up / rechallenge” elevated-permissions flow that integrates with the existing Apollo GraphQL client (via a custom ApolloLink), adds an elevated-token cache, and wires the flow into logout. It also adds a new vip defensive-mode CLI command group backed by new defensive-mode GraphQL mutations, along with a broad suite of unit tests around these behaviors.

Changes:

Add rechallenge (step-up) client, flow orchestration, ApolloLink integration, error types, and elevated-token keychain cache.
Add defensive-mode GraphQL mutation helpers and new CLI commands (vip defensive-mode enable|disable|configure).
Add unit tests for rechallenge components, defensive-mode API, logout behavior, and defensive-mode command behavior.

Reviewed changes

Copilot reviewed 22 out of 30 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/lib/token.ts	Switches API host/constants import to avoid pulling full `api.ts` graph.
src/lib/rechallenge/types.ts	Defines rechallenge protocol/types and constants.
src/lib/rechallenge/token-cache.ts	Implements keychain-backed elevated-token cache keyed by scope.
src/lib/rechallenge/open-browser.ts	Adds wrapper around ESM-only `open` for browser launching.
src/lib/rechallenge/link.ts	Adds ApolloLink that pre-attaches cached elevated token and runs rechallenge on elevated-permission errors.
src/lib/rechallenge/index.ts	Exposes rechallenge module exports.
src/lib/rechallenge/flow.ts	Implements rechallenge orchestration (session create → poll → exchange → cache).
src/lib/rechallenge/errors.ts	Adds typed rechallenge error classes.
src/lib/rechallenge/client.ts	Adds HTTP client for rechallenge session/status/exchange endpoints.
src/lib/logout.ts	Clears elevated-token cache on logout in addition to primary token purge.
src/lib/defensive-mode/api.ts	Adds GraphQL mutations/helpers to update defensive-mode status/config.
src/lib/api/http.ts	Switches `API_HOST` import to new constants module (reduces circular deps).
src/lib/api/feature-flags.ts	Lazy-initializes Apollo client to break a circular dependency chain.
src/lib/api/constants.ts	Introduces shared API host/URL constants (re-exported by `api.ts`).
src/lib/api.ts	Re-exports constants and injects rechallenge link via lazy `require()` to avoid Jest mock timing issues.
src/bin/vip.js	Registers the new `defensive-mode` top-level CLI command.
src/bin/vip-defensive-mode.js	Adds `vip defensive-mode` parent command and subcommand wiring.
src/bin/vip-defensive-mode-enable.js	Adds `vip defensive-mode enable` command implementation.
src/bin/vip-defensive-mode-disable.js	Adds `vip defensive-mode disable` command implementation.
src/bin/vip-defensive-mode-configure.js	Adds `vip defensive-mode configure` command implementation with flag validation/prompting.
package.json	Adds new `vip-defensive-mode*` bin entries for distribution.
tests/lib/rechallenge/token-cache.test.ts	Unit tests for elevated-token cache behavior and corruption handling.
tests/lib/rechallenge/link.test.ts	Unit tests for rechallenge ApolloLink preflight, retry, and error propagation.
tests/lib/rechallenge/flow.test.ts	Unit tests for rechallenge flow polling/exchange/caching and interactivity detection.
tests/lib/rechallenge/client.test.ts	Unit tests for rechallenge HTTP client request shapes and non-2xx error handling.
tests/lib/logout.test.ts	Verifies logout clears both primary token and elevated-token cache + telemetry.
tests/lib/defensive-mode/api.test.ts	Verifies defensive-mode mutation input formatting and optional field omission.
tests/bin/vip-defensive-mode-enable.js	Tests command registration and enable behavior (including non-interactive production safety).
tests/bin/vip-defensive-mode-disable.js	Tests command registration and disable behavior (including non-interactive production safety).
tests/bin/vip-defensive-mode-configure.js	Tests configure flag validation, non-interactive behavior, logging, and telemetry.

…ad validation, dedupe commands) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

sjinks · 2026-06-09T18:10:23Z

+		console.log( JSON.stringify( currentConfig, null, 2 ) );
+	}
+	console.log( chalk.bold( 'Proposed defensive-mode configuration:' ) );
+	console.log( JSON.stringify( input, null, 2 ) );


Maybe make the output more user-friendly?

Reworked in 5a7ca83: current/proposed values now render as a settings table (via the existing table() helper), and the JSON blob was dropped from the production confirm prompt.

sjinks · 2026-06-09T18:13:46Z

 	await http( '/logout', { method: 'post' } );

 	await Token.purge();
+	await tokenCache.clearAll();


Should we do the same on login?

Elevated tokens are keyed only by the API host and operation scope. Logout clears them, but direct vip login token replacement does not, so cached elevation can cross user identities on the same machine.

This is not a very likely scenario, but I think it makes sense to apply this hardening.

Applied in 5a7ca83: vip login now calls tokenCache.clearAll() right after Token.set() replaces the stored token. That's the only Token.set call site, so login and logout now both clear cached elevation.

sjinks · 2026-06-09T18:16:09Z

+    "vip-defensive-mode": "dist/bin/vip-defensive-mode.js",
+    "vip-defensive-mode-configure": "dist/bin/vip-defensive-mode-configure.js",
+    "vip-defensive-mode-disable": "dist/bin/vip-defensive-mode-disable.js",
+    "vip-defensive-mode-enable": "dist/bin/vip-defensive-mode-enable.js",


I guess we need to update src/lib/cli/internal-bin-loader.js as well.

Added in 5a7ca83. Diffed package.json bin keys against the loader — the four vip-defensive-mode* entries were the only missing ones.

sjinks · 2026-06-09T18:25:45Z

+	return new Promise( ( resolve, reject ) => {
+		if ( signal?.aborted ) {
+			reject( new Error( 'aborted' ) );
+			return;
+		}
+		const timer = setTimeout( () => {
+			signal?.removeEventListener( 'abort', onAbort );
+			resolve();
+		}, ms );
+		function onAbort() {
+			clearTimeout( timer );
+			reject( new Error( 'aborted' ) );
+		}
+		signal?.addEventListener( 'abort', onAbort, { once: true } );
+	} );


import { setTimeout } from 'node:timers/promises';

Suggested change

return new Promise( ( resolve, reject ) => {

if ( signal?.aborted ) {

reject( new Error( 'aborted' ) );

return;

}

const timer = setTimeout( () => {

signal?.removeEventListener( 'abort', onAbort );

resolve();

}, ms );

function onAbort() {

clearTimeout( timer );

reject( new Error( 'aborted' ) );

}

signal?.addEventListener( 'abort', onAbort, { once: true } );

} );

try {

await setTimeout( ms, undefined, { signal } );

} catch ( err ) {

if ( err.name === 'AbortError' ) {

throw new Error( 'aborted' );

} else {

throw err;

}

}

Done in 5a7ca83: the hand-rolled sleep() is replaced with setTimeout from node:timers/promises (with { signal }). The caller already converts any sleep rejection to RechallengeAbortedError, so the AbortError branch lives there.

sjinks · 2026-06-09T18:32:16Z

+}
+
+function parsePositiveInt( raw ) {
+	if ( raw === undefined || raw === null || raw === '' ) {


Suggested change

if ( raw === undefined || raw === null || raw === '' ) {

if ( raw === undefined || raw === null || raw === '' || typeof raw === 'boolean' ) {

--connection-threshold-absolute without parameters is treated like --connection-threshold-absolute=true; Number( true ) coerces to 1. I am not sure if it is OK.

Fixed in 5a7ca83: parsePositiveInt now rejects booleans (bare flags) and blank/whitespace strings, so --connection-threshold-absolute without a value errors instead of coercing to 1. Added a regression test.

…bin loader, sleep, flag parsing) - Render current/proposed defensive-mode config as a table instead of raw JSON, and drop the JSON blob from the production confirm prompt - Clear the elevated-token cache when `vip login` replaces the stored token, so cached elevation cannot carry across user identities - Register the four vip-defensive-mode* bins in internal-bin-loader.js - Replace the hand-rolled abortable sleep in rechallenge flow with setTimeout from node:timers/promises - Reject boolean/blank values in parsePositiveInt so bare flags like `--connection-threshold-absolute` error instead of coercing to 1 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The loop polls session status sequentially by design; each iteration must finish before the next starts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…e client trunk replaced node-fetch with undici (#2837), so in the PR merge ref the node-fetch types no longer resolve and type-aware lint flags every member access as unsafe. Derive the response type from http() instead, which tracks whichever fetch implementation the merged tree uses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

sonarqubecloud · 2026-06-10T17:52:47Z

Quality Gate passed

Issues
7 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
1.8% Duplication on New Code

See analysis details on SonarQube Cloud

rinatkhaziev and others added 16 commits June 4, 2026 14:00

feat(rechallenge): add types and error classes

8934b71

feat(rechallenge): add keychain-backed per-scope elevated-token cache

8655df4

feat(rechallenge): add REST client for Parker v2 session endpoints

35f9916

fix(rechallenge): satisfy tsc check-types in test files

a6df809

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

feat(rechallenge): orchestrate session create, poll, exchange

b2c46de

feat(rechallenge): add Apollo link that intercepts elevated-permissio…

3e6cc2f

…n errors

feat(api): insert rechallenge link into Apollo chain

f03c423

Also extracts API_HOST/API_URL/PRODUCTION_API_HOST into a leaf constants module and lazy-requires the rechallenge link inside API() to break a circular-dependency cycle that caused Jest mocks to misfire in the rechallenge test suite.

feat(defensive-mode): add GraphQL helpers for status and config mutat…

a461f52

…ions

feat(cli): add vip defensive-mode parent command

b8815af

feat(cli): add vip defensive-mode enable subcommand

20e4e0f

feat(cli): add vip defensive-mode disable subcommand

2dc5804

feat(cli): add vip defensive-mode configure subcommand

1109b2c

Implements `vip defensive-mode configure` with full flag validation, interactive prompting for missing required flags, and non-interactive hard-error mode.

feat(cli): register vip defensive-mode top-level command

c700b4e

feat(logout): clear elevated-token cache on logout

80e0d1d

fix(api): lint cleanup for lazy rechallenge link import

6763ec4

rinatkhaziev requested a review from Copilot June 4, 2026 21:01

Copilot started reviewing on behalf of rinatkhaziev June 4, 2026 21:02 View session

Copilot AI reviewed Jun 4, 2026

View reviewed changes

Comment thread src/lib/defensive-mode/api.ts

Comment thread src/lib/defensive-mode/api.ts

Comment thread src/lib/rechallenge/errors.ts

Comment thread src/lib/rechallenge/link.ts

Comment thread __tests__/lib/rechallenge/client.test.ts

fix: address PR review feedback (data guards, version constant, paylo…

c9fc8df

…ad validation, dedupe commands) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

rinatkhaziev added the [Status] Needs Review label Jun 9, 2026

sjinks reviewed Jun 9, 2026

View reviewed changes

rinatkhaziev and others added 3 commits June 10, 2026 12:33

chore(lint): scope no-await-in-loop disable to rechallenge polling loop

8736474

The loop polls session status sequentially by design; each iteration must finish before the next starts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Merge branch 'trunk' into feat/rechallenge-v2-defensive-mode

2750aa4

	if ( raw === undefined \|\| raw === null \|\| raw === '' ) {
	if ( raw === undefined \|\| raw === null \|\| raw === '' \|\| typeof raw === 'boolean' ) {

Conversation

rinatkhaziev commented Jun 4, 2026

Description

Changelog Description

Added

Removed

Fixed

Changed

Pull request checklist

New release checklist

Steps to Test

Uh oh!

github-actions Bot commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependency Review

Scanned Files

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Jun 10, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Jun 4, 2026 •

edited

Loading