PR #391 #1138
cameroncookecodeql
on: dynamic
Matrix: analyze
Annotations
12 warnings
|
Analyze (actions)
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 (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.
|
|
Discovered test total bumped from 52 to 57 without new items in fixture:
src/snapshot-tests/__fixtures__/json/device/test--failure.json#L34
The `discovered.total` field changes from 52 to 57, but the visible portion of the `items` array in this hunk and surrounding context does not show five new test entries to justify the increase. If the underlying test list did not actually grow by five, this fixture update may be a churn-only change unrelated to the workspace filesystem cleanup, which violates the guardrail that fixture updates must map to intentional behavior changes and must not be updated solely to make tests pass. Reviewers should confirm the discovered list was regenerated from real tooling output and matches MCP/CLI fixtures.
|
|
[8D2-BKD] Discovered test total bumped from 52 to 57 without new items in fixture (additional location):
src/snapshot-tests/__fixtures__/json/simulator/test--failure.json#L33
The `discovered.total` field changes from 52 to 57, but the visible portion of the `items` array in this hunk and surrounding context does not show five new test entries to justify the increase. If the underlying test list did not actually grow by five, this fixture update may be a churn-only change unrelated to the workspace filesystem cleanup, which violates the guardrail that fixture updates must map to intentional behavior changes and must not be updated solely to make tests pass. Reviewers should confirm the discovered list was regenerated from real tooling output and matches MCP/CLI fixtures.
|
|
New testCases entries omit the required 'suite' field, breaking the JSON envelope contract:
src/snapshot-tests/__fixtures__/json/simulator/test--failure.json#L68
The newly-added testCases items (lines 68-239) only include test/status/durationMs and drop the 'suite' field that every other entry in this fixture (and in adjacent fixtures) carries. This breaks the stable structured-output envelope that the skill requires JSON fixtures to preserve, and suggests volatile data is being patched ad hoc into the fixture rather than normalized in code. Downstream consumers relying on a uniform testCases shape will see inconsistent records.
|
|
TOCTOU race between lstatSync and statSync in validateSocketDir:
src/daemon/socket-path.ts#L68
validateSocketDir calls lstatSync to reject symlinks, then issues a separate statSync on the same path. Because the daemon run directory now lives under tmpdir() (a shared, often world-writable location), a local attacker could swap the directory for a symlink between the two syscalls, bypassing the symlink check while statSync silently follows the link. The subsequent ownership/mode checks then validate the link target rather than the actual socket parent, which can let another user influence where the daemon socket is created.
|
|
[6BZ-9EX] TOCTOU race between lstatSync and statSync in validateSocketDir (additional location):
src/daemon/socket-path.ts#L89
validateSocketDir calls lstatSync to reject symlinks, then issues a separate statSync on the same path. Because the daemon run directory now lives under tmpdir() (a shared, often world-writable location), a local attacker could swap the directory for a symlink between the two syscalls, bypassing the symlink check while statSync silently follows the link. The subsequent ownership/mode checks then validate the link target rather than the actual socket parent, which can let another user influence where the daemon socket is created.
|
|
TOCTOU between lstatSync symlink check and statSync ownership check in validateSocketDir:
src/daemon/socket-path.ts#L68
validateSocketDir performs lstatSync to reject symlinks, then a separate statSync to verify ownership and mode. Because the daemon socket directory lives under tmpdir() (typically world-writable /tmp) at a path derived from a 12-hex workspace hash that an attacker can predict, a local attacker can race between the two syscalls — replacing the directory with a symlink to an attacker-controlled location after lstatSync succeeds but before statSync runs. The follow-up chmodSync(dir, 0o700) and subsequent socket creation may then operate on the attacker-controlled target, enabling local privilege/permission tampering on the daemon's IPC socket.
|
|
[MCX-RGM] TOCTOU between lstatSync symlink check and statSync ownership check in validateSocketDir (additional location):
src/daemon/socket-path.ts#L89
validateSocketDir performs lstatSync to reject symlinks, then a separate statSync to verify ownership and mode. Because the daemon socket directory lives under tmpdir() (typically world-writable /tmp) at a path derived from a 12-hex workspace hash that an attacker can predict, a local attacker can race between the two syscalls — replacing the directory with a symlink to an attacker-controlled location after lstatSync succeeds but before statSync runs. The follow-up chmodSync(dir, 0o700) and subsequent socket creation may then operate on the attacker-controlled target, enabling local privilege/permission tampering on the daemon's IPC socket.
|
|
[MCX-RGM] TOCTOU between lstatSync symlink check and statSync ownership check in validateSocketDir (additional location):
src/daemon/socket-path.ts#L97
validateSocketDir performs lstatSync to reject symlinks, then a separate statSync to verify ownership and mode. Because the daemon socket directory lives under tmpdir() (typically world-writable /tmp) at a path derived from a 12-hex workspace hash that an attacker can predict, a local attacker can race between the two syscalls — replacing the directory with a symlink to an attacker-controlled location after lstatSync succeeds but before statSync runs. The follow-up chmodSync(dir, 0o700) and subsequent socket creation may then operate on the attacker-controlled target, enabling local privilege/permission tampering on the daemon's IPC socket.
|
|
Scheduled sweep cooldown bypassed when sweep is skipped by lock or cooldown:
src/utils/workspace-filesystem-lifecycle.ts#L418
In scheduleWorkspaceFilesystemLifecycleSweep, lastScheduledAtByScope/lastScheduledAtByPreKey are only updated when the sweep actually ran (skippedByCooldown=false and skippedByLock=false). When another process holds the lock, every subsequent schedule call within the cooldown window will pass the cooldown check (because lastAt is never set), re-enter the runningScheduledSweeps gate, set a new timer, and call runWorkspaceFilesystemLifecycleSweep again. Combined with the timer being scheduled before the lock attempt, this can produce repeated lock-acquisition storms across processes for the same workspace, defeating the cooldown the PR is trying to enforce.
|