diff --git a/package-lock.json b/package-lock.json index 0bf0456..8bc2c93 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6601,9 +6601,10 @@ }, "packages/explicabl": { "name": "@gatewaystack/explicabl", - "version": "0.0.7", + "version": "0.0.8", "license": "MIT", "dependencies": { + "@gatewaystack/request-context": "^0.0.6", "express": "^4.22.0", "express-rate-limit": "^8.2.2" }, diff --git a/packages/explicabl-core/src/reporting/dcrLastSeen.js b/packages/explicabl-core/src/reporting/dcrLastSeen.js deleted file mode 100644 index 3918c74..0000000 --- a/packages/explicabl-core/src/reporting/dcrLastSeen.js +++ /dev/null @@ -1 +0,0 @@ -"use strict"; diff --git a/packages/explicabl-core/src/reporting/saveReport.js b/packages/explicabl-core/src/reporting/saveReport.js deleted file mode 100644 index 3e90f4a..0000000 --- a/packages/explicabl-core/src/reporting/saveReport.js +++ /dev/null @@ -1,16 +0,0 @@ -import fs from "node:fs"; -const out = { - version: "0.1.0", - spec: "Apps SDK OAuth 2.1 + MCP Authorization (subset)", - categories: { - pkce: "pass", - jwt_verify: "pass", - scope_enforcement: "pass", - allowlist: "pass", - expiry: "pass" - }, - timestamp: new Date().toISOString() -}; -fs.mkdirSync("docs", { recursive: true }); -fs.writeFileSync("docs/conformance.json", JSON.stringify(out, null, 2)); -console.log("[conformance] wrote docs/conformance.json"); diff --git a/packages/explicabl/__tests__/hardening.test.ts b/packages/explicabl/__tests__/hardening.test.ts new file mode 100644 index 0000000..8c26a66 --- /dev/null +++ b/packages/explicabl/__tests__/hardening.test.ts @@ -0,0 +1,77 @@ +// Enforcement-core hardening (#41) — explicabl. +// +// - audit events must carry identity from the GatewayContext (ALS), which the +// old code never read (it looked at res.locals.gatewayContext, unset). +// - aborted requests (connection close, no `finish`) must still emit exactly +// one audit event. +// - the /webhooks/auth0 mount must call the handler FACTORY (the old code +// mounted the factory itself, so requests hung forever). + +import { describe, it, expect, vi } from "vitest"; +import { EventEmitter } from "node:events"; +import express from "express"; +import request from "supertest"; +import { runWithGatewayContext } from "@gatewaystack/request-context"; +import { explicablLoggingMiddleware, explicablRouter } from "../src/index.ts"; +import type { ExplicablEvent } from "../src/index.ts"; + +// Minimal req/res doubles so we can drive `finish` / `close` directly. +function fakeReqRes() { + const req: any = { method: "GET", path: "/x", headers: {} }; + const res: any = new EventEmitter(); + res.locals = {}; + res.statusCode = 200; + return { req, res }; +} + +describe("explicablLoggingMiddleware: identity comes from the GatewayContext", () => { + it("reads identity from the ALS context, not res.locals", async () => { + const logger = vi.fn(); + const { req, res } = fakeReqRes(); + + runWithGatewayContext({ identity: { sub: "auth0|abc", source: "auth0" } }, () => { + explicablLoggingMiddleware(logger)(req, res, () => {}); + }); + res.emit("finish"); + + expect(logger).toHaveBeenCalledTimes(1); + const event = logger.mock.calls[0][0] as ExplicablEvent; + expect((event.context as any)?.identity?.sub).toBe("auth0|abc"); + }); +}); + +describe("explicablLoggingMiddleware: aborted requests still audit, exactly once", () => { + it("emits one event on `close` when the request was aborted (no finish)", () => { + const logger = vi.fn(); + const { req, res } = fakeReqRes(); + explicablLoggingMiddleware(logger)(req, res, () => {}); + + res.emit("close"); // client aborted; `finish` never fired + expect(logger).toHaveBeenCalledTimes(1); + }); + + it("emits exactly one event when both finish and close fire", () => { + const logger = vi.fn(); + const { req, res } = fakeReqRes(); + explicablLoggingMiddleware(logger)(req, res, () => {}); + + res.emit("finish"); + res.emit("close"); + expect(logger).toHaveBeenCalledTimes(1); + }); +}); + +describe("explicablRouter: /webhooks/auth0 is wired (does not hang)", () => { + it("responds to the webhook route instead of mounting the factory", async () => { + const app = express(); + app.use(express.json()); + app.use(explicablRouter(process.env)); + + // Without LOG_WEBHOOK_SECRET the handler refuses (503/401) — the point is + // that it RESPONDS at all. The old code mounted the factory as middleware, + // so this request never got a response. + const res = await request(app).post("/webhooks/auth0").send({}).timeout(3000); + expect(res.status).toBeGreaterThanOrEqual(400); + expect(res.status).toBeLessThan(600); + }); +}); diff --git a/packages/explicabl/package.json b/packages/explicabl/package.json index 4a360ce..c2c84b5 100644 --- a/packages/explicabl/package.json +++ b/packages/explicabl/package.json @@ -19,6 +19,7 @@ "prepublishOnly": "npm run build" }, "dependencies": { + "@gatewaystack/request-context": "^0.0.6", "express": "^4.22.0", "express-rate-limit": "^8.2.2" }, diff --git a/packages/explicabl/src/health.js b/packages/explicabl/src/health.js deleted file mode 100644 index 1642e6c..0000000 --- a/packages/explicabl/src/health.js +++ /dev/null @@ -1,55 +0,0 @@ -import { Router } from "express"; -import fetch from "node-fetch"; -export function healthRoutes(env) { - const r = Router(); - r.get("/", (_req, res) => { - res.json({ - ok: true, - mode: env.MODE || "firebase", - version: process.env.COMMIT_SHA || "dev", - time: new Date().toISOString(), - }); - }); - r.get("/auth0", async (_req, res) => { - const issuer = env.AUTH0_ISSUER; - const audience = env.AUTH0_AUDIENCE; - const jwksUri = env.AUTH0_JWKS_URI; - const out = { issuer, audience, jwksUri, jwksReachable: false }; - try { - const resp = await fetch(jwksUri); - out.jwksReachable = resp.ok; - if (!resp.ok) - out.jwksError = `HTTP ${resp.status}`; - } - catch (e) { - out.jwksError = e?.message || String(e); - } - if (env.MGMT_DOMAIN && env.MGMT_CLIENT_ID && env.MGMT_CLIENT_SECRET) { - out.managementConfigured = true; - try { - const tokenResp = await fetch(`https://${env.MGMT_DOMAIN}/oauth/token`, { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ - client_id: env.MGMT_CLIENT_ID, - client_secret: env.MGMT_CLIENT_SECRET, - audience: `https://${env.MGMT_DOMAIN}/api/v2/`, - grant_type: "client_credentials", - }), - }); - out.managementTokenOk = tokenResp.ok; - if (!tokenResp.ok) - out.managementError = `HTTP ${tokenResp.status}`; - } - catch (e) { - out.managementTokenOk = false; - out.managementError = e?.message || String(e); - } - } - else { - out.managementConfigured = false; - } - res.json(out); - }); - return r; -} diff --git a/packages/explicabl/src/index.d.ts b/packages/explicabl/src/index.d.ts deleted file mode 100644 index 56c1632..0000000 --- a/packages/explicabl/src/index.d.ts +++ /dev/null @@ -1,9 +0,0 @@ -import { type RequestHandler } from "express"; -export { healthRoutes } from "./health"; -export { auth0LogsWebhook } from "./webhooks/auth0LogWebhook"; -/** - * Combined router for the Explicabl layer: - * - health endpoints - * - logging/audit webhooks - */ -export declare function explicablRouter(env: NodeJS.ProcessEnv): RequestHandler; diff --git a/packages/explicabl/src/index.js b/packages/explicabl/src/index.js deleted file mode 100644 index 8b806dc..0000000 --- a/packages/explicabl/src/index.js +++ /dev/null @@ -1,19 +0,0 @@ -import { Router } from "express"; -import { healthRoutes } from "./health"; -import { auth0LogsWebhook } from "./webhooks/auth0LogWebhook"; -export { healthRoutes } from "./health"; -export { auth0LogsWebhook } from "./webhooks/auth0LogWebhook"; -/** - * Combined router for the Explicabl layer: - * - health endpoints - * - logging/audit webhooks - */ -export function explicablRouter(env) { - const r = Router(); - // Health routes (public) - r.use(healthRoutes(env)); - // Webhooks (auth0 logs, etc.) - r.use("/webhooks/auth0", auth0LogsWebhook); - // Important: cast router to RequestHandler - return r; -} diff --git a/packages/explicabl/src/index.ts b/packages/explicabl/src/index.ts index 2d818f0..b75b06e 100644 --- a/packages/explicabl/src/index.ts +++ b/packages/explicabl/src/index.ts @@ -7,6 +7,7 @@ import { type NextFunction, type RequestHandler, } from "express"; +import { getGatewayContext } from "@gatewaystack/request-context"; import { healthRoutes } from "./health.js"; import { auth0LogsWebhook } from "./webhooks/auth0LogWebhook.js"; @@ -87,27 +88,59 @@ export function createConsoleLogger( * It is intentionally defensive: if no context is present, it still logs * method/path/status/latency. */ +let warnedNoContext = false; + export function explicablLoggingMiddleware( logger: ExplicablLogger, ): RequestHandler { return (req: Request, res: Response, next: NextFunction) => { const startedAt = Date.now(); - res.on("finish", () => { - const latencyMs = Date.now() - startedAt; + // Capture the ambient GatewayContext reference now, while we are inside the + // request's async-local-storage scope. Later layers (identifiabl, etc.) + // mutate this same object in place, so the reference sees their updates — + // and reading it here means we still have it if ALS has unwound by the time + // the response settles. + const capturedContext = getGatewayContext(); + + // Emit exactly one event whether the response finishes normally or the + // connection is aborted. `finish` fires on a completed response; `close` + // fires on client abort/socket close and used to emit nothing at all — so + // aborted requests left no audit trail. The guard makes them mutually + // exclusive. + let emitted = false; + const emit = () => { + if (emitted) return; + emitted = true; - // Try a few common places where you might stash requestId/context. + const latencyMs = Date.now() - startedAt; const locals: any = res.locals ?? {}; const requestId: string | undefined = (locals.requestId as string | undefined) ?? (locals.reqId as string | undefined) ?? (req.headers["x-request-id"] as string | undefined); + // Identity lives in the GatewayContext (ALS), not res.locals — the old + // `res.locals.gatewayContext` read was always undefined, so every audit + // line logged `context: undefined`. Prefer the live context, fall back to + // the captured reference, then res.locals for back-compat. const context: unknown = + getGatewayContext() ?? + capturedContext ?? locals.gatewayContext ?? locals.context ?? undefined; + if (context === undefined && !warnedNoContext) { + warnedNoContext = true; + // eslint-disable-next-line no-console + console.warn( + "[explicabl] no GatewayContext found on the request — audit events " + + "will carry no identity. Ensure runWithGatewayContext() wraps the " + + "request before this middleware.", + ); + } + const event: ExplicablEvent = { ts: new Date().toISOString(), kind: "gateway.request", @@ -128,7 +161,10 @@ export function explicablLoggingMiddleware( // eslint-disable-next-line no-console console.error("[explicabl:logger_error]", err); } - }); + }; + + res.on("finish", emit); + res.on("close", emit); next(); }; @@ -149,12 +185,12 @@ export function explicablRouter(env: NodeJS.ProcessEnv): RequestHandler { // Health routes (public) r.use(healthRoutes(env) as unknown as RequestHandler); - // Webhooks (auth0 logs, etc.) - // NOTE: auth0LogsWebhook is already a RequestHandler in your current code, - // so we do NOT call it as a function here. - // NOTE: auth0LogsWebhook is already a RequestHandler in your current code, - // so we do NOT call it as a function here. - r.use("/webhooks/auth0", auth0LogsWebhook as unknown as RequestHandler); + // Webhooks (auth0 logs, etc.). + // auth0LogsWebhook is a FACTORY that returns the handler — it must be called. + // Mounting the factory itself made Express invoke it as (req, res, next); it + // returned a value and never called next(), so every request to this path + // hung forever and the webhook was effectively dead. + r.use("/webhooks/auth0", auth0LogsWebhook()); // Important: cast router to RequestHandler return r as unknown as RequestHandler; diff --git a/packages/explicabl/src/webhooks/auth0LogWebhook.js b/packages/explicabl/src/webhooks/auth0LogWebhook.js deleted file mode 100644 index 9606545..0000000 --- a/packages/explicabl/src/webhooks/auth0LogWebhook.js +++ /dev/null @@ -1,283 +0,0 @@ -// packages/explicabl-express/src/webhooks/auth0-log-webhook.ts -import express from "express"; -/** - * Env - * - MGMT_DOMAIN (e.g. "your-tenant.us.auth0.com") - * - MGMT_CLIENT_ID - * - MGMT_CLIENT_SECRET - * - LOG_WEBHOOK_SECRET (shared secret for Auth0 Log Streams → "Authorization: Bearer ") - * - GOOGLE_CONNECTION_NAME (defaults to "google-oauth2") - */ -const MGMT_DOMAIN = process.env.MGMT_DOMAIN; -const MGMT_CLIENT_ID = process.env.MGMT_CLIENT_ID; -const MGMT_CLIENT_SECRET = process.env.MGMT_CLIENT_SECRET; -const LOG_WEBHOOK_SECRET = process.env.LOG_WEBHOOK_SECRET || "dev-change-me"; -const GOOGLE_CONNECTION_NAME = process.env.GOOGLE_CONNECTION_NAME || "google-oauth2"; -// Optional: used to create a client grant with all tool scopes -const OAUTH_AUDIENCE = process.env.OAUTH_AUDIENCE; -const TOOL_SCOPES = (process.env.TOOL_SCOPES || process.env.REQUIRED_SCOPES || "") - .split(",") - .map((s) => s.trim()) - .filter(Boolean); -async function getMgmtToken() { - const r = await fetch(`https://${MGMT_DOMAIN}/oauth/token`, { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ - grant_type: "client_credentials", - client_id: MGMT_CLIENT_ID, - client_secret: MGMT_CLIENT_SECRET, - audience: `https://${MGMT_DOMAIN}/api/v2/` - }) - }); - if (!r.ok) - throw new Error(`mgmt_token_http_${r.status}`); - const j = (await r.json()); - if (!j?.access_token) - throw new Error("mgmt_token_missing_access_token"); - return j.access_token; -} -async function promoteClient(mgmtToken, clientId) { - const patchBody = { - app_type: "regular_web", - is_first_party: true, - token_endpoint_auth_method: "none", - grant_types: ["authorization_code", "refresh_token"] - }; - const r = await fetch(`https://${MGMT_DOMAIN}/api/v2/clients/${clientId}`, { - method: "PATCH", - headers: { "content-type": "application/json", authorization: `Bearer ${mgmtToken}` }, - body: JSON.stringify(patchBody) - }); - if (!r.ok) { - const txt = await r.text(); - throw new Error(`promote_http_${r.status}:${txt}`); - } -} -async function enableGoogleForClient(mgmtToken, clientId) { - // 1) find the connection id for google-oauth2 - const rc = await fetch(`https://${MGMT_DOMAIN}/api/v2/connections?name=${encodeURIComponent(GOOGLE_CONNECTION_NAME)}`, { headers: { authorization: `Bearer ${mgmtToken}` } }); - if (!rc.ok) - throw new Error(`conn_lookup_http_${rc.status}`); - const arr = (await rc.json()); - const conn = Array.isArray(arr) ? arr[0] : null; - if (!conn || !conn.id) - throw new Error(`connection_not_found:${GOOGLE_CONNECTION_NAME}`); - // 2) add clientId to enabled_clients (idempotent) - const enabled = new Set(Array.isArray(conn.enabled_clients) ? conn.enabled_clients : []); - enabled.add(clientId); - const rp = await fetch(`https://${MGMT_DOMAIN}/api/v2/connections/${conn.id}`, { - method: "PATCH", - headers: { "content-type": "application/json", authorization: `Bearer ${mgmtToken}` }, - body: JSON.stringify({ enabled_clients: Array.from(enabled) }) - }); - if (!rp.ok) { - const txt = await rp.text(); - throw new Error(`conn_patch_http_${rp.status}:${txt}`); - } -} -async function ensureClientGrant(mgmtToken, clientId, audience, scopes = []) { - const aud = (audience || "").trim(); - if (!aud || !scopes.length) - return; - const r = await fetch(`https://${MGMT_DOMAIN}/api/v2/client-grants`, { - method: "POST", - headers: { - "content-type": "application/json", - authorization: `Bearer ${mgmtToken}` - }, - body: JSON.stringify({ - client_id: clientId, - audience: aud, - scope: scopes - }) - }); - if (!r.ok) { - const txt = await r.text(); - throw new Error(`client_grant_http_${r.status}:${txt}`); - } - console.log("[dcr] created client grant", { clientId, audience: aud, scopes }); -} -// ---- log helpers ---- -function unwrap(ev) { - return (ev && typeof ev === "object" && ev.data && typeof ev.data === "object") ? ev.data : ev; -} -function evtPath(e) { - return String(e?.details?.request?.path || - e?.details?.request?.url || - e?.http?.request?.path || - "").toLowerCase(); -} -function isDcrEventRaw(ev) { - const e = unwrap(ev); - const type = String(e?.type || "").toLowerCase(); - const desc = String(e?.description || "").toLowerCase(); - const path = evtPath(e); - const method = String(e?.details?.request?.method || e?.http?.method || "").toUpperCase(); - return ((type === "sapi" && desc.includes("dynamic client registration")) || - path.includes("/oidc/register") || - (method === "POST" && (path === "/api/v2/clients" || path.endsWith("/api/v2/clients")))); -} -async function findNewestChatGPTClientId(mgmtToken) { - const url = `https://${MGMT_DOMAIN}/api/v2/clients` + - "?is_global=false&per_page=10&sort=created_at:-1&fields=client_id,name,created_at,app_type,grant_types,token_endpoint_auth_method&include_fields=true"; - const r = await fetch(url, { headers: { authorization: `Bearer ${mgmtToken}` } }); - if (!r.ok) { - const txt = await r.text(); - console.warn("[dcr] clients list failed", r.status, txt); - return null; - } - const arr = (await r.json()); - const now = Date.now(); - for (const c of arr) { - const name = String(c?.name || ""); - const createdAt = Date.parse(c?.created_at || ""); - const within5min = isFinite(createdAt) && (now - createdAt) < 5 * 60 * 1000; - const looksLikeDcr = name.toLowerCase().startsWith("chatgpt") || name.toLowerCase().includes("chat gpt"); - const publicPkce = (c?.token_endpoint_auth_method === "none") && - Array.isArray(c?.grant_types) && - c.grant_types.includes("authorization_code"); - if (within5min && looksLikeDcr && publicPkce && c?.client_id) { - return c.client_id; - } - } - return null; -} -function extractClientIdFromDcrRaw(ev) { - const e = unwrap(ev); - return (e?.details?.response?.body?.client_id ?? - e?.client_id ?? - e?.details?.request?.body?.client_id ?? - null); -} -// ---- handler ---- -async function handleAuth0LogWebhook(req, res) { - // Shared-secret auth - // const auth = req.header("authorization") || ""; - // if (auth !== `Bearer ${LOG_WEBHOOK_SECRET}`) { - // res.status(401).json({ ok: false, error: "unauthorized" }); - // return; - // } - // Shared-secret auth (supports either Authorization or X-Webhook-Secret) - const auth = req.header("authorization") || ""; - const xSecret = req.header("x-webhook-secret") || ""; - const ok = auth === `Bearer ${LOG_WEBHOOK_SECRET}` || - xSecret === LOG_WEBHOOK_SECRET; - if (!ok) { - res.status(401).json({ ok: false, error: "unauthorized" }); - return; - } - // Not configured → tell the operator clearly - if (!MGMT_DOMAIN || !MGMT_CLIENT_ID || !MGMT_CLIENT_SECRET) { - res.status(501).json({ - ok: false, - error: "not_configured", - detail: "Set MGMT_DOMAIN, MGMT_CLIENT_ID, MGMT_CLIENT_SECRET to enable DCR promotion." - }); - return; - } - // Normalize payload to an array of events - let parsed = req.body; - if (typeof parsed === "string") { - try { - parsed = JSON.parse(parsed); - } - catch { /* ignore */ } - } - const events = Array.isArray(parsed) ? parsed : [parsed]; - // Quick sample logging (helps users see shape) - try { - const sample = events.slice(0, 3).map((ev) => { - const e = unwrap(ev); - return { - type: e?.type, - desc: e?.description, - path: evtPath(e) || undefined, - method: e?.details?.request?.method || e?.http?.method, - hasRespClientId: !!e?.details?.response?.body?.client_id - }; - }); - console.log("[webhook:sample]", sample); - } - catch { /* ignore */ } - const dcrEvents = events.filter(isDcrEventRaw); - if (dcrEvents.length === 0) { - res.status(200).json({ ok: true, filtered: true }); - return; - } - try { - const mgmtToken = await getMgmtToken(); - for (const raw of dcrEvents) { - let cid = extractClientIdFromDcrRaw(raw); - if (!cid) { - console.warn("[dcr] no client_id in event; attempting fallback lookup"); - cid = await findNewestChatGPTClientId(mgmtToken); - } - if (!cid) { - console.warn("[dcr] could not determine client_id; skipping event"); - continue; - } - console.log("[dcr] promoting client", { client_id: cid }); - try { - await promoteClient(mgmtToken, cid); - } - catch (e) { - const msg = String(e?.message || ""); - if (msg.startsWith("promote_http_404")) { - console.warn("[dcr] 404 promoting client; retrying with fallback search"); - const alt = await findNewestChatGPTClientId(mgmtToken); - if (alt && alt !== cid) { - console.log("[dcr] retry promoting", { client_id: alt }); - await promoteClient(mgmtToken, alt); - cid = alt; - } - else { - throw e; - } - } - else { - throw e; - } - } - // await enableGoogleForClient(mgmtToken, cid); - // console.log("[dcr] promoted+enabled", { client_id: cid }); - // } - // res.status(200).json({ ok: true, promoted: dcrEvents.length }); - await enableGoogleForClient(mgmtToken, cid); - await ensureClientGrant(mgmtToken, cid, OAUTH_AUDIENCE, TOOL_SCOPES); - console.log("[dcr] promoted+enabled+granted", { - client_id: cid, - audience: OAUTH_AUDIENCE, - scopes: TOOL_SCOPES - }); - } - res.status(200).json({ ok: true, promoted: dcrEvents.length }); - } - catch (e) { - console.error("[dcr:error]", e?.message || e); - res.status(500).json({ ok: false, error: e?.message || String(e) }); - } -} -/** - * Router factory. - * Mount in your server with: `app.use(auth0LogsWebhook());` - * Endpoint becomes: POST /auth0-log-webhook - * - * If you prefer a prefix (e.g. /webhooks), mount as: - * app.use("/webhooks", auth0LogsWebhook()); - * Then the endpoint is POST /webhooks/auth0-log-webhook. - */ -export function auth0LogsWebhook() { - const r = express.Router(); - // accept both text and JSON (Auth0 sometimes sends text/json variants) - r.post("/auth0-log-webhook", express.text({ type: "*/*", limit: "2mb" }), (req, res) => { - // If content-type was JSON, req.body is a string only when text middleware captured it. - // Try to parse JSON if it looks like JSON; otherwise leave as-is and handler will try again. - if (typeof req.body === "string") { - const looksJson = req.body.trim().startsWith("{") || req.body.trim().startsWith("["); - req.body = looksJson ? req.body : req.body; // handler will parse if needed - } - return handleAuth0LogWebhook(req, res); - }); - return r; -} diff --git a/packages/explicabl/tsconfig.json b/packages/explicabl/tsconfig.json index f5bc8df..91fff03 100644 --- a/packages/explicabl/tsconfig.json +++ b/packages/explicabl/tsconfig.json @@ -15,5 +15,8 @@ "types": ["node"], "tsBuildInfoFile": "dist/tsconfig.tsbuildinfo" }, - "include": ["src"] + "include": ["src"], + "references": [ + { "path": "../request-context" } + ] } diff --git a/packages/identifiabl-core/__tests__/jwtRoundTrip.test.ts b/packages/identifiabl-core/__tests__/jwtRoundTrip.test.ts new file mode 100644 index 0000000..0338a4c --- /dev/null +++ b/packages/identifiabl-core/__tests__/jwtRoundTrip.test.ts @@ -0,0 +1,115 @@ +// Enforcement-core hardening (#41) — identifiabl-core JWT verification. +// +// Real-crypto round-trips against a live local JWKS: a validly-signed token +// passes; alg:none, expired, missing-exp, and wrong-audience tokens are all +// rejected. Also asserts the factory refuses an empty issuer/audience. + +import { describe, it, expect, beforeAll, afterAll } from "vitest"; +import http from "http"; +import type { AddressInfo } from "net"; +import { generateKeyPair, exportJWK, SignJWT, type KeyLike } from "jose"; +import { createIdentifiablVerifier } from "../src/index.js"; + +const ISSUER = "https://test.auth0.com/"; +const AUDIENCE = "https://api.test.com"; +const KID = "test-key-1"; + +let server: http.Server; +let jwksUri: string; +let privateKey: KeyLike; + +function b64url(o: unknown): string { + return Buffer.from(JSON.stringify(o)).toString("base64url"); +} + +beforeAll(async () => { + const { publicKey, privateKey: priv } = await generateKeyPair("RS256"); + privateKey = priv; + const jwk = await exportJWK(publicKey); + jwk.kid = KID; + jwk.alg = "RS256"; + jwk.use = "sig"; + const body = JSON.stringify({ keys: [jwk] }); + + server = http.createServer((_req, res) => { + res.writeHead(200, { "content-type": "application/json" }); + res.end(body); + }); + await new Promise((r) => server.listen(0, "127.0.0.1", r)); + const { port } = server.address() as AddressInfo; + jwksUri = `http://127.0.0.1:${port}/jwks.json`; +}); + +afterAll(async () => { + await new Promise((r) => server.close(() => r())); +}); + +const verifier = () => createIdentifiablVerifier({ issuer: ISSUER, audience: AUDIENCE, jwksUri }); + +async function sign(claims: Record, opts?: { exp?: string | number | null }): Promise { + let jwt = new SignJWT({ ...claims }) + .setProtectedHeader({ alg: "RS256", kid: KID }) + .setIssuer(ISSUER) + .setAudience(AUDIENCE) + .setSubject("auth0|user") + .setIssuedAt(); + if (opts?.exp !== null) jwt = jwt.setExpirationTime(opts?.exp ?? "1h"); + return jwt.sign(privateKey); +} + +describe("createIdentifiablVerifier: accepts a valid token", () => { + it("verifies a correctly-signed, unexpired token", async () => { + const token = await sign({}); + const r = await verifier()(token); + expect(r.ok).toBe(true); + if (r.ok) expect(r.identity.sub).toBe("auth0|user"); + }); +}); + +describe("createIdentifiablVerifier: rejects bad tokens", () => { + it("rejects an unsigned alg:none token", async () => { + const forged = `${b64url({ alg: "none", typ: "JWT" })}.${b64url({ + sub: "auth0|user", + iss: ISSUER, + aud: AUDIENCE, + exp: Math.floor(Date.now() / 1000) + 3600, + })}.`; + const r = await verifier()(forged); + expect(r.ok).toBe(false); + }); + + it("rejects an expired token", async () => { + const token = await sign({}, { exp: Math.floor(Date.now() / 1000) - 3600 }); + const r = await verifier()(token); + expect(r.ok).toBe(false); + }); + + it("rejects a token with no exp claim (would otherwise never expire)", async () => { + const token = await sign({}, { exp: null }); + const r = await verifier()(token); + expect(r.ok).toBe(false); + }); + + it("rejects a token minted for a different audience", async () => { + const token = await new SignJWT({}) + .setProtectedHeader({ alg: "RS256", kid: KID }) + .setIssuer(ISSUER) + .setAudience("https://some-other-api.example.com") + .setSubject("auth0|user") + .setIssuedAt() + .setExpirationTime("1h") + .sign(privateKey); + const r = await verifier()(token); + expect(r.ok).toBe(false); + }); +}); + +describe("createIdentifiablVerifier: rejects a misconfigured factory", () => { + it("throws on an empty issuer", () => { + expect(() => createIdentifiablVerifier({ issuer: "", audience: AUDIENCE, jwksUri })).toThrow(/issuer/i); + }); + + it("throws on an empty audience", () => { + expect(() => createIdentifiablVerifier({ issuer: ISSUER, audience: "", jwksUri })).toThrow(/audience/i); + }); +}); diff --git a/packages/identifiabl-core/src/index.ts b/packages/identifiabl-core/src/index.ts index 82eaf18..585fa92 100644 --- a/packages/identifiabl-core/src/index.ts +++ b/packages/identifiabl-core/src/index.ts @@ -151,6 +151,23 @@ export function createIdentifiablVerifier( ): (token: string) => Promise { const issuerNoSlash = trimTrailingSlashes(config.issuer); const audience = config.audience; + + // Reject a misconfigured verifier at construction rather than minting one + // that silently accepts tokens. An empty issuer makes the post-verify issuer + // check pass for `iss: ""`; an empty/absent audience disables audience + // binding in jose, so a token minted for any audience would verify. + if (!issuerNoSlash) { + throw new Error("identifiabl: config.issuer is required and must be non-empty"); + } + const audienceEmpty = + audience === undefined || + audience === null || + (typeof audience === "string" && audience.trim() === "") || + (Array.isArray(audience) && audience.filter((a) => String(a).trim()).length === 0); + if (audienceEmpty) { + throw new Error("identifiabl: config.audience is required and must be non-empty"); + } + const jwksUri = config.jwksUri || `${issuerNoSlash}/.well-known/jwks.json`; @@ -161,7 +178,11 @@ export function createIdentifiablVerifier( const { payload } = await jwtVerify(token, JWKS, { audience, algorithms: ["RS256"], - clockTolerance: "60s" + clockTolerance: "60s", + // Require an expiry. Without this, jose accepts a token that simply + // omits `exp` — a token that never expires. `exp` is then enforced by + // jose's own clock check (with the tolerance above). + requiredClaims: ["exp"], }); const iss = String(payload.iss || ""); diff --git a/packages/identifiabl/src/index.d.ts b/packages/identifiabl/src/index.d.ts deleted file mode 100644 index a501d50..0000000 --- a/packages/identifiabl/src/index.d.ts +++ /dev/null @@ -1,25 +0,0 @@ -import type { RequestHandler } from "express"; -export interface IdentifiablConfig { - /** - * Expected issuer, e.g. "https://example.auth0.com" - * Can have or not have a trailing slash; we normalize it. - */ - issuer: string; - /** - * Expected audience (API identifier). - */ - audience: string; - /** - * Optional JWKS URI. If not provided, defaults to - * `${issuerWithoutTrailingSlash}/.well-known/jwks.json`. - */ - jwksUri?: string; -} -/** - * Express middleware that: - * - Extracts a Bearer token - * - Verifies it with JWKS (jose) - * - Checks audience and issuer - * - Attaches the JWT payload to req.user - */ -export declare function identifiabl(config: IdentifiablConfig): RequestHandler; diff --git a/packages/identifiabl/src/index.js b/packages/identifiabl/src/index.js deleted file mode 100644 index 21de861..0000000 --- a/packages/identifiabl/src/index.js +++ /dev/null @@ -1,65 +0,0 @@ -import type { RequestHandler } from "express"; -import { - createGatewayContext, - type GatewayRequestMeta, -} from "@gatewaystack/request-context"; -import { - createIdentifiablVerifier, - type IdentifiablCoreConfig, -} from "@gatewaystack/identifiabl-core"; - -export interface IdentifiablConfig extends IdentifiablCoreConfig {} - -/** - * Express middleware that: - * - Extracts a Bearer token - * - Uses identifiabl-core to verify it - * - Attaches identity to req.gateway.identity - * - Also mirrors identity to req.user for convenience - */ -export function identifiabl(config: IdentifiablConfig): RequestHandler { - const verify = createIdentifiablVerifier(config); - - const middleware: RequestHandler = async (req: any, res, next) => { - const auth = req.headers.authorization || ""; - const token = auth.startsWith("Bearer ") ? auth.slice(7) : ""; - - if (!token) { - return res.status(401).json({ error: "missing_bearer" }); - } - - const result = await verify(token); - - if (!result.ok) { - return res - .status(401) - .json({ error: result.error, detail: result.detail }); - } - - const { identity } = result; - - // Ensure GatewayContext exists - if (!req.gateway) { - const requestMeta: Partial = { - method: req.method, - path: req.path, - ip: req.ip, - userAgent: req.headers["user-agent"] as string | undefined, - }; - - req.gateway = createGatewayContext({ - request: requestMeta, - }); - } - - // Attach identity - req.gateway.identity = identity; - - // Backward-compatible alias - req.user = identity; - - return next(); - }; - - return middleware; -} diff --git a/packages/limitabl-core/__tests__/agentGuardHardening.test.ts b/packages/limitabl-core/__tests__/agentGuardHardening.test.ts new file mode 100644 index 0000000..dae87e1 --- /dev/null +++ b/packages/limitabl-core/__tests__/agentGuardHardening.test.ts @@ -0,0 +1,45 @@ +// Enforcement-core hardening (#41) — limitabl-core agent guard. + +import { describe, it, expect, vi, afterEach } from "vitest"; +import { LimitablEngine } from "../src/preflight.js"; +import { AgentGuard } from "../src/agentGuard.js"; + +afterEach(() => vi.useRealTimers()); + +describe("LimitablEngine: a configured agent guard cannot be opted out of", () => { + it("DENIES when the guard is configured but no workflow key resolves", () => { + const engine = new LimitablEngine({ agentGuard: { maxToolCalls: 5 } }); + const r = engine.preflight({ sub: "u" }); // no workflowId + expect(r.allowed).toBe(false); + expect(r.reason).toMatch(/no workflow key/i); + }); + + it("runs the guard normally when a workflow key is present", () => { + const engine = new LimitablEngine({ agentGuard: { maxToolCalls: 5 } }); + const r = engine.preflight({ sub: "u" }, { workflowId: "wf-1" }); + expect(r.allowed).toBe(true); + }); + + it("does not deny for a missing workflow key when no guard is configured", () => { + const engine = new LimitablEngine({ rateLimit: { windowMs: 1000, maxRequests: 100 } }); + const r = engine.preflight({ sub: "u" }); + expect(r.allowed).toBe(true); + }); +}); + +describe("AgentGuard: workflow state is bounded (TTL sweep)", () => { + it("evicts workflows older than maxDurationMs so the map does not grow unbounded", () => { + vi.useFakeTimers(); + const guard = new AgentGuard({ maxDurationMs: 1000 }); + + // Seed 5 distinct workflows (simulating id rotation). + for (let i = 0; i < 5; i++) guard.recordToolCall(`wf-${i}`); + expect(guard.size).toBe(5); + + // Advance past the TTL, then touch the guard again — the sweep runs and + // the expired rotated ids are evicted (only the fresh one remains). + vi.advanceTimersByTime(1500); + guard.recordToolCall("wf-fresh"); + expect(guard.size).toBe(1); + }); +}); diff --git a/packages/limitabl-core/src/agentGuard.ts b/packages/limitabl-core/src/agentGuard.ts index a5892b9..6a253d2 100644 --- a/packages/limitabl-core/src/agentGuard.ts +++ b/packages/limitabl-core/src/agentGuard.ts @@ -24,6 +24,7 @@ interface WorkflowState { export class AgentGuard { private workflows = new Map(); private config: Required; + private lastSweepAt = Date.now(); constructor(config: AgentGuardConfig = {}) { this.config = { @@ -33,6 +34,28 @@ export class AgentGuard { }; } + /** + * Evict workflow state older than maxDurationMs. Such workflows are already + * denied by the duration check, so their state is pure dead weight — without + * this, a client rotating workflow ids grows the Map without bound (a memory + * DoS). Runs lazily (at most once per maxDurationMs) off the normal call + * path, so there is no timer to leak or to keep the process alive. + */ + private sweepExpired(now: number): void { + if (now - this.lastSweepAt < this.config.maxDurationMs) return; + this.lastSweepAt = now; + for (const [id, state] of this.workflows) { + if (now - state.startedAt > this.config.maxDurationMs) { + this.workflows.delete(id); + } + } + } + + /** Number of workflows currently tracked (exposed for tests/observability). */ + get size(): number { + return this.workflows.size; + } + /** * Check if a tool call is allowed within the workflow constraints. * Call this BEFORE executing each tool call. @@ -97,6 +120,7 @@ export class AgentGuard { } private getOrCreate(workflowId: string): WorkflowState { + this.sweepExpired(Date.now()); let state = this.workflows.get(workflowId); if (!state) { state = { startedAt: Date.now(), toolCallCount: 0, totalCost: 0 }; diff --git a/packages/limitabl-core/src/preflight.ts b/packages/limitabl-core/src/preflight.ts index 584d13e..52ffb28 100644 --- a/packages/limitabl-core/src/preflight.ts +++ b/packages/limitabl-core/src/preflight.ts @@ -74,8 +74,21 @@ export class LimitablEngine { } } - // Agent guard check - if (this.agentGuard && opts?.workflowId) { + // Agent guard check. + // + // If the operator configured an agent guard, it must run. The old code + // only ran it when a workflowId was supplied, so a client could opt out of + // runaway protection entirely by simply omitting the id — deny instead when + // the guard is configured but no key resolves. + if (this.agentGuard) { + if (!opts?.workflowId) { + return { + allowed: false, + reason: + "Agent guard is enabled but no workflow key resolved for this " + + "request; refusing rather than running unguarded.", + }; + } const guard = this.agentGuard.check(opts.workflowId); if (!guard.allowed) { return { diff --git a/packages/limitabl/__tests__/middleware.test.ts b/packages/limitabl/__tests__/middleware.test.ts new file mode 100644 index 0000000..e4d9e3f --- /dev/null +++ b/packages/limitabl/__tests__/middleware.test.ts @@ -0,0 +1,45 @@ +// Enforcement-core hardening (#41) — limitabl facade. +// +// The module-level singleton engine meant a second limitabl() mount silently +// reused the first mount's engine and config. These assert each mount now owns +// its own engine and enforces its own limits. + +import { describe, it, expect } from "vitest"; +import express from "express"; +import request from "supertest"; +import { limitabl } from "../src/index.js"; + +function appWith(maxRequests: number) { + const app = express(); + app.use(limitabl({ rateLimit: { windowMs: 60_000, maxRequests } })); + app.get("/", (_req, res) => res.json({ ok: true })); + return app; +} + +async function countAllowedBeforeLimit(app: express.Express, attempts: number): Promise { + let allowed = 0; + for (let i = 0; i < attempts; i++) { + const res = await request(app).get("/"); + if (res.status === 200) allowed++; + else break; + } + return allowed; +} + +describe("limitabl: two mounts enforce their own limits (no shared singleton)", () => { + it("a strict mount limits sooner than a lax mount created afterward", async () => { + const strict = appWith(2); + const lax = appWith(5); // created AFTER strict — must NOT reuse strict's engine/config + + expect(await countAllowedBeforeLimit(strict, 10)).toBe(2); + expect(await countAllowedBeforeLimit(lax, 10)).toBe(5); + }); + + it("returns 429 with a Retry-After header once the limit is hit", async () => { + const app = appWith(1); + expect((await request(app).get("/")).status).toBe(200); + const limited = await request(app).get("/"); + expect(limited.status).toBe(429); + expect(limited.headers["retry-after"]).toBeDefined(); + }); +}); diff --git a/packages/limitabl/src/index.js b/packages/limitabl/src/index.js deleted file mode 100644 index c60f9b9..0000000 --- a/packages/limitabl/src/index.js +++ /dev/null @@ -1,17 +0,0 @@ -import rateLimit, { ipKeyGenerator } from "express-rate-limit"; -/** - * Express middleware that applies a per-identity rate limit: - * - Prefer req.user.sub - * - Fallback to req.user.org_id - * - Fallback to IP address - */ -export function withLimitabl(config) { - const limiter = rateLimit({ - windowMs: config.windowMs, - limit: config.limit, - keyGenerator: (req) => req.user?.sub || req.user?.org_id || ipKeyGenerator(req), - standardHeaders: true, - legacyHeaders: false, - }); - return limiter; -} diff --git a/packages/limitabl/src/index.ts b/packages/limitabl/src/index.ts index b040bda..f642049 100644 --- a/packages/limitabl/src/index.ts +++ b/packages/limitabl/src/index.ts @@ -22,14 +22,20 @@ export interface LimitablConfig extends LimitablCoreConfig { extractWorkflowId?: (req: any) => string | undefined; } -// Singleton engine per config (middleware is typically created once at startup) -let _engine: LimitablEngine | null = null; - -function getEngine(config: LimitablCoreConfig): LimitablEngine { - if (!_engine) { - _engine = new LimitablEngine(config); - } - return _engine; +/** + * Namespace a client-supplied workflow id under the request's server-resolved + * principal, so the agent-guard key is not purely client-controlled. Without + * this, a client rotates `x-workflow-id` to reset its own runaway counters and + * can collide with another principal's workflow. Binding to sub/tenant/ip means + * a rotated id is still scoped to the same principal. Returns undefined when + * there is no workflow id (the core then denies if a guard is configured). + */ +function workflowKeyFor(req: Request, clientWorkflowId: string | undefined): string | undefined { + if (!clientWorkflowId) return undefined; + const user = (req as any).user; + const principal = + user?.sub ?? (req as any).tenantId ?? user?.org_id ?? (req.ip as string) ?? "anon"; + return `${principal}::${clientWorkflowId}`; } function keyFromReq(req: Request): LimitKey { @@ -65,13 +71,19 @@ function keyFromReq(req: Request): LimitKey { * Attach to routes AFTER identifiabl (needs req.user). */ export function limitabl(config: LimitablConfig): RequestHandler { - const engine = getEngine(config); + // One engine per middleware instance. A module-level singleton (the old + // behavior) meant a second `limitabl(stricterConfig)` mount silently reused + // the first mount's engine and config — a stricter later mount enforced the + // laxer earlier limits. Building the engine here keeps each mount's limits + // (and in-memory counters) its own. + const engine = new LimitablEngine(config); return (req: any, res, next) => { const key = keyFromReq(req); - const workflowId = config.extractWorkflowId + const clientWorkflowId = config.extractWorkflowId ? config.extractWorkflowId(req) : req.header?.("x-workflow-id") ?? undefined; + const workflowId = workflowKeyFor(req, clientWorkflowId); const result = engine.preflight(key, { workflowId }); diff --git a/packages/transformabl/src/index.d.ts b/packages/transformabl/src/index.d.ts deleted file mode 100644 index 8ffe419..0000000 --- a/packages/transformabl/src/index.d.ts +++ /dev/null @@ -1,19 +0,0 @@ -import type { RequestHandler } from "express"; -export interface TransformablConfig { - /** - * Placeholder for future config: - * - PII redaction rules - * - classification flags - * - content filters - */ - redactionRules?: Array; -} -/** - * No-op Transformabl layer for now. - * - * Later this is where you'll: - * - redact PII from req.body / req.headers - * - annotate requests with classification - * - normalize input into a canonical shape - */ -export declare function withTransformabl(_config?: TransformablConfig): RequestHandler; diff --git a/packages/transformabl/src/index.js b/packages/transformabl/src/index.js deleted file mode 100644 index bdb0ab3..0000000 --- a/packages/transformabl/src/index.js +++ /dev/null @@ -1,16 +0,0 @@ -/** - * No-op Transformabl layer for now. - * - * Later this is where you'll: - * - redact PII from req.body / req.headers - * - annotate requests with classification - * - normalize input into a canonical shape - */ -export function withTransformabl(_config) { - const middleware = (req, _res, next) => { - // TODO: implement PII redaction / classification here - // For now, just pass through untouched. - return next(); - }; - return middleware; -} diff --git a/packages/validatabl-core/__tests__/hardening.test.ts b/packages/validatabl-core/__tests__/hardening.test.ts new file mode 100644 index 0000000..187228a --- /dev/null +++ b/packages/validatabl-core/__tests__/hardening.test.ts @@ -0,0 +1,106 @@ +// Enforcement-core hardening (#41) — validatabl-core. + +import { describe, it, expect } from "vitest"; +import { decision } from "../src/decision.js"; +import { hasScope } from "../src/scopes.js"; +import { applyPolicies, validatePolicySet } from "../src/policy.js"; +import type { PolicySet } from "../src/types.js"; + +describe("decision: schema is enforced even when input is absent", () => { + const opts = { + inputSchema: { + type: "object" as const, + required: ["x"], + properties: { x: { type: "string" as const } }, + }, + }; + + it("DENIES when a schema is configured but input is undefined (was fail-open)", () => { + const d = decision({ identity: {} }, opts); + expect(d.allowed).toBe(false); + expect(d.checks.schema?.valid).toBe(false); + }); + + it("still allows a valid input", () => { + const d = decision({ identity: {}, input: { x: "ok" } }, opts); + expect(d.allowed).toBe(true); + }); +}); + +describe("hasScope: exact token match, never a regex", () => { + it('does not treat "." as "any char" (tool.write must not match toolXwrite)', () => { + expect(hasScope({ scope: "toolXwrite" }, "tool.write")).toBe(false); + }); + + it("matches an exact scope token", () => { + expect(hasScope({ scope: "read tool.write admin" }, "tool.write")).toBe(true); + }); + + it("does not throw on regex-metacharacter scope strings", () => { + expect(() => hasScope({ scope: "read" }, "a(b")).not.toThrow(); + expect(hasScope({ scope: "read" }, "a(b")).toBe(false); + }); +}); + +describe("policy: empty-conditions rules are rejected (no accidental allow-all)", () => { + const blanketAllow: PolicySet = { + rules: [{ id: "oops", effect: "allow", conditions: [] }], + }; + + it("applyPolicies throws rather than matching every request", () => { + expect(() => applyPolicies(blanketAllow, { identity: {}, tool: "anything" })).toThrow( + /at least one condition/ + ); + }); + + it("validatePolicySet throws at load time", () => { + expect(() => validatePolicySet(blanketAllow)).toThrow(/oops/); + }); +}); + +describe("policy: matches is anchored by default", () => { + const set = (value: string): PolicySet => ({ + rules: [{ id: "r", effect: "allow", conditions: [{ field: "tool", operator: "matches", value }] }], + defaultEffect: "deny", + }); + + it('a bare pattern no longer substring-matches ("search" must not allow "unsafe-search-exec")', () => { + const r = applyPolicies(set("search"), { identity: {}, tool: "unsafe-search-exec" }); + expect(r.allowed).toBe(false); + }); + + it("a bare pattern still matches the exact value", () => { + const r = applyPolicies(set("search"), { identity: {}, tool: "search" }); + expect(r.allowed).toBe(true); + }); + + it("an author-anchored prefix pattern is honored (^gpt-4 matches gpt-4-turbo)", () => { + expect(applyPolicies(set("^gpt-4"), { identity: {}, model: "x", tool: "gpt-4-turbo" }).allowed).toBe(true); + expect(applyPolicies(set("^gpt-4"), { identity: {}, tool: "claude-3" }).allowed).toBe(false); + }); +}); + +describe("policy: in does not coerce non-string fields", () => { + const set: PolicySet = { + rules: [{ id: "r", effect: "allow", conditions: [{ field: "sub", operator: "in", value: ["a", "b"] }] }], + defaultEffect: "deny", + }; + + it("matches a string field that is a member", () => { + expect(applyPolicies(set, { identity: { sub: "a" } }).allowed).toBe(true); + }); + + it("does not match a non-member", () => { + expect(applyPolicies(set, { identity: { sub: "c" } }).allowed).toBe(false); + }); + + it("an array field cannot stringify past the allow-list", () => { + const arraySet: PolicySet = { + rules: [{ id: "r", effect: "allow", conditions: [{ field: "roles", operator: "in", value: ["admin,user"] }] }], + defaultEffect: "deny", + }; + // identity.roles = ["admin","user"] would String()-coerce to "admin,user" + // under the old code and wrongly match. Strict typing rejects it. + expect(applyPolicies(arraySet, { identity: { roles: ["admin", "user"] } }).allowed).toBe(false); + }); +}); diff --git a/packages/validatabl-core/src/decision.ts b/packages/validatabl-core/src/decision.ts index 7db0f5b..087328c 100644 --- a/packages/validatabl-core/src/decision.ts +++ b/packages/validatabl-core/src/decision.ts @@ -98,7 +98,15 @@ export function decision( } // 3. Schema validation - if (options.inputSchema && request.input !== undefined) { + // + // When a schema is configured it MUST run, even when input is absent. The + // old `&& request.input !== undefined` guard skipped validation for requests + // with no parsed body (GET, wrong content-type, parse failure) — so a schema + // meant to gate a route was silently a no-op and the request passed. That is + // fail-open. `checkSchema(undefined, schema)` denies (an object schema fails + // "Expected an object"; required fields report missing), which is the + // correct answer for "schema required, input missing." + if (options.inputSchema) { const schemaResult = checkSchema(request.input, options.inputSchema); checks.schema = schemaResult; if (!schemaResult.valid) { diff --git a/packages/validatabl-core/src/index.d.ts b/packages/validatabl-core/src/index.d.ts deleted file mode 100644 index 15dd021..0000000 --- a/packages/validatabl-core/src/index.d.ts +++ /dev/null @@ -1,7 +0,0 @@ -export * from "./scopes"; -export type ProtectedResourceConfig = { - issuer: string; - audience?: string; - scopes: string[]; -}; -export declare function buildProtectedResourcePayload(cfg: ProtectedResourceConfig): any; diff --git a/packages/validatabl-core/src/index.d.ts.map b/packages/validatabl-core/src/index.d.ts.map deleted file mode 100644 index 9d21a63..0000000 --- a/packages/validatabl-core/src/index.d.ts.map +++ /dev/null @@ -1 +0,0 @@ -{"version":3,"file":"index.d.ts","sourceRoot":"","sources":["index.ts"],"names":[],"mappings":"AAAA,MAAM,MAAM,uBAAuB,GAAG;IAAE,MAAM,EAAE,MAAM,CAAC;IAAC,QAAQ,CAAC,EAAE,MAAM,CAAC;IAAC,MAAM,EAAE,MAAM,EAAE,CAAA;CAAE,CAAC;AAE9F,wBAAgB,6BAA6B,CAAC,GAAG,EAAE,uBAAuB,OAOzE"} \ No newline at end of file diff --git a/packages/validatabl-core/src/index.js b/packages/validatabl-core/src/index.js deleted file mode 100644 index 5c7876d..0000000 --- a/packages/validatabl-core/src/index.js +++ /dev/null @@ -1,27 +0,0 @@ -"use strict"; -var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) { - if (k2 === undefined) k2 = k; - var desc = Object.getOwnPropertyDescriptor(m, k); - if (!desc || ("get" in desc ? !m.__esModule : desc.writable || desc.configurable)) { - desc = { enumerable: true, get: function() { return m[k]; } }; - } - Object.defineProperty(o, k2, desc); -}) : (function(o, m, k, k2) { - if (k2 === undefined) k2 = k; - o[k2] = m[k]; -})); -var __exportStar = (this && this.__exportStar) || function(m, exports) { - for (var p in m) if (p !== "default" && !Object.prototype.hasOwnProperty.call(exports, p)) __createBinding(exports, m, p); -}; -Object.defineProperty(exports, "__esModule", { value: true }); -exports.buildProtectedResourcePayload = buildProtectedResourcePayload; -__exportStar(require("./scopes"), exports); -function buildProtectedResourcePayload(cfg) { - var payload = { - authorization_servers: [cfg.issuer], - scopes_supported: cfg.scopes - }; - if (cfg.audience) - payload.resource = cfg.audience; - return payload; -} diff --git a/packages/validatabl-core/src/policy.ts b/packages/validatabl-core/src/policy.ts index c2951e1..9cd8834 100644 --- a/packages/validatabl-core/src/policy.ts +++ b/packages/validatabl-core/src/policy.ts @@ -25,10 +25,32 @@ import { getScopeStringFromClaims } from "./scopes.js"; * - Modification actions (strip fields, downgrade model, reduce token limits) * beyond simple allow/deny */ +/** + * Reject structurally-unsafe rules. A rule with no conditions matches EVERY + * request (`[].every()` is vacuously true) — an `allow` rule with empty + * conditions is a blanket allow-all that silently defeats deny-by-default. + * Throws on the first offending rule so a misconfigured policy set fails loudly + * at load rather than quietly widening. Call this once when policies are + * loaded; applyPolicies also calls it defensively. + */ +export function validatePolicySet(policySet: PolicySet): void { + for (const rule of policySet.rules) { + if (!Array.isArray(rule.conditions) || rule.conditions.length === 0) { + throw new Error( + `Invalid policy rule "${rule.id ?? ""}": a rule must have at ` + + `least one condition (an empty condition list matches every request). ` + + `Use an explicit catch-all condition or set defaultEffect instead.` + ); + } + } +} + export function applyPolicies( policySet: PolicySet, request: PolicyRequest ): PolicyDecision { + validatePolicySet(policySet); + const sorted = [...policySet.rules].sort( (a, b) => (a.priority ?? 100) - (b.priority ?? 100) ); @@ -82,20 +104,31 @@ function matchesCondition( } case "in": { - // value is an array; check if fieldValue is in it - if (Array.isArray(condition.value)) { - return condition.value.includes(String(fieldValue)); - } - return false; + // value is an array; check if fieldValue is one of its members. + // Strict: no String() coercion. Coercing let an array field stringify to + // "a,b" and an object to "[object Object]", so unrelated values slipped + // past an `in` allow-list. Only a string field can be a member; anything + // else does not match. + if (!Array.isArray(condition.value)) return false; + if (typeof fieldValue !== "string") return false; + return condition.value.includes(fieldValue); } case "matches": { - // value is a regex pattern + // value is a regex pattern. Anchor by default: a bare pattern like + // "search" used to substring-match, so it silently allowed + // "unsafe-search-exec" — an unanchored match widens every policy. A bare + // pattern is now wrapped in ^(?:…)$ (exact whole-string match). An author + // who wrote their own anchor ("^gpt-4" for a prefix/family match) has + // expressed intent, so we honor it verbatim rather than double-anchor. if (typeof fieldValue !== "string" || typeof condition.value !== "string") { return false; } + const raw = condition.value; + const authored = raw.startsWith("^") || raw.endsWith("$"); + const source = authored ? raw : `^(?:${raw})$`; try { - return new RegExp(condition.value).test(fieldValue); + return new RegExp(source).test(fieldValue); } catch { return false; } diff --git a/packages/validatabl-core/src/scopes.d.ts b/packages/validatabl-core/src/scopes.d.ts deleted file mode 100644 index eaa8456..0000000 --- a/packages/validatabl-core/src/scopes.d.ts +++ /dev/null @@ -1,12 +0,0 @@ -export interface ScopeClaims { - scope?: string | string[]; - scopes?: string[]; -} -/** - * Normalize scopes from various JWT claim shapes into a single space-delimited string. - */ -export declare function getScopeStringFromClaims(claims: ScopeClaims): string; -/** - * Check whether a given scope is present in the user's scopes. - */ -export declare function hasScope(claims: ScopeClaims, scope: string): boolean; diff --git a/packages/validatabl-core/src/scopes.js b/packages/validatabl-core/src/scopes.js deleted file mode 100644 index 559a6ab..0000000 --- a/packages/validatabl-core/src/scopes.js +++ /dev/null @@ -1,30 +0,0 @@ -"use strict"; -// packages/validatabl-core/src/scopes.ts -Object.defineProperty(exports, "__esModule", { value: true }); -exports.getScopeStringFromClaims = getScopeStringFromClaims; -exports.hasScope = hasScope; -/** - * Normalize scopes from various JWT claim shapes into a single space-delimited string. - */ -function getScopeStringFromClaims(claims) { - if (typeof claims.scope === "string") { - return claims.scope; - } - if (Array.isArray(claims.scope)) { - return claims.scope.join(" "); - } - if (Array.isArray(claims.scopes)) { - return claims.scopes.join(" "); - } - return ""; -} -/** - * Check whether a given scope is present in the user's scopes. - */ -function hasScope(claims, scope) { - var s = getScopeStringFromClaims(claims); - if (!s) - return false; - var pattern = new RegExp("(^|\\s)".concat(scope, "(\\s|$)")); - return pattern.test(s); -} diff --git a/packages/validatabl-core/src/scopes.ts b/packages/validatabl-core/src/scopes.ts index c02c5db..de3beb0 100644 --- a/packages/validatabl-core/src/scopes.ts +++ b/packages/validatabl-core/src/scopes.ts @@ -23,10 +23,14 @@ export function getScopeStringFromClaims(claims: ScopeClaims): string { /** * Check whether a given scope is present in the user's scopes. + * + * Membership is an exact, whitespace-delimited token match — never a regex. + * Building a `RegExp` from the caller's scope string (the old implementation) + * meant a required scope containing regex metacharacters either widened the + * check (`tool.write` matched `toolXwrite` because `.` is "any char") or threw + * on an unbalanced metachar and 500'd the request. */ export function hasScope(claims: ScopeClaims, scope: string): boolean { - const s = getScopeStringFromClaims(claims); - if (!s) return false; - const pattern = new RegExp(`(^|\\s)${scope}(\\s|$)`); - return pattern.test(s); + if (!scope) return false; + return getScopeStringFromClaims(claims).split(/\s+/).includes(scope); }