Conversation
fastify-type-provider-zod@6.1.0 does not apply Zod `.transform()`
outputs to `request.query` on GET routes the way it does for POST
bodies. A single `?resource=X` arrived as the bare string `'X'`, but
the authorize handler passed it straight to
`authorizationCodes.create({ resource: ... })` where the `jsonb`
column expects an array. In practice that meant the auth code stored
`resource: []` (the `'' ?? []` fallback path was never taken — but
the NOT-NULL DEFAULT `'[]'::jsonb` fired and the transform-produced
array was dropped by the ORM somewhere before insert).
End effect: every browser-driven PKCE flow with a single `resource`
param got back a token request the AS then rejected with
`invalid_target: requested resource is outside the authorization-code
binding`, because the token body carried the resource (POST-body
transforms work) but the code row did not.
Fix: introduce a `normalizeResource(v: unknown): string[]` helper at
the top of `authorize.ts` and call it on `query.resource` before
persisting. Schema-level Zod validation still runs (URI shape, no
fragment, length caps) — only the array coercion is duplicated here.
Test: adds an authorize test with `query.resource: 'string-value'`
(not an array) simulating the raw-string arrival, asserting the code
row is created with `resource: ['string-value']`.
172/172 auth-server tests pass.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
View your CI Pipeline Execution ↗ for commit b8a73c9
☁️ Nx Cloud last updated this comment at |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces manual normalization for the resource query parameter in the OAuth authorize route. Due to limitations in fastify-type-provider-zod@6.1.0 where Zod transforms are not applied to GET query parameters, a single resource value could arrive as a string instead of an array. The changes include a new normalizeResource helper function and a corresponding test case to ensure single-value resources are correctly coerced into an array. I have no feedback to provide.
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
1 participant
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
End-to-end bug caught after deploying #160: every browser-driven PKCE flow with a single
resourcequery param gotinvalid_target: requested resource is outside the authorization-code bindingat/oauth/tokentime. Production traffic showed dozens of failed exchanges despite clients correctly sending?resource=https://mcp.example.com.Root cause:
fastify-type-provider-zod@6.1.0does not apply Zod.transform()outputs torequest.queryon GET routes the way it does for POST bodies.resourceParamSchematransformsstring → string[], so the schema test (which hands it an array) passed, but real browser traffic (single value → raw string) landed in the handler as a bare string and was dropped when written to thejsonb NOT NULL DEFAULT '[]'::jsonbcolumn.Token-side binding check then rejected the exchange because
authCode.resourcewas[]whilebody.resource(POST body transform does work) carried the requested URI.Fix
Tiny
normalizeResource(v: unknown): string[]helper at the top ofauthorize.ts, called onquery.resourcebefore persistence. Schema-level validation (URI shape, no fragment, length caps) still runs — only the array coercion is duplicated.Test plan
query.resource: 'bare-string'(not an array) → code row persisted withresource: ['bare-string']aud=<resource URI>and memory-mcp accepts it🤖 Generated with Claude Code