diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index b0120f35014..4154958b038 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -44,7 +44,10 @@ jobs: for node in $node_versions; do OS_NODE+=("ubuntu-latest:$node") done - if [[ "$HEAD_REF" == release-please--* ]]; then + # XXX(serhalp): also run the macOS + Windows matrix on this branch so we can verify the new keychain code + # paths before merge. + # Revert before merging. + if [[ "$HEAD_REF" == release-please--* || "$HEAD_REF" == "feat/native-token-keychain-storage" ]]; then for os in $extra_oses; do OS_NODE+=("$os:$primary") done diff --git a/package-lock.json b/package-lock.json index bfae83c43ac..c3c47953bbc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,6 +11,7 @@ "license": "MIT", "dependencies": { "@fastify/static": "^9.1.1", + "@napi-rs/keyring": "^1.3.0", "@netlify/ai": "^0.4.1", "@netlify/api": "^14.0.19", "@netlify/blobs": "^10.7.7", @@ -2676,6 +2677,225 @@ "node": ">=18" } }, + "node_modules/@napi-rs/keyring": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring/-/keyring-1.3.0.tgz", + "integrity": "sha512-WrOw/bcXm0f9qHkumlT1QlArXSTWqaY9sunsDpOk+yCCorCKMxvWT/a3xko4EYHVdeZoh00yI2TydXn6eyICDA==", + "license": "MIT", + "engines": { + "node": ">= 10" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/Brooooooklyn" + }, + "optionalDependencies": { + "@napi-rs/keyring-darwin-arm64": "1.3.0", + "@napi-rs/keyring-darwin-x64": "1.3.0", + "@napi-rs/keyring-freebsd-x64": "1.3.0", + "@napi-rs/keyring-linux-arm-gnueabihf": "1.3.0", + "@napi-rs/keyring-linux-arm64-gnu": "1.3.0", + "@napi-rs/keyring-linux-arm64-musl": "1.3.0", + "@napi-rs/keyring-linux-riscv64-gnu": "1.3.0", + "@napi-rs/keyring-linux-x64-gnu": "1.3.0", + "@napi-rs/keyring-linux-x64-musl": "1.3.0", + "@napi-rs/keyring-win32-arm64-msvc": "1.3.0", + "@napi-rs/keyring-win32-ia32-msvc": "1.3.0", + "@napi-rs/keyring-win32-x64-msvc": "1.3.0" + } + }, + "node_modules/@napi-rs/keyring-darwin-arm64": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-darwin-arm64/-/keyring-darwin-arm64-1.3.0.tgz", + "integrity": "sha512-pl76hJvdYUBn6I24bXiOBMA9nbDapo3I5B+f3OorjDU4dUMSypXeKbOVehJe8fhgTiH24flMyTS3aAIy43xegQ==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-darwin-x64": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-darwin-x64/-/keyring-darwin-x64-1.3.0.tgz", + "integrity": "sha512-YcJtEV5LA3cvA4z3BurgxH5IhTsW1JfIvcAAcqcecwk06Si9F9NqkxbZVIfDwQ8oRHgaBmT3zZJnLAotCrVahw==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-freebsd-x64": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-freebsd-x64/-/keyring-freebsd-x64-1.3.0.tgz", + "integrity": "sha512-vlLf31TGhfRAaxLDBhg8b89ss0HHD/lyNmL5F3UjSaz5CUXElsJmKYq9fqA/B+cZKUEUcLHHGhF0I/CqcFdaVw==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "freebsd" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-arm-gnueabihf": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-arm-gnueabihf/-/keyring-linux-arm-gnueabihf-1.3.0.tgz", + "integrity": "sha512-KiWdMMu/Inz/bHHIAGrnF7r54FZDYXuHO6UFF/rhIrshUsxbMG1Rl9lEymNtqqsVo927G0VYcb02FzWQ3iBQRQ==", + "cpu": [ + "arm" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-arm64-gnu": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-arm64-gnu/-/keyring-linux-arm64-gnu-1.3.0.tgz", + "integrity": "sha512-eyKGpY40lm9Jvs1aD294XRH4y7+TlJM0YVAryZeXA6TX0mb4gMkxVXwSQv7MCwgah7raeUd0dKUb4BPAYIgcMg==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-arm64-musl": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-arm64-musl/-/keyring-linux-arm64-musl-1.3.0.tgz", + "integrity": "sha512-iIK6JWHXAJqDrEyLY3TmswwloVyt2vj+04TZnew+uSJ9gnDO8EwRbp3/iw3LpWaXiDO7VomGO6y8I0Id8uBZSw==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-riscv64-gnu": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-riscv64-gnu/-/keyring-linux-riscv64-gnu-1.3.0.tgz", + "integrity": "sha512-/PGqrwn6EwgtK6vccASSXJRfOSP4vN1F4ASsIQ+7MdrK6hNvAJ1FZPrIuD5gGGdxezo3F++To2Wq7DbuGIeuNQ==", + "cpu": [ + "riscv64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-x64-gnu": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-x64-gnu/-/keyring-linux-x64-gnu-1.3.0.tgz", + "integrity": "sha512-2PDK1WKWTu9lBGq9VvNEkSlQD3O7YwVpmnyN2M3cy4v7NJ/8gDMd9GXv3G+FVXN13uhp4gnnPBS+ScefmEeD2A==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-x64-musl": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-x64-musl/-/keyring-linux-x64-musl-1.3.0.tgz", + "integrity": "sha512-oJ2HkX8YUo46QBkn0pG+HuIKQNqr523q6vBobCn+P95s4C4K6/kLBqHY/1bg5J4ap31DzsznhnFKcfBNBsjCnw==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-win32-arm64-msvc": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-win32-arm64-msvc/-/keyring-win32-arm64-msvc-1.3.0.tgz", + "integrity": "sha512-tOd3c/uAaeoE4ycVlmAdSvygz0Zt3zdca6Y7gokBeIbaRDWpjDIUOpU3MvML59XAaqyuKGsVVu0F/DZb1lHPmw==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-win32-ia32-msvc": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-win32-ia32-msvc/-/keyring-win32-ia32-msvc-1.3.0.tgz", + "integrity": "sha512-sPSqeAFZMGqP1R++M2JTza7GQJJ/TpCo6JU6Vcd4jnebvOaEDs9b7eipakU1PJdSvhpC2yXMCNRk9gXfrhuwHQ==", + "cpu": [ + "ia32" + ], + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-win32-x64-msvc": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-win32-x64-msvc/-/keyring-win32-x64-msvc-1.3.0.tgz", + "integrity": "sha512-4DnCWXwDc0HRKwyRlG5y0VhKZW2tNRQfKKfyj6IX/KWfDNyq9hn4n+GL1auyDcOO/v8PwnhmYo2+rOOqCkvvOg==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">= 10" + } + }, "node_modules/@netlify/ai": { "version": "0.4.1", "resolved": "https://registry.npmjs.org/@netlify/ai/-/ai-0.4.1.tgz", diff --git a/package.json b/package.json index 47b932334e8..19499a77329 100644 --- a/package.json +++ b/package.json @@ -56,6 +56,7 @@ }, "dependencies": { "@fastify/static": "^9.1.1", + "@napi-rs/keyring": "^1.3.0", "@netlify/ai": "^0.4.1", "@netlify/api": "^14.0.19", "@netlify/blobs": "^10.7.7", diff --git a/src/commands/base-command.ts b/src/commands/base-command.ts index d3b40a27b6e..f99b188cff5 100644 --- a/src/commands/base-command.ts +++ b/src/commands/base-command.ts @@ -18,6 +18,7 @@ import merge from 'lodash/merge.js' import pick from 'lodash/pick.js' import { getAgent } from '../lib/http-agent.js' +import { writeAuthTokenForStorage } from '../lib/secure-storage.js' import { NETLIFY_CYAN, USER_AGENT, @@ -187,16 +188,18 @@ export type BaseOptionValues = { verbose?: boolean } -export function storeToken( +export async function storeToken( globalConfig: Awaited>, { userId, name, email, accessToken }: { userId: string; name?: string; email?: string; accessToken: string }, -) { +): Promise<{ mode: 'keychain' | 'legacy'; keychainFailed: boolean }> { + const result = await writeAuthTokenForStorage(userId, accessToken) + const userData = merge(globalConfig.get(`users.${userId}`), { id: userId, name, email, auth: { - token: accessToken, + token: result.mode === 'keychain' ? undefined : accessToken, github: { user: undefined, token: undefined, @@ -205,6 +208,7 @@ export function storeToken( }) globalConfig.set('userId', userId) globalConfig.set(`users.${userId}`, userData) + return result } /** Base command class that provides tracking and config initialization */ @@ -551,7 +555,14 @@ export default class BaseCommand extends Command { return logAndThrowError('Could not retrieve user ID from Netlify API') } - storeToken(this.netlify.globalConfig, { userId, name, email, accessToken }) + const { keychainFailed } = await storeToken(this.netlify.globalConfig, { userId, name, email, accessToken }) + if (keychainFailed) { + warn( + `Could not store the auth token in your OS keychain. Falling back to the plaintext config file. Set ${chalk.cyanBright( + 'NETLIFY_USE_LEGACY_AUTH_STORAGE=1', + )} to silence this and always use the plaintext config file.`, + ) + } await identify({ name, diff --git a/src/commands/login/login-check.ts b/src/commands/login/login-check.ts index bf74cb2be8a..930b2e9b9e9 100644 --- a/src/commands/login/login-check.ts +++ b/src/commands/login/login-check.ts @@ -1,7 +1,7 @@ import { NetlifyAPI } from '@netlify/api' import { OptionValues } from 'commander' -import { log, logAndThrowError, logJson } from '../../utils/command-helpers.js' +import { chalk, log, logAndThrowError, logJson, warn } from '../../utils/command-helpers.js' import { storeToken } from '../base-command.js' import type { NetlifyOptions } from '../types.js' @@ -45,12 +45,19 @@ export const loginCheck = async ( return logAndThrowError('Could not retrieve user ID from Netlify API') } - storeToken(globalConfig, { + const { keychainFailed } = await storeToken(globalConfig, { userId: user.id, name: user.full_name, email: user.email, accessToken, }) + if (keychainFailed) { + warn( + `Could not store the auth token in your OS keychain. Falling back to the plaintext config file. Set ${chalk.cyanBright( + 'NETLIFY_USE_LEGACY_AUTH_STORAGE=1', + )} to silence this.`, + ) + } logJson({ status: 'authorized', diff --git a/src/commands/login/login.ts b/src/commands/login/login.ts index ab3d4a6c34c..5791432a4e2 100644 --- a/src/commands/login/login.ts +++ b/src/commands/login/login.ts @@ -12,6 +12,8 @@ const msg = function (location: TokenLocation) { return 'via CLI --auth flag' case 'config': return 'via netlify config on your machine' + case 'keychain': + return 'via your OS keychain (secure storage)' default: return '' } diff --git a/src/commands/logout/logout.ts b/src/commands/logout/logout.ts index 7a6c63a998d..1f49ed566b6 100644 --- a/src/commands/logout/logout.ts +++ b/src/commands/logout/logout.ts @@ -1,5 +1,6 @@ import { OptionValues } from 'commander' +import { deleteTokenFromKeychain } from '../../lib/secure-storage.js' import { exit, getToken, log } from '../../utils/command-helpers.js' import { track } from '../../utils/telemetry/index.js' import BaseCommand from '../base-command.js' @@ -16,7 +17,11 @@ export const logout = async (_options: OptionValues, command: BaseCommand) => { await track('user_logout') - // unset userID without deleting key + const userId = command.netlify.globalConfig.get('userId') as string | undefined + if (userId) { + await deleteTokenFromKeychain(userId) + command.netlify.globalConfig.set(`users.${userId}.auth.token`, undefined) + } command.netlify.globalConfig.set('userId', null) if (location === 'env') { diff --git a/src/commands/status/status.ts b/src/commands/status/status.ts index ed44723aaad..fc856fe9831 100644 --- a/src/commands/status/status.ts +++ b/src/commands/status/status.ts @@ -2,6 +2,7 @@ import clean from 'clean-deep' import type { OptionValues } from 'commander' import prettyjson from 'prettyjson' +import { getLegacyConfigFilePath } from '../../lib/secure-storage.js' import { chalk, logAndThrowError, @@ -13,12 +14,28 @@ import { type APIError, } from '../../utils/command-helpers.js' import { isInteractive } from '../../utils/scripted-commands.js' +import type { TokenLocation } from '../../utils/types.js' import type BaseCommand from '../base-command.js' +const formatTokenStorage = (location: TokenLocation): string => { + switch (location) { + case 'keychain': + return 'native secure storage' + case 'config': + return `plaintext config file (${getLegacyConfigFilePath()})` + case 'env': + return 'NETLIFY_AUTH_TOKEN environment variable' + case 'flag': + return '--auth flag' + default: + return 'not stored' + } +} + export const status = async (options: OptionValues, command: BaseCommand) => { const { accounts, api, globalConfig, site, siteInfo } = command.netlify const currentUserId = globalConfig.get('userId') as string | undefined - const [accessToken] = await getToken() + const [accessToken, tokenLocation] = await getToken(command.opts<{ auth?: string }>().auth) if (!accessToken) { log(`Not logged in. Please log in to see project status.`) @@ -67,7 +84,7 @@ export const status = async (options: OptionValues, command: BaseCommand) => { // another lib. (clean as unknown as >(obj: T) => Partial)(accountData) - log(prettyjson.render(cleanAccountData)) + log(prettyjson.render({ ...cleanAccountData, 'Auth token storage': formatTokenStorage(tokenLocation) })) if (!siteId) { warn('Did you run `netlify link` yet?') @@ -78,6 +95,10 @@ export const status = async (options: OptionValues, command: BaseCommand) => { if (options.json) { logJson({ account: cleanAccountData, + authTokenStorage: { + source: tokenLocation, + ...(tokenLocation === 'config' ? { configPath: getLegacyConfigFilePath() } : {}), + }, siteData: { 'site-name': siteInfo.name, 'config-path': site.configPath, diff --git a/src/lib/secure-storage.ts b/src/lib/secure-storage.ts new file mode 100644 index 00000000000..dc4f5b5f691 --- /dev/null +++ b/src/lib/secure-storage.ts @@ -0,0 +1,201 @@ +import path from 'path' + +import { getAPIToken, getGlobalConfigStore, type GlobalConfigStore } from '@netlify/dev-utils' +import envPaths from 'env-paths' +import inquirer from 'inquirer' + +import { chalk, log } from '../utils/command-helpers.js' +import { isInteractive } from '../utils/scripted-commands.js' +import { reportError, track } from '../utils/telemetry/index.js' + +const SERVICE_NAME = 'netlify-cli' +const MIGRATION_DECLINED_KEY = 'auth.keychainMigrationDeclined' + +export const LEGACY_STORAGE_ENV_VAR = 'NETLIFY_USE_LEGACY_AUTH_STORAGE' + +export type TokenStorageMode = 'keychain' | 'legacy' + +type KeyringModule = typeof import('@napi-rs/keyring') + +let keyringPromise: Promise | undefined +let migrationPromptedThisSession = false + +export const getLegacyConfigFilePath = (): string => { + const paths = envPaths('netlify', { suffix: '' }) + return path.join(paths.config, 'config.json') +} + +const loadKeyring = (): Promise => { + if (keyringPromise == null) { + keyringPromise = import('@napi-rs/keyring').catch(() => null) + } + return keyringPromise +} + +const createEntry = async (userId: string) => { + const keyring = await loadKeyring() + if (!keyring) return null + try { + return new keyring.Entry(SERVICE_NAME, userId) + } catch { + return null + } +} + +export const isLegacyAuthStorageForced = (): boolean => + process.env[LEGACY_STORAGE_ENV_VAR] != null && process.env[LEGACY_STORAGE_ENV_VAR] !== '' + +export const storeTokenInKeychain = async (userId: string, token: string): Promise => { + const entry = await createEntry(userId) + if (!entry) return false + try { + entry.setPassword(token) + return true + } catch { + return false + } +} + +export const getTokenFromKeychain = async (userId: string): Promise => { + const entry = await createEntry(userId) + if (!entry) return null + try { + return entry.getPassword() + } catch { + return null + } +} + +export const deleteTokenFromKeychain = async (userId: string): Promise => { + const entry = await createEntry(userId) + if (!entry) return false + try { + return entry.deletePassword() + } catch { + return false + } +} + +const trackStored = (mode: TokenStorageMode, migrated: boolean, keychainFailed = false): void => { + void track('user_authTokenStored', { mode, migrated, ...(keychainFailed ? { keychainFailed: true } : {}) }) +} + +const trackRead = (mode: TokenStorageMode): void => { + void track('user_authTokenRead', { mode }) +} + +type PromptOutcome = { kind: 'answered'; confirmed: boolean } | { kind: 'error' } + +const promptForMigration = async (): Promise => { + log() + log(`Your Netlify auth token is currently stored in plaintext at ${chalk.cyanBright(getLegacyConfigFilePath())}.`) + log(`The CLI can move it to your OS keychain (more secure). Your operating system may prompt you to allow access.`) + log() + const { confirm } = await inquirer.prompt<{ confirm: boolean }>([ + { + type: 'confirm', + name: 'confirm', + message: 'Move the token to the keychain now?', + default: true, + }, + ]) + return { kind: 'answered', confirmed: confirm } +} + +// Mirrors the production gate (interactive shell, not in CI) with a TESTING_PROMPTS escape +// hatch so integration tests can drive the migration prompt via handleQuestions without a pty. +const isMigrationAllowed = (): boolean => isInteractive() || process.env.TESTING_PROMPTS === 'true' + +const attemptMigration = async (userId: string, token: string, globalConfig: GlobalConfigStore): Promise => { + if (!isMigrationAllowed()) return false + if (migrationPromptedThisSession) return false + if (globalConfig.get(MIGRATION_DECLINED_KEY) === true) return false + + migrationPromptedThisSession = true + + let outcome: PromptOutcome + try { + outcome = await promptForMigration() + } catch (error) { + void reportError(error, { severity: 'error' }) + outcome = { kind: 'error' } + } + + if (outcome.kind === 'error') return false + + if (!outcome.confirmed) { + globalConfig.set(MIGRATION_DECLINED_KEY, true) + log( + chalk.dim( + `Got it. The CLI will not ask again. To revisit, run ${chalk.cyanBright( + '`netlify logout`', + )} and ${chalk.cyanBright('`netlify login`')}.`, + ), + ) + void track('user_authTokenMigrationDeclined', {}) + return false + } + + const ok = await storeTokenInKeychain(userId, token) + if (!ok) { + log(chalk.yellow('Could not write to the OS keychain. Keeping the token in the config file.')) + trackStored('legacy', false, true) + return false + } + globalConfig.set(`users.${userId}.auth.token`, undefined) + log(chalk.green('Auth token moved to the OS keychain.')) + trackStored('keychain', true) + return true +} + +export const getStoredAPIToken = async (): Promise<{ token: string | undefined; fromKeychain: boolean }> => { + const globalConfig = await getGlobalConfigStore() + const userId = globalConfig.get('userId') as string | undefined + + if (isLegacyAuthStorageForced()) { + const token = await getAPIToken() + if (token) trackRead('legacy') + return { token, fromKeychain: false } + } + + if (userId) { + const keychainToken = await getTokenFromKeychain(userId) + if (keychainToken) { + trackRead('keychain') + return { token: keychainToken, fromKeychain: true } + } + } + + const legacyToken = await getAPIToken() + if (!legacyToken) { + return { token: undefined, fromKeychain: false } + } + + if (userId) { + const migrated = await attemptMigration(userId, legacyToken, globalConfig) + if (migrated) { + return { token: legacyToken, fromKeychain: true } + } + } + trackRead('legacy') + return { token: legacyToken, fromKeychain: false } +} + +export const writeAuthTokenForStorage = async ( + userId: string, + accessToken: string, +): Promise<{ mode: TokenStorageMode; keychainFailed: boolean }> => { + if (isLegacyAuthStorageForced() || !isMigrationAllowed()) { + trackStored('legacy', false) + return { mode: 'legacy', keychainFailed: false } + } + + const ok = await storeTokenInKeychain(userId, accessToken) + if (ok) { + trackStored('keychain', false) + return { mode: 'keychain', keychainFailed: false } + } + + trackStored('legacy', false, true) + return { mode: 'legacy', keychainFailed: true } +} diff --git a/src/utils/command-helpers.ts b/src/utils/command-helpers.ts index 0775c79f230..b05b4490887 100644 --- a/src/utils/command-helpers.ts +++ b/src/utils/command-helpers.ts @@ -4,12 +4,12 @@ import process from 'process' import { format, inspect } from 'util' import type { NetlifyAPI } from '@netlify/api' -import { getAPIToken } from '@netlify/dev-utils' import { Chalk, type ChalkInstance as ChalkInstancePrimitiveType } from 'chalk' import type { Option } from 'commander' import WSL from 'is-wsl' import terminalLink from 'terminal-link' +import { getStoredAPIToken } from '../lib/secure-storage.js' import { startSpinner } from '../lib/spinner.js' import getCLIPackageJson from './get-cli-package-json.js' @@ -141,10 +141,11 @@ export const getToken = async (tokenFromOptions?: string): Promise = if (NETLIFY_AUTH_TOKEN && NETLIFY_AUTH_TOKEN !== 'null') { return [NETLIFY_AUTH_TOKEN, 'env'] } - // 3. If no env var use global user setting - const tokenFromConfig = await getAPIToken() - if (tokenFromConfig) { - return [tokenFromConfig, 'config'] + // 3. If no env var use global user setting (keychain when secure storage is enabled, + // otherwise the plaintext token in the global config file) + const { token, fromKeychain } = await getStoredAPIToken() + if (token) { + return [token, fromKeychain ? 'keychain' : 'config'] } return [null, 'not found'] } diff --git a/src/utils/types.ts b/src/utils/types.ts index 556cf788d0f..f354f588d91 100644 --- a/src/utils/types.ts +++ b/src/utils/types.ts @@ -186,7 +186,7 @@ export interface SiteInfo { user_id: string } -export type TokenLocation = 'env' | 'flag' | 'config' | 'not found' +export type TokenLocation = 'env' | 'flag' | 'config' | 'keychain' | 'not found' export type EnvVar = { key: string diff --git a/tests/integration/commands/secure-storage/secure-storage.test.ts b/tests/integration/commands/secure-storage/secure-storage.test.ts new file mode 100644 index 00000000000..c7d2b516466 --- /dev/null +++ b/tests/integration/commands/secure-storage/secure-storage.test.ts @@ -0,0 +1,348 @@ +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import path from 'node:path' + +import execa from 'execa' +import { afterEach, beforeEach, describe, expect, test } from 'vitest' + +import { callCli } from '../../utils/call-cli.js' +import { cliPath } from '../../utils/cli-path.js' +import { CONFIRM, NO, handleQuestions } from '../../utils/handle-questions.js' +import { withMockApi } from '../../utils/mock-api.js' +import type { MinimalAccount } from '../../../../src/utils/types.js' + +interface StoredConfig { + telemetryDisabled?: boolean + cliId?: string + userId?: string + users?: Record + auth?: { keychainMigrationDeclined?: boolean } +} + +interface StatusJSON { + authTokenStorage: { source: string; configPath?: string } + account: Record + siteData: Record +} + +const readConfig = (configDir: string): StoredConfig => + JSON.parse(readFileSync(path.join(configDir, 'config.json'), 'utf8')) as StoredConfig + +const SERVICE_NAME = 'netlify-cli' + +const siteInfo = { + account_slug: 'test-account', + id: 'site_id', + name: 'site-name', + admin_url: 'https://app.netlify.com/projects/test-site/overview', + url: 'https://test-site.netlify.app/', +} +const user = { full_name: 'Test User', email: 'test@netlify.com', id: 'user-id' } +const accounts: MinimalAccount[] = [ + { + id: 'user-id', + name: user.full_name, + slug: siteInfo.account_slug, + default: true, + team_logo_url: null, + on_pro_trial: false, + organization_id: null, + type_name: 'placeholder', + type_slug: 'placeholder', + members_count: 1, + }, +] +const routes = [ + { path: 'sites/site_id', response: siteInfo }, + { path: 'sites/site_id/service-instances', response: [] }, + { path: 'accounts', response: accounts }, + { path: 'user', response: user }, +] + +const detectKeychainAvailability = async (): Promise => { + const keyring = await import('@napi-rs/keyring').catch(() => null) + if (!keyring) return false + try { + const probeAccount = `netlify-cli-test-probe-${String(Date.now())}` + const entry = new keyring.Entry(SERVICE_NAME, probeAccount) + entry.setPassword('ok') + const value = entry.getPassword() + entry.deletePassword() + return value === 'ok' + } catch { + return false + } +} + +const cleanupKeychainEntry = async (account: string): Promise => { + const keyring = await import('@napi-rs/keyring').catch(() => null) + if (!keyring) return + try { + new keyring.Entry(SERVICE_NAME, account).deletePassword() + } catch { + // ignore + } +} + +const readKeychainEntry = async (account: string): Promise => { + const keyring = await import('@napi-rs/keyring').catch(() => null) + if (!keyring) return null + try { + return new keyring.Entry(SERVICE_NAME, account).getPassword() + } catch { + return null + } +} + +const writeLegacyTokenToConfig = (configDir: string, userId: string, token: string) => { + mkdirSync(configDir, { recursive: true }) + const config: StoredConfig = { + telemetryDisabled: true, + cliId: 'test-cli-id', + userId, + users: { [userId]: { id: userId, auth: { token } } }, + } + writeFileSync(path.join(configDir, 'config.json'), JSON.stringify(config, null, '\t')) +} + +let tmpHome: string +let configDir: string +const accountsToCleanup: string[] = [] + +const isolatedEnv = (apiUrl: string, extra: Record = {}) => ({ + HOME: tmpHome, + USERPROFILE: tmpHome, + XDG_CONFIG_HOME: path.join(tmpHome, '.config'), + APPDATA: path.join(tmpHome, 'AppData', 'Roaming'), + PATH: process.env.PATH ?? '', + NETLIFY_SITE_ID: 'site_id', + NETLIFY_API_URL: apiUrl, + ...extra, +}) + +beforeEach(() => { + tmpHome = mkdtempSync(path.join(tmpdir(), 'nf-cli-secure-storage-it-')) + configDir = path.join(tmpHome, '.config', 'netlify') +}) + +afterEach(async () => { + rmSync(tmpHome, { recursive: true, force: true }) + await Promise.all(accountsToCleanup.splice(0).map(cleanupKeychainEntry)) +}) + +const keychainReady = await detectKeychainAvailability() + +describe('netlify status auth token storage', () => { + test('reports env source in JSON when NETLIFY_AUTH_TOKEN is set', async () => { + await withMockApi(routes, async ({ apiUrl }) => { + const json = (await callCli( + ['status', '--json'], + { env: isolatedEnv(apiUrl, { NETLIFY_AUTH_TOKEN: 'env-tok' }), extendEnv: false }, + true, + )) as StatusJSON + + expect(json.authTokenStorage).toEqual({ source: 'env' }) + expect(json.account.Email).toBe('test@netlify.com') + expect(json.account['Auth token storage']).toBeUndefined() + }) + }) + + test('reports env source in human output with a friendly label', async () => { + await withMockApi(routes, async ({ apiUrl }) => { + const stdout = (await callCli(['status'], { + env: isolatedEnv(apiUrl, { NETLIFY_AUTH_TOKEN: 'env-tok' }), + extendEnv: false, + })) as string + + expect(stdout).toContain('Auth token storage') + expect(stdout).toContain('NETLIFY_AUTH_TOKEN') + }) + }) + + test('reports config source with the config file path when reading from the legacy plaintext config', async () => { + writeLegacyTokenToConfig(configDir, user.id, 'plain-tok') + + await withMockApi(routes, async ({ apiUrl }) => { + const json = (await callCli( + ['status', '--json'], + { env: isolatedEnv(apiUrl, { NETLIFY_USE_LEGACY_AUTH_STORAGE: '1' }), extendEnv: false }, + true, + )) as StatusJSON + + expect(json.authTokenStorage.source).toBe('config') + expect(json.authTokenStorage.configPath).toMatch(/[\\/]netlify[\\/]config\.json$/) + expect(json.authTokenStorage.configPath?.startsWith(tmpHome)).toBe(true) + }) + }) + + test('reports flag source when --auth is used', async () => { + await withMockApi(routes, async ({ apiUrl }) => { + const json = (await callCli( + ['status', '--json', '--auth=flag-tok'], + { env: isolatedEnv(apiUrl), extendEnv: false }, + true, + )) as StatusJSON + + expect(json.authTokenStorage).toEqual({ source: 'flag' }) + }) + }) +}) + +describe('NETLIFY_USE_LEGACY_AUTH_STORAGE env var', () => { + test('forces the CLI to read from the legacy config and skip the keychain entirely', async () => { + writeLegacyTokenToConfig(configDir, user.id, 'plain-tok') + + await withMockApi(routes, async ({ apiUrl }) => { + const json = (await callCli( + ['status', '--json'], + { env: isolatedEnv(apiUrl, { NETLIFY_USE_LEGACY_AUTH_STORAGE: '1' }), extendEnv: false }, + true, + )) as StatusJSON + + expect(json.authTokenStorage.source).toBe('config') + const config = readConfig(configDir) + expect(config.users?.[user.id]?.auth?.token).toBe('plain-tok') + }) + }) +}) + +describe('migration prompt non-interactive safety', () => { + test('never prompts when the CLI is invoked with CI=true', async () => { + writeLegacyTokenToConfig(configDir, user.id, 'plain-tok') + + await withMockApi(routes, async ({ apiUrl }) => { + const stdout = (await callCli(['status'], { + env: isolatedEnv(apiUrl, { CI: 'true' }), + extendEnv: false, + })) as string + + expect(stdout).not.toContain('Move the token to the keychain') + const config = readConfig(configDir) + expect(config.users?.[user.id]?.auth?.token).toBe('plain-tok') + expect(config.auth?.keychainMigrationDeclined).not.toBe(true) + }) + }) + + test('never prompts when stdin is not a TTY (piped input)', async () => { + writeLegacyTokenToConfig(configDir, user.id, 'plain-tok') + + await withMockApi(routes, async ({ apiUrl }) => { + const { stdout } = await execa(cliPath, ['status'], { + env: isolatedEnv(apiUrl), + extendEnv: false, + timeout: 60_000, + input: '', + }) + + expect(stdout).not.toContain('Move the token to the keychain') + const config = readConfig(configDir) + expect(config.users?.[user.id]?.auth?.token).toBe('plain-tok') + }) + }) +}) + +describe('migration prompt interactive flow', () => { + test('declining the prompt persists the choice and leaves the legacy token in place', async () => { + writeLegacyTokenToConfig(configDir, user.id, 'plain-tok') + + await withMockApi(routes, async ({ apiUrl }) => { + const childProcess = execa(cliPath, ['status'], { + env: isolatedEnv(apiUrl, { TESTING_PROMPTS: 'true' }), + extendEnv: false, + timeout: 60_000, + }) + + handleQuestions(childProcess, [{ question: 'Move the token to the keychain', answer: NO + CONFIRM }]) + + await childProcess + + const config = readConfig(configDir) + expect(config.users?.[user.id]?.auth?.token).toBe('plain-tok') + expect(config.auth?.keychainMigrationDeclined).toBe(true) + }) + + await withMockApi(routes, async ({ apiUrl }) => { + const stdout = (await callCli(['status'], { + env: isolatedEnv(apiUrl, { TESTING_PROMPTS: 'true' }), + extendEnv: false, + })) as string + + expect(stdout).not.toContain('Move the token to the keychain') + }) + }) + + test.runIf(keychainReady)( + 'confirming the prompt moves the token to the keychain and removes it from the plaintext config', + async () => { + accountsToCleanup.push(user.id) + writeLegacyTokenToConfig(configDir, user.id, 'plain-tok') + + await withMockApi(routes, async ({ apiUrl }) => { + const childProcess = execa(cliPath, ['status'], { + env: isolatedEnv(apiUrl, { TESTING_PROMPTS: 'true' }), + extendEnv: false, + timeout: 60_000, + }) + + handleQuestions(childProcess, [{ question: 'Move the token to the keychain', answer: CONFIRM }]) + + await childProcess + + expect(await readKeychainEntry(user.id)).toBe('plain-tok') + const config = readConfig(configDir) + expect(config.users?.[user.id]?.auth?.token).toBeUndefined() + expect(config.auth?.keychainMigrationDeclined).not.toBe(true) + }) + }, + ) + + test.runIf(keychainReady)( + 'reports keychain source in status output after the token has been moved to the keychain', + async () => { + accountsToCleanup.push(user.id) + mkdirSync(configDir, { recursive: true }) + const config: StoredConfig = { + telemetryDisabled: true, + cliId: 'test-cli-id', + userId: user.id, + users: { [user.id]: { id: user.id, auth: {} } }, + } + writeFileSync(path.join(configDir, 'config.json'), JSON.stringify(config)) + + const keyring = await import('@napi-rs/keyring').catch(() => null) + if (!keyring) throw new Error('keychain reported available but module is missing') + new keyring.Entry(SERVICE_NAME, user.id).setPassword('keychain-tok') + + await withMockApi(routes, async ({ apiUrl }) => { + const json = (await callCli( + ['status', '--json'], + { env: isolatedEnv(apiUrl), extendEnv: false }, + true, + )) as StatusJSON + + expect(json.authTokenStorage).toEqual({ source: 'keychain' }) + + const stdout = (await callCli(['status'], { + env: isolatedEnv(apiUrl), + extendEnv: false, + })) as string + expect(stdout).toContain('native secure storage') + }) + }, + ) +}) + +describe('secure-storage command is removed', () => { + test('`secure-storage:status` no longer exists', async () => { + await expect( + callCli(['secure-storage:status'], { env: isolatedEnv('http://unused'), extendEnv: false }), + ).rejects.toThrow() + }) +}) + +test('integration tests never touch the real netlify config', () => { + const realConfig = process.env.HOME ? path.join(process.env.HOME, '.config', 'netlify', 'config.json') : null + if (!realConfig || !existsSync(realConfig)) return + const raw = readFileSync(realConfig, 'utf8') + expect(raw).not.toContain('test-cli-id') +}) diff --git a/tests/unit/lib/secure-storage.test.ts b/tests/unit/lib/secure-storage.test.ts new file mode 100644 index 00000000000..c32a6e307fa --- /dev/null +++ b/tests/unit/lib/secure-storage.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, test } from 'vitest' + +import { getLegacyConfigFilePath, isLegacyAuthStorageForced } from '../../../src/lib/secure-storage.js' + +describe('isLegacyAuthStorageForced', () => { + test('returns true when the env var is non-empty', () => { + const previous = process.env.NETLIFY_USE_LEGACY_AUTH_STORAGE + process.env.NETLIFY_USE_LEGACY_AUTH_STORAGE = '1' + try { + expect(isLegacyAuthStorageForced()).toBe(true) + } finally { + if (previous === undefined) delete process.env.NETLIFY_USE_LEGACY_AUTH_STORAGE + else process.env.NETLIFY_USE_LEGACY_AUTH_STORAGE = previous + } + }) + + test('returns false when the env var is unset', () => { + const previous = process.env.NETLIFY_USE_LEGACY_AUTH_STORAGE + delete process.env.NETLIFY_USE_LEGACY_AUTH_STORAGE + try { + expect(isLegacyAuthStorageForced()).toBe(false) + } finally { + if (previous !== undefined) process.env.NETLIFY_USE_LEGACY_AUTH_STORAGE = previous + } + }) + + test('returns false when the env var is empty', () => { + const previous = process.env.NETLIFY_USE_LEGACY_AUTH_STORAGE + process.env.NETLIFY_USE_LEGACY_AUTH_STORAGE = '' + try { + expect(isLegacyAuthStorageForced()).toBe(false) + } finally { + if (previous === undefined) delete process.env.NETLIFY_USE_LEGACY_AUTH_STORAGE + else process.env.NETLIFY_USE_LEGACY_AUTH_STORAGE = previous + } + }) +}) + +describe('getLegacyConfigFilePath', () => { + test('resolves to a config.json under the OS-appropriate config dir', () => { + const resolved = getLegacyConfigFilePath() + expect(resolved).toMatch(/[\\/]netlify[\\/]config\.json$/) + }) +})