typesense / typesense-js

JavaScript / TypeScript client for Typesense
https://typesense.org/docs/api
Apache License 2.0
393 stars 74 forks source link

Axios Cross-Site Request Forgery Vulnerability #187

Closed krisnegi123 closed 4 months ago

krisnegi123 commented 8 months ago

image

I am getting this warning while using the JS module.

jasonbosco commented 8 months ago

This is already addressed in v1.8.0-2. Could you give it a shot and let me know?

milsosa commented 8 months ago

@jasonbosco Is it safe to use pre-releases (i.e. v1.8.0-X)?

jasonbosco commented 8 months ago

Yes it is!

geisterfurz007 commented 7 months ago

@jasonbosco I found a breaking change from this

typesenseClient
  .collections('collection')
  .documents()
  .delete(id.toString());

fails in 1.8.0-2 with this error:

Request #1704393210967: Request to Node 0 failed due to "undefined target must be an object" Request #1704393210967: Sleeping for 0.1s and then retrying request... Request #1704393210967: Request to Node 0 failed due to "undefined target must be an object" Request #1704393210967: Sleeping for 0.1s and then retrying request...

Moving the ID up into the documents call solves this issue:

typesenseClient
  .collections('collection')
  .documents(id.toString())
  .delete()

I traced it through several functions to one of the utils in axios:

https://github.com/axios/axios/blob/8790b8e7847c7f450544e7195c837ffc10fcb160/lib/helpers/toFormData.js#L86-L89

which in the former way of deleting a single document is called with "the-id" as first argument, thus failing the object check.

Granted, the former way is not actually documented but the types allow it with a param explicitly called idOrQuery suggesting that passing an ID should work (and it did at least up to 1.7.2).

jasonbosco commented 7 months ago

@geisterfurz007 To delete a single document, the intended method signature is:

typesenseClient
  .collections('collection')
  .documents(id.toString())
  .delete()

The other method signature you used is actually not intended to be used for deleting a single document by ID. I've fixed the code and types for that method to make it clear that it's only meant to be used when deleting by query. I've published this in v1.8.0-3.

geisterfurz007 commented 7 months ago

Yeah, I am guilty of coding by autocomplete instead of by docs :') Thanks for the fast reaction!

wodCZ commented 4 months ago

The other method signature you used is actually not intended to be used for deleting a single document by ID

Ah, another user guilty of coding by autocomplete (are there even JS client docs?) who got caught with this BC break.

Maybe you could update the GH release description? Something like


Previous versions allowed the following method signature for deleting single document by ID:

typesenseClient
  .collections('collection')
  .documents()
  .delete(id.toString());

That was not the intended way and it was removed in this release. Please use the following method instead:

typesenseClient
  .collections('collection')
  .documents(id.toString())
  .delete()

The release is still quite new and this note could save others the time wasted looking through the diffs :)

jasonbosco commented 4 months ago

@wodCZ Will add that to the release notes. Thank you for the suggestion.

are there even JS client docs?

Yes, the API reference section has docs for JS client. For eg: https://typesense.org/docs/26.0/api/documents.html#delete-documents