Skip to content

fix(workflow): stop leaking peer share tokens from participant session API#6241

Merged
Frooodle merged 3 commits intomainfrom
claude/elated-kowalevski-17354b
Apr 28, 2026
Merged

fix(workflow): stop leaking peer share tokens from participant session API#6241
Frooodle merged 3 commits intomainfrom
claude/elated-kowalevski-17354b

Conversation

@ConnorYoh
Copy link
Copy Markdown
Member

@ConnorYoh ConnorYoh commented Apr 27, 2026

Description of Changes

⚠️ Disclosure timing: this fix addresses GHSA-qgg6-mxw4-xg62, which is currently in draft state. Merging this PR publicly before the advisory is published makes the diff visible against the 2.9.2 release tag, from which the PoC can be reconstructed. Consider holding the merge or moving this to a temporary private fork from the GHSA UI before landing on main.

What changed

GET /api/v1/workflow/participant/session?token=<participant_token> was returning the full WorkflowSession via WorkflowMapper.toResponse(session), which serialized every participant's shareToken into participants[*].shareToken. Combined with submit-signature and decline resolving the target participant solely by token, one valid participant token was enough to act as any peer in the same workflow (force them into SIGNED or DECLINED).

This PR adds an includeShareToken(s) flag to WorkflowMapper, defaulting to true so existing callers (including the owner-facing SigningSessionController, which legitimately needs tokens for share-link distribution) keep their behavior. The four response sites in WorkflowParticipantController now pass false:

  • getSessionByToken
  • getParticipantDetails
  • submitSignature
  • declineParticipation

The participant's own bearer token comes from the URL on the frontend, so stripping it from these responses is UX-neutral. Verified by grepping every consumer of WorkflowSessionResponse.participants — no frontend code reads participants[i].shareToken.

Why

CVSS 7.6 (High) — AV:N/AC:L/PR:L/UI:N/S:U/C:L/I:H/A:L. Closing the disclosure breaks the impersonation chain at step 2 of the reporter's PoC.

Files

  • app/proprietary/src/main/java/.../workflow/util/WorkflowMapper.java — new overloads with includeShareToken(s); legacy signatures delegate with true.
  • app/proprietary/src/main/java/.../workflow/controller/WorkflowParticipantController.java — pass false at four call sites.
  • app/proprietary/src/test/java/.../workflow/util/WorkflowMapperShareTokenTest.java — 5 new tests covering both modes.
  • frontend/src/proprietary/services/workflowService.ts — widen ParticipantResponse.shareToken to string | null so the TS type matches the new wire payload.

SigningSessionController is intentionally untouched.

Tests

  • New: WorkflowMapperShareTokenTest (5 tests) — legacy overload still emits tokens (owner path), new flag strips tokens for all participants including peers, other fields survive.
  • Pre-existing: :proprietary:test — 10 workflow tests still green with the change in place.
:proprietary:test → BUILD SUCCESSFUL
WorkflowMapperShareTokenTest: tests=5, failures=0, errors=0
WorkflowParticipantValidateCertificateTest: tests=4, failures=0, errors=0
CertificateValidationIntegrationTest: tests=6, failures=0, errors=0

Follow-up: ParticipantResponse.shareToken may be a feature gap

While auditing this fix I confirmed that nothing in this repo currently reads participant.shareToken from any response — owner side or participant side:

  • Frontend: zero matches for participant.shareToken / p.shareToken across frontend/src/. The owner-side SessionDetailWorkbenchView and SessionActionsPanel iterate session.participants for status only. There is no "copy share link" or "resend invite" UI.
  • Backend: the only participant.getShareToken() call sites are the mapper that puts it on the wire, an expiry log line in UnifiedAccessControlService, and the repository findByShareToken lookup. No notification/email service uses it. The createSession docstring says it "distributes share links, and optionally notifies participants", but I cannot find code that does either.

So the field is dead weight as far as this codebase is concerned. Three plausible explanations:

  1. Feature gap. The "owner gets a share link to give to a participant" UX was never finished. An owner today has no in-app way to deliver the link; they would have to inspect the API response in dev tools.
  2. External API consumer. Some integration outside this repo (Tauri, mobile, customer integration) is calling POST /api/v1/security/cert-sign/sessions and reading the tokens to send invitations themselves. Removing it would silently break them.
  3. Stale. Left over from an earlier flow that did email out tokens, since deleted.

I deliberately did not drop the field from owner responses in this PR — the blast radius for #2 is unclear, and the security goal (peer-token enumeration) is already addressed by the participant-side strip.

Suggested follow-up: confirm whether owner-side share-link distribution is a feature gap or external-consumer concern, then either build the UI or drop the field from ParticipantResponse entirely.

Refs: GHSA-qgg6-mxw4-xg62


Checklist

General

  • I have read the Contribution Guidelines
  • I have performed a self-review of my own code
  • My changes generate no new warnings

Testing

  • I have tested my changes locally (added regression tests; ran the proprietary test suite)

…n API

The participant-facing endpoints under /api/v1/workflow/participant/* were
serializing every peer's shareToken in the WorkflowSessionResponse. A caller
holding one valid participant bearer token could read peer tokens from
participants[*].shareToken and then call submit-signature or decline as any
other participant in the same workflow.

Add an includeShareToken(s) flag to WorkflowMapper, defaulting to true so
the owner-facing SigningSessionController (which legitimately needs tokens
for share-link distribution) is unchanged. Pass false from the four
WorkflowParticipantController response sites, which closes the disclosure
chain. Caller's own token comes from the URL, so no UX regression.

Refs: GHSA-qgg6-mxw4-xg62
@dosubot dosubot Bot added size:M This PR changes 30-99 lines ignoring generated files. Bugfix Pull requests that fix bugs labels Apr 27, 2026
@stirlingbot stirlingbot Bot added Java Pull requests that update Java code Test Testing-related issues or pull requests labels Apr 27, 2026
The previous commit makes the four participant-facing endpoints emit
shareToken: null in their JSON responses (peer-token disclosure fix). The
TypeScript interface still declared shareToken: string, which would silently
mislead any future consumer into a non-null assertion.

Widen to string | null and document why. No runtime change — no consumer
currently reads this field; this only makes the type honest.

Refs: GHSA-qgg6-mxw4-xg62
@stirlingbot stirlingbot Bot added the Front End Issues or pull requests related to front-end development label Apr 27, 2026
@ConnorYoh ConnorYoh enabled auto-merge April 27, 2026 11:38
@ConnorYoh ConnorYoh added this pull request to the merge queue Apr 27, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Apr 27, 2026
@Frooodle Frooodle merged commit 4e4918b into main Apr 28, 2026
33 of 34 checks passed
@Frooodle Frooodle deleted the claude/elated-kowalevski-17354b branch April 28, 2026 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bugfix Pull requests that fix bugs Front End Issues or pull requests related to front-end development Java Pull requests that update Java code size:M This PR changes 30-99 lines ignoring generated files. Test Testing-related issues or pull requests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants