Warden: code-review

[github-actions]

Warden Scheduled Scan Results

Run: 2026-07-06T09:18:38.560Z
Commit: db7e055

Summary

Severity	Count
High	5
Medium	19
Low	16

Findings

src/mcp/tools/project-scaffolding/scaffold_ios_project.ts

• W2X-8CE orientationToIOSConstant never matches schema enum values — raw strings written to xcconfig (L42-L56) · high
  The switch statement matches PascalCase strings ('Portrait', 'LandscapeLeft', etc.) but the Zod schema emits lowercase kebab-case values ('portrait', 'landscape-left', 'portrait-upside-down'). Every call falls through to default: return orientation, so the xcconfig file is written with raw values like portrait instead of UIInterfaceOrientationPortrait.
• KAR-8DS Hardcoded simulatorName: 'iPhone 17' causes runtime failure when that simulator is not installed (L404) · medium
  The next-step params always request 'iPhone 17', which will cause a build or launch error on any machine (CI or developer) that does not have an iPhone 17 simulator installed — consider using a more broadly available model like 'iPhone 16' or reading the available simulators dynamically.

src/utils/log-paths.ts

• 5X2-42T Path traversal via .. workspace key bypasses validation (L44-L49) · high
  The normalizeWorkspaceKey function blocks / and \ but not .., so a caller passing ".." would escape the workspaces directory (e.g. resolving to the app root). Consider adding if (normalized === '..' || normalized.startsWith('../') ...) or checking path.normalize(normalized) !== normalized.

src/utils/template-manager.ts

• P7M-R5N process.chdir in async context causes process-wide side effects and race conditions (L99-L113) · high
  Using process.chdir in an async function changes the working directory for the entire Node.js process; concurrent template downloads (e.g. iOS and macOS simultaneously) will race on CWD. Replace with unzip -q -d <tempDir> <zipPath> to extract to a specific directory without touching CWD.

src/utils/video_capture.ts

• TWP-RPL process.on('exit', stopAll) fires async cleanup that Node.js will abandon (L137) · high
  The exit event fires synchronously; Node.js abandons any pending async work immediately after all exit listeners return, so the void stopSession(...) promises will never resolve and recording sessions won't be stopped on normal process exit.

src/utils/xcodemake.ts

• J34-4TT Downloaded executable has no integrity verification before execution (L61-L79) · high
  The installXcodemake function fetches a script from GitHub's main branch and immediately makes it executable without verifying a checksum or signature — a compromised repository or MITM could silently deliver malicious code to users' machines.

src/benchmarks/claude-ui/harness.ts

• EH5-M5B resolveSuitePath silently returns a non-existent path when no suite file is found (L107) · medium
  When matches.length === 0, the function falls through to return candidates[0]! and returns a path that doesn't exist on disk, rather than throwing a descriptive error. Downstream callers will get a confusing file-not-found failure instead of a clear "suite not found" message. Consider throwing here analogously to the multi-match case.
• KRW-9LC Write streams lack error handlers, risking uncaught exceptions (L302-L303) · medium
  The stdout and stderr WriteStreams created via createWriteStream have no 'error' listeners; if a write fails (e.g. disk full), Node.js will throw an uncaught exception rather than routing the error through rejectCommand. Consider adding stdout.on('error', rejectCommand) and stderr.on('error', rejectCommand) immediately after the streams are created.
  Suggested fix: Attach error handlers to the write streams immediately after creation.

src/cli/cli-tool-catalog.ts

• FUF-KM2 No test coverage for buildCliToolCatalog business logic (L157) · medium
  The new buildCliToolCatalog function and its helpers (tool deduplication by cliName, merging catalog, getBridgeDiscoveryTimeoutMs env-var override) have no corresponding test file — consider adding unit tests covering the merge/dedup path and the daemon-not-running early-return paths.

src/cli/daemon-control.ts

• QYK-22S ensureDaemonRunning silently returns when status() fails with a non-version-mismatch error (L213) · medium
  When isRunning() returns true but status() throws any error that is not a DaemonVersionMismatchError (e.g. a timeout, connection reset, or unexpected protocol error), the else { return; } branch causes the function to return silently as if the daemon is ready, violating the documented contract that it "returns when the daemon is ready to accept requests."

src/daemon/framing.ts

• 5KJ-U9L O(n²) buffer growth from repeated Buffer.concat on every data chunk (L25) · medium
  Calling Buffer.concat([buffer, chunk]) on every data event copies the entire accumulated buffer into a new allocation each time. For a large message fragmented across many small TCP chunks this becomes quadratic in the total message size; consider using a preallocated ring buffer or an array of chunks flushed only when a complete frame is ready.

