Skip to content

fix(orchestrator): makes the bodyParser limit configurable.#2802

Open
lholmquist wants to merge 6 commits intoredhat-developer:mainfrom
lholmquist:RHDHSUPP-351
Open

fix(orchestrator): makes the bodyParser limit configurable.#2802
lholmquist wants to merge 6 commits intoredhat-developer:mainfrom
lholmquist:RHDHSUPP-351

Conversation

@lholmquist
Copy link
Copy Markdown
Member

Hey, I just made a Pull Request!

A customer was having an issue where the workflows content length was over the 100kb limit. This adds a configurable value to make the content lenght larger

fixes: https://redhat.atlassian.net/browse/RHDHSUPP-351

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge bot commented Apr 16, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. New config key missing from schema🐞
Description
The code reads orchestrator.contentLengthLimit via config.getOptionalString, but this key is not
declared in plugins/orchestrator-common/config.d.ts, which is the plugin's registered
configSchema. Backstage uses this schema for config validation and documentation; the missing
declaration means strict validation will reject the key, and IDE/tooling autocomplete won't surface
it.
Code

workspaces/orchestrator/plugins/orchestrator-backend/src/service/router.ts[R204-206]

+  const contentLengthLimit = config.getOptionalString(
+    'orchestrator.contentLengthLimit',
+  );
Relevance

⭐⭐⭐ High

Team maintains orchestrator config.d.ts; missing schema for new key likely flagged and fixed for
validation/tooling.

PR-#2749
PR-#2386

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The config schema file (config.d.ts) is declared as the configSchema in
orchestrator-common/package.json. The schema defines all known orchestrator.* keys but has no
contentLengthLimit field, so the new key is undeclared.

workspaces/orchestrator/plugins/orchestrator-common/package.json[41-41]
workspaces/orchestrator/plugins/orchestrator-common/config.d.ts[17-144]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new `orchestrator.contentLengthLimit` config key is consumed by the backend router but is not declared in the plugin's config schema. Backstage validates config against the declared schema; an undeclared key will cause validation failures in strict mode and won't appear in generated documentation or IDE autocomplete.

## Issue Context
The config schema is defined in `config.d.ts` and referenced as `configSchema` in `package.json`. All other `orchestrator.*` keys (e.g., `sonataFlowService`, `dataIndexService`, `kafka`) are declared there.

## Fix Focus Areas
- workspaces/orchestrator/plugins/orchestrator-common/config.d.ts[21-144]

