Skip to content

A $ in a group name makes every custom subject ID in that group display and export as the same value, and names differing only by space vs underscore share subjects #1725

Open

Description

@joshunrau

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

  1. As an admin, create a group named R$D Lab.
  2. In it, start sessions for custom IDs S001 and S002, and complete a form for each.
  3. 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 named R$D Lab shows 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

  1. added
    BugType: existing behavior is wrong
    Priority: LowNice to have, cosmetic or speculative
    Difficulty: LowIsolated change, about 2 hours or less
    Area: APIapps/api (NestJS backend, Prisma models, CASL permissions)
    Area: SharedCross-tier packages (schemas, react-core, etc.) not covered by another area
    on Oct 7, 2026
  2. joshunrau commented on Oct 26, 2026

    @joshunrau
    CollaboratorAuthor

    Still reproduces on main (d86740e). Nothing has touched runtime-internal, subject-utils, schemas/src/group or groups.service.ts since this was filed, and no open PR touches them.

    Unscope on the last $, not the first

    The suggested id.slice(id.indexOf('$') + 1) does not fix the reported case: R$D_Lab$S001 becomes D_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);. lastIndexOf returns -1 for an unscoped ID, so the includes branch can go. Existing R$D Lab subjects then display and export correctly with no data change. The UI refuses $ in a custom ID: StartSessionForm.tsx:256 since a301eaf, which introduced scoped IDs, and the upload's SUBJECT_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 hold Group$A$B. That shows as A today and would show as B. No split of the string alone is right for both that and R$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:14 uses 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

    • Put it on $CreateGroupData.name, not on $Group. $Group parses every groups response (useGroupsQuery.ts:11, useGroupQuery.ts:12, useUpdateGroupMutation.ts:32, admin/groups/index.tsx:40) and $AuditLog picks its name (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 split packages/schemas/AGENTS.md describes for isGrantablePermission and $UserPermission. If renames stay (below), override name in $UpdateGroupData with the same schema, since it inherits $Group.name.
    • Give the regex no message. admin/groups/create.tsx:48 passes $CreateGroupData to the form, which then shows the localized formatPattern copy from zodErrorMap.ts. A message written in packages/schemas skips 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 a description that says $ is not allowed, in en, es and fr.

    The scope check must normalise both sides

    exists({ name: name.replaceAll(' ', '_') }) catches a new My Group when My_Group exists. It misses the reverse, because stored names are raw. Groups are few, so read every group's id and name, unscoped like the exists it replaces. Compare encodeScopedSubjectId('', { groupName }) on both sides (the idiom at subjects.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 in admin-management.spec.ts asserts it.

    Also refuse root (DEFAULT_GROUP_NAME). IDs entered with no group selected are scoped root$ (StartSessionForm.tsx:69, upload.ts:1005), and findDefaultGroupCustomIds (subjects.service.ts:228-240) matches on that prefix.

    Rename is broken on its own

    PATCH /v1/groups/:id accepts name, but stored IDs keep the old prefix. After a rename, the start-session suggestions still list P001, because findCustomIds matches on groupIds (subjects.service.ts:217). StartSessionForm.tsx:320 then encodes it with the new name, so a returning participant becomes a second subject. Nothing in apps/web sends name. I'd refuse renames: make $UpdateGroupData reject name (for example name: z.never().optional()) and delete the check at groups.service.ts:100-106. Do not just omit the field. Zod strips unknown keys and libnest's parseRequestBody passes the stripped body on, so a rename would return 200 and change nothing. testing/src/specs/authorization.spec.ts:455 uses 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: $CreateGroupData rejects $, $UpdateGroupData rejects name, and $Group still parses a stored R$D Lab.
    • apps/api/src/groups/__tests__/groups.service.test.ts: My_Group is refused when My Group exists, the reverse is refused too, and so is root.
    • E2E: in 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. In testing/src/specs/datahub.spec.ts, next to the export test at :25, seed R$D_${uniqueId}$S001 through api.uploadRecords (its subjectId is any string) and assert that the exported subjectId is S001.

    Triage note written with Claude Code. Code references point at main @ d86740e.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Area: APIapps/api (NestJS backend, Prisma models, CASL permissions)Area: SharedCross-tier packages (schemas, react-core, etc.) not covered by another areaBugType: existing behavior is wrongDifficulty: LowIsolated change, about 2 hours or lessPriority: LowNice to have, cosmetic or speculative

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions