fix(security): validate path characters in skill removal request (fixes #53)

[ThankNIXlater]

Adds regex validation for baseDir, workspaceDir, and managedSkillsDir in the skill removal endpoint. These values are passed as SSH positional arguments and should only contain safe filesystem characters.

Uses the same safe-path pattern established in PR #39 for trashDir validation, ensuring consistency across all SSH-facing inputs.

Fixes #53.

---

Note

Medium Risk
Tightens validation on SSH-bound path parameters, which is low complexity but can cause previously accepted (e.g., space-containing) paths to start failing with 400s in the skill removal API.

Overview
Adds input validation hardening to the POST /api/gateway/skills/remove endpoint by enforcing a safe-character regex for baseDir, workspaceDir, and managedSkillsDir before invoking local or SSH-based removal.

Updates error classification so path validation failures (contains invalid characters) return 400 instead of 500.

^{Written by Cursor Bugbot for commit 59ec7eb. This will update automatically on new commits. Configure here.}

[iamlukethedev]

This closes the SSH argument-injection hole, but I found two regressions that should be addressed before merge:

1. The new "contains invalid characters" validation path now returns 500 instead of 400 because the route's error-to-status mapping was not updated.
2. The new path regex is POSIX-only and rejects valid Windows SSH paths like or , even though the project documents Windows SSH targets.

I've left inline comments on the relevant lines.

[iamlukethedev]

This introduces a new validation failure mode ("contains invalid characters"), but the catch block below does not classify that message as client input. As written, malformed paths will now return 500 instead of 400. Please either include this message in the 400 mapping or switch to a structured validation error check.

[iamlukethedev]

This regex only allows POSIX-style path characters. It will reject valid Windows SSH paths such as C:/Users/... or C:\Users\..., which looks incompatible with the repos documented Windows SSH support. If the goal is to protect the SSH argv boundary, consider validating only truly dangerous characters here and leaving platform-specific path semantics to the existing containment checks.

[iamlukethedev]

Thanks for addressing the two regressions I called out earlier. The 400 mapping for contains invalid characters is fixed, and allowing : / \\ addresses the Windows-path issue.

I still don't think this PR should be merged as-is, because there is one remaining regression:

The new path regex still rejects valid filesystem paths containing spaces. That means legitimate paths such as /Users/name/My Skills/... or C:\\Users\\Alice Smith\\... will now fail at the API boundary even though the downstream local/SSH removers already do the real safety check by resolving the path and verifying it stays under the allowed root.

Please either relax/remove this route-level character filter so valid paths are not rejected, or move the protection to the SSH boundary itself. This should also get route tests covering:

• invalid-character input returns 400,
• Windows-style paths are accepted,
• valid paths with spaces are accepted.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

[cursor]

skillKey SSH argument not validated for unsafe characters

High Severity

skillKey is passed as an SSH positional argument in the same argv array as baseDir, workspaceDir, and managedSkillsDir, but it is only validated with normalizeRequired (non-empty check), not with SAFE_PATH_RE or equivalent character validation. Since SSH concatenates remote command arguments into a shell-interpreted string, a skillKey containing shell metacharacters could break the argument boundary on the remote side — the same class of injection this PR aims to prevent.

 

[iamlukethedev]

Findings
Medium: the path regex still rejects valid paths with spaces, e.g. /Users/name/My Skills/... and C:\Users\Alice Smith.... The regex only allows letters, digits, _, ., ~, /, :, , and -. 8b2134fe-1bb0-4981-b537-a8ee1002c223.txt:76-82
Process blocker: GitHub still shows reviewDecision: CHANGES_REQUESTED, despite the PR being mergeable. 8b2134fe-1bb0-4981-b537-a8ee1002c223.txt:1
Coverage gap: requested route tests for invalid characters, Windows paths, and paths with spaces were not added. The PR still only changes src/app/api/gateway/skills/remove/route.ts. 8b2134fe-1bb0-4981-b537-a8ee1002c223.txt:1