Add an optional `contentLengthLimit` string field inside the `orchestrator` interface, with a JSDoc comment explaining the default (100 KB / 102400 bytes) and accepted format (e.g., `'10mb'`, `'500kb'`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Global limit applied to all routes🐞
Description
The configurable contentLengthLimit is passed to the root-level express.json() middleware, so it
applies to every orchestrator endpoint, not just the /workflows routes that need larger payloads.
If a user sets a very large limit (e.g., 500mb) to accommodate large workflow definitions, all
other endpoints (health, permissions, etc.) will also accept bodies of that size, unnecessarily
expanding the attack surface for memory-exhaustion requests.
Code

workspaces/orchestrator/plugins/orchestrator-backend/src/service/router.ts[R207-209]

+  router.use(express.json({ limit: contentLengthLimit }));
  router.use(permissionsIntegrationRouter);
-  router.use('/workflows', express.text());
+  router.use('/workflows', express.text({ limit: contentLengthLimit }));
Relevance

⭐⭐ Medium

Security concern is valid, but repo often mounts global parsers; unclear they’ll require
route-scoped limits.

PR-#2581
PR-#2597

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
express.json() is mounted at the router root (line 207) without any path restriction, so it runs for
every request. The /workflows-specific express.text() (line 209) also uses the same limit. A
separate, tighter limit for the root-level JSON parser would confine the large-body allowance to
only the routes that need it.

workspaces/orchestrator/plugins/orchestrator-backend/src/service/router.ts[207-209]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The configurable body-size limit is applied to the global `express.json()` middleware, meaning every orchestrator endpoint accepts bodies up to the configured limit. Only the `/workflows` routes actually need a larger limit.

## Issue Context
The intent is to allow large workflow definitions to be uploaded. Other endpoints (health checks, permission checks, etc.) have no need for a large body limit.

## Fix Focus Areas
- workspaces/orchestrator/plugins/orchestrator-backend/src/service/router.ts[204-209]

Keep the root-level `express.json()` at the default limit (omit the `limit` option or use a fixed small default). Apply the configurable `contentLengthLimit` only to the `/workflows` middleware, e.g.:
```ts
router.use(express.json()); // default 100kb for all routes
router.use('/workflows', express.text({ limit: contentLengthLimit })); // configurable for workflow uploads
```

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@rhdh-gh-app
Copy link
Copy Markdown

rhdh-gh-app bot commented Apr 16, 2026

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/backstage-plugin-orchestrator-backend workspaces/orchestrator/plugins/orchestrator-backend patch v8.8.2
@red-hat-developer-hub/backstage-plugin-orchestrator-common workspaces/orchestrator/plugins/orchestrator-common patch v3.6.1

@rhdh-qodo-merge
Copy link
Copy Markdown

Review Summary by Qodo

Make bodyParser content length limit configurable

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Makes bodyParser content length limit configurable via config
• Allows workflows to exceed default 100kb limit
• Applies limit to both JSON and text request parsers
• Includes configuration example with 10mb default suggestion
Diagram
flowchart LR
  config["Config: orchestrator.contentLengthLimit"]
  jsonParser["express.json with limit"]
  textParser["express.text with limit"]
  config -- "applies to" --> jsonParser
  config -- "applies to" --> textParser
Loading

Grey Divider

File Changes

1. workspaces/orchestrator/plugins/orchestrator-backend/src/service/router.ts ✨ Enhancement +5/-2

Add configurable content length limit to parsers

• Reads orchestrator.contentLengthLimit from configuration
• Passes limit to express.json() middleware
• Passes limit to express.text() middleware for workflows endpoint

workspaces/orchestrator/plugins/orchestrator-backend/src/service/router.ts


2. workspaces/orchestrator/app-config.yaml 📝 Documentation +2/-0

Document configurable content length limit option

• Adds commented example configuration for contentLengthLimit
• Documents default value of 102400 bytes (100kb)
• Shows example setting of 10mb for larger workflows

workspaces/orchestrator/app-config.yaml


3. workspaces/orchestrator/.changeset/purple-eagles-share.md ⚙️ Configuration changes +5/-0

Add changeset for bodyParser limit fix

• Creates changeset entry for the fix
• Documents change in orchestrator-backend package

workspaces/orchestrator/.changeset/purple-eagles-share.md


Grey Divider

Qodo Logo

@rhdh-qodo-merge rhdh-qodo-merge bot added enhancement New feature or request bug_fix labels Apr 16, 2026
Copy link
Copy Markdown
Member

@PatAKnight PatAKnight left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add some docs to this as well. I don't know where the current product docs live for the orchestrator plugin but having a small blurb here can give us something to point to for the docs team to ingest.

Comment thread workspaces/orchestrator/app-config.yaml
const contentLengthLimit = config.getOptionalString(
'orchestrator.contentLengthLimit',
);
router.use(express.json({ limit: contentLengthLimit }));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
router.use(express.json({ limit: contentLengthLimit }));
router.use(express.json());

It looks like the QODO review is complaining about having the limit added globally as well as the /workflows endpoint. I agree with the suggestion to drop the limit globally and just have the one on the endpoints that best need the larger limits.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So i decided to ask cursor about that /workflows endpoint and it doesn't look like it is actually being used. In fact, if you try to go to it, it will return a 404. The endpoint to actually interact with workflows is /v2/workflows/.... i should probably remove the limit config from that /workflows endpoint and it should be kept on the global one

@lholmquist lholmquist requested a review from PatAKnight April 17, 2026 14:20
@sonarqubecloud
Copy link
Copy Markdown

@lholmquist
Copy link
Copy Markdown
Member Author

@PatAKnight I added a comment on the router, which i think we can point the docs to at, https://github.com/redhat-developer/rhdh-plugins/pull/2802/changes#diff-b34e43c026bf84e8d06cc07eceed2806b08b96206b3c033d493ec8a7b1ce3ceeR208-R215

Also see my comment about the /workflows route, #2802 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants