[common] Add non-mutating log_telemetry bridge action (errorCode), Fixes AB#3688631

[wzhipan]

Summary

Implements AB#3688631 (child of Feature AB#3688629 — Auth UX Telemetry Native Host: Broker / common-core (AndroidCommon)).

Adds a dedicated, non-mutating log_telemetry action to the existing AuthUxJavaScriptInterface JS bridge so Auth UX page JS can push an opaque server error code (e.g. an STS code such as 530003) to the native host over the already-shipped window.broker.postMessageToBroker(...) → receiveAuthUxMessage path — without creating a new bridge, WebView interface, or telemetry island.

Wire format

Implemented against the Auth UX design-doc shape:

{
  "correlationID": "$correlationId",
  "action_name": "log_telemetry",
  "action_component": "host",
  "params": {
    "v": 1,
    "sessionID": "$sessionID",
    "errorCode": 530003,
    "pageId": "ConvergedTFA",
    "trackingId": "$trackingId"
  }
}

Adding new key/value pairs — at the top level and inside params — does not break parsing or dispatch: the custom deserializer reads only known top-level keys, and Gson ignores unknown params fields. Covered by explicit tests.

Changes

Dispatch

• Telemetry is matched on the top-level action_name == "log_telemetry" first, before params.operation. This ordering is load-bearing: a log_telemetry message that smuggles params.operation = "number_matching" must never reach the number-match device store. Pinned by a test that fails if the branches are reordered.
• number_matching continues to dispatch off params.operation, unchanged.

Sink seam

