Skip to content
Open
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
116 changes: 92 additions & 24 deletions server/modules/providers/list/claude/claude-auth.provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,53 +100,121 @@ export class ClaudeProviderAuth implements IProviderAuth {
return { authenticated: true, email: 'Configured via settings.json', method: 'api_key' };
}

// Fall back to a plaintext credentials file if one exists (older Claude
// Code versions, or platforms where OAuth creds aren't OS-keychain backed).
const fileCredentials = await this.readCredentialsFile();
if (fileCredentials) {
return fileCredentials;
}

// Claude Code stores OAuth credentials in a macOS Keychain entry whose
// service name is derived from CLAUDE_CONFIG_DIR (a different, hashed
// service name per profile) rather than a file we can read directly.
// That derivation isn't a stable contract we should reimplement, so defer
// to the CLI's own `claude auth status`, which already resolves it
// correctly (file, keychain, or otherwise) for whichever CLAUDE_CONFIG_DIR
// is active in this process's environment.
return this.checkCliAuthStatus(missingCredentialsError);
}

/**
* Reads the plaintext OAuth credentials file, when Claude Code uses one.
* Returns null (rather than an "unauthenticated" result) when the file is
* simply absent, so the caller can fall back to `claude auth status`.
*/
private async readCredentialsFile(): Promise<ClaudeCredentialsStatus | null> {
try {
const credPath = path.join(os.homedir(), '.claude', '.credentials.json');
const content = await readFile(credPath, 'utf8');
const creds = readObjectRecord(JSON.parse(content)) ?? {};
const oauth = readObjectRecord(creds.claudeAiOauth);
const accessToken = readOptionalString(oauth?.accessToken);

if (accessToken) {
const expiresAt = typeof oauth?.expiresAt === 'number' ? oauth.expiresAt : undefined;
const email = readOptionalString(creds.email) ?? readOptionalString(creds.user) ?? null;
if (!expiresAt || Date.now() < expiresAt) {
return {
authenticated: true,
email,
method: 'credentials_file',
};
}

return {
authenticated: false,
email: null,
method: null,
error: 'Claude login has expired. Run claude /login again.',
};
if (!accessToken) {
return null;
}

const expiresAt = typeof oauth?.expiresAt === 'number' ? oauth.expiresAt : undefined;
const email = readOptionalString(creds.email) ?? readOptionalString(creds.user) ?? null;
if (!expiresAt || Date.now() < expiresAt) {
return { authenticated: true, email, method: 'credentials_file' };
}

return {
authenticated: false,
email: null,
method: null,
error: missingCredentialsError,
error: 'Claude login has expired. Run claude /login again.',
};
} catch (error) {
let errorMessage = 'Unable to read Claude credentials. Run claude /login again.';

if (hasErrorCode(error, 'ENOENT')) {
errorMessage = missingCredentialsError;
} else if (error instanceof SyntaxError) {
errorMessage = 'Claude credentials are unreadable. Run claude /login again.';
return null;
}

return {
authenticated: false,
email: null,
method: null,
error: errorMessage,
error: error instanceof SyntaxError
? 'Claude credentials are unreadable. Run claude /login again.'
: 'Unable to read Claude credentials. Run claude /login again.',
};
}
}

/**
* Resolves OAuth login state via `claude auth status --json`, inheriting
* this process's environment (including CLAUDE_CONFIG_DIR) so it reports
* the profile actually in use rather than always the default one.
*/
private async checkCliAuthStatus(missingCredentialsError: string): Promise<ClaudeCredentialsStatus> {
const cliPath = resolveClaudeCodeExecutablePath(process.env.CLAUDE_CLI_PATH);

let result;
try {
result = spawn.sync(cliPath, ['auth', 'status', '--json'], {
timeout: 10000,
encoding: 'utf8',
env: process.env,
});
} catch {
return {
authenticated: false,
email: null,
method: null,
error: 'Unable to check Claude authentication status. Run claude /login again.',
};
}

const stdout = typeof result.stdout === 'string' ? result.stdout.trim() : '';
if (!stdout) {
return { authenticated: false, email: null, method: null, error: missingCredentialsError };
}
Comment on lines +173 to +192

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Avoid blocking the event loop and handle execution errors correctly.

Using synchronous spawn.sync inside an async function blocks the Node.js main thread for up to 10 seconds (the configured timeout). In a server environment, this stalls all concurrent requests and degrades availability.

Additionally, spawn.sync does not throw an exception when it fails to start the process (e.g., missing executable) or when it times out; it returns an error in result.error. The current try/catch block will not catch these failures, causing the code to swallow the error and fall through to reporting a misleading "missing credentials" error due to an empty stdout.

Consider replacing spawn.sync with an asynchronous alternative (such as child_process.execFile wrapped in util.promisify), which correctly throws on spawn failures, timeouts, and non-zero exit codes without blocking the event loop.

⚡ Proposed fix using promisified `execFile`

Assuming you have access to a promisified execFile (e.g., const execFileAsync = util.promisify(require('child_process').execFile)):

-    let result;
+    let stdout = '';
     try {
-      result = spawn.sync(cliPath, ['auth', 'status', '--json'], {
+      const result = await execFileAsync(cliPath, ['auth', 'status', '--json'], {
         timeout: 10000,
         encoding: 'utf8',
         env: process.env,
       });
+      stdout = result.stdout.trim();
     } catch {
       return {
         authenticated: false,
         email: null,
         method: null,
         error: 'Unable to check Claude authentication status. Run claude /login again.',
       };
     }
 
-    const stdout = typeof result.stdout === 'string' ? result.stdout.trim() : '';
     if (!stdout) {
       return { authenticated: false, email: null, method: null, error: missingCredentialsError };
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/modules/providers/list/claude/claude-auth.provider.ts` around lines
173 - 192, Replace the synchronous spawn.sync call in the Claude authentication
status flow with an asynchronous execFile-based implementation, using the
existing async function context and preserving the 10-second timeout, JSON
output, environment, and argument handling. Ensure spawn failures, timeouts, and
non-zero exits are caught and mapped to the existing authentication error
response rather than falling through to missingCredentialsError; update
result.stdout handling to match the asynchronous API.


let parsed: Record<string, unknown>;
try {
parsed = readObjectRecord(JSON.parse(stdout)) ?? {};
} catch {
return {
authenticated: false,
email: null,
method: null,
error: 'Unable to parse Claude authentication status. Run claude /login again.',
};
}

if (parsed.loggedIn !== true) {
return { authenticated: false, email: null, method: null, error: missingCredentialsError };
}

const orgName = readOptionalString(parsed.orgName);
const email = readOptionalString(parsed.email) ?? (orgName ? `Authenticated (${orgName})` : null);
const authMethod = readOptionalString(parsed.authMethod);

return {
authenticated: true,
email,
method: authMethod === 'claude.ai' ? 'oauth' : (readOptionalString(parsed.apiProvider) ?? 'api_key'),
};
}
}