Repository navigation
plannotator: pkgscan-allow the 0.28.x CredentialAccess findings (one group is an accepted real capability) - #1265
Conversation
The 0.27.16 -> 0.28.44 bot update aborted on 2026-11-02 with 29 gating CredentialAccess findings across 11 files. Each file gets one hash-pinned entry. Two entries are an ACCEPTED REAL CAPABILITY, not false positives: package-changes-service.ts and package-metadata.ts make the shipped binary read ~/.npmrc (plus the global and project npmrc) at runtime, by default, when a reviewed diff has package changes. It parses only `registry=` lines and uses them only to decide which names to withhold from the opt-in registry.npmjs.org lookup. Bryan accepted this as intended behaviour of a code-review tool. The other nine are false positives: JSDoc comments, security tests that write decoy credentials into a temp HOME, and the VS Code extension, which build.sh does not compile. Hashes match v0.28.44, v0.28.45 and v0.28.46 (the files are identical), and a production pkgscan diff to 0.28.46 gives the same findings. 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 2 minutes for your next included review. View limit details
|
There was a problem hiding this comment.
I checked all 11 pinned sha256 values against the v0.28.44 and v0.28.46 tag archives, and every one matches. I read each flagged line in context. The nine false positives are JSDoc comments, the VS Code extension (build.sh never compiles it), or tests that write decoys into a temp HOME and assert that nothing is served. I also checked that no shipped file imports them. For the two accepted-capability entries, I confirmed in package-changes-service.ts and package-metadata.ts that the ~/.npmrc read happens only when PLANNOTATOR_PACKAGE_LOOKUP is not 0, that readNpmrc() keeps only registry= lines, and that the result only decides which names are withheld from the opt-in npmjs.org lookup. Approving accepts that runtime read as intended behaviour.
Adds
packages/plannotator/pkgscan-allow.tomlso that the plannotator 0.27.16 -> 0.28.x bot update can ship. Today's run (2026-11-02T17:13Z,update_outcome=failed, reason "pkgscan Critical — needs --allow-critical-pkgscan to publish") aborted on 29 gatingCredentialAccessfindings across 11 files. This PR adds one hash-pinned entry per file.Important
Not every entry is a false positive. The two entries for
package-changes-service.tsandpackage-metadata.tsallowlist a real, accepted capability. The shipped binary reads~/.npmrcat runtime. Bryan, who maintains this recipe, accepted it as intended behaviour: "the tool is meant to do those things, so people understand". The other nine entries are false positives.The accepted capability
build.shcompilesapps/hook/server/index.ts, which imports@plannotator/server/review, which uses@plannotator/shared/package-changes-service. That code is in the plannotator binary.~/.npmrc(or$npm_config_userconfig), npm's globalnpmrcand the reviewed repo's.npmrc, plus the Yarn and Bun equivalents.PLANNOTATOR_PACKAGE_LOOKUP=0.readNpmrc()keeps onlyregistry=and@scope:registry=lines, so an_authTokenline never matches. The result is used only to decide which package names to withhold from the npm lookup. That lookup is opt-in (config.jsonpackageChanges.lookup, default false) and only ever contactsre 8000 gistry.npmjs.organdapi.npmjs.org; a test override is accepted for loopback addresses only. Nothing read from the files leaves the machine.Findings
These are from the production image (
pkgmgr:0b705d3-sc2e851aa) runningpkgscan diff plannotator --old 0.27.16 --new 0.28.44in Cloud Build. Every file was read in context from the upstream tag.packages/shared/package-changes-service.ts:138.npmrcpackages/shared/package-changes-service.ts:160.npmrcjoin(home, ".npmrc")packages/shared/package-changes-service.ts:16.npmrcpackages/shared/package-metadata.ts:257.npmrcregistry=lines only)packages/shared/doc-resolve.ts:172.ssh/id_apps/vscode-extension/src/guides/safe-path.ts:4,:36.aws/credentialspackages/server/doc-any-path.scenarios.ts:12,:186,:190,:243.ssh/id_SECRET-SSH-KEYinto a tempHOME(process.env.HOME = <makeTempDir>/home) and asserts the server refuses to serve it. Line 12 is JSDoc. Added in 96e4a36packages/server/reference-readers.scenarios.ts:149,:153.ssh/id_HOME. Added in a92e3ebpackages/server/inbox-attachments-kinds.test.ts:324.docker/config.jsonpackages/core/inbox-actions.test.ts:147(line 133 is already Info).ssh/id_ssh-copy-id …instruction text in a markdown-parser test. Added in 94a4709packages/shared/package-metadata.test.ts:7.npmrc_authToken=x; no file is read. Added in c0010c8packages/server/package-changes-endpoint.test.ts:12,:14,:91,:98,:99,:186.npmrc.npmrcfiles written intomkdtempSyncrepos; lines 12 and 14 are comments. Added in c0010c8packages/shared/package-changes-service.test.ts:111,:113,:134,:143,:280,:286,:315.npmrchome: tempDir()). Added in c0010c8 and e92b102build.shrunsbun install, the two UI builds andbun build --compileon the hook server. It runs no tests, and no test or scenario file is imported by the compiled entrypoint.Versions and hashes
shasum -a 256. A second productionpkgscan diff 0.27.16 -> 0.28.46gives exactly the same 30 findings and hashes.After merge
The plannotator update must be re-triggered by hand. The package has been parked in a
failedaction state since 10-31, so the nightly run won't pick it up by itself. Use the Slack upgrade button, or runpkgmgr update --package=plannotator --create-pr. The allowlist is read when the gate runs, so that run re-scans and regrades these findings. If any listed file changes in a later release, its hash stops matching and the finding gates again.The CODEOWNERS change for
pkgscan-allow.toml(#1113) is still open, and its self-approval deadlock applies to this PR too. The agent did not merge or approve this PR.🤖 Generated with Claude Code