Skip to content

fix: Enforce stricter validation rules for the plugin name. [INS-5310 & INS-5408 & INS-5333] - #8639

Merged
pavkout merged 57 commits into
Kong:developfrom
pavkout:feature/ins-5310
May 21, 2025
Merged

fix: Enforce stricter validation rules for the plugin name. [INS-5310 & INS-5408 & INS-5333]#8639
pavkout merged 57 commits into
Kong:developfrom
pavkout:feature/ins-5310

Conversation

@pavkout

@pavkout pavkout commented Apr 23, 2025

Copy link
Copy Markdown
Contributor

Enforce stricter validation rules for the plugin name.

Empty state - without plugins

image

With Installed plugin list

image

Initial View

image

Error exists

image

Closes: INS-5310, INS-5310

Comment thread packages/insomnia/src/plugins/create.ts Fixed
Comment thread packages/insomnia/src/plugins/create.ts Fixed
Comment thread packages/insomnia/src/plugins/create.ts Fixed
Comment thread packages/insomnia/src/plugins/create.ts Fixed
Comment thread packages/insomnia/src/plugins/create.ts Fixed
@pavkout pavkout self-assigned this Apr 23, 2025
@pavkout
pavkout requested a review from a team April 23, 2025 15:32
Comment thread packages/insomnia-smoke-test/tests/smoke/plugins-interactions.test.ts Outdated
Comment thread packages/insomnia/src/plugins/create.ts Outdated
Comment thread packages/insomnia/src/ui/components/settings/plugins.tsx Outdated
);
} catch (err) {
// Log and rethrow any installation errors
console.error(`[plugins] Failed to install plugin ${pluginName}:`, err);

Check notice

Code scanning / Semgrep OSS

Semgrep Finding: javascript.lang.security.audit.unsafe-formatstring.unsafe-formatstring

Detected string concatenation with a non-literal variable in a util.format / console.log function. If an attacker injects a format specifier in the string, it will forge the log message. Try to use constant values for the format string.
try {
await rm(tmpDir, { recursive: true, force: true });
} catch (error) {
console.warn(`[plugins] Failed to clean tmp dir ${tmpDir}:`, error);

Check notice

Code scanning / Semgrep OSS

Semgrep Finding: javascript.lang.security.audit.unsafe-formatstring.unsafe-formatstring

Detected string concatenation with a non-literal variable in a util.format / console.log function. If an attacker injects a format specifier in the string, it will forge the log message. Try to use constant values for the format string.
const pluginDir = getSafePluginDir(pluginName);

try {
const packagePath = path.resolve(pluginDir, 'package.json');

Check warning

Code scanning / Semgrep OSS

Semgrep Finding: javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal

Detected possible user input going into a `path.join` or `path.resolve` function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.

try {
const packagePath = path.resolve(pluginDir, 'package.json');
const mainJsPath = path.resolve(pluginDir, 'main.js');

Check warning

Code scanning / Semgrep OSS

Semgrep Finding: javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal

Detected possible user input going into a `path.join` or `path.resolve` function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
try {
const modulePath = path.join(p, filename);
const packageJSONPath = path.join(modulePath, 'package.json');
const modulePath = path.resolve(p, filename);

Check warning

Code scanning / Semgrep OSS

Semgrep Finding: javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal

Detected possible user input going into a `path.join` or `path.resolve` function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
try {
const modulePath = path.join(p, filename);
const packageJSONPath = path.join(modulePath, 'package.json');
const modulePath = path.resolve(p, filename);

Check warning

Code scanning / Semgrep OSS

Semgrep Finding: javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal

Detected possible user input going into a `path.join` or `path.resolve` function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
const modulePath = path.join(p, filename);
const packageJSONPath = path.join(modulePath, 'package.json');
const modulePath = path.resolve(p, filename);
const packageJSONPath = path.resolve(modulePath, 'package.json');

Check warning

Code scanning / Semgrep OSS

Semgrep Finding: javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal

Detected possible user input going into a `path.join` or `path.resolve` function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
if (p.indexOf(modulePath) === 0) {
delete global.require.cache[p];
// Sanitize paths and check for known module patterns to prevent command injection
const safeModulePath = path.resolve(modulePath);

Check warning

Code scanning / Semgrep OSS

Semgrep Finding: javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal

Detected possible user input going into a `path.join` or `path.resolve` function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
@pavkout
pavkout force-pushed the feature/ins-5310 branch 2 times, most recently from dd41f52 to 95a150b Compare April 30, 2025 13:53
@godfrzero

Copy link
Copy Markdown
Contributor

Let's maintain the warning-type styling for enabling elevated plugin access. This has a significant impact on plugin access and the visual styling should emphasize that it's not a checkbox to be ticked lightly.

image

Comment thread packages/insomnia/src/__tests__/install-plugin.test.ts
jackkav
jackkav previously approved these changes May 6, 2025
@pavkout

pavkout commented May 6, 2025

Copy link
Copy Markdown
Contributor Author

Let's maintain the warning-type styling for enabling elevated plugin access. This has a significant impact on plugin access and the visual styling should emphasize that it's not a checkbox to be ticked lightly.

image

@godfrzero I have changed it like this

image

@pavkout
pavkout force-pushed the feature/ins-5310 branch from 03effe1 to 8ceb72e Compare May 6, 2025 12:52
@pavkout
pavkout requested a review from jackkav May 6, 2025 13:21
@pavkout
pavkout force-pushed the feature/ins-5310 branch 2 times, most recently from 786af2c to dddded8 Compare May 9, 2025 11:24
Comment thread packages/insomnia/src/utils/plugin.ts Outdated
@pavkout
pavkout force-pushed the feature/ins-5310 branch 2 times, most recently from 3babbe8 to 3aa067a Compare May 16, 2025 20:53
@pavkout
pavkout requested review from gatzjames and godfrzero May 16, 2025 21:45
@pavkout pavkout changed the title fix: Enforce stricter validation rules for the plugin name. [INS-5310] fix: Enforce stricter validation rules for the plugin name. [INS-5310 & INS-5408 & INS-5333] May 19, 2025
pavkout added 27 commits May 20, 2025 19:20
@pavkout
pavkout force-pushed the feature/ins-5310 branch from af4b749 to 81a0ff1 Compare May 20, 2025 17:20
@pavkout
pavkout merged commit 010c7e7 into Kong:develop May 21, 2025
cwangsmv pushed a commit that referenced this pull request Jun 10, 2025
INS-5408 & INS-5333] (#8639)

* feat: Add CreatePluginModal component for plugin creation

* feat: Integrate CreatePluginModal for streamlined plugin creation

* feat: Enhance plugin creation with path validation and error handling

* feat: Add comprehensive validation tests for plugin name generation

* feat: Improve path validation for plugin directory to prevent traversal attacks

* feat: Add checks to prevent overwriting existing plugin files during creation

* feat: Add plugin name validation and sanitization to prevent path traversal

* fix: Correct import order in create-plugin-modal component

* fix: Correct import order in plugins interactions test

* feat: Enhance plugin name validation to prevent invalid formats and add test ID for button

* test: Refactor plugin interactions tests for improved readability and maintainability

* fix: Improve code readability by adding comments and removing unnecessary whitespace in create plugin directory function

* fix: Correct import order in plugins interactions test

* Imports

* refactor: Improve import order and enhance code readability in CreatePluginModal component

* test: Update plugin interactions tests to use soft assertions for improved error handling

* fix: Remove version parameter from createPlugin function call in Root component

* refactor: Enhance plugin name validation and remove version parameter from createPlugin function

* refactor: Remove plugin name validation logic from CreatePluginModal component

* refactor: Simplify destructuring and improve button class order in Plugins component

* test: Update plugin name validation messages and add additional rejection cases

* refactor: Enhance plugin name validation to prevent OS command injection and disallow path traversal

* refactor: Simplify error handling in CreatePluginModal component

* test: Add validation for plugin name restrictions including path traversal and length

* refactor: Rename moduleName to pluginName in createPlugin function for clarity

* refactor: Add path traversal validation in getSafePluginDir function

* test: Add comprehensive validation for plugin name restrictions in smoke tests

* feat: Implement comprehensive validation for plugin names and directory safety

* fix: Adjust modal content height to auto for better responsiveness

* Modify the plugins tab

* feat: Enhance CreatePluginModal with accessibility attributes for plugin name input

* refactor: Rename _traversePluginPath to traversePluginPath and enhance path resolution for security

* refactor: Simplify plugin directory validation by extracting getSafePluginDir and improving path handling

* Modify the install plugin functionality

* Improvements

* feat: Export containsOnlyDeprecationWarnings function for broader usage

* refactor: Remove isDeprecatedDependencies tests to streamline install.js test suite

* feat: Export utility functions for broader access in the plugin system

* fix: Validate plugin metadata by checking for missing tarball URL

* Improve plugin system by enhancing metadata validation and exporting additional utility functions

* fix: Set modal content height to 100% for consistent display

* refactor: Rename isInsomniaPlugin to getPluginInfo and streamline plugin installation logic

* test: Add comprehensive tests for plugin installation and utility functions

* delete: Remove obsolete test file for plugin installation

* refactor: Update elevated access warning display and improve layout in plugin settings

* feat: Add support for scoped package names in plugin installation functions

* refactor: Improve layout and error handling in plugin settings component

* feat: Add validation for plugin name input during installation

* feat: Enhance plugin name validation to support scoped package names

* test: Update error messages for invalid scoped package names in plugin interactions tests

* feat: Pass allowScopedPackageNames parameter to getPluginInfo for improved plugin validation

* feat: Add support for scoped package names in plugin validation regex

* feat: Update installPlugin to accept allowScopedPackageNames parameter and rename webUtils import

* feat: Update installPlugin handler to accept allowScopedPackageNames parameter

* refactor: Simplify mock implementations in install-plugin tests by removing unnecessary properties

* fix: Correct plugin name length validation to enforce a maximum of 214 characters

* refactor: Simplify state management in Plugins component and update plugin reload logic
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.

5 participants