Skip to content

feat(workflow-executor): validate bearer claims via a zod middleware [PRD-508]#1659

Merged
Scra3 merged 2 commits into
mainfrom
feature/prd-508-validate-executor-http-route-inputs-early-via-a-zod
Jun 12, 2026
Merged

feat(workflow-executor): validate bearer claims via a zod middleware [PRD-508]#1659
Scra3 merged 2 commits into
mainfrom
feature/prd-508-validate-executor-http-route-inputs-early-via-a-zod

Conversation

@Scra3

@Scra3 Scra3 commented Jun 11, 2026

Copy link
Copy Markdown
Member

What

Validates the bearer JWT payload as early as possible via a zod middleware, instead of the ad-hoc Number.isFinite check in handleTrigger. Follow-up to PRD-477.

koa-jwt only validates a token's signature + expiry — never the payload shape. So ctx.state.user.id was not guaranteed; hasRunAccessMiddleware even did as StepUser without validating (latent gap).

Changes

  • src/http/bearer-claims.tsBearerClaimsSchema = z.object({ id: z.number() }). Non-strict on purpose: jsonwebtoken adds iat/exp to the decoded payload and Forest may send extra claims — a strict schema would reject every real token. Only id is consumed downstream (handleTrigger + hasRunAccess?userId=); StepUserSchema (9 required fields) is a different contract and isn't reusable.
  • Middleware right after koa-jwt — validates ctx.state.user once; on failure throws UnauthorizedHttpError. Both routes now receive a user with a guaranteed numeric id.
  • handleTrigger — drops the Number.isFinite branch.

⚠️ Behaviour change

A well-signed token without a valid numeric id now returns 401 Unauthorized (invalid credentials, consistent with koa-jwt's own 401s) instead of 400.

Out of scope (decided)

pendingData is not boundary-validatable — its schema is step-type-dependent (patchBodySchemas[execution.type]), known only after getAvailableRun. It stays validated downstream in base-step-executor. A boundary z.union/z.discriminatedUnion was considered and rejected (accepts the wrong step type; .transform() footgun; no discriminator in the front-sent payload).

Tests

  • test/http/bearer-claims.test.ts: accepts { id } and { id, iat, exp, … } (the non-strict trap), rejects missing/non-numeric id.
  • test/http/executor-http-server.test.ts: invalid claims → 401; a token with extra claims → 200 (end-to-end non-strict proof).
  • Full suite: 1025 passing.

Note

Stacked on feature/prd-477-… (base of this PR) — it builds on the typed HTTP errors / middleware from PRD-477. Retarget to main once PRD-477 merges.

fixes PRD-508

Note

Validate bearer JWT claims via Zod middleware in workflow-executor HTTP server

  • Adds a Zod-based middleware in executor-http-server.ts that validates ctx.state.user against BearerClaimsSchema (integer id required); invalid payloads receive 401 before reaching any route handler.
  • Introduces http-errors.ts with typed HTTP error classes and a toHttpError utility that maps domain error categories to HTTP statuses: 404 for NotFoundError, 403 for AccessDeniedError, 503 for UnavailableError, 400 for uncategorized WorkflowExecutorError, and 401 for koa-jwt errors.
  • Refactors domain errors in errors.ts into abstract category base classes (NotFoundError, AccessDeniedError, UnavailableError) so HTTP translation is driven by error category rather than per-handler branching.
  • Removes manual user-id extraction and per-error branching from route handlers; all error responses are now handled by a centralized error-translation middleware.
  • UserMismatchError now requires both bearerUserId and ownerUserId in its constructor, including both in the technical log message while keeping a generic user-facing message.

Macroscope summarized bde84af.

@linear-code

linear-code Bot commented Jun 11, 2026

Copy link
Copy Markdown

PRD-508

@qltysh

qltysh Bot commented Jun 11, 2026

Copy link
Copy Markdown

Qlty


Coverage Impact

This PR will not change total coverage.

Modified Files with Diff Coverage (2)

RatingFile% DiffUncovered Line #s
Coverage rating: A Coverage rating: A
packages/workflow-executor/src/http/executor-http-server.ts100.0%
New Coverage rating: A
packages/workflow-executor/src/http/bearer-claims.ts100.0%
Total100.0%
🚦 See full report on Qlty Cloud »

🛟 Help
  • Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

  • Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

  • File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

    • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

Base automatically changed from feature/prd-477-refactor-centralize-executor-http-error-mapping-typed-http to main June 12, 2026 13:44
@qltysh

qltysh Bot commented Jun 12, 2026

Copy link
Copy Markdown

All good ✅

@matthv matthv left a comment

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.

LGTM

alban bertolini and others added 2 commits June 12, 2026 15:47
…[PRD-508]

koa-jwt validates a token's signature/expiry but not its payload shape. Add a
BearerClaimsSchema (non-strict z.object({ id: z.number() }) — tolerates iat/exp
and extra Forest claims) and a middleware right after koa-jwt that validates
ctx.state.user once. handleTrigger drops its ad-hoc Number.isFinite check, and
both routes now get a user with a guaranteed numeric id.

Behaviour change: a well-signed token without a valid numeric id now returns 401
(invalid credentials) instead of 400.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…view [PRD-508]

- Narrow hasRunAccess(runId, user) to { id: number } (only the id is used), removing
  the unsound `as StepUser` cast on the validated bearer user — both routes now cast
  to BearerClaims consistently.
- Log invalid bearer claims (logger.warn, issue paths/codes only — never the payload)
  before the 401: a signed-but-malformed token is rare and high-signal, not churn.
- Tighten BearerClaimsSchema id to z.number().int() (rejects NaN/Infinity/floats),
  preserving the previous Number.isFinite invariant.
- Tests: GET route 401 on invalid claims (access check not reached), warn assertion,
  non-integer id rejection.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Scra3 Scra3 force-pushed the feature/prd-508-validate-executor-http-route-inputs-early-via-a-zod branch from deb01a4 to bde84af Compare June 12, 2026 13:48
@Scra3 Scra3 merged commit 5a0a63c into main Jun 12, 2026
30 checks passed
@Scra3 Scra3 deleted the feature/prd-508-validate-executor-http-route-inputs-early-via-a-zod branch June 12, 2026 15:06
forest-bot added a commit that referenced this pull request Jun 12, 2026
# @forestadmin/workflow-executor [1.2.0](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/workflow-executor@1.1.5...@forestadmin/workflow-executor@1.2.0) (2026-06-12)

### Features

* **workflow-executor:** validate bearer claims via a zod middleware [PRD-508] ([#1659](#1659)) ([5a0a63c](5a0a63c))
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.

2 participants