Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
5 changes: 3 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,13 @@
"testemonials"
],
"scripts": {
"postinstall": "node dist/track-installation.js",
"open-chat": "open -n /Applications/Claude.app",
"sync-version": "node scripts/sync-version.js",
"bump": "node scripts/sync-version.js --bump",
"bump:minor": "node scripts/sync-version.js --bump --minor",
"bump:major": "node scripts/sync-version.js --bump --major",
"build": "tsc && shx cp setup-claude-server.js uninstall-claude-server.js dist/ && shx chmod +x dist/*.js",
"build": "tsc && shx cp setup-claude-server.js uninstall-claude-server.js track-installation.js dist/ && shx chmod +x dist/*.js",
"watch": "tsc --watch",
"start": "node dist/index.js",
"start:debug": "node --inspect-brk=9229 dist/index.js",
Expand Down
1 change: 1 addition & 0 deletions src/config-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export interface ServerConfig {
defaultShell?: string;
allowedDirectories?: string[];
telemetryEnabled?: boolean; // New field for telemetry control
analyticsAuditEnabled?: boolean; // Enable/disable local analytics audit logging (default: false)
fileWriteLineLimit?: number; // Line limit for file write operations
fileReadLineLimit?: number; // Default line limit for file read operations (changed from character-based)
clientId?: string; // Unique client identifier for analytics
Expand Down
2 changes: 2 additions & 0 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ const CONFIG_DIR = path.join(USER_HOME, '.claude-server-commander');
// Paths relative to the config directory
export const CONFIG_FILE = path.join(CONFIG_DIR, 'config.json');
export const TOOL_CALL_FILE = path.join(CONFIG_DIR, 'claude_tool_call.log');
export const ANALYTICS_AUDIT_FILE = path.join(CONFIG_DIR, 'analytics_audit.log');
export const TOOL_CALL_FILE_MAX_SIZE = 1024 * 1024 * 10; // 10 MB
export const ANALYTICS_AUDIT_FILE_MAX_SIZE = 1024 * 1024 * 5; // 5 MB

export const DEFAULT_COMMAND_TIMEOUT = 1000; // milliseconds
22 changes: 10 additions & 12 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,16 @@ async function runServer() {
return;
}


try {
console.error("Loading configuration...");
await configManager.loadConfig();
console.error("Configuration loaded successfully");
} catch (configError) {
console.error(`Failed to load configuration: ${configError instanceof Error ? configError.message : String(configError)}`);
console.error(configError instanceof Error && configError.stack ? configError.stack : 'No stack trace available');
console.error("Continuing with in-memory configuration only");
// Continue anyway - we'll use an in-memory config
}

const transport = new FilteredStdioServerTransport();

Expand Down Expand Up @@ -100,17 +109,6 @@ async function runServer() {

capture('run_server_start');

try {
console.error("Loading configuration...");
await configManager.loadConfig();
console.error("Configuration loaded successfully");
} catch (configError) {
console.error(`Failed to load configuration: ${configError instanceof Error ? configError.message : String(configError)}`);
console.error(configError instanceof Error && configError.stack ? configError.stack : 'No stack trace available');
console.error("Continuing with in-memory configuration only");
// Continue anyway - we'll use an in-memory config
}


console.error("Connecting server...");
await server.connect(transport);
Expand Down
10 changes: 7 additions & 3 deletions src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -630,9 +630,13 @@ server.setRequestHandler(CallToolRequestSchema, async (request: CallToolRequest)
const {name, arguments: args} = request.params;

try {
capture_call_tool('server_call_tool', {
name
});
// Prepare telemetry data - add config key for set_config_value
const telemetryData: any = { name };
if (name === 'set_config_value' && args && typeof args === 'object' && 'key' in args) {
telemetryData.set_config_value_key_name = (args as any).key;
}

capture_call_tool('server_call_tool', telemetryData);

// Track tool call
trackToolCall(name, args);
Expand Down
157 changes: 145 additions & 12 deletions src/utils/capture.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import {platform} from 'os';
import {randomUUID} from 'crypto';
import * as https from 'https';
import * as fs from 'fs';
import * as path from 'path';
import {configManager} from '../config-manager.js';
import { currentClient } from '../server.js';
import { ANALYTICS_AUDIT_FILE, ANALYTICS_AUDIT_FILE_MAX_SIZE } from '../config.js';

let VERSION = 'unknown';
try {
Expand Down Expand Up @@ -70,6 +73,75 @@ export function sanitizeError(error: any): { message: string, code?: string } {
}


/**
* Log analytics data to local audit file for debugging and investigation
* Only logs if analyticsAuditEnabled is true in config (default: false)
* @param event Event name
* @param properties Event properties
* @param success Whether the analytics call succeeded
* @param error Any error that occurred
*/
async function logAnalyticsAudit(event: string, properties: any, success: boolean, error?: string) {
try {
// Check if analytics audit logging is enabled (default: false for production)
const analyticsAuditEnabled = await configManager.getValue('analyticsAuditEnabled');
if (!analyticsAuditEnabled) {
return; // Skip logging if not explicitly enabled
}

// Ensure analytics audit directory exists
const auditDir = path.dirname(ANALYTICS_AUDIT_FILE);
if (!fs.existsSync(auditDir)) {
await fs.promises.mkdir(auditDir, { recursive: true });
}

// Check if file size is approaching limit and rotate if needed
let fileSize = 0;
try {
const stats = await fs.promises.stat(ANALYTICS_AUDIT_FILE);
fileSize = stats.size;
} catch (error) {
// File doesn't exist yet, size remains 0
}

// If file size is at limit, rotate the log file
if (fileSize >= ANALYTICS_AUDIT_FILE_MAX_SIZE) {
const fileExt = path.extname(ANALYTICS_AUDIT_FILE);
const fileBase = path.basename(ANALYTICS_AUDIT_FILE, fileExt);
const dirName = path.dirname(ANALYTICS_AUDIT_FILE);

// Create a timestamp-based filename for the old log
const date = new Date();
const rotateTimestamp = `${date.getFullYear()}-${String(date.getMonth() + 1).padStart(2, '0')}-${String(date.getDate()).padStart(2, '0')}_${String(date.getHours()).padStart(2, '0')}-${String(date.getMinutes()).padStart(2, '0')}-${String(date.getSeconds()).padStart(2, '0')}`;
const newFileName = path.join(dirName, `${fileBase}_${rotateTimestamp}${fileExt}`);

// Rename the current file
await fs.promises.rename(ANALYTICS_AUDIT_FILE, newFileName);
}

// Prepare audit log entry
const timestamp = new Date().toISOString();
const auditEntry = {
timestamp,
event,
properties,
success,
error: error || null,
client_id: uniqueUserId
};

// Format as readable JSON log entry
const logLine = `${timestamp} | ${success ? 'SUCCESS' : 'FAILED'} | ${event} | ${JSON.stringify(auditEntry)}\n`;

// Append to audit log file
await fs.promises.appendFile(ANALYTICS_AUDIT_FILE, logLine, 'utf8');

} catch (auditError) {
// Don't let audit logging errors affect the main functionality
console.error(`Analytics audit logging error: ${auditError instanceof Error ? auditError.message : String(auditError)}`);
}
}

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.

🛠️ Refactor suggestion

⚠️ Potential issue

Audit file may capture unsanitized PII and “unknown” client_id in some paths.

  • When telemetry is skipped (Line 159) and in the general catch (Lines 346-348), you pass raw properties to logAnalyticsAudit. That can write unsanitized paths and error objects to disk.
  • uniqueUserId can remain 'unknown' for audit entries when telemetry is skipped.

Make logAnalyticsAudit robust: sanitize properties internally and lazily ensure a clientId before writing. This protects privacy even if callers forget to sanitize.

Apply this diff:

 async function logAnalyticsAudit(event: string, properties: any, success: boolean, error?: string) {
     try {
+        // Ensure we have a client id for auditing
+        if (uniqueUserId === 'unknown') {
+            try {
+                uniqueUserId = await getOrCreateUUID();
+            } catch {
+                // Best-effort; if it fails, we'll keep 'unknown'
+            }
+        }
+
         // Check if analytics audit logging is enabled (default: false for production)
         const analyticsAuditEnabled = await configManager.getValue('analyticsAuditEnabled');
         if (!analyticsAuditEnabled) {
             return; // Skip logging if not explicitly enabled
         }
 
         // Ensure analytics audit directory exists
         const auditDir = path.dirname(ANALYTICS_AUDIT_FILE);
         if (!fs.existsSync(auditDir)) {
             await fs.promises.mkdir(auditDir, { recursive: true });
         }
 
         // Check if file size is approaching limit and rotate if needed
         let fileSize = 0;
         try {
             const stats = await fs.promises.stat(ANALYTICS_AUDIT_FILE);
             fileSize = stats.size;
         } catch (error) {
             // File doesn't exist yet, size remains 0
         }
 
         // If file size is at limit, rotate the log file
         if (fileSize >= ANALYTICS_AUDIT_FILE_MAX_SIZE) {
             const fileExt = path.extname(ANALYTICS_AUDIT_FILE);
             const fileBase = path.basename(ANALYTICS_AUDIT_FILE, fileExt);
             const dirName = path.dirname(ANALYTICS_AUDIT_FILE);
             
             // Create a timestamp-based filename for the old log
             const date = new Date();
             const rotateTimestamp = `${date.getFullYear()}-${String(date.getMonth() + 1).padStart(2, '0')}-${String(date.getDate()).padStart(2, '0')}_${String(date.getHours()).padStart(2, '0')}-${String(date.getMinutes()).padStart(2, '0')}-${String(date.getSeconds()).padStart(2, '0')}`;
             const newFileName = path.join(dirName, `${fileBase}_${rotateTimestamp}${fileExt}`);
             
             // Rename the current file
             await fs.promises.rename(ANALYTICS_AUDIT_FILE, newFileName);
         }
 
         // Prepare audit log entry
         const timestamp = new Date().toISOString();
+        // Last line of defense: sanitize before persisting locally
+        const safeProperties = sanitizeProperties(properties || {});
         const auditEntry = {
             timestamp,
             event,
-            properties,
+            properties: safeProperties,
             success,
             error: error || null,
             client_id: uniqueUserId
         };
 
         // Format as readable JSON log entry
         const logLine = `${timestamp} | ${success ? 'SUCCESS' : 'FAILED'} | ${event} | ${JSON.stringify(auditEntry)}\n`;
         
         // Append to audit log file
         await fs.promises.appendFile(ANALYTICS_AUDIT_FILE, logLine, 'utf8');
         
     } catch (auditError) {
         // Don't let audit logging errors affect the main functionality
         console.error(`Analytics audit logging error: ${auditError instanceof Error ? auditError.message : String(auditError)}`);
     }
 }

Additionally, add this small helper (outside the selected range) so callers and audit have a single, correct sanitizer:

// Place near sanitizeError
function sanitizeProperties(input: any): any {
  let props: any;
  try {
    props = input ? JSON.parse(JSON.stringify(input)) : {};
  } catch {
    props = {};
  }

  if (props.error) {
    if (typeof props.error === 'object' && props.error !== null) {
      const sanitized = sanitizeError(props.error);
      props.error = sanitized.message;
      if (sanitized.code) props.errorCode = sanitized.code;
    } else if (typeof props.error === 'string') {
      props.error = sanitizeError(props.error).message;
    }
  }

  const sensitiveTokens = ['path', 'filepath', 'file_path', 'directory', 'sourcepath', 'destinationpath', 'fullpath', 'rootpath'];
  for (const k of Object.keys(props)) {
    const lk = k.toLowerCase();
    if (sensitiveTokens.some(t => lk.includes(t)) && lk !== 'fileextension') {
      delete props[k];
    }
  }
  return props;
}



/**
* Send an event to Google Analytics
Expand All @@ -83,6 +155,8 @@ export const captureBase = async (captureURL: string, event: string, properties?

// If telemetry is explicitly disabled or GA credentials are missing, don't send
if (telemetryEnabled === false || !captureURL) {
// Log that telemetry was skipped
await logAnalyticsAudit(event, properties || {}, false, telemetryEnabled === false ? 'Telemetry disabled' : 'GA credentials missing');
return;
}

Expand Down Expand Up @@ -139,16 +213,66 @@ export const captureBase = async (captureURL: string, event: string, properties?
isDXT = 'true';
}

// Is MCP running in a Docker container
let isDocker: string = 'false';
if (process.env.MCP_CLIENT_DOCKER) {
isDocker = 'true'
// Is MCP running in a container - use robust detection
const { getSystemInfo } = await import('./system-info.js');
const systemInfo = getSystemInfo();
const isContainer: string = systemInfo.docker.isContainer ? 'true' : 'false';
const containerType: string = systemInfo.docker.containerType || 'none';
const orchestrator: string = systemInfo.docker.orchestrator || 'none';

// Add container metadata (with privacy considerations)
let containerName: string = 'none';
let containerImage: string = 'none';

if (systemInfo.docker.isContainer && systemInfo.docker.containerEnvironment) {
const env = systemInfo.docker.containerEnvironment;

// Container name - sanitize to remove potentially sensitive info
if (env.containerName) {
// Keep only alphanumeric chars, dashes, and underscores
// Remove random IDs and UUIDs for privacy
containerName = env.containerName
.replace(/[0-9a-f]{8,}/gi, 'ID') // Replace long hex strings with 'ID'
.replace(/[0-9]{8,}/g, 'ID') // Replace long numeric IDs with 'ID'
.substring(0, 50); // Limit length
}

// Docker image - sanitize registry info for privacy
if (env.dockerImage) {
// Remove registry URLs and keep just image:tag format
containerImage = env.dockerImage
.replace(/^[^/]+\/[^/]+\//, '') // Remove registry.com/namespace/ prefix
.replace(/^[^/]+\//, '') // Remove simple registry.com/ prefix
.replace(/@sha256:.*$/, '') // Remove digest hashes
.substring(0, 100); // Limit length
}
}

// Detect if we're running through Smithery at runtime
let runtimeSource: string = 'unknown';
const processArgs = process.argv.join(' ');
try {
if (processArgs.includes('@smithery/cli') || processArgs.includes('smithery')) {
runtimeSource = 'smithery-runtime';
} else if (processArgs.includes('npx')) {
runtimeSource = 'npx-runtime';
} else {
runtimeSource = 'direct-runtime';
}
} catch (error) {
// Ignore detection errors
}

// Prepare standard properties
const baseProperties = {
timestamp: new Date().toISOString(),
platform: platform(),
isDocker,
isContainer,
containerType,
orchestrator,
containerName,
containerImage,
runtimeSource,
isDXT,
app_version: VERSION,
engagement_time_msec: "100"
Expand Down Expand Up @@ -190,28 +314,37 @@ export const captureBase = async (captureURL: string, event: string, properties?
data += chunk;
});

res.on('end', () => {
if (res.statusCode !== 200 && res.statusCode !== 204) {
// Optional debug logging
// console.debug(`GA tracking error: ${res.statusCode} ${data}`);
res.on('end', async () => {
const success = res.statusCode === 200 || res.statusCode === 204;
if (!success) {
// Log failure to audit
await logAnalyticsAudit(event, eventProperties, false, `HTTP ${res.statusCode}: ${data}`);
} else {
// Log success to audit
await logAnalyticsAudit(event, eventProperties, true);
}
});
});

req.on('error', () => {
req.on('error', async (error) => {
// Log error to audit
await logAnalyticsAudit(event, eventProperties, false, error.message);
// Silently fail - we don't want analytics issues to break functionality
});

// Set timeout to prevent blocking the app
req.setTimeout(3000, () => {
req.setTimeout(3000, async () => {
await logAnalyticsAudit(event, eventProperties, false, 'Request timeout');
req.destroy();
});

// Send data
req.write(postData);
req.end();

} catch {
} catch (error) {
// Log general error to audit
await logAnalyticsAudit(event, properties || {}, false, `General error: ${error instanceof Error ? error.message : String(error)}`);
// Silently fail - we don't want analytics issues to break functionality
}
};
Expand Down
Loading