Limit block failure to the current WAF run

When _DD_APPSEC_BLOCKING_UNAVAILABLE is set, SetBlockFailed latches on the whole ContextOperation. Since the request is not interrupted in that mode, later RASP/WAF runs in the same request can still happen; copying op.IsBlockFailed() here marks those later runs as block:failure even when their own result.Actions did not request a block, so rasp.rule.match telemetry is misclassified for subsequent SQL/SSRF/etc. matches. Gate this value on the current blocking result or otherwise track the failure per run before passing it to RegisterWafRun.

Useful? React with 👍 / 👎.

This only happens in proxies. Not exactly meaningful as the target for "framework that don't allow us to block". What would really make sense in terms of "blocking failed" should be emitted in case we are trying to block when the customer already used ResponseWriter.WriteHeader, which we can do btw because we have a wrapper for it in APM code. At some point I had a PR to do this but this was just too complex and low prio to finish because it goes hand in hand with trying to delay the response from being sent until we know if we will not be able to block or the response payload become too big.

Removed entirely in 92353b6 — the whole block_failure feature (SetBlockFailed/IsBlockFailed, block_failure: telemetry tag, block:failure RASP variant, _dd.appsec.block.failed span tag) is dead code because blockingUnavailable suppresses blocking capability advertisements to RC so BlockingSecurityEvent can never fire.

I see the metrics object just above, is this really necessary to add this boolean here?

Gone in 92353b6 — the blockFailed bool field is removed along with the rest of the block_failure feature.

Nit — byte-slicing can split multi-byte UTF-8 characters

Slicing at maxExceptionMsgBytes (512) bytes can cut a multi-byte UTF-8 character mid-sequence, producing trailing invalid bytes. Not a correctness bug in Go (strings tolerate arbitrary bytes), but could cause issues if downstream consumers (backend, log pipelines) are strict about UTF-8.

If worth addressing, a common pattern is to walk backward to the last valid rune boundary, or apply strings.ToValidUTF8 after truncation.

Fixed in 92353b6 — switched to strings.ToValidUTF8(msg[:maxExceptionMsgBytes], "") which drops any incomplete trailing rune rather than preserving the raw bytes.

Informational — remoteconfig.ClientID() is captured at Feature creation, not lazily

ClientID() returns "" if the RC client hasn't started yet. If the Feature is created before RC starts (possible during early startup), rcClientID is permanently empty for this Feature instance.

Likely fine since an RC config update would create a fresh Feature with the correct ID. But if the first WAF instance is expected to carry the ID (e.g., RC starts between Feature creation and first request), reading ClientID() lazily inside the reportRulesTags.Do closure would fix it:

waf.reportRulesTags.Do(func() {
    AddRulesMonitoringTags(op, remoteconfig.ClientID())
})

Is the current capture-at-creation behavior intentional?

Fixed in 92353b6 — moved the remoteconfig.ClientID() call inside the reportRulesTags.Do closure so it reads the ID lazily on first request rather than at Feature creation time.

Low severity — fallthrough to raspError with zero-value rule type when !ok

When RASPRuleTypeFromAddressSet returns !ok, ruleType is the zero value (RASPRuleTypeLFI = 0). The instrumentation exception is recorded correctly, but execution falls through to raspError, which:

1. Updates m.RASPErrorCodes[RASPRuleTypeLFI] — attributing the error to the wrong rule type.
2. Emits a rasp.error telemetry metric tagged as LFI.

This was pre-existing (old code had the same fallthrough with m.logger.Error), but since this block was rewritten, adding a return after RecordException would be a natural fix:

if !ok {
    m.RecordException(ExceptionTypeInstrumentation, in)
    return
}
m.raspError(in, ruleType)

Fixed in 92353b6 — added return after RecordException in the !ok branch, also wrapped the error with the address-set context (same commit as 3167015464 below).

Bug: missing return after failed RASP rule-type lookup — can index OOB

When RASPRuleTypeFromAddressSet returns ok==false, ruleType gets RASPRuleTypeLFI (zero value). The instrumentation exception is recorded, but execution falls through to m.raspError(in, ruleType) which attributes the error to the wrong rule type and emits rasp.error telemetry tagged as LFI.

Since this block was rewritten in this PR, adding a return is a natural fix:

if !ok {
    m.RecordException(ExceptionTypeInstrumentation,
        fmt.Errorf("unknown RASP rule type for scope %q: %w", addrs.TimerKey, in))
    return
}
m.raspError(in, ruleType)

Fixed in 92353b6 — added return after RecordException and wrapped the error with the address-set for context.

Race: sync.Once serializes the write, but reads of exceptionType/exceptionMsg are unprotected

ExceptionType() and ExceptionMsg() (lines 443-446) are plain field reads. AddWAFMonitoringTags calls them from onFinish, but in-flight Run() goroutines may still be inside RecordException. sync.Once.Do only guarantees that the func runs once — it does NOT create a happens-before edge for reads from a different goroutine that doesn't call Do.

This is a data race -race would flag, and can produce torn reads (type from one call, empty message).

Fix: publish both fields atomically via atomic.Pointer:

type recordedException struct{ typ, msg string }

// in ContextMetrics:
exception atomic.Pointer[recordedException]

Store inside Once.Do, load in ExceptionType()/ExceptionMsg().

Fixed in 91ab7c0 — replaced the two plain exceptionType/exceptionMsg string fields with atomic.Pointer[exceptionRecord]. The write happens inside Once.Do (still first-error-wins); the getters do an atomic Load, which gives a proper happens-before with any reader regardless of whether it called Do.

Nit: byte slicing can produce invalid UTF-8 in _dd.appsec.error.message

string([]byte(msg)[:maxExceptionMsgBytes]) splits at a raw byte boundary, which can cut a multi-byte rune. Also allocates twice (string→[]byte→string).

msg = msg[:maxExceptionMsgBytes]
for !utf8.ValidString(msg) {
    msg = msg[:len(msg)-1]
}

(needs "unicode/utf8" import)

Fixed in 92353b6 — same as the other UTF-8 nit; using strings.ToValidUTF8.

Context loss: invalid scope value dropped from telemetry

The previous code logged slog.String("scope", string(addrs.TimerKey)). Now only the bare in error reaches RecordException, so production telemetry will say "appsec exception" without identifying which scope triggered it.

m.RecordException(ExceptionTypeInstrumentation,
    fmt.Errorf("unexpected WAF error scope %q: %w", addrs.TimerKey, in))

Fixed in 91ab7c0 — the default case now wraps the error: fmt.Errorf("unexpected WAF error scope %q: %w", addrs.TimerKey, in).

CI: this metric breaks test_shema_metric[gin] system test

The system test asserts that only api_security.request.schema and api_security.request.no_schema telemetry metrics are present. This new api_security.missing_route metric fails that assertion:

AssertionError: Only api_security.request.schema metrics should be present,
no missing routes should be generated

The system test allowlist needs updating to include api_security.missing_route.

Fixed in 92353b6 — added && resp.StatusCode != 404 guard. A 404 means no route matched (expected gin behaviour); only a non-404 response with an empty route indicates the framework genuinely lacks routing support. Matches dd-trace-py behaviour.

Updated in 3b54935 — the previous != 404 guard wasn't sufficient: gin also returns an empty route for trailing-slash redirects (301), method-not-allowed (405), and any other response without a matched handler. Changed the condition to status >= 200 && status < 300: only 2xx responses are meaningful for "missing route" diagnostics (a framework without routing support still exposes this on successful requests; gin never triggers it because all its 2xx responses carry a non-empty route).