Skip to content

feat: Remove paths from plugin packages#7580

Merged
JohnMcLear merged 1 commit intodevelopfrom
sanitize_plugin_packages
Apr 22, 2026
Merged

feat: Remove paths from plugin packages#7580
JohnMcLear merged 1 commit intodevelopfrom
sanitize_plugin_packages

Conversation

@Gared
Copy link
Copy Markdown
Member

@Gared Gared commented Apr 21, 2026

This will remove realPath and path from plugin package before sending it to the client

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

Review Summary by Qodo

Sanitize plugin packages by removing path information

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Sanitize plugin packages before sending to client
• Remove sensitive path information from plugin data
• Keep only name and version in plugin package metadata
Diagram
flowchart LR
  A["plugins.plugins object"] -- "iterate and extract" --> B["package data"]
  B -- "filter to name, version" --> C["pluginsSanitized"]
  C -- "send to client" --> D["ClientReady message"]
Loading

Grey Divider

File Changes

1. src/node/handler/PadMessageHandler.ts ✨ Enhancement +6/-1

Sanitize plugin packages in ClientReady handler

• Create pluginsSanitized object by iterating through plugin packages
• Extract only name and version fields from each plugin package
• Replace full plugins.plugins with sanitized version in ClientReady message
• Prevents sensitive path information from being exposed to clients

src/node/handler/PadMessageHandler.ts


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented Apr 21, 2026

Code Review by Qodo

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

Grey Divider


Action required

1. pluginsSanitized mutates plugins.plugins 📘 Rule violation ☼ Reliability
Description
The new sanitization logic assigns pluginsSanitized to plugins.plugins and then overwrites each
plugin's package, permanently removing fields like realPath/path from the shared plugin
registry. This creates a breaking behavior change because other server code expects
plugin.package.realPath to exist (e.g., to serve plugin static assets).
Code

src/node/handler/PadMessageHandler.ts[R1071-1075]

+    let pluginsSanitized: any = plugins.plugins
+    Object.keys(plugins.plugins).forEach(function(element) {
+      const p: any = plugins.plugins[element].package
+      pluginsSanitized[element].package = {name: p.name, version: p.version};
+    });
Evidence
In handleClientReady, pluginsSanitized is an alias of the global plugins.plugins object and
the code overwrites package with {name, version}, removing realPath/path from the shared
registry. Other server code later reads plugin.package.realPath to resolve plugin static file
paths, so this change can break runtime behavior and violates the requirement to avoid unnecessary
breaking changes.

src/node/handler/PadMessageHandler.ts[1071-1075]
src/node/utils/Minify.ts[173-181]
Best Practice: Repository guidelines

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

## Issue description
`pluginsSanitized` is currently just a reference to `plugins.plugins`, so overwriting `pluginsSanitized[element].package` mutates the global plugin registry and removes `realPath`/`path`. This can break other server code paths that rely on `plugin.package.realPath`.

## Issue Context
The intent is to sanitize what is sent to the client, not to modify the server-side plugin definitions.

## Fix Focus Areas
- src/node/handler/PadMessageHandler.ts[1071-1075]

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


Grey Divider

Qodo Logo

Comment on lines +1071 to +1075
let pluginsSanitized: any = plugins.plugins
Object.keys(plugins.plugins).forEach(function(element) {
const p: any = plugins.plugins[element].package
pluginsSanitized[element].package = {name: p.name, version: p.version};
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. pluginssanitized mutates plugins.plugins 📘 Rule violation ☼ Reliability

The new sanitization logic assigns pluginsSanitized to plugins.plugins and then overwrites each
plugin's package, permanently removing fields like realPath/path from the shared plugin
registry. This creates a breaking behavior change because other server code expects
plugin.package.realPath to exist (e.g., to serve plugin static assets).
Agent Prompt
## Issue description
`pluginsSanitized` is currently just a reference to `plugins.plugins`, so overwriting `pluginsSanitized[element].package` mutates the global plugin registry and removes `realPath`/`path`. This can break other server code paths that rely on `plugin.package.realPath`.

## Issue Context
The intent is to sanitize what is sent to the client, not to modify the server-side plugin definitions.

## Fix Focus Areas
- src/node/handler/PadMessageHandler.ts[1071-1075]

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

@SamTV12345
Copy link
Copy Markdown
Member

Is there a better way to type that? I always try to avoid any.

@JohnMcLear JohnMcLear merged commit fe6a373 into develop Apr 22, 2026
40 of 41 checks passed
@JohnMcLear JohnMcLear deleted the sanitize_plugin_packages branch April 22, 2026 09:52
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.

3 participants