Conversation
Add sanitized href and aria-label to the click action composed_path_selector. This helps identify the clicked element without exposing PII. The href value keeps only the origin and the path. The path removes segments that look like generated identifiers. The href value drops the query string and the hash. A non-http(s) href keeps only the scheme. The aria-label value follows the same privacy rules as action names. This feature is behind the composed_path_selector_attributes experimental flag. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
❌ ErrorsYour PR has failed checks. Please review the issues below and take necessary action before merging. 🚦 2 Pipeline jobs failed
|
Bundles Sizes Evolution
|
Run Prettier to fix a formatting issue. Fix the e2e assertion to match CSS.escape output for spaces and slashes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add a preceding sibling in the test body so the link's nth-child position stays fixed. Without it, the async and npm CI test setups append an init script after </body>. Browsers reparent that script into <body> as a sibling, which changed the number of children and added an unexpected nth-child to the selector. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Change the href sanitization to keep the (deduplicated) query parameter names while still dropping their values. Parameter names are typically static field names, not user data, so keeping them makes the selector more useful without exposing what was in the query string. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Do not apply the privacy-level masking to href. Its own sanitization (dropping query values, the hash, and any non-http(s) payload) is already the intended protection for this attribute, regardless of the privacy level in effect. aria-label keeps the existing masking behavior. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fix bugs found by code review: - Exclude href and aria-label from the safe-attributes allowlist, so setting actionNameAttribute to either no longer leaks the raw, unsanitized value alongside the sanitized one. - Detect the href's non-http(s) scheme from the parsed URL instead of a regex on the raw string, so a leading whitespace or control character can no longer bypass the scheme-only shortcut and leak the full payload (an email address, a script...). - Mask aria-label at the hidden and ignore privacy levels too, not only mask and mask-unless-allowlisted, matching how session replay already handles them. - Compare the resolved origin against the current page's origin to decide whether to show it, instead of pattern-matching the raw href, so a backslash-led href cannot hide a real cross-origin navigation. - Decode a path segment before checking if it looks generated, so a percent-encoded non-ASCII segment (ex: an accented word) is not redacted by mistake. - Resolve an empty href to the current document instead of treating it as absent. Also extract the aria-label masking logic into a shared, exported maskAttributeIfNeeded in privacy.ts, and fix a test cleanup that used an inline delete instead of registerCleanupTask. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The backslash-led href test relied on old Chrome/Edge (BrowserStack legacy devices) resolving "\\evil.example/..." as a file: URL, unlike current browsers, which resolve it as http:. Replace it with a <base> element pointing to a different origin, which resolves consistently across browsers and still exercises the same origin-comparison code path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add `_dd.action.target.attributes`, a key-value map of href, aria-label, name, title, alt, id, role, and data-* attributes collected from a click's composedPath, behind a new `composed_path_selector_attributes_map` experimental flag. This lets customers filter and group RUM click actions by these values, which the existing `composed_path_selector` CSS-like string does not support. href and aria-label are no longer collected in `composed_path_selector`, to avoid collecting the same PII-sensitive data in two fields. Every collected value is dropped if it contains an email address or a digit, regardless of privacy level, so the map never carries a numeric identifier or an email address. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
`maskAttributeIfNeeded` was gated behind `enablePrivacyForActionName`, so disabling that action-name-only option would also disable masking for the new attributes facet map. `data-dd-privacy` was leaking into the map as a regular data-* attribute, and `placeholder` was missing from the masked-text set despite the code claiming parity with `shouldMaskAttribute`. The new e2e test also enabled the wrong experimental-flag string, so it silently ran against unflagged behavior. Also dedupes `FILTERED_TAGNAMES` into a shared export, fixes a stale comment, and adds dedicated `urlSanitizer.spec.ts` coverage. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Not asked for in the original request; removing to keep the collected attribute set limited to what customers actually need.
id, role, and data-* commonly carry digits customers want (product ids, SKUs, test ids). The blanket digit-drop now applies only to aria-label/name/title/alt, which are the free-form text attributes most likely to leak a raw identifier when unmasked. The email check still applies to every attribute.
…tization Move the field to action.target.attributes (public schema) instead of _dd.action.target.attributes, per RFC feedback. Remove the specialized digit/email content filter and href structural sanitization: every attribute (including href) now goes through the same masking pipeline already used for action names (shouldMaskAttribute/maskAttributeIfNeeded). composed_path_selector already excluded href/aria-label, so it needed no change beyond dropping the now-unused urlSanitizer import. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Continuation of 2de9037: update rawRumEvent.types.ts and actionCollection.ts to place attributes under the public action.target instead of _dd.action.target, rewrite getComposedPathAttributes.ts to drop custom href/digit sanitization in favor of the existing maskAttributeIfNeeded pipeline, and update the affected unit/e2e tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The mask-unless-allowlisted test cases set window.$DD_ALLOW per iteration but never reset it, leaving the last case's value (['foo']) leaked globally for the rest of the Karma run. This made CI's randomized test order intermittently unmask attributes equal to 'foo' in unrelated specs (e.g. getComposedPathAttributes.spec.ts). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
57ec69a to
ce8aac5
Compare
111d4e8 to
ecc658d
Compare
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ecc658d29f
ℹ️ 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".
| resource?: Count | ||
| target: { | ||
| name: string | ||
| attributes?: Record<string, string> |
There was a problem hiding this comment.
Regenerate the public action type before exposing attributes
issue: Adding the field only to RawRumActionEvent leaves the generated public RumActionEvent.action.target type unchanged, where this property is inferred through the index signature as unknown. Consequently, TypeScript customers cannot type-safely inspect or redact action.target.attributes in beforeSend, despite this being introduced as a public field; merge the companion schema change, update the schema dependency, and regenerate rumEvent.types.ts.
AGENTS.md reference: AGENTS.md:L124-L128
Useful? React with 👍 / 👎.
- Enforce MASK without consulting $DD_ALLOW: only MASK_UNLESS_ALLOWLISTED should honor the allowlist, so an explicitly masked value can no longer leak raw just because it's allowlisted. - Only exclude href/aria-label from composed_path_selector once the click_target_attributes_map flag is enabled, so customers using actionNameAttribute without the flag keep seeing the value there. - Add action.target.attributes to the modifiable field paths so beforeSend can redact it, matching the existing header-map pattern. - Compare tagName case-insensitively when collecting/masking href, so SVG <a href> elements (lowercase tagName) are no longer skipped. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Left the |
Motivation
_dd.action.target.composed_path_selectoridentifies the clicked element for auto-collected click actions. It is a single CSS-like string. Customers cannot filter or group RUM click actions by one attribute value, for examplehreforaria-label.This change adds a key-value map of click-target attributes. Customers can use it as a facet to filter and group click actions.
Design rationale, prior-art comparison (Heap, PostHog, FullStory, Amplitude, GA4), and customer motivation are written up in RFC: Collecting Click-Target Attributes as a Facetable Key-Value Map.
JIRA: PANA-8907
The field is public:
action.target.attributes. This needs a companion schema change inrum-events-format: DataDog/rum-events-format#439 (draft).Changes
action.target.attributes, a key-value map (href,aria-label,name,title,alt,id,role,data-*) collected target-first from a click'scomposedPath(), capped at 20 keys / 100 chars per value.shouldMaskAttribute/maskAttributeIfNeeded) for every masked attribute — no attribute-specific sanitization is added.click_target_attributes_mapexperimental flag;href/aria-labelare no longer duplicated incomposed_path_selector.getClickTargetAttributes.ts.Test instructions
yarn test:e2e -g "composed path"covers the attribute map and the selector string, with the correct experimental flag enabled.yarn dev, open the sandbox athttp://localhost:8080(the port increments if 8080 is busy — check the terminal output for the actual URL), and update theDD_RUM.init()call insandbox/index.htmlto enable the flag:{ "clientToken": "xxx", "applicationId": "xxx", "sessionReplaySampleRate": 100, "profilingSampleRate": 100, "trackResources": true, "trackLongTasks": true, "telemetrySampleRate": 100, "telemetryConfigurationSampleRate": 100, "telemetryUsageSampleRate": 100, "enableExperimentalFeatures": ["click_target_attributes_map"], "proxy": "/proxy" }aria-label, then confirmaction.target.attributeson the intake payload contains the rawhrefandaria-labelvalues at the default privacy level.Checklist
🤖 Generated with Claude Code