Skip to content

pr: [Nightly Fix] - Security - Harden Inline JSON Output#8

Open
jewel-claw wants to merge 1 commit intomasterfrom
nightly-fix/harden-inline-json
Open

pr: [Nightly Fix] - Security - Harden Inline JSON Output#8
jewel-claw wants to merge 1 commit intomasterfrom
nightly-fix/harden-inline-json

Conversation

@jewel-claw
Copy link

What

  • replace raw json_encode calls used inside inline <script> tags with wp_json_encode

Why

  • these values are injected into HTML script contexts
  • WordPress provides wp_json_encode specifically to produce safer JSON output for frontend rendering and avoid edge-case script-breaking payloads

Fix

  • update the TinyMCE table payload and two public inline JS payloads to use wp_json_encode

Confidence

  • linted app/Hooks/Handlers/EditorBlockHandler.php and app/Modules/DataProviders/NinjaFooTable.php with php -l

@greptile-apps
Copy link

greptile-apps bot commented Mar 10, 2026

PR author is not in the allowed authors list.

@qodo-code-review
Copy link

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Review Summary by Qodo

Harden inline JSON output with wp_json_encode

🐞 Bug fix ✨ Enhancement

Grey Divider

Walkthroughs

Description
• Replace unsafe json_encode with WordPress wp_json_encode in inline script tags
• Hardens JSON output security for TinyMCE editor and frontend table rendering
• Prevents potential script-breaking payloads in HTML script contexts
Diagram
flowchart LR
  A["json_encode calls"] -- "replaced with" --> B["wp_json_encode"]
  B -- "applied to" --> C["EditorBlockHandler.php"]
  B -- "applied to" --> D["NinjaFooTable.php"]
  C -- "secures" --> E["TinyMCE table payload"]
  D -- "secures" --> F["Cart items inline JS"]
  D -- "secures" --> G["Table vars inline JS"]
Loading

Grey Divider

File Changes

1. app/Hooks/Handlers/EditorBlockHandler.php Security enhancement +1/-1

Secure TinyMCE table payload JSON encoding

• Replaced json_encode($tables) with wp_json_encode($tables) in TinyMCE editor footer script
• Secures JSON output for table data injected into inline script context

app/Hooks/Handlers/EditorBlockHandler.php


2. app/Modules/DataProviders/NinjaFooTable.php Security enhancement +2/-2

Secure inline JavaScript JSON encoding for tables

• Replaced json_encode($cartItems) with wp_json_encode($cartItems) in WooCommerce cart footer
 script
• Replaced json_encode($vars, true) with wp_json_encode($vars) in table instance variables
 footer script
• Removes unnecessary second parameter from wp_json_encode call

app/Modules/DataProviders/NinjaFooTable.php


Grey Divider

Qodo Logo

@qodo-code-review
Copy link

qodo-code-review bot commented Mar 10, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Remediation recommended

1. Inline JSON escaping changed 🐞 Bug ⛨ Security
Description
NinjaFooTable::addInlineVars now emits wp_json_encode($vars) with default options, removing the
previously-passed non-zero json_encode options value (true), which can change how characters like
'<' are represented in the inline <script> payload. Because $vars includes user-controlled values
(e.g., post title/caption) and is consumed directly by frontend JS from window[instance], a payload
containing '</script>' can terminate the script early and break initialization and/or enable script
injection.
Code

app/Modules/DataProviders/NinjaFooTable.php[771]

+                window['<?php echo esc_attr($table_instance_name);?>'] = <?php echo wp_json_encode($vars); ?>
Evidence
The table config object is printed directly into an inline script tag (no additional escaping
layer), and it includes user-controlled fields (title, caption) plus can be modified by a filter;
frontend JS reads that global and uses it to initialize tables. If the JSON encoder outputs literal
'<' sequences for those values (e.g., in a '</script>' substring), the HTML parser will end the
script tag before the assignment completes, breaking the global config and the subsequent table
initialization path.

app/Modules/DataProviders/NinjaFooTable.php[500-516]
app/Modules/DataProviders/NinjaFooTable.php[766-775]
resources/public/js/_tableApp.js[48-59]
boot/ninja-tables-global-function.php[1137-1140]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`addInlineVars()` prints a JSON object directly into an inline `&lt;script&gt;` assignment. The PR switched `json_encode($vars, true)` to `wp_json_encode($vars)` but dropped the previously supplied non-zero options value, which can change how characters (notably `&lt;` as part of `&lt;/script&gt;`) are emitted, risking early script termination and breaking table initialization.

### Issue Context
- `$vars` includes user-controlled data (e.g., `$table-&gt;post_title`, caption meta) and is also mutable via the `ninja_table_rendering_table_vars` filter.
- Frontend JS reads `window[table_instance_name]` and initializes the table from it.

### Fix Focus Areas
- app/Modules/DataProviders/NinjaFooTable.php[766-775]
- app/Modules/DataProviders/NinjaFooTable.php[500-516]
- resources/public/js/_tableApp.js[48-59]

### Implementation sketch
- Change `wp_json_encode($vars)` to `wp_json_encode($vars, JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT)` (or at minimum ensure the option that prevents literal `&lt;` in output is enabled), and consider adding a trailing `;` after the assignment.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@kilo-code-bot
Copy link

kilo-code-bot bot commented Mar 10, 2026

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Overview

This is a security-focused PR that replaces PHP's native json_encode() with WordPress's wp_json_encode() for inline JSON output in JavaScript <script> tags.

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 0

Security Analysis

The changes are security improvements using WordPress best practices:

  • wp_json_encode() properly handles special characters, unicode escaping, and ensures safe encoding for script tag contexts
  • This prevents potential XSS vulnerabilities when JSON data is embedded inline

Changes Verified

File Line Change
app/Hooks/Handlers/EditorBlockHandler.php 54 json_encode($tables)wp_json_encode($tables)
app/Modules/DataProviders/NinjaFooTable.php 527 json_encode($cartItems)wp_json_encode($cartItems)
app/Modules/DataProviders/NinjaFooTable.php 771 json_encode($vars, true)wp_json_encode($vars)

Notes

  • The phpcs ignore comment on line 54 is appropriate since $tables is documented as already escaped before being passed in
  • The removal of the true parameter in line 771 is correct (that parameter made json_encode return a PHP array, but we need a JSON string for JavaScript)
Files Reviewed (2 files)
  • app/Hooks/Handlers/EditorBlockHandler.php - No issues
  • app/Modules/DataProviders/NinjaFooTable.php - No issues

This PR is ready to merge. The changes follow WordPress security best practices for hardening inline JSON output.

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