From d5abc7d69170132cc5c604c453e45f54fa2973b6 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Wed, 11 Feb 2026 11:24:34 +0700 Subject: [PATCH] fix(config): lazy-evaluate paths, fix TOCTOU, segment-boundary cloud detection - Convert 4 module-level constants to lazy-evaluated functions to avoid import-time caching: openrouter-catalog, aggregator, disk-cache, auth-middleware - Fix symlink-checks.ts to use ccsDir parameter instead of homedir/.ccs, remove unused homedir parameter from checkSettingsSymlinks() - Replace TOCTOU existsSync+statSync with single statSync in try/catch for --config-dir validation in ccs.ts - Switch detectCloudSyncPath from substring to path-segment-boundary matching to prevent false positives (e.g., megauser != MEGA, Dropbox-api != Dropbox) - Add test for false-positive protection --- src/api/services/openrouter-catalog.ts | 12 ++++++---- src/ccs.ts | 10 ++++++-- src/utils/config-manager.ts | 7 +++--- src/web-server/health-service.ts | 2 +- src/web-server/health/symlink-checks.ts | 8 ++----- src/web-server/middleware/auth-middleware.ts | 12 ++++++---- src/web-server/usage/aggregator.ts | 10 ++++---- src/web-server/usage/disk-cache.ts | 24 ++++++++++++-------- tests/unit/config-dir-override.test.js | 6 +++++ 9 files changed, 55 insertions(+), 36 deletions(-) diff --git a/src/api/services/openrouter-catalog.ts b/src/api/services/openrouter-catalog.ts index 3161f7ce..601f90eb 100644 --- a/src/api/services/openrouter-catalog.ts +++ b/src/api/services/openrouter-catalog.ts @@ -8,7 +8,9 @@ import * as path from 'path'; import { getCcsDir } from '../../utils/config-manager'; const OPENROUTER_API_URL = 'https://openrouter.ai/api/v1/models'; -const CACHE_FILE = path.join(getCcsDir(), 'openrouter-models-cache.json'); +function getCacheFile() { + return path.join(getCcsDir(), 'openrouter-models-cache.json'); +} const CACHE_TTL_MS = 24 * 60 * 60 * 1000; // 24 hours export interface OpenRouterModel { @@ -30,8 +32,8 @@ interface CacheData { /** Check if cached data is valid */ function getCachedModels(): OpenRouterModel[] | null { try { - if (!fs.existsSync(CACHE_FILE)) return null; - const data = JSON.parse(fs.readFileSync(CACHE_FILE, 'utf8')) as CacheData; + if (!fs.existsSync(getCacheFile())) return null; + const data = JSON.parse(fs.readFileSync(getCacheFile(), 'utf8')) as CacheData; if (Date.now() - data.fetchedAt > CACHE_TTL_MS) return null; return data.models; } catch { @@ -42,10 +44,10 @@ function getCachedModels(): OpenRouterModel[] | null { /** Save models to cache */ function setCachedModels(models: OpenRouterModel[]): void { try { - const dir = path.dirname(CACHE_FILE); + const dir = path.dirname(getCacheFile()); if (!fs.existsSync(dir)) fs.mkdirSync(dir, { recursive: true }); fs.writeFileSync( - CACHE_FILE, + getCacheFile(), JSON.stringify({ models, fetchedAt: Date.now(), diff --git a/src/ccs.ts b/src/ccs.ts index b29d3f71..c0a39550 100644 --- a/src/ccs.ts +++ b/src/ccs.ts @@ -286,8 +286,14 @@ async function main(): Promise { process.exit(1); } - if (!fs.existsSync(configDirValue) || !fs.statSync(configDirValue).isDirectory()) { - console.error(fail(`Config directory not found or not a directory: ${configDirValue}`)); + try { + const stat = fs.statSync(configDirValue); + if (!stat.isDirectory()) { + console.error(fail(`Not a directory: ${configDirValue}`)); + process.exit(1); + } + } catch { + console.error(fail(`Config directory not found: ${configDirValue}`)); console.error(info('Create the directory first, then copy your config files into it.')); process.exit(1); } diff --git a/src/utils/config-manager.ts b/src/utils/config-manager.ts index db6a9e16..8512efec 100644 --- a/src/utils/config-manager.ts +++ b/src/utils/config-manager.ts @@ -73,10 +73,11 @@ const CLOUD_SYNC_PATTERNS = [ */ export function detectCloudSyncPath(dir: string): string | null { const normalized = dir.replace(/\\/g, '/').toLowerCase(); + const segments = normalized.split('/'); for (const pattern of CLOUD_SYNC_PATTERNS) { - if (normalized.includes(pattern.toLowerCase())) { - return pattern; // Return original casing for display - } + const patternLower = pattern.toLowerCase(); + // Exact path segment match to avoid false positives (e.g., "megauser" != "MEGA") + if (segments.some((s) => s === patternLower)) return pattern; } return null; } diff --git a/src/web-server/health-service.ts b/src/web-server/health-service.ts index 21a8723d..4947c71a 100644 --- a/src/web-server/health-service.ts +++ b/src/web-server/health-service.ts @@ -89,7 +89,7 @@ export async function runHealthChecks(): Promise { const healthChecks: HealthCheck[] = []; healthChecks.push(checkPermissions(ccsDir)); healthChecks.push(checkCcsSymlinks()); - healthChecks.push(checkSettingsSymlinks(homedir, ccsDir, claudeDir)); + healthChecks.push(checkSettingsSymlinks(ccsDir, claudeDir)); groups.push({ id: 'system-health', name: 'System Health', icon: 'Shield', checks: healthChecks }); // Group 6: CLIProxy diff --git a/src/web-server/health/symlink-checks.ts b/src/web-server/health/symlink-checks.ts index a5e4e0aa..40b90b5f 100644 --- a/src/web-server/health/symlink-checks.ts +++ b/src/web-server/health/symlink-checks.ts @@ -49,13 +49,9 @@ export function checkCcsSymlinks(): HealthCheck { /** * Check settings symlinks */ -export function checkSettingsSymlinks( - homedir: string, - ccsDir: string, - claudeDir: string -): HealthCheck { +export function checkSettingsSymlinks(ccsDir: string, claudeDir: string): HealthCheck { try { - const sharedDir = `${homedir}/.ccs/shared`; + const sharedDir = `${ccsDir}/shared`; const sharedSettings = `${sharedDir}/settings.json`; const claudeSettings = `${claudeDir}/settings.json`; diff --git a/src/web-server/middleware/auth-middleware.ts b/src/web-server/middleware/auth-middleware.ts index af4dd424..664f5782 100644 --- a/src/web-server/middleware/auth-middleware.ts +++ b/src/web-server/middleware/auth-middleware.ts @@ -24,7 +24,9 @@ declare module 'express-session' { const PUBLIC_PATHS = ['/api/auth/login', '/api/auth/check', '/api/auth/setup', '/api/health']; /** Path to persistent session secret file */ -const SESSION_SECRET_PATH = path.join(getCcsDir(), '.session-secret'); +function getSessionSecretPath() { + return path.join(getCcsDir(), '.session-secret'); +} /** * Generate or retrieve persistent session secret. @@ -38,8 +40,8 @@ function getSessionSecret(): string { // 2. Try to read persisted secret try { - if (fs.existsSync(SESSION_SECRET_PATH)) { - const secret = fs.readFileSync(SESSION_SECRET_PATH, 'utf-8').trim(); + if (fs.existsSync(getSessionSecretPath())) { + const secret = fs.readFileSync(getSessionSecretPath(), 'utf-8').trim(); if (secret.length >= 32) { return secret; } @@ -51,11 +53,11 @@ function getSessionSecret(): string { // 3. Generate and persist new random secret const newSecret = crypto.randomBytes(32).toString('hex'); try { - const dir = path.dirname(SESSION_SECRET_PATH); + const dir = path.dirname(getSessionSecretPath()); if (!fs.existsSync(dir)) { fs.mkdirSync(dir, { recursive: true }); } - fs.writeFileSync(SESSION_SECRET_PATH, newSecret, { mode: 0o600 }); + fs.writeFileSync(getSessionSecretPath(), newSecret, { mode: 0o600 }); } catch (err) { // Log warning - sessions won't persist across restarts console.warn('[!] Failed to persist session secret:', (err as Error).message); diff --git a/src/web-server/usage/aggregator.ts b/src/web-server/usage/aggregator.ts index 8be8ee93..7bbe4e79 100644 --- a/src/web-server/usage/aggregator.ts +++ b/src/web-server/usage/aggregator.ts @@ -31,22 +31,24 @@ import { getCcsDir } from '../../utils/config-manager'; // ============================================================================ /** Path to CCS instances directory */ -const CCS_INSTANCES_DIR = path.join(getCcsDir(), 'instances'); +function getCcsInstancesDir() { + return path.join(getCcsDir(), 'instances'); +} /** * Get list of CCS instance paths that have usage data * Only returns instances with existing projects/ directory */ function getInstancePaths(): string[] { - if (!fs.existsSync(CCS_INSTANCES_DIR)) { + if (!fs.existsSync(getCcsInstancesDir())) { return []; } try { - const entries = fs.readdirSync(CCS_INSTANCES_DIR, { withFileTypes: true }); + const entries = fs.readdirSync(getCcsInstancesDir(), { withFileTypes: true }); return entries .filter((entry) => entry.isDirectory()) - .map((entry) => path.join(CCS_INSTANCES_DIR, entry.name)) + .map((entry) => path.join(getCcsInstancesDir(), entry.name)) .filter((instancePath) => { // Only include instances that have a projects directory const projectsPath = path.join(instancePath, 'projects'); diff --git a/src/web-server/usage/disk-cache.ts b/src/web-server/usage/disk-cache.ts index 9053e5c1..46836bfc 100644 --- a/src/web-server/usage/disk-cache.ts +++ b/src/web-server/usage/disk-cache.ts @@ -15,8 +15,12 @@ import { ok, info, warn } from '../../utils/ui'; import { getCcsDir } from '../../utils/config-manager'; // Cache configuration -const CACHE_DIR = path.join(getCcsDir(), 'cache'); -const CACHE_FILE = path.join(CACHE_DIR, 'usage.json'); +function getCacheDir() { + return path.join(getCcsDir(), 'cache'); +} +function getCacheFile() { + return path.join(getCacheDir(), 'usage.json'); +} const CACHE_TTL_MS = 5 * 60 * 1000; // 5 minutes const STALE_TTL_MS = 7 * 24 * 60 * 60 * 1000; // 7 days (max age for stale data) @@ -38,8 +42,8 @@ const CACHE_VERSION = 3; * Ensure ~/.ccs/cache directory exists */ function ensureCacheDir(): void { - if (!fs.existsSync(CACHE_DIR)) { - fs.mkdirSync(CACHE_DIR, { recursive: true }); + if (!fs.existsSync(getCacheDir())) { + fs.mkdirSync(getCacheDir(), { recursive: true }); } } @@ -50,11 +54,11 @@ function ensureCacheDir(): void { */ export function readDiskCache(): UsageDiskCache | null { try { - if (!fs.existsSync(CACHE_FILE)) { + if (!fs.existsSync(getCacheFile())) { return null; } - const data = fs.readFileSync(CACHE_FILE, 'utf-8'); + const data = fs.readFileSync(getCacheFile(), 'utf-8'); const cache: UsageDiskCache = JSON.parse(data); // Version check - invalidate if schema changed @@ -112,9 +116,9 @@ export function writeDiskCache( }; // Write atomically using temp file + rename - const tempFile = CACHE_FILE + '.tmp'; + const tempFile = getCacheFile() + '.tmp'; fs.writeFileSync(tempFile, JSON.stringify(cache), 'utf-8'); - fs.renameSync(tempFile, CACHE_FILE); + fs.renameSync(tempFile, getCacheFile()); console.log(ok('Disk cache updated')); } catch (err) { @@ -144,8 +148,8 @@ export function getCacheAge(cache: UsageDiskCache | null): string { */ export function clearDiskCache(): void { try { - if (fs.existsSync(CACHE_FILE)) { - fs.unlinkSync(CACHE_FILE); + if (fs.existsSync(getCacheFile())) { + fs.unlinkSync(getCacheFile()); console.log(ok('Disk cache cleared')); } } catch (err) { diff --git a/tests/unit/config-dir-override.test.js b/tests/unit/config-dir-override.test.js index 93adf68b..29ca2435 100644 --- a/tests/unit/config-dir-override.test.js +++ b/tests/unit/config-dir-override.test.js @@ -153,5 +153,11 @@ describe('Config Directory Override', function () { const { detectCloudSyncPath } = require('../../dist/utils/config-manager'); assert.strictEqual(detectCloudSyncPath(path.join(os.homedir(), '.ccs')), null); }); + + it('should not false-positive on substrings (e.g., megauser, Dropbox-api)', function () { + const { detectCloudSyncPath } = require('../../dist/utils/config-manager'); + assert.strictEqual(detectCloudSyncPath('/home/megauser/.ccs'), null); + assert.strictEqual(detectCloudSyncPath('/home/kai/Dropbox-api-client/ccs'), null); + }); }); });