chore(appsec): skip user block when auto mode resolves to disabled by RomainMuller · Pull Request #17580 · DataDog/dd-trace-py

RomainMuller · 2026-04-16T16:11:57Z

Summary

block_request_if_user_blocked has a guard-ordering bug: it checks
mode == DISABLED before resolving AUTO to the configured mode.
When called with mode="auto" (via set_user_for_asm) and
DD_APPSEC_AUTO_USER_INSTRUMENTATION_MODE=disabled, the guard reads
"auto" == "disabled" → False and proceeds, calling should_block_user
unnecessarily. That WAF call returns keep=True and causes the tracer
to force-keep the trace via _asm_manual_keep — even though no
automated user event was generated.

Fix: resolve AUTO to the configured mode first, then apply the
disabled guard.

# Before (buggy)
if not asm_config._asm_enabled or mode == LOGIN_EVENTS_MODE.DISABLED:
    return
if mode == LOGIN_EVENTS_MODE.AUTO:
    mode = asm_config._user_event_mode   # too late — guard already passed

# After
if mode == LOGIN_EVENTS_MODE.AUTO:
    mode = asm_config._user_event_mode   # resolve first
if not asm_config._asm_enabled or mode == LOGIN_EVENTS_MODE.DISABLED:
    return                               # now correctly catches disabled

How the bug was found

While writing end-to-end tests for the APPSEC_AUTO_EVENTS_TRACKING=disabled
scenario in system-tests,
login-success traces were unexpectedly force-kept (_sampling_priority_v1=2,
_dd.p.dm=-5) even though no user event tags were emitted. Debug
tracing confirmed _asm_manual_keep was called from _processor.py:399
via should_block_user → call_waf_callback(usr.id=...), bypassing the
disabled guard due to the ordering bug described above.

Release notes

Note: Release notes still needed before this can be merged.

When `block_request_if_user_blocked` is called with `mode="auto"` (which happens via `set_user_for_asm` during authenticated requests), the disabled-mode guard was evaluated against the *incoming* parameter value before the AUTO→configured-mode resolution occurred. This meant that when `DD_APPSEC_AUTO_USER_INSTRUMENTATION_MODE=disabled` was configured, the check at the guard read `"auto" == "disabled"` → False, and the function proceeded to call `should_block_user`, which invoked the WAF with the user ID. The WAF returning `keep=True` for that call resulted in the trace being force-kept via `_asm_manual_keep` even though no automated user event was generated. Fix: resolve the AUTO mode to the configured mode first, then apply the disabled guard. This ensures that a configured disabled state is respected regardless of whether the caller passes `"auto"` or `"disabled"` directly. JJ-Change-Id: qvyyqx

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b554208d30

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

pr-commenter · 2026-04-16T16:26:41Z

Performance SLOs

Comparing candidate romain.marcadier/appsec-block-user-disabled-mode (cedaeca) with baseline main (a9cc850)

📈 Performance Regressions (2 suites)

📈 iastaspectsospath - 24/24

✅ ospathbasename_aspect

Time: ✅ 530.233µs (SLO: <700.000µs 📉 -24.3%) vs baseline: 📈 +27.4%

Memory: ✅ 43.838MB (SLO: <46.000MB -4.7%) vs baseline: +5.1%

✅ ospathbasename_noaspect

Time: ✅ 434.075µs (SLO: <700.000µs 📉 -38.0%) vs baseline: +1.1%

Memory: ✅ 43.819MB (SLO: <46.000MB -4.7%) vs baseline: +4.9%

✅ ospathjoin_aspect

Time: ✅ 630.910µs (SLO: <700.000µs -9.9%) vs baseline: +0.3%

Memory: ✅ 43.916MB (SLO: <46.000MB -4.5%) vs baseline: +5.1%

✅ ospathjoin_noaspect

Time: ✅ 639.208µs (SLO: <700.000µs -8.7%) vs baseline: +0.5%

Memory: ✅ 43.817MB (SLO: <46.000MB -4.7%) vs baseline: +5.0%

✅ ospathnormcase_aspect

Time: ✅ 349.971µs (SLO: <700.000µs 📉 -50.0%) vs baseline: +0.4%

Memory: ✅ 43.891MB (SLO: <46.000MB -4.6%) vs baseline: +5.0%

✅ ospathnormcase_noaspect

Time: ✅ 358.089µs (SLO: <700.000µs 📉 -48.8%) vs baseline: -0.4%

Memory: ✅ 43.874MB (SLO: <46.000MB -4.6%) vs baseline: +5.2%

✅ ospathsplit_aspect

Time: ✅ 490.195µs (SLO: <700.000µs 📉 -30.0%) vs baseline: -0.3%

Memory: ✅ 43.777MB (SLO: <46.000MB -4.8%) vs baseline: +4.7%

