Skip to content

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

Closed
prix-cloud[bot] wants to merge 1 commit intomainfrom
task-33764465
Closed

fix(security): add safeJoin() to block path traversal in fs ops#19
prix-cloud[bot] wants to merge 1 commit intomainfrom
task-33764465

Conversation

@prix-cloud
Copy link
Copy Markdown
Contributor

@prix-cloud prix-cloud Bot commented Apr 25, 2026

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 removefs.rmSync on plugin root
src/lib/subagents.ts installSubagentCentrally, removeSubagent, installSubagentToAgent, removeSubagentFromAgentfs.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, removePermissionSetfs.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

# 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.

@muqsitnawaz
Copy link
Copy Markdown
Owner

Duplicate of #21

@muqsitnawaz muqsitnawaz marked this as a duplicate of #21 Apr 26, 2026
@muqsitnawaz muqsitnawaz deleted the task-33764465 branch April 28, 2026 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant