diff --git a/docs/logging-security.md b/docs/logging-security.md new file mode 100644 index 0000000..3fc25ad --- /dev/null +++ b/docs/logging-security.md @@ -0,0 +1,275 @@ +# Logging Security and Credential Redaction + +This document describes the comprehensive credential redaction system implemented in the Claude GitHub Webhook service to prevent sensitive information from being exposed in logs. + +## Overview + +The logging system uses [Pino](https://getpino.io/) with comprehensive redaction patterns to automatically remove sensitive information from all log outputs. This ensures that credentials, secrets, tokens, and other sensitive data are never exposed in log files, console output, or external monitoring systems. + +## Redaction Coverage + +### Credential Types Protected + +#### 1. AWS Credentials +- **AWS_SECRET_ACCESS_KEY** - AWS secret access keys +- **AWS_ACCESS_KEY_ID** - AWS access key identifiers (AKIA* pattern) +- **AWS_SESSION_TOKEN** - Temporary session tokens +- **AWS_SECURITY_TOKEN** - Security tokens + +#### 2. GitHub Credentials +- **GITHUB_TOKEN** - GitHub personal access tokens (ghp_* pattern) +- **GH_TOKEN** - Alternative GitHub token environment variable +- **GitHub PAT tokens** - Fine-grained personal access tokens (github_pat_* pattern) +- **GITHUB_WEBHOOK_SECRET** - Webhook signature secrets + +#### 3. Anthropic API Keys +- **ANTHROPIC_API_KEY** - Claude API keys (sk-ant-* pattern) + +#### 4. Database Credentials +- **DATABASE_URL** - Full database connection strings +- **DB_PASSWORD** - Database passwords +- **REDIS_PASSWORD** - Redis authentication passwords +- **connectionString** - SQL Server connection strings +- **mongoUrl** - MongoDB connection URLs +- **redisUrl** - Redis connection URLs + +#### 5. Generic Sensitive Patterns +- **password**, **passwd**, **pass** - Any password fields +- **secret**, **secretKey**, **secret_key** - Any secret fields +- **token** - Any token fields +- **apiKey**, **api_key** - API key fields +- **credential**, **credentials** - Credential fields +- **key** - Generic key fields +- **privateKey**, **private_key** - Private key content +- **auth**, **authentication** - Authentication objects + +#### 6. JWT and Token Types +- **JWT_SECRET** - JWT signing secrets +- **ACCESS_TOKEN** - OAuth access tokens +- **REFRESH_TOKEN** - OAuth refresh tokens +- **BOT_TOKEN** - Bot authentication tokens +- **API_KEY** - Generic API keys +- **SECRET_KEY** - Generic secret keys + +#### 7. HTTP Headers +- **authorization** - Authorization headers +- **x-api-key** - API key headers +- **x-auth-token** - Authentication token headers +- **x-github-token** - GitHub token headers +- **bearer** - Bearer token headers + +### Context Coverage + +#### 1. Top-Level Fields +All sensitive field names are redacted when they appear as direct properties of logged objects. + +#### 2. Nested Objects (up to 4 levels deep) +Sensitive patterns are caught in deeply nested object structures: +- `object.nested.password` +- `config.database.connectionString` +- `application.config.api.secret` +- `deeply.nested.auth.token` + +#### 3. Environment Variable Containers +- **envVars.*** - Environment variable objects +- **env.*** - Environment configuration objects +- **process.env.*** - Process environment variables (using bracket notation) + +#### 4. Error Objects +- **error.message** - Error messages that might contain leaked credentials +- **error.stderr** - Standard error output +- **error.stdout** - Standard output +- **error.dockerCommand** - Docker commands with embedded secrets +- **err.*** - Alternative error object structures + +#### 5. Output Streams +- **stderr** - Standard error output +- **stdout** - Standard output +- **output** - Command output +- **logs** - Log content +- **message** - Message content +- **data** - Generic data fields + +#### 6. Docker and Command Context +- **dockerCommand** - Docker run commands with -e flags +- **dockerArgs** - Docker argument arrays +- **command** - Shell commands that might contain secrets + +#### 7. HTTP Request/Response Objects +- **request.headers.authorization** +- **response.headers.authorization** +- **req.headers.*** +- **res.headers.*** + +#### 8. File Paths +- **credentialsPath** - Paths to credential files +- **keyPath** - Paths to key files +- **secretPath** - Paths to secret files + +## Implementation Details + +### Pino Redaction Configuration + +The redaction is implemented using Pino's built-in `redact` feature with a comprehensive array of path patterns: + +```javascript +redact: { + paths: [ + // Over 200+ specific patterns covering all scenarios + 'password', + '*.password', + '*.*.password', + '*.*.*.password', + 'AWS_SECRET_ACCESS_KEY', + '*.AWS_SECRET_ACCESS_KEY', + 'envVars.AWS_SECRET_ACCESS_KEY', + '["process.env.AWS_SECRET_ACCESS_KEY"]', + // ... many more patterns + ], + censor: '[REDACTED]' +} +``` + +### Pattern Types + +1. **Direct patterns**: `'password'` - matches top-level fields +2. **Single wildcard**: `'*.password'` - matches one level deep +3. **Multi-wildcard**: `'*.*.password'` - matches multiple levels deep +4. **Bracket notation**: `'["process.env.GITHUB_TOKEN"]'` - handles special characters +5. **Nested paths**: `'envVars.AWS_SECRET_ACCESS_KEY'` - specific nested paths + +## Testing + +### Test Coverage + +The system includes comprehensive tests to verify redaction effectiveness: + +#### 1. Basic Redaction Test (`test-logger-redaction.js`) +- Tests all major credential types +- Verifies nested object redaction +- Ensures safe data remains visible + +#### 2. Comprehensive Test Suite (`test-logger-redaction-comprehensive.js`) +- 17 different test scenarios +- Tests deep nesting (4+ levels) +- Tests mixed safe/sensitive data +- Tests edge cases and complex structures + +### Running Tests + +```bash +# Run basic redaction test +node test/test-logger-redaction.js + +# Run comprehensive test suite +node test/test-logger-redaction-comprehensive.js + +# Run full test suite +npm test +``` + +### Validation Checklist + +When reviewing logs, ensure: + +✅ **Should be [REDACTED]:** +- All passwords, tokens, secrets, API keys +- AWS credentials and session tokens +- GitHub tokens and webhook secrets +- Database connection strings and passwords +- Docker commands containing sensitive environment variables +- Error messages containing leaked credentials +- HTTP headers with authorization data + +✅ **Should remain visible:** +- Usernames, emails, repo names, URLs +- Public configuration values +- Non-sensitive debugging information +- Timestamps, log levels, component names + +## Security Benefits + +### 1. Compliance +- Prevents credential exposure in logs +- Supports audit requirements +- Enables safe log aggregation and monitoring + +### 2. Development Safety +- Developers can safely share logs for debugging +- Reduces risk of accidental credential exposure +- Enables comprehensive logging without security concerns + +### 3. Production Security +- Log monitoring systems don't receive sensitive data +- External log services (CloudWatch, Datadog, etc.) are safe +- Log files can be safely stored and rotated + +### 4. Incident Response +- Detailed logs available for debugging without credential exposure +- Error correlation IDs help track issues without revealing secrets +- Safe log sharing between team members + +## Best Practices + +### 1. Regular Testing +- Run redaction tests after any logging changes +- Verify new credential patterns are covered +- Test with realistic data scenarios + +### 2. Pattern Maintenance +- Add new patterns when introducing new credential types +- Review and update patterns periodically +- Consider deep nesting levels for complex objects + +### 3. Monitoring +- Monitor logs for any credential leakage +- Use tools to scan logs for patterns that might indicate leaked secrets +- Review error logs regularly for potential exposure + +### 4. Development Guidelines +- Always use structured logging with the logger utility +- Avoid concatenating sensitive data into log messages +- Use specific log levels appropriately +- Test logging in development with real-like data structures + +## Configuration + +### Environment Variables +The logger automatically redacts these environment variables when they appear in logs: +- `GITHUB_TOKEN` +- `ANTHROPIC_API_KEY` +- `AWS_SECRET_ACCESS_KEY` +- `AWS_ACCESS_KEY_ID` +- `GITHUB_WEBHOOK_SECRET` +- And many more... + +### Log Levels +- **info**: General application flow +- **warn**: Potentially harmful situations +- **error**: Error events with full context (sanitized) +- **debug**: Detailed information for diagnosing problems + +### File Rotation +- Production logs are automatically rotated at 10MB +- Keeps up to 5 backup files +- All rotated logs maintain redaction + +## Troubleshooting + +### If credentials appear in logs: +1. Identify the specific pattern that wasn't caught +2. Add the new pattern to the redaction paths in `src/utils/logger.js` +3. Add a test case in the test files +4. Run tests to verify the fix +5. Deploy the updated configuration + +### Common issues: +- **Deep nesting**: Add more wildcard levels (`*.*.*.*.pattern`) +- **Special characters**: Use bracket notation (`["field-with-dashes"]`) +- **New credential types**: Add to all relevant categories (top-level, nested, env vars) + +## Related Documentation + +- [AWS Authentication Best Practices](./aws-authentication-best-practices.md) +- [Credential Security](./credential-security.md) +- [Container Security](./container-limitations.md) \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index 699c1de..6890ed8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -27,6 +27,9 @@ "nodemon": "^3.0.1", "prettier": "^3.0.0", "supertest": "^7.1.1" + }, + "engines": { + "node": ">=20.0.0" } }, "node_modules/@ampproject/remapping": { diff --git a/src/controllers/githubController.js b/src/controllers/githubController.js index 2742780..b2d420d 100644 --- a/src/controllers/githubController.js +++ b/src/controllers/githubController.js @@ -168,7 +168,7 @@ Complete the auto-tagging task using only GitHub CLI commands.`; }, 'Claude CLI tagging may have failed, attempting fallback' ); - + // Fall back to basic tagging based on keywords const fallbackLabels = await githubService.getFallbackLabels(issue.title, issue.body); if (fallbackLabels.length > 0) { @@ -444,7 +444,7 @@ Please check with an administrator to review the logs for more details.` // Check if we should wait for all check suites or use a specific trigger const triggerWorkflowName = process.env.PR_REVIEW_TRIGGER_WORKFLOW; const waitForAllChecks = process.env.PR_REVIEW_WAIT_FOR_ALL_CHECKS === 'true'; - + let shouldTriggerReview = false; let triggerReason = ''; @@ -456,22 +456,25 @@ Please check with an administrator to review the logs for more details.` }); shouldTriggerReview = allChecksPassed; - triggerReason = allChecksPassed ? 'All check suites passed' : 'Waiting for other check suites to complete'; + triggerReason = allChecksPassed + ? 'All check suites passed' + : 'Waiting for other check suites to complete'; } else { // Use specific workflow trigger const workflowName = await getWorkflowNameFromCheckSuite(checkSuite, repo); - + // For GitHub Actions, we need to check the actual workflow name // Since we can't reliably get it from the check suite alone, // we'll assume PR_REVIEW_TRIGGER_WORKFLOW matches if it's GitHub Actions - const effectiveWorkflowName = workflowName === 'GitHub Actions' ? triggerWorkflowName : workflowName; - + const effectiveWorkflowName = + workflowName === 'GitHub Actions' ? triggerWorkflowName : workflowName; + shouldTriggerReview = effectiveWorkflowName === triggerWorkflowName; - triggerReason = shouldTriggerReview ? - `Triggered by workflow: ${triggerWorkflowName}` : - `Workflow '${workflowName}' does not match trigger '${triggerWorkflowName}'`; + triggerReason = shouldTriggerReview + ? `Triggered by workflow: ${triggerWorkflowName}` + : `Workflow '${workflowName}' does not match trigger '${triggerWorkflowName}'`; } - + logger.info( { repo: repo.full_name, @@ -1074,7 +1077,7 @@ Please perform a comprehensive review of PR #${pr.number} in repository ${repo.f */ async function checkAllCheckSuitesComplete({ repo, pullRequests }) { const debounceDelayMs = parseInt(process.env.PR_REVIEW_DEBOUNCE_MS || '5000', 10); - + try { // Add a small delay to account for GitHub's eventual consistency await new Promise(resolve => setTimeout(resolve, debounceDelayMs)); @@ -1091,19 +1094,22 @@ async function checkAllCheckSuitesComplete({ repo, pullRequests }) { }); const checkSuites = checkSuitesResponse.check_suites || []; - - logger.info({ - repo: repo.full_name, - pr: pr.number, - sha: pr.head.sha, - totalCheckSuites: checkSuites.length, - checkSuites: checkSuites.map(cs => ({ - id: cs.id, - app: cs.app?.name, - status: cs.status, - conclusion: cs.conclusion - })) - }, 'Retrieved check suites for PR'); + + logger.info( + { + repo: repo.full_name, + pr: pr.number, + sha: pr.head.sha, + totalCheckSuites: checkSuites.length, + checkSuites: checkSuites.map(cs => ({ + id: cs.id, + app: cs.app?.name, + status: cs.status, + conclusion: cs.conclusion + })) + }, + 'Retrieved check suites for PR' + ); // Check if any check suite is still in progress or has failed for (const suite of checkSuites) { @@ -1114,34 +1120,43 @@ async function checkAllCheckSuitesComplete({ repo, pullRequests }) { // If any check is in progress, we should wait if (suite.status !== 'completed') { - logger.info({ - repo: repo.full_name, - pr: pr.number, - checkSuite: suite.id, - app: suite.app?.name, - status: suite.status - }, 'Check suite still in progress'); + logger.info( + { + repo: repo.full_name, + pr: pr.number, + checkSuite: suite.id, + app: suite.app?.name, + status: suite.status + }, + 'Check suite still in progress' + ); return false; } // If any check failed, we shouldn't review if (suite.conclusion !== 'success') { - logger.info({ - repo: repo.full_name, - pr: pr.number, - checkSuite: suite.id, - app: suite.app?.name, - conclusion: suite.conclusion - }, 'Check suite did not succeed'); + logger.info( + { + repo: repo.full_name, + pr: pr.number, + checkSuite: suite.id, + app: suite.app?.name, + conclusion: suite.conclusion + }, + 'Check suite did not succeed' + ); return false; } } } catch (error) { - logger.error({ - err: error, - repo: repo.full_name, - pr: pr.number - }, 'Failed to check PR status'); + logger.error( + { + err: error, + repo: repo.full_name, + pr: pr.number + }, + 'Failed to check PR status' + ); return false; } } @@ -1149,10 +1164,13 @@ async function checkAllCheckSuitesComplete({ repo, pullRequests }) { // All checks passed! return true; } catch (error) { - logger.error({ - err: error, - repo: repo.full_name - }, 'Failed to check all check suites'); + logger.error( + { + err: error, + repo: repo.full_name + }, + 'Failed to check all check suites' + ); return false; } } @@ -1160,7 +1178,7 @@ async function checkAllCheckSuitesComplete({ repo, pullRequests }) { /** * Extract workflow name from check suite by fetching check runs * @param {Object} checkSuite - The check suite object - * @param {Object} repo - The repository object + * @param {Object} repo - The repository object * @returns {Promise} - The workflow name or null */ async function getWorkflowNameFromCheckSuite(checkSuite, repo) { @@ -1171,7 +1189,7 @@ async function getWorkflowNameFromCheckSuite(checkSuite, repo) { // or head branch if available return checkSuite.app.name || 'GitHub Actions'; } - + // For other apps, return the app name return checkSuite.app ? checkSuite.app.name : null; } catch (error) { diff --git a/src/services/claudeService.js b/src/services/claudeService.js index 0155b96..01af369 100644 --- a/src/services/claudeService.js +++ b/src/services/claudeService.js @@ -96,16 +96,16 @@ For real functionality, please configure valid GitHub and Claude API tokens.`; // Select appropriate entrypoint script based on operation type let entrypointScript; switch (operationType) { - case 'auto-tagging': - entrypointScript = '/scripts/runtime/claudecode-tagging-entrypoint.sh'; - logger.info({ operationType }, 'Using minimal tools for auto-tagging operation'); - break; - case 'pr-review': - case 'default': - default: - entrypointScript = '/scripts/runtime/claudecode-entrypoint.sh'; - logger.info({ operationType }, 'Using full tool set for standard operation'); - break; + case 'auto-tagging': + entrypointScript = '/scripts/runtime/claudecode-tagging-entrypoint.sh'; + logger.info({ operationType }, 'Using minimal tools for auto-tagging operation'); + break; + case 'pr-review': + case 'default': + default: + entrypointScript = '/scripts/runtime/claudecode-entrypoint.sh'; + logger.info({ operationType }, 'Using full tool set for standard operation'); + break; } // Create unique container name (sanitized to prevent command injection) @@ -114,7 +114,7 @@ For real functionality, please configure valid GitHub and Claude API tokens.`; // Create the full prompt with context and instructions based on operation type let fullPrompt; - + if (operationType === 'auto-tagging') { fullPrompt = `You are Claude, an AI assistant analyzing a GitHub issue for automatic label assignment. @@ -210,49 +210,49 @@ Please complete this task fully and autonomously.`; ); // Build docker run command as an array to prevent command injection - const dockerArgs = [ - 'run', - '--rm' - ]; - + const dockerArgs = ['run', '--rm']; + // Apply container security constraints based on environment variables if (process.env.CLAUDE_CONTAINER_PRIVILEGED === 'true') { dockerArgs.push('--privileged'); } else { // Apply only necessary capabilities instead of privileged mode const requiredCapabilities = [ - 'NET_ADMIN', // Required for firewall setup - 'SYS_ADMIN' // Required for certain filesystem operations + 'NET_ADMIN', // Required for firewall setup + 'SYS_ADMIN' // Required for certain filesystem operations ]; - + // Add optional capabilities const optionalCapabilities = { - 'NET_RAW': process.env.CLAUDE_CONTAINER_CAP_NET_RAW === 'true', - 'SYS_TIME': process.env.CLAUDE_CONTAINER_CAP_SYS_TIME === 'true', - 'DAC_OVERRIDE': process.env.CLAUDE_CONTAINER_CAP_DAC_OVERRIDE === 'true', - 'AUDIT_WRITE': process.env.CLAUDE_CONTAINER_CAP_AUDIT_WRITE === 'true' + NET_RAW: process.env.CLAUDE_CONTAINER_CAP_NET_RAW === 'true', + SYS_TIME: process.env.CLAUDE_CONTAINER_CAP_SYS_TIME === 'true', + DAC_OVERRIDE: process.env.CLAUDE_CONTAINER_CAP_DAC_OVERRIDE === 'true', + AUDIT_WRITE: process.env.CLAUDE_CONTAINER_CAP_AUDIT_WRITE === 'true' }; - + // Add required capabilities requiredCapabilities.forEach(cap => { dockerArgs.push(`--cap-add=${cap}`); }); - + // Add optional capabilities if enabled Object.entries(optionalCapabilities).forEach(([cap, enabled]) => { if (enabled) { dockerArgs.push(`--cap-add=${cap}`); } }); - + // Add resource limits dockerArgs.push( - '--memory', process.env.CLAUDE_CONTAINER_MEMORY_LIMIT || '2g', - '--cpu-shares', process.env.CLAUDE_CONTAINER_CPU_SHARES || '1024', - '--pids-limit', process.env.CLAUDE_CONTAINER_PIDS_LIMIT || '256' + '--memory', + process.env.CLAUDE_CONTAINER_MEMORY_LIMIT || '2g', + '--cpu-shares', + process.env.CLAUDE_CONTAINER_CPU_SHARES || '1024', + '--pids-limit', + process.env.CLAUDE_CONTAINER_PIDS_LIMIT || '256' ); } - + // Add container name dockerArgs.push('--name', containerName); @@ -277,7 +277,7 @@ Please complete this task fully and autonomously.`; // Create sanitized version for logging (remove sensitive values) const sanitizedArgs = dockerArgs.map(arg => { if (typeof arg !== 'string') return arg; - + // Check if this is an environment variable assignment const envMatch = arg.match(/^([A-Z_]+)=(.*)$/); if (envMatch) { @@ -312,7 +312,7 @@ Please complete this task fully and autonomously.`; // Use promisified version of child_process.execFile (safer than exec) const { promisify } = require('util'); const execFileAsync = promisify(require('child_process').execFile); - + const result = await execFileAsync('docker', dockerArgs, { maxBuffer: 10 * 1024 * 1024, // 10MB buffer timeout: containerLifetimeMs // Container lifetime in milliseconds @@ -395,7 +395,7 @@ Please complete this task fully and autonomously.`; sanitized = sanitized.replace(new RegExp(escapedValue, 'g'), '[REDACTED]'); } }); - + // Then apply pattern-based redaction for any missed credentials const sensitivePatterns = [ /AKIA[0-9A-Z]{16}/g, // AWS Access Key pattern @@ -408,7 +408,7 @@ Please complete this task fully and autonomously.`; sensitivePatterns.forEach(pattern => { sanitized = sanitized.replace(pattern, '[REDACTED]'); }); - + return sanitized; }; @@ -423,10 +423,14 @@ Please complete this task fully and autonomously.`; ) { logger.error('Docker image not found. Attempting to rebuild...'); try { - execFileSync('docker', ['build', '-f', 'Dockerfile.claudecode', '-t', dockerImageName, '.'], { - cwd: path.join(__dirname, '../..'), - stdio: 'pipe' - }); + execFileSync( + 'docker', + ['build', '-f', 'Dockerfile.claudecode', '-t', dockerImageName, '.'], + { + cwd: path.join(__dirname, '../..'), + stdio: 'pipe' + } + ); logger.info('Successfully rebuilt Docker image'); } catch (rebuildError) { logger.error( diff --git a/src/services/githubService.js b/src/services/githubService.js index dcef777..dce62ab 100644 --- a/src/services/githubService.js +++ b/src/services/githubService.js @@ -418,7 +418,7 @@ async function getCombinedStatus({ repoOwner, repoName, ref }) { * Check if we've already reviewed this PR at the given commit SHA * @param {Object} params * @param {string} params.repoOwner - Repository owner - * @param {string} params.repoName - Repository name + * @param {string} params.repoName - Repository name * @param {number} params.prNumber - Pull request number * @param {string} params.commitSha - Commit SHA to check * @returns {Promise} True if already reviewed at this SHA @@ -456,9 +456,11 @@ async function hasReviewedPRAtCommit({ repoOwner, repoName, prNumber, commitSha // Check if any review mentions this specific commit SHA const botUsername = process.env.BOT_USERNAME || 'ClaudeBot'; const existingReview = reviews.find(review => { - return review.user.login === botUsername && - review.body && - review.body.includes(`commit: ${commitSha}`); + return ( + review.user.login === botUsername && + review.body && + review.body.includes(`commit: ${commitSha}`) + ); }); return !!existingReview; @@ -498,22 +500,27 @@ async function getCheckSuitesForRef({ repoOwner, repoName, ref }) { throw new Error('Invalid ref - contains unsafe characters'); } - logger.info({ - repo: `${repoOwner}/${repoName}`, - ref - }, 'Getting check suites for ref'); + logger.info( + { + repo: `${repoOwner}/${repoName}`, + ref + }, + 'Getting check suites for ref' + ); // In test mode, return mock data const client = getOctokit(); if (process.env.NODE_ENV === 'test' || !client) { return { total_count: 1, - check_suites: [{ - id: 12345, - app: { slug: 'github-actions', name: 'GitHub Actions' }, - status: 'completed', - conclusion: 'success' - }] + check_suites: [ + { + id: 12345, + app: { slug: 'github-actions', name: 'GitHub Actions' }, + status: 'completed', + conclusion: 'success' + } + ] }; } @@ -526,12 +533,15 @@ async function getCheckSuitesForRef({ repoOwner, repoName, ref }) { return data; } catch (error) { - logger.error({ - err: error.message, - repo: `${repoOwner}/${repoName}`, - ref - }, 'Failed to get check suites'); - + logger.error( + { + err: error.message, + repo: `${repoOwner}/${repoName}`, + ref + }, + 'Failed to get check suites' + ); + throw error; } } @@ -545,7 +555,13 @@ async function getCheckSuitesForRef({ repoOwner, repoName, ref }) { * @param {string[]} params.labelsToAdd - Labels to add * @param {string[]} params.labelsToRemove - Labels to remove */ -async function managePRLabels({ repoOwner, repoName, prNumber, labelsToAdd = [], labelsToRemove = [] }) { +async function managePRLabels({ + repoOwner, + repoName, + prNumber, + labelsToAdd = [], + labelsToRemove = [] +}) { try { // Validate parameters const repoPattern = /^[a-zA-Z0-9._-]+$/; diff --git a/src/utils/awsCredentialProvider.js b/src/utils/awsCredentialProvider.js index 0168d6c..f607880 100644 --- a/src/utils/awsCredentialProvider.js +++ b/src/utils/awsCredentialProvider.js @@ -15,18 +15,18 @@ class AWSCredentialProvider { /** * Get AWS credentials - PROFILES ONLY - * - * This method implements a caching mechanism to avoid repeatedly reading + * + * This method implements a caching mechanism to avoid repeatedly reading * credential files. It checks for cached credentials first, and only reads * from the filesystem if necessary. - * + * * The cached credentials are cleared when: * 1. clearCache() is called explicitly * 2. When credentials expire (for temporary credentials) - * + * * Static credentials from profiles don't expire, so they remain cached * until the process ends or cache is explicitly cleared. - * + * * @returns {Promise} Credential object with accessKeyId, secretAccessKey, and region * @throws {Error} If AWS_PROFILE is not set or credential retrieval fails */ diff --git a/src/utils/logger.js b/src/utils/logger.js index 9aff5bc..137a3ee 100644 --- a/src/utils/logger.js +++ b/src/utils/logger.js @@ -19,33 +19,33 @@ const logFileName = path.join(logsDir, 'app.log'); // Configure different transports based on environment const transport = isProduction ? { - targets: [ - // File transport for production - { - target: 'pino/file', - options: { destination: logFileName, mkdir: true } - }, - // Console pretty transport - { - target: 'pino-pretty', - options: { - colorize: true, - levelFirst: true, - translateTime: 'SYS:standard' + targets: [ + // File transport for production + { + target: 'pino/file', + options: { destination: logFileName, mkdir: true } }, - level: 'info' - } - ] - } - : { - // Just use pretty logs in development - target: 'pino-pretty', - options: { - colorize: true, - levelFirst: true, - translateTime: 'SYS:standard' + // Console pretty transport + { + target: 'pino-pretty', + options: { + colorize: true, + levelFirst: true, + translateTime: 'SYS:standard' + }, + level: 'info' + } + ] } - }; + : { + // Just use pretty logs in development + target: 'pino-pretty', + options: { + colorize: true, + levelFirst: true, + translateTime: 'SYS:standard' + } + }; // Configure the logger const logger = pino({ @@ -64,45 +64,305 @@ const logger = pino({ }, redact: { paths: [ + // HTTP headers that might contain credentials 'headers.authorization', + 'headers["x-api-key"]', + 'headers["x-auth-token"]', + 'headers["x-github-token"]', + 'headers.bearer', + '*.headers.authorization', + '*.headers["x-api-key"]', + '*.headers["x-auth-token"]', + '*.headers["x-github-token"]', + '*.headers.bearer', + + // Generic sensitive field patterns (top-level) + 'password', + 'passwd', + 'pass', + 'token', + 'secret', + 'secretKey', + 'secret_key', + 'apiKey', + 'api_key', + 'credential', + 'credentials', + 'key', + 'private', + 'privateKey', + 'private_key', + 'auth', + 'authentication', + + // Generic sensitive field patterns (nested) '*.password', + '*.passwd', + '*.pass', '*.token', '*.secret', '*.secretKey', + '*.secret_key', + '*.apiKey', + '*.api_key', + '*.credential', + '*.credentials', + '*.key', + '*.private', + '*.privateKey', + '*.private_key', + '*.auth', + '*.authentication', + + // Specific environment variables (top-level) 'AWS_SECRET_ACCESS_KEY', 'AWS_ACCESS_KEY_ID', + 'AWS_SESSION_TOKEN', + 'AWS_SECURITY_TOKEN', 'GITHUB_TOKEN', 'GH_TOKEN', 'ANTHROPIC_API_KEY', + 'GITHUB_WEBHOOK_SECRET', + 'WEBHOOK_SECRET', + 'BOT_TOKEN', + 'API_KEY', + 'SECRET_KEY', + 'ACCESS_TOKEN', + 'REFRESH_TOKEN', + 'JWT_SECRET', + 'DATABASE_URL', + 'DB_PASSWORD', + 'REDIS_PASSWORD', + + // Nested in any object (*) '*.AWS_SECRET_ACCESS_KEY', '*.AWS_ACCESS_KEY_ID', + '*.AWS_SESSION_TOKEN', + '*.AWS_SECURITY_TOKEN', '*.GITHUB_TOKEN', '*.GH_TOKEN', '*.ANTHROPIC_API_KEY', + '*.GITHUB_WEBHOOK_SECRET', + '*.WEBHOOK_SECRET', + '*.BOT_TOKEN', + '*.API_KEY', + '*.SECRET_KEY', + '*.ACCESS_TOKEN', + '*.REFRESH_TOKEN', + '*.JWT_SECRET', + '*.DATABASE_URL', + '*.DB_PASSWORD', + '*.REDIS_PASSWORD', + + // Docker-related sensitive content 'dockerCommand', '*.dockerCommand', + 'dockerArgs', + '*.dockerArgs', + 'command', + '*.command', + + // Environment variable containers 'envVars.AWS_SECRET_ACCESS_KEY', 'envVars.AWS_ACCESS_KEY_ID', + 'envVars.AWS_SESSION_TOKEN', + 'envVars.AWS_SECURITY_TOKEN', 'envVars.GITHUB_TOKEN', 'envVars.GH_TOKEN', 'envVars.ANTHROPIC_API_KEY', + 'envVars.GITHUB_WEBHOOK_SECRET', + 'envVars.WEBHOOK_SECRET', + 'envVars.BOT_TOKEN', + 'envVars.API_KEY', + 'envVars.SECRET_KEY', + 'envVars.ACCESS_TOKEN', + 'envVars.REFRESH_TOKEN', + 'envVars.JWT_SECRET', + 'envVars.DATABASE_URL', + 'envVars.DB_PASSWORD', + 'envVars.REDIS_PASSWORD', + 'env.AWS_SECRET_ACCESS_KEY', 'env.AWS_ACCESS_KEY_ID', + 'env.AWS_SESSION_TOKEN', + 'env.AWS_SECURITY_TOKEN', 'env.GITHUB_TOKEN', 'env.GH_TOKEN', 'env.ANTHROPIC_API_KEY', + 'env.GITHUB_WEBHOOK_SECRET', + 'env.WEBHOOK_SECRET', + 'env.BOT_TOKEN', + 'env.API_KEY', + 'env.SECRET_KEY', + 'env.ACCESS_TOKEN', + 'env.REFRESH_TOKEN', + 'env.JWT_SECRET', + 'env.DATABASE_URL', + 'env.DB_PASSWORD', + 'env.REDIS_PASSWORD', + + // Process environment variables (using bracket notation for nested objects) + 'process["env"]["AWS_SECRET_ACCESS_KEY"]', + 'process["env"]["AWS_ACCESS_KEY_ID"]', + 'process["env"]["AWS_SESSION_TOKEN"]', + 'process["env"]["AWS_SECURITY_TOKEN"]', + 'process["env"]["GITHUB_TOKEN"]', + 'process["env"]["GH_TOKEN"]', + 'process["env"]["ANTHROPIC_API_KEY"]', + 'process["env"]["GITHUB_WEBHOOK_SECRET"]', + 'process["env"]["WEBHOOK_SECRET"]', + 'process["env"]["BOT_TOKEN"]', + 'process["env"]["API_KEY"]', + 'process["env"]["SECRET_KEY"]', + 'process["env"]["ACCESS_TOKEN"]', + 'process["env"]["REFRESH_TOKEN"]', + 'process["env"]["JWT_SECRET"]', + 'process["env"]["DATABASE_URL"]', + 'process["env"]["DB_PASSWORD"]', + 'process["env"]["REDIS_PASSWORD"]', + + // Process environment variables (as top-level bracket notation keys) + '["process.env.AWS_SECRET_ACCESS_KEY"]', + '["process.env.AWS_ACCESS_KEY_ID"]', + '["process.env.AWS_SESSION_TOKEN"]', + '["process.env.AWS_SECURITY_TOKEN"]', + '["process.env.GITHUB_TOKEN"]', + '["process.env.GH_TOKEN"]', + '["process.env.ANTHROPIC_API_KEY"]', + '["process.env.GITHUB_WEBHOOK_SECRET"]', + '["process.env.WEBHOOK_SECRET"]', + '["process.env.BOT_TOKEN"]', + '["process.env.API_KEY"]', + '["process.env.SECRET_KEY"]', + '["process.env.ACCESS_TOKEN"]', + '["process.env.REFRESH_TOKEN"]', + '["process.env.JWT_SECRET"]', + '["process.env.DATABASE_URL"]', + '["process.env.DB_PASSWORD"]', + '["process.env.REDIS_PASSWORD"]', + + // Output streams that might contain leaked credentials 'stderr', '*.stderr', 'stdout', '*.stdout', + 'output', + '*.output', + 'logs', + '*.logs', + 'message', + '*.message', + 'data', + '*.data', + + // Error objects that might contain sensitive information 'error.dockerCommand', 'error.stderr', 'error.stdout', - 'process.env.GITHUB_TOKEN', - 'process.env.GH_TOKEN', - 'process.env.ANTHROPIC_API_KEY', - 'process.env.AWS_SECRET_ACCESS_KEY', - 'process.env.AWS_ACCESS_KEY_ID' + 'error.output', + 'error.message', + 'error.data', + 'err.dockerCommand', + 'err.stderr', + 'err.stdout', + 'err.output', + 'err.message', + 'err.data', + + // HTTP request/response objects + 'request.headers.authorization', + 'response.headers.authorization', + 'req.headers.authorization', + 'res.headers.authorization', + '*.request.headers.authorization', + '*.response.headers.authorization', + '*.req.headers.authorization', + '*.res.headers.authorization', + + // File paths that might contain credentials + 'credentialsPath', + '*.credentialsPath', + 'keyPath', + '*.keyPath', + 'secretPath', + '*.secretPath', + + // Database connection strings and configurations + 'connectionString', + '*.connectionString', + 'dbUrl', + '*.dbUrl', + 'mongoUrl', + '*.mongoUrl', + 'redisUrl', + '*.redisUrl', + + // Authentication objects + 'auth.token', + 'auth.secret', + 'auth.key', + 'auth.password', + '*.auth.token', + '*.auth.secret', + '*.auth.key', + '*.auth.password', + 'authentication.token', + 'authentication.secret', + 'authentication.key', + 'authentication.password', + '*.authentication.token', + '*.authentication.secret', + '*.authentication.key', + '*.authentication.password', + + // Deep nested patterns (up to 4 levels deep) + '*.*.password', + '*.*.secret', + '*.*.token', + '*.*.apiKey', + '*.*.api_key', + '*.*.credential', + '*.*.key', + '*.*.privateKey', + '*.*.private_key', + '*.*.AWS_SECRET_ACCESS_KEY', + '*.*.AWS_ACCESS_KEY_ID', + '*.*.GITHUB_TOKEN', + '*.*.ANTHROPIC_API_KEY', + '*.*.connectionString', + '*.*.DATABASE_URL', + + '*.*.*.password', + '*.*.*.secret', + '*.*.*.token', + '*.*.*.apiKey', + '*.*.*.api_key', + '*.*.*.credential', + '*.*.*.key', + '*.*.*.privateKey', + '*.*.*.private_key', + '*.*.*.AWS_SECRET_ACCESS_KEY', + '*.*.*.AWS_ACCESS_KEY_ID', + '*.*.*.GITHUB_TOKEN', + '*.*.*.ANTHROPIC_API_KEY', + '*.*.*.connectionString', + '*.*.*.DATABASE_URL', + + '*.*.*.*.password', + '*.*.*.*.secret', + '*.*.*.*.token', + '*.*.*.*.apiKey', + '*.*.*.*.api_key', + '*.*.*.*.credential', + '*.*.*.*.key', + '*.*.*.*.privateKey', + '*.*.*.*.private_key', + '*.*.*.*.AWS_SECRET_ACCESS_KEY', + '*.*.*.*.AWS_ACCESS_KEY_ID', + '*.*.*.*.GITHUB_TOKEN', + '*.*.*.*.ANTHROPIC_API_KEY', + '*.*.*.*.connectionString', + '*.*.*.*.DATABASE_URL' ], censor: '[REDACTED]' } diff --git a/test/debug-check-suite-webhook.js b/test/debug-check-suite-webhook.js index 8aeb739..7f541aa 100644 --- a/test/debug-check-suite-webhook.js +++ b/test/debug-check-suite-webhook.js @@ -31,7 +31,7 @@ app.use((req, res, next) => { app.post('/webhook', (req, res) => { const event = req.headers['x-github-event']; const delivery = req.headers['x-github-delivery']; - + logger.info( { event, @@ -91,4 +91,4 @@ app.listen(PORT, () => { console.log('2. Make sure to include check_suite events in the webhook configuration'); console.log('3. Trigger a check suite completion in your repository'); console.log('4. Check the logs above for detailed information\n'); -}); \ No newline at end of file +}); diff --git a/test/test-credential-leak.js b/test/test-credential-leak.js index bb79ae5..224d5d2 100644 --- a/test/test-credential-leak.js +++ b/test/test-credential-leak.js @@ -14,7 +14,7 @@ console.log('Testing credential sanitization...\n'); // Test dockerCommand sanitization const dockerCommand = `docker run --rm --privileged -e GITHUB_TOKEN="${mockEnv.GITHUB_TOKEN}" -e AWS_ACCESS_KEY_ID="${mockEnv.AWS_ACCESS_KEY_ID}" -e AWS_SECRET_ACCESS_KEY="${mockEnv.AWS_SECRET_ACCESS_KEY}" claude-code-runner:latest`; -const sanitizedCommand = dockerCommand.replace(/-e [A-Z_]+="[^"]*"/g, (match) => { +const sanitizedCommand = dockerCommand.replace(/-e [A-Z_]+="[^"]*"/g, match => { const envKey = match.match(/-e ([A-Z_]+)="/)[1]; const sensitiveKeys = ['GITHUB_TOKEN', 'AWS_ACCESS_KEY_ID', 'AWS_SECRET_ACCESS_KEY']; if (sensitiveKeys.includes(envKey)) { @@ -37,7 +37,7 @@ AWS Secret: ${mockEnv.AWS_SECRET_ACCESS_KEY} Some other error information `; -const sanitizeOutput = (output) => { +const sanitizeOutput = output => { if (!output) return output; let sanitized = output.toString(); const sensitiveValues = [ @@ -45,7 +45,7 @@ const sanitizeOutput = (output) => { mockEnv.AWS_ACCESS_KEY_ID, mockEnv.AWS_SECRET_ACCESS_KEY ].filter(val => val && val.length > 0); - + sensitiveValues.forEach(value => { if (value) { sanitized = sanitized.replace(new RegExp(value, 'g'), '[REDACTED]'); @@ -78,4 +78,4 @@ if (failedChecks.length === 0) { } else { console.log('❌ FAILED: The following credentials were found:'); failedChecks.forEach(check => console.log(` - ${check}`)); -} \ No newline at end of file +} diff --git a/test/test-logger-redaction-comprehensive.js b/test/test-logger-redaction-comprehensive.js new file mode 100644 index 0000000..bdcb174 --- /dev/null +++ b/test/test-logger-redaction-comprehensive.js @@ -0,0 +1,293 @@ +/** + * Comprehensive test script to verify logger credential redaction coverage + * Tests all credential patterns, nested objects, and edge cases + */ + +const { createLogger } = require('../src/utils/logger'); + +// Create a test logger +const logger = createLogger('test-redaction-comprehensive'); + +console.log('Testing comprehensive logger redaction coverage...\n'); + +// Test counter to track number of tests +let testCount = 0; + +/** + * Test helper to run a redaction test + * @param {string} testName - Name of the test + * @param {object} testData - Data to log and test + */ +function runRedactionTest(testName, testData) { + testCount++; + console.log(`\n=== Test ${testCount}: ${testName} ===`); + logger.info(testData, `Testing: ${testName}`); +} + +// Test 1: AWS Credentials in various forms +runRedactionTest('AWS Credentials - Direct', { + AWS_SECRET_ACCESS_KEY: 'wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY', + AWS_ACCESS_KEY_ID: 'AKIAIOSFODNN7EXAMPLE', + AWS_SESSION_TOKEN: 'AQoDYXdzEJr...', + AWS_SECURITY_TOKEN: 'AQoDYXdzEJr...' +}); + +// Test 2: AWS Credentials nested in objects +runRedactionTest('AWS Credentials - Nested', { + config: { + AWS_SECRET_ACCESS_KEY: 'wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY', + AWS_ACCESS_KEY_ID: 'AKIAIOSFODNN7EXAMPLE' + }, + envVars: { + AWS_SECRET_ACCESS_KEY: 'another-secret-key', + AWS_ACCESS_KEY_ID: 'AKIAI44QH8DHBEXAMPLE' + }, + env: { + AWS_SECRET_ACCESS_KEY: 'yet-another-secret', + AWS_SESSION_TOKEN: 'session-token-example' + } +}); + +// Test 3: GitHub Tokens in various patterns +runRedactionTest('GitHub Tokens', { + GITHUB_TOKEN: 'github_pat_11ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890abcdefghijklmnopqrstuvwxyz', + GH_TOKEN: 'ghp_1234567890abcdefghijklmnopqrstuvwxyz', + token: 'github_pat_11ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890abcdefghijklmnopqrstuvwxyz', + secret: 'some-github-secret-value', + headers: { + authorization: + 'Bearer github_pat_11ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890abcdefghijklmnopqrstuvwxyz', + 'x-github-token': 'ghp_1234567890abcdefghijklmnopqrstuvwxyz' + } +}); + +// Test 4: Anthropic API Keys +runRedactionTest('Anthropic API Keys', { + ANTHROPIC_API_KEY: 'sk-ant-api03-1234567890abcdefghijklmnopqrstuvwxyz', + apiKey: 'sk-ant-api03-abcdefghijklmnopqrstuvwxyz1234567890', + api_key: 'sk-ant-another-key-example', + authentication: { + key: 'sk-ant-nested-key-example', + token: 'sk-ant-token-example' + } +}); + +// Test 5: Webhook Secrets +runRedactionTest('Webhook Secrets', { + GITHUB_WEBHOOK_SECRET: 'webhook-secret-12345', + WEBHOOK_SECRET: 'another-webhook-secret', + secretKey: 'my-secret-key-value', + secret_key: 'another_secret_key_value' +}); + +// Test 6: Database and Connection Strings +runRedactionTest('Database Credentials', { + DATABASE_URL: 'postgresql://user:password@host:port/database', + DB_PASSWORD: 'super-secret-db-password', + REDIS_PASSWORD: 'redis-secret-password', + connectionString: + 'Server=myServerAddress;Database=myDataBase;User Id=myUsername;Password=myPassword;', + dbUrl: 'mongodb://username:password@host:port/database', + mongoUrl: 'mongodb+srv://user:pass@cluster.mongodb.net/db', + redisUrl: 'redis://user:password@host:port/0' +}); + +// Test 7: Docker Commands with embedded secrets +runRedactionTest('Docker Commands', { + dockerCommand: + 'docker run -e GITHUB_TOKEN="ghp_secrettoken123" -e AWS_SECRET_ACCESS_KEY="secretkey456" myimage', + dockerArgs: [ + 'run', + '-e', + 'GITHUB_TOKEN=ghp_anothersecret789', + '-e', + 'AWS_SECRET_ACCESS_KEY=anothersecret' + ], + command: 'export AWS_SECRET_ACCESS_KEY="leaked-secret" && docker run myimage' +}); + +// Test 8: Process Environment Variables +runRedactionTest('Process Environment Variables', { + 'process.env.GITHUB_TOKEN': 'ghp_process_env_token', + 'process.env.AWS_SECRET_ACCESS_KEY': 'process-env-secret-key', + 'process.env.ANTHROPIC_API_KEY': 'sk-ant-process-env-key' +}); + +// Test 9: Output Streams (stderr, stdout) +runRedactionTest('Output Streams', { + stderr: 'Error: Authentication failed with token ghp_leaked_in_stderr', + stdout: 'Success: Connected with AWS_SECRET_ACCESS_KEY=leaked-in-stdout', + output: 'Command output contains GITHUB_TOKEN=ghp_output_leak', + logs: 'Log entry: Failed to authenticate with secret=leaked-secret', + message: 'Error message: Invalid API key sk-ant-leaked-key', + data: { + errorOutput: 'Data contains AWS_ACCESS_KEY_ID=AKIAI44QH8DHBEXAMPLE' + } +}); + +// Test 10: Error Objects +runRedactionTest('Error Objects', { + error: { + message: 'Connection failed with password=secret123', + stderr: 'Error: Invalid GITHUB_TOKEN=ghp_error_token', + stdout: 'Output: Using AWS_SECRET_ACCESS_KEY=error-secret', + dockerCommand: 'docker run -e SECRET_KEY="error-leaked-secret" image', + data: { + credential: 'leaked-credential-in-error' + } + }, + err: { + message: 'Another error with token=leaked-token', + output: 'Error output with API_KEY=leaked-api-key' + } +}); + +// Test 11: HTTP Headers and Request/Response Objects +runRedactionTest('HTTP Objects', { + headers: { + authorization: 'Bearer secret-bearer-token', + 'x-api-key': 'secret-api-key-header', + 'x-auth-token': 'secret-auth-token', + bearer: 'secret-bearer-value' + }, + request: { + headers: { + authorization: 'Bearer request-auth-token' + } + }, + response: { + headers: { + authorization: 'Bearer response-auth-token' + } + }, + req: { + headers: { + authorization: 'Bearer req-auth-token' + } + }, + res: { + headers: { + authorization: 'Bearer res-auth-token' + } + } +}); + +// Test 12: Generic sensitive field patterns +runRedactionTest('Generic Sensitive Fields', { + password: 'user-password-123', + passwd: 'another-password', + pass: 'simple-pass', + privateKey: 'private-key-content', + private_key: 'another-private-key', + credential: 'some-credential', + credentials: 'user-credentials', + key: 'secret-key-value', + private: 'private-data' +}); + +// Test 13: Authentication Objects +runRedactionTest('Authentication Objects', { + auth: { + token: 'auth-object-token', + secret: 'auth-object-secret', + key: 'auth-object-key', + password: 'auth-object-password' + }, + authentication: { + token: 'authentication-token', + secret: 'authentication-secret', + key: 'authentication-key', + password: 'authentication-password' + }, + userAuth: { + token: 'nested-user-auth-token', + secret: 'nested-user-auth-secret' + } +}); + +// Test 14: File Paths that might contain credentials +runRedactionTest('File Paths', { + credentialsPath: '/home/user/.aws/credentials', + keyPath: '/path/to/private/key.pem', + secretPath: '/secrets/api-key.txt' +}); + +// Test 15: JWT and other token types +runRedactionTest('JWT and Other Tokens', { + JWT_SECRET: 'jwt-secret-signing-key', + ACCESS_TOKEN: 'access-token-value', + REFRESH_TOKEN: 'refresh-token-value', + BOT_TOKEN: 'bot-token-secret', + API_KEY: 'api-key-secret-value', + SECRET_KEY: 'general-secret-key' +}); + +// Test 16: Complex nested structures +runRedactionTest('Complex Nested Structures', { + application: { + config: { + database: { + password: 'nested-db-password', + connectionString: 'Server=server;Password=nested-password;' + }, + api: { + key: 'nested-api-key', + secret: 'nested-api-secret', + auth: { + token: 'deeply-nested-token' + } + } + }, + runtime: { + envVars: { + GITHUB_TOKEN: 'runtime-github-token', + AWS_SECRET_ACCESS_KEY: 'runtime-aws-secret' + }, + process: { + env: { + ANTHROPIC_API_KEY: 'runtime-anthropic-key' + } + } + } + } +}); + +// Test 17: Mixed safe and sensitive data +runRedactionTest('Mixed Safe and Sensitive Data', { + // Safe data that should NOT be redacted + username: 'user123', + email: 'user@example.com', + repo: 'owner/repository', + issueNumber: 42, + url: 'https://api.github.com/repos/owner/repo', + + // Sensitive data that SHOULD be redacted + token: 'should-be-redacted-token', + secret: 'should-be-redacted-secret', + AWS_SECRET_ACCESS_KEY: 'should-be-redacted-aws-key', + + // Mixed nested object + config: { + publicSetting: 'this-is-fine', + apiKey: 'this-should-be-redacted', + debug: true, + password: 'this-should-be-redacted-too' + } +}); + +console.log('\n=== Redaction Test Summary ==='); +console.log(`Total tests run: ${testCount}`); +console.log('\nReview the logged output above. All sensitive values should appear as [REDACTED].'); +console.log('If you see any actual secrets, passwords, or tokens, the redaction is incomplete.'); +console.log('\nThe following should be visible (not redacted):'); +console.log('- Usernames, emails, repo names, URLs'); +console.log('- Public configuration values'); +console.log('- Non-sensitive debugging information'); +console.log('\nThe following should be [REDACTED]:'); +console.log('- All passwords, tokens, secrets, API keys'); +console.log('- AWS credentials and session tokens'); +console.log('- GitHub tokens and webhook secrets'); +console.log('- Database connection strings and passwords'); +console.log('- Docker commands containing sensitive environment variables'); +console.log('- Error messages containing leaked credentials'); +console.log('- HTTP headers with authorization data'); diff --git a/test/test-logger-redaction.js b/test/test-logger-redaction.js index 21b4bcb..25e5a07 100644 --- a/test/test-logger-redaction.js +++ b/test/test-logger-redaction.js @@ -1,5 +1,6 @@ /** - * Test script to verify logger credential redaction + * Enhanced test script to verify logger credential redaction + * This is the main test for backwards compatibility */ const { createLogger } = require('../src/utils/logger'); @@ -7,50 +8,153 @@ const { createLogger } = require('../src/utils/logger'); // Create a test logger const logger = createLogger('test-redaction'); -console.log('Testing logger redaction...\n'); +console.log('Testing enhanced logger redaction...\n'); -// Test various scenarios +// Test various scenarios with enhanced coverage const testData = { - // Direct sensitive fields - GITHUB_TOKEN: 'github_token_example_1234567890', - AWS_ACCESS_KEY_ID: 'EXAMPLE_KEY_ID', - AWS_SECRET_ACCESS_KEY: 'EXAMPLE_SECRET_KEY', + // Direct sensitive fields - expanded + GITHUB_TOKEN: 'github_pat_11ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890abcdefghijklmnopqrstuvwxyz', + GH_TOKEN: 'ghp_1234567890abcdefghijklmnopqrstuvwxyz', + AWS_ACCESS_KEY_ID: 'AKIAIOSFODNN7EXAMPLE', + AWS_SECRET_ACCESS_KEY: 'wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY', + AWS_SESSION_TOKEN: 'AQoDYXdzEJr...', + ANTHROPIC_API_KEY: 'sk-ant-api03-1234567890abcdefghijklmnopqrstuvwxyz', + GITHUB_WEBHOOK_SECRET: 'webhook-secret-example-123', - // Nested in envVars + // Nested in envVars - expanded envVars: { GITHUB_TOKEN: 'github_token_example_nested', - AWS_ACCESS_KEY_ID: 'EXAMPLE_NESTED_KEY_ID', - AWS_SECRET_ACCESS_KEY: 'EXAMPLE_NESTED_SECRET_KEY' + AWS_ACCESS_KEY_ID: 'AKIAI44QH8DHBEXAMPLE', + AWS_SECRET_ACCESS_KEY: 'nested-secret-key-example', + ANTHROPIC_API_KEY: 'sk-ant-nested-key-example', + DATABASE_URL: 'postgresql://user:password@host:port/database', + JWT_SECRET: 'jwt-signing-secret' }, - // Docker command - dockerCommand: - 'docker run -e GITHUB_TOKEN="github_token_example_command" -e AWS_SECRET_ACCESS_KEY="secretInCommand"', + // Nested in env object + env: { + GITHUB_TOKEN: 'env-github-token', + AWS_SECRET_ACCESS_KEY: 'env-aws-secret', + REDIS_PASSWORD: 'env-redis-password' + }, - // Error outputs - stderr: 'Error: Failed with token github_token_example_error and key EXAMPLE_ERROR_KEY_ID', - stdout: 'Output contains secret EXAMPLE_OUTPUT_SECRET_KEY', + // Docker command with multiple secrets + dockerCommand: + 'docker run -e GITHUB_TOKEN="github_pat_command_example" -e AWS_SECRET_ACCESS_KEY="secretInCommand" -e ANTHROPIC_API_KEY="sk-ant-command-key"', + + // Docker args array + dockerArgs: [ + 'run', + '-e', + 'GITHUB_TOKEN=ghp_array_token', + '-e', + 'AWS_SECRET_ACCESS_KEY=array_secret_key' + ], + + // Error outputs with more patterns + stderr: + 'Error: Failed with token github_pat_stderr_example and key AKIAI44QH8DHBEXAMPLE and secret wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY', + stdout: 'Output contains secret EXAMPLE_OUTPUT_SECRET_KEY and API key sk-ant-stdout-key', + output: 'Command output with password=secret123 and apiKey=leaked-key', + + // HTTP headers + headers: { + authorization: 'Bearer secret-auth-token', + 'x-api-key': 'secret-header-api-key', + 'x-github-token': 'ghp_header_token' + }, + + // Authentication objects + auth: { + token: 'auth-object-token', + secret: 'auth-object-secret', + password: 'auth-object-password' + }, + + // Generic sensitive fields + password: 'user-password-123', + secret: 'generic-secret-value', + privateKey: 'private-key-content', + apiKey: 'api-key-secret', + credential: 'user-credential', // Other fields that should pass through normalField: 'This is normal data', repo: 'owner/repo', - issueNumber: 123 + issueNumber: 123, + username: 'testuser', + email: 'test@example.com', + publicConfig: { + debug: true, + timeout: 5000, + retries: 3 + } }; // Log the test data -logger.info(testData, 'Testing logger redaction'); +logger.info(testData, 'Testing enhanced logger redaction'); -// Also test nested objects +// Test nested error objects with comprehensive scenarios logger.error( { error: { - message: 'Something failed', - dockerCommand: 'docker run -e AWS_SECRET_ACCESS_KEY="shouldBeRedacted"', - stderr: 'Contains AWS_SECRET_ACCESS_KEY=actualSecretKey' + message: 'Authentication failed with token github_pat_error_example', + dockerCommand: + 'docker run -e AWS_SECRET_ACCESS_KEY="shouldBeRedacted" -e GITHUB_TOKEN="ghp_should_be_redacted"', + stderr: + 'Contains AWS_SECRET_ACCESS_KEY=actualSecretKey and ANTHROPIC_API_KEY=sk-ant-leaked-key', + stdout: 'Output with JWT_SECRET=jwt-secret and DATABASE_URL=postgresql://user:pass@host/db', + data: { + credentials: 'nested-credential-data', + auth: { + token: 'deeply-nested-token' + } + } + }, + request: { + headers: { + authorization: 'Bearer request-auth-token' + } } }, - 'Testing nested redaction' + 'Testing comprehensive nested redaction' ); -console.log('\nCheck the logged output above - all sensitive values should show as [REDACTED]'); -console.log('If you see any actual secrets, the redaction is not working properly.'); +// Test process.env patterns +logger.warn( + { + 'process.env.GITHUB_TOKEN': 'process-env-github-token', + 'process.env.AWS_SECRET_ACCESS_KEY': 'process-env-aws-secret', + 'process.env.ANTHROPIC_API_KEY': 'process-env-anthropic-key' + }, + 'Testing process.env redaction' +); + +// Test database and connection strings +logger.info( + { + DATABASE_URL: 'postgresql://username:password@localhost:5432/mydb', + connectionString: 'Server=myServer;Database=myDB;User Id=myUser;Password=myPassword;', + mongoUrl: 'mongodb+srv://user:pass@cluster.mongodb.net/database', + redisUrl: 'redis://user:password@host:6379/0' + }, + 'Testing database connection string redaction' +); + +console.log('\n=== Enhanced Redaction Test Results ==='); +console.log('Check the logged output above - all sensitive values should show as [REDACTED]'); +console.log( + 'If you see any actual secrets, passwords, tokens, or API keys, the redaction needs improvement.' +); +console.log('\nSensitive patterns that should be redacted:'); +console.log('- GitHub tokens (github_pat_*, ghp_*)'); +console.log('- AWS credentials (AKIA*, access keys, session tokens)'); +console.log('- Anthropic API keys (sk-ant-*)'); +console.log('- Database passwords and connection strings'); +console.log('- Docker commands with embedded secrets'); +console.log('- HTTP authorization headers'); +console.log('- Generic passwords, secrets, tokens, API keys'); +console.log('\nData that should remain visible:'); +console.log('- Usernames, emails, repo names'); +console.log('- Public configuration values'); +console.log('- Non-sensitive debugging information'); diff --git a/test/unit/controllers/githubController-check-suite.test.js b/test/unit/controllers/githubController-check-suite.test.js index 5e4fe75..437c934 100644 --- a/test/unit/controllers/githubController-check-suite.test.js +++ b/test/unit/controllers/githubController-check-suite.test.js @@ -55,12 +55,10 @@ describe('GitHub Controller - Check Suite Events', () => { githubService.hasReviewedPRAtCommit.mockReset(); githubService.managePRLabels.mockReset(); githubService.getCheckSuitesForRef.mockReset(); - + // Mock the check runs API response to return the expected workflow name githubService.getCheckSuitesForRef.mockResolvedValue({ - check_runs: [ - { name: 'Pull Request CI' } - ] + check_runs: [{ name: 'Pull Request CI' }] }); }); @@ -105,14 +103,12 @@ describe('GitHub Controller - Check Suite Events', () => { // Mock workflow name extraction to match PR_REVIEW_TRIGGER_WORKFLOW githubService.getCheckSuitesForRef.mockResolvedValue({ - check_runs: [ - { name: 'Pull Request CI' } - ] + check_runs: [{ name: 'Pull Request CI' }] }); // Mock that PR has not been reviewed yet githubService.hasReviewedPRAtCommit.mockResolvedValue(false); - + // Mock label management githubService.managePRLabels.mockResolvedValue(); @@ -128,7 +124,7 @@ describe('GitHub Controller - Check Suite Events', () => { prNumber: 42, commitSha: 'pr-sha-123' }); - + // Verify labels were managed (in-progress and complete) expect(githubService.managePRLabels).toHaveBeenCalledTimes(2); expect(githubService.managePRLabels).toHaveBeenCalledWith({ @@ -140,7 +136,7 @@ describe('GitHub Controller - Check Suite Events', () => { }); expect(githubService.managePRLabels).toHaveBeenCalledWith({ repoOwner: 'owner', - repoName: 'repo', + repoName: 'repo', prNumber: 42, labelsToAdd: ['claude-review-complete'], labelsToRemove: ['claude-review-in-progress', 'claude-review-needed'] @@ -284,7 +280,7 @@ describe('GitHub Controller - Check Suite Events', () => { // Mock that neither PR has been reviewed yet githubService.hasReviewedPRAtCommit.mockResolvedValue(false); - + // Mock label management githubService.managePRLabels.mockResolvedValue(); @@ -295,7 +291,7 @@ describe('GitHub Controller - Check Suite Events', () => { // Verify both PRs were checked for existing reviews expect(githubService.hasReviewedPRAtCommit).toHaveBeenCalledTimes(2); - + // Verify Claude was called twice, once for each PR expect(claudeService.processCommand).toHaveBeenCalledTimes(2); @@ -354,7 +350,7 @@ describe('GitHub Controller - Check Suite Events', () => { // Mock that PR has not been reviewed yet githubService.hasReviewedPRAtCommit.mockResolvedValue(false); - + // Mock label management githubService.managePRLabels.mockResolvedValue(); @@ -417,7 +413,7 @@ describe('GitHub Controller - Check Suite Events', () => { // Verify Claude was NOT called because workflow doesn't match expect(claudeService.processCommand).not.toHaveBeenCalled(); - // Verify simple success response + // Verify simple success response expect(mockRes.status).toHaveBeenCalledWith(200); expect(mockRes.json).toHaveBeenCalledWith({ message: 'Webhook processed successfully' @@ -556,7 +552,6 @@ describe('GitHub Controller - Check Suite Events', () => { }); }); - it('should skip PR review when already reviewed at same commit', async () => { // Setup successful check suite with pull request mockReq.body = { diff --git a/test/unit/services/claudeService.test.js b/test/unit/services/claudeService.test.js index 8d2cccc..dc7f663 100644 --- a/test/unit/services/claudeService.test.js +++ b/test/unit/services/claudeService.test.js @@ -11,7 +11,7 @@ jest.mock('child_process', () => ({ })); jest.mock('util', () => ({ - promisify: jest.fn((fn) => { + promisify: jest.fn(fn => { if (fn.name === 'execFile') { return jest.fn().mockResolvedValue({ stdout: 'Claude response from container', @@ -37,13 +37,14 @@ jest.mock('../../../src/utils/logger', () => ({ })); jest.mock('../../../src/utils/sanitize', () => ({ - sanitizeBotMentions: jest.fn((input) => input) + sanitizeBotMentions: jest.fn(input => input) })); jest.mock('../../../src/utils/secureCredentials', () => ({ - get: jest.fn((key) => { + get: jest.fn(key => { if (key === 'GITHUB_TOKEN') return 'ghp_test_github_token_mock123456789012345678901234'; - if (key === 'ANTHROPIC_API_KEY') return 'sk-ant-test-anthropic-key12345678901234567890123456789'; + if (key === 'ANTHROPIC_API_KEY') + return 'sk-ant-test-anthropic-key12345678901234567890123456789'; return null; }) })); @@ -72,7 +73,7 @@ describe('Claude Service', () => { }); // Verify test mode response - expect(result).toContain('Hello! I\'m Claude responding to your request.'); + expect(result).toContain("Hello! I'm Claude responding to your request."); expect(result).toContain('test/repo'); expect(sanitizeBotMentions).toHaveBeenCalled(); @@ -83,35 +84,35 @@ describe('Claude Service', () => { test('processCommand should properly set up Docker command in production mode', async () => { // Mock for this test only const originalProcessCommand = claudeService.processCommand; - + // Override the actual function with a test implementation - claudeService.processCommand = async (options) => { + claudeService.processCommand = async options => { // Set production mode for this function const originalNodeEnv = process.env.NODE_ENV; process.env.NODE_ENV = 'production'; - + // Mock dependencies needed in production mode execFileSync.mockImplementation((cmd, args, _options) => { if (args[0] === 'inspect') return '{}'; return 'mocked output'; }); - + // Configure execFileAsync mock const execFileAsync = promisify(require('child_process').execFile); execFileAsync.mockResolvedValue({ stdout: 'Claude response from container', stderr: '' }); - + // Call the original implementation to test it const result = await originalProcessCommand(options); - + // Restore env process.env.NODE_ENV = originalNodeEnv; - + return result; }; - + try { // Call the overridden function await claudeService.processCommand({ @@ -120,7 +121,7 @@ describe('Claude Service', () => { command: 'Test command', isPullRequest: false }); - + // Our assertions happen in the override function // We just need to verify the execFileSync was called expect(execFileSync).toHaveBeenCalled(); @@ -133,13 +134,13 @@ describe('Claude Service', () => { test('processCommand should handle errors properly', async () => { // Save original function for restoration const originalProcessCommand = claudeService.processCommand; - + // Create a testing implementation - claudeService.processCommand = async (options) => { + claudeService.processCommand = async options => { // Set test environment variables const originalNodeEnv = process.env.NODE_ENV; process.env.NODE_ENV = 'production'; - + // Mock the Docker inspect to succeed execFileSync.mockImplementation((cmd, args, _options) => { if (args[0] === 'inspect') return '{}'; @@ -147,7 +148,7 @@ describe('Claude Service', () => { if (args[0] === 'kill') return ''; return 'mocked output'; }); - + // Mock execFileAsync to throw an error const execFileAsync = promisify(require('child_process').execFile); execFileAsync.mockRejectedValue({ @@ -155,23 +156,25 @@ describe('Claude Service', () => { stderr: 'Error: container exited with non-zero status', stdout: '' }); - + // Throw error from original implementation await originalProcessCommand(options); - + // Restore environment process.env.NODE_ENV = originalNodeEnv; }; - + try { // Call the function and expect it to throw - await expect(claudeService.processCommand({ - repoFullName: 'test/repo', - issueNumber: 123, - command: 'Test command', - isPullRequest: false - })).rejects.toThrow(); - + await expect( + claudeService.processCommand({ + repoFullName: 'test/repo', + issueNumber: 123, + command: 'Test command', + isPullRequest: false + }) + ).rejects.toThrow(); + // Verify execFileSync was called expect(execFileSync).toHaveBeenCalled(); } finally { @@ -183,57 +186,57 @@ describe('Claude Service', () => { test('processCommand should handle long commands properly', async () => { // Save original function for restoration const originalProcessCommand = claudeService.processCommand; - + // Create a testing implementation that checks for long command handling - claudeService.processCommand = async (options) => { + claudeService.processCommand = async options => { // Set up test environment const originalNodeEnv = process.env.NODE_ENV; process.env.NODE_ENV = 'production'; - + // Mock the Docker inspect to succeed execFileSync.mockImplementation((cmd, args, _options) => { if (args[0] === 'inspect') return '{}'; return 'mocked output'; }); - + // Configure execFileAsync mock const execFileAsync = promisify(require('child_process').execFile); execFileAsync.mockResolvedValue({ stdout: 'Claude response for long command', stderr: '' }); - + // Call the original implementation const result = await originalProcessCommand(options); - + // Verify Docker was called with the command as an environment variable expect(execFileAsync).toHaveBeenCalled(); const dockerArgs = execFileAsync.mock.calls[0][1]; - + // Check that COMMAND env var is present in the docker args // The format is ['-e', 'COMMAND=value'] - const commandEnvIndex = dockerArgs.findIndex((arg) => - typeof arg === 'string' && arg.startsWith('COMMAND=') + const commandEnvIndex = dockerArgs.findIndex( + arg => typeof arg === 'string' && arg.startsWith('COMMAND=') ); expect(commandEnvIndex).toBeGreaterThan(-1); - + // Restore environment process.env.NODE_ENV = originalNodeEnv; - + return result; }; - + try { // Call the function with a long command const longCommand = 'A'.repeat(1000); - + const result = await claudeService.processCommand({ repoFullName: 'test/repo', issueNumber: 123, command: longCommand, isPullRequest: false }); - + // Verify we got a response expect(result).toBe('Claude response for long command'); } finally { @@ -241,4 +244,4 @@ describe('Claude Service', () => { claudeService.processCommand = originalProcessCommand; } }); -}); \ No newline at end of file +}); diff --git a/test/unit/utils/awsCredentialProvider.test.js b/test/unit/utils/awsCredentialProvider.test.js index ed7ba36..852d4b5 100644 --- a/test/unit/utils/awsCredentialProvider.test.js +++ b/test/unit/utils/awsCredentialProvider.test.js @@ -1,4 +1,3 @@ - const fs = require('fs'); // Mock dependencies @@ -75,28 +74,28 @@ region = us-west-2 test('should cache credentials', async () => { // First clear any existing cache awsCredentialProvider.clearCache(); - + // Reset mock counters fs.promises.readFile.mockClear(); - + // First call should read from files const credentials1 = await awsCredentialProvider.getCredentials(); - + // Count how many times readFile was called on first request const firstCallCount = fs.promises.readFile.mock.calls.length; - + // Should be exactly 2 calls (credentials and config files) expect(firstCallCount).toBe(2); - + // Reset counter to clearly see calls for second request fs.promises.readFile.mockClear(); - + // Second call should use cached credentials and not read files again const credentials2 = await awsCredentialProvider.getCredentials(); - + // Verify credentials are the same object (cached) expect(credentials1).toBe(credentials2); - + // Verify no additional file reads occurred on second call expect(fs.promises.readFile).not.toHaveBeenCalled(); }); @@ -139,7 +138,7 @@ region = us-west-2 process.env.AWS_PROFILE = 'non-existent-profile'; await expect(awsCredentialProvider.getCredentials()).rejects.toThrow( - 'Profile \'non-existent-profile\' not found' + "Profile 'non-existent-profile' not found" ); // Restore AWS_PROFILE @@ -157,7 +156,7 @@ aws_access_key_id = test-access-key fs.promises.readFile.mockImplementationOnce(() => Promise.resolve(mockConfigFile)); await expect(awsCredentialProvider.getCredentials()).rejects.toThrow( - 'Incomplete credentials for profile \'test-profile\'' + "Incomplete credentials for profile 'test-profile'" ); }); });