Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
52 changes: 52 additions & 0 deletions src/__tests__/bugfixes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -695,3 +695,55 @@ describe('Dev-channels dialog coverage', () => {
)
})
})

// ---------------------------------------------------------------------------
// Fix: onboarding + trust dialog skipped entirely for third-party providers
// ---------------------------------------------------------------------------
// Behavioral coverage lives in src/utils/setupScreenGates.test.ts — the
// gating decisions were extracted into that provider-free seam because this
// module's import chain cannot be loaded under bun test (compile-time
// feature() macro checker, same constraint as the dev-channels tests above).
// These wiring checks assert showSetupScreens actually consults the seam and
// that no provider gate was re-introduced around the dialogs.
describe('Onboarding and trust dialog — third-party providers', () => {
test('showSetupScreens routes both dialogs through the provider-free seam', async () => {
const content = await file('interactiveHelpers.tsx').text()

expect(content).toContain('getRequiredSetupScreens({')
expect(content).toContain('if (setupScreens.onboarding)')
expect(content).toContain('if (setupScreens.trustDialog)')
})

test('the env-config option never renders the raw endpoint', async () => {
// OPENAI_BASE_URL/OPENAI_API_BASE can carry credentials (userinfo or
// token query params) and everything rendered lands in terminal
// scrollback. Redaction behavior is tested in envProviderOption.test.ts;
// this guards the wiring — the raw `envBaseUrl` may only reach profile
// persistence (addProviderProfile/getProviderProfiles/label), never a
// rendered label or status message.
const content = await file('components/ConsoleOAuthFlow.tsx').text()

expect(content).toContain('getEnvProviderOption()')
// Rendered sites use the redacted value.
expect(content).toMatch(/\{envBaseUrlVarName\}=\{envBaseUrlForDisplay\}/)
expect(content).toMatch(/\$\{envBaseUrlForDisplay\}\) as your active provider/)
// No rendered site interpolates the raw endpoint.
expect(content).not.toMatch(/\{envBaseUrl\}/)
expect(content).not.toMatch(/\$\{envBaseUrl\}/)
})

test('no dialog is gated behind usesAnthropicSetup', async () => {
const content = await file('interactiveHelpers.tsx').text()

// Theme choice + security notes are universal, and workspace trust is
// orthogonal to the API provider: an untrusted repo is exactly as
// dangerous over a local model as over Anthropic. The seam takes no
// provider input, so the only way to regress is to add a gate at the
// call sites — which this guards against.
expect(content).not.toMatch(/usesAnthropicSetup\s*&&\s*\(?\s*setupScreens/)
expect(content).not.toMatch(/usesAnthropicSetup\s*&&\s*\(\s*!config\.theme/)
expect(content).not.toMatch(
/usesAnthropicSetup\s*&&\s*!checkHasTrustDialogAccepted/,
)
})
})
106 changes: 106 additions & 0 deletions src/components/ConsoleOAuthFlow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,15 @@ import { sendNotification } from '../services/notifier.js';
import { OAuthService } from '../services/oauth/index.js';
import { getOauthAccountInfo, validateForceLoginOrg } from '../utils/auth.js';
import { logError } from '../utils/log.js';
import { getEnvProviderOption } from '../utils/envProviderOption.js';
import { getLocalOpenAICompatibleProviderLabel } from '../utils/providerDiscovery.js';
import { type ProviderProfile } from '../utils/config.js';
import {
addProviderProfile,
getProviderProfiles,
setActiveProviderProfile,
updateProviderProfile,
} from '../utils/providerProfiles.js';
import { getSettings_DEPRECATED } from '../utils/settings/settings.js';
import { ProviderManager } from './ProviderManager.js';
import { Select } from './CustomSelect/select.js';
Expand Down Expand Up @@ -386,7 +395,42 @@ function OAuthStatusMessage({
startingMessage ||
'OpenClaude can be used with your Claude subscription or billed based on API usage through your Console account.'

// OPENAI_BASE_URL/OPENAI_MODEL in the environment signal an
// OpenAI-compatible setup the user already has — offer to adopt it as
// the active provider profile instead of walking them through login for
// an account they may never have wanted. Env vars alone do NOT activate
// the route (resolveActiveRouteIdFromEnv requires CLAUDE_CODE_USE_OPENAI
// or a saved profile), so selecting this saves + activates a profile.
// Both fields gate the option because a profile requires baseUrl+model.
// getEnvProviderOption owns the secret-disclosure boundary: only
// `displayBaseUrl` (redacted) may be rendered — the raw `baseUrl`
// exists solely for profile creation/activation. See its tests for
// the credential cases.
const {
available: envConfigAvailable,
varName: envBaseUrlVarName,
baseUrl: envBaseUrl,
displayBaseUrl: envBaseUrlForDisplay,
model: envModel,
} = getEnvProviderOption()

const loginOptions = [
...(envConfigAvailable
? [
{
label: (
<Text>
Use current environment configuration ·{' '}
<Text dimColor>
{envBaseUrlVarName}={envBaseUrlForDisplay}
</Text>
{'\n'}
</Text>
),
value: 'environment' as const,
},
]
: []),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
{
label: (
<Text>
Expand Down Expand Up @@ -427,6 +471,68 @@ function OAuthStatusMessage({
<Select
options={loginOptions}
onChange={value => {
if (value === 'environment') {
// Re-entering this flow with the same env must not stack
// near-identical profiles: reuse (and activate) an existing
// profile matching the env base URL + model before creating
// a new one. setActiveProviderProfile also re-applies the
// profile env and syncs the startup profile file.
const existing = getProviderProfiles().find(
profile =>
profile.baseUrl?.trim() === envBaseUrl?.trim() &&
profile.model?.trim() === envModel?.trim(),
)
let saved: ProviderProfile | null
if (existing) {
// Refresh the stored credential from the environment
// before activating: the env is the source of truth the
// user just chose, so a rotated OPENAI_API_KEY must not
// leave the profile running on a stale key. Keep the
// existing key when the env no longer carries one rather
// than blanking a working credential.
//
// updateProviderProfile REPLACES the profile (toProfile
// builds a fresh object, it does not merge), so spread
// the existing profile first — otherwise a configured
// apiFormat / auth header / customHeaders / context
// length would be silently dropped on refresh.
const refreshed = updateProviderProfile(existing.id, {
...existing,
apiKey: process.env.OPENAI_API_KEY ?? existing.apiKey,
})
if (!refreshed) {
// The env values failed profile validation. Activating
// now would claim a refresh that did not happen, so send
// the user to guided setup instead of silently running
// on the stale credential.
setOAuthStatus({ state: 'platform_setup' })
return
}
saved = setActiveProviderProfile(existing.id)
} else {
saved = addProviderProfile(
{
name: getLocalOpenAICompatibleProviderLabel(envBaseUrl),
baseUrl: envBaseUrl as string,
model: envModel as string,
apiKey: process.env.OPENAI_API_KEY,
},
{ makeActive: true },
)
}
if (!saved) {
// Env values failed profile validation — fall back to the
// guided provider setup with fields prefilled from env.
setOAuthStatus({ state: 'platform_setup' })
return
}
logEvent('tengu_oauth_env_config_selected', {})
setOAuthStatus({
state: 'platform_setup_complete',
message: `${existing ? 'Activated' : 'Saved'} ${saved.name} (${envBaseUrlForDisplay}) as your active provider.`,
})
return
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if (value === 'platform') {
logEvent('tengu_oauth_platform_selected', {})
setOAuthStatus({ state: 'platform_setup' })
Expand Down
26 changes: 20 additions & 6 deletions src/interactiveHelpers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { onChangeAppState } from './state/onChangeAppState.js';
import { normalizeApiKeyForConfig } from './utils/authPortable.js';
import { getExternalClaudeMdIncludes, getMemoryFiles, shouldShowClaudeMdExternalIncludesWarning } from './utils/claudemd.js';
import { checkHasTrustDialogAccepted, getCustomApiKeyStatus, getGlobalConfig, saveGlobalConfig } from './utils/config.js';
import { getRequiredSetupScreens } from './utils/setupScreenGates.js';
import { updateDeepLinkTerminalPreference } from './utils/deepLink/terminalPreference.js';
import { isEnvTruthy, isRunningOnHomespace } from './utils/envUtils.js';
import { type FpsMetrics, FpsTracker } from './utils/fpsTracker.js';
Expand Down Expand Up @@ -114,9 +115,20 @@ export async function showSetupScreens(root: Root, permissionMode: PermissionMod
const config = getGlobalConfig();
let onboardingShown = false;

// Skip onboarding dialog for third-party providers (no Anthropic account needed)
if (usesAnthropicSetup && (!config.theme || !config.hasCompletedOnboarding) // always show onboarding at least once
) {
// Onboarding runs for ALL providers: theme + security notes are universal,
// and the component itself drops the preflight/OAuth steps when Anthropic
// auth is not enabled (see oauthEnabled in Onboarding.tsx). Gating this on
// the Anthropic account flow left third-party users with no theme choice
// and, worse, no prompt-injection/safety notes. The decisions live in the
// provider-free setupScreenGates seam (behaviorally tested there — this
// module's import chain cannot be loaded under bun test).
const setupScreens = getRequiredSetupScreens({
theme: config.theme,
hasCompletedOnboarding: config.hasCompletedOnboarding,
trustDialogAccepted: checkHasTrustDialogAccepted(),
isClaubbit: isEnvTruthy(process.env.CLAUBBIT),
});
if (setupScreens.onboarding) {
onboardingShown = true;
const {
Onboarding
Expand All @@ -136,9 +148,11 @@ export async function showSetupScreens(root: Root, permissionMode: PermissionMod
// Note: non-interactive sessions (CI/CD with -p) never reach showSetupScreens at all.
// Skip permission checks in claubbit
if (!isEnvTruthy(process.env.CLAUBBIT)) {
// Skip trust dialog UI for third-party providers (no Anthropic auth), but still
// run trust state initialization below so the REPL mounts correctly.
if (usesAnthropicSetup && !checkHasTrustDialogAccepted()) {
// The trust dialog is the workspace trust boundary — it has nothing to do
// with which API provider is configured, so it runs for third-party
// providers too (an untrusted repo is exactly as dangerous over Ollama as
// over Anthropic).
if (setupScreens.trustDialog) {
const {
TrustDialog
} = await import('./components/TrustDialog/TrustDialog.js');
Expand Down
89 changes: 89 additions & 0 deletions src/utils/envProviderOption.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import { describe, expect, test } from 'bun:test'

import { getEnvProviderOption } from './envProviderOption.js'

// Secret-disclosure regression coverage: OPENAI_BASE_URL / OPENAI_API_BASE
// are credential-bearing in the wild, and everything the onboarding option
// renders lands in terminal scrollback. displayBaseUrl must never carry the
// secret; baseUrl must stay intact so the saved profile still works.

describe('getEnvProviderOption — credential redaction', () => {
test('URL userinfo is redacted for display but preserved for the profile', () => {
const raw = 'https://svc-user:sup3r-s3cret@api.example.com/v1'
const option = getEnvProviderOption({
OPENAI_BASE_URL: raw,
OPENAI_MODEL: 'gpt-4o',
})

expect(option.displayBaseUrl).not.toContain('sup3r-s3cret')
expect(option.displayBaseUrl).not.toContain('svc-user')
expect(option.displayBaseUrl).toContain('api.example.com')
// The working URL is untouched — the profile must still authenticate.
expect(option.baseUrl).toBe(raw)
expect(option.available).toBe(true)
})

test('sensitive query parameters are redacted for display', () => {
const raw = 'https://api.example.com/v1?token=abcd1234secret&api_key=zzz9999'
const option = getEnvProviderOption({
OPENAI_BASE_URL: raw,
OPENAI_MODEL: 'gpt-4o',
})

expect(option.displayBaseUrl).not.toContain('abcd1234secret')
expect(option.displayBaseUrl).not.toContain('zzz9999')
expect(option.displayBaseUrl).toContain('api.example.com')
expect(option.baseUrl).toBe(raw)
})

test('a credential-bearing OPENAI_API_BASE is redacted and named correctly', () => {
const raw = 'https://user:pass@gateway.internal:8443/v1'
const option = getEnvProviderOption({
OPENAI_API_BASE: raw,
OPENAI_MODEL: 'llama3',
})

expect(option.varName).toBe('OPENAI_API_BASE')
expect(option.displayBaseUrl).not.toContain('pass')
expect(option.baseUrl).toBe(raw)
})

test('a plain endpoint passes through unchanged', () => {
const option = getEnvProviderOption({
OPENAI_BASE_URL: 'http://localhost:11434/v1',
OPENAI_MODEL: 'llama3',
})
expect(option.displayBaseUrl).toContain('localhost:11434')
expect(option.varName).toBe('OPENAI_BASE_URL')
})

test('a malformed endpoint still does not leak userinfo', () => {
// redactUrlForDisplay has a non-URL fallback path; the option must not
// regress to echoing the raw string when parsing fails.
const option = getEnvProviderOption({
OPENAI_BASE_URL: 'not a url://user:secret-pw@host/v1',
OPENAI_MODEL: 'gpt-4o',
})
expect(option.displayBaseUrl).not.toContain('secret-pw')
})
})

describe('getEnvProviderOption — availability and var naming', () => {
test('OPENAI_BASE_URL wins over OPENAI_API_BASE and is named as such', () => {
const option = getEnvProviderOption({
OPENAI_BASE_URL: 'https://primary.example.com/v1',
OPENAI_API_BASE: 'https://fallback.example.com/v1',
OPENAI_MODEL: 'gpt-4o',
})
expect(option.baseUrl).toBe('https://primary.example.com/v1')
expect(option.varName).toBe('OPENAI_BASE_URL')
})

test('a profile needs both a base URL and a model', () => {
expect(
getEnvProviderOption({ OPENAI_BASE_URL: 'https://x.example/v1' }).available,
).toBe(false)
expect(getEnvProviderOption({ OPENAI_MODEL: 'gpt-4o' }).available).toBe(false)
expect(getEnvProviderOption({}).available).toBe(false)
})
})
40 changes: 40 additions & 0 deletions src/utils/envProviderOption.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { redactUrlForDisplay } from './redaction.js'

/**
* Derivation for the "use current environment configuration" onboarding
* option (ConsoleOAuthFlow), extracted as a pure seam so the
* secret-disclosure boundary is regression-tested directly rather than
* asserted against component source text.
*
* The split between `baseUrl` and `displayBaseUrl` is load-bearing:
* OPENAI_BASE_URL / OPENAI_API_BASE are credential-bearing in the wild
* (userinfo like https://user:pass@host/v1, or ?token=/?api_key= query
* params). Anything rendered lands in terminal scrollback, so ONLY
* `displayBaseUrl` may reach the UI; `baseUrl` keeps the working URL for
* profile creation/activation.
*/
export type EnvProviderOption = {
/** True when both a base URL and a model are present (a profile needs both). */
available: boolean
/** The env var the base URL actually came from, for accurate troubleshooting. */
varName: 'OPENAI_BASE_URL' | 'OPENAI_API_BASE'
/** Raw endpoint — for profile persistence/activation only, never rendered. */
baseUrl: string | undefined
/** Redacted endpoint — the only form safe to render. */
displayBaseUrl: string | undefined
model: string | undefined
}

export function getEnvProviderOption(
processEnv: NodeJS.ProcessEnv = process.env,
): EnvProviderOption {
const baseUrl = processEnv.OPENAI_BASE_URL ?? processEnv.OPENAI_API_BASE
const model = processEnv.OPENAI_MODEL
return {
available: Boolean(baseUrl && model),
varName: processEnv.OPENAI_BASE_URL ? 'OPENAI_BASE_URL' : 'OPENAI_API_BASE',
baseUrl,
displayBaseUrl: baseUrl ? redactUrlForDisplay(baseUrl) : baseUrl,
model,
}
}
Loading