Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/cli/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,11 @@ In addition to a project settings file, a project's `.llxprt` directory can cont
- **Default:** `false`
- **Requires restart:** Yes

- **`security.disableOsKeyring`** (boolean):
- **Description:** Disable use of the OS keyring/keychain for credential storage and use the encrypted file fallback. Can also be set with the LLXPRT_DISABLE_OS_KEYRING=1 environment variable.
- **Default:** `false`
- **Requires restart:** Yes

- **`security.enablePermanentToolApproval`** (boolean):
- **Description:** Enable the "Allow for all future sessions" option in tool confirmation dialogs.
- **Default:** `false`
Expand Down
13 changes: 13 additions & 0 deletions packages/cli/src/config/postConfigRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
type Config,
type ImageOperationBackend,
} from '@vybestack/llxprt-code-core';
import { setOsKeyringDisabledBySetting } from '@vybestack/llxprt-code-storage';
import { DebugLogger } from '@vybestack/llxprt-code-telemetry';
import { ProfileManager } from '@vybestack/llxprt-code-settings';
import type {
Expand Down Expand Up @@ -684,6 +685,18 @@ function finalizeMetadata(input: PostConfigInput): void {
* Step 17: finalizeMetadata() — seed default disabled tools, store model params, store bootstrap args, log warnings
*/
export async function finalizeConfig(input: PostConfigInput): Promise<Config> {
// Propagate security.disableOsKeyring into the storage package's process-wide
// opt-out (issue #2928 R3.2) BEFORE any profile/auth application. Profile
// auth wiring (applyProfileToRuntime → createProviderKeyStorage().getKey())
// performs a real SecureStore read during steps 12-13 below, so this MUST run
// first to suppress the OS keyring before that first read — otherwise a user
// who sets security.disableOsKeyring still gets a Keychain prompt at startup.
// The env var LLXPRT_DISABLE_OS_KEYRING=1 is independent and read directly in
// storage, so it keeps working with zero CLI involvement.
setOsKeyringDisabledBySetting(
input.profileSettingsWithTools.security?.disableOsKeyring === true,
);

// Step 10-11: Set runtime context + re-register provider infra
await setupRuntimeContext(input);

Expand Down
10 changes: 10 additions & 0 deletions packages/cli/src/config/settings-schema/schema-extensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,16 @@ export const EXTENSION_SETTINGS_SCHEMA = {
description: 'Disable YOLO mode, even if enabled by a flag.',
showInDialog: true,
},
disableOsKeyring: {
type: 'boolean',
label: 'Disable OS Keyring',
category: 'Security',
requiresRestart: true,
default: false,
description:
'Disable use of the OS keyring/keychain for credential storage and use the encrypted file fallback. Can also be set with the LLXPRT_DISABLE_OS_KEYRING=1 environment variable.',
showInDialog: true,
},
enablePermanentToolApproval: {
type: 'boolean',
label: 'Allow Permanent Tool Approval',
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/config/settings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ describe('Settings Loading and Merging', () => {
});
expect(settings.merged.security).toStrictEqual({
disableYoloMode: false,
disableOsKeyring: false,
folderTrust: { enabled: false },
auth: {},
blockGitExtensions: false,
Expand Down
2 changes: 1 addition & 1 deletion packages/storage/bunfig.toml
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
[test]
preload = ["../../test-setup/augment-bun-vi.ts", "./test-setup-storage-isolation.ts"]
preload = ["../../test-setup/augment-bun-vi.ts", "./test-setup-storage-isolation.ts", "./test-setup-bun-session-reset.ts"]
Comment thread
acoliver marked this conversation as resolved.
4 changes: 4 additions & 0 deletions packages/storage/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ export {
isSecureStoreError,
isRuntimeReplacedError,
} from './secure-store/secure-store-errors.js';
// OS keyring opt-out setter for the CLI settings bridge (issue #2928 R3.2).
// Storage is a low-level package and must not read CLI settings; the CLI
// pushes the resolved setting in here.
export { setOsKeyringDisabledBySetting } from './secure-store/keyring-session-state.js';
export {
isKeychainGrantPersistenceBroken,
GRANT_NOT_PERSISTING_MESSAGE,
Expand Down
104 changes: 104 additions & 0 deletions packages/storage/src/secure-store/classify-error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/**
* @license
* Copyright 2026 Vybestack LLC
* SPDX-License-Identifier: Apache-2.0
*/

/**
* Dependency-leaf error classifier for OS keyring errors (issue #2928).
*
* Extracted from secure-store.ts so the adapter boundary
* (default-keyring-adapter.ts → createGuardedAdapter) can classify and latch
* on keyring errors WITHOUT importing from secure-store.ts (which would create
* a cycle, since secure-store.ts imports default-keyring-adapter.ts). Imports
* only `SecureStoreErrorCode` / `isSecureStoreError` from secure-store-errors.ts
* (a true dependency leaf), so it introduces no cycles.
*
* @plan PLAN-20260805-ISSUE2928
* @requirement R1.1, R2.5
*/

import {
isSecureStoreError,
type SecureStoreErrorCode,
} from './secure-store-errors.js';

function isErrorWithCode(value: unknown): value is { code: string } {
return (
typeof value === 'object' &&
value !== null &&
'code' in value &&
typeof value.code === 'string'
);
}

/**
* Classifies an unknown thrown value into a SecureStoreErrorCode using message
* heuristics. A SecureStoreError already carries an authoritative code and is
* returned as-is.
*
* Ordering matters: the "access platform storage" check runs BEFORE the
* cancellation/denied checks so a headless "no Secret Service" machine is
* classified UNAVAILABLE (degradable) rather than DENIED (latching).
*
* The cancellation test is narrowly targeted at genuine
* USER cancellation only. A bare `msg.includes('cancel')` would also match
* abort/timeout text such as "request cancelled due to timeout"
* (@napi-rs/keyring accepts an AbortSignal on every method), which would
* irreversibly latch the keyring off for the whole process. Only the macOS
* status names and explicit user-cancellation phrasing match.
*
* @plan PLAN-20260805-ISSUE2928
* @requirement R1.1
*/
// Read-path limitation: @napi-rs/keyring (every published version through
// 1.3.0, the latest) does `Ok(self.inner.get_password().ok())` in
// PasswordTask::compute, discarding the OSStatus, so getPassword returns null
// for BOTH a denial and a genuine absence. The classifications below therefore
// only fire on the write, delete-verification and probe paths, which do
// propagate errors. Recovering read-path fidelity requires changing the native
// binding — tracked in issue #3067.
export function classifyError(error: unknown): SecureStoreErrorCode {
// A SecureStoreError already carries an authoritative classification.
// RUNTIME_REPLACED in particular matches none of the message heuristics
// below and would be downgraded to UNAVAILABLE, which the get()/has()
// fallback paths are allowed to swallow — absorbing a terminal error that
// the runtime-replaced invariant requires callers to rethrow.
if (isSecureStoreError(error)) {
return error.code;
}
const msg =
error instanceof Error
? error.message.toLowerCase()
: String(error).toLowerCase();
// "Couldn't access platform storage: PermissionDenied" is what the keyring
// crate reports when the machine has no Secret Service at all — a headless
// Linux box, container, ssh session or WSL. Despite the wording it means "no
// credential backend here", not "you lack permission to use one", so it has
// to be classified UNAVAILABLE and degrade to the encrypted file. Checked
// before the generic denied/permission test below, which would otherwise
// match on the substring and turn a routine no-keyring machine into a hard
// error.
if (msg.includes('access platform storage')) return 'UNAVAILABLE';
// macOS errSecUserCanceled and explicit user-cancellation messages. Narrowly
// targeted: a bare "cancel" substring would also match abort/timeout
// messages like "request cancelled due to timeout", which would irreversibly
// latch the keyring off for the whole process. @napi-rs/keyring accepts an
// AbortSignal on every method, so abort-related text must NOT latch. Only
// genuine USER cancellation matches. `cancell?ed` covers both the US
// "canceled" and the British "cancelled" spellings.
if (
msg.includes('errsecusercanceled') ||
msg.includes('errseccanceled') ||
/\buser cancell?ed\b/.test(msg) ||
/cancell?ed by the user\b/.test(msg)
) {
return 'DENIED';
}
if (msg.includes('locked')) return 'LOCKED';
if (msg.includes('denied') || msg.includes('permission')) return 'DENIED';
if (msg.includes('timeout') || msg.includes('timed out')) return 'TIMEOUT';
if (msg.includes('not found')) return 'NOT_FOUND';
if (isErrorWithCode(error) && error.code === 'ENOENT') return 'NOT_FOUND';
return 'UNAVAILABLE';
}
Loading
Loading