From ab30e3ecae71380d987eefe541987834cea78d2e Mon Sep 17 00:00:00 2001 From: davida-jfrog Date: Thu, 3 Sep 2026 20:00:14 +0300 Subject: [PATCH 1/2] AX-2205: Add self-hosted support via the /bridge-client base path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-hosted JPDs do not serve /ml/core off the platform root — the same APIs sit behind /bridge-client — so every probe 404s and the caller reports the feature as absent. Each /ml/core call site now retries once behind /bridge-client after a 404, and adopts that result only when it evidences a working endpoint, so no pre-existing failure changes its status, detail or exit code: - modules/core/agent-guard-check.mjs and its jfrog-mcp-management copy: the HTTP call is split into fetchSetting(), giving each attempt its own AbortController so the retry cannot inherit a spent timeout budget. Only an enabled/registry-off answer replaces the root verdict. - jfrog-detect-catalog-runtime.mjs: the fallback runs in both parts. Part A resolves the prefix anonymously; Part B retries independently for a JPD that answers 401 anonymously at the root but 404s the authenticated call. Part A adopts only a code that says the catalog is deployed, Part B only a 2xx of the catalog's shape or a 403 — so a proxy's 400/501 or a WAF's 401 cannot promote a non-blocking exit 1 into a blocking exit 3, or blame credentials that are fine. The marketplace registration script already had this fallback; it is unchanged. Tests: 13 new unit tests drive the injectable module copy through a route-map fetch double (no retry on 200/401/403/5xx/network, retry on 404, both-404 keeps the original reason, a fresh unaborted signal per attempt). A new integration suite spawns both skill scripts against a localhost JPD stub with a fake jf on PATH and HOME redirected, covering SaaS (asserting /bridge-client is never probed), both resolution paths, not_entitled, and every case where the fallback must not win. Co-Authored-By: Claude Opus 5 (1M context) --- .claude-plugin/plugin.json | 2 +- .github/workflows/validate.yml | 3 + modules/core/agent-guard-check.mjs | 36 +- scripts/agent-guard-check.test.mjs | 217 +++++++++- scripts/bridge-client-fallback.test.mjs | 374 ++++++++++++++++++ .../scripts/jfrog-detect-catalog-runtime.mjs | 68 +++- .../scripts/jfrog-agent-guard-check.mjs | 33 +- 7 files changed, 716 insertions(+), 17 deletions(-) create mode 100644 scripts/bridge-client-fallback.test.mjs diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 94fd492..5c98c73 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -2,7 +2,7 @@ "name": "jfrog", "displayName": "JFrog", "description": "Official JFrog plugin. Connect Claude Code to JFrog to manage, secure, and govern your software supply chain. Give agents the context to build secure, compliant software.", - "version": "0.3.3", + "version": "0.3.4", "author": { "name": "JFrog Ltd.", "email": "devrel@jfrog.com", diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 6b3d491..5fecd8b 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -25,5 +25,8 @@ jobs: - name: Run MCP rewrite hook unit tests run: node --test scripts/claude-mcp-json-discover.test.mjs scripts/claude-align-mcp-json.test.mjs scripts/rewrite-mcp-json.test.mjs scripts/agent-guard-check.test.mjs + + - name: Run /bridge-client fallback integration tests + run: node --test scripts/bridge-client-fallback.test.mjs - name: Validate the skill-enforcement hook run: node scripts/validate-enforce-hook.mjs diff --git a/modules/core/agent-guard-check.mjs b/modules/core/agent-guard-check.mjs index 4372b1a..db533d8 100644 --- a/modules/core/agent-guard-check.mjs +++ b/modules/core/agent-guard-check.mjs @@ -20,6 +20,9 @@ import { isMainEntry } from "./entry.mjs"; export const SETTINGS_PATH = "/ml/core/api/v1/administration/account-settings/mcp_gateway_plugin_enabled"; +// Self-hosted JPDs serve the same API behind `/bridge-client`. Tried ONLY +// after the root path 404s, so SaaS still costs exactly one request. +export const BRIDGE_CLIENT_PREFIX = "/bridge-client"; export const REQUEST_TIMEOUT_MS = 5000; export const EXIT_ENABLED = 0; @@ -171,6 +174,11 @@ function resolveFromCliConfig(opts) { }; } +/** Drops the internal `notFound` marker from a fetchSetting() result. */ +function strip({ notFound, ...result }) { + return result; +} + /** * @param {string} baseUrl * @param {string} token @@ -186,7 +194,32 @@ export async function isGatewayPluginEnabled(baseUrl, token, opts = {}) { const timeoutMs = opts.timeoutMs ?? REQUEST_TIMEOUT_MS; const root = baseUrl.replace(/\/+$/, "").replace(/\/artifactory$/, ""); - const url = root + SETTINGS_PATH; + + const rootResult = await fetchSetting(root + SETTINGS_PATH, token, { + debug, + fetchFn, + timeoutMs, + }); + if (!rootResult.notFound) return strip(rootResult); + + // Root 404 -> possibly self-hosted. Each attempt gets its OWN timeout + // budget: a reused AbortController would start the retry already spent. + debug(`Root ${SETTINGS_PATH} returned 404; retrying behind ${BRIDGE_CLIENT_PREFIX}.`); + const bridgeResult = await fetchSetting( + root + BRIDGE_CLIENT_PREFIX + SETTINGS_PATH, + token, + { debug, fetchFn, timeoutMs }, + ); + // The fallback may only IMPROVE the outcome — anything inconclusive keeps + // the root verdict, and with it every pre-existing reason and exit code. + if (bridgeResult.ok || bridgeResult.registryOff) return strip(bridgeResult); + return strip(rootResult); +} + +// One HTTP attempt against a fully-built settings URL. `notFound` marks the +// 404 that triggers the `/bridge-client` retry; callers strip it before +// returning so the public result shape is unchanged. +async function fetchSetting(url, token, { debug, fetchFn, timeoutMs }) { debug(`Fetching gateway plugin setting from ${url}`); const controller = new AbortController(); @@ -204,6 +237,7 @@ export async function isGatewayPluginEnabled(baseUrl, token, opts = {}) { debug(`Settings request returned HTTP ${response.status}.`); return { ok: false, + notFound: response.status === 404, reason: `settings endpoint returned HTTP ${response.status}`, }; } diff --git a/scripts/agent-guard-check.test.mjs b/scripts/agent-guard-check.test.mjs index 5486edd..f44ed99 100644 --- a/scripts/agent-guard-check.test.mjs +++ b/scripts/agent-guard-check.test.mjs @@ -5,7 +5,42 @@ import assert from "node:assert/strict"; import { test } from "node:test"; -import { resolveAgentGuardCredentials } from "../modules/core/agent-guard-check.mjs"; +import { + BRIDGE_CLIENT_PREFIX, + EXIT_DISABLED, + EXIT_ENABLED, + EXIT_REGISTRY_DISABLED, + SETTINGS_PATH, + isGatewayPluginEnabled, + resolveAgentGuardCredentials, + runAgentGuardCheck, +} from "../modules/core/agent-guard-check.mjs"; + +const ROOT = "https://acme.jfrog.io"; +const ROOT_URL = `${ROOT}${SETTINGS_PATH}`; +const BRIDGE_URL = `${ROOT}${BRIDGE_CLIENT_PREFIX}${SETTINGS_PATH}`; + +// A fetch double driven by a url -> response map. Records every URL it was +// called with so tests can assert the SaaS path stays a single request. +function stubFetch(routes) { + const calls = []; + const fetchFn = async (url) => { + calls.push(url); + const route = routes[url]; + if (route === undefined) throw new Error(`unstubbed URL: ${url}`); + if (typeof route === "function") return route(); + const { status = 200, body } = route; + return { + ok: status >= 200 && status < 300, + status, + json: async () => body, + }; + }; + return { fetchFn, calls }; +} + +const enabledBody = { settings: { mcpGatewayPluginEnabled: true } }; +const disabledBody = { settings: { mcpGatewayPluginEnabled: false } }; test("resolveAgentGuardCredentials falls back when JFROG_URL is empty", () => { const creds = resolveAgentGuardCredentials({ @@ -57,3 +92,183 @@ test("resolveAgentGuardCredentials prefers non-empty JFROG_* over JF_*", () => { assert.equal(creds?.baseUrl, "https://primary.jfrog.io"); assert.equal(creds?.token, "primary-tok"); }); + +// ---- /bridge-client fallback (self-hosted JPDs) ---- + +test("isGatewayPluginEnabled does not retry when the root path answers", async () => { + const { fetchFn, calls } = stubFetch({ [ROOT_URL]: { body: enabledBody } }); + const result = await isGatewayPluginEnabled(ROOT, "tok", { fetchFn }); + assert.deepEqual(result, { ok: true }); + assert.deepEqual(calls, [ROOT_URL]); +}); + +test("isGatewayPluginEnabled retries behind /bridge-client on a root 404", async () => { + const { fetchFn, calls } = stubFetch({ + [ROOT_URL]: { status: 404 }, + [BRIDGE_URL]: { body: enabledBody }, + }); + const result = await isGatewayPluginEnabled(ROOT, "tok", { fetchFn }); + assert.deepEqual(result, { ok: true }); + assert.deepEqual(calls, [ROOT_URL, BRIDGE_URL]); +}); + +test("isGatewayPluginEnabled surfaces a registry-off answer from /bridge-client", async () => { + const { fetchFn } = stubFetch({ + [ROOT_URL]: { status: 404 }, + [BRIDGE_URL]: { body: disabledBody }, + }); + const result = await isGatewayPluginEnabled(ROOT, "tok", { fetchFn }); + assert.equal(result.registryOff, true); + assert.equal(result.ok, false); +}); + +test("isGatewayPluginEnabled keeps the root 404 when /bridge-client 404s too", async () => { + const { fetchFn, calls } = stubFetch({ + [ROOT_URL]: { status: 404 }, + [BRIDGE_URL]: { status: 404 }, + }); + const result = await isGatewayPluginEnabled(ROOT, "tok", { fetchFn }); + assert.deepEqual(result, { + ok: false, + reason: "settings endpoint returned HTTP 404", + }); + assert.deepEqual(calls, [ROOT_URL, BRIDGE_URL]); +}); + +test("isGatewayPluginEnabled keeps the root 404 when /bridge-client is inconclusive", async () => { + const inconclusive = [ + { status: 401 }, + { status: 500 }, + { body: { settings: { mcpGatewayPluginEnabled: "yes" } } }, + () => { + throw new Error("ECONNREFUSED"); + }, + ]; + for (const bridge of inconclusive) { + const { fetchFn } = stubFetch({ + [ROOT_URL]: { status: 404 }, + [BRIDGE_URL]: bridge, + }); + const result = await isGatewayPluginEnabled(ROOT, "tok", { fetchFn }); + assert.deepEqual(result, { + ok: false, + reason: "settings endpoint returned HTTP 404", + }); + } +}); + +test("isGatewayPluginEnabled never retries a non-404 failure", async () => { + for (const status of [401, 403, 500, 502]) { + const { fetchFn, calls } = stubFetch({ [ROOT_URL]: { status } }); + const result = await isGatewayPluginEnabled(ROOT, "tok", { fetchFn }); + assert.deepEqual(result, { + ok: false, + reason: `settings endpoint returned HTTP ${status}`, + }); + assert.deepEqual(calls, [ROOT_URL], `status ${status} must not retry`); + } +}); + +test("isGatewayPluginEnabled never retries an unreachable root", async () => { + const { fetchFn, calls } = stubFetch({ + [ROOT_URL]: () => { + throw new Error("ENOTFOUND"); + }, + }); + const result = await isGatewayPluginEnabled(ROOT, "tok", { fetchFn }); + assert.deepEqual(result, { + ok: false, + reason: "settings endpoint unreachable (ENOTFOUND)", + }); + assert.deepEqual(calls, [ROOT_URL]); +}); + +test("the /bridge-client retry gets its own timeout budget", async () => { + // The root attempt outlives its full budget before 404-ing. If the retry + // reused that AbortController it would start already-aborted and report a + // timeout instead of the bridge path's real answer. + const timeoutMs = 20; + const signals = []; + const fetchFn = async (url, { signal }) => { + signals.push(signal); + if (url === ROOT_URL) { + await new Promise((resolve) => setTimeout(resolve, timeoutMs * 3)); + return { ok: false, status: 404, json: async () => ({}) }; + } + assert.equal(signal.aborted, false, "retry received a spent signal"); + return { ok: true, status: 200, json: async () => enabledBody }; + }; + const result = await isGatewayPluginEnabled(ROOT, "tok", { fetchFn, timeoutMs }); + assert.deepEqual(result, { ok: true }); + assert.equal(signals.length, 2); + assert.notEqual(signals[0], signals[1]); +}); + +test("a /bridge-client retry that times out leaves the root 404 verdict", async () => { + const fetchFn = (url, { signal }) => + new Promise((resolve, reject) => { + if (url === ROOT_URL) { + resolve({ ok: false, status: 404, json: async () => ({}) }); + return; + } + signal.addEventListener("abort", () => { + reject(Object.assign(new Error("aborted"), { name: "AbortError" })); + }); + }); + const result = await isGatewayPluginEnabled(ROOT, "tok", { fetchFn, timeoutMs: 20 }); + assert.deepEqual(result, { + ok: false, + reason: "settings endpoint returned HTTP 404", + }); +}); + +test("the /artifactory suffix is stripped before both attempts", async () => { + const { fetchFn, calls } = stubFetch({ + [ROOT_URL]: { status: 404 }, + [BRIDGE_URL]: { body: enabledBody }, + }); + const result = await isGatewayPluginEnabled(`${ROOT}/artifactory/`, "tok", { + fetchFn, + }); + assert.deepEqual(result, { ok: true }); + assert.deepEqual(calls, [ROOT_URL, BRIDGE_URL]); +}); + +// ---- exit-code contract through the public entry point ---- + +test("runAgentGuardCheck maps a /bridge-client hit to EXIT_ENABLED", async () => { + const { fetchFn } = stubFetch({ + [ROOT_URL]: { status: 404 }, + [BRIDGE_URL]: { body: enabledBody }, + }); + const result = await runAgentGuardCheck({ + env: { JFROG_URL: ROOT, JFROG_ACCESS_TOKEN: "tok" }, + fetchFn, + }); + assert.equal(result.code, EXIT_ENABLED); +}); + +test("runAgentGuardCheck maps a /bridge-client registry-off to EXIT_REGISTRY_DISABLED", async () => { + const { fetchFn } = stubFetch({ + [ROOT_URL]: { status: 404 }, + [BRIDGE_URL]: { body: disabledBody }, + }); + const result = await runAgentGuardCheck({ + env: { JFROG_URL: ROOT, JFROG_ACCESS_TOKEN: "tok" }, + fetchFn, + }); + assert.equal(result.code, EXIT_REGISTRY_DISABLED); +}); + +test("runAgentGuardCheck still reports an all-404 platform as EXIT_DISABLED", async () => { + const { fetchFn } = stubFetch({ + [ROOT_URL]: { status: 404 }, + [BRIDGE_URL]: { status: 404 }, + }); + const result = await runAgentGuardCheck({ + env: { JFROG_URL: ROOT, JFROG_ACCESS_TOKEN: "tok" }, + fetchFn, + }); + assert.equal(result.code, EXIT_DISABLED); + assert.equal(result.reason, "Disabled: settings endpoint returned HTTP 404"); +}); diff --git a/scripts/bridge-client-fallback.test.mjs b/scripts/bridge-client-fallback.test.mjs new file mode 100644 index 0000000..5f29689 --- /dev/null +++ b/scripts/bridge-client-fallback.test.mjs @@ -0,0 +1,374 @@ +// Copyright (c) JFrog Ltd. 2026 +// Licensed under the Apache License, Version 2.0 +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Integration coverage for the `/bridge-client` fallback in the two skill +// scripts that probe `/ml/core` (self-hosted JPDs do not serve it off the +// platform root). +// +// Neither script is unit-testable in-process: jfrog-agent-guard-check.mjs +// runs its gate at import time and exports nothing, and +// jfrog-detect-catalog-runtime.mjs shells out to `jf` for its config. Both +// are therefore driven the way a user drives them — spawned as a child +// process against a real localhost JPD stub, with a fake `jf` on PATH and +// HOME redirected so lib/jf.mjs's ~/.jfrog/bin self-heal cannot shadow it. + +import assert from "node:assert/strict"; +import { spawn } from "node:child_process"; +import { createServer } from "node:http"; +import { chmodSync, mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { after, describe, test } from "node:test"; +import { fileURLToPath } from "node:url"; + +const repoRoot = join(dirname(fileURLToPath(import.meta.url)), ".."); +const GUARD_SCRIPT = join( + repoRoot, + "skills/jfrog-mcp-management/scripts/jfrog-agent-guard-check.mjs", +); +const CATALOG_SCRIPT = join( + repoRoot, + "skills/jfrog-init/scripts/jfrog-detect-catalog-runtime.mjs", +); + +const SETTINGS_PATH = + "/ml/core/api/v1/administration/account-settings/mcp_gateway_plugin_enabled"; +const CATALOG_PATH = "/ml/core/api/v1/mcp-registry/ml-projects"; +const BRIDGE = "/bridge-client"; + +// A shell-script `jf` is the whole isolation strategy here; Windows would +// need a .cmd shim and a different homedir override. +const windows = process.platform === "win32"; + +const sandbox = mkdtempSync(join(tmpdir(), "jfrog-bridge-test-")); +const fakeJfDir = join(sandbox, "bin"); +const fakeHome = join(sandbox, "home"); +mkdirSync(fakeJfDir); +mkdirSync(fakeHome); + +// ---- localhost JPD stub ---- + +// Starts a server whose `routes` map a full request path (query stripped) to +// a handler. Records every {path, authed} pair so a test can prove the SaaS +// case never touches `/bridge-client`. +async function startJpd(routes) { + const requests = []; + const server = createServer((req, res) => { + const path = req.url.split("?")[0]; + requests.push({ path, authed: Boolean(req.headers.authorization) }); + const route = routes[path]; + if (!route) { + res.writeHead(404).end("not found"); + return; + } + route(req, res); + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const url = `http://127.0.0.1:${server.address().port}`; + const close = () => + new Promise((resolve) => { + // A child's keep-alive socket can outlive the child briefly; without + // this, close() waits on it and the suite hangs. + server.closeAllConnections(); + server.close(resolve); + }); + return { url, requests, close }; +} + +const json = (status, body) => (_req, res) => { + res.writeHead(status, { "content-type": "application/json" }); + res.end(JSON.stringify(body)); +}; +const status = (code) => (_req, res) => res.writeHead(code).end(); +// Splits one path between the anonymous probe and the authenticated call, +// which is how a JPD that answers 401 anonymously at the root path still +// 404s there once credentials are sent. +const byAuth = (anon, authed) => (req, res) => + (req.headers.authorization ? authed : anon)(req, res); + +// ---- child-process helpers ---- + +function run(script, { args = [], env = {} } = {}) { + return new Promise((resolve) => { + const child = spawn(process.execPath, [script, ...args], { + env: { + PATH: `${fakeJfDir}:${process.env.PATH}`, + HOME: fakeHome, + ...env, + }, + stdio: ["ignore", "pipe", "pipe"], + }); + let stdout = ""; + let stderr = ""; + child.stdout.on("data", (chunk) => (stdout += chunk)); + child.stderr.on("data", (chunk) => (stderr += chunk)); + child.on("close", (code) => resolve({ code, stdout, stderr })); + }); +} + +// Writes a `jf` that answers the three subcommands lib/jf.mjs uses, wired to +// `url`. Rewritten per test because each one points at a different port. +function writeFakeJf(url) { + const cfg = Buffer.from( + JSON.stringify({ serverId: "test", url, accessToken: "tok" }), + ).toString("base64"); + const configShow = JSON.stringify([ + { serverId: "test", url, isDefault: true }, + ]); + writeFileSync( + join(fakeJfDir, "jf"), + [ + "#!/bin/sh", + 'if [ "$1" = "--version" ]; then echo "jf version 2.0.0"; exit 0; fi', + `if [ "$1" = "config" ] && [ "$2" = "show" ]; then echo '${configShow}'; exit 0; fi`, + `if [ "$1" = "config" ] && [ "$2" = "export" ]; then echo '${cfg}'; exit 0; fi`, + 'echo "fake jf: unsupported: $@" >&2', + "exit 1", + "", + ].join("\n"), + ); + chmodSync(join(fakeJfDir, "jf"), 0o755); +} + +async function runCatalog(routes) { + const jpd = await startJpd(routes); + writeFakeJf(jpd.url); + try { + const result = await run(CATALOG_SCRIPT); + return { ...result, ...jpd, report: JSON.parse(result.stdout.trim()) }; + } finally { + await jpd.close(); + } +} + +async function runGuard(routes) { + const jpd = await startJpd(routes); + try { + const result = await run(GUARD_SCRIPT, { + env: { JFROG_URL: jpd.url, JFROG_ACCESS_TOKEN: "tok" }, + }); + return { ...result, ...jpd }; + } finally { + await jpd.close(); + } +} + +after(() => { + // The sandbox lives under the OS temp dir; leaving it is harmless and + // avoids a recursive rm racing a still-exiting child on slow CI. +}); + +describe("jfrog-agent-guard-check.mjs", { skip: windows }, () => { + const ENABLED = { settings: { mcpGatewayPluginEnabled: true } }; + const DISABLED = { settings: { mcpGatewayPluginEnabled: false } }; + + test("SaaS: root answers, /bridge-client is never probed", async () => { + const { code, stdout, requests } = await runGuard({ + [SETTINGS_PATH]: json(200, ENABLED), + }); + assert.equal(code, 0); + assert.match(stdout, /^Enabled:/); + assert.deepEqual( + requests.map((r) => r.path), + [SETTINGS_PATH], + ); + }); + + test("self-hosted: a root 404 is retried behind /bridge-client", async () => { + const { code, stdout, requests } = await runGuard({ + [BRIDGE + SETTINGS_PATH]: json(200, ENABLED), + }); + assert.equal(code, 0); + assert.match(stdout, /^Enabled:/); + assert.deepEqual( + requests.map((r) => r.path), + [SETTINGS_PATH, BRIDGE + SETTINGS_PATH], + ); + }); + + test("self-hosted: a registry turned off behind /bridge-client exits 2", async () => { + const { code, stdout } = await runGuard({ + [BRIDGE + SETTINGS_PATH]: json(200, DISABLED), + }); + assert.equal(code, 2); + assert.match(stdout, /^RegistryDisabled:/); + }); + + test("neither path hosts the setting: unchanged Unknown + exit 1", async () => { + const { code, stdout, requests } = await runGuard({}); + assert.equal(code, 1); + assert.equal(stdout.trim(), "Unknown: settings endpoint returned HTTP 404"); + assert.equal(requests.length, 2); + }); + + test("a 401 at the root is not retried and keeps its reason", async () => { + const { code, stdout, requests } = await runGuard({ + [SETTINGS_PATH]: status(401), + [BRIDGE + SETTINGS_PATH]: json(200, ENABLED), + }); + assert.equal(code, 1); + assert.equal(stdout.trim(), "Unknown: settings endpoint returned HTTP 401"); + assert.deepEqual( + requests.map((r) => r.path), + [SETTINGS_PATH], + ); + }); + + test("an inconclusive /bridge-client reply keeps the root 404 verdict", async () => { + const { code, stdout } = await runGuard({ + [BRIDGE + SETTINGS_PATH]: status(500), + }); + assert.equal(code, 1); + assert.equal(stdout.trim(), "Unknown: settings endpoint returned HTTP 404"); + }); +}); + +describe("jfrog-detect-catalog-runtime.mjs", { skip: windows }, () => { + const CATALOG_BODY = { projectKeys: [] }; + + test("SaaS: root answers, /bridge-client is never probed", async () => { + const { code, report, requests } = await runCatalog({ + [CATALOG_PATH]: json(200, CATALOG_BODY), + }); + assert.equal(code, 0); + assert.equal(report.status, "green"); + assert.ok(requests.every((r) => !r.path.startsWith(BRIDGE))); + }); + + test("self-hosted: Part A resolves the prefix on a 404", async () => { + const { code, report, requests } = await runCatalog({ + [BRIDGE + CATALOG_PATH]: json(200, CATALOG_BODY), + }); + assert.equal(code, 0); + assert.equal(report.status, "green"); + // Anonymous root 404, anonymous bridge, then the authenticated bridge + // call — Part B must inherit Part A's resolved prefix, not re-probe root. + assert.deepEqual(requests, [ + { path: CATALOG_PATH, authed: false }, + { path: BRIDGE + CATALOG_PATH, authed: false }, + { path: BRIDGE + CATALOG_PATH, authed: true }, + ]); + }); + + test("self-hosted: Part B resolves it when the root 401s anonymously", async () => { + // Part A passes at the root path, so only the authenticated call can + // discover the /bridge-client layout. + const { code, report, requests } = await runCatalog({ + [CATALOG_PATH]: byAuth(status(401), status(404)), + [BRIDGE + CATALOG_PATH]: json(200, CATALOG_BODY), + }); + assert.equal(code, 0); + assert.equal(report.status, "green"); + assert.deepEqual(requests, [ + { path: CATALOG_PATH, authed: false }, + { path: CATALOG_PATH, authed: true }, + { path: BRIDGE + CATALOG_PATH, authed: true }, + ]); + }); + + test("self-hosted: a 403 behind /bridge-client is not_entitled, not red", async () => { + const { code, report } = await runCatalog({ + [CATALOG_PATH]: byAuth(status(401), status(404)), + [BRIDGE + CATALOG_PATH]: status(403), + }); + assert.equal(code, 4); + assert.equal(report.status, "not_entitled"); + }); + + test("neither path hosts the catalog: unchanged red + exit 1", async () => { + const { code, report, url, requests } = await runCatalog({}); + assert.equal(code, 1); + assert.equal(report.status, "red"); + // The detail must still name the ROOT endpoint, exactly as before. + assert.equal( + report.detail, + `catalog endpoint returned 404 at ${url}${CATALOG_PATH}?pageSize=1 — this JPD may not host the AI Catalog`, + ); + assert.deepEqual( + requests.map((r) => r.path), + [CATALOG_PATH, BRIDGE + CATALOG_PATH], + ); + }); + + test("an authenticated 404 on both paths stays red and names the root", async () => { + const { code, report, url, requests } = await runCatalog({ + [CATALOG_PATH]: byAuth(status(401), status(404)), + }); + assert.equal(code, 1); + assert.equal(report.status, "red"); + assert.equal( + report.detail, + `catalog endpoint returned 404 at ${url}${CATALOG_PATH}?pageSize=1 — this JPD may not host the AI Catalog`, + ); + assert.deepEqual(requests, [ + { path: CATALOG_PATH, authed: false }, + { path: CATALOG_PATH, authed: true }, + { path: BRIDGE + CATALOG_PATH, authed: true }, + ]); + }); + + test("a /bridge-client reply that is not a deployed catalog never wins", async () => { + // A proxy answering unknown paths with 400/501, or the bridge path itself + // erroring, must leave the root 404 verdict — turning the non-blocking + // exit 1 into an exit 3 would block the whole jfrog-init walk. + for (const bridge of [status(400), status(501), status(503)]) { + const { code, report } = await runCatalog({ + [BRIDGE + CATALOG_PATH]: bridge, + }); + assert.equal(code, 1); + assert.equal(report.status, "red"); + assert.match(report.detail, /may not host the AI Catalog/); + } + }); + + test("an authenticated /bridge-client 401 does not blame the credentials", async () => { + // A WAF that rejects the bearer on unknown paths would otherwise be + // reported as "credentials rejected" (exit 3) for creds that are fine. + const { code, report } = await runCatalog({ + [CATALOG_PATH]: byAuth(status(401), status(404)), + [BRIDGE + CATALOG_PATH]: status(401), + }); + assert.equal(code, 1); + assert.equal(report.status, "red"); + assert.match(report.detail, /may not host the AI Catalog/); + }); + + test("a captive 200 behind /bridge-client does not win either", async () => { + const { code, report } = await runCatalog({ + [CATALOG_PATH]: byAuth(status(401), status(404)), + [BRIDGE + CATALOG_PATH]: json(200, { login: "please" }), + }); + assert.equal(code, 1); + assert.equal(report.status, "red"); + assert.match(report.detail, /may not host the AI Catalog/); + }); + + test("a non-404 root failure is never retried", async () => { + const { code, report, requests } = await runCatalog({ + [CATALOG_PATH]: status(503), + [BRIDGE + CATALOG_PATH]: json(200, CATALOG_BODY), + }); + assert.equal(code, 1); + assert.equal(report.status, "red"); + assert.match(report.detail, /HTTP 503 \(server error\)/); + assert.deepEqual( + requests.map((r) => r.path), + [CATALOG_PATH], + ); + }); + + test("an unreachable JPD is reported as such, with no fallback probe", async () => { + // Bind a port, learn it, then release it: nothing is listening, so the + // anonymous probe fails with "000" rather than 404. + const jpd = await startJpd({}); + const { url } = jpd; + await jpd.close(); + writeFakeJf(url); + const { code, stdout } = await run(CATALOG_SCRIPT); + const report = JSON.parse(stdout.trim()); + assert.equal(code, 1); + assert.equal(report.status, "red"); + assert.match(report.detail, /connection failed/); + }); +}); diff --git a/skills/jfrog-init/scripts/jfrog-detect-catalog-runtime.mjs b/skills/jfrog-init/scripts/jfrog-detect-catalog-runtime.mjs index 066124d..5a39f55 100755 --- a/skills/jfrog-init/scripts/jfrog-detect-catalog-runtime.mjs +++ b/skills/jfrog-init/scripts/jfrog-detect-catalog-runtime.mjs @@ -2,7 +2,9 @@ // AI Catalog readiness check for the current user + JPD, against // /ml/core/api/v1/mcp-registry/ml-projects?pageSize=1 — this skill // does NOT read a separate JFROG_PLATFORM_URL / JFROG_URL env var; the -// source of truth is what `jf` itself is configured with. +// source of truth is what `jf` itself is configured with. A 404 there is +// retried once behind /bridge-client, where self-hosted JPDs serve +// the same API (see BRIDGE_CLIENT_PREFIX below). // // Two sub-checks, both must pass (mirrors jfrog-detect-server-ping.mjs's // reachability/credentials split): @@ -51,6 +53,17 @@ import { emit, isMainModule, resolveCreds, urlForServer, normalizeJpdUrl, authed import { resolveServerOrEmit } from "./jfrog-resolve-jf-server.mjs"; const CATALOG_PATH = "/ml/core/api/v1/mcp-registry/ml-projects?pageSize=1"; +// Self-hosted JPDs serve the same API behind `/bridge-client`. Tried ONLY +// after the root path 404s, so SaaS still costs one request per part. +const BRIDGE_CLIENT_PREFIX = "/bridge-client"; + +// The anonymous codes that let Part A proceed to Part B. Shared by the Part A +// gate and its `/bridge-client` fallback so the two can't drift. +const partAReachable = (code) => /^[23]/.test(code) || ["401", "403", "405", "406"].includes(code); + +// A 2xx alone isn't proof this is the AI Catalog — a captive portal or +// misrouted network can answer 200 too. Require the expected shape. +const looksLikeCatalog = (body) => Boolean(body) && typeof body === "object" && Array.isArray(body.projectKeys); // Shared by both Part A (anonymous) and Part B (authenticated) below — each // probe can independently come back "000" (connection failed) or "404" @@ -95,10 +108,28 @@ export async function detectCatalogRuntime(serverIdArg) { emit({ check: "catalog", status: "red", detail: `no url found in jf config for server-id=${serverId}` }); return 1; } - const endpoint = `${url}${CATALOG_PATH}`; + // Which prefix this JPD serves the catalog under: "" for SaaS, + // `/bridge-client` for self-hosted. Part A resolves it when it can; Part B + // otherwise, since a JPD answering 401 anonymously at the root passes Part + // A without revealing that the authenticated call 404s there. + let prefix = ""; + let endpoint = `${url}${CATALOG_PATH}`; // ---------- Part A: anonymous reachability ---------- - const anonCode = await anonymousFetchStatus(endpoint); + let anonCode = await anonymousFetchStatus(endpoint); + + // Adopt the fallback only when it evidences a deployed catalog. Anything + // else (a proxy's 400/501, a 5xx, a failed connection) leaves the root 404 + // verdict — and its non-blocking exit 1 — exactly as it was. + if (anonCode === "404") { + const bridgeEndpoint = `${url}${BRIDGE_CLIENT_PREFIX}${CATALOG_PATH}`; + const bridgeCode = await anonymousFetchStatus(bridgeEndpoint); + if (partAReachable(bridgeCode)) { + prefix = BRIDGE_CLIENT_PREFIX; + endpoint = bridgeEndpoint; + anonCode = bridgeCode; + } + } if (anonCode === "000") { return emitUnreachable(endpoint); @@ -109,7 +140,7 @@ export async function detectCatalogRuntime(serverIdArg) { if (/^5/.test(anonCode)) { return emitServerError(endpoint, anonCode); } - if (!/^2/.test(anonCode) && !/^3/.test(anonCode) && !["401", "403", "405", "406"].includes(anonCode)) { + if (!partAReachable(anonCode)) { emit({ check: "catalog", status: "error", detail: `catalog probe returned unexpected HTTP ${anonCode} at ${endpoint}` }); return 3; } @@ -126,19 +157,32 @@ export async function detectCatalogRuntime(serverIdArg) { return 1; } - const { code, body } = await authedFetch(creds, CATALOG_PATH); - const httpCode = code === 0 ? "000" : String(code); + const authed = await authedFetch(creds, `${prefix}${CATALOG_PATH}`); + let body = authed.body; + let httpCode = authed.code === 0 ? "000" : String(authed.code); - // A 2xx status alone isn't proof this is really the AI Catalog endpoint — - // a captive portal or misrouted network can also answer 200. Require the - // expected shape (an object with a `projectKeys` array) too. - const looksLikeCatalog = body && typeof body === "object" && Array.isArray(body.projectKeys); + // Part A passed at the root (e.g. an anonymous 401) but the authenticated + // call 404s there — the same self-hosted layout, one part later. Adopted + // only on proof of a catalog: a 2xx of the right shape, or the 403 that + // means "deployed, this user isn't entitled". A WAF's 401 or a captive 200 + // must NOT win, or a fine set of credentials gets reported as rejected. + // (authedFetch builds its own base from `jf config export`; `endpoint` comes + // from `jf config show`. Only the prefix is shared, so re-derive it here.) + if (httpCode === "404" && !prefix) { + const retry = await authedFetch(creds, `${BRIDGE_CLIENT_PREFIX}${CATALOG_PATH}`); + const retryCode = retry.code === 0 ? "000" : String(retry.code); + if ((/^2/.test(retryCode) && looksLikeCatalog(retry.body)) || retryCode === "403") { + endpoint = `${url}${BRIDGE_CLIENT_PREFIX}${CATALOG_PATH}`; + body = retry.body; + httpCode = retryCode; + } + } - if (httpCode.startsWith("2") && looksLikeCatalog) { + if (httpCode.startsWith("2") && looksLikeCatalog(body)) { emit({ check: "catalog", status: "green", detail: `catalog reachable, user entitled (HTTP ${httpCode})` }); return 0; } - if (httpCode.startsWith("2") && !looksLikeCatalog) { + if (httpCode.startsWith("2") && !looksLikeCatalog(body)) { emit({ check: "catalog", status: "error", detail: `got HTTP ${httpCode} from ${endpoint} but the response wasn't the expected AI Catalog shape — this may not be the JPD's real endpoint (captive portal / proxy?)` }); return 3; } diff --git a/skills/jfrog-mcp-management/scripts/jfrog-agent-guard-check.mjs b/skills/jfrog-mcp-management/scripts/jfrog-agent-guard-check.mjs index ed84da9..98c5c75 100644 --- a/skills/jfrog-mcp-management/scripts/jfrog-agent-guard-check.mjs +++ b/skills/jfrog-mcp-management/scripts/jfrog-agent-guard-check.mjs @@ -23,6 +23,9 @@ import process from "node:process"; const SETTINGS_PATH = "/ml/core/api/v1/administration/account-settings/mcp_gateway_plugin_enabled"; +// Self-hosted JPDs serve the same API behind `/bridge-client`. Tried ONLY +// after the root path 404s, so SaaS still costs exactly one request. +const BRIDGE_CLIENT_PREFIX = "/bridge-client"; const REQUEST_TIMEOUT_MS = 5000; const debugEnabled = process.env.JF_AGENT_GUARD_DEBUG === "true"; @@ -173,14 +176,39 @@ function resolveFromCliConfig(serverId) { return { baseUrl, token, source: `JF CLI config (server '${id}')` }; } +/** Drops the internal `notFound` marker from a fetchSetting() result. */ +function strip({ notFound, ...result }) { + return result; +} + async function isGatewayPluginEnabled(baseUrl, token) { // Normalize to the platform root: drop trailing slashes and a trailing // `/artifactory` segment. Users commonly export JFROG_URL as // `https://myco.jfrog.io/artifactory`, but the settings path lives under // `/ml/core` off the platform root — without this, Path A would build - // `.../artifactory/ml/core/...` and 404 into a false "disabled" (exit 1). + // `.../artifactory/ml/core/...` and 404 into a false "unknown" (exit 1). const root = baseUrl.replace(/\/+$/, "").replace(/\/artifactory$/, ""); - const url = root + SETTINGS_PATH; + + const rootResult = await fetchSetting(root + SETTINGS_PATH, token); + if (!rootResult.notFound) return strip(rootResult); + + // Root 404 -> possibly self-hosted. Each attempt gets its OWN timeout + // budget: a reused AbortController would start the retry already spent. + debug(`Root ${SETTINGS_PATH} returned 404; retrying behind ${BRIDGE_CLIENT_PREFIX}.`); + const bridgeResult = await fetchSetting( + root + BRIDGE_CLIENT_PREFIX + SETTINGS_PATH, + token, + ); + // The fallback may only IMPROVE the outcome — anything inconclusive keeps + // the root verdict, and with it every pre-existing reason and exit code. + if (bridgeResult.ok || bridgeResult.registryOff) return strip(bridgeResult); + return strip(rootResult); +} + +// One HTTP attempt against a fully-built settings URL. `notFound` marks the +// 404 that triggers the `/bridge-client` retry; callers strip it before +// returning so the result shape main() sees is unchanged. +async function fetchSetting(url, token) { debug(`Fetching gateway plugin setting from ${url}`); // Trade-off: we use a direct fetch() rather than `jf api` (the pattern other @@ -210,6 +238,7 @@ async function isGatewayPluginEnabled(baseUrl, token) { // claiming disabled. Only HTTP 200 + value:false is "disabled". return { ok: false, + notFound: response.status === 404, reason: `settings endpoint returned HTTP ${response.status}`, }; } From 9e3b7987b0487e58098ab63212c737665b34e6d1 Mon Sep 17 00:00:00 2001 From: davida-jfrog Date: Thu, 3 Sep 2026 20:59:30 +0300 Subject: [PATCH 2/2] =?UTF-8?q?AX-2205:=20address=20review=20=E2=80=94=20o?= =?UTF-8?q?ne-line=20comments,=20drop=20unread=20endpoint=20reassignment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5 (1M context) --- modules/core/agent-guard-check.mjs | 3 +-- skills/jfrog-init/scripts/jfrog-detect-catalog-runtime.mjs | 5 ++--- .../jfrog-mcp-management/scripts/jfrog-agent-guard-check.mjs | 3 +-- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/modules/core/agent-guard-check.mjs b/modules/core/agent-guard-check.mjs index db533d8..5d401dd 100644 --- a/modules/core/agent-guard-check.mjs +++ b/modules/core/agent-guard-check.mjs @@ -210,8 +210,7 @@ export async function isGatewayPluginEnabled(baseUrl, token, opts = {}) { token, { debug, fetchFn, timeoutMs }, ); - // The fallback may only IMPROVE the outcome — anything inconclusive keeps - // the root verdict, and with it every pre-existing reason and exit code. + // Bridge may only UPGRADE the verdict; anything else keeps the root result. if (bridgeResult.ok || bridgeResult.registryOff) return strip(bridgeResult); return strip(rootResult); } diff --git a/skills/jfrog-init/scripts/jfrog-detect-catalog-runtime.mjs b/skills/jfrog-init/scripts/jfrog-detect-catalog-runtime.mjs index 5a39f55..961bc30 100755 --- a/skills/jfrog-init/scripts/jfrog-detect-catalog-runtime.mjs +++ b/skills/jfrog-init/scripts/jfrog-detect-catalog-runtime.mjs @@ -166,13 +166,12 @@ export async function detectCatalogRuntime(serverIdArg) { // only on proof of a catalog: a 2xx of the right shape, or the 403 that // means "deployed, this user isn't entitled". A WAF's 401 or a captive 200 // must NOT win, or a fine set of credentials gets reported as rejected. - // (authedFetch builds its own base from `jf config export`; `endpoint` comes - // from `jf config show`. Only the prefix is shared, so re-derive it here.) + // Adopting leaves only the green and not_entitled branches reachable, and + // neither reports `endpoint`, so it stays the root path it was built from. if (httpCode === "404" && !prefix) { const retry = await authedFetch(creds, `${BRIDGE_CLIENT_PREFIX}${CATALOG_PATH}`); const retryCode = retry.code === 0 ? "000" : String(retry.code); if ((/^2/.test(retryCode) && looksLikeCatalog(retry.body)) || retryCode === "403") { - endpoint = `${url}${BRIDGE_CLIENT_PREFIX}${CATALOG_PATH}`; body = retry.body; httpCode = retryCode; } diff --git a/skills/jfrog-mcp-management/scripts/jfrog-agent-guard-check.mjs b/skills/jfrog-mcp-management/scripts/jfrog-agent-guard-check.mjs index 98c5c75..ff8726f 100644 --- a/skills/jfrog-mcp-management/scripts/jfrog-agent-guard-check.mjs +++ b/skills/jfrog-mcp-management/scripts/jfrog-agent-guard-check.mjs @@ -199,8 +199,7 @@ async function isGatewayPluginEnabled(baseUrl, token) { root + BRIDGE_CLIENT_PREFIX + SETTINGS_PATH, token, ); - // The fallback may only IMPROVE the outcome — anything inconclusive keeps - // the root verdict, and with it every pre-existing reason and exit code. + // Bridge may only UPGRADE the verdict; anything else keeps the root result. if (bridgeResult.ok || bridgeResult.registryOff) return strip(bridgeResult); return strip(rootResult); }