Repository navigation
fix(devtools): route /api/entity and /api/entities to the integration router - #667
Conversation
seanspeaks
added
release
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
If your organization's extra usage balance is empty, an organization admin can add extra usage credits at claude.ai/admin-settings/usage. If its monthly spend limit was reached, an admin can raise it on the same page. If neither applies, contact Anthropic support.
Once extra usage is available, someone with write access to this repository can comment @claude review on this pull request to trigger a review.
✅ Deploy Preview for friggframework-org canceled.
|
|
Frigg 2.0.1 review probe — checking whether COMMENT reviews/comments can be posted from this agent. |
Frigg 2.0.1 review (Group 2) — COMMENT only(Ignore the earlier “review probe” comment from this agent.) Checklist
Verdict (would): changes needed Findings
Gateway route table + |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| }, | ||
| }, | ||
| { httpApi: { path: '/api/authorize', method: 'ANY' } }, | ||
| { httpApi: { path: '/api/entity', method: 'ANY' } }, |
There was a problem hiding this comment.
Checklist note (pass for routing): ANY /api/entity, /api/entity/{proxy+}, /api/entities, /api/entities/{proxy+} correctly attach the Management API entity prefixes to auth.
blocking elsewhere: once these routes work, Express handlers still call Module.getEntityModelFromDefinition (undefined on Module). Authenticated GET /api/entities/:id, options, and test-auth become 500 instead of the previous gateway 404. Fix or land the missing static helper in the same release train as this route PR.
There was a problem hiding this comment.
Verdict: confirmed. Fixed in f74cf3b. Findings, checked on 6dc4b398:
POST /api/entity(packages/core/integrations/integration-router.js:597) calledmodule.findOrCreateEntity(...).Modulehas no such method; it only exists onProcessAuthorizationCallback. The module also came fromGetModuleInstanceFromTypewith no credential loaded, sogetEntityDetailsran against an API with no tokens.GetModule,TestModuleAuth,GetEntityOptionsByIdandRefreshEntityOptionscallModule.getEntityModelFromDefinition, which has never been defined onModule(git log -Sfinds no definition). SoGET /api/entities/:id,/test-auth,POST /optionsand/options/refreshall return 500.GetModulewould also have hit a second TypeError atmodule.entity.user.toString(), because the repositories returnuserId, notuser.
Fix (core, hexagonal):
- New use case
modules/use-cases/create-entity-from-credential.js. The handler now just callscreateEntityFromCredential.execute({ user, entityType, credentialId }). The use case loads the credential through the credential repository and refuses a missing or foreign one with the same 400Invalid credential ID, so ids can't be probed. It builds theModuleon the stored credential and persists throughProcessAuthorizationCallback.findOrCreateEntity, the same find-or-create (and credential repointing) that/api/authorizeuses, with no copy of that logic. Its file name andexecutesignature match the Stack A version on feat(core): entity scopes and ownership by scope (ADR-056 §3.1–§3.4, §3.6) #682, so that branch's ownership-aware version replaces this one cleanly on rebase. - New helper
modules/utils/find-module-definition.js(findModuleDefinitionForEntity). It matches on the storedmoduleName, or a legacymodelName. The four use cases now use it instead of the missing static. GetModulenow returnscredentialId+authIsValidand readsuserIdfromentity.userId. Response-shape change: it no longer returns the decryptedcredential. The route has 500'd until now, so no client can depend on that field, and returning raw tokens is the exposure flagged on feat(core): entity scopes and ownership by scope (ADR-056 §3.1–§3.4, §3.6) #682.
Tests (all failed before the fix, 6/8 red): integrations/tests/integration-router-entity-routes.test.js runs the real router, use cases and Module over in-memory repositories:
- POST create (and idempotent re-POST), foreign/missing credential → identical 400, unknown type → 400
- GET
/api/entities/:idreturns no token material - GET
/api/entities/:id/test-auth - the v1 entity list (
GET /api/integrations→entities.authorized)
There's also modules/use-cases/__tests__/create-entity-from-credential.test.js, which asserts delegation to findOrCreateEntity.
Note: v1 has no GET /api/entities list route. The list is entities.authorized on GET /api/integrations, which already worked, so those two tests pass before and after. /api/entities on the gateway is still correct for the /{proxy+} children.
npx jest modules integrations in core: 873 passed. The 8 failures are pre-existing and in files this change doesn't touch: find-integration-context-by-external-entity-id.test.js (error-code name) and the DocumentDB mapping-encryption test.
Not changed here: POST /api/entity still returns the persisted entity record. That record includes the populated credential for the caller's own credential (the shape the old code meant to return, and the same as Stack A). The DTO-level fix belongs with the one recommended on #682.
🤖 Generated with Claude Code
Follow-up finding (COMMENT)blocking — In addition to the missing Scenario: After the gateway routes land, Frigg UI “create entity” → Express reaches the handler → |
There was a problem hiding this comment.
If your organization's extra usage balance is empty, an organization admin can add extra usage credits at claude.ai/admin-settings/usage. If its monthly spend limit was reached, an admin can raise it on the same page. If neither applies, contact Anthropic support.
Once extra usage is available, someone with write access to this repository can comment @claude review on this pull request to trigger a review.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f74cf3b. Configure here.
| const entityDetails = await module.getEntityDetails( | ||
| module.api, | ||
| null, | ||
| null, |
There was a problem hiding this comment.
POST entity leaks credential secrets
High Severity
POST /api/entity now reaches Express and returns the raw findOrCreateEntity row, whose populated credential includes access and refresh tokens. GET /api/entities/:entityId was sanitized to credentialId and authIsValid in this same change, but the create path still sends secrets to the client.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f74cf3b. Configure here.
… router
The auth Lambda serves core's integration router, which registers
POST /api/entity, GET /api/entity/options/:credentialId and the
/api/entities/:entityId family (get, test-auth, options, options/refresh).
The generated HTTP API only declared /api/integrations,
/api/integrations/{proxy+} and /api/authorize, and HTTP APIs have no
implicit catch-all, so every entity route 404'd at the gateway in a
deployed app (and under serverless-offline) before reaching Express.
Add /api/entity, /api/entity/{proxy+}, /api/entities and
/api/entities/{proxy+} (ANY) to the auth function, and add a test that
reads the routes off a live createIntegrationRouter(), composes a full
app definition, and asserts each one selects an API Gateway route on the
auth function, so a route added to core later fails until the gateway
is updated too. It also pins the neighbouring report, admin-script,
webhook, extension and integration-defined routes to their own
functions.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
POST /api/entity called module.findOrCreateEntity, which Module does not have, on a Module built without the credential. It now calls a CreateEntityFromCredential use case: it checks that the caller owns the credential (missing and foreign ids get the same 400), builds the Module on that credential, and persists through ProcessAuthorizationCallback.findOrCreateEntity, the same find-or-create the authorization flow uses. GetModule, TestModuleAuth, GetEntityOptionsById and RefreshEntityOptions looked up the definition with Module.getEntityModelFromDefinition, which does not exist, so every /api/entities/:entityId route returned 500. They now share findModuleDefinitionForEntity (moduleName, or a legacy modelName). GetModule also read entity.user, which the repositories do not return, and returned the decrypted credential. It now returns credentialId and authIsValid, like the v2 entity DTO. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013b2UUFaQ5Lk6TFPqzRr1j7
f74cf3b to
a0620c7
Compare
…on responses The repositories attach the decrypted credential to every entity they return, and three responses sent those records as they were, so a member of an organization received the organization connection's access_token, refresh_token or api_key: - v1 GET /api/integrations (entities.authorized): mapModuleClassToModuleDTO returned the entity record; - v1 GET /api/integrations and v2 GET /api/v2/integrations: the integration DTOs embedded the entities from findEntitiesByIds; - v1 GET /api/entities/:id: GetModule returned module.credential. One shared toPublicEntity (modules/utils/public-entity.js) now decides what an entity looks like in any response: descriptive fields plus credentialId and authIsValid, for every caller including the owner. It is used by the module DTO, GetModule, the v1 integration DTO, the v2 toIntegrationDto, v1 GET /api/integrations/:id and the POST /api/entity response. GetModule, TestModuleAuth, GetEntityOptionsById and RefreshEntityOptions also looked up the module definition with Module.getEntityModelFromDefinition, which does not exist, so every /api/entities/:entityId route answered 500. They now use findModuleDefinitionForEntity, ported unchanged from #667 (f74cf3b). management-api/credential-exposure.test.js drives the real v1 and v2 routers as an organization member, with repositories that attach the decrypted credential, and asserts no token, key or credential object appears in any entity or integration response. On the previous commit it fails on the three leaks above. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HyrF4oqcTdK2AtvcrocxE6
|
|
🚀 PR was released in |
Summary
The
authLambda serves core's integration router (createIntegrationRouter()plus the redirect route inhandlers/routers/auth.js). That router registers the v1 entity routes, but the API Gateway HTTP API thatbase-definition-factory.jsgenerates only declared/api/integrations,/api/integrations/{proxy+}and/api/authorize. HTTP APIs have no implicit catch-all, and the composed definition has no$defaultor/{proxy+}route, so every/api/entityand/api/entities/*request got a 404 from the gateway before it reached Express. This happened in deployed apps and underfrigg start(serverless-offline). The Frigg UI calls these routes fromIntegrationConfigurationModal.Fix: add
ANY /api/entity,ANY /api/entity/{proxy+},ANY /api/entitiesandANY /api/entities/{proxy+}to theauthfunction. Like/api/integrations, the bare/api/entitiespath is declared so the whole prefix belongs to the Management API. Express returns its own 404 for a path it doesn't handle.Guard:
infrastructure/__tests__/management-api-gateway-routes.test.jsreads the routes from a livecreateIntegrationRouter()instead of a hard-coded list. It composes a full app definition (with admin scripts, built-in reports, websockets, a webhook integration, an extension binding and an integration namedentities), with discovery and the Prisma layer stubbed. It then models HTTP API route selection and asserts that every Management API route selects a route onauth. A route added to core later (for example the v2 routes) fails this test until the gateway routes are added too. The test also keeps the neighbouring routes on their own functions and checks that no method + path pair is declared on two functions.Route table (composed app, after the fix)
HTTP API matching works like this: literal segments must match exactly, and
{param}matches one segment.{proxy+}is greedy and matches one or more trailing segments, so it does not match the bare prefix. That is why both/xand/x/{proxy+}are declared. A non-greedy match beats a greedy one, andANYmatches every method. There is no catch-all./api/integrations(integration-router)ANY /api/integrations→ auth/api/integrations/:integrationId,…/config/options,…/config/options/refresh,…/actions,…/actions/:actionId,…/actions/:actionId/options,…/actions/:actionId/options/refresh,…/test-authANY /api/integrations/{proxy+}→ auth/api/integrations/redirect/:appId(auth.js)ANY /api/integrations/{proxy+}→ auth/api/authorizeANY /api/authorize→ auth/api/entityANY /api/entity→ auth/api/entity/options/:credentialIdANY /api/entity/{proxy+}→ auth/api/entities/:entityIdANY /api/entities/{proxy+}→ auth/api/entities/:entityId/test-authANY /api/entities/{proxy+}→ auth/api/entities/:entityId/optionsANY /api/entities/{proxy+}→ auth/api/entities/:entityId/options/refreshANY /api/entities/{proxy+}→ auth/user/login,/user/create(user.js)ANY /user/{proxy+}→ user/health,/health/detailed,/health/live,/health/readyGET /health,GET /health/{proxy+}→ health/admin/db-migrate,/status,/resolve,/:migrationId/admin/scripts/*(admin-script-router)/api/v2/reports,/api/v2/reports/*(report-router)GET /api/v2/reports,GET/POST/PUT/DELETE /api/v2/reports/{proxy+}→ reportRouter/api/{name}-integration/webhooks[/:integrationId]{name}Webhook/api/{name}-integration/{binding}{path}(extensions){name}__{binding}/api/{name}-integration/*(Definition.routes)ANY /api/{name}-integration/{proxy+}→{name}entitiesgets/api/entities-integration/…, a different literal segment)Test plan
management-api-gateway-routes.test.js(12 tests). It fails onnextand lists the six unreachable entity routes ("no API Gateway route"). It passes with the fix.base-definition-factory.test.js, which now asserts the exactauthroute list (44/44 pass).npx jest infrastructurein devtools: the same 18 suites fail before and after this change. They are pre-existing, environment-dependent failures (AWS/discovery, Prisma layer build, health adapters), and nothing new fails.auth.handleragainst MongoDB, with theauthevents generated bycreateBaseDefinition:next):POST /api/entity,GET /api/entity/options/x,GET /api/entities/x,GET /api/entities/x/test-auth,POST /api/entities/x/optionsandPOST /api/entities/x/options/refreshall returned 404 from the gateway (existingRoutes: [/api/authorize, /api/integrations, /api/integrations/{proxy*}]).No valid authentication providedfrom Express, with routeKeysANY /api/entity,ANY /api/entity/{proxy+}andANY /api/entities/{proxy+}.GET /api/integrationsand/api/authorizestill return 401.🤖 Generated with Claude Code
📦 Published PR as canary version:
2.0.0--canary.667.a0620c7.0✨ Test out this PR locally via:
npm install @friggframework/admin-scripts@2.0.0--canary.667.a0620c7.0 npm install @friggframework/core@2.0.0--canary.667.a0620c7.0 npm install @friggframework/devtools@2.0.0--canary.667.a0620c7.0 npm install @friggframework/eslint-config@2.0.0--canary.667.a0620c7.0 npm install @friggframework/prettier-config@2.0.0--canary.667.a0620c7.0 npm install @friggframework/schemas@2.0.0--canary.667.a0620c7.0 npm install @friggframework/serverless-plugin@2.0.0--canary.667.a0620c7.0 npm install @friggframework/test@2.0.0--canary.667.a0620c7.0 npm install @friggframework/ui@2.0.0--canary.667.a0620c7.0 # or yarn add @friggframework/admin-scripts@2.0.0--canary.667.a0620c7.0 yarn add @friggframework/core@2.0.0--canary.667.a0620c7.0 yarn add @friggframework/devtools@2.0.0--canary.667.a0620c7.0 yarn add @friggframework/eslint-config@2.0.0--canary.667.a0620c7.0 yarn add @friggframework/prettier-config@2.0.0--canary.667.a0620c7.0 yarn add @friggframework/schemas@2.0.0--canary.667.a0620c7.0 yarn add @friggframework/serverless-plugin@2.0.0--canary.667.a0620c7.0 yarn add @friggframework/test@2.0.0--canary.667.a0620c7.0 yarn add @friggframework/ui@2.0.0--canary.667.a0620c7.0Note
Medium Risk
Touches credential validation, entity persistence, and API Gateway routing for paths the Frigg UI depends on; response shape for GET entity drops embedded credential data (intentional security change).
Overview
Fixes gateway 404s on v1 Management API entity paths by wiring
ANY /api/entity,/api/entity/{proxy+},/api/entities, and/api/entities/{proxy+}to theauthLambda. Addsmanagement-api-gateway-routes.test.jsso every route from a livecreateIntegrationRouter()must map toauth(and other Lambdas stay unshadowed).Core:
POST /api/entitynow goes throughCreateEntityFromCredential, which validates the caller’s credential (same 400 for missing/other-user IDs), builds the module from stored tokens, calls the provider viagetEntityDetails, and persists viaProcessAuthorizationCallback.findOrCreateEntityso it matches the OAuth callback path.GetModulestops returning credential secrets—onlycredentialIdandauthIsValid. Module lookup for stored entities uses sharedfindModuleDefinitionForEntity(moduleNameor legacymodelName). New router and use-case tests cover entity creation, idempotency, and auth boundaries.Reviewed by Cursor Bugbot for commit f74cf3b. Bugbot is set up for automated code reviews on this repo. Configure here.