PR #391 #1134
codeql
on: dynamic
Matrix: analyze
Annotations
9 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.
|
|
Test discovered.total bumped from 52 to 57 without matching items list:
src/snapshot-tests/__fixtures__/json/device/test--failure.json#L34
The fixture's `discovered.total` increased from 52 to 57, but this change is unrelated to the stated workspace filesystem cleanup behavior change. Per the skill guardrails, fixture updates must map to intentional behavior changes and must not be updated solely to make tests pass. If the test count change is incidental (e.g., new tests added in the example project) it should be a separate, intentional update; otherwise it risks masking an unrelated regression in test discovery.
|
|
Stale lock recovery without owner file skips ownership validation, can delete a freshly-created lock:
src/utils/fs-lock-sync.ts#L87
In `tryRecoverExpiredLockDir`, when `shouldRecoverLockDir` returns `{recover: true, owner: null}` (the lock directory exists but has no owner.json and is older than `leaseMs`), the post-rename validation block at lines 87-97 is skipped because `recovery.owner` is falsy. Between the staleness check and `renameSync`, another process can legitimately write `owner.json` into the still-existing directory; the racing process then quarantines that directory and `rmSync`s it without comparing the owner, destroying a live lock held by another process. The TOCTOU window is small but real, and violates the multi-process cleanup boundaries this PR is trying to enforce.
|
|
[6A7-4LT] Stale lock recovery without owner file skips ownership validation, can delete a freshly-created lock (additional location):
src/daemon/socket-path.ts#L68
In `tryRecoverExpiredLockDir`, when `shouldRecoverLockDir` returns `{recover: true, owner: null}` (the lock directory exists but has no owner.json and is older than `leaseMs`), the post-rename validation block at lines 87-97 is skipped because `recovery.owner` is falsy. Between the staleness check and `renameSync`, another process can legitimately write `owner.json` into the still-existing directory; the racing process then quarantines that directory and `rmSync`s it without comparing the owner, destroying a live lock held by another process. The TOCTOU window is small but real, and violates the multi-process cleanup boundaries this PR is trying to enforce.
|
|
normalizeWorkspaceKey allows path traversal via '..' and null bytes:
src/utils/log-paths.ts#L39
normalizeWorkspaceKey only rejects '/' and '\\' separators but accepts values like '..', '.', or strings containing null bytes. Since the returned key is joined into a filesystem path under the workspaces directory, a workspace key of '..' would resolve to the parent (APP_DIR), and null bytes would either crash path operations or be interpreted unsafely by downstream consumers. If workspace keys can ever be derived from untrusted input (project paths, env vars, etc.), this enables directory traversal allowing one workspace's cleanup logic to delete files outside its intended root.
|
|
[6Y6-3WV] normalizeWorkspaceKey allows path traversal via '..' and null bytes (additional location):
src/daemon/socket-path.ts#L29
normalizeWorkspaceKey only rejects '/' and '\\' separators but accepts values like '..', '.', or strings containing null bytes. Since the returned key is joined into a filesystem path under the workspaces directory, a workspace key of '..' would resolve to the parent (APP_DIR), and null bytes would either crash path operations or be interpreted unsafely by downstream consumers. If workspace keys can ever be derived from untrusted input (project paths, env vars, etc.), this enables directory traversal allowing one workspace's cleanup logic to delete files outside its intended root.
|
|
scheduleWorkspaceFilesystemLifecycleSweep marks scope as running before timer fires, blocking subsequent schedules during the delay window:
src/utils/workspace-filesystem-lifecycle.ts#L405
`runningScheduledSweeps.add(scheduleKey)` is called synchronously before the `setTimeout` callback is queued. Any caller invoking `scheduleWorkspaceFilesystemLifecycleSweep` during the `WORKSPACE_FILESYSTEM_LIFECYCLE_SCHEDULE_DELAY_MS` delay (or while the sweep is running) will be silently dropped via the `runningScheduledSweeps.has(scheduleKey)` guard, even if `force: true` is set. The early force-bypass only short-circuits the cooldown checks but still falls through to the running-set guard, so forced sweeps requested while one is in flight are silently discarded.
|