Skip to content

feat(pii): add data collection options#2147

Open
Litarnus wants to merge 4 commits into
data-collectionfrom
data-collection-configs
Open

feat(pii): add data collection options#2147
Litarnus wants to merge 4 commits into
data-collectionfrom
data-collection-configs

Conversation

@Litarnus

Copy link
Copy Markdown
Contributor

Adds the configuration structure necessary for data collection.

This is our first nested configuration and to be backwards compatible, it defines OptionsResolvers per layer.

Comment thread src/Options.php Outdated
Comment thread src/Options.php Outdated
Comment thread src/DataCollection/DataCollectionOptions.php

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ee33464. Configure here.

return [
'request' => $headers,
'response' => $headers,
];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shared request response header arrays

Medium Severity

When flat http_headers config is normalized, request and response are assigned the same array instance. Later in-place changes to one direction’s mode or terms also change the other, unlike DataCollectionOptions::default() and per-direction config.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ee33464. Configure here.

Comment on lines +172 to +177
public function setHttpBodies(array $httpBodies): self
{
$this->options['http_bodies'] = $httpBodies;

return $this;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The setHttpBodies() method lacks validation, allowing invalid values to be set. This bypasses the validation logic applied during initial option configuration.
Severity: LOW

Suggested Fix

The setHttpBodies() method should validate its input to ensure it only contains allowed values, similar to how setHttpHeaders() calls normalizeHttpHeaders(). Create a normalizeHttpBodies() method that validates the input array against the HTTP_BODY_TYPES constants and call it from within setHttpBodies().

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/DataCollection/DataCollectionOptions.php#L172-L177

Potential issue: The `setHttpBodies()` method in `DataCollectionOptions` directly sets
the `http_bodies` option without any validation. This is inconsistent with the
validation logic present in `Options.php` which ensures `http_bodies` only contains
values from `HTTP_BODY_TYPES`. It is possible to bypass this initial validation by
retrieving the `DataCollectionOptions` object and calling `setHttpBodies()` directly
with an invalid array. While the immediate impact is unclear as the SDK does not yet
seem to use this value, storing invalid data violates the class's invariants and could
lead to unexpected behavior in future code that consumes it. Other setters like
`setHttpHeaders()` already perform normalization, highlighting this inconsistency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant