Skip to content

Add warning screen when server is added after community creation#3007

Open
adrastaea wants to merge 110 commits into
developfrom
feat/2907-warn-on-unexpected-server
Open

Add warning screen when server is added after community creation#3007
adrastaea wants to merge 110 commits into
developfrom
feat/2907-warn-on-unexpected-server

Conversation

@adrastaea

@adrastaea adrastaea commented Oct 8, 2025

Copy link
Copy Markdown
Collaborator

Warns users when a server is authorized on the chain which does not match the QSS endpoint or the endpoints the community was initialized with.

Pull Request Checklist

  • I have linked this PR to a related GitHub issue.
  • I have added a description of the change (and Github issue number, if any) to the root CHANGELOG.md.

(Optional) Mobile checklist

Please ensure you completed the following checks if you did any changes to the mobile package:

  • I have run e2e tests for mobile
  • I have updated base screenshots for visual regression tests

@adrastaea
adrastaea marked this pull request as ready for review October 8, 2025 20:21
Enter a comma separated list of server host URLs to add to the current community.
</Typography>
</Paper>
</details>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not clear on why this is here. Am I reviewing the right PR?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a debug utility to forcibly add malicious looking servers to a community for testing. It will only show up in development builds

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, sorry, missed that.

@holmesworcester

holmesworcester commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

These were some findings from Codex. I reviewed them and they make sense to me.


I reviewed this PR locally against origin/pr/3007 and current origin/develop without checking out the branch. Findings below are from read-only diff/source inspection. I did not run the full test suites.

Findings

1. Mobile warning drawer is not mounted

ServerAddedDrawer exists, but it only appears in its implementation and Storybook story:

packages/mobile/src/components/ModalBottomDrawer/drawers/ServerAdded.drawer.tsx
packages/mobile/src/components/ModalBottomDrawer/drawers/ServerAdded.drawer.stories.tsx

packages/mobile/src/App.tsx imports/renders CaptchaDrawer, but not ServerAddedDrawer:

58  import { CaptchaDrawer } from './components/ModalBottomDrawer/drawers/Captcha.drawer'
133 <CaptchaDrawer />

So mobile can receive/update state for SERVER_ADDED, but the new warning UI is never in the app tree.

2. Desktop accept-with-ToS path can persist QSS/server acceptance before ToS is accepted

In ServerAddedModal, clicking “Continue With Server” requests ToS if needed, then immediately dispatches updateCommunityData with qssEnabled: true and accepted server hosts:

46 if (!currentCommunity.tosAccepted) {
47   dispatch(communities.actions.requestTermsOfService())
48 }
49 dispatch(communities.actions.updateCommunityData(updateCommunityPayload))

The ToS modal later dispatches only setTermsOfServiceAccepted:

31 dispatch(
32   communities.actions.setTermsOfServiceAccepted({
33     communityId: currentCommunity?.id,
34     accepted,
35   })
36 )

That reducer only updates Redux state:

77 communitiesAdapter.updateOne(state.communities, {
78   id: communityId,
79   changes: {
80     tosAccepted: accepted,
81   },
82 })

There is no socket emit from ToS acceptance, while backend QSS connect checks tosAccepted from backend local DB. This means backend can have qssEnabled: true / accepted server hosts but still have tosAccepted: false.

3. In the “server added, ToS not accepted yet” path, normal QSS usage does not start

The earlier symptom is not just that UPDATE_COMMUNITY lacks an immediate connect call. For the ToS-required path, new QSS data sync remains blocked: new OrbitDB entries are queued locally rather than sent to QSS, because backend ToS never gets updated.

Accepting the modal emits UPDATE_COMMUNITY via state-manager:

14 yield* apply(socket, socket.emit, applyEmitParams(SocketActions.UPDATE_COMMUNITY, action.payload))

Backend handles that by only writing the community object:

724 this.socketService.on(SocketActions.UPDATE_COMMUNITY, async (payload: UpdateCommunityPayload) => {
726   const community = await this.localDbService.getCommunity(payload.id)
731   await this.localDbService.setCommunity({ ...community, ...payload })
732 })

There is no qssService.connect(...), no QSS_HANDLE_SIGN_IN, and no auth sync start in that handler.

The normal explicit QSS connect calls in this PR are limited to startup-from-storage, create-community, and join-via-QSS:

connections-manager.service.ts:229  this.qssService.connect(community.qssEndpoint)
connections-manager.service.ts:412  this.qssService.connect(this.qssEndpoint)
connections-manager.service.ts:441  await this.qssService.connect(inviteData.qssEndpoint, true)

A later reconnect/startup path could only proceed if backend local DB has sufficient state. But the new ToS flow does not write tosAccepted: true to backend, and QSS connect exits early when backend local DB says ToS is not accepted:

318 const initStatus = await this.getQssInitStatus()
319 if (!initStatus.tosAccepted) {
320   this.logger.warn(`Can't connect to QSS until TOS is accepted`)
321   return QSSOperationResult.ERROR
322 }

