3 ms·

I tried booting it up and two things: - this is just evil. Pure. evil. https://github.com/colanode/colanode/blob/v0.1.3/apps/desktop/src/main/services/server-s

I tried booting it up and two things: - this is just evil. Pure. evil. https://github.com/colanode/colanode/blob/v0.1.3/apps/desktop/src/main/services/server-service.ts#L107-L112 https://github.com/colanode/colanode/blob/v0.1.3/apps/deskto... If that's the kind of error handling that you believe in, one should have religious backups of any data placed into this - It seems to actually puke if one doesn't provide it a live, TLS enabled, SMTP server[2] which (a) WTF (b) isn't present in the docker-compose Thankfully replacing .verify with return new Promise(() => true) at least let the server start 2: https://github.com/colanode/colanode/blob/v0.1.3/apps/server/src/services/email-service.ts#L32-L39 https://github.com/colanode/colanode/blob/v0.1.3/apps/server...

hakanshehu 2y ago
Thank you for taking the time to test it and call these issues out. Both points slipped through our refactor/cleanup checklist. - We’ll replace the current error handling for server sync with something safer and more graceful. - We’ll make SMTP optional, expose TLS verification as a configurable setting and update the docker-compose. We’ll make these improvements soon, thanks again for the heads-up.
salahuddin_dev 2y ago
[flagged]
yencabulator 2y ago
Here an example of it taking arbitrary input and blindly casting it to a type; anything after this point can blow up. There seems to be no input validation anywhere. const input = req.body as SyncMutationsInput; https://github.com/colanode/colanode/blob/9e69f29858a2ced6b1bd33b62993e154a1c91304/apps/server/src/api/client/controllers/workspaces/mutations/mutations-sync.ts#L32 https://github.com/colanode/colanode/blob/9e69f29858a2ced6b1... And the database use looks racy, sometimes not using transactions at all but having a read-modify-write cycle, no GET FOR UPDATE seen anywhere in transactions. Somebody is going to figure out how to do nasty things to the data.

hakanshehu 2y ago

Thank you for taking the time to test it and call these issues out. Both points slipped through our refactor/cleanup checklist. - We’ll replace the current error handling for server sync with something safer and more graceful. - We’ll make SMTP optional, expose TLS verification as a configurable setting and update the docker-compose. We’ll make these improvements soon, thanks again for the heads-up.

salahuddin_dev 2y ago

[flagged]

yencabulator 2y ago

Here an example of it taking arbitrary input and blindly casting it to a type; anything after this point can blow up. There seems to be no input validation anywhere. const input = req.body as SyncMutationsInput; https://github.com/colanode/colanode/blob/9e69f29858a2ced6b1bd33b62993e154a1c91304/apps/server/src/api/client/controllers/workspaces/mutations/mutations-sync.ts#L32 https://github.com/colanode/colanode/blob/9e69f29858a2ced6b1... And the database use looks racy, sometimes not using transactions at all but having a read-modify-write cycle, no GET FOR UPDATE seen anywhere in transactions. Somebody is going to figure out how to do nasty things to the data.