Code Quality: PR #391 #1114
cursor[bot]codeql
on: dynamic
Matrix: analyze
Annotations
2 errors and 8 warnings
|
Daemon socket directory moved to world-writable tmpdir without ownership/permission verification:
src/daemon/socket-path.ts#L29
`daemonDirForWorkspaceKey` now resolves under `tmpdir()` (e.g. `/tmp/xcodebuildmcp-<hash>/`) instead of the previous `~/.xcodebuildmcp/daemons/` path inside the user's home. The workspace hash is deterministic and predictable per-user/per-project, and on multi-user systems `/tmp` is world-writable with the sticky bit. An attacker with a local account can pre-create `/tmp/xcodebuildmcp-<hash>/` (or place a symlink at `d.sock`/`daemon.json`) before the legitimate user starts the daemon. `ensureSocketDir` only applies `mode 0o700` when creating a non-existent directory, so a pre-existing attacker-owned directory is used as-is, enabling socket hijacking, IPC eavesdropping, or registry tampering.
|
|
[Q3D-ZHH] Daemon socket directory moved to world-writable tmpdir without ownership/permission verification (additional location):
src/daemon.ts#L412
`daemonDirForWorkspaceKey` now resolves under `tmpdir()` (e.g. `/tmp/xcodebuildmcp-<hash>/`) instead of the previous `~/.xcodebuildmcp/daemons/` path inside the user's home. The workspace hash is deterministic and predictable per-user/per-project, and on multi-user systems `/tmp` is world-writable with the sticky bit. An attacker with a local account can pre-create `/tmp/xcodebuildmcp-<hash>/` (or place a symlink at `d.sock`/`daemon.json`) before the legitimate user starts the daemon. `ensureSocketDir` only applies `mode 0o700` when creating a non-existent directory, so a pre-existing attacker-owned directory is used as-is, enabling socket hijacking, IPC eavesdropping, or registry tampering.
|
|
Analyze (python)
Starting April 2026, the CodeQL Action will skip computing file coverage information on pull requests to improve analysis performance. File coverage information will still be computed on non-PR analyses.
To opt out of this change, create a custom repository property with the name `github-codeql-file-coverage-on-prs` and the type "True/false", then set this property to `true` in the repository's settings.
|
|
Analyze (javascript-typescript)
Starting April 2026, the CodeQL Action will skip computing file coverage information on pull requests to improve analysis performance. File coverage information will still be computed on non-PR analyses.
To opt out of this change, create a custom repository property with the name `github-codeql-file-coverage-on-prs` and the type "True/false", then set this property to `true` in the repository's settings.
|
|
Test count and failure list changes appear unrelated to workspace cleanup:
src/snapshot-tests/__fixtures__/mcp/simulator/test--failure.txt#L9
This hunk updates the workspace-scoped Derived Data path (expected for this PR) but also changes the discovered test count from 52 to 57 and adds two new test failures (Swift Testing-style entries). These changes don't trace to the stated 'Workspace filesystem cleanup' behavior change and may indicate the fixture was updated to match drifted test output rather than an intentional behavior change. Per skill guardrails, fixture updates must map to intentional behavior changes and not just be updated to make tests pass.
|
|
isPidAlive(0) and negative pids signal process groups, falsely reporting 'alive':
src/utils/process-liveness.ts#L1
process.kill(pid, 0) with pid=0 sends the null signal to every process in the caller's process group, and a negative pid targets the corresponding process group. Both succeed when the caller itself is alive, so isPidAlive returns true for these inputs. Most callers (daemon-registry.ts:219, fs-lock.ts:93, fs-lock-sync.ts:59, simulator-launch-oslog-sessions.ts:85) pass entry.pid / staleOwner.pid without validating it is a positive integer, so a corrupted or zero-valued pid in a registry/owner file would be treated as a live owner forever, preventing recovery of stale locks and registry entries — a DoS-style resource leak in the very multi-process cleanup boundaries this PR introduces.
|
|
[KRL-3PT] isPidAlive(0) and negative pids signal process groups, falsely reporting 'alive' (additional location):
src/utils/fs-lock.ts#L78
process.kill(pid, 0) with pid=0 sends the null signal to every process in the caller's process group, and a negative pid targets the corresponding process group. Both succeed when the caller itself is alive, so isPidAlive returns true for these inputs. Most callers (daemon-registry.ts:219, fs-lock.ts:93, fs-lock-sync.ts:59, simulator-launch-oslog-sessions.ts:85) pass entry.pid / staleOwner.pid without validating it is a positive integer, so a corrupted or zero-valued pid in a registry/owner file would be treated as a live owner forever, preventing recovery of stale locks and registry entries — a DoS-style resource leak in the very multi-process cleanup boundaries this PR introduces.
|
|
[KRL-3PT] isPidAlive(0) and negative pids signal process groups, falsely reporting 'alive' (additional location):
src/utils/fs-lock.ts#L205
process.kill(pid, 0) with pid=0 sends the null signal to every process in the caller's process group, and a negative pid targets the corresponding process group. Both succeed when the caller itself is alive, so isPidAlive returns true for these inputs. Most callers (daemon-registry.ts:219, fs-lock.ts:93, fs-lock-sync.ts:59, simulator-launch-oslog-sessions.ts:85) pass entry.pid / staleOwner.pid without validating it is a positive integer, so a corrupted or zero-valued pid in a registry/owner file would be treated as a live owner forever, preventing recovery of stale locks and registry entries — a DoS-style resource leak in the very multi-process cleanup boundaries this PR introduces.
|
|
[KRL-3PT] isPidAlive(0) and negative pids signal process groups, falsely reporting 'alive' (additional location):
src/utils/fs-lock.ts#L184
process.kill(pid, 0) with pid=0 sends the null signal to every process in the caller's process group, and a negative pid targets the corresponding process group. Both succeed when the caller itself is alive, so isPidAlive returns true for these inputs. Most callers (daemon-registry.ts:219, fs-lock.ts:93, fs-lock-sync.ts:59, simulator-launch-oslog-sessions.ts:85) pass entry.pid / staleOwner.pid without validating it is a positive integer, so a corrupted or zero-valued pid in a registry/owner file would be treated as a live owner forever, preventing recovery of stale locks and registry entries — a DoS-style resource leak in the very multi-process cleanup boundaries this PR introduces.
|
|
Race window between cooldown check and runningScheduledSweeps set allows duplicate scheduling:
src/utils/workspace-filesystem-lifecycle.ts#L437
scheduleWorkspaceFilesystemLifecycleSweep updates lastScheduledAtByScope and runningScheduledSweeps synchronously, but the actual sweep runs after a setTimeout delay. If the timer-fired sweep takes longer than the cooldown, subsequent calls between the timer firing and runWorkspaceFilesystemLifecycleSweep completing may still see runningScheduledSweeps as set (good), but once the .finally() removes the key, a new schedule call can immediately enter again because lastScheduledAtByScope was set at schedule time, not completion time. Under heavy artifact-created bursts this causes the cooldown gate to leak. Consequence: more frequent sweeps than intended, but bounded by the file lock so not catastrophic.
|