fix: instance creation broken by sanitization guard, silent error swallow, and build failures#2608
Open
pastoriniMatheus wants to merge 2 commits into
Open
Conversation
…swallow
Fixes two cascading bugs that prevented instance creation and produced a
misleading `Setting_instanceId_fkey` FK constraint error at runtime, plus
three build-time issues that broke `npm run build` / `docker build`.
---
## Runtime: instance/create always returning 400
### Root cause 1 — sanitizeUntrustedInput stripping instanceName from body
`PROTECTED_INSTANCE_FIELDS` correctly blocks `instanceName` from being
overridden via the request body on routes that already receive it through
the URL parameter (e.g. `/message/sendText/:instanceName`). However,
`POST /instance/create` has no URL parameter — `instanceName` can only
come from the body.
`sanitizeUntrustedInput()` was silently discarding `instanceName` before
it reached the controller, making `instanceData.instanceName` undefined.
Prisma 7 treats `name: undefined` as a missing required field and rejects
the `instance.create()` call with "Argument name is missing".
Fix: for the `/instance/create` route, explicitly pass `instanceName`
through the sanitization barrier since it is a required creation input,
not an untrusted override attempt.
src/api/abstract/abstract.router.ts
### Root cause 2 — saveInstance() swallowing the create error silently
The try/catch in `saveInstance()` logged the error but did not rethrow
it. `createInstance()` assumed success and continued the flow, eventually
reaching `settingsService.create()` → `setSettings()` → `setting.upsert()`.
Because the `Instance` row was never committed, the FK constraint on
`Setting.instanceId` fired — surfacing as a confusing error deep inside
Typebot's `integrationSession.update()` call in the minified bundle.
Fix: rethrow after logging so the error propagates to the HTTP layer with
the correct message instead of a misleading FK violation.
Additionally, `setting.upsert()` is now called immediately after a
successful `instance.create()` inside `saveInstance()`. This guarantees
the `Setting` row exists before any chatbot integration can attempt to
write an `IntegrationSession` for the new instance, eliminating the race
window that could still trigger the FK error under concurrent load.
src/api/services/monitor.service.ts
---
## Build: npm run build / docker build failing
### tsup.config.ts — define block misplaced inside esbuildOptions callback
The `define` object (used to bake `__LICENSE_ENDPOINT_ENCODED__` and
`__LICENSE_ENDPOINT_XOR_KEY__` into the bundle at compile time) was
nested inside the `esbuildOptions(options) { ... }` callback instead of
being a root-level `defineConfig` property. The callback was also left
unclosed, producing a malformed config that tsup silently ignored,
resulting in a bundle with the licensing defines missing.
tsup.config.ts
### evohub.controlplane.router.ts — TypeScript type errors on req.params.id
Express types `req.params` properties as `string | undefined`. The two
call sites that forwarded `req.params.id` directly to functions expecting
`string` caused TypeScript strict-mode errors that aborted the build.
src/api/integrations/channel/evohub/evohub.controlplane.router.ts
### Dockerfile — prisma.config.ts missing from final stage
`prisma.config.ts` was only copied into the builder stage. The final
image did not include it, so `prisma migrate deploy` (executed at
container boot via `deploy_database.sh`) failed with:
Error: The datasource.url property is required
Fix: propagate the file from builder to the final stage.
Dockerfile
### .env.example — ?schema= param causes Prisma 7 to embed schema name in generated client
Prisma 7 with driver adapters reads the datasource URL at `prisma generate`
time and embeds the target schema name into every generated query. With
`?schema=evolution_api` in `DATABASE_CONNECTION_URI`, all generated SQL
used `"evolution_api"."Table"` prefixes. Because the actual database schema
is `public`, every query failed with "table not found".
Removing the `?schema=` parameter lets Prisma default to the `public`
schema, which matches the migration output and the actual DB layout.
The surrounding single-quotes were also removed — Docker Compose includes
them literally in the connection string, breaking the Postgres driver.
.env.example
### patches/ directory — COPY directive failing on missing path
Dockerfile referenced `COPY ./patches ./patches` but the directory was
absent from the repository, causing `docker build` to error before any
application code was processed.
patches/.gitkeep
Contributor
Reviewer's GuideFixes instance creation failures by preserving instanceName through sanitization and propagating DB errors correctly, and restores build/Docker reliability by correcting tsup config, TypeScript param types, Dockerfile copy paths, Prisma configuration, and env defaults. Sequence diagram for fixed /instance/create flow with sanitization and Setting upsertsequenceDiagram
actor Client
participant RouterBroker
participant sanitizeUntrustedInput
participant WAMonitoringService
participant PrismaInstance as prismaRepository.instance
participant PrismaSetting as prismaRepository.setting
Client->>RouterBroker: POST /instance/create { instanceName, integration }
RouterBroker->>sanitizeUntrustedInput: sanitizeUntrustedInput(body)
sanitizeUntrustedInput-->>RouterBroker: sanitized
RouterBroker->>RouterBroker: [ensure sanitized.instanceName from body]
RouterBroker->>WAMonitoringService: saveInstance(instanceData)
WAMonitoringService->>PrismaInstance: create(data)
alt [Instance created]
PrismaInstance-->>WAMonitoringService: Instance
WAMonitoringService->>PrismaSetting: setting.upsert({ where: { instanceId }, create: { ...defaults } })
PrismaSetting-->>WAMonitoringService: Setting
WAMonitoringService-->>RouterBroker: success
RouterBroker-->>Client: 201 Created
else [Instance.create throws]
PrismaInstance-->>WAMonitoringService: error
WAMonitoringService->>WAMonitoringService: logger.error(error)
WAMonitoringService-->>RouterBroker: throw error
RouterBroker-->>Client: 4xx/5xx with DB error message
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The special-casing for
/instance/createinabstract.router.tsusingrequest.originalUrl.includes('/instance/create')is a bit brittle; consider routing this exception via a more explicit flag or route metadata so future path refactors don’t silently break instance creation. - In
evohub.controlplane.router.ts, theas stringcasts onreq.params.idwill hide theundefinedcase at compile time; it would be safer to validate the param and return a 4xx if it’s missing rather than asserting the type. - The
setting.upsertinsaveInstance()hard-codes a full set of default booleans; if there’s (or will be) a single source of truth for default settings, it might be cleaner to centralize these defaults to avoid drift with other code paths that create or reset settings.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The special-casing for `/instance/create` in `abstract.router.ts` using `request.originalUrl.includes('/instance/create')` is a bit brittle; consider routing this exception via a more explicit flag or route metadata so future path refactors don’t silently break instance creation.
- In `evohub.controlplane.router.ts`, the `as string` casts on `req.params.id` will hide the `undefined` case at compile time; it would be safer to validate the param and return a 4xx if it’s missing rather than asserting the type.
- The `setting.upsert` in `saveInstance()` hard-codes a full set of default booleans; if there’s (or will be) a single source of truth for default settings, it might be cleaner to centralize these defaults to avoid drift with other code paths that create or reset settings.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The special-case logic for
/instance/createinsanitizeUntrustedInputcould become brittle over time; consider passing an explicit allowlist/flag into the sanitizer for that route instead of checkingoriginalUrl.includes('/instance/create')and manually re-addinginstanceName. - In
EvoHubControlPlaneRouter, rather than castingreq.params.id as string, it may be safer to validate thatidis present and return a 400 if missing to avoid unexpectedundefinedvalues at the client layer. - When rethrowing the error in
saveInstance, consider wrapping or augmenting it with context (e.g. instance identifiers or operation name) so upstream log aggregation is easier to correlate with the failing operation.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The special-case logic for `/instance/create` in `sanitizeUntrustedInput` could become brittle over time; consider passing an explicit allowlist/flag into the sanitizer for that route instead of checking `originalUrl.includes('/instance/create')` and manually re-adding `instanceName`.
- In `EvoHubControlPlaneRouter`, rather than casting `req.params.id as string`, it may be safer to validate that `id` is present and return a 400 if missing to avoid unexpected `undefined` values at the client layer.
- When rethrowing the error in `saveInstance`, consider wrapping or augmenting it with context (e.g. instance identifiers or operation name) so upstream log aggregation is easier to correlate with the failing operation.
## Individual Comments
### Comment 1
<location path="src/api/services/monitor.service.ts" line_range="263-259" />
<code_context>
+ // Ensure Setting record exists immediately after Instance creation.
+ // Prevents FK constraint violations (Setting_instanceId_fkey) in chatbot
+ // integrations that write to IntegrationSession before setSettings() runs.
+ await this.prismaRepository.setting.upsert({
+ where: { instanceId: data.instanceId },
+ update: {},
+ create: {
+ instanceId: data.instanceId,
+ rejectCall: false,
+ groupsIgnore: false,
+ alwaysOnline: false,
+ readMessages: false,
+ readStatus: false,
+ syncFullHistory: false,
+ },
+ });
} catch (error) {
this.logger.error(error);
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider wrapping instance creation and Setting upsert in a single transaction for atomicity.
If the Instance is created but the Setting upsert fails (e.g., transient DB error), you can still end up with an Instance without a Setting, which undermines the intent of this change. Wrapping both operations in `this.prismaRepository.$transaction` would keep them atomic and prevent this partial state.
Suggested implementation:
```typescript
businessId: data.businessId,
},
});
await this.prismaRepository.$transaction(async (tx) => {
// Ensure Instance creation and Setting record creation happen atomically.
await tx.instance.create({
data: {
businessId: data.businessId,
},
});
// Ensure Setting record exists immediately after Instance creation.
// Prevents FK constraint violations (Setting_instanceId_fkey) in chatbot
// integrations that write to IntegrationSession before setSettings() runs.
await tx.setting.upsert({
where: { instanceId: data.instanceId },
update: {},
create: {
instanceId: data.instanceId,
rejectCall: false,
groupsIgnore: false,
alwaysOnline: false,
readMessages: false,
readStatus: false,
syncFullHistory: false,
},
});
});
```
The `tx.instance.create` block in the replacement currently only includes `businessId: data.businessId` because only that part of the original `data` object was visible in the snippet. To avoid changing behavior, the full `data` payload used in the original `this.prismaRepository.instance.create({ data: { ... } })` call must be copied into the new `tx.instance.create({ data: { ... } })` block, preserving all fields that were previously set (e.g., any other properties before `businessId`).
</issue_to_address>
### Comment 2
<location path="src/api/abstract/abstract.router.ts" line_range="54-59" />
<code_context>
if (request.originalUrl.includes('/instance/create')) {
- Object.assign(instance, sanitizeUntrustedInput(body));
+ const sanitized = sanitizeUntrustedInput(body);
+ // instanceName must come from the body on create — there is no URL param on this route
+ if (body?.instanceName !== undefined) {
+ sanitized.instanceName = body.instanceName;
+ }
+ Object.assign(instance, sanitized);
}
</code_context>
<issue_to_address>
**🚨 issue (security):** Overriding `instanceName` after sanitization may reintroduce unsafe data from the raw body.
By setting `sanitized.instanceName = body.instanceName` after calling `sanitizeUntrustedInput`, you circumvent any validation or filtering that function would apply to `instanceName`. If `sanitizeUntrustedInput` is responsible for enforcing allowed keys or validating value formats, consider either updating it to explicitly handle `instanceName` or adding explicit validation here before assigning, rather than copying the raw value.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -257,8 +257,25 @@ export class WAMonitoringService { | |||
| businessId: data.businessId, | |||
| }, | |||
| }); | |||
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Consider wrapping instance creation and Setting upsert in a single transaction for atomicity.
If the Instance is created but the Setting upsert fails (e.g., transient DB error), you can still end up with an Instance without a Setting, which undermines the intent of this change. Wrapping both operations in this.prismaRepository.$transaction would keep them atomic and prevent this partial state.
Suggested implementation:
businessId: data.businessId,
},
});
await this.prismaRepository.$transaction(async (tx) => {
// Ensure Instance creation and Setting record creation happen atomically.
await tx.instance.create({
data: {
businessId: data.businessId,
},
});
// Ensure Setting record exists immediately after Instance creation.
// Prevents FK constraint violations (Setting_instanceId_fkey) in chatbot
// integrations that write to IntegrationSession before setSettings() runs.
await tx.setting.upsert({
where: { instanceId: data.instanceId },
update: {},
create: {
instanceId: data.instanceId,
rejectCall: false,
groupsIgnore: false,
alwaysOnline: false,
readMessages: false,
readStatus: false,
syncFullHistory: false,
},
});
});The tx.instance.create block in the replacement currently only includes businessId: data.businessId because only that part of the original data object was visible in the snippet. To avoid changing behavior, the full data payload used in the original this.prismaRepository.instance.create({ data: { ... } }) call must be copied into the new tx.instance.create({ data: { ... } }) block, preserving all fields that were previously set (e.g., any other properties before businessId).
Comment on lines
+54
to
+59
| const sanitized = sanitizeUntrustedInput(body); | ||
| // instanceName must come from the body on create — there is no URL param on this route | ||
| if (body?.instanceName !== undefined) { | ||
| sanitized.instanceName = body.instanceName; | ||
| } | ||
| Object.assign(instance, sanitized); |
Contributor
There was a problem hiding this comment.
🚨 issue (security): Overriding instanceName after sanitization may reintroduce unsafe data from the raw body.
By setting sanitized.instanceName = body.instanceName after calling sanitizeUntrustedInput, you circumvent any validation or filtering that function would apply to instanceName. If sanitizeUntrustedInput is responsible for enforcing allowed keys or validating value formats, consider either updating it to explicitly handle instanceName or adding explicit validation here before assigning, rather than copying the raw value.
Before running `prisma migrate deploy`, the entrypoint now attempts to connect to the database server's admin database (`postgres` / default) and issues `CREATE DATABASE` only when the target database is absent. Motivation: a fresh install pointing at a managed Postgres (RDS, Supabase, self-hosted) fails immediately if the database has not been pre-created. The container logs showed a cryptic migration error rather than an actionable message about a missing database. ### What changed `Docker/scripts/create_database.js` (new) - Parses DATABASE_CONNECTION_URI to extract host, port, credentials, and target database name. - Connects to the admin database (`postgres` for PG, no DB for MySQL). - PostgreSQL: `SELECT 1 FROM pg_database WHERE datname = $1` — creates only when rowcount is 0. - MySQL: `CREATE DATABASE IF NOT EXISTS` (idempotent by design). - Validates the database name against `/^[\w-]+$/` before interpolating it into the DDL to prevent injection. - Non-fatal by design: if the user lacks CREATE DATABASE permission (managed DB with restricted roles, read replica, etc.) the error is logged as a warning and the script exits 0 so `prisma migrate deploy` still runs and produces a clear, actionable error. - Connection timeout set to 10 s to fail fast on unreachable hosts. `Docker/scripts/deploy_database.sh` - Added `node ./Docker/scripts/create_database.js` call before the existing `npm run db:deploy` step. ### Behaviour | Scenario | Result | |---|---| | Database does not exist, user has CREATE DATABASE | DB created, migrations run | | Database already exists | Skipped, migrations run normally | | Database does not exist, no CREATE permission | Warning logged, migration fails with clear Prisma error | | DATABASE_CONNECTION_URI not set | Warning logged, exits 0 |
|
🟢 👍 Mira a |
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
2 participants
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.
Summary
This PR fixes two cascading runtime bugs that made `POST /instance/create` always return 400, plus three build-time issues that broke `npm run build` and `docker build` on the v2.4.0 branch.
Runtime fixes
[1] `sanitizeUntrustedInput` stripping `instanceName` from the create body
File: `src/api/abstract/abstract.router.ts`
`PROTECTED_INSTANCE_FIELDS` correctly blocks `instanceName` from being overridden via the body on routes that already receive it through the URL parameter (e.g. `/message/sendText/:instanceName`). However, `POST /instance/create` has no URL parameter — `instanceName` can only come from the body.
`sanitizeUntrustedInput()` was silently discarding `instanceName` before it reached the controller, making `instanceData.instanceName` undefined. Prisma 7 treats `name: undefined` as a missing required field and rejects `instance.create()` with "Argument name is missing".
Fix: for the `/instance/create` route, explicitly pass `instanceName` through the sanitization barrier since it is a required creation input, not an untrusted override attempt.
[2] `saveInstance()` swallowing the create error silently
File: `src/api/services/monitor.service.ts`
The `try/catch` in `saveInstance()` logged the error but did not rethrow it. `createInstance()` assumed success and continued the flow, eventually reaching `settingsService.create()` → `setSettings()` → `setting.upsert()`. Because the `Instance` row was never committed, the FK constraint on `Setting.instanceId` fired — surfacing as a confusing error deep inside Typebot's `integrationSession.update()` call in the minified bundle, pointing nowhere near the real cause.
Fix: rethrow after logging so the error propagates to the HTTP layer with the correct message. Additionally, `setting.upsert()` is now called immediately after a successful `instance.create()` inside `saveInstance()`, guaranteeing the `Setting` row exists before any chatbot integration can attempt to write an `IntegrationSession` for the new instance.
Build fixes
[3] `tsup.config.ts` — `define` block misplaced inside `esbuildOptions` callback
The `define` object (bakes `LICENSE_ENDPOINT_ENCODED` and `LICENSE_ENDPOINT_XOR_KEY` into the bundle at compile time) was nested inside the `esbuildOptions(options) { ... }` callback instead of being a root-level `defineConfig` property. The callback was also left unclosed, producing a malformed config tsup silently ignored.
[4] `evohub.controlplane.router.ts` — TypeScript type errors on `req.params.id`
Express types `req.params` properties as `string | undefined`. Two call sites forwarded `req.params.id` directly to functions expecting `string`, causing TypeScript strict-mode errors that aborted the build (lines 40 and 117).
[5] `Dockerfile` — `prisma.config.ts` missing from the final stage
`prisma.config.ts` was only copied into the builder stage. The final image did not include it, so `prisma migrate deploy` at container boot failed with:
[6] `.env.example` — `?schema=` param causes Prisma 7 to embed wrong schema in generated client
Prisma 7 with driver adapters reads the datasource URL at `prisma generate` time and embeds the schema name into every generated query. With `?schema=evolution_api`, all SQL used `"evolution_api"."Table"` prefixes. The actual database schema is `public`, so every query failed with table not found. Removed `?schema=evolution_api` and the surrounding single-quotes (Docker Compose includes them literally in the connection string).
[7] `patches/` directory — `COPY` directive failing on missing path
Dockerfile referenced `COPY ./patches ./patches` but the directory was absent from the repository, failing `docker build` before any application code was processed.
Files changed
Test plan
Summary by Sourcery
Fix instance creation failures caused by overzealous request sanitization and swallowed persistence errors, and restore broken build and Docker workflows.
Bug Fixes:
Build: