fix(api): return a boolean from endpoint_status mitigated under V3 Locations - #15811
Open
svader0 wants to merge 1 commit into
Open
fix(api): return a boolean from endpoint_status mitigated under V3 Locations#15811svader0 wants to merge 1 commit into
svader0 wants to merge 1 commit into
Conversation
…cations V3EndpointStatusCompatibleSerializer.get_mitigated was a copy-paste of get_date two methods above it, so it returned obj.created.date() instead of a status check. Every row of GET /api/v2/endpoint_status/ therefore reported mitigated as a date string. The blast radius is wider than the one field. get_mitigated_time and get_mitigated_by both branch on self.get_mitigated(obj), and a date is truthy, so both answered as though every status was mitigated. A customer polling for mitigated statuses got a wrong answer with no error to signal it. The fix matches the three sibling methods right below it, which already compare obj.status against the matching FindingLocationStatus member. Found while auditing the "api/v2 endpoints return 500 on V3-Locations tenants" story. It was left out of that fix because it is a separate bug with a separate blast radius.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
V3EndpointStatusCompatibleSerializer.get_mitigatedreturned a date instead of a boolean. It was a copy ofget_datetwo methods above it:On a tenant with
V3_FEATURE_LOCATIONSenabled, this affects three fields ofGET /api/v2/endpoint_status/:mitigatedreports a date string on every row instead oftrue/false.mitigated_timeandmitigated_byboth branch onself.get_mitigated(obj). A date is truthy, so both answer as though every status is mitigated.A client polling for mitigated statuses gets a wrong answer and no error.
The fix compares the status, which is what the three sibling methods below it (
get_false_positive,get_out_of_scope,get_risk_accepted) already do:Test results
Two new regression tests in
unittests/test_endpoint_init_v3.py:test_endpoint_status_mitigated_is_a_bool_tracking_the_statusassertsmitigatedis abooland is true only forMitigated. It runs over all fiveFindingLocationStatusvalues.test_endpoint_status_mitigated_time_and_by_follow_mitigatedassertsmitigated_timeandmitigated_bystay null when the location is not mitigated.Before the fix, 6 of the 7 cases failed:
The
Mitigatedcase of the first test passed before the fix as well, because the date was truthy. It is the control case.After the fix all 7 cases pass. Five other tests in that module fail on my local stack, before and after this change, because the stack forces
SECURE_SSL_REDIRECT = Trueand they get a 301. They are unrelated to this change.Documentation
docs/content/asset_modelling/locations/PRO__migrating_from_endpoints.mdgains a bullet under "Behavioural Differences to Watch For" that records the old behavior, so a reader on an older release knows the field cannot be trusted.Checklist
dev.dev.bugfixbranch.