fix: Validate inputs to prevent command injection

[hustcer]

fix: Validate inputs to prevent command injection

Summary by CodeRabbit

• Bug Fixes

  • Improved error handling to gracefully manage unexpected error types.

• Security

  • Added input validation for plugin registration to prevent injection vulnerabilities and enforce safe configuration values.

• Chores

  • Updated project configuration to ignore internal build artifacts.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Input validation for plugin registration is added via two new validators (validatePluginInput and validateVersion) in plugin-related files. Error handling in the main index file is improved to gracefully convert non-Error throwables to strings. The .claude/ directory is added to .gitignore.

Changes

Cohort / File(s)	Summary
Configuration & Build .gitignore	Added .claude/ entry to ignore list.
Error Handling src/index.ts	Modified catch block to handle non-Error throwables by converting them to strings using err instanceof Error ? err.message : String(err).
Input Validation (Plugin Handlers) src/plugins-tpl.ts, src/plugins.ts	Added two new internal validators: validatePluginInput() (enforces enablePlugins as true/false or comma-separated alphanumeric/underscore identifiers) and validateVersion() (enforces alphanumeric/dot/hyphen pattern). Both validators are invoked at the start of registerPlugins before legacy version computation and command execution.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Pay close attention to the regex patterns in validatePluginInput and validateVersion to ensure they cannot be bypassed or create unintended rejections of valid inputs.
• Verify that error handling in src/index.ts correctly captures and converts all non-Error throwable types without masking critical debugging information.
• Confirm that early validation in registerPlugins does not introduce regression for edge cases (e.g., empty strings, whitespace-only inputs).

Possibly related PRs

• feat: add strict input validation and robust plugin registration #189: Modifies the same registerPlugins function in src/plugins.ts with command execution refactoring using execOrThrow and unified command construction—directly related to the same plugin registration logic.

Poem

🐰 Validators hop with glee,
Sanitizing inputs, wild and free!
No injection past our guards so keen,
The safest plugins ever seen!
With .claude/ tucked away,
Our codebase shines another day! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main change: adding input validation to prevent command injection vulnerabilities across multiple files.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch develop

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

Code Analysis

• Core Requirements: The changes focus on backend/Node.js security validation rather than frontend React/Vue components. The code implements input validation functions to prevent command injection in a plugin registration system.
• ES Specification Compliance: Uses modern JavaScript features (arrow functions, template literals, regex patterns) appropriately. The error handling improvement in src/index.ts properly handles non-Error objects.
• Component Design Patterns: Not applicable as this is backend Node.js code.
• State Management: Not applicable.
• Accessibility: Not applicable.

Security Review

• ✅ Input Validation Added: The validatePluginInput() and validateVersion() functions provide strong input validation using regex patterns to prevent command injection attacks.
• ✅ Improved Error Handling: The fix in src/index.ts properly handles cases where err might not be an Error object, preventing potential crashes.
• ✅ Input Sanitization: The regex patterns (/^(true|false|[\w]+(,[\w]+)*)$/i and /^[\w.-]+$/) effectively restrict allowed characters to prevent injection attacks.
• ⚠️ Consider Additional Security: While the input validation is good, consider adding output encoding when these values are used in shell commands or file operations.

Optimization Suggestions

• Performance: The validation functions are lightweight and efficient, using regex patterns that should have minimal performance impact.
• Code Duplication: The same validation functions appear in both src/plugins-tpl.ts and src/plugins.ts. Consider extracting these to a shared utility module to follow DRY principles.
• Bundle Size: Not applicable for backend code, but the duplicated code could be optimized by sharing the validation logic.

Overall Quality: 4

Rationale: The changes demonstrate good security practices with proper input validation to prevent command injection. The error handling improvement is also valuable. The main area for improvement is code duplication between the two plugin files. The security implementation is solid but could be enhanced with additional defense-in-depth measures like output encoding.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/plugins-tpl.ts (1)

22-28: Address documentation inconsistency and consider tightening the version regex.

The implementation has two concerns:

1. Documentation mismatch: The comment (line 20) and error message (line 25) state "alphanumeric characters, dots, and hyphens" but the regex [\w.-] also allows underscores (since \w = [a-zA-Z0-9_]).

2. Overly permissive pattern: The regex /^[\w.-]+$/ accepts edge cases that may not be valid versions:

   • Leading/trailing special characters: .0.95.0, 0.95.0-, _version
   • Consecutive special characters: 0..95, version--1
   • Only special characters: ..., ---

Consider this more restrictive pattern that requires starting with alphanumeric and prevents consecutive special characters:

 function validateVersion(version: string): void {
-  if (!/^[\w.-]+$/.test(version)) {
+  // Start with alphanumeric, then allow alphanumeric/dots/hyphens, end with alphanumeric
+  if (!/^[a-zA-Z0-9]+([.-][a-zA-Z0-9]+)*$/.test(version)) {
     throw new Error(
-      `Invalid version format: "${version}". Only alphanumeric characters, dots, and hyphens are allowed.`
+      `Invalid version format: "${version}". Must start and end with alphanumeric characters; dots and hyphens allowed as separators.`
     );
   }
 }

This pattern ensures versions like 0.95.0 and nightly-56ed69a are valid while rejecting problematic edge cases.

Alternatively, if the current permissive validation is intentional, update the documentation to explicitly mention underscores are allowed.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0539b92 and 923ed07.

⛔ Files ignored due to path filters (1)

• dist/index.js is excluded by !**/dist/**

📒 Files selected for processing (4)

• .gitignore (1 hunks)
• src/index.ts (1 hunks)
• src/plugins-tpl.ts (2 hunks)
• src/plugins.ts (2 hunks)

🔇 Additional comments (5)

.gitignore (1)

4-4: LGTM!

Adding .claude/ to .gitignore is appropriate for excluding tool-specific directories from version control.

src/index.ts (1)

52-52: LGTM!

The enhanced error handling correctly checks if the thrown value is an Error instance before accessing its message property. This prevents potential runtime issues when non-Error values are thrown.

src/plugins.ts (1)

5-28: Note: This file appears to be generated from plugins-tpl.ts.

Since src/plugins.ts is listed in .gitignore (line 3), it appears to be a generated file. The detailed review of the validation logic is provided on the source template file src/plugins-tpl.ts.

Also applies to: 101-103

src/plugins-tpl.ts (2)

9-16: LGTM! Solid input validation for plugin names.

The regex pattern correctly validates that enablePlugins is either "true", "false", or a comma-separated list of valid identifiers. The pattern properly rejects edge cases like empty strings, leading/trailing commas, and consecutive commas.

---

41-43: LGTM! Validation is correctly placed early in the flow.

The validators are called immediately after the early-return check and before any command construction or file operations. This ensures inputs are sanitized before they can be used in potentially dangerous operations.