Repository navigation
plannotator: allowlist the launchctl test-name match in 0.28.51 (pkgscan false positive) - #1384
Conversation
pkgscan flagged packages/server/inbox-launch-env.test.ts:151 Critical (launchctl). The line is a test name; v0.28.51 never runs launchctl. Pinned to the file sha256 at the v0.28.51 tag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reached
This review includes 1 billable file and costs up to $0.25.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Or wait 22 minutes for your next included review. View limit details
|
norrietaylor
left a comment
There was a problem hiding this comment.
Scope matches the title. The diff adds one [[allow]] entry to packages/plannotator/pkgscan-allow.toml and changes nothing else. build.ncl stays at 0.28.50, as the PR body says it will. Clobber none. git log on packages/plannotator shows only version bumps and the earlier allowlist PR #1265 since the file was created. No entry was removed or changed. Correctness The entry has the same shape as its siblings: file, category, pattern, sha256, reason with an upstream commit link. category = "FileSystemAccess" for pattern = "launchctl" matches the two existing launchctl entries in packages/atuin/pkgscan-allow.toml. The entry sits in the (b) False positives: tests section, which fits a test file. Not checked The sha256 against the v0.28.51 tag, and the claim that launchctl occurs only at inbox-launch-env.test.ts:151 and inbox-launch-env.ts:12. Both need a network fetch of the upstream tree, and this sandbox denied that fetch. The security owner must confirm the hash before merge. The change is otherwise good.
| category = "FileSystemAccess" | ||
| pattern = "launchctl" | ||
| sha256 = "5a6bc693670d6c43ec299171ca848ac4f32d8142b280fb22befd1d2f09202568" | ||
| reason = "False positive: line 151 is a bun test NAME (\"what launchd and the session give (SSH_AUTH_SOCK, a launchctl proxy, LC_*) reaches the shell and the --bg env\"); the test only asserts that existing env vars pass through createLoginEnv/launchSpawnEnv with a fake runner. Nothing in v0.28.51 executes launchctl (the only other occurrence, packages/server/inbox-launch-env.ts:12, is a JSDoc comment). Test files are not bundled (build.sh bundles apps/hook/server/index.ts). Added in https://github.com/backnotprop/plannotator/commit/37f0e88cd6184b9745564c6febae3f98b7ef898a (#2376)" |
There was a problem hiding this comment.
nit · The file header (lines 6-9) says the entries were triaged for the 0.28.44 update and that 'all 11 files' are byte-identical across v0.28.44-v0.28.46. This entry is the twelfth and is pinned to v0.28.51. A reader who trusts the header will check this hash against the wrong tags. Add one line to the header that names the 0.28.50 -> 0.28.51 triage and this file.
agent-137
left a comment
There was a problem hiding this comment.
Review on Karl's behalf. This is a comment, not an approval. Nothing was built for it. The findings come from reading the declarations and fetching upstream sources, and each one says how it was checked. The CI rollup on this head is now SUCCESS.
CI: 15 checks pass. CodeRabbit is NEUTRAL because it hit its rate limit and did not review, so there is no bot review. No human reviews.
No findings. What I checked:
-
Scope: the entry is as narrow as the format allows: one file, one category, one pattern, and the sha256 of the whole file. The format has no version field. The file hash is the version scope, and any change to the file makes the finding gate again (header lines 2-4).
-
sha256:
5a6bc693…2568matchesraw.githubusercontent.com/backnotprop/plannotator/v0.28.51/packages/server/inbox-launch-env.test.ts, fetched fresh. -
The test name: line 151 is the bun
test("what launchd and the session give (SSH_AUTH_SOCK, a launchctl proxy, LC_*) …")name. The test body uses a fakerunand only asserts that environment variables pass throughlaunchSpawnEnv. It executes nothing. -
Whole-tree grep:
grep -rn launchctlover the v0.28.51 tag tarball finds only three hits:- the test name above;
- a JSDoc comment at
packages/server/inbox-launch-env.ts:12; - a marketing docs
.mdfile.
The PR's claim holds.
-
Category:
FileSystemAccess+launchctlis the same pair as the two existing atuin entries on main, so it is the category pkgscan uses for this pattern. -
Format: the field order and the
reasonstyle (with an upstream commit link) match the file's existing entries. -
Placement: the entry is appended after the "(b) False positives: tests" block, which is where it belongs.
-
Build: the plannotator
build.shon main compiles only the hook server entry, so test files do not ship.
Nit
packages/plannotator/pkgscan-allow.toml:6-9: the header still describes only the 0.27.16 → 0.28.44 triage. Add one line for the 0.28.50 → 0.28.51 triage (the date and the gating finding), so the file's provenance stays complete.
Cross-PR note: if #1113 merges as written, with @bryan-minimal as the sole owner, this PR (authored by bryan-minimal) can no longer get code-owner approval. Merge order matters.
Verdict #1384: correct and minimally scoped. I verified the sha256 and the false-positive claim against the v0.28.51 tag. Approvable once a human security owner signs off.
The updater refused plannotator 0.28.50 → 0.28.51 on a Critical pkgscan finding. This adds an allowlist entry for it, so the next update run can open the version-bump PR.
The finding is a false positive.
packages/server/inbox-launch-env.test.ts:151, is a bun test name:"what launchd and the session give (SSH_AUTH_SOCK, a launchctl proxy, LC_*) reaches the shell and the --bg env".createLoginEnvand asserts thatSSH_AUTH_SOCK,HTTPS_PROXYandLC_ALLsurvivelaunchSpawnEnv.launchctloccurs nowhere else in the v0.28.51 tree except a JSDoc comment atpackages/server/inbox-launch-env.ts:12(the Info finding). Nothing runs it.apps/hook/server/index.ts.5a6bc693…. That matches the hash the gate reported and a fresh clone ofv0.28.51(dd204b51, released by github-actions 2026-11-06T12:49Z).What 0.28.51 does add (for the reviewer, not a reason to block). The release is large (428 files, +45.7k lines). Most of it is a new Inbox feature that starts Claude Code sessions from the Inbox (
packages/server/inbox-launch*.ts, upstream #2376). It:$SHELL -ilc 'env -0'(5 s timeout);claude, using--permission-mode bypassPermissionsonly when the user has recorded bypass consent.It is reachable from the shipped hook server through
inbox.ts, but it is off by default:inboxLaunch.enabledis config-only and every launch route returns 404 unless it is set. That is a real capability of the shipped binary, not a supply-chain payload. The allowlist entry covers only the launchctl test-name match.Only
packages/plannotator/pkgscan-allow.tomlchanges. It needs a security-owner review (pkgs#1113 proposes the CODEOWNERS rule for this file).🤖 Generated with Claude Code