diff --git a/ddtrace/appsec/_asm_request_context.py b/ddtrace/appsec/_asm_request_context.py index 0e2ba7ad4eb..37d94438afc 100644 --- a/ddtrace/appsec/_asm_request_context.py +++ b/ddtrace/appsec/_asm_request_context.py @@ -400,8 +400,15 @@ def finalize_asm_env(env: ASM_Environment) -> None: entry_span._set_attribute(APPSEC.EVENT_RULE_ERROR_COUNT, info.failed) except Exception: logger.debug("asm_context::finalize_asm_env::exception", extra=log_extra, exc_info=True) - if asm_config._rc_client_id is not None: - entry_span.set_tag(APPSEC.RC_CLIENT_ID, asm_config._rc_client_id) + if asm_config._rc_client_id_enabled: + from ddtrace.internal.remoteconfig.worker import remoteconfig_poller + + # Fetch the current id from the RC client at span finalization. asm_config only + # tracks whether AppSec RC enabled tagging; mirroring the id there would go stale + # when runtime identity refresh rebuilds the RC client. + rc_client_id = remoteconfig_poller._client.id + if rc_client_id is not None: + entry_span.set_tag(APPSEC.RC_CLIENT_ID, rc_client_id) waf_adresses = env.waf_addresses req_headers = waf_adresses.get(SPAN_DATA_NAMES.REQUEST_HEADERS_NO_COOKIES, {}) if req_headers: diff --git a/ddtrace/appsec/_remoteconfiguration.py b/ddtrace/appsec/_remoteconfiguration.py index f7085e99d95..7fe2da28515 100644 --- a/ddtrace/appsec/_remoteconfiguration.py +++ b/ddtrace/appsec/_remoteconfiguration.py @@ -80,10 +80,13 @@ def enable_appsec_rc(callback: "AppSecCallback") -> None: if asm_config._asm_enabled: telemetry_writer.product_activated(TELEMETRY_APM_PRODUCT.APPSEC, True) - asm_config._rc_client_id = remoteconfig_poller._client.id + + asm_config._rc_client_id_enabled = True def disable_appsec_rc() -> None: + asm_config._rc_client_id_enabled = False + for product_name in APPSEC_PRODUCTS: remoteconfig_poller.unregister_callback(product_name) remoteconfig_poller.disable_product(product_name) diff --git a/ddtrace/internal/settings/asm.py b/ddtrace/internal/settings/asm.py index c129617c3f8..cba47f46631 100644 --- a/ddtrace/internal/settings/asm.py +++ b/ddtrace/internal/settings/asm.py @@ -268,7 +268,9 @@ class ASMConfig(DDConfig): sys.platform.startswith("win") or sys.platform.startswith("cygwin") ) - _rc_client_id: Optional[str] = None + # Set by enable_appsec_rc()/disable_appsec_rc(); gates _dd.rc.client_id span tagging so it's + # only emitted while AppSec RC is actually enabled, not just whenever a live RC client exists. + _rc_client_id_enabled: bool = False def __init__(self): super().__init__() diff --git a/tests/appsec/appsec/test_asm_request_context.py b/tests/appsec/appsec/test_asm_request_context.py index c42a8768652..94241ee87b5 100644 --- a/tests/appsec/appsec/test_asm_request_context.py +++ b/tests/appsec/appsec/test_asm_request_context.py @@ -14,6 +14,20 @@ config_asm = {"_asm_enabled": True} +@pytest.mark.parametrize("auto_enable_crashtracking", [False]) +def test_import_does_not_load_remoteconfig_worker(run_python_code_in_subprocess, auto_enable_crashtracking): + code = """ +import sys + +import ddtrace.appsec._asm_request_context # noqa: F401 + +assert "ddtrace.internal.remoteconfig.worker" not in sys.modules +""" + + _, stderr, status, _ = run_python_code_in_subprocess(code) + assert status == 0, stderr + + def test_context_set_and_reset(): with asm_context( ip_addr=_TEST_IP, diff --git a/tests/appsec/appsec/test_remoteconfiguration.py b/tests/appsec/appsec/test_remoteconfiguration.py index f0c7f66dd17..c169626ebba 100644 --- a/tests/appsec/appsec/test_remoteconfiguration.py +++ b/tests/appsec/appsec/test_remoteconfiguration.py @@ -270,6 +270,46 @@ def test_rc_activation_validate_client_id(tracer, rc_poller, appsec_callback): disable_appsec_rc() +def test_rc_client_id_tag_reflects_live_value_not_a_stale_cache(tracer, rc_poller, appsec_callback): + """_dd.rc.client_id must be read live at span-tagging time, not cached once at RC enable time. + + Otherwise the tag would go stale after e.g. an AWS Lambda MicroVM identity refresh + regenerates the real client id. + """ + from ddtrace.internal.remoteconfig.worker import remoteconfig_poller + + with override_global_config(dict(_asm_enabled=True, _remote_config_enabled=True, api_version="v0.4")): + tracer.configure(appsec_enabled=True) + enable_appsec_rc(appsec_callback) + + with mock.patch.object(remoteconfig_poller._client, "id", "client-id-one"): + with asm_context(tracer) as span: + set_http_meta(span, {}, raw_uri="http://example.com/", status_code="200") + assert span._local_root._get_str_attribute(APPSEC.RC_CLIENT_ID) == "client-id-one" + + with mock.patch.object(remoteconfig_poller._client, "id", "client-id-two"): + with asm_context(tracer) as span: + set_http_meta(span, {}, raw_uri="http://example.com/", status_code="200") + assert span._local_root._get_str_attribute(APPSEC.RC_CLIENT_ID) == "client-id-two" + disable_appsec_rc() + + +def test_rc_client_id_tag_not_set_when_rc_disabled(tracer): + """_dd.rc.client_id must not be tagged when AppSec RC was never enabled, even though a live + RC client (and id) exists process-wide -- otherwise every ASM-tracked span would get tagged + with an id from a Remote Config subscription AppSec never activated. + """ + from ddtrace.internal.remoteconfig.worker import remoteconfig_poller + + with override_global_config(dict(_asm_enabled=True, api_version="v0.4")): + tracer.configure(appsec_enabled=True) + + with mock.patch.object(remoteconfig_poller._client, "id", "client-id-one"): + with asm_context(tracer) as span: + set_http_meta(span, {}, raw_uri="http://example.com/", status_code="200") + assert span._local_root._get_str_attribute(APPSEC.RC_CLIENT_ID) is None + + @pytest.mark.parametrize( "env_rules, expected", [