src/mcp/tools/debugging/debug_attach_sim.ts

• V4F-NEZ waitFor: true with bundleId fails at PID resolution before attach can wait (L143-L157) · medium
  When waitFor: true is used with bundleId (its documented-only-valid mode), resolveSimulatorAppPid is called first and throws if the app isn't already running — making it impossible to use waitFor to attach before a process launches, which is the common expectation for the flag.

src/mcp/tools/simulator/get_sim_app_path.ts

• 5HV-3G4 nextStepParams uses hardcoded 'SIMULATOR_UUID' instead of actual simulator ID from params (L171-L177) · medium
  The boot_sim, install_app_sim, and launch_app_sim entries in nextStepParams use the literal string 'SIMULATOR_UUID' instead of the actual simulator identifier from params.simulatorId ?? params.simulatorName, making these hints non-functional. Compare with boot_sim.ts which uses resolvedParams.simulatorId for the same fields.
  Suggested fix: Use actual simulator param values in nextStepParams

src/mcp/tools/swift-package/swift_package_run.ts

• V68-K22 Underlying process keeps running after timeout — resource leak (L292-L295) · medium
  When the timeout fires and wins the race, the commandPromise (and its child swift process) continues running until natural completion because there is no reference to the ChildProcess before the promise resolves — consider wrapping the executor call in an AbortController or storing the process handle so it can be killed on timeout.
• 2CE-RUE resolveExecutablePath spawns an extra subprocess even when background execution fails (L193-L199) · low
  Move the resolveExecutablePath call to after the if (!result.success) guard so swift build --show-bin-path is not run unnecessarily on failure.

src/snapshot-tests/suites/coverage-suite.ts

• 8GV-L7J beforeAll timeout too tight: inner harness.invoke alone can consume the full 120s budget (L42) · medium
  The beforeAll timeout is 120,000ms but it also runs harness.invoke('simulator', 'test', ...), which has its own 120,000ms internal timeout (SNAPSHOT_COMMAND_TIMEOUT_MS). Any time spent on harness creation, simulator boot, or temp directory setup will eat into the shared budget, causing beforeAll to be killed by Vitest before a clean subprocess-level timeout or assertion can fire. Consider increasing the beforeAll timeout to at least 240,000ms to allow for setup overhead on top of the subprocess limit.

src/snapshot-tests/suites/swift-package-suite.ts

• 4F7-2NG harness.cleanup() throws if beforeAll fails before assignment (L62) · medium
  If resetSwiftPackageState() or createHarnessForRuntime() throws in beforeAll, harness remains undefined, and harness.cleanup() in afterAll will throw a TypeError, masking the original error and potentially leaving resources uncleaned.

src/snapshot-tests/xcode-ide-availability.ts

• ABH-ZGR Two sequential blocking execSync calls at module load time can stall the event loop up to 16 seconds (L6-L14) · medium
  Both execSync calls run synchronously at module evaluation time (invoked in xcode-ide-suite.ts top-level const XCODE_IDE_BRIDGE_READY = isXcodeIdeBridgeAvailable()), so if either command hangs until its 8 s timeout the Node.js event loop is blocked for up to 16 s before any test can start. Consider making the check lazy (call it inside beforeAll) or using spawnSync with a shorter probe timeout if the only goal is exit-code detection.

src/utils/debugger/backends/lldb-cli-backend.ts

• 9HW-DE5 resume() response leaks into the next command's output buffer (L92) · medium
  After resume() writes process continue , it returns without emitting a sentinel, so LLDB's response (e.g. "Process 1234 resuming") accumulates in this.buffer. The next enqueued runCommand call's sentinel match will capture this leftover data as part of its output, causing assertNoLldbError to potentially throw spuriously if that response contains "error:", or silently corrupting the returned output.

src/utils/log-capture/simulator-launch-oslog-registry.ts

• 3KA-6VF N+1 ps subprocess spawning when batch sampleProcessCommands fails (L342-L346) · medium
  When the batch sampleProcessCommands call returns null (i.e., ps itself fails), both listSimulatorLaunchOsLogProtectedPaths (line 342) and listSimulatorLaunchOsLogRegistryRecords (line 387) fall through to sequential isRecordActive calls that each re-invoke sampleProcessCommands([record.helperPid]) — spawning N additional failing ps processes instead of reusing the known-failed result.

src/utils/logger.ts

• KVH-D8C Log file writes bypass severity level filter (L269-L275) · medium
  The file stream write at line 269 checks only clientLogLevel !== 'none' but doesn't compare severity, so debug-level messages are written to the log file even when clientLogLevel is set to a coarser level like 'error'. Consider calling shouldLog(level) before writing to the file stream, or restructure so the file write follows the same severity check.

