Skip to content

Gateway appends series answers without checking which item they answer, so a second tab locks the patient out or makes the series unimportable #1750

Open

Description

@joshunrau

PATCH /api/assignments/:id stores a series item's answers by appending them to encryptedData, without being told which item they answer and without checking that this is the next unanswered one. Position is everything downstream. GatewaySynchronizer pairs the nth ciphertext with the nth series item, and the root router resumes a returning patient at "number of stored entries". So any submission for an item that has already been answered corrupts the assignment. The easiest way to send one is to open the link twice, which happens readily: clicking the email link again, or restoring tabs. A tab left behind on an earlier item submits it again, and the gateway files it under the next position. Once that happens the patient cannot reopen the link (the server render throws and returns a 500), or the finished series is rejected by the API on every pass and never imported. #1668 fixes the same symptom for a double click inside one tab on the client; it cannot stop a second tab, because only the gateway sees both submissions.

Where

apps/gateway/src/routers/api.router.ts:125-138. A series submission is appended unconditionally:

let encryptedData: string, symmetricKey: string;
if (kind === 'SCALAR') {
  // ...
} else {
  encryptedData = (assignment.encryptedData ?? '').concat(
    '$',
    Buffer.from(encryptResult.cipherText).toString('base64')
  );
  symmetricKey = (assignment.symmetricKey ?? '').concat(
    '$',
    Buffer.from(encryptResult.symmetricKey).toString('base64')
  );
}

apps/gateway/src/routers/root.router.ts:39-42. The resume position is the entry count:

let initialSeriesIndex: number | undefined;
if (assignment.encryptedData?.startsWith('$')) {
  initialSeriesIndex = assignment.encryptedData.slice(1).split('$').length;
}

packages/react-core/src/components/InstrumentRenderer/SeriesInstrumentRenderer.tsx:52-61 throws when that count reaches the number of items. Nothing in the PATCH body says which item is being answered: $UpdateRemoteAssignmentData is { data, kind, status }, and the series submit context's index is the renderer's step (0/1/2), not the item.

Reproduce

  1. Assign the built-in DNP_HAPPINESS_QUESTIONNAIRE_WITH_CONSENT series (items: DNP_GENERAL_CONSENT_FORM, then DNP_HAPPINESS_QUESTIONNAIRE) and open its link in two tabs. Verify and click Begin in both.
  2. In tab A, give consent and submit. Tab A moves on to the happiness questionnaire.
  3. In tab B, which still shows the consent form, submit it.
  4. Reload either tab.
  5. Alternatively, skip step 4 and finish the happiness questionnaire in tab A, then wait one sync interval.

Actual: the gateway now holds two entries for a two-item series, both of them consent answers. Step 4 computes initialSeriesIndex = 2, SeriesInstrumentRenderer throws "Initial series index '2' must be less than length of items '2'" during the server render, and the patient gets {"message":"Internal Server Error","statusCode":500} from then on. The assignment never completes. In step 5 the series is marked COMPLETE with three entries, and GatewaySynchronizer throws "Expected number of submitted series items '3' to be between 1 and the series instrument content length '2'" on every pass, so it is never imported, while the patient was shown "Thank You!".
Expected: the second submission of the consent form is refused (for example with 409), and that tab reloads to the server's position, so the stored entries always line up one-to-one with the series items.

Tests

  • apps/gateway/src/routers/__tests__/api.router.test.ts: it('should reject a series item that is not the next unanswered one, so a second tab cannot shift later answers') and it('should reject any submission once the assignment is complete').
  • apps/gateway/src/__tests__/Root.test.tsx: it('should send the position of the series item being submitted').
  • e2e, testing/src/specs/gateway-assignment.spec.ts: like the existing malformed-submission test, verify once in the page and then send two PATCHes for item 0 with the page's token. Assert that the second is refused and that reloading the link resumes at item 1.

Suggested fix

Add the item's position (currentItemIndex) to the series submit context in react-core and to $UpdateRemoteAssignmentData as, for example, itemIndex. In the PATCH handler, compute the stored entry count, reject a SERIES submission whose itemIndex differs with 409, and reject any submission once completedAt is set. On 409, have Root.tsx reload the page so it resumes from the server's count.

