Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix SentinelOne limit error parsing #129

Closed

Conversation

xC0uNt3r7hr34t
Copy link
Contributor

@xC0uNt3r7hr34t xC0uNt3r7hr34t commented Jul 21, 2023

removed 2000 limit option as this causes issues with multiple endpoints and is irrelevant with pagination.

resolves #128

@xC0uNt3r7hr34t xC0uNt3r7hr34t changed the title ix limit error parsing Fix SentinelOne limit error parsing Jul 21, 2023
@TreWilkinsRC
Copy link
Contributor

TreWilkinsRC commented Jul 21, 2023

Just tested the upper range. This change seems to be far more reliable. Thanks!

@xC0uNt3r7hr34t
Copy link
Contributor Author

xC0uNt3r7hr34t commented Jul 21, 2023

Both PQ and DV have limitations for 1000. DV endpoint allows 20000 limit but the other endpoints hit by surveyor do not. It is recommended to use 1000 for everything. currently your fix still sets things to 20000 when using DV

@TreWilkinsRC
Copy link
Contributor

TreWilkinsRC commented Jul 21, 2023

Okay, that makes sense. I'm assuming the concern here is the time it takes to get results returned. But I feel like that unnecessarily limits the API. If a user has a desired limit, they can set it using the --limit argument during the search. I definitely don't want to cap the use of Surveyor if there's no need, but also don't want to make the program less user-friendly. Any thoughts from this perspective?

Gonna hold of on merging in the meantime.

@xC0uNt3r7hr34t
Copy link
Contributor Author

xC0uNt3r7hr34t commented Jul 21, 2023

the /accounts endpoint is what had a limit of 1000 that threw an error. if we want the ability to do a 20000 limit, it should only be used when hitting the /dv/events endpoint as this is where results are returned. if the limit isn't maximized the results will be returned via pagination. If we are truly looking for speed, we can just set the default for that endpoint to 20000. this pretty much makes the entire function for pagination irrelevant for DV.

Edit: I reread your response and I think I better understand your questioning. If the user doesn't want the full results they can specify a smaller limit. If they want a larger limit (max of 20000) they could specify this. We should either set 20000 or 1000 as the default and then leave it to the user to either specify higher or lower limit. Regardless of that the only endpoint that should be higher than 1000 is the one that fetches the telemetry events as all other will never have that high of a limit or don't support it.

@rc-abodkins
Copy link
Contributor

@xC0uNt3r7hr34t with the merge of #125 - can you test again and see if the issue is fixed?

@xC0uNt3r7hr34t
Copy link
Contributor Author

Issue is fixed via #125. Thanks

@xC0uNt3r7hr34t xC0uNt3r7hr34t deleted the 128-bug-s1-limit branch July 25, 2023 21:38
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.

[BUG] Limit is too large causing 400 errors on SentinelOne requests
3 participants