Code Quality: PR #391 #1108
cameroncookecodeql
on: dynamic
Matrix: analyze
Annotations
5 warnings
|
Build Logs path not updated to workspace-scoped layout:
src/snapshot-tests/__fixtures__/cli/device/build--error-wrong-scheme.txt#L8
The Derived Data path was updated to the new workspace-keyed location (`workspaces/XcodeBuildMCP-<HASH>/DerivedData/...`), but the Build Logs path on line 15 still points to the old top-level `logs/` directory. Per the PR description, XcodeBuildMCP-owned logs are now moved into workspace-keyed paths under shared application support. If the rendered output now writes logs under a workspace-scoped path, this fixture would be inconsistent with actual output and CLI/MCP/JSON fixtures may diverge.
|
|
[5AX-QC4] Build Logs path not updated to workspace-scoped layout (additional location):
src/snapshot-tests/__fixtures__/mcp/simulator/test--failure.txt#L36
The Derived Data path was updated to the new workspace-keyed location (`workspaces/XcodeBuildMCP-<HASH>/DerivedData/...`), but the Build Logs path on line 15 still points to the old top-level `logs/` directory. Per the PR description, XcodeBuildMCP-owned logs are now moved into workspace-keyed paths under shared application support. If the rendered output now writes logs under a workspace-scoped path, this fixture would be inconsistent with actual output and CLI/MCP/JSON fixtures may diverge.
|
|
New 'Running tests' progress lines may be volatile and should be normalized:
src/snapshot-tests/__fixtures__/cli/swift-package/test--failure.txt#L8
The fixture now includes seven sequential 'Running tests (N completed, M failure, ...)' progress lines that were previously absent. These progress counters depend on test execution order and timing, which is typically volatile across runs. Per the skill's guardrails, volatile values should be normalized in code rather than baked into fixtures; otherwise this snapshot will become flaky on re-runs. Confirm whether these progress lines are deterministic for this fixture's input or whether normalization in fixture-io / contracts is needed.
|
|
Daemon socket directory created in world-writable /tmp without permission verification:
src/daemon/socket-path.ts#L21
The daemon directory is now placed under `tmpdir()` (typically `/tmp`, world-writable with sticky bit) using a predictable path `xcodebuildmcp-<12-hex-chars>`. Combined with `ensureSocketDir`'s `existsSync`-then-`mkdirSync` pattern (in this file at lines 77-82), an attacker on a multi-user system could pre-create the directory or a symlink at the predictable path before the daemon starts, bypassing the intended `mode: 0o700` and exposing the Unix domain socket to other local users. The previous location under `~/.xcodebuildmcp/daemons/<key>` was inside the user's home directory and not subject to this race.
|
|
isPidAlive treats EPERM as alive but ignores invalid PIDs (0, negative):
src/utils/process-liveness.ts#L1
process.kill(0, 0) on POSIX sends signal 0 to every process in the caller's process group, and negative PIDs target a process group rather than a single process. Because isPidAlive does not validate the input, callers passing a stale/zero/negative PID (e.g. from a corrupted registry file) can inadvertently probe or signal unrelated processes, and the function will report 'alive' based on group membership rather than the intended PID. In a cleanup path that uses liveness to decide whether to delete another workspace's artifacts, this can both prevent legitimate cleanup and, if the signal value were ever changed, target unintended processes.
|