Skip to content

Commit 9a3693f

Browse files
chore(wsgi): remove some appsec code from wsgi contrib (#6326)
This change adjusts the `flask_block` callback-setting logic to use the Core API rather than the AppSec-specific `set_value` call it had used previously. In the case of request blocking, the separation of concerns ideally breaks down as follows. The AppSec Product code in the `ddtrace/appsec` directory knows how to make a block/don't block decision based on communication with libddwaf. The Wsgi code in `ddtrace/contrib` knows how to take that blocking decision into account when processing requests. ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [ ] Title is accurate. - [ ] No unnecessary changes are introduced. - [ ] Description motivates each change. - [ ] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [ ] Testing strategy adequately addresses listed risk(s). - [ ] Change is maintainable (easy to change, telemetry, documentation). - [ ] Release note makes sense to a user of the library. - [ ] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. - [ ] Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) --------- Co-authored-by: Federico Mon <[email protected]>
1 parent 4e17ec3 commit 9a3693f

File tree

2 files changed

+21
-12
lines changed

2 files changed

+21
-12
lines changed

ddtrace/appsec/_asm_request_context.py

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ def __init__(self):
120120

121121
def finalise(self):
122122
if self.active:
123-
env = core.get_item("asm_env")
123+
env = self.execution_context.get_item("asm_env")
124124
# assert _CONTEXT_ID.get() == self._id
125125
callbacks = GLOBAL_CALLBACKS.get(_CONTEXT_CALL, []) + env.callbacks.get(_CONTEXT_CALL)
126126
if callbacks is not None:
@@ -319,12 +319,26 @@ def asm_request_context_manager(
319319
The ASM context manager
320320
"""
321321
if config._appsec_enabled:
322-
resources = _DataHandler()
323-
asm_request_context_set(remote_ip, headers, headers_case_sensitive, block_request_callable)
322+
resources = _on_context_started(remote_ip, headers, headers_case_sensitive, block_request_callable)
324323
try:
325324
yield resources
326325
finally:
327-
resources.finalise()
328-
core.set_item("asm_env", None)
326+
_on_context_ended(resources)
329327
else:
330328
yield None
329+
330+
331+
def _on_context_started(remote_ip, headers, headers_case_sensitive, block_request_callable):
332+
resources = _DataHandler()
333+
asm_request_context_set(remote_ip, headers, headers_case_sensitive, block_request_callable)
334+
core.on("wsgi.block_decided", _on_block_decided)
335+
return resources
336+
337+
338+
def _on_context_ended(resources):
339+
resources.finalise()
340+
core.set_item("asm_env", None)
341+
342+
343+
def _on_block_decided(callback):
344+
set_value(_CALLBACKS, "flask_block", callback)

ddtrace/contrib/wsgi/wsgi.py

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -165,29 +165,25 @@ def __call__(self, environ, start_response):
165165
headers = get_request_headers(environ)
166166
closing_iterator = ()
167167
not_blocked = True
168-
with _asm_request_context.asm_request_context_manager(
169-
environ.get("REMOTE_ADDR"), headers, headers_case_sensitive=True
170-
):
168+
with _asm_request_context.asm_request_context_manager(environ.get("REMOTE_ADDR"), headers, True):
171169
req_span = self.tracer.trace(
172170
self._request_span_name,
173171
service=trace_utils.int_service(self._pin, self._config),
174172
span_type=SpanTypes.WEB,
175173
)
176174

177175
if self.tracer._appsec_enabled:
178-
# [IP Blocking]
179176
if core.get_item(HTTP_REQUEST_BLOCKED, span=req_span):
180177
ctype, content = self._make_block_content(environ, headers, req_span)
181178
start_response("403 FORBIDDEN", [("content-type", ctype)])
182179
closing_iterator = [content]
183180
not_blocked = False
184181

185-
# [Suspicious Request Blocking on request]
186182
def blocked_view():
187183
ctype, content = self._make_block_content(environ, headers, req_span)
188184
return content, 403, [("content-type", ctype)]
189185

190-
_asm_request_context.set_value(_asm_request_context._CALLBACKS, "flask_block", blocked_view)
186+
core.dispatch("wsgi.block_decided", [blocked_view])
191187

192188
if not_blocked:
193189
req_span.set_tag_str(COMPONENT, self._config.integration_name)
@@ -212,7 +208,6 @@ def blocked_view():
212208
req_span.finish()
213209
raise
214210
if self.tracer._appsec_enabled and core.get_item(HTTP_REQUEST_BLOCKED, span=req_span):
215-
# [Suspicious Request Blocking on request or response]
216211
_, content = self._make_block_content(environ, headers, req_span)
217212
closing_iterator = [content]
218213

0 commit comments

Comments
 (0)