From 6a2cb794000a8b1f673e64bf623caf6c7f77a1dc Mon Sep 17 00:00:00 2001 From: Philippe Serhal Date: Tue, 26 May 2026 18:28:50 -0400 Subject: [PATCH] feat: store auth token in native OS keychain by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Netlify auth token is stored in plaintext in the global netlify config file (e.g. `~/Library/Preferences/netlify/config.json` on macOS), which is readable by any process running as the same user. Malicious or compromised tools on developer machines may attempt to steal these tokens. This change adds support for storing and retrieving the token in the OS keychain (macOS Keychain, Windows Credential Manager, Linux Secret Service via libsecret). This change is enabled by default, with an opt-out available (someday we may remove support for plaintext storage). On `netlify login`, the token now goes to the keychain via `@napi-rs/keyring`. If the keychain isn't available for any reason, we fall back to the plaintext file and warn so the user knows. For users who already have a token in the plaintext config, the first time `getToken()` runs in an interactive shell we explicitly prompt them: ``` Your Netlify auth token is currently stored in plaintext at /Users/…/Library/Preferences/netlify/config.json. The CLI can move it to your OS keychain (more secure). Your operating system may prompt you to allow access. ? Move the token to the keychain now? (Y/n) ``` "Yes" migrates and clears the plaintext entry. "No" is persisted (`auth.keychainMigrationDeclined`) so we don't nag again, with a hint to run `netlify logout && netlify login` to revisit at a later time. `NETLIFY_USE_LEGACY_AUTH_STORAGE=1` skips the keychain entirely on both read and write. Something I was concerned about: Agents, scripts, and CI should never see an OS keychain dialog they didn't ask for, as they likely won't be able to respond interactively. Three gates for this: 1. The migration prompt is gated on `isInteractive()` (TTY + not CI). 2. The keychain write in `writeAuthTokenForStorage` is also gated on `isInteractive()`, so even a non-interactive login (unusual but possible; people do all sorts of things in their CI workflows) won't trigger the first-write OS dialog. 3. The keyring library itself fails closed; e.g. on Linux without a daemon, `setPassword` throws `AccessDenied` synchronously and we fall back. I added some output to `netlify status` (both human format and `--json`) that indicates the token storage mechanism and location. I added some telemetry so we can track migration progress: - `user_authTokenStored` (mode, migrated, keychainFailed): fires on login and on successful migration - `user_authTokenRead` (mode): fires when a stored token is returned - `user_authTokenMigrationDeclined`: user explicitly said no --- .github/workflows/integration-tests.yml | 5 +- package-lock.json | 220 +++++++++++ package.json | 1 + src/commands/base-command.ts | 19 +- src/commands/login/login-check.ts | 11 +- src/commands/login/login.ts | 2 + src/commands/logout/logout.ts | 7 +- src/commands/status/status.ts | 25 +- src/lib/secure-storage.ts | 201 ++++++++++ src/utils/command-helpers.ts | 11 +- src/utils/types.ts | 2 +- .../secure-storage/secure-storage.test.ts | 348 ++++++++++++++++++ tests/unit/lib/secure-storage.test.ts | 44 +++ 13 files changed, 880 insertions(+), 16 deletions(-) create mode 100644 src/lib/secure-storage.ts create mode 100644 tests/integration/commands/secure-storage/secure-storage.test.ts create mode 100644 tests/unit/lib/secure-storage.test.ts 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$/) + }) +})