Harden the pkgxray-guard install-gate (git-URL, fail-mode, cache, edit-diff)#18
Open
adamsjack711-ux wants to merge 7 commits into
Open
Conversation
Guards AI-agent package installs with pkgxray before they run. Registers OnBeforeExecution (parse npm/pnpm/yarn/bun installs + npx/bunx/pnpm-dlx runners + `claude mcp add -- <launcher>`, run `pkgxray guard` per package, deny on BLOCK with cited evidence) and an opt-in OnAfterFileEdit lockfile audit. Policy layer (strict/balanced/permissive) fails closed on a broken pkgxray under the default balanced policy. Folded into the root module (no separate go.mod) so `go build/test ./...` covers it, matching the multi-hook example. Core lives in a stdlib-only pkgxrayguard package with offline table + fake-pkgxray exec tests. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t-diff) Follow-up hardening for the pkgxray × hookshot install guard. Keeps main a thin shim; all logic stays in the stdlib-only pkgxrayguard package. - Parser: git+https/git@/tarball-URL install specs were silently dropped (an `npm i git+https://…` sailed through). They now classify as an unvettable KindVCS spec that Guard short-circuits to REVIEW — surfaced, not allowed. Local file:/link:/workspace: paths are still skipped. - Fail-mode is shape-aware: npx/bunx/pnpm-dlx/bun x run code the instant they resolve. DecideResult escalates an immediate-exec spec whose verdict is UNKNOWN (pkgxray errored) or REVIEW to ASK even under `permissive`, so it never fails open; persistent installs still follow the policy table. DecideAll folds per-package results into the strongest action. - Deny/ask messages now include the finding's file path ([category] reason (file)). - MemoGuard caches verdicts per exact ref@version for the process lifetime (~1.3–1.5s cold each); never caches UNKNOWN/errored results or across versions. PKGXRAY_CACHE_URL is forwarded to the CLI on cmd.Env. - OnAfterFileEdit diffs the edit hunks: package.json deep-guards only the newly added/changed deps via ManifestAddedSpecs (reusing the memo); lockfiles keep the full `pkgxray audit`, which honors the sibling .pkgxray.lock memory. Falls back to a full audit when no added dep is extracted, so it's never less safe. Builds/vets/tests green in-tree (`go build/vet/test ./examples/pkgxray-guard/...`); 66 offline pkgxrayguard subtests.
Two integration-completeness follow-ups: - Document the pkgxray CLI contract the hook depends on (top-level `decision`, exit codes 2/3/0, `report.findings[].file`) and the minimum tested version (pkgxray >= 0.15.0), on the guardJSON type and in a README "Requirements" section. pkgxray has no `--version` flag, so there's no runtime probe; the hook instead degrades safely (missing/old/erroring pkgxray → UNKNOWN → denied under strict/balanced, never a false allow). - Add ready-to-edit host configs for the two agents main.go already advertised but shipped no config for: Factory Droid (~/.factory/settings.json) and Windsurf Cascade (~/.codeium/windsurf/hooks.json). Values match hookshot's own installer (droid-pre-tool-use / droid-after-file-edit; cascade-pre-run-command / cascade-post-write-code). README layout now lists all five configs and the cache.go/manifest.go core files.
The unit tests drive the pkgxrayguard package against a fake pkgxray. e2e-smoke.sh exercises the whole chain instead: it builds the hook binary and feeds it genuine Claude PreToolUse payloads on stdin, so the compiled binary and the real pkgxray CLI are actually integrated. Deterministic cases (git-URL install → deny/ask, non-install → allow, local path → skipped) run offline and are hard assertions. One live case runs `npm install left-pad` through the real pkgxray CLI and asserts a valid decision comes back (verdict not pinned — it depends on the registry). Skips the live case if pkgxray isn't on PATH. README "Try it" + layout updated.
Mirror of the pkgxray-side change: a command installing several packages guarded them sequentially (~N × per-package pkgxray latency; a 20-pkg install ≈ 10s). - pkgxrayguard.CheckAll: stdlib-only bounded fan-out (each Check writes its own result slot; MemoGuard mutex-guards its cache → concurrency-safe, go test -race clean). Used in decideInstalls + decideManifestSpecs. Cap via PKGXRAY_HOOK_CONCURRENCY (default 8). Results fold order-independently, so verdicts are unchanged. - load-test.py: throughput/latency harness (offline + fake-pkgxray at high concurrency; real pkgxray at low volume). Measured one 20-pkg command at 1607ms serial → 216ms (default 8) → 143ms (16). Measured with go 1.26; builds/tests green in-tree.
The two exec sites — guard.go (`pkgxray guard <ref>`) and helpers.go
(`pkgxray audit <file>`) — tripped gosec G204 ("subprocess launched with a
variable"). Both are safe: exec.Command passes args as a []string argv (no
shell), so a package ref / path can't inject a command (CWE-78 is unreachable),
and `bin` is the operator-configured PKGXRAY_BIN, not attacker-controlled.
Close the findings with genuine defense-in-depth rather than bare suppression:
- guard.go: validateInstallRef() rejects an install ref that is empty, begins
with '-' (argument injection into the pkgxray CLI), or carries control bytes —
a sanitizer checkpoint on the untrusted-input → subprocess path. Registry refs
the parser produces (name, @scope/name, name@version) always pass; a flag-like
ref is surfaced as Review instead of being shelled out.
- Both exec sites carry a `#nosec G204` annotation documenting why the variable
target is safe (no shell, operator-set bin, validated argv).
Verified: `gosec -include=G204 ./...` now reports 0 issues (2 nosec). Tests
cover validateInstallRef (valid refs pass, flag-like/control/empty refs error)
and that Check refuses a flag-like ref before exec. `go build`/`go vet`/
`go test ./...` all green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author
|
On the helpers.go:25 finding: On the guard.go:97 finding: |
adamsjack711-ux
added a commit
to adamsjack711-ux/hookshot
that referenced
this pull request
Jul 4, 2026
…idorSecurity#18) Brings the recheck branch up to date with the pkgxray source-of-truth copy after the recheck + parallel-guard PRs merged there: adds pkgxrayguard/batch.go + batch_test.go and the main.go dispatch changes. Byte-identical per sync-to-hookshot.sh --check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up hardening for the
examples/pkgxray-guardinstall guard.main.gostays a thin shim; all logic lives in the stdlib-onlypkgxrayguardpackage, and everything builds/tests in-tree with the root module (go build/vet/test ./examples/pkgxray-guard/..., 66 offline subtests).Changes
git+https:///git@/ tarball-URL specs were silently skipped by the parser, sonpm i git+https://…bypassed the gate. They now classify as an unvettableKindVCSspec thatGuardshort-circuits to REVIEW (pkgxray can't triage an arbitrary remote spec). Localfile:/link:/workspace:paths are still skipped.npx/bunx/pnpm dlx/bun xrun package code the instant it resolves, with no persistent install to inspect.DecideResultescalates an immediate-exec spec whose verdict isUNKNOWN(pkgxray errored) orREVIEWto ask even underpermissive— so it never fails open. Persistent installs still follow the policy table.DecideAllfolds per-package results into the strongest action.[category] rationale (path).MemoGuardcaches verdicts per exactref@versionfor the process lifetime (each guard call is ~1.3–1.5s cold). Never cachesUNKNOWN/errored results or across versions.PKGXRAY_CACHE_URLis forwarded to the CLI oncmd.Env.OnAfterFileEditdiffs the edit hunks:package.jsondeep-guards only newly added/changed deps (ManifestAddedSpecs, reusing the memo) — a formatting/script edit triages nothing; lockfiles keep the fullpkgxray audit, which honors the sibling.pkgxray.lockallow/block memory. Falls back to a full audit when no added dep is extracted, so it's never less safe.Notes
examples/pkgxray-guard/; no new dependencies (stdlib only).gofmt/go vetclean; the repo'sgo.ymlexercises this on push.🤖 Generated with Claude Code