fix(security): add safeJoin() to block path traversal in fs ops

.rush/pr.md

@@ -0,0 +1,44 @@
+fix(security): add safeJoin() to block path traversal in fs ops
+
+## What changed
+
+Introduced `src/lib/paths.ts` with a `safeJoin(base, name)` helper that:
+
+1. Rejects names not matching `^[a-zA-Z0-9][a-zA-Z0-9._-]{0,63}$` (no slashes, no `..`, no leading dots).
+2. Resolves the joined path and verifies it still lives under `base` (defense-in-depth against any future bypass).
+
+Routed every user-named filesystem operation through `safeJoin`:
+
+| File | Operations protected |
+|---|---|
+| `src/commands/plugins.ts` | `plugins remove` — `fs.rmSync` on plugin root |
+| `src/lib/subagents.ts` | `installSubagentCentrally`, `removeSubagent`, `installSubagentToAgent`, `removeSubagentFromAgent` — `fs.rmSync` / `fs.cpSync` / `fs.writeFileSync` / `fs.unlinkSync` |
+| `src/lib/routines.ts` | `readJob`, `writeJob`, `deleteJob`, `getJobPath` — YAML read/write/delete under `~/.agents/routines/` |
+| `src/commands/routines.ts` | `routines edit` — template `fs.writeFileSync` on new job file |
+| `src/lib/permissions.ts` | `savePermissionSet`, `removePermissionSet` — `fs.writeFileSync` / `fs.unlinkSync` under `~/.agents/permissions/groups/` |
+
+## Why
+
+`path.join(baseDir, userInput)` without validation allows `../` traversal. For example:
+
+```
+agents plugins remove ../../Documents
+```
+
+…would call `fs.rmSync(~/Documents, { recursive: true })`, wiping an arbitrary directory. Same class of bug existed in subagent, routine, and permission commands.
+
+## How to test
+
+```bash
+# Should throw "Invalid name"
+agents plugins remove '../../Documents'
+agents subagents remove '../../../etc/passwd'
+agents routines remove '../../sensitive'
+agents permissions remove '../../../../home'
+
+# Normal names should still work
+agents plugins remove my-plugin
+agents routines remove my-job
+```
+
+Closes RUSH-555.

src/commands/plugins.ts

@@ -13,6 +13,7 @@ import chalk from 'chalk';
 import { checkbox } from '@inquirer/prompts';
 
 import { AGENTS, PLUGINS_CAPABLE_AGENTS, agentLabel } from '../lib/agents.js';
+import { safeJoin } from '../lib/paths.js';
 import type { AgentId } from '../lib/types.js';
 import { discoverPlugins, getPlugin, pluginSupportsAgent, removePluginFromVersion } from '../lib/plugins.js';
 import {
@@ -303,7 +304,7 @@ Examples:
       }
       const name = nameArg;
       const pluginsDir = path.join(process.env.HOME || '', '.agents', 'plugins');
-      const pluginRoot = path.join(pluginsDir, name);
+      const pluginRoot = safeJoin(pluginsDir, name);
 
       // Use discovered plugin when present; fall back to name+root if source is already gone
       const plugin = getPlugin(name);

src/commands/routines.ts

@@ -36,6 +36,7 @@ import {
 } from '../lib/routines.js';
 import type { JobConfig } from '../lib/routines.js';
 import { getRoutinesDir } from '../lib/state.js';
+import { safeJoin } from '../lib/paths.js';
 import { executeJob } from '../lib/runner.js';
 import { JobScheduler } from '../lib/scheduler.js';
 import { isInteractiveTerminal, requireInteractiveSelection } from './utils.js';
