[minor] MLAI-1288 - Enforce Cursor skill governance through agent-guard - #81
[minor] MLAI-1288 - Enforce Cursor skill governance through agent-guard#81shmuelqwak wants to merge 4 commits into
Conversation
Replaces the in-plugin Node governance with a direct npx call to agent-guard, and brings the Cursor wiring in line with the Claude Code plugin. Infrastructure failure now fails OPEN. `|| exit 2` is gone and `failClosed` is false: npx missing, a failed install, an unreachable registry or no configured server let the skill through. 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. A real policy denial still blocks — on Cursor the verdict travels as JSON with exit 0, so the payload carries it regardless of failClosed. The waiver helper is deleted. agent-guard v1.11.0 files waivers itself via --request-waiver, and --waiver-helper is no longer in its flag table, so the plugin holds no credentials and no waiver logic. Nothing may pin the version: the hook resolves `latest` so a shipped GA fix reaches users without a plugin release. The call also bounds its fetch, prefers the cache, and passes an absolute deadline computed fresh at invocation — with no inheritable fallback, since an absolute instant inherited from an earlier process pins every later invocation to the past. No sessionStart pre-warm: Cursor's hook schema has no async field, so one would stall session start. Cold start therefore allows; tracked as a follow-up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…sor-governance-via-agent-guard # Conflicts: # .cursor-plugin/marketplace.json # README.md # plugins/jfrog/.cursor-plugin/plugin.json
--prefer-offline on the governed hooks is only safe if something refreshes the npx cache. Claude Code's plugin does that with an async SessionStart pre-warm; dropping it here left a machine able to serve a stale binary indefinitely. Measured: a cached 1.10.0 kept being used while 1.11.0 was latest, which silently reinstated a bug 1.11.0 had fixed — 1.10.0 decides "no project" locally and BLOCKS, where 1.11.0 refers it to the service and fails open on unresolvable credentials. The stale cache read as a governance verdict. Cursor's hook schema has no async field, so the command detaches itself: a subshell background plus an explicit exit 0, which returns in 0s and never fails session start. It deliberately omits --prefer-offline, since hitting the registry is its only purpose. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An absent date(1) made $(($(date +%s) + 25)) expand to 25 — an epoch in 1970 — which floors the enforcement budget at 500ms and blocks every skill, reported as a governance timeout. Passing empty instead is safe: agent-guard ignores an empty deadline and uses its own default budget. failClosed: false does not cover this. The block arrives as a deny in the JSON payload, which agent-guard produced deliberately; failClosed only governs a hook that fails on its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
YoniMelki
left a comment
There was a problem hiding this comment.
The inline comments list the required changes.
| **Requirements & behavior** | ||
|
|
||
| > [!IMPORTANT] | ||
| > **On Cursor, only an answer can block.** A verdict reaches Cursor as JSON on the hook's stdout; |
There was a problem hiding this comment.
This text says that only JSON on stdout can block and that the hook always exits 0. Cursor treats exit code 2 as a deny. failClosed: false does not change that. scripts/validate-skill-governance.mjs already asserts that path. An Agent Guard internal deadline that exits 2 will block. A Cursor hook kill after the timeout will allow. Split those two cases in this note.
| name: Validate skill governance | ||
|
|
||
| on: | ||
| pull_request: |
There was a problem hiding this comment.
This workflow runs only on a pull request. .github/workflows/release.yml does not run scripts/validate-skill-governance.mjs. CONTRIBUTING.md states that pull-request checks do not see the merge commit. Add the same command to the release job.
| > there. A user entitled to nothing is unaffected either way: the Agent Guard answers "allow" for an | ||
| > unconfigured or unentitled user, so no setup is needed to opt out of the feature. | ||
| > | ||
| > A user who is entitled to nothing is unaffected either way: the Agent Guard returns "allow" for |
There was a problem hiding this comment.
Remove this paragraph. The sentence above it already states the same rule.
| "preToolUse": [ | ||
| { | ||
| "command": "_JFAG_NOW=$(date +%s 2>/dev/null); npm_config_fetch_retries=0 npm_config_fetch_timeout=10000 JF_AGENT_GUARD_ENFORCE_DEADLINE=\"${_JFAG_NOW:+$((_JFAG_NOW + 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 cursor", | ||
| "matcher": "Read", |
There was a problem hiding this comment.
preToolUse matches only Read. Other tools can put a skill body in front of the model. The Skills governance section already states that gap. Align the capabilities table with that limit.
| assert(blocks(r), "a deny payload must read as a block"); | ||
| }); | ||
|
|
||
| await check(`${step}: an allow is forwarded as-is (Cursor requires explicit JSON, not silence)`, async () => { |
There was a problem hiding this comment.
These checks use a stub npx. They do not run a real @jfrog/agent-guard binary. They also omit JSON that is not valid and empty stdout with exit 0.
| branches: [main] | ||
| paths: | ||
| # Governance lives entirely in agent-guard now, so the wiring IS the hooks file. The | ||
| # scripts/** glob stays so that re-adding any plugin-side governance code re-triggers the |
There was a problem hiding this comment.
This path list watches plugins/jfrog/scripts/**. It does not watch plugins/jfrog/modules/**. The no-governance check uses a fixed list under scripts/governance/. Include plugins/jfrog/modules/** in the path list.
| "timeout": 60 | ||
| }, | ||
| { | ||
| "command": "( npx --yes --registry \"${JFROG_AGENT_GUARD_REPO:-https://releases.jfrog.io/artifactory/api/npm/coding-agents-npm/}\" @jfrog/agent-guard --version >/dev/null 2>&1 & ) ; exit 0", |
There was a problem hiding this comment.
This command returns at once with exit 0. If Cursor stops the hook process group, the background npx does not refresh the cache. The validator only checks the command shape. Confirm that the detached process stays alive after the hook returns.
Moves Cursor skill governance out of in-plugin Node and into
agent-guard, and brings the Cursor wiring in line with the Claude Code plugin — where the behaviour genuinely should match, and deliberately not where Cursor differs.What the hook does now
Byte-identical on both governed surfaces —
beforeSubmitPrompt(unmatched, so every submission) andpreToolUse(matcherRead, since Cursor has noSkilltool).|| exit 2--waiver-helper${JFROG_AGENT_GUARD_VERSION:+@…}latestretries=0,timeout=10000--prefer-offline, refreshed by a sessionStart pre-warmfailClosedtruefalseInfrastructure failure now fails OPEN
|| exit 2is gone andfailClosedisfalse.npxmissing, a failed install, an unreachable registry, or no configured JFrog server all let the skill through. A machine that cannot get a verdict is not governed by it, and refusing every skill there enforces nothing except the user's inability to work.A real policy denial still blocks: on Cursor the verdict travels as JSON on stdout with exit 0, so the payload carries it regardless of
failClosed.failClosedwas only ever a crash net, never how a block is signalled.This is where Cursor deliberately diverges from Claude Code. The Claude plugin blocks when the guard reaches the check but cannot finish it in time; here that case allows, because a killed or failed hook is not a block under
failClosed: false. The README states the divergence rather than papering over it.Verified against a genuinely unconfigured machine (0
jfservers, nosetup.json, no env):Both are the correct Cursor allow shapes — explicit JSON, never silence.
The waiver helper is deleted
agent-guardv1.11.0 files waivers itself via--request-waiver, and--waiver-helperis no longer in its flag table.plugins/jfrog/scripts/governance/is removed entirely, so the plugin holds no credentials and no waiver logic.Two earlier commits on this branch had rewritten
request-waiver.mjsto the Unified Policy Phase II payload. That work is obsolete under the new split, so the branch was recommitted without it.Nothing may pin the version
All three invocations name
@jfrog/agent-guardbare. Proven empirically, not just asserted — withJFROG_AGENT_GUARD_VERSION=9.9.9exported, argv still carries bare@jfrog/agent-guard. A shipped GA fix therefore reaches users without a plugin release.Minimum required: agent-guard ≥ 1.11.0. This matters more than it looks. On 1.10.0 the same event blocks:
{"continue":false}— "no project is set"{"continue":true}— allowed1.10.0 decided "no project" locally; 1.11.0 refers it to the service and fails open on unresolvable credentials.
Why sessionStart pre-warms the cache
--prefer-offlineon the governed hooks is only safe if something refreshes the npx cache. Without it, a machine can serve a staleagent-guardindefinitely — this was hit in real testing: a cached 1.10.0 kept being used while 1.11.0 waslatest, silently reinstating the bug above. It read as a governance verdict, not as a stale binary.Cursor's hook schema has no
asyncfield, so the pre-warm detaches itself:Measured: returns in 0s, and the detached fetch lands the latest version in the cache. It deliberately omits
--prefer-offline, since hitting the registry is its only purpose, and the explicitexit 0keeps a warm failure from ever failing session start.Validation
scripts/validate-skill-governance.mjswas rewritten — its old assertions demanded the exact things this PR removes (must end in "|| exit 2",must pass the waiver helper,failClosed === true). 34 checks, all passing, asserting the inverted contract and executing the real command string fromhooks.jsonagainst a stubnpx.Behavioural checks run against both surfaces with surface-shaped payloads, rather than testing
preToolUseand inheriting the result. Notable checks:npm_config_fetch_timeoutis asserted by value (<= 10000), so an edit back to npm's 300000 default fails rather than passing a presence check.JF_AGENT_GUARD_ENFORCE_DEADLINEdoes not survive.--prefer-offline, must not pin a version, must detach, and must end inexit 0.Not verified
shellfield, so the command runs in the platform's default shell. It is POSIX (${VAR:-default},$(( ))), so oncmd/PowerShell it cannot run and governed actions are allowed unchecked. Stated in the README; unchanged from before this PR, which was equally POSIX-only.npxand the guard on every prompt submission and everyRead, with no throttle and no verdict cache.--prefer-offlineis what makes that affordable, but the warm cost has not been measured.failClosed: false, the first governed action on a cold machine can be allowed. The detached pre-warm narrows the window but does not close it.Release
[minor]—0.5.18→0.6.0, bumped in bothplugins/jfrog/.cursor-plugin/plugin.jsonand.cursor-plugin/marketplace.jsonasvalidate-version.ymlrequires.