Skip to content

fix(security): default-deny generic MasterData CRUD on both invoke surfaces#325

Open
0xcucumbersalad wants to merge 1 commit into
mainfrom
fix/gate-internal-invoke-actions
Open

fix(security): default-deny generic MasterData CRUD on both invoke surfaces#325
0xcucumbersalad wants to merge 1 commit into
mainfrom
fix/gate-internal-invoke-actions

Conversation

@0xcucumbersalad

@0xcucumbersalad 0xcucumbersalad commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Problem

The generic VTEX MasterData v2 CRUD actions — searchDocuments, createDocument, patchDocument, getDocument, uploadAttachment (@decocms/apps/vtex/actions/masterData) — 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/<hash> — built at build time by scripts/generate-invoke.ts (and the migration scaffold scripts/migrate/templates/server-entry.ts) from @decocms/apps' vtex/invoke.ts.
  2. POST /deco/invoke/vtex/actions/masterData/<fn> — registered at runtime by setupApps() walking the whole app manifest.

A searchDocuments(entity="CL", filter="id=*") call therefore dumps the customer (CL) table's PII. The two machines run at different times (build vs. request) and share no registry, so closing one left the action reachable via the other.

Fix

Introduce a single source of truth — src/admin/invokePolicy.ts — consulted by both machines so they can't drift:

  • generate-invoke.ts — skips denied actions; never emits the createServerFn const, so no _serverFn route is minted. New --exclude / --allow flags.
  • migrate/templates/server-entry.ts — drops the hardcoded MasterData imports/consts/map entries so a failed generator run can't fail open.
  • setupApps() — skips 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 (runtime) or --allow (build). allow wins over deny.

Tests

  • invokePolicy unit tests (both key shapes, allow/deny precedence).
  • setupApps runtime-door integration test — /deco/invoke/…/masterData/* → 404, cart → 200, allow re-opens exactly one.
  • generate-invoke build tests — default-deny drops MasterData while keeping cart actions; --allow re-includes surgically.

Verified live

Ran on casaevideo-tanstack (real VTEX site) against this branch: default-deny shuts both doors (/deco/invoke 404, no _serverFn consts), --allow / policy.allow re-open each door surgically, homepage 200, no errors introduced.


Summary by cubic

Default-deny the generic VTEX MasterData v2 CRUD actions across both invoke surfaces to block unauthenticated access. Adds a shared invokePolicy used at build time and runtime, with --allow/--exclude escape hatches for audited opt-ins.

  • Bug Fixes

    • Build surface (/_serverFn/<hash>): scripts/generate-invoke.ts consults src/admin/invokePolicy.ts and skips denied actions; adds --allow and --exclude.
    • Runtime surface (/deco/invoke/...): src/sdk/setupApps.ts filters manifest keys with invokePolicy before registering; autoconfigApps() accepts an optional policy param.
    • Migration scaffold: removed MasterData CRUD imports/handlers so a failed generator run can’t fail open.
  • Migration

    • No changes needed to gain protection after upgrading @decocms/start.
    • To re-allow a specific action, run the generator with --allow <name> or pass { allow: ["searchDocuments"] } to autoconfigApps/setupApps.

Written for commit fc7fbb2. Summary will update on new commits.

Review in cubic

…rfaces

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/<hash>   — built by generate-invoke.ts / the migration
                                scaffold from @decocms/apps' invoke.ts
  2. POST /deco/invoke/vtex/actions/masterData/<fn>
                              — 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) <noreply@anthropic.com>
@0xcucumbersalad 0xcucumbersalad requested a review from a team July 8, 2026 15:56
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.

1 participant