fix: type a foreign key by the model it points at, and raise on a cross-type id - #66
Merged
Conversation
…mismatch A join model's foreign keys are also its primary key — `belongsToMany` drops the model's own `id` and makes `roleId`/`userId` the composite key — and both the encoder and `globalKeyTargets` tested `primaryKey` first. So `RoleUser.userId` was minted and demanded as a `RoleUser` id, though the value it holds is a `User` key: an id no client can produce from anywhere else in the schema. `foreignTarget` now wins wherever it exists. Two adapters could not name the target at all. Valkey's `ensureJoinModel` marked the auto-created join model's keys `foreignKey` without a `foreignTarget`; it now takes the two ends. Sequelize threw outright for a through model declared without its own `belongsTo`s, because sequelize builds a join model's `BelongsTo`s with `new BelongsTo(...)`, which never registers them on the model — it now falls back to the model owning the table the attribute references. And a cross-type id is an error rather than an empty result set. It used to be left undecoded, so it was compared literally and matched nothing, which is indistinguishable from a filter that legitimately found nothing. The check moves out of the codecs into `decodeGlobalId`: a codec's `null` already means "not one of mine, pass it through", so folding a forged id into the same answer made the two the same answer, and only the caller knows the field to name in the message. A codec declaring `carriesType: false` is exempt. Fixes #65 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013aahHVCcqUpyw8TfWM1gSs
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
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.
Fixes #65.
The bug
belongsToManydrops a join model's ownidand makes its two foreign keys the composite primary key, soRoleUser.roleId/RoleUser.userIdare a primary key and a foreign key at once. Both the encoder (create-basic-fields) and the decoder (globalKeyTargets) testedprimaryKeyfirst:So
RoleUser.userIdwas minted and demanded as aRoleUserid — an id no client can produce from anywhere else in the schema.foreignTargetnow wins wherever it exists. A shared-primary-key 1:1 table has the same shape and is fixed by the same rule.Adapters that could not name the target at all
ensureJoinModelmarked the auto-created join model's two keysforeignKey: truewith noforeignTarget, so they fell through the|| defNamefallback. It now takes the two ends.belongsTos threw at startup: sequelize builds a join model's twoBelongsTos withnew BelongsTo(...), which never registers them onthrough.model.associations. The attribute still carriesreferences, so the model owning that table is used before falling back to the throw.Cross-type ids now raise
Previously an id minted for the wrong type failed to decode; the opaque string was then compared literally and matched nothing — indistinguishable from a filter that legitimately found nothing.
GraphQLError,extensions.code = "GLOBAL_ID_TYPE_MISMATCH". The check moved out of the codecs intodecodeGlobalId: a codec'snullalready means "not one of mine, pass it through" (which is what lets a raw primary key survive a filter untouched), so folding a forged id into the same answer made the two the same answer — and only the caller knows which field to name. An out-of-tree codec gets the check for free; a codec declaringcarriesType: falseis exempt.node(id:)is unaffected — it decodes with no expected type, since the id is the type declaration there. It now filters on the already-decoded key rather than handing the global id back for a second decode, which a pk-and-fk column would have failed.Breaking changes (both noted in
docs/migration-6-to-7.md)IdCodecs should stop returningnullfor a recognised id whose type does not match.Verification
pnpm test— 9/9 packages, 1593 tests green (incl. the valkey suite against redis-memory-server).pnpm typecheckandpnpm lintclean.New coverage: end-to-end
Role -> RoleUser <- Userround-trip ingqlize/__tests__/codecs/id-regressions.test.ts; both join-model shapes (with and withoutbelongsToon the join) in the sequelize adapter'sreplace-id-codec.test.ts; a cross-adapter parity assertion in the valkeyrelations.test.ts. Every test that asserted the old "silently matches nothing" behaviour now asserts the error message.🤖 Generated with Claude Code
https://claude.ai/code/session_013aahHVCcqUpyw8TfWM1gSs