Repository navigation
Description
Custom subject IDs are stored as <group name with spaces → _>$<id>, but group names are never checked against that encoding. There are two consequences. A group name containing $ breaks unscoping: removeSubjectIdScope returns id.split('$')[1], which is the part of the group name after its $, not the subject's ID. In a group named R$D Lab, subjects S001 and S002 are stored as R$D_Lab$S001 and R$D_Lab$S002, and both display and export as D_Lab. The datahub subjects table, the start-session ID suggestions and the subjectId column of every instrument-record export then show one ID for the whole group, and exported records can no longer be told apart by subject. Second, two groups whose names differ only by space versus underscore (My Group and My_Group) share one ID namespace. Custom ID P001 in either group is the same My_Group$P001 subject document. SessionsService reuses the existing subject and adds the second group to it, so two different people are merged into one subject, and the second keeps the first one's date of birth and sex. $CreateGroupData accepts both names, and so does the duplicate-name check in GroupsService.create.
Where
packages/subject-utils/src/index.ts:54-56:
export function encodeScopedSubjectId(id: string, options: { groupName: string }) {
return options.groupName.replaceAll(' ', '_') + '$' + id;
}packages/runtime-internal/src/index.js:26-31:
export function removeSubjectIdScope(id) {
if (id.includes('$')) {
return /** @type {string} */ (id.split('$')[1]);
}
return id;
}The group name gets no format check at packages/schemas/src/group/group.ts:50 (name: z.string().min(1)), and apps/api/src/groups/groups.service.ts:20 checks uniqueness on the raw name. Unscoped IDs are read at apps/web/src/routes/_app/datahub/subjects/index.tsx:498 and apps/api/src/instrument-records/export-worker.js:103 and :125.
Reproduce
- As an admin, create a group named
R$D Lab. - In it, start sessions for custom IDs
S001andS002, and complete a form for each. - Open Data Hub, then export the records as CSV.
Actual: both subjects are listed as D_Lab, and every exported row has subjectId D_Lab.
Expected: either the group name is refused at creation, or the subjects show and export as S001 and S002.
For the namespace collision, create groups My Group and My_Group, then start a session for custom ID P001 in each with different dates of birth. Both groups now point at a single subject, My_Group$P001, with the first date of birth.
Tests
packages/schemas/src/group/__tests__/group.test.ts:it('should reject a group name containing $, since it is the separator in scoped subject IDs').apps/api/src/groups/__tests__/groups.service.test.ts:it('should refuse a group name that scopes subject IDs the same way as an existing group, so their custom IDs cannot merge').packages/runtime-internal/src/__tests__/index.test.ts:it('should keep everything after the first $ as the subject ID').- E2E in
testing/: creating a group namedR$D Labshows a validation error.
Suggested fix
Refuse $ in $CreateGroupData.name. $UpdateGroupData inherits it from $Group, so refuse it there too, or derive both from a shared $GroupName. In GroupsService.create and the rename path of updateById, check uniqueness on name.replaceAll(' ', '_') rather than on the raw name. To check existing data first, look for groups whose name contains $ and for pairs of names that collide once spaces become underscores. Separately, make removeSubjectIdScope return everything after the first $ (id.slice(id.indexOf('$') + 1)) so that it is the exact inverse of encodeScopedSubjectId for any ID.
Activity
joshunrau commentedon Oct 26, 2026
Still reproduces on
main(d86740e). Nothing has touchedruntime-internal,subject-utils,schemas/src/grouporgroups.service.tssince this was filed, and no open PR touches them.Unscope on the last
$, not the firstThe suggested
id.slice(id.indexOf('$') + 1)does not fix the reported case:R$D_Lab$S001becomesD_Lab$S001. Refusing$in new names leaves existing$groups broken, and repairing them would mean rewriting subject primary keys that sessions, records and assignments reference.Use
return id.slice(id.lastIndexOf('$') + 1);.lastIndexOfreturns-1for an unscoped ID, so theincludesbranch can go. ExistingR$D Labsubjects then display and export correctly with no data change. The UI refuses$in a custom ID:StartSessionForm.tsx:256since a301eaf, which introduced scoped IDs, and the upload'sSUBJECT_ID_REGEX(upload.ts:20) since 96e8c46, first released in v1.8.5. CSV upload accepted$from v1.4.0 to v1.8.4, so an older instance can holdGroup$A$B. That shows asAtoday and would show asB. No split of the string alone is right for both that andR$D_Lab$S001, so the existing-data check should also look for subjects with more than one$that no$group explains. Still refuse$in new names, so the first and last$agree from here on.The same function builds the set that
apps/web/src/utils/table.ts:14uses to filter the Data Hub export (datahub/subjects/index.tsx:358-360). In a$group, narrowing the table to one subject therefore still exports the whole group.Where the name rule goes
$CreateGroupData.name, not on$Group.$Groupparses every groups response (useGroupsQuery.ts:11,useGroupQuery.ts:12,useUpdateGroupMutation.ts:32,admin/groups/index.tsx:40) and$AuditLogpicks itsname(audit.ts:47), so a rule there would break the group list and the audit log on any instance that already has a$group. This is the splitpackages/schemas/AGENTS.mddescribes forisGrantablePermissionand$UserPermission. If renames stay (below), overridenamein$UpdateGroupDatawith the same schema, since it inherits$Group.name.admin/groups/create.tsx:48passes$CreateGroupDatato the form, which then shows the localizedformatPatterncopy fromzodErrorMap.ts. A message written inpackages/schemasskips the error map and shows in English (packages/react-core/AGENTS.md, "Validation messages"). That copy is generic ("Does not match the expected format"), so give the name field adescriptionthat says$is not allowed, inen,esandfr.The scope check must normalise both sides
exists({ name: name.replaceAll(' ', '_') })catches a newMy GroupwhenMy_Groupexists. It misses the reverse, because stored names are raw. Groups are few, so read every group'sidandname, unscoped like theexistsit replaces. CompareencodeScopedSubjectId('', { groupName })on both sides (the idiom atsubjects.service.ts:234), and skip the group's own id on rename. This subsumes the raw-name check. Keep "already exists" in the message, because #1825's new e2e inadmin-management.spec.tsasserts it.Also refuse
root(DEFAULT_GROUP_NAME). IDs entered with no group selected are scopedroot$(StartSessionForm.tsx:69,upload.ts:1005), andfindDefaultGroupCustomIds(subjects.service.ts:228-240) matches on that prefix.Rename is broken on its own
PATCH /v1/groups/:idacceptsname, but stored IDs keep the old prefix. After a rename, the start-session suggestions still listP001, becausefindCustomIdsmatches ongroupIds(subjects.service.ts:217).StartSessionForm.tsx:320then encodes it with the new name, so a returning participant becomes a second subject. Nothing inapps/websendsname. I'd refuse renames: make$UpdateGroupDatarejectname(for examplename: z.never().optional()) and delete the check atgroups.service.ts:100-106. Do not just omit the field. Zod strips unknown keys and libnest'sparseRequestBodypasses the stripped body on, so a rename would return 200 and change nothing.testing/src/specs/authorization.spec.ts:455uses a rename to prove row scoping and would need another field. Proper rename support needs a scope stored on the group. Either way this changes the API in/spec.json, so it needs a decision first.Tests
packages/runtime-internal/src/__tests__/index.test.ts:it('should keep what follows the last $, so a $ in the group name does not replace the subject ID').packages/schemas/src/group/__tests__/group.test.ts:$CreateGroupDatarejects$,$UpdateGroupDatarejectsname, and$Groupstill parses a storedR$D Lab.apps/api/src/groups/__tests__/groups.service.test.ts:My_Groupis refused whenMy Groupexists, the reverse is refused too, and so isroot.testing/src/specs/admin-management.spec.ts, next to fix: resolve 26 low-difficulty issues, one commit each #1825's conflict test, check the$error and the underscore collision. Intesting/src/specs/datahub.spec.ts, next to the export test at :25, seedR$D_${uniqueId}$S001throughapi.uploadRecords(itssubjectIdis any string) and assert that the exportedsubjectIdisS001.Triage note written with Claude Code. Code references point at
main@d86740e.