Skip to content

[minor] MLAI-1230 - Enforce JFrog skill governance via an agent-guard hook - #66

Merged
shmuelqwak merged 16 commits into
mainfrom
MLAI-1230-skill-enforcement-hook
Aug 27, 2026
Merged

[minor] MLAI-1230 - Enforce JFrog skill governance via an agent-guard hook#66
shmuelqwak merged 16 commits into
mainfrom
MLAI-1230-skill-enforcement-hook

Conversation

@shmuelqwak

@shmuelqwak shmuelqwak commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

claude-plugin — Skill Governance enforcement hook

Branch MLAI-1230-skill-enforcement-hook, rebased on main. Diff vs main: 5 files, +446/−2.

file change
hooks/hooks.json +33 — the three governance hooks
scripts/validate-enforce-hook.mjs +392 — new, 27 checks
README.md +18/−1 — Windows prerequisite, fail-open behaviour
.github/workflows/validate.yml +2 — runs the validator in CI
.claude-plugin/plugin.json 0.2.22 → 0.3.0

The title must keep [minor] — the release workflow reads only the squash subject's first line,
and that marker is what ships the 0.3.0 bump (CONTRIBUTING.md#releasing).

What it does

Enforces JFrog skill governance by calling agent-guard from hooks.json directly. The plugin
carries zero governance logic
— no plugin script sits in the enforcement path. Three hooks:

SessionStart              async, 180s   npx … --version >/dev/null 2>&1 || true   (cache pre-warm)
PreToolUse (Skill|Read)           30s   npx … --enforce-skill --client claude-code
UserPromptExpansion               30s   npx … --enforce-skill --client claude-code

The two governed hooks run byte-identical commands, and the validator asserts they stay that way:

npm_config_fetch_retries=0 npm_config_fetch_timeout=10000 \
JF_AGENT_GUARD_ENFORCE_DEADLINE="$(($(date +%s) + 25))" \
npx --yes --prefer-offline \
  --registry "${JFROG_AGENT_GUARD_REPO:-https://releases.jfrog.io/artifactory/api/npm/coding-agents-npm/}" \
  @jfrog/agent-guard --enforce-skill --client claude-code

It runs against the released agent-guard

No local-run affordances. The hook resolves the production registry by default and never pins a
version. JFROG_AGENT_GUARD_REPO is a product knob for air-gapped and self-hosted mirrors, and
the validator asserts every governed hook keeps it. Local test harnesses work by overriding that
variable from outside; nothing local is baked into the plugin. The version-pin override that used
to exist is gone, and the validator asserts it stays gone — pinning a security-relevant component
contradicts JFrog's posture everywhere else.

.claude-plugin/marketplace.json is a local docker-harness artifact and is deliberately not
committed; main does not track it either.

Decisions worth knowing before reviewing

Infrastructure failure fails OPEN. The hook does not wrap the command in anything — no ||,
no exit-code remapping — so agent-guard's exit code reaches Claude Code untouched. Exit 2 is the
only code Claude Code treats as a blocking PreToolUse error; exit 1, exit 127, a spawn failure
and a hook killed at the client's own timeout are all non-blocking. So agent-guard's own deny
(exit 2) blocks, while a missing npx, an unreachable registry, or a package that will not install
all allow. A machine that cannot run the guard is not governed by it, and refusing every skill
there enforces nothing except the user's inability to work. The validator covers both halves:
agent-guard's exit 2 must propagate, and exit 1 / a missing npx must not.

Governance state is decided by agent-guard, not by this plugin. This PR changes none of it:

user state outcome
No JFrog config at all (no URL, no token) allow, silently
Credentials, not entitled to AI Catalog allow
Credentials, entitled, no project deny, with the actionable message
Credentials + project verdict

Row 1 verified against agent-guard at e5e6bf7 (shipped as v1.11.0): with an empty HOME and no
JFROG_URL / JFROG_ACCESS_TOKEN, a real non-exempt skill returns exit 0 with
could not resolve JFrog credentials, so this skill is NOT governed. The code path is
agent-guard/commands/enforce_skill.go:456-469"No credentials means no governance, not a
block."

Rows 2–4 are decided by the service, not locally: @Entitled sits on
BatchGetAiAssetGovernanceQuery.execute ahead of validateResource, so an empty project returns
403 "feature is not enabled" for an unentitled account and
400 resource.project.project_key must not be blank for an entitled one. Those three rows have not
been re-measured for this PR and are stated as agent-guard's documented behaviour.

The anti-bypass property comes from the service: an entitled account that unsets JF_PROJECT still
gets the 400 and is still blocked.

"shell": "bash" is pinned on all three hooks. Left to default, a Windows machine without Git
Bash runs under PowerShell, where the env-prefix assignment npm_config_fetch_retries=0 … is not
valid syntax. The hook then cannot run, and under the fail-open rule above the action is allowed
unchecked. README.md lists Git for Windows as a prerequisite. Hooks have no per-OS command field.
Not verified on a real Windows machine.

npm_config_fetch_retries=0 and npm_config_fetch_timeout=10000 bound the failure, not the
outcome.
npm's defaults (2 retries with 10s-then-60s backoff, a 300s timeout) made a refused
registry take 70s and a packet-dropping registry over 10 minutes. Under fail-open the verdict is an
allow either way, so these are a latency control rather than a security one: without them a
broken registry stalls every governed Skill and Read for the full 30s ceiling before allowing.
Measured with the bounds in place: 354ms for a refused registry, 10.2s for a packet-dropping one.
One retry pushes the dropped-packet case back to ~21s, so retries stay at 0.

The 30s timeout is a ceiling, not a delay. Real cost is ~318ms on a warm npx; cold start
measures 10.6s, which is why the ceiling is not lower. JF_AGENT_GUARD_ENFORCE_DEADLINE is set to
+25s so it falls inside that ceiling — agent-guard writes its verdict before the client can kill
the hook — and it is computed fresh on every invocation with no ${VAR:-…} fallback, because an
absolute instant is valid for one run only. The validator asserts both.

Known gaps

  • PreToolUse fires on every Read. ~318ms warm per call, with no throttle and no cache of a
    recent verdict. A read-heavy session pays it repeatedly. A verdict cache is a follow-up.
  • The SessionStart pre-warm is async, so a first Skill or Read can start before the cache
    holds @jfrog/agent-guard and pays the 10.6s cold start inside the 30s ceiling.
  • Windows is unverified, as above.

Validation

scripts/validate-enforce-hook.mjs (27 checks, all passing) executes the real command string out
of hooks.json
against a stub npx, rather than asserting on its shape. It covers: stdin
forwarded verbatim, verdict stdout forwarded, exit 0 for allow, fail-open for an agent-guard
failure, fail-open when npx is absent entirely, exit 2 propagated when agent-guard blocks, the
registry override honoured, the version override ignored, the deadline computed fresh and
inside the timeout, the npm bounds set to their intended values, a hook killed at the client's
ceiling not surfacing as a block, both governed hooks byte-identical, and --waiver-helper absent
from every command. Every behavioural check runs against both governed events rather than
PreToolUse alone.

node scripts/validate-claude-plugin.mjs and node scripts/validate-enforce-hook.mjs both pass on
this branch.

release.yml runs validate-claude-plugin.mjs only, so neither this validator nor the existing MCP
rewrite tests gate a release. That gap predates this PR and is being closed separately.

Release ordering

Resolved. --request-waiver shipped in agent-guard v1.11.0 (tag agent-guard/v1.11.0,
2026-08-25), which contains PR #205 (e5e6bf7); commands/request_waiver.go is absent in v1.10.0
and present in v1.11.0. The registry agrees: dist-tags.latest = 1.11.0, published
2026-08-25T14:24:15Z. The hook resolves latest, so the waiver flow it renders is runnable today
and there is no ordering constraint left on this merge.

Review remarks addressed

remark outcome
Version pin is counterintuitive for a security component Removed; validator asserts it stays removed
Timeouts look huge Partly a misreading — timeout is a ceiling. Real cost ~318ms/hook; 30s kept because cold start is 10.6s
Fail-closed blocks users who don't use the feature Fixed — the hook no longer forces a block on infrastructure failure, and agent-guard allows an unconfigured or unentitled user
Why is a plugin script passed to agent-guard? Removed — scripts/governance/ deleted entirely
Don't replicate credential resolution Gone with the script. The two plugin copies had already drifted, and the Node resolver was weaker than agent-guard's: needed jf on PATH, ignored --server, and named JF_ACCESS_TOKEN where agent-guard's own message says JFROG_ACCESS_TOKEN
request-waiver.mjs is over-commented Moot — file deleted
Deadline could be inherited from the environment Fixed — computed fresh, no ${VAR:-…} fallback; validator asserts the negative
Validator header was far too long Trimmed from 40 lines to 14
Move "no JFrog server configured" out of the README's allow list Declined — the README is correct; enforce_skill.go:456-469 allows, verified against the v1.11.0 binary. Reasoning in thread
Add a CI smoke test against the published agent-guard Declined — it would put registry reachability in front of every PR and, without credentials on the runner, assert only that the binary starts. Reasoning in thread

@shmuelqwak
shmuelqwak requested a review from a team as a code owner August 24, 2026 06:54
@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@shmuelqwak
shmuelqwak marked this pull request as draft August 24, 2026 06:55
Comment thread .claude-plugin/plugin.json
shmuelqwak and others added 11 commits August 24, 2026 09:59
Adds a PreToolUse (Skill|Read) and UserPromptExpansion hook that evaluates
the skill Claude is about to use against the account's JFrog skill
governance policies and blocks the ones that violate them, showing the
violated policies and the exact command to request a waiver.

The plugin carries NO governance logic. scripts/enforce-skill.mjs is a
zero-dependency transport shim: it pipes the raw hook event to
`agent-guard --enforce-skill --client claude-code` and forwards its stdout
back byte-for-byte. Empty stdout means allow. All decision-making —
resolving the skill folder, fingerprinting, calling JFrog, and rendering
the block card — lives in agent-guard (MLAI-1230 there).

The shim owns the fail-closed contract: a missing binary, a child that
exits non-zero, or any exception exits 2, the only exit code Claude Code
treats as blocking. hooks.json uses exec form rather than a shell
one-liner because shell form runs under PowerShell on Windows when Git
Bash is absent, where a parse error exits 1 — non-blocking, i.e. it failed
open. `node <script>` needs no shell on any platform.

REQUIRES an agent-guard release that supports --enforce-skill. An older
binary does not know the flag, falls through to the MCP loader and exits 1,
which this shim correctly turns into a block — blocking every skill until
it is upgraded. The failure message names that possibility.

scripts/validate-enforce-hook.mjs checks the wiring hermetically (exec
form, matcher coverage, stdin/stdout pass-through, and both fail-closed
paths) and runs in the validate workflow. request-waiver.mjs is the
user-facing waiver helper the block card points at.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ode wrapper

The PreToolUse/UserPromptExpansion hooks invoked a plugin-side wrapper
(scripts/enforce-skill.mjs) in exec form. Replace it with a direct npx call, so no
plugin code sits in the enforcement path at all, and delete the wrapper.

npx ONLY: the earlier `command -v agent-guard` fast path is gone. An agent-guard
earlier on PATH could be anything, whereas npx always resolves the package from the
pinned registry, so dropping that branch removes a hijack vector. It also removes the
`command -v` that forced POSIX-only syntax, which is half of why a wrapper was needed.

`|| exit 2` is kept and is load-bearing. Claude Code blocks a PreToolUse hook on exit
2 ONLY — 1, 127 and spawn failures are all non-blocking, and a timeout is too. So when
npx itself is missing the shell returns 127 and agent-guard never runs; no exit code
inside agent-guard could have blocked that. Only `|| exit 2` in the hook converts it
into a block. That is also why agent-guard needs no change here.

"shell": "bash" is set explicitly. Left to default, a Windows machine without Git Bash
runs the command under PowerShell, where `||` is a parse error exiting 1 — non-blocking,
i.e. governance silently fails OPEN. That was the reason c013e53 introduced the wrapper.
Pinning bash instead makes that machine fail LOUDLY: Claude Code raises a visible
"requires bash … Install Git for Windows" error on every invocation. It still fails open
there, so README now lists Git Bash as a Windows prerequisite. Hooks have no per-OS
command field, and a second `shell: "powershell"` entry would run on every platform,
erroring visibly on macOS/Linux where no PowerShell exists.

JFROG_AGENT_GUARD_VERSION is new alongside the existing JFROG_AGENT_GUARD_REPO: npx
resolves `latest`, which in a dev repo is the newest GA, so testing a dev build needs a
version pin. Both are redirect knobs for mirrored/air-gapped installs.

A SessionStart pre-warm fetches the package with --version (no credentials, no side
effects) and is async:true, so a cold 33 MB download never delays session start. Without
it that download can exceed the hook's 20s timeout on first use — and a hook timeout is
non-blocking, so it would silently allow.

validate-enforce-hook.mjs now executes the real command string out of hooks.json against
a stub npx instead of asserting the wrapper's shape: stdin forwarded verbatim, verdict
stdout forwarded, exit 0 for allow, exit 2 for an agent-guard failure, and exit 2 when
npx is absent entirely. Verified each assertion fails when its guard is removed, and
verified the production string against the real published agent-guard in the bare-machine
container (npx absent, unknown version, and a GA without --enforce-skill all yield 2).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
request-waiver.mjs still POSTed the legacy flat scope — {application_key, stage_key,
stage_gate} and no action field at all. Unified Policy's v012 migration ("Phase II waivers:
expand the waiver scope & action model") backfilled that shape into a new one and archived the
legacy jfup_waiver_scope table. WaiverCreatePayload now requires a mandatory `action` plus a
`scopes` array discriminated on `type`, with exactly one `organization` entry.

The stage/gate moved out of the scope and into the action, where `stage` is optional — and
omitting it waives ALL stages and gates for the action type, far broader than the single block
the user is responding to. So it is always sent.

justification is capped at 255 characters server-side. The text is the user's own reason relayed
by a model that may pad it, so it is truncated here rather than returned as an opaque HTTP 400.

cursor-plugin's copy gets the identical change and is kept byte-identical below the imports:
agent-guard renders one shared directive that invokes both, so they must not drift.

Not yet exercised against a live Unified Policy — the target environment is unavailable, so
this follows the service's own schema and migration rather than an observed 2xx.
…ccepts it

Two bugs, either of which alone made every waiver request fail.

The payload named certify_to_gate with a stage and an application sub_scope. A skill is
governed by use_skill, which is stage-less by contract - Unified Policy's action schema
rejects any key beyond `type` - and its waiver is scoped by the waiver-only `skill`
sub_scope: project_keys for where, plus skills[] naming the package by the (name, version,
repo_path) triple UP matches on. An application-scoped waiver does not match a use_skill
evaluation at all.

The endpoint was /ui/api/v1/unifiedpolicy/api/v1/waivers - the UI reverse-proxy route,
which authenticates a browser session and answers a bearer-token client with a bare 403
"Forbidden". Measured against a live JPD, and the reason this helper had never once
created a waiver.

Verified end to end on a live JPD: the request now returns a pending waiver, and once
approved the evaluation drops from 9 policies to the 2 configured
waiver_request_config=forbidden, which UP deliberately keeps immune.
New capability rather than a fix, so a minor bump. The version lives only in this
manifest (CONTRIBUTING.md#releasing) and the release workflow refuses to re-tag a
shipped version, so the bump is reviewed here rather than pushed to main by a bot.

The [minor] marker belongs in the squash subject, not in this body — the workflow
reads the first line only, on purpose.
…nsumers

The banner claimed the resolver was shared by "the SessionStart injector and the
skills-governance hooks". Neither holds in this tree: there is no injector here,
and the hooks reach it only indirectly - request-waiver.mjs is the single
importer, spawned by agent-guard through --waiver-helper.

Also records why environment variables are tried before the CLI, since the order
reads like an oversight otherwise: README documents the JFrog CLI as optional for
credential resolution, so an env-only setup with no configured server still has
to be able to file a waiver.

Comment-only; no behaviour change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…in override

The hook commands expanded ${JFROG_AGENT_GUARD_VERSION} into the npx package
spec, so any environment could choose which build of agent-guard governed the
machine. That inverts the trust relationship a governance control depends on:
the host being enforced against picks its own enforcer, and can hold itself on
a release that predates the policy it would otherwise be blocked by. A pin is
also silent — nothing in the UI says the guard is stale.

It is inconsistent as well. Every other JFrog client invokes agent-guard
unpinned and picks up whatever is published; only this plugin offered an
escape hatch.

The registry override stays. ${JFROG_AGENT_GUARD_REPO} answers a real need
(air-gapped and self-hosted mirrors) without letting the caller choose the
version served from it, and the SessionStart pre-warm keeps refreshing the
cache from that registry once per session so "latest" stays current.

The two validator checks that asserted the pin behaviour now assert its
absence: JFROG_AGENT_GUARD_VERSION is still set in the test env, and the spec
handed to npx must stay plain "@jfrog/agent-guard". A second check sweeps every
hook in hooks.json for a reintroduced pin, and asserts the registry override
survives, so the two cannot be conflated in a later edit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… hot path

PreToolUse matches Skill|Read, and Read is one of the most frequent tool calls
in a session. Every one of them was paying a full npx resolution: measured at
1.15s and 1.27s warm, almost all of it a registry round trip to re-answer a
question the session had already answered. Over a working session that is
minutes of dead time bought nothing.

Both governed hooks now pass --prefer-offline, so npx serves the package from
the local cache and only reaches the network on a cache miss. Measured warm:
1.15-1.27s before, 0.30-0.31s after. That the round trip is genuinely gone and
not merely faster was confirmed by re-running with npm_config_offline=true
against the real registry, which still succeeded in 0.28s -- the cache alone
satisfies the resolution.

The async SessionStart pre-warm deliberately does NOT get --prefer-offline. It
is the one online resolution per session and the only thing that pulls a newly
published agent-guard into the cache. It is what keeps "latest" actually latest
now that the version-pin override is gone, so the two changes hold each other
up: without the online pre-warm, --prefer-offline would freeze every user on
whichever release they first downloaded.

hooks.json is strict JSON and cannot carry a comment, so the pre-warm's
statusMessage now states the division of labour, and the validator asserts both
halves of it: the governed hooks must pass --prefer-offline, the pre-warm must
not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tead of timing out

The product decision is that a user who cannot run agent-guard must be blocked.
`|| exit 2` delivers that only for failures that EXIT. It does nothing for
failures that HANG, and Claude Code treats a hook killed at its timeout as
non-blocking, so a hang is a silent allow. Governance was therefore failing
closed on fast infrastructure failures and failing open on slow ones, purely
according to which side of the 20s hook timeout npm happened to land on:

  npx missing              exit 127, immediate  -> exit 2, BLOCKED   (correct)
  registry refused         70s                  -> hook timeout, ALLOWED
  registry drops packets   >10 minutes          -> hook timeout, ALLOWED

Both slow cases are npm's own defaults, not network physics: fetch-retries is 2
with a 10s-then-60s backoff (exactly the 70s), and fetch-timeout is 300s, 15x
the hook's entire budget. Hook timeout semantics cannot be changed from
hooks.json, so the fix is to make the command lose patience first. The two
governed hooks now prefix npm_config_fetch_retries=0 and
npm_config_fetch_timeout=10000 inline, which needs no external binary --
notably not coreutils `timeout`, which macOS does not ship.

Measured with the exact command string from hooks.json:

  npx missing              16ms      exit 2
  DNS failure              274ms     exit 2
  registry refused         354ms     exit 2   (was 70s, allowed)
  registry drops packets   10,234ms  exit 2   (was >10min, allowed)

Every infrastructure failure now reaches `|| exit 2` with at least 9s of the
hook's 20s still unspent.

Retries are 0 rather than 1 on purpose: a single retry doubles the
dropped-packet case to ~21s, back over the hook timeout, which would trade the
guarantee away for the resilience. Resilience belongs in the SessionStart
pre-warm, which keeps npm's defaults, is async, has 180s and ends in `|| true`,
so it can retry through a flaky network without blocking anyone. The validator
asserts that split, and asserts the worst-case fetch stays within half the hook
timeout so the two numbers cannot drift into each other.

A legitimate cold start is unaffected: with a completely empty npm cache the
exact hook command resolved, downloaded and ran agent-guard in 10.6s, so no
individual request came near the 10s per-request bound.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… copy

agent-guard never spawned request-waiver.mjs. It interpolated the path into a
`node ...` command line, wrote that into the model-only channel and exited; the
model ran it later as a separate process. So the "helper" was never a callee,
only a string agent-guard had to be told - and the plugin had to ship a waiver
client and a JFrog credential resolver to make that string work.

agent-guard now owns the POST behind its own --request-waiver command and names
itself in the directive, so the flag, the script and the resolver all go:

  scripts/governance/request-waiver.mjs      176 lines
  scripts/governance/helpers/credentials.mjs  58 lines

That removes the third credential resolver in the codebase - the reviewer's
objection - and with it a real defect: the claude-plugin and cursor-plugin
copies were required to stay byte-identical and had ALREADY drifted, one
carrying a debug parameter the other lacked. The Node resolver was also the
weaker of the two, needing `jf` on PATH, ignoring --server, and telling users to
set JF_ACCESS_TOKEN where agent-guard's own message names JFROG_ACCESS_TOKEN.

The plugin now carries zero governance logic, which was the stated design when
d92781d moved the rest of it out and left this one exception behind.

validate-enforce-hook.mjs asserts the absence rather than dropping the checks:
the flag must not reappear in either governed hook, and must not reach
agent-guard's argv. A stale --waiver-helper would name a file that is gone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… Read

The hook blocks when agent-guard cannot be run at all, and nothing said so. The
blast radius is wider than a reader would assume from "skill governance": the
PreToolUse matcher is `Skill|Read`, so an unreachable registry stops every file
read, not just skill invocations.

Records the two things that keep this from being punitive - the async
SessionStart pre-warm means a machine that reached the registry once keeps
working from cache, and an unentitled user is allowed rather than blocked, so
opting out costs no setup - and promotes Node/npx from a soft prerequisite to a
hard one, since its absence is now a block rather than a degraded feature.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@shmuelqwak

Copy link
Copy Markdown
Collaborator Author

recheck

@shmuelqwak
shmuelqwak force-pushed the MLAI-1230-skill-enforcement-hook branch from 17a1a75 to c2a8f76 Compare August 24, 2026 07:00
Comment thread hooks/hooks.json Outdated
Comment thread hooks/hooks.json Outdated
Comment thread hooks/hooks.json Outdated
Comment thread hooks/hooks.json Outdated
…rrectly

The 20s hook timeout, the 10s npm bound and agent-guard's 12s default budget are
additive, not overlapping: the client's timer starts when it spawns the command,
npm runs to completion first, and only then does agent-guard start its own clock.
10 + 12 = 22s against a 20s hook, and overrunning a hook is not a block - the
client kills it and treats that as ALLOWED. A slow-but-working registry in front
of a slow JPD was enough to silently disable enforcement.

The hook now supplies JF_AGENT_GUARD_ENFORCE_TIMEOUT and raises its own timeout to
30s, so one number drives both repos: 10s npm + 15s budget + 5s render = 30s.
Written as ${VAR:-15s} rather than a bare assignment, which would beat the
inherited environment and silently disable agent-guard's documented operator
override.

Drift can no longer reopen the hole in any direction: an older agent-guard that
does not know the variable uses its own 12s (22s, fits), and a future one that
raises its default is overridden by the hook (25s, fits).

The validator now reads the budget out of the shipped command string and asserts
npm + budget + render <= hook timeout, replacing a 'half the hook budget stays
free' rule that was a guess at agent-guard's share rather than a measurement of it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread scripts/validate-enforce-hook.mjs
Requirement change: npx failing, the package not installing, or no JFrog server being
configured must ALLOW. A machine that cannot run the guard is not governed by it, and
blocking there stops work without enforcing anything.

|| exit 2 is gone. It converted every failure into a block, which is exactly the
behaviour being inverted - and it also overrode agent-guard on the paths where
agent-guard had already decided. agent-guard's exit code is now the verdict: its own
exit 2 still blocks (budget expiry, or a block it could not deliver), everything else
falls through non-blocking. The validator gains a check for that second half, because
passing the code through untouched is the part that must never regress.

The budget is now supplied as JF_AGENT_GUARD_ENFORCE_DEADLINE, an absolute instant, not
a duration. A duration is stale by the time agent-guard reads it - the client's timer
starts at spawn and npx runs first - so a slow fetch left agent-guard planning to finish
after the hook had already been killed, losing a verdict that may have been a real block.
From a deadline its budget shrinks instead.

The fetch-bound check no longer asserts npm's cost fits the hook timeout. That sum was
unsound: npm_config_fetch_timeout is per REQUEST, and a cold npx makes several requests
plus a 17MB tarball, so the real worst case was never the single 10s the arithmetic
assumed. The bounds stay - they stop a dead registry hanging - but they no longer claim
to guarantee something they cannot.

README no longer promises fail-closed, and drops the claim that a Windows machine without
Git Bash reports a 'requires bash' error: that text was never observed and the PowerShell
path exits 1, which is non-blocking.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@shmuelqwak
shmuelqwak force-pushed the MLAI-1230-skill-enforcement-hook branch from 8aa59b9 to ea16338 Compare August 25, 2026 11:43
Comment thread hooks/hooks.json Outdated
shmuelqwak and others added 2 commits August 25, 2026 16:45
…le fallback

${JF_AGENT_GUARD_ENFORCE_DEADLINE:-...} let an inherited value win. An absolute instant
is valid for one invocation only, so a variable exported anywhere - a shell profile, a
parent process, a CI runner - pinned every governed skill to a deadline in the past.
Measured against a reachable JPD: 'leaves only -1h0m0.663s' then a deny on every skill.

Computed fresh now. The operator knob is the DURATION form, which agent-guard takes as one
of three lower bounds; a deadline is not something an operator can usefully pin. The
validator asserts the ${...} form cannot come back.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
main added the plugin MCP-rewrite feature (#59), which lands two more SessionStart hooks
and a FileChanged event. All three conflicts were additive - both sides wanted a new row,
step or hook - so both are kept.

One real fix fell out of it: validate-enforce-hook.mjs found the agent-guard pre-warm at
SessionStart[0].hooks[1], and main's two new hooks moved it to [3], so the check was
asserting against whichever hook happened to sit at that index. It now finds the pre-warm
by content.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md
Comment thread hooks/hooks.json
Comment thread scripts/validate-enforce-hook.mjs Outdated
Comment thread scripts/validate-enforce-hook.mjs Outdated
Comment thread scripts/validate-enforce-hook.mjs
Comment thread scripts/validate-enforce-hook.mjs Outdated
Comment thread scripts/validate-enforce-hook.mjs Outdated

@YoniMelki YoniMelki left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hook wiring and the split between the plugin and agent-guard are sound. The validator runs the real command string, which catches drift that a shape assertion would miss. Four items block a merge.

  1. README.md lines 16 to 27 do not match agent-guard. An unentitled user gets an allow. A user with no JFrog configuration gets a denial. The text joins the two states and calls both an allow.
  2. release.yml runs validate-claude-plugin.mjs only. CONTRIBUTING.md says that validate.yml cannot gate a release, so the new hook wiring can ship broken.
  3. The stale comments in scripts/validate-enforce-hook.mjs are still present. Two paragraphs give opposite rules for the deadline, one cites a 20-second hook, and one names a carve-out that does not exist.
  4. The PR description still describes || exit 2, fail-closed behavior, and a 20-second ceiling. The branch fails open with a 30-second ceiling.

Details are in the inline comments.

…alidator accuracy

README: split the IMPORTANT block into the three outcomes it actually has — a policy
denial, a guard that ran but could not answer in time, and a guard that could not run at
all — so a reader can tell an agent-guard exit 2 from a failed spawn. Narrow the feature
row from "every skill" to the skills Claude invokes.

validate-enforce-hook.mjs:

- Assert npm_config_fetch_timeout's VALUE, not just its presence. 300000 is npm's own
  default, so the old assertion would have let an edit back to the default pass.
- Run every behavioural check against both governed events. The byte-identical check
  already made divergence loud, but only while it ran first.
- Emulate a hook killed at the client's ceiling and assert it cannot surface as exit 2.
  spawnSync's ETIMEDOUT is that outcome, so it is no longer treated as a spawn failure.
- Delete the deadline paragraph that argued for ${VAR:-default}; it contradicted the
  check directly below it and told a future editor to undo the fix.
- Drop the "cold-start carve-out" parenthetical — no such carve-out exists in either
  repo; a past deadline still floors at minEnforcementBudget and denies.
- Correct the 20s hook reference to 30s.
- Say why the npm + budget + render check from 432a6a9 is gone: fetch_timeout is per
  request, so that sum was never a worst case. The absolute deadline subsumes it.
- Record the Windows coverage gap where BASH is defined, and note that the registry is
  fixed while the package spec deliberately is not.

27 checks, all passing. validate-claude-plugin.mjs passes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@shmuelqwak
shmuelqwak marked this pull request as ready for review August 27, 2026 09:25
@shmuelqwak

Copy link
Copy Markdown
Collaborator Author

I have read the CLA Document and I hereby sign the CLA

@shmuelqwak
shmuelqwak merged commit 1748d79 into main Aug 27, 2026
2 of 3 checks passed
@shmuelqwak
shmuelqwak deleted the MLAI-1230-skill-enforcement-hook branch August 27, 2026 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants