From 829521396faeba61444cdc0191147509c0f410dc Mon Sep 17 00:00:00 2001
From: Nikhil Sonti <nikhilsv92@gmail.com>
Date: Thu, 12 Mar 2026 14:06:23 -0700
Subject: [PATCH] fix: skills CRUD broken on Windows due to hardcoded path
 separator
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

safeSkillDir() used a hardcoded `/` in the startsWith path traversal
check. On Windows, path.resolve() returns backslash paths, so the check
always failed — blocking getSkill, createSkill, updateSkill, deleteSkill.

Replace `${skillsDir}/` with `${skillsDir}${sep}` using path.sep from
node:path, which returns `\` on Windows and `/` on POSIX.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 apps/server/src/skills/service.ts        |  4 ++--
 apps/server/tests/skills/service.test.ts | 27 ++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 2 deletions(-)
 create mode 100644 apps/server/tests/skills/service.test.ts

diff --git a/apps/server/src/skills/service.ts b/apps/server/src/skills/service.ts
index 0b5c0782..493e82a7 100644
--- a/apps/server/src/skills/service.ts
+++ b/apps/server/src/skills/service.ts
@@ -1,5 +1,5 @@
 import { mkdir, readFile, rm, stat, writeFile } from 'node:fs/promises'
-import { join, resolve } from 'node:path'
+import { join, resolve, sep } from 'node:path'
 import matter from 'gray-matter'
 import { getSkillsDir } from '../lib/browseros-dir'
 import { logger } from '../lib/logger'
@@ -23,7 +23,7 @@ export function slugify(name: string): string {
 function safeSkillDir(id: string): string {
   const skillsDir = getSkillsDir()
   const resolved = resolve(skillsDir, id)
-  if (!resolved.startsWith(`${skillsDir}/`)) {
+  if (!resolved.startsWith(`${skillsDir}${sep}`)) {
     throw new Error('Invalid skill id')
   }
   return resolved
diff --git a/apps/server/tests/skills/service.test.ts b/apps/server/tests/skills/service.test.ts
new file mode 100644
index 00000000..11881f48
--- /dev/null
+++ b/apps/server/tests/skills/service.test.ts
@@ -0,0 +1,27 @@
+import { describe, it } from 'bun:test'
+import assert from 'node:assert'
+import { sep } from 'node:path'
+
+import { createSkillsRoutes } from '../../src/api/routes/skills'
+
+describe('skills routes', () => {
+  const app = createSkillsRoutes()
+
+  it('GET /:id returns 404 for non-existent skill (not 500 from path check)', async () => {
+    const res = await app.request('/valid-skill-id')
+    assert.strictEqual(res.status, 404)
+    const body = await res.json()
+    assert.strictEqual(body.error, 'Skill not found')
+  })
+
+  it('GET /:id rejects path traversal attempts', async () => {
+    const res = await app.request('/../../../etc/passwd')
+    assert.notStrictEqual(res.status, 200)
+  })
+})
+
+describe('safeSkillDir uses platform separator', () => {
+  it(`path.sep is "${sep}" on this platform`, () => {
+    assert.ok(sep === '/' || sep === '\\')
+  })
+})