Security: Skill removal passes unsanitized paths as SSH positional arguments

[ThankNIXlater]

Summary

The skill removal over SSH (src/lib/ssh/skills-remove.ts) passes five user-controlled values as positional arguments to bash -s -- via spawnSync("ssh", ...). While the Python script receiving these arguments handles them relatively safely, the values baseDir, workspaceDir, and managedSkillsDir are passed through without character validation.

Attack Vector

The POST /api/gateway/skills/remove endpoint accepts a JSON body with baseDir, workspaceDir, and managedSkillsDir. These are passed as SSH argv:

argv: [
  "bash", "-s", "--",
  params.request.skillKey,
  params.request.source,
  params.request.baseDir,       // user-controlled
  params.request.workspaceDir,  // user-controlled
  params.request.managedSkillsDir, // user-controlled
],

Since spawnSync passes these as separate argv entries (not through a shell), direct shell injection is mitigated by Node.js. However, the values reach a Python script via sys.argv and are used with pathlib.Path().expanduser().resolve(). A crafted path containing newlines or null bytes could cause unexpected behavior in the Python layer.

Additionally, skillKey is passed without the same regex validation applied in the agent-state route (isSafeAgentId).

Location

src/lib/ssh/skills-remove.ts, lines 74-85
src/app/api/gateway/skills/remove/route.ts, normalizeRemoveRequest()

Impact

• Severity: Medium - argv separation prevents classic injection, but defense-in-depth is missing
• Null bytes or newlines in paths could confuse Python pathlib
• No character validation on skillKey (unlike agent-state which validates with regex)
• source is validated against an allowlist, but the path values are not

Fix

Add path character validation matching the pattern from PR #39:

const SAFE_PATH_RE = /^[a-zA-Z0-9_.~\/-]+$/;

const validatePath = (value: string, field: string): string => {
  const trimmed = value.trim();
  if (!trimmed) throw new Error(`${field} is required.`);
  if (!SAFE_PATH_RE.test(trimmed)) {
    throw new Error(`${field} contains invalid characters.`);
  }
  return trimmed;
};

Apply to baseDir, workspaceDir, managedSkillsDir, and skillKey in normalizeRemoveRequest().