From cb92f4b6ce91e8c023a6b164da9bb3db0709afeb Mon Sep 17 00:00:00 2001 From: betamod Date: Mon, 13 Apr 2026 18:00:21 -0400 Subject: [PATCH] Harden secret storage and debug logging --- manifest.json | 2 +- src/acp/acp-client.ts | 29 +++++++-- src/acp/acp-handler.ts | 14 +++- src/acp/permission-handler.ts | 13 +++- src/acp/terminal-handler.ts | 10 ++- src/plugin.ts | 118 ++++++++++++++++++++++++++++++++-- src/types/agent.ts | 6 ++ src/ui/SettingsTab.ts | 26 +++++--- 8 files changed, 191 insertions(+), 27 deletions(-) diff --git a/manifest.json b/manifest.json index 424cd51..2f05860 100644 --- a/manifest.json +++ b/manifest.json @@ -2,7 +2,7 @@ "id": "agent-client", "name": "Agent Client", "version": "0.10.0", - "minAppVersion": "0.15.0", + "minAppVersion": "1.11.4", "description": "Chat with AI agents via the Agent Client Protocol directly from your vault.", "author": "RAIT-09", "authorUrl": "https://github.com/RAIT-09", diff --git a/src/acp/acp-client.ts b/src/acp/acp-client.ts index a09d8e9..8c049d0 100644 --- a/src/acp/acp-client.ts +++ b/src/acp/acp-client.ts @@ -110,7 +110,7 @@ export class AcpClient { async initialize(config: AgentConfig): Promise { this.logger.log( "[AcpClient] Starting initialization with config:", - config, + this.getSafeConfigForLog(config), ); this.logger.log( `[AcpClient] Current state - process: ${!!this.agentProcess}, PID: ${this.agentProcess?.pid}`, @@ -204,7 +204,7 @@ export class AcpClient { this.logger.log( "[AcpClient] Prepared spawn command:", spawnCommand, - spawnArgs, + `(${spawnArgs.length} args)`, ); // Spawn the agent process @@ -299,9 +299,12 @@ export class AcpClient { agentProcess.stderr?.setEncoding("utf8"); agentProcess.stderr?.on("data", (data) => { - this.logger.log(`[AcpClient] ${agentLabel} stderr:`, data); + const stderrChunk = String(data); + this.logger.log( + `[AcpClient] ${agentLabel} stderr chunk (${stderrChunk.length} chars)`, + ); // Keep a rolling window of recent stderr for error diagnostics - this.recentStderr += data; + this.recentStderr += stderrChunk; if (this.recentStderr.length > 8192) { this.recentStderr = this.recentStderr.slice(-4096); } @@ -384,6 +387,24 @@ export class AcpClient { } } + private getSafeConfigForLog(config: AgentConfig): { + id: string; + displayName: string; + command: string; + args: string[]; + envKeys: string[]; + workingDirectory: string; + } { + return { + id: config.id, + displayName: config.displayName, + command: config.command, + args: [...config.args], + envKeys: Object.keys(config.env || {}), + workingDirectory: config.workingDirectory, + }; + } + /** * Create a new chat session with the agent. */ diff --git a/src/acp/acp-handler.ts b/src/acp/acp-handler.ts index 5084589..c38d739 100644 --- a/src/acp/acp-handler.ts +++ b/src/acp/acp-handler.ts @@ -68,7 +68,10 @@ export class AcpHandler { const update = params.update; const sessionId = params.sessionId; this.promptSessionUpdateCount++; - this.logger.log("[AcpHandler] sessionUpdate:", { sessionId, update }); + this.logger.log("[AcpHandler] sessionUpdate:", { + sessionId, + type: update.sessionUpdate, + }); switch (update.sessionUpdate) { case "agent_message_chunk": @@ -174,7 +177,7 @@ export class AcpHandler { ): Promise { this.logger.log( `[AcpHandler] Extension notification received: ${method}`, - params, + Object.keys(params), ); } @@ -199,7 +202,12 @@ export class AcpHandler { ): Promise { this.logger.log( "[AcpHandler] createTerminal called with params:", - params, + { + command: params.command, + argCount: params.args?.length ?? 0, + hasEnv: (params.env?.length ?? 0) > 0, + cwd: params.cwd || this.getWorkingDirectory(), + }, ); const terminalId = this.terminalManager.createTerminal({ diff --git a/src/acp/permission-handler.ts b/src/acp/permission-handler.ts index 4403320..680ec51 100644 --- a/src/acp/permission-handler.ts +++ b/src/acp/permission-handler.ts @@ -78,7 +78,12 @@ export class PermissionManager { ): Promise { this.logger.log( "[PermissionManager] Permission request received:", - params, + { + sessionId: params.sessionId, + optionCount: params.options.length, + toolCallId: params.toolCall?.toolCallId, + title: params.toolCall?.title, + }, ); // If auto-allow is enabled, automatically approve the first allow option @@ -94,7 +99,11 @@ export class PermissionManager { this.logger.log( "[PermissionManager] Auto-allowing permission request:", - allowOption, + { + optionId: allowOption.optionId, + kind: allowOption.kind, + name: allowOption.name, + }, ); return Promise.resolve({ diff --git a/src/acp/terminal-handler.ts b/src/acp/terminal-handler.ts index 780bb03..ca73b6d 100644 --- a/src/acp/terminal-handler.ts +++ b/src/acp/terminal-handler.ts @@ -93,7 +93,7 @@ export class TerminalManager { this.logger.log(`[Terminal ${terminalId}] Creating terminal:`, { command, - args, + argCount: args.length, cwd: params.cwd, }); @@ -135,13 +135,17 @@ export class TerminalManager { // Capture stdout and stderr childProcess.stdout?.on("data", (data: Buffer) => { const output = data.toString(); - this.logger.log(`[Terminal ${terminalId}] stdout:`, output); + this.logger.log( + `[Terminal ${terminalId}] stdout chunk (${Buffer.byteLength(output, "utf8")} bytes)`, + ); this.appendOutput(terminal, output); }); childProcess.stderr?.on("data", (data: Buffer) => { const output = data.toString(); - this.logger.log(`[Terminal ${terminalId}] stderr:`, output); + this.logger.log( + `[Terminal ${terminalId}] stderr chunk (${Buffer.byteLength(output, "utf8")} bytes)`, + ); this.appendOutput(terminal, output); }); diff --git a/src/plugin.ts b/src/plugin.ts index 90da26b..33962d0 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -185,6 +185,14 @@ const DEFAULT_SETTINGS: AgentClientPluginSettings = { floatingButtonPosition: null, }; +const AGENT_API_KEY_SECRET_IDS = { + claude: "agent-client-claude-api-key", + codex: "agent-client-codex-api-key", + gemini: "agent-client-gemini-api-key", +} as const; + +type BuiltInAgentKey = keyof typeof AGENT_API_KEY_SECRET_IDS; + export default class AgentClientPlugin extends Plugin { settings: AgentClientPluginSettings; settingsService!: SettingsService; @@ -830,6 +838,7 @@ export default class AgentClientPlugin extends Plugin { async loadSettings() { const raw = ((await this.loadData()) ?? {}) as Record; const D = DEFAULT_SETTINGS; + let migratedSecrets = false; // Extract agent sub-objects const rc = obj(raw.claude) ?? {}; @@ -865,7 +874,17 @@ export default class AgentClientPlugin extends Plugin { claude: { id: D.claude.id, // Fixed — never from raw displayName: str(rc.displayName, D.claude.displayName), - apiKey: str(rc.apiKey, D.claude.apiKey), + apiKeySecretId: str( + rc.apiKeySecretId, + AGENT_API_KEY_SECRET_IDS.claude, + ), + apiKey: this.loadAgentApiKey( + str(rc.apiKeySecretId, AGENT_API_KEY_SECRET_IDS.claude), + str(rc.apiKey, D.claude.apiKey), + () => { + migratedSecrets = true; + }, + ), // Migration: claude.command ← claudeCodeAcpCommandPath (old name) command: str(rc.command, "") || @@ -877,7 +896,17 @@ export default class AgentClientPlugin extends Plugin { codex: { id: D.codex.id, displayName: str(rk.displayName, D.codex.displayName), - apiKey: str(rk.apiKey, D.codex.apiKey), + apiKeySecretId: str( + rk.apiKeySecretId, + AGENT_API_KEY_SECRET_IDS.codex, + ), + apiKey: this.loadAgentApiKey( + str(rk.apiKeySecretId, AGENT_API_KEY_SECRET_IDS.codex), + str(rk.apiKey, D.codex.apiKey), + () => { + migratedSecrets = true; + }, + ), command: str(rk.command, "") || D.codex.command, args: sanitizeArgs(rk.args), env: normalizeEnvVars(rk.env), @@ -885,7 +914,17 @@ export default class AgentClientPlugin extends Plugin { gemini: { id: D.gemini.id, displayName: str(rg.displayName, D.gemini.displayName), - apiKey: str(rg.apiKey, D.gemini.apiKey), + apiKeySecretId: str( + rg.apiKeySecretId, + AGENT_API_KEY_SECRET_IDS.gemini, + ), + apiKey: this.loadAgentApiKey( + str(rg.apiKeySecretId, AGENT_API_KEY_SECRET_IDS.gemini), + str(rg.apiKey, D.gemini.apiKey), + () => { + migratedSecrets = true; + }, + ), // Migration: gemini.command ← geminiCommandPath (old name) command: str(rg.command, "") || @@ -1017,16 +1056,87 @@ export default class AgentClientPlugin extends Plugin { }; this.ensureDefaultAgentId(); + + if (migratedSecrets) { + await this.saveSettings(); + } } async saveSettings() { - await this.saveData(this.settings); + await this.saveData(this.getPersistedSettings()); } async saveSettingsAndNotify(nextSettings: AgentClientPluginSettings) { await this.settingsService.updateSettings(nextSettings); } + private loadAgentApiKey( + secretId: string, + legacyApiKey: string, + onMigrate: () => void, + ): string { + const storedSecret = this.app.secretStorage.getSecret(secretId); + if (storedSecret !== null) { + return storedSecret; + } + + if (legacyApiKey.trim().length > 0) { + this.app.secretStorage.setSecret(secretId, legacyApiKey); + onMigrate(); + return legacyApiKey; + } + + return ""; + } + + private getPersistedSettings(): AgentClientPluginSettings { + return { + ...this.settings, + claude: { + ...this.settings.claude, + apiKey: "", + apiKeySecretId: + this.settings.claude.apiKeySecretId ?? + AGENT_API_KEY_SECRET_IDS.claude, + }, + codex: { + ...this.settings.codex, + apiKey: "", + apiKeySecretId: + this.settings.codex.apiKeySecretId ?? + AGENT_API_KEY_SECRET_IDS.codex, + }, + gemini: { + ...this.settings.gemini, + apiKey: "", + apiKeySecretId: + this.settings.gemini.apiKeySecretId ?? + AGENT_API_KEY_SECRET_IDS.gemini, + }, + }; + } + + async updateAgentApiKey( + agent: BuiltInAgentKey, + apiKey: string, + ): Promise { + const secretId = AGENT_API_KEY_SECRET_IDS[agent]; + this.app.secretStorage.setSecret(secretId, apiKey); + + if (agent === "claude") { + this.settings.claude.apiKey = apiKey; + this.settings.claude.apiKeySecretId = secretId; + } else if (agent === "codex") { + this.settings.codex.apiKey = apiKey; + this.settings.codex.apiKeySecretId = secretId; + } else { + this.settings.gemini.apiKey = apiKey; + this.settings.gemini.apiKeySecretId = secretId; + } + + await this.saveSettings(); + } + /** * Fetch the latest stable release version from GitHub. */ diff --git a/src/types/agent.ts b/src/types/agent.ts index 818b6f4..b716feb 100644 --- a/src/types/agent.ts +++ b/src/types/agent.ts @@ -61,6 +61,8 @@ export interface BaseAgentSettings { export interface GeminiAgentSettings extends BaseAgentSettings { /** Gemini API key (GEMINI_API_KEY) */ apiKey: string; + /** Secret storage ID containing the Gemini API key */ + apiKeySecretId?: string; } /** @@ -71,6 +73,8 @@ export interface GeminiAgentSettings extends BaseAgentSettings { export interface ClaudeAgentSettings extends BaseAgentSettings { /** Anthropic API key for Claude (ANTHROPIC_API_KEY) */ apiKey: string; + /** Secret storage ID containing the Anthropic API key */ + apiKeySecretId?: string; } /** @@ -81,6 +85,8 @@ export interface ClaudeAgentSettings extends BaseAgentSettings { export interface CodexAgentSettings extends BaseAgentSettings { /** OpenAI API key for Codex (OPENAI_API_KEY) */ apiKey: string; + /** Secret storage ID containing the OpenAI API key */ + apiKeySecretId?: string; } /** diff --git a/src/ui/SettingsTab.ts b/src/ui/SettingsTab.ts index bf47820..c2d1f67 100644 --- a/src/ui/SettingsTab.ts +++ b/src/ui/SettingsTab.ts @@ -813,14 +813,16 @@ export class AgentClientSettingTab extends PluginSettingTab { new Setting(sectionEl) .setName("API key") .setDesc( - "Gemini API key. Required if not logging in with a Google account. (Stored as plain text)", + "Gemini API key. Required if not logging in with a Google account. Stored in Obsidian secret storage.", ) .addText((text) => { text.setPlaceholder("Enter your Gemini API key") .setValue(gemini.apiKey) .onChange(async (value) => { - this.plugin.settings.gemini.apiKey = value.trim(); - await this.plugin.saveSettings(); + await this.plugin.updateAgentApiKey( + "gemini", + value.trim(), + ); }); text.inputEl.type = "password"; }); @@ -863,7 +865,7 @@ export class AgentClientSettingTab extends PluginSettingTab { new Setting(sectionEl) .setName("Environment variables") .setDesc( - "Enter KEY=VALUE pairs, one per line. Required to authenticate with Vertex AI. GEMINI_API_KEY is derived from the field above.(Stored as plain text)", + "Enter KEY=VALUE pairs, one per line. Required to authenticate with Vertex AI. GEMINI_API_KEY is derived from the field above.", ) .addTextArea((text) => { text.setPlaceholder("GOOGLE_CLOUD_PROJECT=...") @@ -886,14 +888,16 @@ export class AgentClientSettingTab extends PluginSettingTab { new Setting(sectionEl) .setName("API key") .setDesc( - "Anthropic API key. Required if not logging in with an Anthropic account. (Stored as plain text)", + "Anthropic API key. Required if not logging in with an Anthropic account. Stored in Obsidian secret storage.", ) .addText((text) => { text.setPlaceholder("Enter your Anthropic API key") .setValue(claude.apiKey) .onChange(async (value) => { - this.plugin.settings.claude.apiKey = value.trim(); - await this.plugin.saveSettings(); + await this.plugin.updateAgentApiKey( + "claude", + value.trim(), + ); }); text.inputEl.type = "password"; }); @@ -963,14 +967,16 @@ export class AgentClientSettingTab extends PluginSettingTab { new Setting(sectionEl) .setName("API key") .setDesc( - "OpenAI API key. Required if not logging in with an OpenAI account. (Stored as plain text)", + "OpenAI API key. Required if not logging in with an OpenAI account. Stored in Obsidian secret storage.", ) .addText((text) => { text.setPlaceholder("Enter your OpenAI API key") .setValue(codex.apiKey) .onChange(async (value) => { - this.plugin.settings.codex.apiKey = value.trim(); - await this.plugin.saveSettings(); + await this.plugin.updateAgentApiKey( + "codex", + value.trim(), + ); }); text.inputEl.type = "password"; });