src/utils/nskeyedarchiver-parser.ts

• HFL-RM7 Run destination silently skipped when ActiveScheme is absent (L128-L135) · medium
  When activeSchemeIdx === -1 (no ActiveScheme key present) but activeRunDestIdx !== -1, findDictWithKey(objects, -1) can never match a UID of -1, so parentDict is undefined and the function returns early before ever attempting to extract the run destination. Have you considered looking up the parent dict using activeRunDestIdx as a fallback when activeSchemeIdx === -1?

src/utils/renderers/cli-text-renderer.ts

• AKV-ECB No test coverage for xcodebuild-line processing path (L350-L409) · medium
  The new xcodebuild-line case and its supporting functions (ensureParserState, drainParserState, flushParserStates) have no tests in cli-text-renderer.test.ts, leaving the multi-operation parser state lifecycle, fragment buffering, and flush-on-finalize behaviour entirely untested.

src/benchmarks/claude-ui/config.ts

• TTD-YTB timeoutSeconds accepts zero, negative, NaN, or Infinity without validation error (L264) · low
  Consider using readOptionalPositiveFiniteNumber instead of readOptionalNumber for timeoutSeconds, as a value of 0, -1, NaN, or Infinity silently passes config parsing but would cause the first-run prompt dismissal timeout to fire immediately or behave unpredictably at runtime.
  Suggested fix: Replace readOptionalNumber with readOptionalPositiveFiniteNumber for the timeoutSeconds field.

src/cli/commands/daemon.ts

• FE3-QR2 Sequential daemon status checks in daemon list scale linearly with number of hung daemons (L295-L308) · low
  In the daemon list command, each registered daemon's status is checked serially with await client.status() inside a for...of loop. In the common failure modes (socket missing / connection refused) DaemonClient.request() rejects immediately, so this is only noticeable when a socket exists but the daemon process is hung — in that case each hung entry burns the full 1000 ms timeout, and total wall time grows as N × 1000 ms. Since this is a user-invoked CLI listing command with typically few entries, impact is limited, but running the checks concurrently via Promise.allSettled would cap worst-case latency at ~1 s regardless of N.

src/cli/commands/setup.ts

• T6N-TM7 withSpinner shows start message on task failure instead of a failure indicator (L112-L113) · low
  In the catch block of withSpinner, s.stop(opts.startMessage) is called with the start message rather than a failure message. When a task throws, the spinner halts with the "starting" label (e.g. "Discovering projects…") as its final displayed text before the error propagates, which is confusing UX. Consider passing a distinct failure string (e.g. `Failed: ${opts.startMessage}`) and/or a non-zero exit code to s.stop so users can distinguish success from failure.

src/mcp/tools/doctor/doctor.ts

• HWU-TMW Doctor reports Sentry enabled incorrectly when XCODEBUILDMCP_SENTRY_DISABLED is set (L713) · low
  The Sentry status line only checks SENTRY_DISABLED, so users who disabled Sentry via the CLI setup (which sets XCODEBUILDMCP_SENTRY_DISABLED=true) will incorrectly see "Sentry enabled: Yes" in doctor output. Consider also checking doctorInfo.environmentVariables.XCODEBUILDMCP_SENTRY_DISABLED !== 'true'.

src/mcp/tools/simulator-management/set_sim_location.ts

• KBC-HWJ NaN coordinates bypass manual range validation and are forwarded to xcrun (L18) · low
  Because NaN < -90 and NaN > 90 both evaluate to false, NaN latitude or longitude values silently pass the bounds checks on lines 74 and 82 and are forwarded to xcrun as "NaN,NaN", producing a confusing tool error instead of the expected validation error. Consider using z.number().finite().min(-90).max(90) and z.number().finite().min(-180).max(180) in the schema to reject NaN/Infinity at parse time.

src/mcp/tools/simulator/build_run_sim.ts

• ZL8-QL4 Two redundant simctl list calls when resolving and booting a named simulator (L338-L344) · low
  When simulatorName is provided, determineSimulatorUuid (line 305) issues xcrun simctl list devices available -j to find the UUID, then findSimulatorById (line 338) immediately issues an identical xcrun simctl list devices available --json to read the device state. The device state is already available from the first list call; the second process fork is redundant.

src/smoke-tests/test-helpers.ts

• 5NE-KGF Keyword-based error detection produces false positives and false negatives (L13-L18) · low
  Scanning message text for substrings like 'error', 'fail', 'missing', etc. will match success messages that happen to contain those words (e.g. "no failures found", "error-free build") and miss error responses that use different phrasing, making test assertions unreliable.

src/snapshot-tests/suites/session-management-suite.ts