• AuthUxTelemetrySink receives an AuthUxTelemetryEvent — correlationId, errorCode, sessionId, pageId, trackingId, version — rather than a bare error code. correlationId is the Kusto / DRI join key; passing the whole context now avoids a source-breaking change to a published SAM interface when a downstream consumer wants those fields.
• The sink returns whether it consumed the event. A sink may legitimately accept a call and record nothing (the AB#3688632 implementation does exactly this while its recorder is not yet attached). Returning false leaves the code eligible for a later retry instead of it being suppressed as already-forwarded and lost.
• Optional (@JvmOverloads) telemetrySink constructor param, so the existing no-arg construction stays source- and binary-compatible.

Validation and bounding — errorCode is page-controlled and (via AB#3688632) ends up in an uploaded blob, so it is:

• shape/length checked against ^[A-Za-z0-9_-]{1,32}$ — permissive enough for both numeric STS codes and the symbolic constants addBlockingError accepts;
• validated before it reaches any log line, so an embedded CR/LF cannot forge log entries;
• de-duplicated and capped per bridge instance — 10 distinct codes handed to a sink, plus a separate bound of 25 total post-validation attempts so the paths that deliberately do not record a handled code (no sink wired, sink declines) are bounded too;
• and every other page-supplied value (action_name, operation, correlationID) is sanitized before logging too.

Observability — distinct log lines for forwarded / malformed / duplicate / capped / not-consumed / no-sink-wired, each carrying the correlation ID; new authux_js_error_code span attribute, set only for codes a sink actually took (so the span cannot disagree with downstream telemetry). A throwing sink gets its own try/catch and a sink-specific message rather than being misreported as a payload parsing failure, and a throw suppresses retry only — it does not set the span attribute or claim the code was forwarded, because nothing reached downstream telemetry.

Model — params is modelled with @SerializedName; all fields nullable. Note v is deliberately typed String?, not Int?: Gson throws on a type mismatch, so an Int? version meant a "1.0" or "v2" value would drop the entire message including errorCode. Please do not "clean this up" back to Int.

Scope of the dedupe/cap

The cap is per bridge instance, and the WebView host rebuilds the bridge on every navigation — so it is a per-page flood guard, not a request-wide guarantee. Request-wide de-duplication belongs to the consumer that owns the blob and is implemented in AB#3688632. This is stated plainly in the KDoc and pinned by a test.

Security (H3 / H6)

The new branch is on a separate code path from write_data / number_matching / storeNumberMatch — telemetry-only, append-oriented, and it never mutates device or credential state. The client takes no action on the code (H6); errorCode is an opaque telemetry value.

Threading

@JavascriptInterface methods are dispatched on the WebView's private JavaBridge thread, not the UI thread. The bridge serializes its own calls per instance and never invokes a sink while holding an internal lock. It cannot make host-owned state safe, so the recorder's collections are made thread-safe in AB#3688632 rather than relying on a documentation-only requirement.

Merge order — please read

The host wiring for this seam lives in the stacked child PR #3201. If this PR lands on dev alone, dev carries a log_telemetry path that no host wires: every message will parse, validate, and then log "no telemetry sink is wired; dropping". Land them together, or expect that silence.

Both flows also remain gated on Auth UX actually emitting log_telemetry server-side (AB#3696811).

Scope note — brokered flow only (by design)

Bridge exposure is gated by shouldExposeJavaScriptInterface(...) → ProcessUtil.isRunningOnAuthService(...) (plus the origin allow-list and the ENABLE_JS_API_FOR_AUTHUX flight), so this action is exercised in the brokered flow only. Non-brokered (OneAuth brokerless WebView) + iOS enablement is tracked under Feature AB#3688630.

Out of scope (follow-up PBIs)

• AB#3688632 — append the code to the onboarding blob's blocking-errors list + non-blocking exclusion (wires the concrete sink).
• AB#3688633 — full page→native security controls (H1, H4–H6).
• AB#3688634 — additional unit-test matrix.

Testing

./gradlew :common:testLocalDebugUnitTest \
  --tests "*AuthUxJavaScriptInterfaceTest" --tests "*AuthUxJsonPayloadTest"

BUILD SUCCESSFUL — AuthUxJavaScriptInterfaceTest 37/37, AuthUxJsonPayloadTest 10/10, 0 failures.

Two behaviours are pinned by revert-tests (they fail if the fix is undone):

• dispatch order — "log_telemetry must never write to the number-match store";
• record-only-when-consumed — "expected:<[530003]> but was:<[]>", i.e. the code would otherwise be permanently lost;
• the attempt bound — "expected:<25> but was:<40>", i.e. an unconsumed code would otherwise re-invoke the sink without limit.

Known, accepted limitations

• errorCode: true is coerced by Gson to the string "true", which is alphanumeric and so indistinguishable from a symbolic code by shape. Validation bounds charset and length, not semantics — judging what is a meaningful error code is the server contract's job. Recorded as a named test rather than left undiscovered.
• A structured v ({major: 2} / [2]) still throws and drops the message. That shape is not part of the contract.
• errorCode: 530003.0 is captured as "530003.0" and then rejected by validation — the recorded value depends on how the page serializes the number.

[github-actions]

✅ Work item link check complete. Description contains link AB#3688631 to an Azure Boards work item.

[github-actions]

✅ Work item link check complete. Description contains link AB#3688631 to an Azure Boards work item.

[github-actions]

✅ Work item link check complete. Description contains link AB#3688631 to an Azure Boards work item.

[Copilot]

Pull request overview

Adds a new non-mutating log_telemetry action to the Auth UX WebView JS bridge so Auth UX page JS can forward an opaque server error code to a host-provided telemetry sink, while keeping number_matching behavior isolated and unchanged.

Changes:

• Introduces AuthUxTelemetryEvent + AuthUxTelemetrySink and dispatches action_name == "log_telemetry" before params.operation, forwarding validated error codes with context.
• Extends the Auth UX payload model (AuthUxParams) to support telemetry fields (v, errorCode, pageId, trackingId) with nullable fields and a string-typed schema version.
• Adds authux_js_error_code OpenTelemetry attribute + expands unit tests and updates changelog.txt.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
common4j/src/main/com/microsoft/identity/common/java/opentelemetry/AttributeName.java	Adds new span attribute authux_js_error_code for Auth UX error-code telemetry.
common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayload.kt	Extends payload params model + deserializer constants to support telemetry fields.
common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt	Adds the log_telemetry dispatch path, sink seam, validation/dedupe/capping, and span attribute setting.
common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadTest.kt	Adds deserialization tests for the log_telemetry wire shape and forward-compat cases.
common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterfaceTest.kt	Adds dispatch/behavioral tests for sink forwarding, retry eligibility, dedupe/caps, and safety invariants.
changelog.txt	Documents the new log_telemetry bridge behavior in vNext.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (2)

common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt:233

• sanitizeCorrelationId() currently uses value.map { ... }.joinToString(""), which walks and allocates a full copy of the correlation ID even though the result is truncated to 128 chars. Since correlationID is page-controlled, this can cause unnecessary large allocations/CPU work for oversized inputs.

Recommendation: sanitize incrementally and stop once MAX_CORRELATION_ID_LENGTH is reached, so processing stays bounded.

        private fun sanitizeCorrelationId(value: String): String {
            val flattened = value.map { if (it.isISOControl()) ' ' else it }.joinToString("")
            return if (flattened.length <= MAX_CORRELATION_ID_LENGTH) {
                flattened
            } else {

common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt:216

• sanitizeForLog() builds a full-size copy of the untrusted page-supplied string via value.replace(...) before truncating. A malicious/buggy page can send a very large value and trigger large allocations/CPU work on the JavaBridge thread (potential DoS) even though the log output is capped.

Recommendation: sanitize while iterating and stop once MAX_LOGGED_VALUE_LENGTH is reached (also replaces control chars), so work is bounded regardless of input size.

This issue also appears on line 229 of the same file.

            val flattened = value.replace('\n', ' ').replace('\r', ' ')
            return if (flattened.length <= MAX_LOGGED_VALUE_LENGTH) {
                flattened
            } else {
                flattened.substring(0, MAX_LOGGED_VALUE_LENGTH) + "...(truncated)"

[wzhipan]

Fixed in 3104c92 — picked up both suppressed comments about sanitizeForLog / sanitizeCorrelationId.

Both were valid. sanitizeCorrelationId was the worse of the two: Kotlin's String.map produces a List<Char>, so it boxed every character of the input before joining. Replaced both with a single sanitizeBounded(value, limit) that walks at most limit characters into a pre-sized StringBuilder, so the work is proportional to the output bound rather than to the size of the untrusted input.

One correction to the framing: this bounds our processing, not the attacker's allocation. By the time the bridge is invoked the value is already fully materialized — WebView marshals the whole payload across JNI and Gson parses it before receiveAuthUxMessage sees it — so the DoS characterization is weaker than stated. The change is still strictly better and trivially cheap, hence taking it.

Bonus from unifying on the shared helper: sanitizeForLog now strips all control characters rather than only CR/LF, so an ESC or NEL can no longer reach a log line either. Two tests added (oversized input bounded; control chars beyond CR/LF stripped), and the existing test asserting a 36-char GUID survives verbatim still passes — 41 bridge tests green.

[Praveen Kumar (Prvnkmr337)]

Overall, the change looks good. The new telemetry action is well isolated from the existing number-matching path, defensively validates page-controlled input, and has strong test coverage. I left a few minor comments around clarity, naming, and additional test coverage; none are blocking.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterfaceTest.kt:236

• Use SpanExtension.makeCurrentSpan(span) instead of Span.makeCurrent() when installing the current span. SpanExtension wraps makeCurrent() to avoid known AbstractMethodError/NPE issues on some devices (and keeps test code aligned with the production pattern).

    /** Runs [block] with [span] installed as the current span. */
    private fun withCurrentSpan(span: Span, block: () -> Unit) {
        span.makeCurrent().use { block() }
    }