Repository navigation
fix(core): only run user actions through the integration actions route, 404 for foreign integrations - #726
Merged
Conversation
…e, 404 for foreign integrations POST /api/integrations/:integrationId/actions/:actionId dispatched any event name, so an integration owner could run ON_DELETE, queue handlers or webhook events inside the HTTP request. The action routes (and their options/refresh routes) now only accept events typed USER_ACTION, including dynamic and extension user actions; anything else is a 404. GetIntegrationInstance, UpdateIntegration and DeleteIntegrationForUser answered another user's or an unknown integration with a plain Error (500). They now throw Boom.notFound with the same message for both, mapping the repositories' "not found" error instead of leaking it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
✅ Deploy Preview for friggframework-org canceled.
|
… gate agree loadUserActions listed every loadDynamicUserActions entry, while the actions route only runs events typed USER_ACTION, so an untyped dynamic action was listed but answered 404. Dynamic entries now default to USER_ACTION in one helper used by initialize() and loadUserActions(), and loadUserActions lists only USER_ACTION entries. A dynamic entry typed as something else is neither listed nor runnable. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Contributor
|
🚀 PR was released in |
8 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Problem
POST /api/integrations/:integrationId/actions/:actionIdcalledintegration.send(actionId, req.body)for any event name. The integration's owner could runON_DELETE, queue/page handlers,ON_WEBHOOK,CRM_CARDor any other internal event inside the HTTP request./actions/:actionId/optionsand/options/refreshaccepted any id as well.GetIntegrationInstance(used byGET /api/integrations/:id,/config/options,/actions*,/test-auth),UpdateIntegration(PATCH) andDeleteIntegrationForUser(DELETE) threw a plainErrorfor a foreign integration. The real repositories also throw a plainErrorfor an unknown id instead of returningnull.Boom.boomifyturned both into500 Internal Server Error.Fix
integration-router.js:assertUserActionruns before the three action routes dispatch anything. It allows only events in the integration's mergedeventsmap whosetypeisUSER_ACTION, the same entriesloadUserActions(GET /actions) lists: static, dynamic (merged byinitialize()) and extension user actions. Unknown ids, prototype keys and non-user-action events get404 User action <id> does not exist. The check reads the mapinitialize()already built, so it adds no I/O, and a failingloadDynamicUserActionsstill doesn't block static actions.GetIntegrationInstance,UpdateIntegration,DeleteIntegrationForUser: a missing integration and another user's integration both throwBoom.notFound('Integration with id of <id> does not exist'). The repositories'... not founderror maps to the same response, so callers can't tell the two cases apart. Other repository errors are rethrown unchanged. 404 matches the existing not-found response inDeleteIntegrationForUser, and no route returned 403 for this case.docs/frigg-management-api.yml: documents the 404 on the three action routes.Breaking change
Only for clients that triggered non-
USER_ACTIONevents through/actions/:actionId. Those calls now return 404. In this monorepo onlypackages/ui/lib/api/api.jscalls the route, and it uses ids returned byGET /actions, which are always user actions. No in-repo code matched on the olddoes not belong to User/No integration found by the IDmessages. Queue workers going throughbackend-utilskeep their retry behaviour, because Boom doesn't set a top-levelstatusCode.Test plan
integrations/tests/integration-router-user-actions.test.jsruns through the real router,GetIntegrationInstanceandIntegrationBaseover an in-memory repository that throws on a missing id, like the real repositories. It covers:ON_DELETE, aQUEUEevent andGET_CONFIG_OPTIONS→ 404, and the handlers don't runconstructorand__proto__ids → 404Boom.conflict/Boom.tooManyRequests→ 409 / 429 with their messagenpx jestinpackages/core(Node 22): 2654 passed, 11 failed, 2 skipped. The 7 failing suites (invocation-scope, mapping-already-exists-error, app-definition-loader, integration-webhook-routers, integration-defined-workers, require-graph, integrations-report) fail the same way onorigin/nextin the same checkout.Caveats
Invalid ID: ...) still return 500. That case is unchanged.🤖 Generated with Claude Code
Note
Medium Risk
Changes authorization and error semantics on integration management and action endpoints; legitimate callers using listed user actions are unaffected, but any client relying on triggering internal events via the actions API will break.
Overview
Tightens integration HTTP security by only allowing
USER_ACTIONevents on the action submit/options/refresh routes, and by returning consistent 404 responses when an integration is missing or not owned by the caller.assertUserActionin the integration router now blocks dispatch for unknown ids, prototype keys, and internal events (ON_DELETE, queue handlers, etc.) that were previously runnable viaPOST .../actions/:actionId. Dynamic user actions without an explicittypeare normalized toUSER_ACTIONinIntegrationBaseso they stay aligned withGET /actionslisting.GetIntegrationInstance,UpdateIntegration, andDeleteIntegrationForUsermap both repository “not found” and cross-user access to the sameBoom.notFoundmessage (fixing prior 500s from plainErrors). The management API spec documents the new action-route 404s. Breaking: clients that invoked non–user-action event names through/actions/:actionIdnow get 404.Reviewed by Cursor Bugbot for commit 7cf570c. Bugbot is set up for automated code reviews on this repo. Configure here.
📦 Published PR as canary version:
2.0.0--canary.726.7cf570c.0✨ Test out this PR locally via:
npm install @friggframework/admin-scripts@2.0.0--canary.726.7cf570c.0 npm install @friggframework/core@2.0.0--canary.726.7cf570c.0 npm install @friggframework/devtools@2.0.0--canary.726.7cf570c.0 npm install @friggframework/eslint-config@2.0.0--canary.726.7cf570c.0 npm install @friggframework/prettier-config@2.0.0--canary.726.7cf570c.0 npm install @friggframework/schemas@2.0.0--canary.726.7cf570c.0 npm install @friggframework/serverless-plugin@2.0.0--canary.726.7cf570c.0 npm install @friggframework/test@2.0.0--canary.726.7cf570c.0 npm install @friggframework/ui@2.0.0--canary.726.7cf570c.0 # or yarn add @friggframework/admin-scripts@2.0.0--canary.726.7cf570c.0 yarn add @friggframework/core@2.0.0--canary.726.7cf570c.0 yarn add @friggframework/devtools@2.0.0--canary.726.7cf570c.0 yarn add @friggframework/eslint-config@2.0.0--canary.726.7cf570c.0 yarn add @friggframework/prettier-config@2.0.0--canary.726.7cf570c.0 yarn add @friggframework/schemas@2.0.0--canary.726.7cf570c.0 yarn add @friggframework/serverless-plugin@2.0.0--canary.726.7cf570c.0 yarn add @friggframework/test@2.0.0--canary.726.7cf570c.0 yarn add @friggframework/ui@2.0.0--canary.726.7cf570c.0Version
Published prerelease version:
v2.0.0-next.125Changelog
🚀 Enhancement
@friggframework/admin-scripts,@friggframework/core,@friggframework/devtools,@friggframework/eslint-config,@friggframework/prettier-config,@friggframework/schemas,@friggframework/serverless-plugin,@friggframework/test,@friggframework/ui🐛 Bug Fix
@friggframework/corenextAuthors: 2