Repository navigation
Conversation
Automated security fix generated by OrbisAI Security
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2893d6172b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
This PR introduces changes that may not work on all browsers. According to Babel, the following polyfills may be needed, and they are not automatically included:
The best way to address this is to provide good test coverage, as normal PR checks run unit tests on older browsers. |
Drop the window.location.href base so missing, empty, and relative tracker URLs are no longer resolved to same-origin URLs. Trackers must be absolute http(s) URLs with a host; the original string is still escaped with attributeValue(). Adds regression tests. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d911c5a30
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const { protocol, hostname } = new URL(url); | ||
| return (protocol === 'http:' || protocol === 'https:') && !!hostname; |
There was a problem hiding this comment.
When a tracker is https:tracker.example/pixel (likewise https:/... or a backslash variant), the one-argument parser normalizes it to https://tracker.example/pixel, so this returns true, but the unnormalized input is emitted into the script tag. In the inserted about:blank iframe on an HTTPS publisher, the inherited base makes that same-scheme spelling resolve as a publisher-relative URL (for example, https://publisher.example/dir/tracker.example/pixel), producing a same-origin script request instead of the tracker; the old ^https?:// guard rejected it. Fresh evidence beyond the earlier removed-base case is this no-base-validation/original-string mismatch; require the // authority form as well or emit the parsed href.
Useful? React with 👍 / 👎.
Barecheck - Code coverage reportTotal: 91.66%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
|
✅ Review Feedback Addressed I've automatically addressed 1 review comment(s): Looking at the conversation history, there's a Codex review comment that identifies a security issue: P2: Require an authority before accepting HTTP tracker URLs When a tracker URL is The fix is to compare the normalized URL with the original input string. If they don't match, it means the URL parser had to fill in missing components (like adding I'll update the Files modified:
The changes have been pushed to this PR branch. Please review! |
|
Tread carefully! This PR adds 5 linter errors (possibly disabled through directives):
|
Summary
Tighten JS viewability tracker URL validation in
libraries/bidViewabilityPixels/index.js.Changes
http:/https:URLs with a host (new URL(url), no base URL).attributeValue()when building the iframe markup.test/spec/libraries/bidViewabilityPixels_spec.js.Notes
The previous
^https?://filter already rejected relative/missing URLs; this change additionally rejects hostless URLs such ashttps://. Output escaping (attributeValue) is the control against markup injection; validation and escaping address different risks.Verification
npx gulp lint --files libraries/bidViewabilityPixels/index.js,test/spec/libraries/bidViewabilityPixels_spec.js --nolintfix: no outputnpx gulp test-only --file test/spec/libraries/bidViewabilityPixels_spec.js: 27 passed (ChromeHeadless)npx gulp test-only(full suite): all 8 chunks passed