@@ -368,7 +369,7 @@ Examples:
       if (!jobPath) {
         // Job doesn't exist - create a new one
         const cronDir = getRoutinesDir();
-        const newPath = path.join(cronDir, `${name}.yml`);
+        const newPath = safeJoin(cronDir, `${name}.yml`);
 
         // Create template
         const template = yaml.stringify({

src/lib/paths.ts

@@ -0,0 +1,14 @@
+import path from 'path';
+
+const NAME_PATTERN = /^[a-zA-Z0-9][a-zA-Z0-9._-]{0,63}$/;
+
+/**
+ * Join base + name, rejecting names that escape the base directory or contain
+ * path traversal sequences. Throws on invalid input.
+ */
+export function safeJoin(base: string, name: string): string {
+  if (!NAME_PATTERN.test(name)) throw new Error(`Invalid name: ${name}`);
+  const resolved = path.resolve(base, name);
+  if (!resolved.startsWith(path.resolve(base) + path.sep)) throw new Error(`Path escape: ${name}`);
+  return resolved;
+}

src/lib/permissions.ts

@@ -21,6 +21,7 @@ import type {
   CodexPermissions,
 } from './types.js';
 import { getPermissionsDir, ensureAgentsDir } from './state.js';
+import { safeJoin } from './paths.js';
 import { AGENTS } from './agents.js';
 
 const HOME = os.homedir();
@@ -384,7 +385,7 @@ export function removePermissionSet(name: string): { success: boolean; error?: s
   const dir = getPermissionsDir();
 
   for (const ext of ['.yml', '.yaml']) {
-    const filePath = path.join(dir, name + ext);
+    const filePath = safeJoin(dir, name + ext);
     if (fs.existsSync(filePath)) {
       try {
         fs.unlinkSync(filePath);
@@ -1186,7 +1187,7 @@ export function exportPermissionsFromPath(filePath: string): PermissionSet | nul
  */
 export function savePermissionSet(set: PermissionSet): { success: boolean; error?: string } {
   ensurePermissionsDir();
-  const filePath = path.join(getPermissionsDir(), set.name + '.yml');
+  const filePath = safeJoin(getPermissionsDir(), set.name + '.yml');
 
   try {
     const content = yaml.stringify({

src/lib/routines.ts

@@ -12,6 +12,7 @@ import * as path from 'path';
 import * as yaml from 'yaml';
 import { Cron } from 'croner';
 import { getRoutinesDir, getRunsDir, ensureAgentsDir } from './state.js';
+import { safeJoin } from './paths.js';
 import type { AgentId } from './types.js';
 import { ALL_AGENT_IDS } from './agents.js';
 
@@ -81,7 +82,7 @@ export function readJob(name: string): JobConfig | null {
   ensureAgentsDir();
   const jobsDir = getRoutinesDir();
   for (const ext of ['.yml', '.yaml']) {
-    const filePath = path.join(jobsDir, name + ext);
+    const filePath = safeJoin(jobsDir, name + ext);
     if (fs.existsSync(filePath)) {
       return readJobFile(filePath);
     }
@@ -109,7 +110,7 @@ function readJobFile(filePath: string): JobConfig | null {
 export function writeJob(config: JobConfig): void {
   ensureAgentsDir();
   const jobsDir = getRoutinesDir();
-  const filePath = path.join(jobsDir, config.name + '.yml');
+  const filePath = safeJoin(jobsDir, config.name + '.yml');
 
   const output: Record<string, unknown> = { ...config };
   if (output.mode === 'plan') delete output.mode;
@@ -125,7 +126,7 @@ export function writeJob(config: JobConfig): void {
 export function deleteJob(name: string): boolean {
   const jobsDir = getRoutinesDir();
   for (const ext of ['.yml', '.yaml']) {
-    const filePath = path.join(jobsDir, name + ext);
+    const filePath = safeJoin(jobsDir, name + ext);
     if (fs.existsSync(filePath)) {
       fs.unlinkSync(filePath);
       return true;
@@ -308,7 +309,7 @@ export function jobExists(name: string): boolean {
 export function getJobPath(name: string): string | null {
   const jobsDir = getRoutinesDir();
   for (const ext of ['.yml', '.yaml']) {
-    const filePath = path.join(jobsDir, name + ext);
+    const filePath = safeJoin(jobsDir, name + ext);
     if (fs.existsSync(filePath)) {
       return filePath;
     }

src/lib/subagents.ts

@@ -11,6 +11,7 @@ import * as fs from 'fs';
 import * as path from 'path';
 import * as yaml from 'yaml';
 import { getSubagentsDir } from './state.js';
+import { safeJoin } from './paths.js';
 import type { AgentId, DiscoveredSubagent, InstalledSubagent, SubagentFrontmatter } from './types.js';
 
 /**
@@ -190,7 +191,7 @@ export function installSubagentCentrally(
   name: string
 ): { success: boolean; error?: string } {
   const subagentsDir = getSubagentsDir();
-  const targetDir = path.join(subagentsDir, name);
+  const targetDir = safeJoin(subagentsDir, name);
 
   try {
     // Ensure subagents directory exists
@@ -217,7 +218,7 @@ export function installSubagentCentrally(
  */
 export function removeSubagent(name: string): { success: boolean; error?: string } {
   const subagentsDir = getSubagentsDir();
-  const targetDir = path.join(subagentsDir, name);
+  const targetDir = safeJoin(subagentsDir, name);
 
   if (!fs.existsSync(targetDir)) {
     return { success: false, error: `Subagent '${name}' not found` };
@@ -320,14 +321,14 @@ export function installSubagentToAgent(
 
     try {
       const transformed = transformSubagentForClaude(subagentDir);
-      fs.writeFileSync(path.join(agentsDir, `${subagentName}.md`), transformed);
+      fs.writeFileSync(safeJoin(agentsDir, `${subagentName}.md`), transformed);
       return { success: true };
     } catch (err) {
       return { success: false, error: String(err) };
     }
   } else if (agent === 'openclaw') {
     // OpenClaw: copy full directory
-    const targetDir = path.join(agentHome, '.openclaw', subagentName);
+    const targetDir = safeJoin(path.join(agentHome, '.openclaw'), subagentName);
     return syncSubagentToOpenclaw(subagentDir, targetDir);
   } else {
     // Other agents don't support subagents yet
@@ -345,13 +346,13 @@ export function removeSubagentFromAgent(
 ): { success: boolean; error?: string } {
   try {
     if (agent === 'claude') {
-      const targetPath = path.join(agentHome, '.claude', 'agents', `${subagentName}.md`);
+      const targetPath = safeJoin(path.join(agentHome, '.claude', 'agents'), `${subagentName}.md`);
       if (fs.existsSync(targetPath)) {
         fs.unlinkSync(targetPath);
       }
       return { success: true };
     } else if (agent === 'openclaw') {
-      const targetDir = path.join(agentHome, '.openclaw', subagentName);
+      const targetDir = safeJoin(path.join(agentHome, '.openclaw'), subagentName);
       if (fs.existsSync(targetDir)) {
         fs.rmSync(targetDir, { recursive: true });
       }