fix(asm): rc must not modify rule file specified by env var [backport #5930 to 1.12] (#5984)

Backport of #5930 to 1.12

When DD_APPSEC_RULES file is specified in the env, RC capabilities that
update the rules should be disabled.

Update two tests (RC capabilities and RC product registration) as
regression tests for this fix.

## 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/contributing.html#Release-Note-Guidelines)
are followed.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).

## Reviewer Checklist

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.

Author: christophe-papazian

ddtrace/appsec/_remoteconfiguration.py

@@ -4,6 +4,7 @@
 from ddtrace import config
 from ddtrace.appsec._constants import PRODUCTS
 from ddtrace.appsec.utils import _appsec_rc_features_is_enabled
+from ddtrace.appsec.utils import _appsec_rc_file_is_not_static
 from ddtrace.constants import APPSEC_ENV
 from ddtrace.internal.logger import get_logger
 from ddtrace.internal.remoteconfig.client import RemoteConfigCallBack
@@ -54,7 +55,7 @@ def enable_appsec_rc(test_tracer=None):
 
         RemoteConfig.register(PRODUCTS.ASM_FEATURES, asm_features_callback)
 
-    if tracer._appsec_enabled:
+    if tracer._appsec_enabled and _appsec_rc_file_is_not_static():
         from ddtrace.internal.remoteconfig import RemoteConfig
 
         RemoteConfig.register(PRODUCTS.ASM_DATA, asm_callback)  # IP Blocking
@@ -138,20 +139,22 @@ def _appsec_1click_activation(self, features):
             log.debug("Updating ASM Remote Configuration ASM_FEATURES: %s", rc_appsec_enabled)
 
             if rc_appsec_enabled:
-                asm_dd_callback = RCASMDDCallBack(self.tracer)
-                asm_callback = RCAppSecCallBack(self.tracer)
-                RemoteConfig.register(PRODUCTS.ASM_DATA, asm_callback)  # IP Blocking
-                RemoteConfig.register(PRODUCTS.ASM, asm_callback)  # Exclusion Filters & Custom Rules
-                RemoteConfig.register(PRODUCTS.ASM_DD, asm_dd_callback)  # DD Rules
+                if _appsec_rc_file_is_not_static():
+                    asm_dd_callback = RCASMDDCallBack(self.tracer)
+                    asm_callback = RCAppSecCallBack(self.tracer)
+                    RemoteConfig.register(PRODUCTS.ASM_DATA, asm_callback)  # IP Blocking
+                    RemoteConfig.register(PRODUCTS.ASM, asm_callback)  # Exclusion Filters & Custom Rules
+                    RemoteConfig.register(PRODUCTS.ASM_DD, asm_dd_callback)  # DD Rules
                 if not self.tracer._appsec_enabled:
                     self.tracer.configure(appsec_enabled=True)
                 else:
                     config._appsec_enabled = True
 
             else:
-                RemoteConfig.unregister(PRODUCTS.ASM_DATA)
-                RemoteConfig.unregister(PRODUCTS.ASM)
-                RemoteConfig.unregister(PRODUCTS.ASM_DD)
+                if _appsec_rc_file_is_not_static():
+                    RemoteConfig.unregister(PRODUCTS.ASM_DATA)
+                    RemoteConfig.unregister(PRODUCTS.ASM)
+                    RemoteConfig.unregister(PRODUCTS.ASM_DD)
                 if self.tracer._appsec_enabled:
                     self.tracer.configure(appsec_enabled=False)
                 else:

ddtrace/appsec/processor.py

@@ -23,6 +23,7 @@
 from ddtrace.appsec._metrics import _set_waf_updates_metric
 from ddtrace.appsec.ddwaf import DDWaf
 from ddtrace.appsec.ddwaf import version
+from ddtrace.appsec.utils import _appsec_rc_file_is_not_static
 from ddtrace.constants import MANUAL_KEEP_KEY
 from ddtrace.constants import ORIGIN_KEY
 from ddtrace.constants import RUNTIME_FAMILY
@@ -197,6 +198,8 @@ def __attrs_post_init__(self):
     def _update_rules(self, new_rules):
         # type: (Dict[str, Any]) -> bool
         result = False
+        if not _appsec_rc_file_is_not_static():
+            return result
         try:
             result = self._ddwaf.update_rules(new_rules)
             _set_waf_updates_metric(self._ddwaf.info)

ddtrace/appsec/utils.py

@@ -27,6 +27,10 @@ def _appsec_rc_features_is_enabled():
     return False
 
 