The actual QSS data-sync path requires both the QSS websocket to be connected and the auth connection to be joined. OrbitDB writes call sendLogEntrySyncMessage, but _sendLogEntrySyncMessage queues locally when disconnected:

635 if (!this.connected) {
636   this.logger.warn('QSS not connected, writing entry to dead letter queue', hash, teamId)
638   await this.localDbService.addPendingQssLogSyncMessage(address, hash)
642   return undefined
643 }

Even if connected, it still queues locally unless auth sync is joined:

645 if (this.joinStatus(teamId) !== JoinStatus.JOINED) {
646   this.logger.warn('QSS not signed in, writing entry to dead letter queue', hash, teamId)
648   await this.localDbService.addPendingQssLogSyncMessage(address, hash)
652   return undefined
653 }

The auth connection starts only after create/sign-in succeeds and emits QSS_START_AUTH_CONN:

491 this.emit(QSSEvents.QSS_START_AUTH_CONN, sigChain.team.id)
563 this.emit(QSSEvents.QSS_START_AUTH_CONN, teamId, teamName)

The post-launch accept-server path does not reach either create/sign-in path. Therefore, for the intended “server added after community creation and user must accept ToS” flow, normal QSS usage does not begin: messages/log entries are dead-lettered locally, and data is not sent through QSS.

Caveat: if backend already had tosAccepted: true, then a later reconnect/startup could eventually start QSS because qssEnabled was persisted. That is not the problematic ToS-required path described above.

4. Mobile ToS request from ServerAddedDrawer does not navigate to ToS

The drawer dispatches requestTermsOfService:

37 if (!currentCommunity.tosAccepted) {
38   dispatch(communities.actions.requestTermsOfService())
39 }

But mobile navigation to TermsOfServiceScreen is wired through username registration / pending navigation:

25 if (
26   (invitationCodes?.version === InvitationDataVersion.v3 && invitationCodes?.qssEnabled) ||
27   pendingNavigation === ScreenNames.TermsOfServiceScreen
28 ) {
29   dispatch(navigationActions.replaceScreen({ screen: ScreenNames.TermsOfServiceScreen }))

Create-community sets that pending navigation:

66 if (useServer) {
67   dispatch(navigationActions.setPendingNavigation({ screen: ScreenNames.TermsOfServiceScreen }))
68 }

The new drawer does not set pending navigation and there is no mobile observer that navigates on tosRequested.

5. New state-manager saga files contain unused imports/variables

addServer.saga.ts imports unused symbols and accepts an unused socket parameter:

1 import { apply, select, put, call, take } from 'typed-redux-saga'
3 import { applyEmitParams, type Socket } from '../../../types'
5 import { type Community, type UpdateCommunityPayload } from '@quiet/types'
11 export function* addServerSaga(
12   socket: Socket,

Only select, put, Socket, and UpdateCommunityPayload are used. apply, call, take, applyEmitParams, Community, and socket are unused.

updateCommunityData.saga.ts also imports unused effects and creates an unused logger:

1 import { apply, select, put, call, take } from 'typed-redux-saga'
6 import { createLogger } from '../../../utils/logger'
8 const logger = createLogger('updateCommunityDataSaga')

This may fail lint depending on the shared ESLint config.

6. Missing realistic tests for the new server-added behavior

Changed executable test files are only:

packages/backend/src/nest/qss/qss.service.spec.ts
packages/desktop/src/rtl-tests/community.join.test.tsx

The new ServerAdded surfaces have Storybook stories, but no behavior tests:

packages/desktop/src/renderer/components/ServerAdded/ServerAddedComponent.stories.tsx
packages/mobile/src/components/ModalBottomDrawer/drawers/ServerAdded.drawer.stories.tsx

I did not find tests covering SERVER_ADDED -> addServerSaga -> warning UI -> accept/leave -> backend persistence/QSS start, nor a mobile integration test proving ServerAddedDrawer is mounted.

7. Branch is stale and conflicts with current develop

Current local refs:

merge-base:    27a63a22f 2025-11-19 Fix/ci failures (#3029)
origin/develop bd42681ee 2026-06-26 enable qss on prod (#3319)
origin/pr/3007 5835663a9 2025-11-19 Merge branch 'develop' into feat/2907...

A non-mutating merge check (git merge-tree --write-tree origin/develop origin/pr/3007) reports conflicts, including:

CHANGELOG.md
packages/backend/src/nest/auth/sigchain.service.ts
packages/backend/src/nest/connections-manager/connections-manager.service.ts
packages/backend/src/nest/qss/qss.service.ts
packages/desktop/src/renderer/Root.tsx
packages/state-manager/src/sagas/communities/communities.slice.ts
packages/types/src/community.ts
packages/types/src/socket.ts

So this needs a fresh merge/rebase before final review or testing.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Warn users when users join a community which unexpectedly includes a server

3 participants