From 6b0263afaf95f2ec3badb9bbd1908e4952370428 Mon Sep 17 00:00:00 2001 From: Prix Cloud Agent Date: Sat, 25 Apr 2026 05:34:39 +0000 Subject: [PATCH] checkpoint: auto-save 2026-04-25T05:34:39Z --- .rush/pr.md | 44 ++++++++++++++++++++++++++++++++++++++++ src/commands/plugins.ts | 3 ++- src/commands/routines.ts | 3 ++- src/lib/paths.ts | 14 +++++++++++++ src/lib/permissions.ts | 5 +++-- src/lib/routines.ts | 9 ++++---- src/lib/subagents.ts | 13 ++++++------ 7 files changed, 77 insertions(+), 14 deletions(-) create mode 100644 .rush/pr.md create mode 100644 src/lib/paths.ts diff --git a/.rush/pr.md b/.rush/pr.md new file mode 100644 index 0000000..efc716e --- /dev/null +++ b/.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. diff --git a/src/commands/plugins.ts b/src/commands/plugins.ts index f6de618..960d985 100644 --- a/src/commands/plugins.ts +++ b/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); diff --git a/src/commands/routines.ts b/src/commands/routines.ts index 12004f7..23154a3 100644 --- a/src/commands/routines.ts +++ b/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({ diff --git a/src/lib/paths.ts b/src/lib/paths.ts new file mode 100644 index 0000000..4f556a3 --- /dev/null +++ b/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; +} diff --git a/src/lib/permissions.ts b/src/lib/permissions.ts index 16e71e0..b53d6e4 100644 --- a/src/lib/permissions.ts +++ b/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({ diff --git a/src/lib/routines.ts b/src/lib/routines.ts index b82a6dd..3d388c4 100644 --- a/src/lib/routines.ts +++ b/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 = { ...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; } diff --git a/src/lib/subagents.ts b/src/lib/subagents.ts index ffd63a0..8fb2ab4 100644 --- a/src/lib/subagents.ts +++ b/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 }); }