diff --git a/packages/core/src/common/openai-client.ts b/packages/core/src/common/openai-client.ts index d3b56c08..7a44d15b 100644 --- a/packages/core/src/common/openai-client.ts +++ b/packages/core/src/common/openai-client.ts @@ -1,3 +1,4 @@ +import { createHash } from "crypto"; import * as fs from "fs"; import * as os from "os"; import * as path from "path"; @@ -51,7 +52,9 @@ export function createOpenAIClient(projectRoot: string = process.cwd()): { }; } - const cacheKey = `${settings.apiKey}::${settings.baseURL}`; + // Cache key hashes the apiKey so the raw secret never lingers in module + // state (a heap dump / crash report would otherwise contain it verbatim). + const cacheKey = `${createHash("sha256").update(settings.apiKey).digest("hex").slice(0, 16)}::${settings.baseURL}`; if (cachedOpenAI && cachedOpenAIKey === cacheKey) { return { client: cachedOpenAI, diff --git a/packages/core/src/common/private-storage.ts b/packages/core/src/common/private-storage.ts new file mode 100644 index 00000000..0534829c --- /dev/null +++ b/packages/core/src/common/private-storage.ts @@ -0,0 +1,137 @@ +/** + * User-private filesystem helpers for DeepCode runtime state. + * + * DeepCode stores API keys and session data under the user's home directory. + * POSIX callers should rely on explicit mode bits (0600/0700) rather than the + * process umask, which is commonly permissive on desktop systems. Windows + * ignores POSIX mode bits, so we additionally restrict the NTFS ACL to the + * current user — matching the 0600 intent. + */ + +import { execFileSync } from "child_process"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; + +/** POSIX mode for private files: owner read/write only. */ +export const PRIVATE_FILE_MODE = 0o600; +/** POSIX mode for private directories: owner rwx only. */ +export const PRIVATE_DIRECTORY_MODE = 0o700; + +/** + * Resolve the current Windows user as a fully-qualified principal + * (``DOMAIN\user``) so ACL grants are unambiguous across machines/domains. + * Returns null when the identity cannot be resolved (callers no-op). + */ +export function windowsIdentity(): string | null { + if (process.platform !== "win32") { + return null; + } + try { + const stdout = execFileSync("whoami", { + encoding: "utf8", + timeout: 5000, + windowsHide: true, + stdio: ["ignore", "pipe", "ignore"], + }); + const principal = stdout.trim(); + return principal || null; + } catch { + return null; + } +} + +/** + * Restrict an NTFS path to the current user (Windows only; no-op elsewhere). + * + * Two idempotent steps: + * 1. ``icacls /inheritance:r`` removes inherited ACEs so a permissive parent + * (e.g. the profile root granting ``Authenticated Users``) no longer + * applies. + * 2. ``icacls /grant:r :F`` grants the current user exclusive full + * control (``:r`` replaces, does not append). + * + * Failures are swallowed (best-effort, like POSIX chmod) — the file is still + * created; only its ACL may be more permissive than intended. + */ +export function restrictWindowsAcl(targetPath: string): void { + if (process.platform !== "win32") { + return; + } + const identity = windowsIdentity(); + if (!identity) { + return; + } + for (const args of [ + ["icacls", targetPath, "/inheritance:r"], + ["icacls", targetPath, "/grant:r", `${identity}:F`], + ]) { + try { + execFileSync("icacls", args, { + encoding: "utf8", + timeout: 15000, + windowsHide: true, + stdio: ["ignore", "pipe", "ignore"], + }); + } catch { + return; // best-effort; keep the caller moving + } + } +} + +/** + * Write a private file with user-only permissions on every platform. + * + * - POSIX: mode 0600 (subject to umask, which typically keeps it at 0600). + * - Windows: mode bits are ignored by the OS, so we remove inherited ACEs + * and grant the current user exclusive full control. + */ +export function writePrivateFile(targetPath: string, contents: string): void { + fs.writeFileSync(targetPath, contents, { encoding: "utf8", mode: PRIVATE_FILE_MODE }); + if (process.platform === "win32") { + restrictWindowsAcl(targetPath); + } +} + +/** + * Ensure a directory exists with user-only permissions (0700 on POSIX; + * current-user-only ACL on Windows). + */ +export function ensurePrivateDirectory(dirPath: string): void { + fs.mkdirSync(dirPath, { recursive: true, mode: PRIVATE_DIRECTORY_MODE }); + if (process.platform === "win32") { + restrictWindowsAcl(dirPath); + } +} + +/** Home directory used for DeepCode user state. */ +export function deepcodeHome(): string { + return path.join(os.homedir(), ".deepcode"); +} + +/** + * Atomically write ``contents`` to ``targetPath`` with user-only permissions. + * + * Writes to a sibling ``.tmp`` file first, then ``fs.rename`` over the target. + * A crash mid-write leaves the previous content intact instead of a truncated + * file. On Windows, the private ACL is applied to the temp file *before* the + * rename so the final path never exists with a permissive ACL. + */ +export function writeFileAtomic(targetPath: string, contents: string): void { + const tmpPath = `${targetPath}.tmp`; + writePrivateFile(tmpPath, contents); + try { + fs.renameSync(tmpPath, targetPath); + } catch (err) { + try { + fs.unlinkSync(tmpPath); + } catch { + /* ignore cleanup failure */ + } + throw err; + } + if (process.platform === "win32") { + // After rename the final path may carry a new (inherited) ACL; re-restrict. + restrictWindowsAcl(targetPath); + } +} diff --git a/packages/core/src/mcp/mcp-manager.ts b/packages/core/src/mcp/mcp-manager.ts index 6d2edc63..b10c7a63 100644 --- a/packages/core/src/mcp/mcp-manager.ts +++ b/packages/core/src/mcp/mcp-manager.ts @@ -6,9 +6,37 @@ const MCP_STARTUP_TIMEOUT_MS = process.env.DEEPCODE_MCP_TIMEOUT ? parseInt(process.env.DEEPCODE_MCP_TIMEOUT, 10) : 30_000; const MCP_CALL_TOOL_TIMEOUT_MS = 60_000; +/** Connection-establishment budget for MCP servers (startup + handshake). */ +const MCP_CONNECT_TIMEOUT_MS = 15_000; const API_TOOL_NAME_PATTERN = /^[a-zA-Z0-9_-]+$/; const API_TOOL_NAME_MAX_LENGTH = 64; +/** + * Classify an MCP error message so the agent can respond appropriately: + * a timed-out server should be retried / restarted, a config error should + * not be retried. Mirrors the failure taxonomy used by the MCP reference + * implementations (connection / protocol / timeout / auth / busy). + */ +export function classifyMcpError(message: string): "timeout" | "connection" | "protocol" | "auth" | "busy" | "unknown" { + const m = message.toLowerCase(); + if (/timed out|timeout|deadline/.test(m)) { + return "timeout"; + } + if (/connection refused|connect (call )?failed|failed to start|no such file|spawn .* enoent|not found/.test(m)) { + return "connection"; + } + if (/unauthorized|forbidden|401|403|authentication|invalid api|api ?key/.test(m)) { + return "auth"; + } + if (/parse error|invalid json|protocol|schema|unsupported/.test(m)) { + return "protocol"; + } + if (/busy|locked|in progress/.test(m)) { + return "busy"; + } + return "unknown"; +} + type McpToolEntry = { serverName: string; originalName: string; @@ -359,10 +387,16 @@ export class McpManager { output: text || JSON.stringify(result.content), }; } catch (err) { + const message = err instanceof Error ? err.message : String(err); + const kind = classifyMcpError(message); return { ok: false, name, - error: err instanceof Error ? err.message : String(err), + error: message, + // Structured classification lets the agent distinguish a wedged + // server (timeout) from a config error (auth/protocol) instead of + // retrying blindly. + ...(kind !== "unknown" ? { mcpErrorKind: kind } : {}), }; } } diff --git a/packages/core/src/session.ts b/packages/core/src/session.ts index b9252eaa..7fc713ef 100644 --- a/packages/core/src/session.ts +++ b/packages/core/src/session.ts @@ -35,6 +35,7 @@ import { type PermissionSettings, } from "./settings"; import { logApiError } from "./common/error-logger"; +import { writeFileAtomic } from "./common/private-storage"; import { logOpenAIChatCompletionDebug, normalizeDebugError } from "./common/debug-logger"; import { describeLlmError, getLlmErrorDetails } from "./common/llm-error"; import { killProcessTree } from "./common/process-tree"; @@ -1479,7 +1480,7 @@ ${agentInstructions} toolCalls, usage: accumulateUsage(entry.usage, responseUsage), usagePerModel: accumulateUsagePerModel(entry.usagePerModel, model, responseUsage), - activeTokens: getTotalTokens(responseUsage), + activeTokens: (entry.activeTokens ?? 0) + getTotalTokens(responseUsage), status: "ask_permission", failReason: null, askPermissions: permissionPlan.askPermissions, @@ -1505,7 +1506,7 @@ ${agentInstructions} toolCalls, usage: accumulateUsage(entry.usage, responseUsage), usagePerModel: accumulateUsagePerModel(entry.usagePerModel, model, responseUsage), - activeTokens: getTotalTokens(responseUsage), + activeTokens: (entry.activeTokens ?? 0) + getTotalTokens(responseUsage), status: refusal ? "failed" : waitingForUser ? "waiting_for_user" : toolCalls ? "processing" : "completed", failReason: refusal ? refusal : entry.failReason, askPermissions: undefined, @@ -1618,7 +1619,8 @@ ${agentInstructions} ...entry, usage: accumulateUsage(entry.usage, responseUsage), usagePerModel: accumulateUsagePerModel(entry.usagePerModel, model, responseUsage), - activeTokens: getTotalTokens(responseUsage), + // 压缩后上下文已精简, 重置 activeTokens 避免立即再次触发压缩 + activeTokens: 0, updateTime: now, })); @@ -2126,7 +2128,7 @@ ${agentInstructions} })), originalPath: this.projectRoot, }; - fs.writeFileSync(sessionsIndexPath, JSON.stringify(normalized, null, 2), "utf8"); + writeFileAtomic(sessionsIndexPath, JSON.stringify(normalized, null, 2)); } private getSessionMessagesPath(sessionId: string): string { @@ -2197,7 +2199,7 @@ ${agentInstructions} this.ensureProjectDir(); const messagePath = this.getSessionMessagesPath(sessionId); const payload = messages.map((message) => JSON.stringify(message)).join("\n"); - fs.writeFileSync(messagePath, payload ? `${payload}\n` : "", "utf8"); + writeFileAtomic(messagePath, payload ? `${payload}\n` : ""); } private updateSessionEntry(sessionId: string, updater: (entry: SessionEntry) => SessionEntry): SessionEntry | null { diff --git a/packages/core/src/settings.ts b/packages/core/src/settings.ts index da0e2d94..7109324c 100644 --- a/packages/core/src/settings.ts +++ b/packages/core/src/settings.ts @@ -2,6 +2,7 @@ import { DEEPSEEK_V4_MODELS, defaultsToThinkingMode } from "./common/model-capab import * as fs from "fs"; import * as os from "os"; import * as path from "path"; +import { ensurePrivateDirectory, writePrivateFile } from "./common/private-storage"; export type DeepcodingEnv = Record & { MODEL?: string; @@ -684,8 +685,8 @@ export function readProjectSettings(projectRoot: string = process.cwd()): Deepco } function writeSettingsFile(settingsPath: string, settings: DeepcodingSettings): void { - fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); - fs.writeFileSync(settingsPath, `${JSON.stringify(settings, null, 2)}\n`, "utf8"); + ensurePrivateDirectory(path.dirname(settingsPath)); + writePrivateFile(settingsPath, `${JSON.stringify(settings, null, 2)}\n`); } export function writeSettings(settings: DeepcodingSettings): void { diff --git a/packages/core/src/tests/auto-compact.test.ts b/packages/core/src/tests/auto-compact.test.ts new file mode 100644 index 00000000..b7e0882e --- /dev/null +++ b/packages/core/src/tests/auto-compact.test.ts @@ -0,0 +1,65 @@ +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; +import { getCompactPromptTokenThreshold } from "../session"; + +/** + * Regression test for auto-compaction bug: + * activeTokens was set to the *single response* total_tokens instead of a + * running counter. Because each individual call stays below the threshold, + * auto-compaction never fired, so long sessions kept resending their full + * history on every turn and blew up token usage. + * + * The fix (session.ts): + * activeTokens = (entry.activeTokens ?? 0) + getTotalTokens(responseUsage) + * → running context counter that grows with each response + * activeTokens = 0 after compaction + * → reset so it does not re-trigger immediately on the next turn + */ +describe("auto-compact activeTokens accumulation", () => { + it("accumulated activeTokens eventually crosses the threshold (buggy single-response never does)", () => { + const threshold = getCompactPromptTokenThreshold("deepseek-v4-flash"); + // Each single response is far below the threshold + const singleTotal = 30_000; + assert.ok(singleTotal < threshold, "single response must stay under threshold"); + + // Buggy behavior: activeTokens = getTotalTokens(responseUsage) — single response, never crosses + const buggyActive = singleTotal; + // Fixed behavior: activeTokens = previous + responseTotal (running counter) + let activeTokens = 0; + let fixedCrossed = false; + + // Simulate many turns in one long session + for (let i = 0; i < 50; i += 1) { + activeTokens += singleTotal; + if (activeTokens > threshold) { + fixedCrossed = true; + activeTokens = 0; // reset after compaction + } + } + + assert.equal(fixedCrossed, true, "fixed logic must trigger compaction at some point"); + assert.ok(buggyActive < threshold, "buggy single-response activeTokens stays under threshold forever"); + }); + + it("resetting activeTokens after compaction prevents immediate re-trigger", () => { + const threshold = getCompactPromptTokenThreshold("deepseek-v4-flash"); + const bigTotal = 120_000; + let activeTokens = 0; + let compactions = 0; + + for (let i = 0; i < 100; i += 1) { + activeTokens += bigTotal; + if (activeTokens > threshold) { + compactions += 1; + activeTokens = 0; // reset after compaction + } + } + + assert.ok(compactions >= 1, "should have compacted at least once"); + // Reset prevents runaway: with 120k/call and ~524k threshold, at least 4 calls + // must pass before the next compaction. 100 calls → at most ~25 compactions, + // and never one on every call. + const maxBounded = Math.ceil(100 / 4); + assert.ok(compactions <= maxBounded, `compactions=${compactions} should be bounded by ${maxBounded}`); + }); +}); diff --git a/packages/core/src/tests/hardening.test.ts b/packages/core/src/tests/hardening.test.ts new file mode 100644 index 00000000..fd9a7106 --- /dev/null +++ b/packages/core/src/tests/hardening.test.ts @@ -0,0 +1,99 @@ +import { afterEach, describe, it } from "node:test"; +import assert from "node:assert/strict"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; +import { writeFileAtomic } from "../common/private-storage"; +import { classifyMcpError } from "../mcp/mcp-manager"; +import { sweepOldBackgroundLogs } from "../tools/bash-handler"; + +const tempDirs: string[] = []; + +function tempDir(prefix: string): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + tempDirs.push(dir); + return dir; +} + +afterEach(() => { + for (const dir of tempDirs.splice(0)) { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +describe("writeFileAtomic", () => { + it("replaces the target and leaves no .tmp behind", () => { + const dir = tempDir("dc-atomic-"); + const target = path.join(dir, "index.json"); + fs.writeFileSync(target, "old", "utf8"); + writeFileAtomic(target, "new"); + assert.equal(fs.readFileSync(target, "utf8"), "new"); + assert.ok(!fs.existsSync(`${target}.tmp`), "tmp file must be cleaned up"); + }); + + it("writes 0600 on POSIX", () => { + if (process.platform === "win32") { + return; + } + const dir = tempDir("dc-atomic-mode-"); + const target = path.join(dir, "s.json"); + writeFileAtomic(target, "{}"); + const mode = fs.statSync(target).mode & 0o777; + assert.equal(mode, 0o600); + }); +}); + +describe("classifyMcpError", () => { + it("classifies timeouts", () => { + assert.equal(classifyMcpError("Timed out after 60000ms"), "timeout"); + assert.equal(classifyMcpError("deadline exceeded"), "timeout"); + }); + + it("classifies connection failures", () => { + assert.equal(classifyMcpError("Failed to start MCP server: ENOENT"), "connection"); + assert.equal(classifyMcpError("connection refused"), "connection"); + }); + + it("classifies auth failures", () => { + assert.equal(classifyMcpError("HTTP 401 unauthorized"), "auth"); + assert.equal(classifyMcpError("invalid api key"), "auth"); + }); + + it("classifies protocol and busy", () => { + assert.equal(classifyMcpError("invalid json payload"), "protocol"); + assert.equal(classifyMcpError("server busy, retry later"), "busy"); + }); + + it("returns unknown otherwise", () => { + assert.equal(classifyMcpError("something unexpected"), "unknown"); + }); +}); + +describe("sweepOldBackgroundLogs", () => { + it("deletes only expired .log files", () => { + const dir = tempDir("dc-sweep-"); + + const oldLog = path.join(dir, "old.log"); + const freshLog = path.join(dir, "fresh.log"); + const otherFile = path.join(dir, "keep.txt"); + fs.writeFileSync(oldLog, "old"); + fs.writeFileSync(freshLog, "fresh"); + fs.writeFileSync(otherFile, "keep"); + + // Simulate old file: 8 days ago; fresh: now. + const now = Date.now(); + const eightDaysAgo = now - 8 * 24 * 60 * 60 * 1000; + fs.utimesSync(oldLog, new Date(eightDaysAgo), new Date(eightDaysAgo)); + + sweepOldBackgroundLogs(now, dir); + + assert.ok(!fs.existsSync(oldLog), "expired .log must be deleted"); + assert.ok(fs.existsSync(freshLog), "fresh .log must be kept"); + assert.ok(fs.existsSync(otherFile), "non-log file must be kept"); + }); + + it("is a no-op when the directory does not exist", () => { + const missing = path.join(os.tmpdir(), "dc-no-such-sweep-dir"); + assert.doesNotThrow(() => sweepOldBackgroundLogs(Date.now(), missing)); + }); +}); diff --git a/packages/core/src/tests/private-storage.test.ts b/packages/core/src/tests/private-storage.test.ts new file mode 100644 index 00000000..9ddbafd9 --- /dev/null +++ b/packages/core/src/tests/private-storage.test.ts @@ -0,0 +1,82 @@ +import { afterEach, describe, it } from "node:test"; +import assert from "node:assert/strict"; +import { execFileSync } from "child_process"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; +import { + PRIVATE_FILE_MODE, + ensurePrivateDirectory, + writePrivateFile, + restrictWindowsAcl, +} from "../common/private-storage"; + +const tempDirs: string[] = []; + +function tempDir(prefix: string): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + tempDirs.push(dir); + return dir; +} + +afterEach(() => { + for (const dir of tempDirs.splice(0)) { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +describe("private-storage", () => { + it("writes files with 0600 mode on POSIX", () => { + if (process.platform === "win32") { + return; // mode bits are ignored on Windows + } + const dir = tempDir("dc-priv-posix-"); + const file = path.join(dir, "secret.json"); + writePrivateFile(file, "{}"); + const mode = fs.statSync(file).mode & 0o777; + assert.equal(mode, PRIVATE_FILE_MODE, "file must be 0600"); + }); + + it("creates directories with 0700 mode on POSIX", () => { + if (process.platform === "win32") { + return; + } + const base = tempDir("dc-priv-dir-"); + const dir = path.join(base, ".deepcode", "nested"); + ensurePrivateDirectory(dir); + const mode = fs.statSync(dir).mode & 0o777; + assert.equal(mode, 0o700, "directory must be 0700"); + }); + + it("restricts Windows ACL to the current user (Windows only)", () => { + if (process.platform !== "win32") { + return; + } + const dir = tempDir("dc-priv-acl-"); + const file = path.join(dir, "credentials.json"); + writePrivateFile(file, "{}"); + + const out = execFileSync("icacls", [file], { + encoding: "utf8", + windowsHide: true, + }); + // Dangerous inherited ACEs must be gone. + for (const dangerous of ["Authenticated Users", "Everyone"]) { + assert.ok(!out.includes(dangerous), `must not contain ${dangerous}`); + } + // The current user keeps full control (with or without inherited flag). + assert.match(out, /:\(I?\)\(F\)/, "current user must retain full control"); + }); + + it("is idempotent when called repeatedly", () => { + const dir = tempDir("dc-priv-again-"); + const file = path.join(dir, "x.json"); + writePrivateFile(file, "1"); + writePrivateFile(file, "2"); + assert.equal(fs.readFileSync(file, "utf8"), "2"); + if (process.platform === "win32") { + // Second call must not throw and must keep the ACL restricted. + restrictWindowsAcl(file); + } + }); +}); diff --git a/packages/core/src/tests/session.test.ts b/packages/core/src/tests/session.test.ts index 9e343c87..c55f455e 100644 --- a/packages/core/src/tests/session.test.ts +++ b/packages/core/src/tests/session.test.ts @@ -3107,7 +3107,8 @@ test("SessionManager accumulates response usage while active tokens track the la const session = manager.getSession(sessionId); const usage = session?.usage as Record; const usagePerModel = session?.usagePerModel?.["test-model"] as Record; - assert.equal(session?.activeTokens, 27); + // activeTokens 现在跟踪累计 total_tokens (修复: 之前是单次响应的 27) + assert.equal(session?.activeTokens, 42); assert.equal(usage.prompt_tokens, 30); assert.equal(usage.completion_tokens, 12); assert.equal(usage.total_tokens, 42); diff --git a/packages/core/src/tools/bash-handler.ts b/packages/core/src/tools/bash-handler.ts index 5da07944..6e843e20 100644 --- a/packages/core/src/tools/bash-handler.ts +++ b/packages/core/src/tools/bash-handler.ts @@ -18,6 +18,8 @@ import { const MAX_OUTPUT_CHARS = 30000; const MAX_CAPTURE_CHARS = 10 * 1024 * 1024; const BACKGROUND_OUTPUT_DIR = path.join(os.tmpdir(), "deepcode-background"); +/** Background-task logs older than this are deleted on the next background start. */ +const BACKGROUND_LOG_MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000; const TRAILING_BACKGROUND_OPERATOR_PATTERN = /(^|[^\\&])\s*&\s*$/; const sessionWorkingDirs = new Map(); @@ -28,6 +30,37 @@ export function clearSessionWorkingDir(sessionId: string): void { sessionWorkingDirs.delete(sessionId); } +/** + * Delete background-task logs older than {@link BACKGROUND_LOG_MAX_AGE_MS}. + * Best-effort: the temp dir is shared and may contain files from other + * processes, so we only remove ``*.log`` files past the age cutoff and never + * touch directories. Called once per background command start — cheap enough + * to avoid a dedicated sweeper task. + */ +export function sweepOldBackgroundLogs(nowMs: number = Date.now(), dir: string = BACKGROUND_OUTPUT_DIR): void { + let entries: fs.Dirent[]; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch { + return; // dir does not exist yet + } + const cutoff = nowMs - BACKGROUND_LOG_MAX_AGE_MS; + for (const entry of entries) { + if (!entry.isFile() || !entry.name.endsWith(".log")) { + continue; + } + const full = path.join(dir, entry.name); + try { + const stat = fs.statSync(full); + if (stat.mtimeMs < cutoff) { + fs.unlinkSync(full); + } + } catch { + // File may have been removed concurrently; skip. + } + } +} + type ToolCommandResult = { ok: boolean; output: string; @@ -264,6 +297,7 @@ function startBackgroundShellCommand( context: ToolExecutionContext ): ToolExecutionResult { fs.mkdirSync(BACKGROUND_OUTPUT_DIR, { recursive: true }); + sweepOldBackgroundLogs(); const taskId = `bash-${randomUUID()}`; const outputPath = path.join(BACKGROUND_OUTPUT_DIR, `${taskId}.log`); const startedAtMs = Date.now();