fix(api): answer 410 rather than 500 when a v2 route reaches a deprecated Endpoint - #15809
Open
svader0 wants to merge 3 commits into
Open
fix(api): answer 410 rather than 500 when a v2 route reaches a deprecated Endpoint#15809svader0 wants to merge 3 commits into
svader0 wants to merge 3 commits into
Conversation
Two customer automations 500'd on /api/v2/endpoints/?limit=500 and on an ordered /api/v2/test_imports/. Both were fixed in 3.2.100, but nothing held them there. Adds the exact production requests as regression tests, plus a sweep over the registered v2 routes so a route that hydrates a deprecated Endpoint fails the suite instead of paging us.
…d Endpoint Every fix in this crash class so far patched one call site, so the next unconverted path became the next hourly 500. Catches the class once, in DRF's configured exception handler, and answers a machine-readable body instead: a stable code, a replacement route, and a docs link. Endpoint.__init__ now raises its own NotImplementedError subclass so the handler matches on a type rather than a message, and a genuine NotImplementedError still surfaces as the 500 it is. Logged at info, because an expected answer is not an outage. The 403 on writes is unchanged.
The two regression tests were named after the accounts that reported them, which does not belong in a public repository. Renames them after the requests they make, and rewrites the module docstring so it describes the failure instead of the reporters. Widens the route sweep to fail on 410 as well as 5xx. The new backstop logs at info, so a route that drifts into the deprecation sunset would otherwise produce no signal at all, and the sweep is the only check we control. Adds an assertion that DRF is actually wired to the configured handler, since every 410 assertion in this file calls it directly instead. Renames the test that calls the handler by hand, since it hydrates nothing. Drops a false claim from the deprecation module's docstring: importing it does pull in the model layer, through dojo/location/__init__.py, so it cannot say it does not. Notes in the exception handler that the deprecation branch only sees exceptions raised during DRF's dispatch.
|
This pull request contains critical security findings because user 'svader0' modified sensitive files in the
🔴 Configured Sensitive Codepath Modified by Non-Allowed Author in
|
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/api_v2/exception_handler.py' matches configured sensitive codepath pattern 'dojo/api_v2/*.py' and was modified by 'svader0' (commit 7927af2) who is not in the allowed authors list. |
🔴 Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/endpoint/models.py (drs_48457eed)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/endpoint/models.py' matches configured sensitive codepath pattern 'dojo/endpoint/*.py' and was modified by 'svader0' (commit 7927af2) who is not in the allowed authors list. |
We've notified @mtesauro.
Comment to provide feedback on these findings.
Report false positive: @dryrunsecurity fp [FINDING ID] [FEEDBACK]
Report low-impact: @dryrunsecurity nit [FINDING ID] [FEEDBACK]
Example: @dryrunsecurity fp drs_90eda195 This code is not user-facing
All finding details can be found in the DryRun Security Dashboard.
Collaborator
Author
|
[sc-14273] |
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
1 participant
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.
The problem
When Locations is enabled, the old
Endpointmodel raisesNotImplementedErroron init. Nothing on the/api/v2/surface caught that error. Any route that still touched Endpoint data returned a 500 with no explanation. Every retry wrote an error line into alerting.The change
Endpoint.__init__now raisesEndpointDeprecatedError, its own subclass ofNotImplementedError. The api/v2 exception handler catches that one class and answers410 Gone. The body is machine-readable, so a client can branch oncodeinstead of parsing prose:{ "code": "endpoint_api_sunset", "message": "The Endpoint API is not available on instances with Locations enabled. Reads are served from Locations; writes are not available.", "replacement": "/api/v2/location/", "docs": "https://docs.defectdojo.com/asset_modelling/locations/pro__migrating_from_endpoints/" }The handler logs this at info, not error.
A plain
NotImplementedErrorstill returns a 500. Only the new subclass answers 410.Tests
Two reported routes get their exact failing requests as regression tests. Those requests are
GET /api/v2/endpoints/?limit=500and an orderedGET /api/v2/test_imports/. Both routes were fixed in 3.2.100, and nothing held them there until now.A third test sweeps every route registered on the v2 router. It fails if any list route answers 5xx, or if any falls into the deprecation sunset.
One test resolves the configured
EXCEPTION_HANDLERand calls it, because the other tests call the handler directly.What does not change
Writes to
/api/v2/endpoints/and/api/v2/endpoint_status/still return 403. Read compatibility is unchanged. Every other route behaves as before.Docs
The migration guide gains one section describing the 410 body and what to do if a read returns it.