fix(core): strip virtual fields from include before Prisma call#633
Conversation
Virtual field keys named in `include`/`select` used to be forwarded straight through to Prisma, which threw "Unknown field" since virtual fields have no database column. The read path now strips virtual keys from the include payload (fragment, access-controlled merge, and sudo passthrough) while the value is still computed via resolveOutput regardless of whether the field was named. Closes #628 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A6fWyGeMiexwmQLD2usMcn
🦋 Changeset detectedLatest commit: bb84ccc The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
borisno2
left a comment
There was a problem hiding this comment.
Code Review
Overview
Fixes #628: a virtual field named in a caller-supplied include (e.g. context.db.myList.findMany({ include: { myVirtualField: true } })) used to be forwarded straight to Prisma and throw Unknown field at runtime, since virtual fields have no database column. This PR adds stripVirtualFieldsFromInclude() (in access-filter.ts) and applies it as the final step before model.findFirst/model.findMany in context/index.ts, covering all three paths that can produce include (fragment-built, access-controlled merge, sudo passthrough).
Correctness
- Core logic is sound. The strip check (
fieldConfig?.virtual) runs before the relationship check, so a virtual field is dropped regardless of whether its value is baretrueor an object — matches all the ways a caller could name it. - Nested relation includes preserve non-
includekeys correctly. The recursive branch spreads the originalvalue(not the narrowedentryfromasEntryObject) and only overridesinclude, sowhere/orderBy/take/skip/selecton a relation entry survive untouched. findFirstis covered for free since it delegates to the fixedfindMany.createGet(singleton) doesn't accept a callerincludeand its auto-built include only ever contains relationship keys, so it's correctly left untouched.- The sudo passthrough fix is a real, distinct improvement — previously sudo used
args.includecompletely unfiltered, so this bug applied there too. Good that it's covered by both the fix and a dedicated test.
Test coverage
Five new regression tests in context.test.ts under virtual fields named in include no longer reach Prisma (#628):
findMany/findUniquewith a virtual field + real relationship ininclude→ virtual key stripped, relationship include intact, virtual value still returned.- Virtual value populated when omitted from
include(pre-existing behavior, now guarded against regression). - Sudo path strips the virtual key while leaving the real relationship passthrough unfiltered (matches documented sudo semantics).
- Read access control on a virtual field still applies.
This satisfies all the acceptance criteria from the issue's triage brief. Full suite (729 tests), tsc build, and lint all pass with no regressions.
Minor/non-blocking observations
- Unresolvable related config skips nested stripping. If a declared relationship's
refcan't be resolved viagetRelatedListConfig(unlikely — config validation should prevent a dangling ref), the nestedincludeunder that relation is passed through without recursing. This mirrors an existing fallback pattern elsewhere in the codebase (e.g.filterReadableFields's "Related config not found, include the value as-is" branch), so it's consistent rather than a new gap. - Nested
selectinside a relation include entry (Prisma also supports{ include: { posts: { select: {...} } } }) isn't scanned for virtual keys. Out of scope per the issue — worth a follow-up only if that usage becomes common.
Security
No concerns — this only narrows what reaches Prisma; it doesn't loosen any access control path. Field-level read access on virtual fields and relationship-level access filtering both remain intact and tested.
Verdict
Solid, minimally-scoped fix with good test coverage matching the issue's acceptance criteria. No blocking issues found. Approving.
Generated by Claude Code
Coverage Report for Core Package Coverage (./packages/core)
File Coverage
|
||||||||||||||||||||||||||||||||||||||
Coverage Report for UI Package Coverage (./packages/ui)
File CoverageNo changed files found. |
Coverage Report for CLI Package Coverage (./packages/cli)
File CoverageNo changed files found. |
Coverage Report for Auth Package Coverage (./packages/auth)
File CoverageNo changed files found. |
Coverage Report for Storage Package Coverage (./packages/storage)
File CoverageNo changed files found. |
Coverage Report for RAG Package Coverage (./packages/rag)
File CoverageNo changed files found. |
Coverage Report for Storage S3 Package Coverage (./packages/storage-s3)
File CoverageNo changed files found. |
Coverage Report for Storage Vercel Package Coverage (./packages/storage-vercel)
File CoverageNo changed files found. |
Summary
include(e.g.context.db.myList.findMany({ include: { myVirtualField: true } })) used to be forwarded straight through to Prisma, which threwUnknown field '<name>' for include statementbecause virtual fields have no database column.stripVirtualFieldsFromInclude()inpackages/core/src/access/access-filter.ts, which recursively removes virtual-field keys from a Prismaincludeobject while preserving real relationship includes (and their nested access-control filters) at every level.context/index.tsfor bothfindUniqueandfindMany, afterincludeis resolved via the fragment path, the access-controlled merge path, or the sudo passthrough path — so a virtual key never reaches the Prisma client regardless of which path produced it.filterReadableFieldsalready computes it unconditionally viaresolveOutput, whether or not the field was named ininclude.Test plan
packages/core/tests/context.test.ts(virtual fields named in include no longer reach Prisma (#628)):findMany/findUniquewith a virtual field alongside a real relationship ininclude— the virtual key is stripped from the Prisma call, the relationship include is untouched, and the virtual value is still returned.include.pnpm test— fullpackages/coresuite passes (729 tests)pnpm build(tsc) passespnpm lint— no new warnings/errorspnpm format/pnpm manypkg fix— no changes needed beyond the diff@opensaas/stack-core: patch)Closes #628
Generated by Claude Code