• 93L-BXM No error-path tests for session management operations (L48) · low
  The suite only covers success paths; have you considered adding tests for error cases such as using a non-existent profile, providing both global: true and profile simultaneously, or calling use-defaults-profile with an empty profile name?

src/utils/platform-detection.ts

• NPN-RN3 Missing test case for neither-path-provided error branch (L74-L82) · low
  The else branch at lines 74–82 (when both projectPath and workspacePath are undefined) has no corresponding test, leaving this error path unverified.

src/utils/renderers/event-formatting.ts

• RMV-6QW Module-level resolvedDiagnosticPathCache has no eviction policy (L180) · low
  resolvedDiagnosticPathCache is a module-scoped Map in event-formatting.ts with no clear(), size cap, or LRU eviction. In a long-running MCP server it accumulates one entry per unique (baseDir, bareFilename) pair across all sessions/projects. In practice, growth is bounded by the number of distinct bare source filenames per project (the cache is only consulted when filePath contains no path separator), so leakage is modest — but for a daemon serving many projects over long periods it still grows monotonically. Consider capping via a small LRU or clearing at build-session boundaries.

src/utils/xcode.ts

• 7J8-DDP Missing unit tests for new constructDestinationString helper (L6) · low
  constructDestinationString in src/utils/xcode.ts has multiple branches (simulator+ID, simulator+name with/without useLatest, simulator missing both → throws, macOS with/without arch, and generic destinations for iOS/watchOS/tvOS/visionOS) and is already consumed by three call sites (build-utils.ts, build_run_sim.ts, get_sim_app_path.ts). No src/utils/__tests__/xcode.test.ts exists, while sibling utilities under src/utils/__tests__/ follow the project's pattern of unit-testing helper logic. Adding focused unit tests for each branch (including the throw path and the fall-through log) would guard against regressions as more callers adopt it.

src/utils/xcodebuild-run-state.ts

• 7LQ-PSG Compiler diagnostic and test-failure dedup share a single Set (mixed key namespaces) (L152) · low
  seenDiagnostics in createXcodebuildRunState is used both for compiler-diagnostic keys (normalizeDiagnosticKey → ${location}|${message}) and test-failure keys (normalizeTestFailureKey → ${test}|${location}|${message} or ${suite}|${test}|${message}). Practically, compiler diagnostic locations are file paths (e.g. foo.swift:10:5) while test failure keys begin with lowercased test identifiers, so real-world collisions are extremely unlikely. However, mixing two distinct key namespaces in one Set is a minor code smell and a latent hazard if key formats evolve (e.g. a future change that omits location or reorders fields). Consider using separate seenCompilerDiagnosticKeys and seenTestFailureKeys Sets so the two dedup domains cannot interfere.

General

• A6K-URN No tests for new daemon CLI commands · medium
  The entire src/cli/commands/daemon.ts module has no corresponding test file — src/cli/commands/__tests__/ contains tests for init, setup, and upgrade but nothing for daemon.
• CXR-M9C No unit tests for lldb-cli-backend.ts · medium
  The backends/__tests__/ directory contains only dap-backend.test.ts; the new LldbCliBackend — with its buffer state machine, sentinel parsing, timeout handling, and command queue — has no test coverage at all.
• GAG-MX4 Missing unit tests for tools command logic (buildToolList, workflow filtering, --json output) · low
  The tools CLI command's core logic — buildToolList, isToolCanonicalInWorkflow, getCanonicalWorkflow, workflow filtering, and the --json/--flat output shapes — is not covered by unit tests. Only a smoke test in cli-surface.test.ts asserts that the word "build" appears in the default output. Consider extracting these helpers (or exporting them for tests) and adding cases that cover: canonical vs. non-canonical classification, --workflow filtering, and both grouped and flat --json outputs. This would reduce regression risk as workflows evolve.
• 2FD-6HB Missing dedicated tests for tool-catalog.ts resolution and dedup logic · low
  src/runtime/tool-catalog.ts introduces non-trivial behavior in createToolCatalog.resolve() (exact match, kebab alias, ambiguous alias, MCP-name fallback, notFound) plus seenMcpNames dedup and buildToolCatalogFromManifest filtering/module-cache/error-continue paths. None of these branches are covered by a dedicated test file. Consider adding a tool-catalog.test.ts covering: kebab-alias resolution, ambiguous alias handling, MCP-name lookup, duplicate MCP-name skip, and manifest build error recovery. This is a coverage suggestion rather than a blocker.
• B76-6KE New device-name-resolver module has no test coverage · low
  No src/utils/__tests__/device-name-resolver.test.ts exists, leaving cache logic, failure fallback, and formatDeviceId output entirely untested despite comprehensive test coverage for all other utils modules.

---

Generated by Warden