Skip to content

fix(security): stop auto-exposing generic MasterData CRUD as server functions#332

Open
0xcucumbersalad wants to merge 1 commit into
mainfrom
fix/masterdata-crud-serverfn-exposure
Open

fix(security): stop auto-exposing generic MasterData CRUD as server functions#332
0xcucumbersalad wants to merge 1 commit into
mainfrom
fix/masterdata-crud-serverfn-exposure

Conversation

@0xcucumbersalad

@0xcucumbersalad 0xcucumbersalad commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Problem

@decocms/apps-vtex lists five generic VTEX MasterData CRUD actions in its invoke.vtex.actions contract:

  • searchDocuments({ entity, filter })
  • createDocument({ entity, data })
  • getDocument({ entity, documentId })
  • patchDocument({ entity, documentId, data })
  • uploadAttachment(...)

@decocms/blocks-cli's generate-invoke.ts emits one top-level createServerFn per entry into each site's src/server/invoke.gen.ts. TanStack Start compiles every top-level createServerFn().handler() into a public, unauthenticated /_serverFn/<hash> endpoint — addressable over HTTP regardless of whether any UI code references it.

These handlers take a client-supplied entity and proxy straight to the VTEX MasterData API using the app's admin appKey/appToken, with no auth middleware and no entity allow-list. So an anonymous caller can:

  • searchDocuments({ entity: "CL", filter: "..." }) → enumerate the Clients (customer PII) table
  • createDocument / patchDocument → write arbitrary entities

This is broken access control / mass data exposure.

Why not just add an auth check

Even an authenticated customer must not be able to run searchDocuments({ entity: "CL" }) and read other customers' records under the app's admin token. A generic (entity, filter) proxy structurally has no room for per-entity/per-record authorization — the generic, admin-credentialed, client-parameterized shape is the vulnerability. So remove it from the auto-exposed surface rather than guard it.

Changes

  • apps-vtex/src/invoke.ts — drop the 5 MasterData entries (and their imports) from invoke.vtex.actions. The functions stay exported from actions/masterData for server-side use.
  • blocks-cli/scripts/generate-invoke.ts — add a PRIVILEGED_ACTIONS denylist that skips + warns on these names even if re-added to the contract, so the exclusion can't silently regress. Also covers the apps-start legacy source path the generator can fall back to.
  • generate-invoke.test.ts — lock the guard: privileged actions are never emitted as consts / vtexActions entries / imports; safe actions still are; a warning is printed per skip.

How a site adds MasterData back

The masterData.ts functions remain exported, so a site that legitimately needs MasterData defines a narrow, entity-pinned, authenticated createServerFn in its own src/server/invoke.ts (same mechanism sites already use for _vtexAuth/_logout):

const _myCrmEntries = createServerFn({ method: "POST" }).handler(async () => {
  const user = parseVtexAuthJwt(getRequestHeader("cookie") ?? "");
  if (!user) throw new Error("unauthorized");
  return searchDocuments({ entity: "CF", filter: `email=${user.email}` }); // entity pinned, filter from session
});

Rules: pin entity server-side (never from the client), validate input, and for user-scoped data derive identity from the session cookie — not a client-supplied id. There is deliberately no generic re-enable switch; re-emitting the generic handler would recreate an unguardable endpoint.

Verification

  • vitest run packages/blocks-cli/scripts/generate-invoke.test.ts15 passed (9 existing + 6 new).
  • typecheck clean for @decocms/apps-vtex and @decocms/blocks-cli.
  • E2E: ran the fixed generator against the fixed source — output drops from 17 → 12 server functions, zero privileged consts, no actions/masterData import; cart/session/newsletter actions intact.

Follow-ups (out of scope)

  • Consuming sites must bump @decocms/apps-vtex + re-run generate:invoke + redeploy to close the hole on already-generated invoke.gen.ts.
  • The legacy @decocms/apps (apps-start) repo carries the same source list; the generator guard here already protects it, but an optional consistency PR could mirror the data change there.
  • Sibling site-level issues surfaced during the audit (wishlist IDOR trusting a client userId; tableMasterdata.ts arbitrary client tableId) are separate.

🤖 Generated with Claude Code


Summary by cubic

Stopped auto-exposing generic VTEX MasterData CRUD as public /_serverFn endpoints in generated code. This closes a broken access control issue where anonymous callers could read or write MasterData using admin credentials.

  • Bug Fixes

    • @decocms/apps-vtex: removed generic MasterData actions from invoke.vtex.actions; functions remain for server-side use only.
    • @decocms/blocks-cli: added PRIVILEGED_ACTIONS denylist in generate-invoke.ts to skip and warn on these actions (covers legacy path too).
    • Tests ensure privileged actions are never emitted; safe actions still are.
  • Migration

    • Update @decocms/apps-vtex and @decocms/blocks-cli.
    • Re-run generate:invoke to regenerate src/server/invoke.gen.ts.
    • Redeploy to remove any previously exposed endpoints.

Written for commit 61cf768. Summary will update on new commits.

Review in cubic

…unctions

Generic VTEX MasterData CRUD (createDocument, getDocument, patchDocument,
searchDocuments, uploadAttachment) was listed in @decocms/apps-vtex's
`invoke.vtex.actions`. The blocks-cli generator emits one top-level
`createServerFn` per entry, which TanStack Start compiles into a public,
unauthenticated `/_serverFn/<hash>` endpoint. These handlers take a
client-supplied `entity` and proxy to the VTEX MasterData API with the app's
admin appKey/appToken and no auth or entity allow-list — so any caller can
`searchDocuments({ entity: "CL", ... })` to enumerate customer PII, or
create/patch arbitrary entities. Broken access control.

Authentication alone can't make a generic (entity, filter) proxy safe — it has
no room for per-entity/per-record authorization — so remove it from the
auto-exposed surface rather than guard it:

- apps-vtex/invoke.ts: drop the 5 MasterData entries from invoke.vtex.actions
  (and their imports). The functions stay exported from actions/masterData for
  server-side use; sites needing MasterData define a narrow, entity-pinned,
  authenticated action in their own src/server/invoke.ts.
- generate-invoke.ts: add a PRIVILEGED_ACTIONS denylist that skips + warns on
  these names even if re-added to the contract, so the exclusion can't silently
  regress. Also covers the apps-start legacy source path.
- generate-invoke.test.ts: lock the guard — privileged actions are never
  emitted as consts / vtexActions entries / imports, safe actions still are.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@0xcucumbersalad 0xcucumbersalad requested a review from a team July 10, 2026 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants