From 8e79b6ec37e3b146fbbb14e5da7fe5648d6f1efc Mon Sep 17 00:00:00 2001 From: shmuelb Date: Thu, 27 Aug 2026 14:48:27 +0300 Subject: [PATCH 1/5] MLAI-1288 - Enforce Cursor skill governance through agent-guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .cursor-plugin/marketplace.json | 2 +- .../workflows/validate-skill-governance.yml | 37 ++ README.md | 56 ++- plugins/jfrog/.cursor-plugin/plugin.json | 2 +- plugins/jfrog/hooks/hooks.json | 15 + scripts/validate-skill-governance.mjs | 333 ++++++++++++++++++ 6 files changed, 442 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/validate-skill-governance.yml create mode 100644 scripts/validate-skill-governance.mjs diff --git a/.cursor-plugin/marketplace.json b/.cursor-plugin/marketplace.json index 8e1d4f1..942c752 100644 --- a/.cursor-plugin/marketplace.json +++ b/.cursor-plugin/marketplace.json @@ -6,7 +6,7 @@ }, "metadata": { "description": "JFrog Platform plugins for Cursor", - "version": "0.5.13", + "version": "0.6.0", "pluginRoot": "plugins" }, "plugins": [ diff --git a/.github/workflows/validate-skill-governance.yml b/.github/workflows/validate-skill-governance.yml new file mode 100644 index 0000000..baee516 --- /dev/null +++ b/.github/workflows/validate-skill-governance.yml @@ -0,0 +1,37 @@ +# Copyright (c) JFrog Ltd. 2026 +# Licensed under the Apache License, Version 2.0 +# https://www.apache.org/licenses/LICENSE-2.0 + +name: Validate skill governance + +on: + pull_request: + 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 + # validator that asserts it must not exist. + - "plugins/jfrog/hooks/hooks.json" + - "plugins/jfrog/scripts/**" + - "plugins/jfrog/.cursor-plugin/plugin.json" + - "scripts/validate-skill-governance.mjs" + workflow_dispatch: + +permissions: + contents: read + +jobs: + validate: + name: Validate skill governance + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + node-version: "20" + + - name: Run skill-governance validation + run: node scripts/validate-skill-governance.mjs diff --git a/README.md b/README.md index 4e7fe8b..575239d 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,7 @@ JFrog plugin for [Cursor](https://cursor.com): artifact management, security sca ## What's new +- **Skills governance.** A new hook checks every skill invocation against your JFrog governance policy and blocks disallowed or unscanned skills before they run. See [Skills governance](#skills-governance). - **Agent Package Resolution (Preview).** A new opt-in hook automatically routes the packages your AI agent installs through your JFrog Artifactory instead of public registries. See [Agent Package Resolution](#agent-package-resolution-preview). - **AI Catalog skill.** New `jfrog-ai-catalog-skills` skill to discover, install, update, and publish agent skills hosted in the JFrog AI Catalog. --- @@ -17,7 +18,8 @@ The JFrog plugin provides the following capabilities, grouped by component: | **Skill** | JFrog Platform | Interact with Artifactory repositories, builds, permissions, users, access tokens, projects, release bundles, and platform administration via the JFrog CLI and REST/GraphQL APIs. Also covers security audits, CVE lookups, and Advanced Security exposure queries. | | **Skill** | Package safety & download | Check whether npm, Maven, PyPI, Go, and other packages are safe, curated, or allowed, then download them through Artifactory remote caches or curation-aware package managers. | | **Hook + Skill** | Agent Package Resolution (Preview) | Automatically route packages installed by the AI agent through your organization's JFrog Artifactory, keeping agent-driven installs inside your Curation, Xray, and governance perimeter. | -| **Skill** | Agent Guard | Manage MCPs through the JFrog Agent Guard. Through the Agent Guard you can discover, install, configure, update, and remove MCP servers from the JFrog AI Catalog approved for your project, and authenticate to remote HTTP MCPs via OAuth, API key, or bearer token. | +| **Hook** | Agent Guard | Cursor manage MCPs through the JFrog Agent Guard. Through the Agent Guard you can discover, install, configure, update, and remove MCP servers from the JFrog AI Catalog approved for your project, and authenticate to remote HTTP MCPs via OAuth, API key, or bearer token. | +| **Hook** | Skills governance | When a skill is invoked, the plugin checks it against your JFrog governance policy and blocks disallowed or unscanned skills before they run. Applies to skills you run with `/` and to any read of a `SKILL.md` (how a skill's body reaches the model, since Cursor has no dedicated `Skill` tool). Enforced only when your account is entitled to AI Catalog skills governance. | --- @@ -126,6 +128,58 @@ When Agent Package Resolution is enabled and configured, no special prompt synta When an MCP server requires a sensitive configuration, the agent cannot set the value directly. Instead, it returns a CLI command for you to copy and run in your terminal. Secrets such as API keys, tokens, and connection strings are never exposed in the agent chat history. +### Skills governance + +When a skill is about to run, a hook checks it against your JFrog governance policy and blocks it if policy disallows it. Cursor has no dedicated `Skill` tool, so it covers the two entry points that actually carry a skill's identity: + +- you running a skill with `/`, +- and the agent reading a skill's `SKILL.md` — how a skill's body reaches the model, and so also the path a model-decided invocation funnels through, since the agent must read the file before it can act on it. This is caught at `preToolUse` (matcher `Read`), before Cursor reads the file's bytes off disk rather than after. + +These are the two entry points that carry a skill's identity; they are not a claim to cover every way content can reach the model. A skill whose name the hook cannot resolve to a folder on disk is **allowed**, not blocked — resolution completeness is therefore a security property, and the searched locations are listed in the Agent Guard's architecture notes. Anything that puts a file's contents in front of the model without going through a `Read` tool call is outside both surfaces. + +For each, the hook computes the skill's content **fingerprint** and asks the JFrog governance service for a verdict: + +| Verdict | What happens | +| --- | --- | +| **Allowed** | The skill runs. | +| **Blocked** | The skill is prevented from running, and each violated policy is named along with the reason it failed. | +| **Not yet scanned** | The skill is submitted for an on-the-fly scan and blocked with a "scan started — retry shortly" message. | +| **Not entitled** | If your account isn't entitled to AI Catalog skills governance, enforcement is skipped and skills run normally. | + +#### Requesting a waiver + +When a policy block carries a waiver scope, the block message shows the command that requests one, against the blocking policy's application, stage, and gate, with your justification attached. The Agent Guard files it — `agent-guard --request-waiver` — so the plugin holds no credentials and no waiver logic of its own. + +On a blocked `Read` the agent is given the command and can run it once you say why you need access. On a blocked `/` there is no model turn, so the command is printed for you to copy and run yourself. Either way the request goes to your project admin for review — it does not unblock the skill on its own, and nothing is submitted unless you ask for it and give a reason. + +**Requirements & behavior** + +> [!IMPORTANT] +> **On Cursor, only an answer can block.** A verdict reaches Cursor as JSON on the hook's stdout; +> the hook always exits 0 and does not signal through its exit code. So there are two outcomes: +> +> - **The Agent Guard answers** — its answer decides. A policy denial blocks and names the policies +> violated plus the command to request a waiver; anything else runs. +> - **The Agent Guard does not answer** — `npx` missing, the registry unreachable, no JFrog server +> configured, a crash, or the check running out of time — **allowed**. A machine that cannot get a +> verdict is not governed by it, and blocking there would stop work without enforcing anything. +> +> This is deliberately more permissive than the Claude Code plugin, which blocks when the guard +> reaches the check but cannot finish it. Cursor's hooks carry `failClosed: false`, so a hook that +> fails or is killed is not a block — meaning a slow verdict allows here where it would refuse +> 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 +> an unconfigured or unentitled user, so no setup is needed to opt out of the feature. + +- Set `JFROG_URL` and `JF_ACCESS_TOKEN` (or configure the JFrog CLI — see [Authentication](#authentication)) and `JF_PROJECT` (the JFrog project the skill runs in). For an entitled account with credentials but no project, skills are **blocked** with a message telling you what to set; with no credentials at all they are **allowed**, per the table above. +- **Node.js (≥ 18) with `npx` on your `PATH`** — the hook resolves the Agent Guard through `npx`. Without it, governed actions are allowed unchecked. +- **POSIX shell required.** Cursor's hook schema has no `shell` field, so the command runs in the platform's default shell. It uses POSIX syntax (`${VAR:-default}`, `$(( ))`), so on Windows `cmd`/PowerShell it cannot run — and governed actions are allowed unchecked. macOS and Linux are unaffected. +- **Cost per call.** The hook spawns a shell, `npx` and the Agent Guard on *every* prompt submission (`beforeSubmitPrompt` is unmatched, so it sees all of them) and on *every* `Read`. There is no throttle and no cache of a recent verdict, so a read-heavy session pays it repeatedly. `--prefer-offline` keeps a warm machine off the network, which is what makes that affordable; the warm per-call cost has not been measured. +- The Agent Guard logs its decisions to stderr; run Cursor with hook output visible to see them. +- To turn enforcement off, remove the `beforeSubmitPrompt`/`preToolUse` entries from `plugins/jfrog/hooks/hooks.json`. + --- ## Troubleshooting diff --git a/plugins/jfrog/.cursor-plugin/plugin.json b/plugins/jfrog/.cursor-plugin/plugin.json index 693fbe1..6b591e2 100644 --- a/plugins/jfrog/.cursor-plugin/plugin.json +++ b/plugins/jfrog/.cursor-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "jfrog", "displayName": "JFrog Platform", - "version": "0.5.13", + "version": "0.6.0", "description": "JFrog Platform integration with MCP, security skills, Agent Package Resolution, supply-chain best practices, and JFrog Agent Guard governance for adding, removing, and listing MCP servers.", "author": { "name": "JFrog", diff --git a/plugins/jfrog/hooks/hooks.json b/plugins/jfrog/hooks/hooks.json index a5afe5d..2036656 100644 --- a/plugins/jfrog/hooks/hooks.json +++ b/plugins/jfrog/hooks/hooks.json @@ -6,6 +6,21 @@ "command": "node \"./modules/cursor-session-start.mjs\" package-resolution", "timeout": 7 } + ], + "beforeSubmitPrompt": [ + { + "command": "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 cursor", + "timeout": 30, + "failClosed": false + } + ], + "preToolUse": [ + { + "command": "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 cursor", + "matcher": "Read", + "timeout": 30, + "failClosed": false + } ] } } diff --git a/scripts/validate-skill-governance.mjs b/scripts/validate-skill-governance.mjs new file mode 100644 index 0000000..d27797d --- /dev/null +++ b/scripts/validate-skill-governance.mjs @@ -0,0 +1,333 @@ +#!/usr/bin/env node +// Copyright (c) JFrog Ltd. 2026 +// Licensed under the Apache License, Version 2.0 +// https://www.apache.org/licenses/LICENSE-2.0 + +// Tests the skill-governance hook wiring. hooks.json invokes agent-guard DIRECTLY via npx — no +// plugin-side governance code at all — so this validator asserts the wiring's shape and then +// EXECUTES the real command string out of hooks.json against a stub agent-guard. +// +// Four properties carry the whole design, and each is asserted below: +// +// 1. A Cursor verdict travels as JSON on stdout with exit 0. `failClosed` is a crash net, not +// how a block is signalled — so the hook must NOT wrap the call in `|| exit 2`. An allow is +// explicit JSON (`{}`), never silence. +// +// 2. Infrastructure failure fails OPEN. npx missing, a failed install, an unreachable registry +// or no configured JFrog server must 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. `failClosed: false` is what makes that true for crashes and +// timeouts; a real policy denial still blocks, because it rides in the JSON payload. +// +// 3. Nothing may pin the agent-guard version. The hook resolves `latest` so a shipped GA fix +// reaches users without a plugin release. +// +// 4. npx ONLY, with no `command -v agent-guard` fast path: a binary earlier on PATH could be +// anything, whereas npx always resolves the package from the pinned registry. +// +// The plugin root comes from CURSOR_PLUGIN_ROOT, which Cursor sets for a plugin hook (measured +// on Cursor 3.4.20) — the direct analogue of CLAUDE_PLUGIN_ROOT. + +import { spawnSync } from "node:child_process"; +import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import process from "node:process"; +import { fileURLToPath } from "node:url"; + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +const pluginRoot = path.join(repoRoot, "plugins", "jfrog"); +const hooks = JSON.parse(readFileSync(path.join(pluginRoot, "hooks", "hooks.json"), "utf8")); + +const RELEASES_REGISTRY = "https://releases.jfrog.io/artifactory/api/npm/coding-agents-npm/"; +const GOVERNED_STEPS = ["beforeSubmitPrompt", "preToolUse"]; +// An absolute sh: the PATH below is deliberately minimal, so `sh` by name would not resolve. +const SH = "/bin/sh"; + +const sandbox = mkdtempSync(path.join(tmpdir(), "cursor-gov-")); +const binDir = path.join(sandbox, "bin"); +mkdirSync(binDir, { recursive: true }); +// A directory holding ONLY node, so the stub's shebang resolves while the real npx stays +// unreachable. Using node's own directory would defeat the "npx is missing" check, because the +// real npx sits right beside node — that check would then reach the network instead of exercising +// the 127 path. +// `date` lives here too: the hook computes its deadline with $(date +%s), and a PATH without it +// would silently yield "$(( + 25))" = 25 — an epoch in 1970 — rather than exercising the real +// computation. Keeping it beside node (not by adding /bin to PATH) preserves the isolate mode, +// where npx must stay unreachable. +const nodeDir = path.join(sandbox, "node-only"); +mkdirSync(nodeDir, { recursive: true }); +symlinkSync(process.execPath, path.join(nodeDir, "node")); +symlinkSync("/bin/date", path.join(nodeDir, "date")); + +const failures = []; +const check = async (label, fn) => { + try { await fn(); console.log(` ok ${label}`); } + catch (e) { failures.push(label); console.log(` FAIL ${label}\n ${e.message}`); } +}; +const assert = (cond, msg) => { if (!cond) throw new Error(msg); }; + +const entriesFor = (step) => hooks?.hooks?.[step] ?? []; +const commandFor = (step) => entriesFor(step)[0]?.command ?? ""; +// A blocking answer is a deny in the JSON, or agent-guard's own exit 2. Anything else lets the +// skill through — which is what "fails open" has to mean on a failClosed:false hook. +const blocks = (r) => r.code === 2 || /"permission"\s*:\s*"deny"|"continue"\s*:\s*false/.test(r.stdout); + +// A stub npx that records the argv it was handed and the stdin it received, then replays a canned +// result. Installed as `npx` so the hook command finds it first on PATH. +function stubNpx({ stdout = "", exitCode = 0 }) { + const record = path.join(sandbox, "record.json"); + rmSync(record, { force: true }); + writeFileSync(path.join(binDir, "npx"), `#!/usr/bin/env node +const fs = require("node:fs"); +let input = ""; +process.stdin.on("data", (d) => (input += d)); +process.stdin.on("end", () => { + fs.writeFileSync(${JSON.stringify(record)}, JSON.stringify({ argv: process.argv.slice(2), stdin: input, deadline: process.env.JF_AGENT_GUARD_ENFORCE_DEADLINE ?? "" })); + if (${JSON.stringify(stdout)}) process.stdout.write(${JSON.stringify(stdout)}); + process.exit(${exitCode}); +}); +`, { mode: 0o755 }); + chmodSync(path.join(binDir, "npx"), 0o755); + return record; +} + +// Run a hook command the way Cursor does: the string from hooks.json handed to a shell, the event +// JSON on stdin, and CURSOR_PLUGIN_ROOT set as Cursor sets it. `isolate` drops the stub from PATH, +// which is how "npx is not installed at all" is reproduced. +function runHook(command, payload, { isolate = false, extraEnv = {} } = {}) { + const result = spawnSync(SH, ["-c", command], { + input: Buffer.from(payload), + encoding: "buffer", + timeout: 30_000, + env: { + PATH: isolate ? nodeDir : `${binDir}:${nodeDir}`, + HOME: sandbox, + CURSOR_PLUGIN_ROOT: pluginRoot, + ...extraEnv, + }, + }); + if (result.error) throw new Error(`could not run the hook via ${SH}: ${result.error.message}`); + return { + code: result.status, + stdout: result.stdout ? result.stdout.toString() : "", + stderr: result.stderr ? result.stderr.toString() : "", + }; +} + +console.log("Validating the skill-governance hook wiring…"); + +check("no plugin-side governance code remains at all", () => { + const gone = [ + "scripts/governance/block-skill.mjs", + "scripts/governance/request-waiver.mjs", + "scripts/governance/helpers/credentials.mjs", + "scripts/governance/helpers/governance-client.mjs", + "scripts/governance/helpers/skill-fingerprint.mjs", + "scripts/governance/helpers/skill-package.mjs", + "scripts/governance/helpers/skill-path.mjs", + "scripts/governance/helpers/policy-block.mjs", + "scripts/governance/helpers/template-messages.mjs", + ]; + for (const rel of gone) { + let exists = true; + try { readFileSync(path.join(pluginRoot, rel)); } catch { exists = false; } + assert(!exists, `${rel} still exists; all governance logic — waivers included — lives in agent-guard`); + } +}); + +check("the package-resolution sessionStart hook is byte-identical", () => { + const h = entriesFor("sessionStart")[0]; + assert(h?.command === 'node "./modules/cursor-session-start.mjs" package-resolution', + "the package-resolution sessionStart hook command was altered"); + assert(h?.timeout === 7, "the package-resolution sessionStart hook timeout was altered"); +}); + +for (const step of GOVERNED_STEPS) { + check(`${step} invokes agent-guard through npx only, with no plugin script in the path`, () => { + const entries = entriesFor(step); + assert(entries.length === 1, `expected exactly one ${step} hook, got ${entries.length}`); + const h = entries[0]; + assert(/(^|\s)npx\s/.test(h.command), `${step} must invoke npx directly, got: ${h.command}`); + assert(!h.command.includes("command -v"), + `${step} must not have a PATH fast path: a hijackable agent-guard earlier on PATH would win`); + assert(!/block-skill|governance-client|skill-path|request-waiver|--waiver-helper/.test(h.command), + `${step} must not route through a plugin governance script; agent-guard owns the waiver flow`); + assert(h.command.includes("--enforce-skill") && h.command.includes("--client cursor"), + `${step} must pass --enforce-skill --client cursor`); + assert(h.command.includes(RELEASES_REGISTRY), + `${step} must default to the releases registry, so the artifact is the published one`); + }); + + check(`${step} fails OPEN: no exit-2 wrapper, and failClosed is off`, () => { + const h = entriesFor(step)[0]; + assert(!/\|\|\s*exit\b/.test(h.command), + `${step} must not wrap the call in "|| exit": a Cursor verdict is JSON on exit 0, and ` + + `converting an npx/install failure into a block refuses every skill on a machine that ` + + `simply cannot run the guard`); + assert(h.failClosed === false, + `${step} must set failClosed: false — a crash or timeout must let the skill through, ` + + `while a real denial still blocks through the JSON payload`); + }); + + check(`${step} lets npx cold-start and bounds its fetch`, () => { + const h = entriesFor(step)[0]; + assert((h.timeout ?? 0) >= 30, `${step} timeout ${h.timeout} is too short for an npx cold start`); + assert(h.command.includes("--prefer-offline"), + `${step} must prefer the cache, so a warm machine pays no registry round trip`); + const retries = /npm_config_fetch_retries=(\d+)/.exec(h.command); + const fetchTimeout = /npm_config_fetch_timeout=(\d+)/.exec(h.command); + assert(retries && Number(retries[1]) === 0, + `${step} must set npm_config_fetch_retries=0: npm's default of 2 backs off 10s then 60s`); + // The VALUE, not just its presence: npm_config_fetch_timeout=300000 is npm's own default, so + // asserting presence alone would let an edit back to the default pass unnoticed. The bound has + // to stay well inside the hook timeout to be worth setting at all. + assert(fetchTimeout && Number(fetchTimeout[1]) <= 10_000, + `${step} must set npm_config_fetch_timeout <= 10000 (npm's default is 300000ms), got ${fetchTimeout?.[1]}`); + }); + + check(`${step} computes the deadline fresh, with no inheritable fallback`, () => { + const h = entriesFor(step)[0]; + assert(h.command.includes('JF_AGENT_GUARD_ENFORCE_DEADLINE="$(($(date +%s) + 25))"'), + `${step} must compute an absolute deadline at invocation time`); + assert(!/JF_AGENT_GUARD_ENFORCE_DEADLINE:[-=]/.test(h.command), + `${step} must not fall back to an inherited value: an absolute instant inherited from an ` + + `earlier process pins every later invocation to the past`); + }); + + check(`${step} cannot pin the agent-guard version`, () => { + const h = entriesFor(step)[0]; + assert(!h.command.includes("JFROG_AGENT_GUARD_VERSION"), + `${step} must resolve latest, so a shipped GA fix reaches users without a plugin release`); + assert(/@jfrog\/agent-guard(\s|$)/.test(h.command), + `${step} must name the package unpinned: ${h.command}`); + }); +} + +check("preToolUse is scoped to Read", () => { + assert(entriesFor("preToolUse")[0]?.matcher === "Read", + "preToolUse must match Read: a skill the model discovers surfaces as a read of its SKILL.md"); +}); + +check("beforeSubmitPrompt has no matcher, so every submission is seen", () => { + assert(entriesFor("beforeSubmitPrompt")[0]?.matcher === undefined, + "a matcher here would filter out the slash invocations this hook exists to gate"); +}); + +check("the two governed hooks run byte-identical commands", () => { + assert(commandFor("beforeSubmitPrompt") === commandFor("preToolUse"), + "the two surfaces must enforce identically; they have drifted apart"); +}); + +check("every hook command is valid POSIX sh", () => { + for (const step of Object.keys(hooks.hooks ?? {})) { + for (const h of entriesFor(step)) { + const r = spawnSync(SH, ["-n", "-c", h.command], { encoding: "utf8" }); + assert(r.status === 0, `${step} command is not valid sh: ${r.stderr.trim()}`); + } + } +}); + +// --------------------------------------------------------------------------- +// Behavioural: execute the real hooks.json command string. +// --------------------------------------------------------------------------- + +// A payload shaped like the surface actually sends, so a check cannot pass by feeding preToolUse's +// event to the prompt hook. +const payloadFor = (step) => step === "preToolUse" + ? `{"hook_event_name":"preToolUse","tool_name":"Read","tool_input":{"file_path":"/a/b/SKILL.md"}}` + : `{"hook_event_name":"beforeSubmitPrompt","prompt":"/demo-skill"}`; + +// Run every behavioural check against BOTH governed surfaces, not just preToolUse. The +// byte-identical check above already makes divergence loud, but it only holds while it runs first; +// looping here means a future hook that stops being identical is still exercised on its own terms +// rather than inheriting preToolUse's result. +for (const step of GOVERNED_STEPS) { + await check(`${step}: forwards stdin verbatim and hands agent-guard the expected argv`, async () => { + const record = stubNpx({ stdout: "{}" }); + const payload = payloadFor(step); + const r = runHook(commandFor(step), payload); + assert(r.code === 0, `exit=${r.code} stderr=${r.stderr}`); + const seen = JSON.parse(readFileSync(record, "utf8")); + assert(seen.stdin === payload, `stdin altered: ${seen.stdin}`); + assert(seen.argv.includes("--enforce-skill"), `argv missing --enforce-skill: ${seen.argv}`); + assert(seen.argv[seen.argv.indexOf("--client") + 1] === "cursor", `bad --client: ${seen.argv}`); + assert(!seen.argv.includes("--waiver-helper"), `--waiver-helper was removed from agent-guard: ${seen.argv}`); + assert(seen.argv[seen.argv.indexOf("--registry") + 1] === RELEASES_REGISTRY, + `must default to the releases registry: ${seen.argv}`); + }); + + await check(`${step}: hands agent-guard a deadline in the future, computed at invocation`, async () => { + const record = stubNpx({ stdout: "{}" }); + const before = Math.floor(Date.now() / 1000); + runHook(commandFor(step), payloadFor(step), { + // A stale value in the environment must NOT survive into the child. + extraEnv: { JF_AGENT_GUARD_ENFORCE_DEADLINE: "1" }, + }); + const seen = JSON.parse(readFileSync(record, "utf8")); + const deadline = Number(seen.deadline); + assert(Number.isFinite(deadline) && deadline > before, + `the deadline must be recomputed, not inherited; got ${seen.deadline}`); + }); + + await check(`${step}: forwards a deny verdict's stdout verbatim and exits 0 (the JSON decides)`, async () => { + const deny = step === "preToolUse" + ? `{"permission":"deny","user_message":"blocked"}` + : `{"continue":false,"user_message":"blocked"}`; + stubNpx({ stdout: deny }); + const r = runHook(commandFor(step), payloadFor(step)); + assert(r.code === 0, `a rendered verdict must exit 0 and let the JSON decide, got ${r.code}`); + assert(r.stdout === deny, `stdout altered: ${r.stdout}`); + 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 () => { + stubNpx({ stdout: "{}" }); + const r = runHook(commandFor(step), payloadFor(step)); + assert(r.code === 0 && r.stdout === "{}", + `an allow must be explicit JSON: exit=${r.code} stdout=${r.stdout}`); + assert(!blocks(r), "an empty object must not read as a block"); + }); + + await check(`${step}: an agent-guard failure fails OPEN, not closed`, async () => { + stubNpx({ stdout: "", exitCode: 1 }); + const r = runHook(commandFor(step), payloadFor(step)); + assert(!blocks(r), + `an internal failure must let the skill through, got exit=${r.code} stdout=${r.stdout}`); + }); + + // With npx absent the guard never runs at all. That machine is not governed, so it must not be + // punished: this is the case `|| exit 2` used to convert into a block. + await check(`${step}: npx missing entirely fails OPEN`, async () => { + const r = runHook(commandFor(step), payloadFor(step), { isolate: true }); + assert(!blocks(r), `a missing npx must fail open, got exit=${r.code} stdout=${r.stdout}`); + }); + + await check(`${step}: agent-guard's own exit 2 still blocks`, async () => { + stubNpx({ stdout: "", exitCode: 2 }); + const r = runHook(commandFor(step), payloadFor(step)); + assert(r.code === 2, + `agent-guard exits 2 for a block it could not deliver; the hook must not mask it, got ${r.code}`); + }); + + await check(`${step}: JFROG_AGENT_GUARD_REPO redirects the registry, and nothing can pin the version`, async () => { + const record = stubNpx({ stdout: "{}" }); + runHook(commandFor(step), payloadFor(step), { + extraEnv: { + JFROG_AGENT_GUARD_REPO: "https://example.invalid/npm/dev/", + JFROG_AGENT_GUARD_VERSION: "0.0.0-master.1.gabc", + }, + }); + const seen = JSON.parse(readFileSync(record, "utf8")); + assert(seen.argv[seen.argv.indexOf("--registry") + 1] === "https://example.invalid/npm/dev/", + `registry override ignored: ${seen.argv}`); + assert(seen.argv.includes("@jfrog/agent-guard"), + `the package spec must stay unpinned: ${seen.argv}`); + assert(!seen.argv.some((a) => a.startsWith("@jfrog/agent-guard@")), + `no environment variable may pin the version: ${seen.argv}`); + }); +} + +rmSync(sandbox, { recursive: true, force: true }); +if (failures.length) { console.log(`\n${failures.length} check(s) failed.`); process.exit(1); } +console.log("\nAll checks passed."); From 39bfd5f46959f8623c26e170b6961af120c1b686 Mon Sep 17 00:00:00 2001 From: shmuelb Date: Thu, 27 Aug 2026 16:45:34 +0300 Subject: [PATCH 2/5] MLAI-1288 - Pre-warm the agent-guard cache on sessionStart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --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 --- plugins/jfrog/hooks/hooks.json | 4 ++++ scripts/validate-skill-governance.mjs | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/plugins/jfrog/hooks/hooks.json b/plugins/jfrog/hooks/hooks.json index 7500b52..2ded4d8 100644 --- a/plugins/jfrog/hooks/hooks.json +++ b/plugins/jfrog/hooks/hooks.json @@ -9,6 +9,10 @@ { "command": "node \"./scripts/cursor-align-mcp-json.mjs\" session-start", "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", + "timeout": 10 } ], "beforeSubmitPrompt": [ diff --git a/scripts/validate-skill-governance.mjs b/scripts/validate-skill-governance.mjs index b30beec..0fef7bd 100644 --- a/scripts/validate-skill-governance.mjs +++ b/scripts/validate-skill-governance.mjs @@ -146,6 +146,25 @@ check("the package-resolution sessionStart hook is byte-identical", () => { assert(h.timeout === 7, "the package-resolution sessionStart hook timeout was altered"); }); +check("a sessionStart pre-warm refreshes the cache the governed hooks then read", () => { + const warm = entriesFor("sessionStart").find((h) => h.command.includes("@jfrog/agent-guard")); + assert(warm, "sessionStart must pre-warm agent-guard: it is the ONLY thing that refreshes the " + + "npx cache, and without it --prefer-offline below can serve a stale binary indefinitely " + + "(measured: a cached 1.10.0 kept being used while 1.11.0 was latest, reinstating a bug " + + "1.11.0 had fixed)"); + assert(!warm.command.includes("--prefer-offline"), + "the pre-warm MUST hit the registry; --prefer-offline here would defeat its only purpose"); + assert(!warm.command.includes("JFROG_AGENT_GUARD_VERSION"), + "the pre-warm must refresh to latest, not to a pinned version"); + // Cursor's hook schema has no `async`, so the command detaches itself. Both halves matter: + // the subshell-and-background returns control immediately, and the explicit exit 0 keeps a + // spawn failure from surfacing as a failed session-start hook. + assert(/\(.*&\s*\)/.test(warm.command), + "the pre-warm must detach (subshell + &) so it cannot delay session start"); + assert(/exit 0\s*$/.test(warm.command.trim()), + "the pre-warm must end in `exit 0`: a warm failure is never a reason to fail session start"); +}); + for (const step of GOVERNED_STEPS) { check(`${step} invokes agent-guard through npx only, with no plugin script in the path`, () => { const entries = entriesFor(step); From c4cb1d15de36c12d2d6ef0392980450cd27d9de4 Mon Sep 17 00:00:00 2001 From: shmuelb Date: Thu, 27 Aug 2026 18:11:41 +0300 Subject: [PATCH 3/5] MLAI-1288 - Degrade to no deadline when the clock cannot be read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- plugins/jfrog/hooks/hooks.json | 4 ++-- scripts/validate-skill-governance.mjs | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/plugins/jfrog/hooks/hooks.json b/plugins/jfrog/hooks/hooks.json index 2ded4d8..de02cad 100644 --- a/plugins/jfrog/hooks/hooks.json +++ b/plugins/jfrog/hooks/hooks.json @@ -17,14 +17,14 @@ ], "beforeSubmitPrompt": [ { - "command": "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 cursor", + "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", "timeout": 30, "failClosed": false } ], "preToolUse": [ { - "command": "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 cursor", + "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", "timeout": 30, "failClosed": false diff --git a/scripts/validate-skill-governance.mjs b/scripts/validate-skill-governance.mjs index 0fef7bd..4b53bf1 100644 --- a/scripts/validate-skill-governance.mjs +++ b/scripts/validate-skill-governance.mjs @@ -210,8 +210,12 @@ for (const step of GOVERNED_STEPS) { check(`${step} computes the deadline fresh, with no inheritable fallback`, () => { const h = entriesFor(step)[0]; - assert(h.command.includes('JF_AGENT_GUARD_ENFORCE_DEADLINE="$(($(date +%s) + 25))"'), - `${step} must compute an absolute deadline at invocation time`); + assert(/_JFAG_NOW=\$\(date \+%s 2>\/dev\/null\);/.test(h.command), + `${step} must read the clock defensively, tolerating an absent date(1)`); + assert(h.command.includes('JF_AGENT_GUARD_ENFORCE_DEADLINE="${_JFAG_NOW:+$((_JFAG_NOW + 25))}"'), + `${step} must compute an absolute deadline at invocation time, and pass EMPTY when the ` + + `clock could not be read: agent-guard ignores an empty deadline and falls back to its own ` + + `budget, whereas a garbage epoch floors the budget at 500ms and blocks every skill`); assert(!/JF_AGENT_GUARD_ENFORCE_DEADLINE:[-=]/.test(h.command), `${step} must not fall back to an inherited value: an absolute instant inherited from an ` + `earlier process pins every later invocation to the past`); From 6f9aa8ed80d8cdba9c002e5fe8e41ab385139ae2 Mon Sep 17 00:00:00 2001 From: shmuelb Date: Fri, 28 Aug 2026 14:38:29 +0300 Subject: [PATCH 4/5] MLAI-1288 - Correct the block/allow cases and gate the release on the validator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The README claimed the hook always exits 0 and that running out of time allows. Both are wrong: when the Agent Guard's own budget expires it writes a refusal and exits 2, so it BLOCKS. What allows is Cursor killing the hook at its own timeout — a different clock, and the longer of the two. The three cases are now stated separately, and a paragraph that had been duplicated verbatim is gone. The capabilities table promised more coverage than the Skills governance section admits; it now names the same two entry points and the same gap. validate-skill-governance.yml is pull-request only, so nothing checked the merge commit a release is cut from. release.yml now runs the script directly, matching what it already does for validate-template.mjs and what CONTRIBUTING.md requires. Adds validator cases for output that is not valid JSON and for empty stdout with exit 0, and watches modules/** so re-added governance code cannot slip past the trigger. Co-Authored-By: Claude Opus 5 --- .github/workflows/release.yml | 6 ++++ .../workflows/validate-skill-governance.yml | 1 + README.md | 30 +++++++++---------- scripts/validate-skill-governance.mjs | 15 ++++++++++ 4 files changed, 37 insertions(+), 15 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a94aba8..d5fddb5 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -79,6 +79,12 @@ jobs: if: steps.detect.outputs.triggered == 'true' run: node scripts/validate-template.mjs + # Same reason as above: validate-skill-governance.yml is pull-request only, so the merge + # commit the release is cut from is never checked by it. + - name: Validate skill governance before releasing + if: steps.detect.outputs.triggered == 'true' + run: node scripts/validate-skill-governance.mjs + # Tracked files at HEAD only, so nothing left on the runner can end up in the zip. - name: Package release artifact if: steps.detect.outputs.triggered == 'true' diff --git a/.github/workflows/validate-skill-governance.yml b/.github/workflows/validate-skill-governance.yml index baee516..5004c5f 100644 --- a/.github/workflows/validate-skill-governance.yml +++ b/.github/workflows/validate-skill-governance.yml @@ -13,6 +13,7 @@ on: # validator that asserts it must not exist. - "plugins/jfrog/hooks/hooks.json" - "plugins/jfrog/scripts/**" + - "plugins/jfrog/modules/**" - "plugins/jfrog/.cursor-plugin/plugin.json" - "scripts/validate-skill-governance.mjs" workflow_dispatch: diff --git a/README.md b/README.md index 7cd2f1f..7f24e9f 100644 --- a/README.md +++ b/README.md @@ -20,7 +20,7 @@ The JFrog plugin provides the following capabilities, grouped by component: | **Skill** | Package safety & download | Check whether npm, Maven, PyPI, Go, and other packages are safe, curated, or allowed, then download them through Artifactory remote caches or curation-aware package managers. | | **Hook + Skill** | Agent Package Resolution (Preview) | Automatically route packages installed by the AI agent through your organization's JFrog Artifactory, keeping agent-driven installs inside your Curation, Xray, and governance perimeter. | | **Hook** | Agent Guard | Cursor manage MCPs through the JFrog Agent Guard. Through the Agent Guard you can discover, install, configure, update, and remove MCP servers from the JFrog AI Catalog approved for your project, and authenticate to remote HTTP MCPs via OAuth, API key, or bearer token. | -| **Hook** | Skills governance | When a skill is invoked, the plugin checks it against your JFrog governance policy and blocks disallowed or unscanned skills before they run. Applies to skills you run with `/` and to any read of a `SKILL.md` (how a skill's body reaches the model, since Cursor has no dedicated `Skill` tool). Enforced only when your account is entitled to AI Catalog skills governance. | +| **Hook** | Skills governance | When a skill is invoked, the plugin checks it against your JFrog governance policy and blocks disallowed or unscanned skills before they run. Covers the two entry points that carry a skill's identity: skills you run with `/`, and any read of a `SKILL.md` (how a skill's body reaches the model, since Cursor has no dedicated `Skill` tool). Content that reaches the model without a `Read` tool call is outside both — see [Skills governance](#skills-governance). Enforced only when your account is entitled to AI Catalog skills governance. | --- @@ -156,23 +156,23 @@ On a blocked `Read` the agent is given the command and can run it once you say w **Requirements & behavior** > [!IMPORTANT] -> **On Cursor, only an answer can block.** A verdict reaches Cursor as JSON on the hook's stdout; -> the hook always exits 0 and does not signal through its exit code. So there are two outcomes: +> **A verdict reaches Cursor as JSON on the hook's stdout.** Three outcomes, and they are distinct: > -> - **The Agent Guard answers** — its answer decides. A policy denial blocks and names the policies -> violated plus the command to request a waiver; anything else runs. -> - **The Agent Guard does not answer** — `npx` missing, the registry unreachable, no JFrog server -> configured, a crash, or the check running out of time — **allowed**. A machine that cannot get a -> verdict is not governed by it, and blocking there would stop work without enforcing anything. +> - **Your JFrog policies deny the skill** — **blocked**, naming the policies it violated and the +> command to request a waiver. +> - **The Agent Guard reaches the check but cannot finish it in time** — **blocked**. It writes a +> refusal explaining that it could not answer, and exits 2. It got as far as the check, so it does +> not guess. +> - **The Agent Guard cannot be *run at all*** — `npx` missing, the registry unreachable, no JFrog +> server configured, or it fails internally — **allowed**. A machine that cannot get a verdict is +> not governed by it, and blocking there would stop work without enforcing anything. > -> This is deliberately more permissive than the Claude Code plugin, which blocks when the guard -> reaches the check but cannot finish it. Cursor's hooks carry `failClosed: false`, so a hook that -> fails or is killed is not a block — meaning a slow verdict allows here where it would refuse -> 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. +> One case is Cursor-specific: the hooks carry `failClosed: false`, so if **Cursor** kills the hook +> at its own `timeout` the action is allowed. That is a different clock from the Agent Guard's own +> budget, which is deliberately the shorter of the two so it answers first. > -> A user who is entitled to nothing is unaffected either way: the Agent Guard returns "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 answers "allow" for an +> unconfigured or unentitled user, so no setup is needed to opt out of the feature. - Set `JFROG_URL` and `JF_ACCESS_TOKEN` (or configure the JFrog CLI — see [Authentication](#authentication)) and `JF_PROJECT` (the JFrog project the skill runs in). For an entitled account with credentials but no project, skills are **blocked** with a message telling you what to set; with no credentials at all they are **allowed**, per the table above. - **Node.js (≥ 18) with `npx` on your `PATH`** — the hook resolves the Agent Guard through `npx`. Without it, governed actions are allowed unchecked. diff --git a/scripts/validate-skill-governance.mjs b/scripts/validate-skill-governance.mjs index 4b53bf1..2c2e5e4 100644 --- a/scripts/validate-skill-governance.mjs +++ b/scripts/validate-skill-governance.mjs @@ -315,6 +315,21 @@ for (const step of GOVERNED_STEPS) { assert(!blocks(r), "an empty object must not read as a block"); }); + await check(`${step}: output that is not valid JSON is forwarded, not repaired`, async () => { + stubNpx({ stdout: "not json at all" }); + const r = runHook(commandFor(step), payloadFor(step)); + assert(r.stdout === "not json at all", + `the hook must forward bytes verbatim and never rewrite a verdict: ${r.stdout}`); + assert(!blocks(r), `unparseable output is not a deny; failClosed:false lets it through`); + }); + + await check(`${step}: empty stdout with exit 0 stays empty`, async () => { + stubNpx({ stdout: "", exitCode: 0 }); + const r = runHook(commandFor(step), payloadFor(step)); + assert(r.stdout === "", `the hook must not invent a verdict: ${r.stdout}`); + assert(!blocks(r), "silence is not a deny"); + }); + await check(`${step}: an agent-guard failure fails OPEN, not closed`, async () => { stubNpx({ stdout: "", exitCode: 1 }); const r = runHook(commandFor(step), payloadFor(step)); From 1ba4b97b4ffedb976da772c356e4fad09f156c67 Mon Sep 17 00:00:00 2001 From: shmuelb Date: Fri, 28 Aug 2026 15:05:05 +0300 Subject: [PATCH 5/5] MLAI-1288 - Refresh the agent-guard cache on the prompt hook, not sessionStart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sessionStart pre-warm does not work. Measured under a real Cursor session, twice: Cursor kills the hook's process group, and nothing outruns it — not a detached subshell, not sh -mc, not a perl setsid escape. It was dead code that implied a freshness guarantee. That left --prefer-offline with nothing refreshing it, which freezes agent-guard at whatever version was first fetched. That is the failure that made a cached 1.10.0 keep serving verdicts a released 1.11.0 had already fixed. The refresh now happens on a hook Cursor actually waits for. beforeSubmitPrompt drops --prefer-offline and revalidates against the registry; preToolUse keeps it and reads what that left behind. Measured warm: 1091ms revalidating, 324ms from cache. A prompt fires once per turn, a Read dozens of times, so the cost lands where it is invisible and the hot path stays fast. Session start pays nothing. Co-Authored-By: Claude Opus 5 --- README.md | 2 +- plugins/jfrog/hooks/hooks.json | 6 +--- scripts/validate-skill-governance.mjs | 45 ++++++++++++++------------- 3 files changed, 25 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 7f24e9f..8b4692a 100644 --- a/README.md +++ b/README.md @@ -177,7 +177,7 @@ On a blocked `Read` the agent is given the command and can run it once you say w - Set `JFROG_URL` and `JF_ACCESS_TOKEN` (or configure the JFrog CLI — see [Authentication](#authentication)) and `JF_PROJECT` (the JFrog project the skill runs in). For an entitled account with credentials but no project, skills are **blocked** with a message telling you what to set; with no credentials at all they are **allowed**, per the table above. - **Node.js (≥ 18) with `npx` on your `PATH`** — the hook resolves the Agent Guard through `npx`. Without it, governed actions are allowed unchecked. - **POSIX shell required.** Cursor's hook schema has no `shell` field, so the command runs in the platform's default shell. It uses POSIX syntax (`${VAR:-default}`, `$(( ))`), so on Windows `cmd`/PowerShell it cannot run — and governed actions are allowed unchecked. macOS and Linux are unaffected. -- **Cost per call.** The hook spawns a shell, `npx` and the Agent Guard on *every* prompt submission (`beforeSubmitPrompt` is unmatched, so it sees all of them) and on *every* `Read`. There is no throttle and no cache of a recent verdict, so a read-heavy session pays it repeatedly. `--prefer-offline` keeps a warm machine off the network, which is what makes that affordable; the warm per-call cost has not been measured. +- **Cost per call.** The hook spawns a shell, `npx` and the Agent Guard on *every* prompt submission (`beforeSubmitPrompt` is unmatched, so it sees all of them) and on *every* `Read`. There is no throttle and no cache of a recent verdict, so a read-heavy session pays it repeatedly. Measured warm on macOS: **~1.1s** per prompt, where the Agent Guard is revalidated against the registry so a released fix reaches you, and **~0.3s** per `Read`, which reads the copy that revalidation left in the cache. - The Agent Guard logs its decisions to stderr; run Cursor with hook output visible to see them. - To turn enforcement off, remove the `beforeSubmitPrompt`/`preToolUse` entries from `plugins/jfrog/hooks/hooks.json`. diff --git a/plugins/jfrog/hooks/hooks.json b/plugins/jfrog/hooks/hooks.json index de02cad..f8d6a18 100644 --- a/plugins/jfrog/hooks/hooks.json +++ b/plugins/jfrog/hooks/hooks.json @@ -9,15 +9,11 @@ { "command": "node \"./scripts/cursor-align-mcp-json.mjs\" session-start", "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", - "timeout": 10 } ], "beforeSubmitPrompt": [ { - "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", + "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 --registry \"${JFROG_AGENT_GUARD_REPO:-https://releases.jfrog.io/artifactory/api/npm/coding-agents-npm/}\" @jfrog/agent-guard --enforce-skill --client cursor", "timeout": 30, "failClosed": false } diff --git a/scripts/validate-skill-governance.mjs b/scripts/validate-skill-governance.mjs index 2c2e5e4..7be6a15 100644 --- a/scripts/validate-skill-governance.mjs +++ b/scripts/validate-skill-governance.mjs @@ -146,23 +146,24 @@ check("the package-resolution sessionStart hook is byte-identical", () => { assert(h.timeout === 7, "the package-resolution sessionStart hook timeout was altered"); }); -check("a sessionStart pre-warm refreshes the cache the governed hooks then read", () => { - const warm = entriesFor("sessionStart").find((h) => h.command.includes("@jfrog/agent-guard")); - assert(warm, "sessionStart must pre-warm agent-guard: it is the ONLY thing that refreshes the " + - "npx cache, and without it --prefer-offline below can serve a stale binary indefinitely " + - "(measured: a cached 1.10.0 kept being used while 1.11.0 was latest, reinstating a bug " + - "1.11.0 had fixed)"); - assert(!warm.command.includes("--prefer-offline"), - "the pre-warm MUST hit the registry; --prefer-offline here would defeat its only purpose"); - assert(!warm.command.includes("JFROG_AGENT_GUARD_VERSION"), - "the pre-warm must refresh to latest, not to a pinned version"); - // Cursor's hook schema has no `async`, so the command detaches itself. Both halves matter: - // the subshell-and-background returns control immediately, and the explicit exit 0 keeps a - // spawn failure from surfacing as a failed session-start hook. - assert(/\(.*&\s*\)/.test(warm.command), - "the pre-warm must detach (subshell + &) so it cannot delay session start"); - assert(/exit 0\s*$/.test(warm.command.trim()), - "the pre-warm must end in `exit 0`: a warm failure is never a reason to fail session start"); +check("one governed hook refreshes the npx cache, the other reads it", () => { + // Measured under Cursor: a detached sessionStart pre-warm does NOT survive — Cursor kills the + // hook's process group, and no escape (subshell, `sh -mc`, perl setsid) outruns it. So the + // refresh has to happen on a hook Cursor waits for. + // + // beforeSubmitPrompt fires once per prompt and omits --prefer-offline, so it revalidates against + // the registry and pulls a newer agent-guard into the cache. preToolUse fires on every Read and + // keeps --prefer-offline, reading what the prompt hook just refreshed. Measured warm: 1091ms + // revalidating vs 324ms from cache. + assert(!entriesFor("sessionStart").some((h) => h.command.includes("@jfrog/agent-guard")), + "sessionStart must not pre-warm: it is killed with the hook and only pretends to keep the " + + "cache fresh"); + assert(!commandFor("beforeSubmitPrompt").includes("--prefer-offline"), + "beforeSubmitPrompt must NOT pass --prefer-offline: it is the only thing that refreshes the " + + "cache, and without it agent-guard is frozen at whatever version was first fetched"); + assert(commandFor("preToolUse").includes("--prefer-offline"), + "preToolUse must pass --prefer-offline: it fires on every Read, and revalidating each time " + + "costs ~770ms per call"); }); for (const step of GOVERNED_STEPS) { @@ -195,8 +196,6 @@ for (const step of GOVERNED_STEPS) { check(`${step} lets npx cold-start and bounds its fetch`, () => { const h = entriesFor(step)[0]; assert((h.timeout ?? 0) >= 30, `${step} timeout ${h.timeout} is too short for an npx cold start`); - assert(h.command.includes("--prefer-offline"), - `${step} must prefer the cache, so a warm machine pays no registry round trip`); const retries = /npm_config_fetch_retries=(\d+)/.exec(h.command); const fetchTimeout = /npm_config_fetch_timeout=(\d+)/.exec(h.command); assert(retries && Number(retries[1]) === 0, @@ -240,9 +239,11 @@ check("beforeSubmitPrompt has no matcher, so every submission is seen", () => { "a matcher here would filter out the slash invocations this hook exists to gate"); }); -check("the two governed hooks run byte-identical commands", () => { - assert(commandFor("beforeSubmitPrompt") === commandFor("preToolUse"), - "the two surfaces must enforce identically; they have drifted apart"); +check("the two governed hooks differ ONLY in the cache flag", () => { + const norm = (c) => c.replace(" --prefer-offline", ""); + assert(norm(commandFor("beforeSubmitPrompt")) === norm(commandFor("preToolUse")), + "the two surfaces must enforce identically apart from --prefer-offline; they have drifted:\n" + + ` beforeSubmitPrompt: ${commandFor("beforeSubmitPrompt")}\n preToolUse: ${commandFor("preToolUse")}`); }); check("every hook command is valid POSIX sh", () => {