From fc7fbb28fb362f1b836da1d7afe434cb9db8cf06 Mon Sep 17 00:00:00 2001 From: decobot Date: Wed, 8 Jul 2026 23:00:09 +0800 Subject: [PATCH] fix(security): default-deny generic MasterData CRUD on both invoke surfaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The generic VTEX MasterData v2 CRUD actions (searchDocuments, createDocument, patchDocument, getDocument, uploadAttachment) run with the store's admin appKey/appToken and take a caller-controlled entity + _where filter. They were exposed unauthenticated on TWO independent surfaces: 1. POST /_serverFn/ — built by generate-invoke.ts / the migration scaffold from @decocms/apps' invoke.ts 2. POST /deco/invoke/vtex/actions/masterData/ — registered at runtime by setupApps() walking the whole app manifest The two machines run at different times (build vs request) and share no registry, so closing one left the action reachable via the other. Introduce a single source of truth — src/admin/invokePolicy.ts — consulted by BOTH machines so they can't drift: - generate-invoke.ts: skip denied actions, never emit the createServerFn const, so no _serverFn route is minted. New --exclude / --allow flags. - migrate/templates/server-entry.ts: drop the hardcoded MasterData imports/consts/map entries so a failed generator run can't fail open. - setupApps(): skip denied keys before registering, closing /deco/invoke. New optional policy param threaded through autoconfigApps(). Default-deny with an escape hatch: upgrading @decocms/start protects every site with no per-site action; a site with a real need re-allows a specific action via policy.allow / --allow (allow wins over deny). Tests: invokePolicy unit tests, setupApps runtime-door integration test (404 on MasterData, 200 on cart, allow re-opens), and generate-invoke build tests (default-deny + --allow re-inclusion). Co-Authored-By: Claude Opus 4.8 (1M context) --- scripts/generate-invoke.test.ts | 102 ++++++++++++++++++++++ scripts/generate-invoke.ts | 32 +++++++ scripts/migrate/templates/server-entry.ts | 45 ++-------- src/admin/index.ts | 5 ++ src/admin/invokePolicy.test.ts | 57 ++++++++++++ src/admin/invokePolicy.ts | 92 +++++++++++++++++++ src/apps/autoconfig.ts | 9 +- src/sdk/setupApps.test.ts | 101 +++++++++++++++++++++ src/sdk/setupApps.ts | 28 +++++- 9 files changed, 431 insertions(+), 40 deletions(-) create mode 100644 src/admin/invokePolicy.test.ts create mode 100644 src/admin/invokePolicy.ts create mode 100644 src/sdk/setupApps.test.ts diff --git a/scripts/generate-invoke.test.ts b/scripts/generate-invoke.test.ts index d86f4240..fa1646cb 100644 --- a/scripts/generate-invoke.test.ts +++ b/scripts/generate-invoke.test.ts @@ -29,6 +29,7 @@ const FIXTURE_INVOKE_TS = `\ import { createInvokeFn } from "@decocms/start/sdk/createInvoke"; import { getOrCreateCart, simulateCart } from "./actions/checkout"; import { createSession } from "./actions/session"; +import { searchDocuments, createDocument } from "./actions/masterData"; import type { OrderForm } from "./types"; export const invoke = { @@ -49,6 +50,16 @@ export const invoke = { createSession: createInvokeFn( (data: Record) => createSession({ data }), ), + + // Generic MasterData CRUD — admin-credentialed, caller-parameterized. + // These MUST be denied by default (never emitted) so no + // POST /_serverFn/ route is minted for them. + searchDocuments: createInvokeFn( + (data: { entity: string; filter: string }) => searchDocuments(data), + ), + createDocument: createInvokeFn( + (data: { entity: string; data: Record }) => createDocument(data), + ), }, }, } as const; @@ -65,6 +76,10 @@ const FIXTURE_ACTIONS_SESSION_TS = `\ export interface CreateSessionProps { data: Record; } export async function createSession(_props: CreateSessionProps): Promise { return null; } `; +const FIXTURE_ACTIONS_MASTERDATA_TS = `\ +export async function searchDocuments(_data: any): Promise { return null; } +export async function createDocument(_data: any): Promise { return null; } +`; const FIXTURE_TYPES_TS = `export type OrderForm = unknown;\n`; describe("generate-invoke.ts — output shape", () => { @@ -94,6 +109,10 @@ describe("generate-invoke.ts — output shape", () => { path.join(appsDir, "vtex", "actions", "session.ts"), FIXTURE_ACTIONS_SESSION_TS, ); + fs.writeFileSync( + path.join(appsDir, "vtex", "actions", "masterData.ts"), + FIXTURE_ACTIONS_MASTERDATA_TS, + ); fs.writeFileSync(path.join(appsDir, "vtex", "types.ts"), FIXTURE_TYPES_TS); outFile = path.join(siteDir, "src", "server", "invoke.gen.ts"); @@ -172,6 +191,25 @@ describe("generate-invoke.ts — output shape", () => { expect(orderedCalls.length).toBe(3); }); + it("denies generic MasterData CRUD by default (no createServerFn emitted)", () => { + // The exposure hole: searchDocuments/createDocument run with admin + // credentials and a caller-controlled entity + _where filter. They must + // never be emitted as top-level consts, so TanStack's compiler never + // mints a POST /_serverFn/ route for them. + expect(generatedOutput).not.toContain("$searchDocuments"); + expect(generatedOutput).not.toContain("$createDocument"); + expect(generatedOutput).not.toMatch(/\bsearchDocuments\(data\)/); + expect(generatedOutput).not.toMatch(/\bcreateDocument\(data\)/); + // And they must be absent from the exported actions map. + expect(generatedOutput).not.toMatch(/^\s*searchDocuments:/m); + expect(generatedOutput).not.toMatch(/^\s*createDocument:/m); + + // Safe actions are unaffected — only the denylisted ones are dropped. + expect(generatedOutput).toContain("$getOrCreateCart"); + expect(generatedOutput).toContain("$simulateCart"); + expect(generatedOutput).toContain("$createSession"); + }); + it("preserves adapting wrappers verbatim (does not collapse to actionFn(data))", () => { // Regression for the createSession-shape wrapper: the generator // previously hard-coded `${importedFn}(data)` in every handler, @@ -193,3 +231,67 @@ describe("generate-invoke.ts — output shape", () => { expect(generatedOutput).toContain("const result = await simulateCart(data);"); }); }); + +describe("generate-invoke.ts — --allow escape hatch", () => { + let output: string; + let status: number | null; + let appsDir: string; + + beforeAll(() => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "gen-invoke-allow-")); + appsDir = path.join(tmp, "apps"); + const siteDir = path.join(tmp, "site"); + fs.mkdirSync(path.join(appsDir, "vtex", "actions"), { recursive: true }); + fs.mkdirSync(path.join(siteDir, "src", "server"), { recursive: true }); + fs.writeFileSync(path.join(appsDir, "vtex", "invoke.ts"), FIXTURE_INVOKE_TS); + fs.writeFileSync( + path.join(appsDir, "vtex", "actions", "checkout.ts"), + FIXTURE_ACTIONS_CHECKOUT_TS, + ); + fs.writeFileSync( + path.join(appsDir, "vtex", "actions", "session.ts"), + FIXTURE_ACTIONS_SESSION_TS, + ); + fs.writeFileSync( + path.join(appsDir, "vtex", "actions", "masterData.ts"), + FIXTURE_ACTIONS_MASTERDATA_TS, + ); + fs.writeFileSync(path.join(appsDir, "vtex", "types.ts"), FIXTURE_TYPES_TS); + const outFile = path.join(siteDir, "src", "server", "invoke.gen.ts"); + + const result = spawnSync( + "npx", + [ + "tsx", + GENERATOR, + "--apps-dir", + appsDir, + "--out-file", + outFile, + "--allow", + "searchDocuments", + ], + { cwd: siteDir, encoding: "utf8" }, + ); + status = result.status; + output = fs.readFileSync(outFile, "utf8"); + }, 30_000); + + afterAll(() => { + try { + fs.rmSync(path.dirname(appsDir), { recursive: true, force: true }); + } catch { + // ignore + } + }); + + it("re-allows exactly the named action, keeping others denied", () => { + expect(status).toBe(0); + // Explicitly allowed → emitted again. + expect(output).toContain("$searchDocuments"); + expect(output).toContain("const result = await searchDocuments(data);"); + // Still denied — --allow was surgical, not a blanket off switch. + expect(output).not.toContain("$createDocument"); + expect(output).not.toMatch(/\bcreateDocument\(data\)/); + }); +}); diff --git a/scripts/generate-invoke.ts b/scripts/generate-invoke.ts index 620f38bb..afc3a768 100644 --- a/scripts/generate-invoke.ts +++ b/scripts/generate-invoke.ts @@ -17,10 +17,15 @@ * Env / CLI: * --out-file override output (default: src/server/invoke.gen.ts) * --apps-dir override @decocms/apps location (default: auto-resolve from node_modules) + * --exclude comma-separated extra action names to NOT expose (beyond the + * built-in default denylist of generic MasterData CRUD actions) + * --allow comma-separated action names to RE-ALLOW despite the default + * denylist (escape hatch — prefer a purpose-built wrapper) */ import fs from "node:fs"; import path from "node:path"; import { Project, type PropertyAssignment, SyntaxKind } from "ts-morph"; +import { type InvokePolicyOptions, isInternalAction } from "../src/admin/invokePolicy"; const args = process.argv.slice(2); function arg(name: string, fallback: string): string { @@ -28,6 +33,19 @@ function arg(name: string, fallback: string): string { return idx !== -1 && args[idx + 1] ? args[idx + 1] : fallback; } +function list(name: string): string[] { + return arg(name, "").split(",").map((s) => s.trim()).filter(Boolean); +} + +// Invoke-exposure policy. Generic MasterData CRUD is denied by default; +// --exclude adds to that, --allow re-opens specific actions. Same policy the +// runtime registrar (src/sdk/setupApps.ts) applies, so an action can't be +// gated on one exposure surface but left open on the other. +const policy: InvokePolicyOptions = { + deny: list("exclude"), + allow: list("allow"), +}; + const cwd = process.cwd(); const outFile = path.resolve(cwd, arg("out-file", "src/server/invoke.gen.ts")); @@ -126,6 +144,7 @@ if (!invokeVar) { } const actions: ActionDef[] = []; +const skippedInternal: string[] = []; const invokeInit = invokeVar.getInitializer(); if (!invokeInit) { console.error("invoke variable has no initializer"); @@ -162,6 +181,14 @@ for (const prop of actionsObj.getProperties()) { if (prop.getKind() !== SyntaxKind.PropertyAssignment) continue; const pa = prop as PropertyAssignment; const name = pa.getName(); + + // Default-deny internal actions: never emit a createServerFn const for them, + // so TanStack's compiler never mints a POST /_serverFn/ route. + if (isInternalAction(name, policy)) { + skippedInternal.push(name); + continue; + } + const initText = pa.getInitializer()!.getText(); // Check if it uses createInvokeFn with unwrap @@ -467,3 +494,8 @@ export const invoke = { fs.mkdirSync(path.dirname(outFile), { recursive: true }); fs.writeFileSync(outFile, out); console.log(`Generated ${actions.length} server functions → ${path.relative(cwd, outFile)}`); +if (skippedInternal.length > 0) { + console.log( + `🔒 Not exposed (internal/denied — no _serverFn route): ${skippedInternal.join(", ")}`, + ); +} diff --git a/scripts/migrate/templates/server-entry.ts b/scripts/migrate/templates/server-entry.ts index 75ebd971..2e2b0758 100644 --- a/scripts/migrate/templates/server-entry.ts +++ b/scripts/migrate/templates/server-entry.ts @@ -400,13 +400,18 @@ export const vtexActions = {} as const; import { createServerFn } from "@tanstack/react-start"; import { getOrCreateCart, addItemsToCart, updateCartItems, addCouponToCart, simulateCart, getSellersByRegion, setShippingPostalCode, updateOrderFormAttachment } from "@decocms/apps/vtex/actions/checkout"; import { createSession, editSession } from "@decocms/apps/vtex/actions/session"; -import { createDocument, getDocument, patchDocument, searchDocuments, uploadAttachment } from "@decocms/apps/vtex/actions/masterData"; +// Generic MasterData CRUD (createDocument / getDocument / patchDocument / +// searchDocuments / uploadAttachment) is intentionally NOT scaffolded here. +// It runs with the store's admin appKey/appToken and takes a caller-controlled +// entity + _where filter, so it is denied by default (see +// @decocms/start/admin/invokePolicy). The real generator, generate-invoke.ts, +// enforces the same denylist when it overwrites this file post-migration; +// this scaffold matches it so a failed/skipped generator run can't fail open. import { subscribe } from "@decocms/apps/vtex/actions/newsletter"; import { notifyMe } from "@decocms/apps/vtex/actions/misc"; import type { OrderForm } from "@decocms/apps/vtex/types"; import type { SimulationItem, RegionResult } from "@decocms/apps/vtex/actions/checkout"; import type { SessionData } from "@decocms/apps/vtex/actions/session"; -import type { CreateDocumentResult, UploadAttachmentOpts } from "@decocms/apps/vtex/actions/masterData"; import type { SubscribeProps } from "@decocms/apps/vtex/actions/newsletter"; import type { NotifyMeProps } from "@decocms/apps/vtex/actions/misc"; @@ -487,35 +492,8 @@ const $editSession = createServerFn({ method: "POST" }) return unwrapResult(result); }); -const $createDocument = createServerFn({ method: "POST" }) - .inputValidator((data: { entity: string; data: Record }) => data) - .handler(async ({ data }): Promise => { - return createDocument(data); - }); - -const $getDocument = createServerFn({ method: "POST" }) - .inputValidator((data: { entity: string; documentId: string }) => data) - .handler(async ({ data }): Promise => { - return getDocument(data); - }); - -const $patchDocument = createServerFn({ method: "POST" }) - .inputValidator((data: { entity: string; documentId: string; data: Record }) => data) - .handler(async ({ data }): Promise => { - return patchDocument(data); - }); - -const $searchDocuments = createServerFn({ method: "POST" }) - .inputValidator((data: { entity: string; filter: string }) => data) - .handler(async ({ data }): Promise => { - return searchDocuments(data); - }); - -const $uploadAttachment = createServerFn({ method: "POST" }) - .inputValidator((data: UploadAttachmentOpts) => data) - .handler(async ({ data }): Promise => { - return uploadAttachment(data); - }); +// MasterData CRUD server functions intentionally omitted — see the import +// comment above. They are denied by default in @decocms/start/admin/invokePolicy. const $subscribe = createServerFn({ method: "POST" }) .inputValidator((data: SubscribeProps) => data) @@ -540,11 +518,6 @@ export const vtexActions = { updateOrderFormAttachment: $updateOrderFormAttachment as unknown as (ctx: { data: { orderFormId: string; attachment: string; body: Record } }) => Promise, createSession: $createSession, editSession: $editSession as unknown as (ctx: { data: { public: Record } }) => Promise, - createDocument: $createDocument as unknown as (ctx: { data: { entity: string; data: Record } }) => Promise, - getDocument: $getDocument, - patchDocument: $patchDocument as unknown as (ctx: { data: { entity: string; documentId: string; data: Record } }) => Promise, - searchDocuments: $searchDocuments, - uploadAttachment: $uploadAttachment as unknown as (ctx: { data: UploadAttachmentOpts }) => Promise<{ ok: true }>, subscribe: $subscribe as unknown as (ctx: { data: SubscribeProps }) => Promise, notifyMe: $notifyMe as unknown as (ctx: { data: NotifyMeProps }) => Promise, } as const; diff --git a/src/admin/index.ts b/src/admin/index.ts index cb7ccc5b..c5982b9a 100644 --- a/src/admin/index.ts +++ b/src/admin/index.ts @@ -9,6 +9,11 @@ export { setInvokeActions, setInvokeLoaders, } from "./invoke"; +export { + DEFAULT_INTERNAL_ACTIONS, + type InvokePolicyOptions, + isInternalAction, +} from "./invokePolicy"; export { LIVE_CONTROLS_SCRIPT } from "./liveControls"; export { handleMeta, setMetaData } from "./meta"; export { handleRender, setPreviewWrapper, setRenderShell } from "./render"; diff --git a/src/admin/invokePolicy.test.ts b/src/admin/invokePolicy.test.ts new file mode 100644 index 00000000..d460fc8f --- /dev/null +++ b/src/admin/invokePolicy.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it } from "vitest"; +import { + DEFAULT_INTERNAL_ACTIONS, + isInternalAction, +} from "./invokePolicy"; + +describe("invokePolicy — isInternalAction", () => { + it("denies the built-in generic MasterData CRUD actions by default", () => { + for (const name of DEFAULT_INTERNAL_ACTIONS) { + expect(isInternalAction(name)).toBe(true); + } + }); + + it("matches both key shapes the two exposure machines produce", () => { + // Machine 1 (invoke.gen.ts) keys are bare function names. + expect(isInternalAction("searchDocuments")).toBe(true); + // Machine 2 (setupApps) keys are module-path/fnName. + expect(isInternalAction("vtex/actions/masterData/searchDocuments")).toBe(true); + // .ts aliases the registrar also emits. + expect(isInternalAction("vtex/actions/masterData/searchDocuments.ts")).toBe(true); + }); + + it("allows ordinary actions", () => { + expect(isInternalAction("getOrCreateCart")).toBe(false); + expect(isInternalAction("vtex/actions/checkout/addItemsToCart")).toBe(false); + expect(isInternalAction("subscribe")).toBe(false); + }); + + it("honors a site-provided deny list (bare or full key)", () => { + expect(isInternalAction("dangerousThing", { deny: ["dangerousThing"] })).toBe(true); + expect( + isInternalAction("site/actions/dangerousThing", { deny: ["dangerousThing"] }), + ).toBe(true); + expect(isInternalAction("safeThing", { deny: ["dangerousThing"] })).toBe(false); + }); + + it("lets an explicit allow entry override the default denylist", () => { + expect(isInternalAction("searchDocuments", { allow: ["searchDocuments"] })).toBe(false); + expect( + isInternalAction("vtex/actions/masterData/searchDocuments", { + allow: ["searchDocuments"], + }), + ).toBe(false); + }); + + it("allow is surgical — only the named action is re-opened", () => { + const policy = { allow: ["searchDocuments"] }; + expect(isInternalAction("searchDocuments", policy)).toBe(false); + expect(isInternalAction("createDocument", policy)).toBe(true); + }); + + it("allow wins over an explicit deny too", () => { + expect( + isInternalAction("thing", { deny: ["thing"], allow: ["thing"] }), + ).toBe(false); + }); +}); diff --git a/src/admin/invokePolicy.ts b/src/admin/invokePolicy.ts new file mode 100644 index 00000000..d48a70a9 --- /dev/null +++ b/src/admin/invokePolicy.ts @@ -0,0 +1,92 @@ +/** + * Single source of truth for which invoke actions must NOT be publicly + * exposed — consulted by BOTH exposure machines so they can never drift: + * + * 1. Build-time `_serverFn` generation (`scripts/generate-invoke.ts` and the + * migration scaffold `scripts/migrate/templates/server-entry.ts`) — a + * denied action's `createServerFn` const is never emitted, so the TanStack + * compiler never mints a `POST /_serverFn/` route for it. + * + * 2. Runtime manifest registration (`src/sdk/setupApps.ts`) — a denied action + * is never written into the invoke handler registry, so + * `POST /deco/invoke/` returns 404 for it. + * + * The two machines run at different times (build vs. request) and share no + * registry, but they share THIS list — closing one door without the other + * leaves the action reachable through the other, so both import from here. + * + * Why these five: the generic VTEX MasterData v2 CRUD actions + * (`@decocms/apps/vtex/actions/masterData`) run with the store's admin + * appKey/appToken and take a caller-controlled `entity` + `_where` filter. + * Exposed unauthenticated they permit dumping/tampering any MasterData entity + * (e.g. `CL` customer PII). They have no legitimate client-callable use — a + * site that needs to write one specific entity should ship a narrow, + * server-side wrapper action instead of exposing the generic CRUD surface. + * + * Default-deny: these are blocked out of the box, so upgrading @decocms/start + * protects every site with no per-site action. A site that genuinely needs one + * can re-allow it explicitly (see `allow` in InvokePolicyOptions). + */ + +/** Bare function names of the always-denied generic MasterData CRUD actions. */ +export const DEFAULT_INTERNAL_ACTIONS: ReadonlySet = new Set([ + "searchDocuments", + "createDocument", + "patchDocument", + "getDocument", + "uploadAttachment", +]); + +export interface InvokePolicyOptions { + /** + * Extra action keys to deny, beyond the built-in defaults. Matched the same + * way as the defaults (bare last segment OR full key). Sites pass this to + * lock down additional sensitive actions. + */ + deny?: Iterable; + /** + * Escape hatch — bare function names to RE-ALLOW despite the default denylist. + * Use only when a site has a real, audited need to expose one of the generic + * CRUD actions to the client. Prefer a purpose-built wrapper action instead. + */ + allow?: Iterable; +} + +/** + * Reduce an invoke key to the bare function name used for matching. + * Handles both key shapes the two machines produce: + * "searchDocuments" → "searchDocuments" (invoke.gen.ts) + * "vtex/actions/masterData/searchDocuments" → "searchDocuments" (setupApps) + * "...searchDocuments.ts" → "searchDocuments" (.ts aliases) + */ +function lastSegment(key: string): string { + const noExt = key.replace(/\.ts$/, ""); + return noExt.split("/").pop() ?? noExt; +} + +/** + * True if `key` must not be publicly exposed under the given policy. + * + * Precedence: an explicit `allow` entry wins over every denylist (built-in or + * site `deny`), so a site can surgically re-open a single action without + * forking the framework list. + */ +export function isInternalAction( + key: string, + options: InvokePolicyOptions = {}, +): boolean { + const seg = lastSegment(key); + + const allow = options.allow ? new Set(options.allow) : null; + if (allow && (allow.has(seg) || allow.has(key))) return false; + + if (DEFAULT_INTERNAL_ACTIONS.has(seg)) return true; + + if (options.deny) { + for (const d of options.deny) { + if (d === seg || d === key || lastSegment(d) === seg) return true; + } + } + + return false; +} diff --git a/src/apps/autoconfig.ts b/src/apps/autoconfig.ts index 8d36aaec..968e0308 100644 --- a/src/apps/autoconfig.ts +++ b/src/apps/autoconfig.ts @@ -15,6 +15,7 @@ * await autoconfigApps(generatedBlocks, APP_REGISTRY); */ +import type { InvokePolicyOptions } from "../admin/invokePolicy"; import { onChange } from "../cms/loader"; import { resolveSecret } from "../sdk/crypto"; import { @@ -111,17 +112,21 @@ async function configureAllApps( * @param blocks Decofile blocks (from blocks.gen or loadBlocks()). * @param registry List of installable apps — typically * `import { APP_REGISTRY } from "@decocms/apps/registry"`. + * @param policy Optional invoke-exposure policy. The generic MasterData CRUD + * actions are denied by default; pass `allow`/`deny` here only + * to override that (see InvokePolicyOptions). */ export async function autoconfigApps( blocks: Record, registry: AppRegistry, + policy: InvokePolicyOptions = {}, ): Promise { if (typeof document !== "undefined") return; // server-only if (!registry || registry.length === 0) return; const apps = await configureAllApps(blocks, registry); if (apps.length > 0) { - await setupApps(apps); + await setupApps(apps, policy); } // Re-configure on admin hot-reload @@ -129,7 +134,7 @@ export async function autoconfigApps( if (typeof document !== "undefined") return; const updatedApps = await configureAllApps(newBlocks, registry); if (updatedApps.length > 0) { - await setupApps(updatedApps); + await setupApps(updatedApps, policy); } }); } diff --git a/src/sdk/setupApps.test.ts b/src/sdk/setupApps.test.ts new file mode 100644 index 00000000..0cf2e2bd --- /dev/null +++ b/src/sdk/setupApps.test.ts @@ -0,0 +1,101 @@ +// @vitest-environment node +// +// setupApps() early-returns when `document` is defined (it is server-only), +// so this suite must run without a DOM or every registration is skipped. +/** + * Integration test for the runtime exposure door (Machine 2). + * + * setupApps() flattens an app's manifest into the invoke handler registry. + * The regression we lock here: generic MasterData CRUD actions + * (searchDocuments/createDocument/…) must NOT be registered, so + * `POST /deco/invoke/vtex/actions/masterData/searchDocuments` returns 404 — + * while ordinary actions (cart) still resolve. This is the sibling of the + * build-time _serverFn denial exercised in generate-invoke.test.ts; both + * consult the same invokePolicy so they can't drift. + */ + +import { beforeEach, describe, expect, it } from "vitest"; +import { clearInvokeHandlers, handleInvoke } from "../admin/invoke"; +import { setupApps } from "./setupApps"; + +function invokeReq(key: string): Request { + return new Request(`https://site.example/deco/invoke/${key}`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: "{}", + }); +} + +// A fake VTEX-shaped app whose manifest exposes both a safe cart action and +// the dangerous MasterData CRUD actions as named exports. +function fakeVtexApp() { + return { + name: "vtex", + state: {}, + manifest: { + name: "vtex", + loaders: {}, + actions: { + "vtex/actions/checkout": { + getOrCreateCart: async () => ({ ok: "cart" }), + }, + "vtex/actions/masterData": { + searchDocuments: async () => ({ leaked: "PII" }), + createDocument: async () => ({ leaked: "write" }), + patchDocument: async () => ({ leaked: "tamper" }), + getDocument: async () => ({ leaked: "one" }), + uploadAttachment: async () => ({ leaked: "file" }), + }, + }, + }, + }; +} + +async function bodyOf(res: Response): Promise { + return JSON.parse(await res.text()); +} + +describe("setupApps — runtime invoke exposure policy", () => { + beforeEach(() => { + clearInvokeHandlers(); + }); + + it("does not register generic MasterData CRUD actions (404 on invoke)", async () => { + await setupApps([fakeVtexApp()]); + + for (const fn of [ + "searchDocuments", + "createDocument", + "patchDocument", + "getDocument", + "uploadAttachment", + ]) { + const res = await handleInvoke(invokeReq(`vtex/actions/masterData/${fn}`)); + expect(res.status, `${fn} must be unreachable`).toBe(404); + } + }); + + it("still registers ordinary actions (cart resolves)", async () => { + await setupApps([fakeVtexApp()]); + + const res = await handleInvoke(invokeReq("vtex/actions/checkout/getOrCreateCart")); + expect(res.status).toBe(200); + expect(await bodyOf(res)).toEqual({ ok: "cart" }); + }); + + it("re-allows a denied action when the site opts in via policy.allow", async () => { + await setupApps([fakeVtexApp()], { allow: ["searchDocuments"] }); + + const allowed = await handleInvoke( + invokeReq("vtex/actions/masterData/searchDocuments"), + ); + expect(allowed.status).toBe(200); + expect(await bodyOf(allowed)).toEqual({ leaked: "PII" }); + + // Others stay denied — allow is surgical. + const stillDenied = await handleInvoke( + invokeReq("vtex/actions/masterData/createDocument"), + ); + expect(stillDenied.status).toBe(404); + }); +}); diff --git a/src/sdk/setupApps.ts b/src/sdk/setupApps.ts index 316dd452..6fc0a0b6 100644 --- a/src/sdk/setupApps.ts +++ b/src/sdk/setupApps.ts @@ -20,6 +20,10 @@ */ import { clearInvokeHandlers, registerInvokeHandlers } from "../admin/invoke"; +import { + type InvokePolicyOptions, + isInternalAction, +} from "../admin/invokePolicy"; import { registerSections } from "../cms/registry"; import { registerCommerceLoaders, @@ -190,6 +194,7 @@ function registerAppCommerceHandlers( */ export async function setupApps( apps: Array, + policy: InvokePolicyOptions = {}, ): Promise { if (typeof document !== "undefined") return; // server-only @@ -198,6 +203,21 @@ export async function setupApps( clearInvokeHandlers(); clearAppCommerceLoaders(); + // Drop internal/denied actions (e.g. generic MasterData CRUD) from a + // handlers record before it reaches either registry. Same policy the + // build-time _serverFn generator applies, so an action can't be closed on + // one exposure surface but left open on the other. + const gate = ( + handlers: Record Promise>, + ) => { + const kept: typeof handlers = {}; + for (const [key, handler] of Object.entries(handlers)) { + if (isInternalAction(key, policy)) continue; + kept[key] = handler; + } + return kept; + }; + for (const app of flattenDependencies(apps as AppDefinition[])) { const appWithHandlers = app as AppDefinitionWithHandlers; @@ -205,8 +225,9 @@ export async function setupApps( // These also go into the commerce-loaders map so the CMS resolve path // (src/cms/resolve.ts) can dispatch to them by __resolveType. if (appWithHandlers.handlers) { - registerInvokeHandlers(appWithHandlers.handlers); - registerAppCommerceHandlers(appWithHandlers.handlers); + const handlers = gate(appWithHandlers.handlers); + registerInvokeHandlers(handlers); + registerAppCommerceHandlers(handlers); } // 2. Flatten manifest modules → individual invoke handlers. @@ -231,6 +252,9 @@ export async function setupApps( const key = fnName === "default" ? moduleKey : `${moduleKey}/${fnName}`; + // Default-deny internal actions (generic MasterData CRUD, etc.): + // never register them, so `POST /deco/invoke/${key}` stays 404. + if (isInternalAction(key, policy)) continue; const handler = (props: any, req: Request) => (fn as Function)(props, req); registerInvokeHandlers({