Skip to content

A background token refresh failure in a non-interactive process wipes every saved CLI session (all accounts) #8945

Open

Description

@carlwangx

Summary

When a long-running CLI process (e.g. shopify theme dev) hits a refresh failure while running with noPrompt: true, throwOnNoPrompt() calls logout(), which removes the entire session store (sessionStore + currentSessionId) from config.json, for every account and alias. Because the theme dev auto-refresher swallows the error (outputDebug('Session could not be refreshed.')), nothing is shown to the user. The next CLI command in any terminal then asks for a full login.

On a machine that runs several CLI processes at the same time, this makes the CLI feel like it "expires" every few hours.

Environment

  • Shopify CLI 4.9.4 (Homebrew), macOS, Apple Silicon
  • Several shopify theme dev processes for different stores running at the same time, plus one-off CLI commands from scripts/other terminals, all sharing the same ~/Library/Preferences/shopify-cli-kit-nodejs/config.json

Observed

  • While two shopify theme dev processes were running, config.json ended up with only the cache key; sessionStore and currentSessionId were gone.
  • The theme dev terminals showed no error. The next command in another terminal required a full device login.

Relevant code (main @ 060e88b)

  1. throwOnNoPrompt() logs out before it throws, so one non-interactive process failing removes the sessions every other process relies on:
    async function throwOnNoPrompt(noPrompt: boolean) {
    if (!noPrompt) return
    await logout()
    throw new AbortError(
    `The currently available CLI credentials are invalid.
    The CLI is currently unable to prompt for reauthentication.`,
    'Restart the CLI process you were running. If in an interactive terminal, you will be prompted to reauthenticate. If in a non-interactive terminal, ensure the correct credentials are available in the program environment.',
    )
    }

    It is reached from both needs_full_auth and InvalidGrantError during refresh:
    if (validationResult === 'needs_full_auth') {
    await throwOnNoPrompt(noPrompt)
    outputDebug(outputContent`Initiating the full authentication flow...`)
    newSession = await executeCompleteFlow(applications, currentSession?.identity.alias)
    } else if (validationResult === 'needs_refresh' || forceRefresh) {
    outputDebug(outputContent`The current session is valid but needs refresh. Refreshing...`)
    try {
    newSession = await refreshTokens(currentSession!, applications)
    } catch (error) {
    if (error instanceof InvalidGrantError) {
    await throwOnNoPrompt(noPrompt)
    newSession = await executeCompleteFlow(applications, currentSession?.identity.alias)
    } else if (error instanceof InvalidRequestError) {
    await sessionStore.remove()
    throw new AbortError('\nError validating auth session', "We've cleared the current session, please try again")
    } else {
    throw error
    }
    }
    }
  2. The theme dev auto-refresher runs every 30 minutes with noPrompt: true and swallows failures:
    setInterval(() => {
    if (!session.refresh) return
    session
    .refresh()
    .then(() => outputDebug('Refreshed theme session via auto-refresher...'))
    .catch(() => outputDebug('Session could not be refreshed.'))
    }, SESSION_TIMEOUT_IN_MS)
  3. The session store is read, modified and written back as a whole. I couldn't find any cross-process lock around it:
    const completeSession = {...currentSession, ...newSession} as Session
    const newSessionId = completeSession.identity.userId
    const updatedSessions: Sessions = {
    ...sessions,
    [fqdn]: {...sessions[fqdn], [newSessionId]: completeSession},
    }
    // Save the new session info if it has changed
    if (!isEmpty(newSession)) {
    await sessionStore.store(updatedSessions)
    if (!commandSessionId) setCurrentSessionId(newSessionId)
    }

Suspected trigger (not confirmed)

Two processes refresh at around the same time using the same refresh token, or one overwrites the store with stale tokens after the other has refreshed. If refresh tokens are rotated, the later refresh gets invalid_grant, which goes through throwOnNoPrompt(), which wipes all sessions. I couldn't confirm whether Identity rotates refresh tokens, but the "one failure clears everything" behaviour comes straight from the code above, whatever causes the refresh to fail.

Steps to reproduce (likely)

  1. shopify auth login
  2. Start shopify theme dev against two different stores in two terminals and leave them running for a few hours (past the access-token expiry, so the 30-minute auto-refresher has to refresh).
  3. Check config.json: sessionStore disappears at some point, and the next CLI command asks for a full login.

Expected

  • A refresh failure in one non-interactive process should not delete sessions for other processes or accounts. At most it should fail that process, or invalidate only the session it was using.
  • Concurrent refreshes should be serialized (file lock), or the store should be re-read before writing, so that processes don't invalidate each other's tokens.
  • When theme dev loses its session, the user should see it in the terminal instead of a debug-only log line.

Related

#5826 and #3173 report similar "randomly signed out / unable to prompt for reauthentication" symptoms. Both were closed after fixes to single-process error handling. The multi-process case doesn't seem to be covered.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions