From d86c53146ad7302600ad803faf0649391fa8f4e1 Mon Sep 17 00:00:00 2001 From: kaitranntt Date: Thu, 29 Jan 2026 17:28:45 -0500 Subject: [PATCH] refactor(cliproxy): address PR review suggestions - Add hash collision risk note to sanitizer docs (6-char MD5 = ~16M combos) - Fix VALID_CHARS_REGEX comment to include '/' character - Use getCcsDir() instead of os.homedir() for log path (CLAUDE.md compliance) - Add JSDoc for isRecord type guard helper --- src/cliproxy/tool-name-sanitizer.ts | 5 ++++- src/cliproxy/tool-sanitization-proxy.ts | 10 +++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/cliproxy/tool-name-sanitizer.ts b/src/cliproxy/tool-name-sanitizer.ts index f69b8a59..f8e48611 100644 --- a/src/cliproxy/tool-name-sanitizer.ts +++ b/src/cliproxy/tool-name-sanitizer.ts @@ -4,11 +4,14 @@ * Sanitizes MCP tool names to comply with Gemini API constraints: * - Max 64 characters * - Must start with letter or underscore - * - Only a-z A-Z 0-9 _ . : - allowed + * - Only a-z A-Z 0-9 _ . : - / allowed * * Strategies: * 1. Remove duplicate segments (e.g., gitmcp__foo__foo → gitmcp__foo) * 2. Smart truncate with hash suffix if still >64 chars + * + * Note: Hash collision risk is ~1 in 16M with 6-char MD5 prefix. + * Acceptable for typical MCP tool counts (<1000 tools). */ import { createHash } from 'crypto'; diff --git a/src/cliproxy/tool-sanitization-proxy.ts b/src/cliproxy/tool-sanitization-proxy.ts index db5dc301..3e6164e8 100644 --- a/src/cliproxy/tool-sanitization-proxy.ts +++ b/src/cliproxy/tool-sanitization-proxy.ts @@ -13,9 +13,9 @@ import * as http from 'http'; import * as https from 'https'; import * as fs from 'fs'; import * as path from 'path'; -import * as os from 'os'; import { URL } from 'url'; import { ToolNameMapper, type Tool, type ContentBlock } from './tool-name-mapper'; +import { getCcsDir } from '../utils/config-manager'; export interface ToolSanitizationProxyConfig { /** Upstream CLIProxy URL */ @@ -28,6 +28,10 @@ export interface ToolSanitizationProxyConfig { timeoutMs?: number; } +/** + * Type guard to check if a value is a plain object (Record). + * Used for safely accessing properties on unknown JSON values. + */ function isRecord(value: unknown): value is Record { return typeof value === 'object' && value !== null && !Array.isArray(value); } @@ -54,8 +58,7 @@ export class ToolSanitizationProxy { * Initialize log file path and ensure directory exists. */ private initLogFile(): string { - const ccsDir = process.env.CCS_HOME || path.join(os.homedir(), '.ccs'); - const logsDir = path.join(ccsDir, 'logs'); + const logsDir = path.join(getCcsDir(), 'logs'); try { if (!fs.existsSync(logsDir)) { @@ -63,6 +66,7 @@ export class ToolSanitizationProxy { } } catch { // Fallback to temp directory if logs dir creation fails + const os = require('os'); return path.join(os.tmpdir(), 'tool-sanitization-proxy.log'); }