Activity

  1. added
    BugType: existing behavior is wrong
    Difficulty: MediumStraightforward change, about a day or less
    Area: Gatewayapps/gateway (patient-facing app for remote assignments)
    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). None of the cited lines has changed since this was filed, and no open PR touches them.

    The same series also fails in a way the bound check does not catch. apps/api/src/gateway/gateway.synchronizer.ts:139 only bounds the number of entries and never checks positions. If tab B declines consent at step 3, terminate ends the series, and the row becomes COMPLETE with two entries for two items. B's consent answer is then validated as DNP_HAPPINESS_QUESTIONNAIRE and fails with 422 (instrument-records.service.ts:113-121) on every pass. If the next item's schema happens to accept the shifted answer, it is imported under that item with no error.

    Changes to the suggested fix

    • Replace index instead of adding a field. SeriesContext.index (packages/react-core/src/components/InstrumentRenderer/types.ts:29) is always 1 when onSubmit runs, because item content only renders under index: 1 (SeriesInstrumentRenderer.tsx:186). Nothing reads it. Replace it with itemIndex: currentItemIndex. Root.tsx:60-63 spreads ...result into the body, so the field is sent without touching updateData. Update apps/gateway/src/__tests__/Root.test.tsx:48 and apps/web/src/routes/_app/instruments/render/__tests__/$id.test.tsx:188, which set index: 0, and apps/gateway/src/routers/__tests__/api.router.test.ts:263 and :274, which send series PATCHes with no position.
    • Make $UpdateRemoteAssignmentData a discriminated union on kind, with both variants extended from one base. Then itemIndex is required on SERIES and does not exist on SCALAR.
    • Count entries in one place. Move root.router.ts:39-42 into a helper that the router and the PATCH guard both call.
    • Make the write conditional. The handler reads the row at api.router.ts:106 and writes it at :140, with several awaits in between. Use updateMany where { id, encryptedData: <value read>, completedAt: null }, and return 409 when count === 0. Otherwise two requests that read the same row both get 200, and the later write silently replaces the earlier one.
    • Derive completion on the server. Refuse itemIndex >= items.length, and set COMPLETE when itemIndex is the last item, reading items from targetStringified. Otherwise a hand-written last-item PATCH without status fills every slot while completedAt stays null, and the next load of the link is a 500.
    • Do not reload on 409. A reloaded page starts with verified false (Root.tsx:40, :87), so the patient has to solve the proof of work again, although the server still holds the id as verified. Once Gateway lets a patient reload or leave mid-instrument without warning, discarding every unsaved answer #1726 lands, location.reload() also raises "Leave site?". Instead, return the stored count in the 409 body and remount the renderer at that count, with a t({ en, es, fr }) notice. The remount needs a key as well as initialSeriesIndex, because initialSeriesIndex is only read by a useState initialiser (SeriesInstrumentRenderer.tsx:52). handleSubmit must still throw. If it returns, createSubmitHandler advances as though the item was saved.
    • Shared groundwork. Add a code to HttpException and to the errorHandlerMiddleware body, as an enum in @opendatacapture/schemas/gateway. Add a per-request option to apps/gateway/src/services/axios.ts that names the codes the caller handles, so the generic toast is skipped only for those. Web's meta.disableDefaultErrorNotification (apps/web/src/services/axios.ts:18, :163) is the all-or-nothing version of this. After a gateway restart, a patient's submit fails with "HTTP Request Failed 403" and the only way to re-verify discards their answers #1758 needs both. Update apps/gateway/AGENTS.md:54 ("Throw HttpException(status, message)") in the same PR.

    Gotchas

    • The completedAt guard is needed today, not only after After a gateway restart, a patient's submit fails with "HTTP Request Failed 403" and the only way to re-verify discards their answers #1758. COMPLETE clears verification (api.router.ts:151-153), but /api/auth/verify marks any id (cap.router.ts:62). So a second tab opened before the first one finished can solve the Cap afterwards and is verified again. Its PATCH then adds a third entry to a finished series, or replaces a scalar's answers before the next sync.
    • The gateway malformed submission e2e sends kind: 'SERIES' with no position to a FORM assignment and expects 200 (testing/src/specs/gateway-assignment.spec.ts:162-166). Add itemIndex: 0 to it. If the handler also refuses a SERIES submission to a scalar target, that test has to expect 400 instead.
    • Rows corrupted before this ships stay unimportable, and they need to be cancelled and reassigned.

    Tests

    In addition to the tests listed in the issue:

    • api.router.test.ts: 409 when the conditional write matches no row, itemIndex >= items.length is refused, and the last item completes without status.
    • packages/schemas/src/assignment/__tests__/assignment.test.ts: a series update without itemIndex is rejected.
    • SeriesInstrumentRenderer.test.tsx: itemIndex counts from initialSeriesIndex.
    • Root.test.tsx: a 409 remounts the renderer at the returned count and shows the notice, and the page does not reload.
    • apps/gateway/src/services/__tests__/axios.test.ts: a handled code raises no toast, and other errors still do.
    • e2e: use DNP_HAPPINESS_QUESTIONNAIRE_WITH_CONSENT, because its two items look different. Look it up by title, since findInstrumentIdByName (testing/src/support/api-client.ts:185) only matches a scalar's internal.name. Verify once and send one PATCH for item 0 with the page's token. Then click Begin and submit consent in the page. Assert that after the next Begin the page shows the Happiness Questionnaire without a second verification, and that after a reload __ROOT_PROPS__ has initialSeriesIndex: 1. A single Cap solve covers both the server's refusal and the page's recovery.

    Order

    Land this after #1668, which guards the same createSubmitHandler and edits the same onSubmit({...}) call (SeriesInstrumentRenderer.tsx:99-106). Land it before #1758 or with it. #1703 adds its expiry refusal to the same PATCH guard.

    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: Gatewayapps/gateway (patient-facing app for remote assignments)Area: SharedCross-tier packages (schemas, react-core, etc.) not covered by another areaBugType: existing behavior is wrongDifficulty: MediumStraightforward change, about a day or lessPriority: MediumReal value; schedule normally

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions