Skip to content

fix: enforce per-chart authorization in AI Builder endpoints - #1353

Open
lucadobrescu wants to merge 2 commits into
developmentfrom
fix/aibuilder-chart-auth
Open

fix: enforce per-chart authorization in AI Builder endpoints#1353
lucadobrescu wants to merge 2 commits into
developmentfrom
fix/aibuilder-chart-auth

Conversation

@lucadobrescu

Copy link
Copy Markdown
Contributor

The AI Builder's chart-targeted AJAX handlers used a weaker authorization check than the rest of the plugin — edit_post without verifying the target is actually a chart — and the generation-status poller accepted any workflow ID from any logged-in user. This routes every chart-targeted handler through the shared can_edit_chart() guard and binds AI workflow IDs to the user who started them. Follow-up to the authorization hardening started in #1348.

What changed

  • Chart access check_verify_chart_access() now delegates to Visualizer_Module::can_edit_chart(). Before: current_user_can( 'edit_post', $id ) alone, so any editable post type was accepted — saveChart could retitle and force-publish arbitrary posts, and uploadData could overwrite non-chart content. After: only visualizer posts pass, and contributor-authors keep access to their own charts, matching the rule used by the chart editor endpoints.

  • Workflow status pollinggenerateChart stores the returned workflow ID in a user-scoped transient; chartStatus answers 403 for IDs not bound to the current user. Before: any user with edit_posts could poll another user's AI generation and read its full payload (prompt, series, data, generated code).

  • Upload validation — unchanged. The uploaded file is parsed in place by the CSV/XLSX source classes, which reject content without the required header/type rows, and is never written to a web-accessible path, so the extension check is dispatch logic, not a security boundary. MIME sniffing was left out because real-world CSVs often sniff as text/plain and would be rejected.

Note

saveChart keeps the plugin's existing force-publish behavior for charts (same as the classic chart editor save), gated by the same can_edit_chart() rule, so contributor-owned AI charts keep working.

AI Builder request flow

flowchart LR
    A[AI Builder AJAX request] --> B{Nonce valid?}
    B -- no --> X[403]
    B -- yes --> C{Changed:<br/>can_edit_chart<br/>chart_id?}:::changed
    C -- no --> X
    C -- yes --> D{Action}
    D -- generate --> E[Start AI workflow] --> F[New:<br/>bind workflow ID<br/>to user transient]:::added
    D -- poll status --> G{New:<br/>workflow bound<br/>to user?}:::added
    G -- no --> X
    G -- yes --> H[Return status]
    D -- fetch / upload / save --> I[Operate on chart]

    classDef added fill:#1a7f37,color:#fff,stroke:#116329,stroke-width:3px
    classDef changed fill:#9a6700,color:#fff,stroke:#5c3d00,stroke-width:3px,stroke-dasharray:6 3
Loading

Data changes

Key Value Lifetime
viz_ai_wf_<workflow_id> transient ID of the user who started the generation 6 hours

Before → after: workflow IDs were unbound → each is bound to its creator for the generation's lifetime.

QA

  1. WP Admin → Users → Add New: create a user with the Contributor role, then log in as that user in a private window.

  2. WP Admin → Visualizer → Chart Library. In DevTools console, run the following with the ID of a chart owned by admin:

fetch( vizAIBuilder.ajaxUrl, { method: 'POST', body: new URLSearchParams( { action: 'visualizer-ai-fetch', nonce: vizAIBuilder.nonce, chart_id: '<CHART_ID>' } ) } ).then( r => r.json() ).then( console.log );

Expect: { success: false, data: { message: "Unauthorized." } } with HTTP 403. Before this change, the same request returned the chart's full data payload.

  1. Repeat the console call with action: 'visualizer-ai-status' and any made-up workflow_id.

Expect: the same Unauthorized. response — before, the request was proxied to the AI service and returned its payload.

  1. Regression — as the contributor, WP Admin → Visualizer → Chart Library → Add New → AI Builder, and load data from pasted CSV. Then run the visualizer-ai-fetch call from step 2 against the draft chart ID returned by the builder.

Expect: the request succeeds — contributors keep full access to charts they created.

Co-authored-by: Cursor <cursoragent@cursor.com>
@lucadobrescu lucadobrescu self-assigned this Jul 29, 2026
@lucadobrescu
lucadobrescu marked this pull request as ready for review July 29, 2026 10:46
Co-authored-by: Cursor <cursoragent@cursor.com>
@pirate-bot

pirate-bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Plugin build for 76187de is ready 🛎️!

@Soare-Robert-Daniel Soare-Robert-Daniel added the pr-checklist-skip Allow this Pull Request to skip checklist. label Jul 29, 2026
@pirate-bot pirate-bot added the pr-checklist-complete The Pull Request checklist is complete. (automatic label) label Jul 29, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Strengthens AI Builder authorization for chart operations and workflow polling.

Changes:

  • Uses the shared per-chart authorization guard.
  • Binds workflow IDs to their initiating users.
  • Adds authorization E2E coverage.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
classes/Visualizer/Module/AIBuilder.php Enforces chart and workflow ownership checks.
tests/e2e/specs/ai-builder-auth.spec.js Tests AI Builder authorization behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +505 to +506
if ( ! empty( $workflow_id ) ) {
set_transient( 'viz_ai_wf_' . $workflow_id, get_current_user_id(), 6 * HOUR_IN_SECONDS );
if ( adminChartId ) {
await requestUtils.rest( { method: 'DELETE', path: `/wp/v2/visualizer/${ adminChartId }`, params: { force: true } } );
}
for ( const [ id, user ] of [ [ contributorId, CONTRIBUTOR ], [ editorId, EDITOR ] ] ) {

const created = await aiAjax( page, 'visualizer-ai-create', { nonce } );
expect( created.body.success ).toBe( true );
const ownChartId = created.body.data.chart_id;
*/
private function _verify_chart_access( $chart_id ): void {
if ( ! current_user_can( 'edit_post', $chart_id ) ) {
if ( ! self::can_edit_chart( $chart_id ) ) {

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

tests/e2e/specs/ai-builder-auth.spec.js:131

  • The chart created here is never deleted. afterAll deletes the contributor with reassign: 1, which transfers this auto-draft to user 1 instead of removing it, so repeated runs leak charts and can affect later chart-library specs. Track this ID and explicitly delete it during teardown before deleting the user.
		const created = await aiAjax( page, 'visualizer-ai-create', { nonce } );
		expect( created.body.success ).toBe( true );
		const ownChartId = created.body.data.chart_id;

tests/e2e/specs/ai-builder-auth.spec.js:22

  • This fresh browser context has no wordpress_test_cookie, but the form sets testcookie=1. WordPress rejects that first POST with the “cookies are blocked” login error (HTTP 200), so every test using loginAs() fails before reaching the authorization assertions. Prime /wp-login.php once (or explicitly add the test cookie) before posting credentials.
	const response = await context.request.post( '/wp-login.php', {

tests/e2e/specs/ai-builder-auth.spec.js:117

  • This test never creates a workflow owned by someone else; it only verifies that an unknown ID with no transient is rejected. It therefore still passes if generation never stores the owner or if status polling rejects owners too, leaving the new binding/positive path untested. Mock the Agents start/status responses, generate as one user, then assert that user can poll while a second user receives 403.
		const { status, body } = await aiAjax( page, 'visualizer-ai-status', { nonce, workflow_id: 'foreign-workflow-id' } );

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

Labels

pr-checklist-complete The Pull Request checklist is complete. (automatic label) pr-checklist-skip Allow this Pull Request to skip checklist.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants