Merged
Conversation
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded a Changes
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (6)
src/Admin/Settings/API.php (6)
580-581: Consider making column layout configurable via group definition.The column layout is hardcoded for specific slugs. For better maintainability, consider adding a
'columns'key to the group definitions inPage.phpinstead.Alternative approach
- $fields = $group['fields']; - $columns = $group['slug'] === 'tracked_user_roles' || $group['slug'] === 'expand_dashboard_access' ? 'md:!mt-2 md:flex md:flex-wrap' : ''; + $fields = $group['fields']; + $columns = ! empty( $group['columns'] ) ? $group['columns'] : '';Then in
Page.php, add to the group definitions:[ 'label' => esc_html__( 'Track analytics for user roles', 'plausible-analytics' ), 'slug' => 'tracked_user_roles', 'columns' => 'md:!mt-2 md:flex md:flex-wrap', // ... ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Admin/Settings/API.php` around lines 580 - 581, The code currently hardcodes column CSS based on group slugs in Admin/Settings/API.php (see $columns using $group['slug']); update to read an optional 'columns' key from each group definition (add 'columns' => 'md:!mt-2 md:flex md:flex-wrap' to groups in Page.php) and use that value if present, falling back to the existing slug-based or empty string logic; adjust the assignment of $columns in the render logic to prefer $group['columns'] when set, otherwise keep the current behavior.
502-505: Apply consistent output escaping.While these values come from the controlled
get_quick_actions()method, WordPress coding standards recommend escaping at the point of output for defense-in-depth.Suggested improvement
- <a id="<?php echo $quick_action['id']; ?>" class="no-underline text-sm leading-6 text-gray-900" - target="<?php echo $quick_action['target']; ?>" href="<?php echo $quick_action['url']; ?>" - title="<?php echo $quick_action['label']; ?>"> - <?php echo $quick_action['label']; ?> + <a id="<?php echo esc_attr( $quick_action['id'] ); ?>" class="no-underline text-sm leading-6 text-gray-900" + target="<?php echo esc_attr( $quick_action['target'] ); ?>" href="<?php echo esc_url( $quick_action['url'] ); ?>" + title="<?php echo esc_attr( $quick_action['label'] ); ?>"> + <?php echo esc_html( $quick_action['label'] ); ?>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Admin/Settings/API.php` around lines 502 - 505, The output for quick action attributes isn't escaped at point of output; update the rendering in the loop that consumes get_quick_actions() so each value is properly escaped: use esc_attr() for id and target and title, esc_url() for the href, and esc_html() (or esc_html_e/esc_html depending on context) for the label text when echoing $quick_action['id'], $quick_action['target'], $quick_action['url'], $quick_action['label'] inside the anchor; keep the existing keys and structure but wrap each echoed value with the appropriate escaping function to satisfy WordPress coding standards.
753-753: Escape the checkbox label output.For consistency with WordPress coding standards:
Suggested fix
- <span class="ml-2 dark:text-gray-100 text-lg"><?php echo $field['label']; ?></span> + <span class="ml-2 dark:text-gray-100 text-lg"><?php echo esc_html( $field['label'] ); ?></span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Admin/Settings/API.php` at line 753, The checkbox label is output unescaped via "<?php echo $field['label']; ?>" which can expose XSS; update the span to echo the escaped label using esc_html() for $field['label'] (i.e., replace the direct echo with an escaped output of $field['label']), keeping the surrounding span and class intact.
555-565: Apply escaping to group field outputs.The
$group['slug']is used in HTML attributes including anonclickhandler. Escaping prevents potential issues if slugs contain special characters.Suggested improvement
- <div id="<?php echo $group['slug']; ?>_toggle" onclick="plausibleToggleSection('<?php echo $group['slug']; ?>')" class="flex items-center mt-4 space-x-3 hover:cursor-pointer"> + <div id="<?php echo esc_attr( $group['slug'] ); ?>_toggle" onclick="plausibleToggleSection('<?php echo esc_js( $group['slug'] ); ?>')" class="flex items-center mt-4 space-x-3 hover:cursor-pointer"> <span class="dark:text-gray-100 text-lg"> - <?php echo $group['label']; ?> + <?php echo esc_html( $group['label'] ); ?> </span> <!-- Chevron --> - <svg xmlns="http://www.w3.org/2000/svg" id="<?php echo $group['slug']; ?>_chevron" class="h-6 w-6 ml-2 text-gray-400 dark:text-gray-500 transition-transform duration-250" fill="none" - viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor"> + <svg xmlns="http://www.w3.org/2000/svg" id="<?php echo esc_attr( $group['slug'] ); ?>_chevron" class="h-6 w-6 ml-2 text-gray-400 dark:text-gray-500 transition-transform duration-250" fill="none" + viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor"> <path stroke-linecap="round" stroke-linejoin="round" d="m4.5 15.75 7.5-7.5 7.5 7.5"/> </svg> </div> - <div class="hidden" id="<?php echo $group['slug']; ?>_content"> + <div class="hidden" id="<?php echo esc_attr( $group['slug'] ); ?>_content">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Admin/Settings/API.php` around lines 555 - 565, Escape all outputs derived from $group when writing into HTML attributes, IDs and inline JS to prevent injection and broken markup: replace direct uses of $group['slug'] in id attributes and the onclick handler (e.g., occurrences that build "<?php echo $group['slug']; ?>_toggle", "<?php echo $group['slug']; ?>_chevron", "<?php echo $group['slug']; ?>_content" and the plausibleToggleSection(...) call) with a properly escaped value (use htmlspecialchars($group['slug'], ENT_QUOTES, 'UTF-8') for attribute/ID contexts and use json_encode($group['slug']) or similar JS-escaping when embedding into the onclick string), and also escape $group['label'] when rendering the visible text (e.g., htmlspecialchars($group['label'], ENT_QUOTES, 'UTF-8')).
129-129: Consider sanitizing$_GET['tab']input.The tab parameter is used directly without sanitization. While this is in the admin context, it's good practice to sanitize and validate against allowed values to prevent potential issues if an invalid tab is passed (which would access a non-existent key at line 153).
Suggested improvement
- $current_tab = ! empty( $_GET['tab'] ) ? $_GET['tab'] : 'general'; + $current_tab = ! empty( $_GET['tab'] ) ? sanitize_key( $_GET['tab'] ) : 'general'; + if ( ! isset( $this->fields[ $current_tab ] ) ) { + $current_tab = 'general'; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Admin/Settings/API.php` at line 129, $current_tab is assigned directly from $_GET['tab'] without sanitization; update the assignment to sanitize and validate it against the allowed tab keys before use. Use a sanitized input (e.g., cast to string and run through sanitize_text_field or equivalent) and check it exists in your tabs list (the array used to render/lookup tabs, referenced where $current_tab is later used) then fall back to 'general' if not valid; ensure you modify the code around the $current_tab assignment and the validation uses the same allowed-tabs array used elsewhere in the class.
587-587: Escape the group label output.The label is output without escaping while the description below correctly uses
wp_kses_post().Suggested fix
- <h3 class="text-lg mt-0 leading-6 font-medium text-gray-900 dark:text-gray-100" id="<?php echo $group['slug']; ?>"><?php echo $group['label']; ?></h3> + <h3 class="text-lg mt-0 leading-6 font-medium text-gray-900 dark:text-gray-100" id="<?php echo esc_attr( $group['slug'] ); ?>"><?php echo esc_html( $group['label'] ); ?></h3>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Admin/Settings/API.php` at line 587, The group label is echoed unescaped (<?php echo $group['label']; ?>) which can introduce XSS; update the output to escape the label using esc_html() when echoing $group['label']; also ensure the id attribute using $group['slug'] is escaped with esc_attr() to be safe (adjust the h3 rendering that references $group['slug'] and $group['label'] accordingly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Admin/Settings/API.php`:
- Around line 580-581: The code currently hardcodes column CSS based on group
slugs in Admin/Settings/API.php (see $columns using $group['slug']); update to
read an optional 'columns' key from each group definition (add 'columns' =>
'md:!mt-2 md:flex md:flex-wrap' to groups in Page.php) and use that value if
present, falling back to the existing slug-based or empty string logic; adjust
the assignment of $columns in the render logic to prefer $group['columns'] when
set, otherwise keep the current behavior.
- Around line 502-505: The output for quick action attributes isn't escaped at
point of output; update the rendering in the loop that consumes
get_quick_actions() so each value is properly escaped: use esc_attr() for id and
target and title, esc_url() for the href, and esc_html() (or esc_html_e/esc_html
depending on context) for the label text when echoing $quick_action['id'],
$quick_action['target'], $quick_action['url'], $quick_action['label'] inside the
anchor; keep the existing keys and structure but wrap each echoed value with the
appropriate escaping function to satisfy WordPress coding standards.
- Line 753: The checkbox label is output unescaped via "<?php echo
$field['label']; ?>" which can expose XSS; update the span to echo the escaped
label using esc_html() for $field['label'] (i.e., replace the direct echo with
an escaped output of $field['label']), keeping the surrounding span and class
intact.
- Around line 555-565: Escape all outputs derived from $group when writing into
HTML attributes, IDs and inline JS to prevent injection and broken markup:
replace direct uses of $group['slug'] in id attributes and the onclick handler
(e.g., occurrences that build "<?php echo $group['slug']; ?>_toggle", "<?php
echo $group['slug']; ?>_chevron", "<?php echo $group['slug']; ?>_content" and
the plausibleToggleSection(...) call) with a properly escaped value (use
htmlspecialchars($group['slug'], ENT_QUOTES, 'UTF-8') for attribute/ID contexts
and use json_encode($group['slug']) or similar JS-escaping when embedding into
the onclick string), and also escape $group['label'] when rendering the visible
text (e.g., htmlspecialchars($group['label'], ENT_QUOTES, 'UTF-8')).
- Line 129: $current_tab is assigned directly from $_GET['tab'] without
sanitization; update the assignment to sanitize and validate it against the
allowed tab keys before use. Use a sanitized input (e.g., cast to string and run
through sanitize_text_field or equivalent) and check it exists in your tabs list
(the array used to render/lookup tabs, referenced where $current_tab is later
used) then fall back to 'general' if not valid; ensure you modify the code
around the $current_tab assignment and the validation uses the same allowed-tabs
array used elsewhere in the class.
- Line 587: The group label is echoed unescaped (<?php echo $group['label']; ?>)
which can introduce XSS; update the output to escape the label using esc_html()
when echoing $group['label']; also ensure the id attribute using $group['slug']
is escaped with esc_attr() to be safe (adjust the h3 rendering that references
$group['slug'] and $group['label'] accordingly).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 01a3e115-402b-4df8-af70-8e189615f3d3
📒 Files selected for processing (2)
package.jsonsrc/Admin/Settings/API.php
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Admin/Settings/Page.php (1)
690-695:⚠️ Potential issue | 🟡 MinorURL-encode
page-urlbefore nesting it into the iframe query.Appending the raw page URL here works only until that nested URL has its own
&...parameters. At that point the iframe query is split incorrectly and page-level drilldowns break for those URLs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Admin/Settings/Page.php` around lines 690 - 695, The code appends $page_url directly into $shared_link which breaks when the nested URL contains its own query string; update the append logic in Page.php to URL-encode the page value before concatenation by replacing the current use of $page_url with rawurlencode( esc_url( $page_url ) ) (or urlencode if you prefer) so the nested URL is safely encoded: use the encoded value when building $shared_link (reference variables: $page_url and $shared_link).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Admin/Settings/API.php`:
- Line 129: Validate and sanitize the tab value before using it to index
$this->fields: when setting $current_tab (from $_GET['tab']) check that the
value exists as a key in $this->fields (e.g., with isset($this->fields[$tab]) or
array_key_exists) and if not, fall back to 'general'; ensure you use the
validated $current_tab for subsequent indexing to avoid undefined index/illegal
offset when referencing $this->fields.
In `@src/Admin/Settings/Page.php`:
- Around line 122-126: The button label shows "Connected" based only on
non-empty domain/token; update the label conditional to reflect actual
connection state by requiring the client to be built and the token to be valid:
use $this->client instanceof Client && $this->client->is_api_token_valid() in
the ternary so "Connected" is shown only when the client exists and
is_api_token_valid() returns true, otherwise show "Connect". Also verify the
same logical check is used consistently with the 'disabled' flag to avoid
contradictory UI states.
---
Outside diff comments:
In `@src/Admin/Settings/Page.php`:
- Around line 690-695: The code appends $page_url directly into $shared_link
which breaks when the nested URL contains its own query string; update the
append logic in Page.php to URL-encode the page value before concatenation by
replacing the current use of $page_url with rawurlencode( esc_url( $page_url ) )
(or urlencode if you prefer) so the nested URL is safely encoded: use the
encoded value when building $shared_link (reference variables: $page_url and
$shared_link).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 45c5d16c-4a5b-4655-b365-18334a42650c
📒 Files selected for processing (2)
src/Admin/Settings/API.phpsrc/Admin/Settings/Page.php
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (8)
src/Admin/Settings/API.php (8)
805-805: Useesc_textarea()for textarea content.The
$valueis echoed directly. WordPress providesesc_textarea()specifically for escaping textarea content.🛡️ Suggested fix
- name="<?php echo esc_attr( $field['slug'] ); ?>"><?php echo $value; ?></textarea> + name="<?php echo esc_attr( $field['slug'] ); ?>"><?php echo esc_textarea( $value ); ?></textarea>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Admin/Settings/API.php` at line 805, The textarea currently echoes raw content via "$value" which is unsafe for textarea contexts; update the output in the Settings rendering (the line that prints name="<?php echo esc_attr( $field['slug'] ); ?>"><?php echo $value; ?></textarea>) to use esc_textarea($value) instead of raw $value so the textarea content is properly escaped for HTML; keep esc_attr( $field['slug'] ) as-is.
766-766: Missing escaping for$field['label'].Other values in this method are properly escaped (lines 760-761, 768), but
$field['label']is echoed raw.🛡️ Suggested fix
- <span class="ml-2 dark:text-gray-100 text-lg"><?php echo $field['label']; ?></span> + <span class="ml-2 dark:text-gray-100 text-lg"><?php echo esc_html( $field['label'] ); ?></span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Admin/Settings/API.php` at line 766, The span echoes $field['label'] without escaping; update the echo to use the same HTML-escape helper used elsewhere (e.g., htmlspecialchars($field['label'], ENT_QUOTES, 'UTF-8') or your project's esc_html helper) so output is safely escaped before rendering; replace "<?php echo $field['label']; ?>" with the escaped form referencing $field['label'].
727-727: Consideresc_html_efor button text content.
esc_attr_e()works for XSS protection, butesc_html_e()is semantically correct for text content (not HTML attributes).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Admin/Settings/API.php` at line 727, Replace the use of esc_attr_e with esc_html_e for rendering button/text content: locate the call esc_attr_e( $field['label'], 'plausible-analytics' ) and change it to esc_html_e( $field['label'], 'plausible-analytics' ) so the label text is escaped as HTML content (not as an attribute) using the $field['label'] value.
838-838: Missingesc_attr()for id attribute.Line 859 correctly uses
esc_attr()for a similar pattern, but line 838 echoes$field['slug']raw.🛡️ Suggested fix
- <div id="plausible-analytics-hook-<?php echo $field['slug']; ?>" + <div id="plausible-analytics-hook-<?php echo esc_attr( $field['slug'] ); ?>"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Admin/Settings/API.php` at line 838, The id attribute for the div that builds "plausible-analytics-hook-<?php echo $field['slug']; ?>" is outputting $field['slug'] unescaped; change that echo to use esc_attr($field['slug']) so the id becomes safe (mirror the usage found at the later occurrence that already uses esc_attr); update the div id generation in the template where the string "plausible-analytics-hook-" is concatenated with $field['slug'] to use esc_attr.
555-565: Missing escaping for$group['slug']and$group['label'].For consistency with the escaping improvements in
render_quick_actions(), these values should be escaped. While the data likely comes from controlled definitions, escaping provides defense-in-depth.🛡️ Suggested escaping
- <div id="<?php echo $group['slug']; ?>_toggle" onclick="plausibleToggleSection('<?php echo $group['slug']; ?>')" class="flex items-center mt-4 space-x-3 hover:cursor-pointer"> + <div id="<?php echo esc_attr( $group['slug'] ); ?>_toggle" onclick="plausibleToggleSection('<?php echo esc_js( $group['slug'] ); ?>')" class="flex items-center mt-4 space-x-3 hover:cursor-pointer"> <span class="dark:text-gray-100 text-lg"> - <?php echo $group['label']; ?> + <?php echo esc_html( $group['label'] ); ?> </span> <!-- Chevron --> - <svg xmlns="http://www.w3.org/2000/svg" id="<?php echo $group['slug']; ?>_chevron" class="h-6 w-6 ml-2 text-gray-400 dark:text-gray-500 transition-transform duration-250" fill="none" - viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor"> + <svg xmlns="http://www.w3.org/2000/svg" id="<?php echo esc_attr( $group['slug'] ); ?>_chevron" class="h-6 w-6 ml-2 text-gray-400 dark:text-gray-500 transition-transform duration-250" fill="none" + viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor"> <path stroke-linecap="round" stroke-linejoin="round" d="m4.5 15.75 7.5-7.5 7.5 7.5"/> </svg> </div> - <div class="hidden" id="<?php echo $group['slug']; ?>_content"> + <div class="hidden" id="<?php echo esc_attr( $group['slug'] ); ?>_content">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Admin/Settings/API.php` around lines 555 - 565, Escape $group values before output: use an attribute-appropriate escape for the slug and an HTML escape for the label. Replace raw echoes of $group['slug'] in id attributes (e.g., the elements with ids ending in _toggle, _chevron, _content and the onclick calling plausibleToggleSection) with an escaped form (e.g., esc_attr($group['slug']) or equivalent), and replace the label echo (used inside the span) with an HTML-escaped form (e.g., esc_html($group['label']) or equivalent). This matches the escaping approach used in render_quick_actions() and keeps JS ids and visible text safe.
687-698: Missing escaping for field attributes and values.Multiple dynamic values are echoed without escaping in HTML attributes (
id,name,placeholder,value) and text content.🛡️ Suggested fix
<?php if ( ! empty( $field['label'] ) ): ?> <label class="block text-sm font-medium leading-5 !text-gray-700 !dark:text-gray-300" - for="<?php echo $field['slug']; ?>"><?php echo $field['label']; ?></label> + for="<?php echo esc_attr( $field['slug'] ); ?>"><?php echo esc_html( $field['label'] ); ?></label> <?php endif; ?> <div class="mt-1"> <input class="block w-full !border-gray-300 !dark:border-gray-700 !rounded-md focus:ring-indigo-500 focus:border-indigo-500 sm:text-sm dark:bg-gray-900 dark:text-gray-300 py-2 px-3" - id="<?php echo $field['slug']; ?>" placeholder="<?php echo $placeholder; ?>" autocomplete="off" + id="<?php echo esc_attr( $field['slug'] ); ?>" placeholder="<?php echo esc_attr( $placeholder ); ?>" autocomplete="off" type="text" - name="<?php echo $field['slug']; ?>" - value="<?php echo $value; ?>" <?php echo $disabled; ?> /> + name="<?php echo esc_attr( $field['slug'] ); ?>" + value="<?php echo esc_attr( $value ); ?>" <?php echo $disabled; ?> /> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Admin/Settings/API.php` around lines 687 - 698, The echoed dynamic values in the settings input block (e.g., $field['slug'], $placeholder, $value', $disabled, $field['label'], and $classes) are not escaped; replace raw echoes with appropriate escaping functions: use esc_attr() for attribute values (id, name, placeholder, value, classes and any attribute strings like $disabled) and esc_html() (or esc_html_e/esc_html__) for label/text content, and ensure $disabled is rendered as a safe attribute (e.g., conditionally output the literal disabled string after escaping or build it with a boolean check). Update the template around the input and label rendering (references: $field['slug'], $placeholder, $value, $disabled, $field['label'], $classes) so all dynamic output is properly escaped.
587-589: Missing escaping for$group['slug']and$group['label'].
$group['desc']is properly escaped withwp_kses_post(), but$group['slug'](inidattribute) and$group['label'](in heading content) are echoed raw.🛡️ Suggested fix
- <h3 class="text-lg mt-0 leading-6 font-medium text-gray-900 dark:text-gray-100" id="<?php echo $group['slug']; ?>"><?php echo $group['label']; ?></h3> + <h3 class="text-lg mt-0 leading-6 font-medium text-gray-900 dark:text-gray-100" id="<?php echo esc_attr( $group['slug'] ); ?>"><?php echo esc_html( $group['label'] ); ?></h3>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Admin/Settings/API.php` around lines 587 - 589, Escape the raw outputs for the heading id and label: wrap $group['slug'] with esc_attr() when echoing into the id attribute and wrap $group['label'] with esc_html() (or esc_html_e/esc_html__ if translated) when echoing into the H3 content so both the id and visible label are safely escaped; update the echo calls that output $group['slug'] and $group['label'] accordingly in the block around the H3 and ensure $group['desc'] continues to use wp_kses_post().
642-666: Missing escaping for$slugin multiple locations.The
$slugvariable is echoed directly inidattributes andonclickhandlers without escaping.🛡️ Key locations to escape
- <div id="<?php echo $slug; ?>_content" class="plausible-analytics-section <?php echo $group['hidden'] ? 'hidden' : ''; ?> !mt-1 mx-14"> + <div id="<?php echo esc_attr( $slug ); ?>_content" class="plausible-analytics-section <?php echo $group['hidden'] ? 'hidden' : ''; ?> !mt-1 mx-14">- <ol id="<?php echo $slug; ?>_list" class="m-0 mt-4 list-none mb-6"> + <ol id="<?php echo esc_attr( $slug ); ?>_list" class="m-0 mt-4 list-none mb-6">- <button type="button" onclick="plausibleAddField('<?php echo $slug; ?>')" + <button type="button" onclick="plausibleAddField('<?php echo esc_js( $slug ); ?>')"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Admin/Settings/API.php` around lines 642 - 666, The $slug is echoed unescaped in HTML attributes, class names, and JS handlers; update all occurrences to use the proper escaping: wrap $slug in esc_attr() when used in id or class output (e.g. id="<?php echo esc_attr($slug); ?>_content", id="<?php echo esc_attr($slug); ?>_list", and class="<?php echo esc_attr(str_replace('_','-',$slug)); ?>-field"), and use esc_js( $slug ) (or esc_attr when building quoted JS args) for JS handlers (onclick="plausibleAddField('<?php echo esc_js($slug); ?>')" and plausibleRemoveField calls); also ensure the slug passed into render_text_field echo is escaped when output (e.g. 'slug' => esc_attr("{$slug}[{$key}]") or escape on output inside render_text_field if that function handles it).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Admin/Settings/API.php`:
- Line 805: The textarea currently echoes raw content via "$value" which is
unsafe for textarea contexts; update the output in the Settings rendering (the
line that prints name="<?php echo esc_attr( $field['slug'] ); ?>"><?php echo
$value; ?></textarea>) to use esc_textarea($value) instead of raw $value so the
textarea content is properly escaped for HTML; keep esc_attr( $field['slug'] )
as-is.
- Line 766: The span echoes $field['label'] without escaping; update the echo to
use the same HTML-escape helper used elsewhere (e.g.,
htmlspecialchars($field['label'], ENT_QUOTES, 'UTF-8') or your project's
esc_html helper) so output is safely escaped before rendering; replace "<?php
echo $field['label']; ?>" with the escaped form referencing $field['label'].
- Line 727: Replace the use of esc_attr_e with esc_html_e for rendering
button/text content: locate the call esc_attr_e( $field['label'],
'plausible-analytics' ) and change it to esc_html_e( $field['label'],
'plausible-analytics' ) so the label text is escaped as HTML content (not as an
attribute) using the $field['label'] value.
- Line 838: The id attribute for the div that builds
"plausible-analytics-hook-<?php echo $field['slug']; ?>" is outputting
$field['slug'] unescaped; change that echo to use esc_attr($field['slug']) so
the id becomes safe (mirror the usage found at the later occurrence that already
uses esc_attr); update the div id generation in the template where the string
"plausible-analytics-hook-" is concatenated with $field['slug'] to use esc_attr.
- Around line 555-565: Escape $group values before output: use an
attribute-appropriate escape for the slug and an HTML escape for the label.
Replace raw echoes of $group['slug'] in id attributes (e.g., the elements with
ids ending in _toggle, _chevron, _content and the onclick calling
plausibleToggleSection) with an escaped form (e.g., esc_attr($group['slug']) or
equivalent), and replace the label echo (used inside the span) with an
HTML-escaped form (e.g., esc_html($group['label']) or equivalent). This matches
the escaping approach used in render_quick_actions() and keeps JS ids and
visible text safe.
- Around line 687-698: The echoed dynamic values in the settings input block
(e.g., $field['slug'], $placeholder, $value', $disabled, $field['label'], and
$classes) are not escaped; replace raw echoes with appropriate escaping
functions: use esc_attr() for attribute values (id, name, placeholder, value,
classes and any attribute strings like $disabled) and esc_html() (or
esc_html_e/esc_html__) for label/text content, and ensure $disabled is rendered
as a safe attribute (e.g., conditionally output the literal disabled string
after escaping or build it with a boolean check). Update the template around the
input and label rendering (references: $field['slug'], $placeholder, $value,
$disabled, $field['label'], $classes) so all dynamic output is properly escaped.
- Around line 587-589: Escape the raw outputs for the heading id and label: wrap
$group['slug'] with esc_attr() when echoing into the id attribute and wrap
$group['label'] with esc_html() (or esc_html_e/esc_html__ if translated) when
echoing into the H3 content so both the id and visible label are safely escaped;
update the echo calls that output $group['slug'] and $group['label'] accordingly
in the block around the H3 and ensure $group['desc'] continues to use
wp_kses_post().
- Around line 642-666: The $slug is echoed unescaped in HTML attributes, class
names, and JS handlers; update all occurrences to use the proper escaping: wrap
$slug in esc_attr() when used in id or class output (e.g. id="<?php echo
esc_attr($slug); ?>_content", id="<?php echo esc_attr($slug); ?>_list", and
class="<?php echo esc_attr(str_replace('_','-',$slug)); ?>-field"), and use
esc_js( $slug ) (or esc_attr when building quoted JS args) for JS handlers
(onclick="plausibleAddField('<?php echo esc_js($slug); ?>')" and
plausibleRemoveField calls); also ensure the slug passed into render_text_field
echo is escaped when output (e.g. 'slug' => esc_attr("{$slug}[{$key}]") or
escape on output inside render_text_field if that function handles it).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 172211cb-1f32-4c42-9a43-513f09df0d93
📒 Files selected for processing (1)
src/Admin/Settings/API.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Admin
Chores