✅ ospathsplit_noaspect

Time: ✅ 501.710µs (SLO: <700.000µs 📉 -28.3%) vs baseline: +0.3%

Memory: ✅ 43.869MB (SLO: <46.000MB -4.6%) vs baseline: +4.6%

✅ ospathsplitdrive_aspect

Time: ✅ 376.158µs (SLO: <700.000µs 📉 -46.3%) vs baseline: +1.2%

Memory: ✅ 43.859MB (SLO: <46.000MB -4.7%) vs baseline: +5.4%

✅ ospathsplitdrive_noaspect

Time: ✅ 72.936µs (SLO: <700.000µs 📉 -89.6%) vs baseline: +0.3%

Memory: ✅ 43.940MB (SLO: <46.000MB -4.5%) vs baseline: +5.4%

✅ ospathsplitext_aspect

Time: ✅ 468.664µs (SLO: <700.000µs 📉 -33.0%) vs baseline: +2.5%

Memory: ✅ 43.882MB (SLO: <46.000MB -4.6%) vs baseline: +5.0%

✅ ospathsplitext_noaspect

Time: ✅ 472.807µs (SLO: <700.000µs 📉 -32.5%) vs baseline: +1.2%

Memory: ✅ 43.838MB (SLO: <46.000MB -4.7%) vs baseline: +5.1%

📈 iastaspectssplit - 12/12

✅ rsplit_aspect

Time: ✅ 164.378µs (SLO: <250.000µs 📉 -34.2%) vs baseline: 📈 +10.9%

Memory: ✅ 43.844MB (SLO: <46.000MB -4.7%) vs baseline: +4.8%

✅ rsplit_noaspect

Time: ✅ 160.924µs (SLO: <250.000µs 📉 -35.6%) vs baseline: +3.0%

Memory: ✅ 43.868MB (SLO: <46.000MB -4.6%) vs baseline: +5.3%

✅ split_aspect

Time: ✅ 153.591µs (SLO: <250.000µs 📉 -38.6%) vs baseline: +4.8%

Memory: ✅ 43.854MB (SLO: <46.000MB -4.7%) vs baseline: +5.3%

✅ split_noaspect

Time: ✅ 153.641µs (SLO: <250.000µs 📉 -38.5%) vs baseline: +2.6%

Memory: ✅ 43.744MB (SLO: <46.000MB -4.9%) vs baseline: +4.7%

✅ splitlines_aspect

Time: ✅ 148.880µs (SLO: <250.000µs 📉 -40.4%) vs baseline: +3.3%

Memory: ✅ 43.849MB (SLO: <46.000MB -4.7%) vs baseline: +4.9%

✅ splitlines_noaspect

Time: ✅ 152.229µs (SLO: <250.000µs 📉 -39.1%) vs baseline: +0.1%

Memory: ✅ 43.925MB (SLO: <46.000MB -4.5%) vs baseline: +5.8%

✅ All Tests Passing (1 suite)

✅ iastpropagation - 8/8

✅ no-propagation

Time: ✅ 48.731µs (SLO: <60.000µs 📉 -18.8%) vs baseline: +0.6%

Memory: ✅ 40.875MB (SLO: <42.000MB -2.7%) vs baseline: +4.9%

✅ propagation_enabled

Time: ✅ 136.190µs (SLO: <190.000µs 📉 -28.3%) vs baseline: +0.2%

Memory: ✅ 40.934MB (SLO: <42.000MB -2.5%) vs baseline: +5.1%

✅ propagation_enabled_100

Time: ✅ 1.561ms (SLO: <2.300ms 📉 -32.1%) vs baseline: -1.4%

Memory: ✅ 40.875MB (SLO: <42.000MB -2.7%) vs baseline: +4.9%

✅ propagation_enabled_1000

Time: ✅ 29.079ms (SLO: <34.550ms 📉 -15.8%) vs baseline: -0.7%

Memory: ✅ 40.934MB (SLO: <42.000MB -2.5%) vs baseline: +5.2%

ℹ️ Scenarios Missing SLO Configuration (20 scenarios)

The following scenarios exist in candidate data but have no SLO thresholds configured:

iast_aspects-re_expand_aspect
iast_aspects-re_expand_noaspect
iast_aspects-re_findall_aspect
iast_aspects-re_findall_noaspect
iast_aspects-re_finditer_aspect
iast_aspects-re_finditer_noaspect
iast_aspects-re_fullmatch_aspect
iast_aspects-re_fullmatch_noaspect
iast_aspects-re_group_aspect
iast_aspects-re_group_noaspect
iast_aspects-re_groups_aspect
iast_aspects-re_groups_noaspect
iast_aspects-re_match_aspect
iast_aspects-re_match_noaspect
iast_aspects-re_search_aspect
iast_aspects-re_search_noaspect
iast_aspects-re_sub_aspect
iast_aspects-re_sub_noaspect
iast_aspects-re_subn_aspect
iast_aspects-re_subn_noaspect