+def _appsec_rc_file_is_not_static():
+    return "DD_APPSEC_RULES" not in os.environ
+
+
 def _appsec_rc_capabilities(test_tracer=None):
     # type: (Optional[Tracer]) -> str
     r"""return the bit representation of the composed capabilities in base64
@@ -54,7 +58,7 @@ def _appsec_rc_capabilities(test_tracer=None):
     if asbool(os.environ.get("DD_REMOTE_CONFIGURATION_ENABLED", "true")):
         if _appsec_rc_features_is_enabled():
             value |= 1 << 1  # Enable ASM_ACTIVATION
-        if tracer._appsec_processor:
+        if tracer._appsec_processor and _appsec_rc_file_is_not_static():
             value |= 1 << 2  # Enable ASM_IP_BLOCKING
             value |= 1 << 3  # Enable ASM_DD_RULES
             value |= 1 << 4  # Enable ASM_EXCLUSIONS

releasenotes/notes/asm_fix_rc_capabilities_with_rule_file_set_in_env-3c24b75abbdf003c.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    ASM: This fix resolves an issue where the WAF rule file specified by DD_APPSEC_RULES was wrongly
+    updated and modified by remote config.

tests/appsec/test_remoteconfiguration.py

@@ -11,6 +11,7 @@
 
 from ddtrace.appsec import _asm_request_context
 from ddtrace.appsec._constants import APPSEC
+from ddtrace.appsec._constants import DEFAULT
 from ddtrace.appsec._constants import PRODUCTS
 from ddtrace.appsec._remoteconfiguration import RCAppSecCallBack
 from ddtrace.appsec._remoteconfiguration import RCAppSecFeaturesCallBack
@@ -139,16 +140,24 @@ def test_rc_capabilities(rc_enabled, appsec_enabled, capability, tracer):
         assert _appsec_rc_capabilities(test_tracer=tracer) == capability
 
 
-def test_rc_activation_capabilities(tracer, remote_config_worker):
+@pytest.mark.parametrize(
+    "env_rules, expected",
+    [
+        ({}, "Af4="),  # All capabilities
+        ({"DD_APPSEC_RULES": DEFAULT.RULES}, "Ag=="),  # Only ASM_FEATURES
+    ],
+)
+def test_rc_activation_capabilities(tracer, remote_config_worker, env_rules, expected):
     env = {"DD_REMOTE_CONFIGURATION_ENABLED": "true"}
+    env.update(env_rules)
     with override_env(env), override_global_config(dict(_appsec_enabled=False, api_version="v0.4")):
         rc_config = {"asm": {"enabled": True}}
 
         assert not RemoteConfig._worker
 
         RCAppSecFeaturesCallBack(tracer)(None, rc_config)
 
-        assert _appsec_rc_capabilities(test_tracer=tracer) == "Af4="  # All capabilities
+        assert _appsec_rc_capabilities(test_tracer=tracer) == expected
 
 
 def test_rc_activation_validate_products(tracer, remote_config_worker):
@@ -162,13 +171,24 @@ def test_rc_activation_validate_products(tracer, remote_config_worker):
         assert RemoteConfig._worker._client._products["ASM_DATA"]
 
 
-def test_rc_activation_check_asm_features_product_disables_rest_of_products(tracer, remote_config_worker):
-    with override_global_config(dict(_appsec_enabled=True, api_version="v0.4")):
+@pytest.mark.parametrize(
+    "env_rules, expected",
+    [
+        ({}, True),  # All capabilities
+        ({"DD_APPSEC_RULES": DEFAULT.RULES}, False),  # Only ASM_FEATURES
+    ],
+)
+def test_rc_activation_check_asm_features_product_disables_rest_of_products(
+    tracer, remote_config_worker, env_rules, expected
+):
+    env = {"DD_REMOTE_CONFIGURATION_ENABLED": "true"}
+    env.update(env_rules)
+    with override_env(env), override_global_config(dict(_appsec_enabled=True, api_version="v0.4")):
         tracer.configure(appsec_enabled=True, api_version="v0.4")
         enable_appsec_rc(tracer)
 
-        assert RemoteConfig._worker._client._products.get(PRODUCTS.ASM_DATA)
-        assert RemoteConfig._worker._client._products.get(PRODUCTS.ASM)
+        assert bool(RemoteConfig._worker._client._products.get(PRODUCTS.ASM_DATA)) is expected
+        assert bool(RemoteConfig._worker._client._products.get(PRODUCTS.ASM)) is expected
         assert RemoteConfig._worker._client._products.get(PRODUCTS.ASM_FEATURES)
 
         RCAppSecFeaturesCallBack(tracer)(None, False)