Skip to content

Fix path traversal in profile operations #55

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AAtomical wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix path traversal in profile operations #55

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions server/profile-manager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@ const HOME_DIR = os.homedir();
const OPENCODE_DIR = path.join(HOME_DIR, '.config', 'opencode');
const PROFILES_DIR = path.join(HOME_DIR, '.config', 'opencode-profiles');

function safeName(name) {
if (!name || name.includes('/') || name.includes('\\') || name.includes('..')) {
throw new Error('Invalid profile name');
}
return name;
}
Comment on lines +9 to +14

@coderabbitai coderabbitai Bot Jul 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win

Path Traversal (CWE-22): Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')

Reachability: External

Reject . and enforce path containment.

safeName('.') passes validation, but path.join(PROFILES_DIR, '.') resolves to PROFILES_DIR. Consequently, deleteProfile('.') recursively deletes the entire profiles directory, and activateProfile('.') can point OPENCODE_DIR at the profiles root. Reject . and verify the resolved target is a direct child before filesystem operations; add a regression test.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/profile-manager.js` around lines 9 - 14, Update safeName to reject "."
and enforce that the resolved profile path is a direct child of PROFILES_DIR
before deleteProfile or activateProfile performs filesystem operations; add a
regression test covering "." and path containment.

Comment on lines +9 to +14

@greptile-apps greptile-apps Bot Jul 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Reject dot profile names
safeName('.') currently passes, so deleteProfile('.') builds path.join(PROFILES_DIR, '.') and then fs.rmSync removes the entire profiles directory. This leaves the delete path with a concrete filesystem escape despite rejecting separators and ..; reject . before using the name in createProfile, deleteProfile, or activateProfile.

Artifacts

Repro: Node.js harness that invokes deleteProfile dot with an isolated HOME

  • Contains supporting evidence from the run (text/javascript; charset=utf-8).

Repro: verbose harness output showing profiles root deletion

  • Keeps the command output available without making the summary code-heavy.

View artifacts

T-Rex Ran code and verified through T-Rex

Prompt To Fix With AI 
This is a comment left during a code review.
Path: server/profile-manager.js
Line: 9-14

Comment:
**Reject dot profile names**
`safeName('.')` currently passes, so `deleteProfile('.')` builds `path.join(PROFILES_DIR, '.')` and then `fs.rmSync` removes the entire profiles directory. This leaves the delete path with a concrete filesystem escape despite rejecting separators and `..`; reject `.` before using the name in `createProfile`, `deleteProfile`, or `activateProfile`.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Codex Fix in Claude Code Fix in Cursor


if (!fs.existsSync(PROFILES_DIR)) {
fs.mkdirSync(PROFILES_DIR, { recursive: true });
}
Expand Down Expand Up @@ -51,13 +58,15 @@ function listProfiles() {
}

function createProfile(name) {
safeName(name);
const dir = path.join(PROFILES_DIR, name);
if (fs.existsSync(dir)) throw new Error('Profile already exists');
fs.mkdirSync(dir, { recursive: true });
return { success: true };
}

function deleteProfile(name) {
safeName(name);
Comment on lines 68 to +69

@coderabbitai coderabbitai Bot Jul 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Authorization Bypass (CWE-862): Missing Authorization

Reachability: External

Require authorization before profile deletion and activation.

The supplied DELETE /api/profiles/:name and POST /api/profiles/:name/activate handlers pass request-controlled names directly to these methods without an ownership or permission check. safeName prevents traversal but still lets any unauthenticated caller delete another non-active profile or switch the global OPENCODE_DIR. Enforce authentication and authorization before invoking these operations.

Also applies to: 81-82

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/profile-manager.js` around lines 68 - 69, Require authentication and
authorization for profile mutations before invoking deleteProfile or the
activation operation. Update the DELETE /api/profiles/:name and POST
/api/profiles/:name/activate handlers to validate the requester’s permission to
manage the supplied profile, while preserving safeName validation and rejecting
unauthorized requests before any deletion or OPENCODE_DIR change occurs.

const { active } = listProfiles();
if (name === active) throw new Error('Cannot delete active profile');
if (name === 'default') throw new Error('Cannot delete default profile');
Expand All @@ -70,6 +79,7 @@ function deleteProfile(name) {
}

function activateProfile(name) {
safeName(name);
const target = path.join(PROFILES_DIR, name);
if (!fs.existsSync(target)) throw new Error('Profile not found');

Expand Down