cit-pr-commenter-54b7da · 2026-04-16T16:27:13Z

Codeowners resolved as

ddtrace/appsec/_trace_utils.py                                          @DataDog/asm-python

When `block_request_if_user_blocked` is called in `auto` mode and the configured user instrumentation mode resolves to `disabled`, the previous code fell through a combined guard that logged a warning saying "ASM must be enabled" — which is misleading because ASM may well be enabled. Separate the two early-exit conditions: log the warning only when ASM is actually disabled, and emit a `debug`-level message for the expected case where user instrumentation mode is explicitly configured as `disabled`. This avoids log noise on the hot path for a correctly-configured system. JJ-Change-Id: ttpsrz

…17580) ## Summary `block_request_if_user_blocked` has a guard-ordering bug: it checks `mode == DISABLED` **before** resolving `AUTO` to the configured mode. When called with `mode="auto"` (via `set_user_for_asm`) and `DD_APPSEC_AUTO_USER_INSTRUMENTATION_MODE=disabled`, the guard reads `"auto" == "disabled"` → False and proceeds, calling `should_block_user` unnecessarily. That WAF call returns `keep=True` and causes the tracer to force-keep the trace via `_asm_manual_keep` — even though no automated user event was generated. Fix: resolve `AUTO` to the configured mode first, then apply the disabled guard. ```python # Before (buggy) if not asm_config._asm_enabled or mode == LOGIN_EVENTS_MODE.DISABLED: return if mode == LOGIN_EVENTS_MODE.AUTO: mode = asm_config._user_event_mode # too late — guard already passed # After if mode == LOGIN_EVENTS_MODE.AUTO: mode = asm_config._user_event_mode # resolve first if not asm_config._asm_enabled or mode == LOGIN_EVENTS_MODE.DISABLED: return # now correctly catches disabled ``` ## How the bug was found While writing end-to-end tests for the `APPSEC_AUTO_EVENTS_TRACKING=disabled` scenario in [system-tests](DataDog/system-tests#6750), login-success traces were unexpectedly force-kept (`_sampling_priority_v1=2`, `_dd.p.dm=-5`) even though no user event tags were emitted. Debug tracing confirmed `_asm_manual_keep` was called from `_processor.py:399` via `should_block_user` → `call_waf_callback(usr.id=...)`, bypassing the disabled guard due to the ordering bug described above. ## Release notes > **Note:** Release notes still needed before this can be merged. Co-authored-by: romain.marcadier <romain.marcadier@datadoghq.com>

RomainMuller requested a review from a team as a code owner April 16, 2026 16:11

RomainMuller added changelog/no-changelog A changelog entry is not required for this PR. ASM Application Security Monitoring labels Apr 16, 2026

RomainMuller mentioned this pull request Apr 16, 2026

chore(appsec): skip user block when auto mode resolves to disabled #17575

Closed

chatgpt-codex-connector Bot reviewed Apr 16, 2026

View reviewed changes

Comment thread ddtrace/appsec/_trace_utils.py Outdated

RomainMuller and others added 2 commits April 17, 2026 09:58

Don't warn if auto mode + ASM disabled

cedaeca

christophe-papazian approved these changes Apr 17, 2026

View reviewed changes

gh-worker-dd-mergequeue-cf854d Bot merged commit 982e953 into main Apr 17, 2026
581 checks passed

gh-worker-dd-mergequeue-cf854d Bot deleted the romain.marcadier/appsec-block-user-disabled-mode branch April 17, 2026 09:56

Conversation

RomainMuller commented Apr 16, 2026

Summary

How the bug was found

Release notes

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

pr-commenter Bot commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Performance SLOs

✅ ospathbasename_aspect

✅ ospathbasename_noaspect

✅ ospathjoin_aspect

✅ ospathjoin_noaspect

✅ ospathnormcase_aspect

✅ ospathnormcase_noaspect

✅ ospathsplit_aspect

✅ ospathsplit_noaspect

✅ ospathsplitdrive_aspect

✅ ospathsplitdrive_noaspect

✅ ospathsplitext_aspect

✅ ospathsplitext_noaspect

✅ rsplit_aspect

✅ rsplit_noaspect

✅ split_aspect

✅ split_noaspect

✅ splitlines_aspect

✅ splitlines_noaspect

✅ no-propagation

✅ propagation_enabled

✅ propagation_enabled_100

✅ propagation_enabled_1000

Uh oh!

cit-pr-commenter-54b7da Bot commented Apr 16, 2026

Codeowners resolved as

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pr-commenter Bot commented Apr 16, 2026 •

edited

Loading