fix(asm): update required address when new rules are updated [backport 1.20] (#8425)

Backport
ac9bc9b
from #8420 to 1.20.

Required waf addresses were not updated in the appsec processor after a
remote config rules update.
Hence, it was possible for some custom rules to be ignored.

This PR fixes this.

## 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] Risks are described (performance impact, potential for breakage,
maintainability)
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed or label `changelog/no-changelog` is set
- [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))
- [x] If this PR changes the public interface, I've notified
`@DataDog/apm-tees`.
- [x] If change touches code that signs or publishes builds or packages,
or handles credentials of any kind, I've requested a review from
`@DataDog/security-design-and-guidance`.

## Reviewer Checklist

- [x] Title is accurate
- [x] All changes are related to the pull request's stated goal
- [x] Description motivates each change
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- [x] Testing strategy adequately addresses listed risks
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] Release note makes sense to a user of the library
- [x] Author has acknowledged and discussed the performance implications
of this PR as reported in the benchmarks PR comment
- [x] 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)

Author: christophe-papazian

ddtrace/appsec/processor.py

@@ -195,12 +195,16 @@ def __attrs_post_init__(self):
                 # Partial of DDAS-0005-00
                 log.warning("[DDAS-0005-00] WAF initialization failed")
                 raise
+            self._update_required()
+
+    def _update_required(self):
+        self._addresses_to_keep.clear()
         for address in self._ddwaf.required_data:
-            self._mark_needed(address)
+            self._addresses_to_keep.add(address)
         # we always need the request headers
-        self._mark_needed(WAF_DATA_NAMES.REQUEST_HEADERS_NO_COOKIES)
+        self._addresses_to_keep.add(WAF_DATA_NAMES.REQUEST_HEADERS_NO_COOKIES)
         # we always need the response headers
-        self._mark_needed(WAF_DATA_NAMES.RESPONSE_HEADERS_NO_COOKIES)
+        self._addresses_to_keep.add(WAF_DATA_NAMES.RESPONSE_HEADERS_NO_COOKIES)
 
     def _update_actions(self, rules):
         new_actions = rules.get("actions", [])
@@ -227,6 +231,7 @@ def _update_rules(self, new_rules):
             error_msg = "Error updating ASM rules. Invalid rules"
             log.debug(error_msg)
             _set_waf_error_metric(error_msg, "", self._ddwaf.info)
+        self._update_required()
         return result
 
     def on_span_start(self, span):
@@ -412,10 +417,6 @@ def update_metric(name, value):
                 span.set_tag_str(ORIGIN_KEY, APPSEC.ORIGIN_VALUE)
         return waf_results.derivatives
 
-    def _mark_needed(self, address):
-        # type: (str) -> None
-        self._addresses_to_keep.add(address)
-
     def _is_needed(self, address):
         # type: (str) -> bool
         return address in self._addresses_to_keep

releasenotes/notes/fix_required_waf_addresses-6c51ddbcaca95129.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    ASM: This fix resolves an issue where rules updated through remote config were not
+    properly updating required WAF addresses. This could lead to custom rules being ignored.

tests/appsec/test_processor.py

@@ -677,3 +677,56 @@ def test_asm_context_registration(tracer_appsec):
         span.span_type = SpanTypes.HTTP
         assert core.get_item("asm_env") is not None
     assert core.get_item("asm_env") is None
+
+
+def test_required_addresses():
+    with override_env(dict(DD_APPSEC_RULES=RULES_GOOD_PATH)):
+        processor = AppSecSpanProcessor()
+
+    assert processor._addresses_to_keep == {
+        "grpc.server.request.message",
+        "http.client_ip",
+        "server.request.body",
+        "server.request.cookies",
+        "server.request.headers.no_cookies",
+        "server.request.path_params",
+        "server.request.query",
+        "server.response.headers.no_cookies",
+        "usr.id",
+    }
+
+    processor._update_rules(
+        {
+            "custom_rules": [
+                {
+                    "conditions": [
+                        {
+                            "operator": "match_regex",
+                            "parameters": {
+                                "inputs": [{"address": "server.request.method"}],
+                                "options": {"case_sensitive": False},
+                                "regex": "GET",
+                            },
+                        }
+                    ],
+                    "id": "32b243c7-26eb-4046-adf4-custom",
+                    "name": "test required",
+                    "tags": {"category": "attack_attempt", "custom": "1", "type": "custom"},
+                    "transformers": [],
+                }
+            ]
+        }
+    )
+
+    assert processor._addresses_to_keep == {
+        "grpc.server.request.message",
+        "http.client_ip",
+        "server.request.body",
+        "server.request.cookies",
+        "server.request.headers.no_cookies",
+        "server.request.method",  # New required address
+        "server.request.path_params",
+        "server.request.query",
+        "server.response.headers.no_cookies",
+        "usr.id",
+    }