From 1fadb7e7ee3b29ab435881998e8a58c72207f8c1 Mon Sep 17 00:00:00 2001 From: Richard Solomou Date: Sat, 25 Jul 2026 11:56:07 +0300 Subject: [PATCH 1/5] Allow symlinked local skill bundles Resolve trusted local skill roots before bundling while retaining repository containment checks. Generated-By: PostHog Code Task-Id: 5cf8bddc-dae4-4dc9-84fb-195b06f48068 --- .../src/services/skills/skill-bundler.ts | 14 +------------- .../src/services/skills/skills.test.ts | 18 ++++++++---------- 2 files changed, 9 insertions(+), 23 deletions(-) diff --git a/packages/workspace-server/src/services/skills/skill-bundler.ts b/packages/workspace-server/src/services/skills/skill-bundler.ts index 3ba622ee97..7bfe2301a8 100644 --- a/packages/workspace-server/src/services/skills/skill-bundler.ts +++ b/packages/workspace-server/src/services/skills/skill-bundler.ts @@ -23,19 +23,7 @@ function getSafeSkillFileName(name: string): string { } async function assertSkillRoot(skillPath: string): Promise { - const lexical = path.resolve(skillPath); - const parentReal = await fs.promises.realpath(path.dirname(lexical)); - const root = await fs.promises.realpath(lexical); - // A symlinked skill root bundles whatever it points at, so a repository could - // commit `.claude/skills/foo -> ~/.claude/skills/foo` and exfiltrate a - // directory from outside the repo into an uploaded bundle. Only the skill - // directory itself must be real; symlinked ancestors (e.g. /tmp on macOS) - // stay legal. - if (root !== path.join(parentReal, path.basename(lexical))) { - throw new Error( - "Local skill bundle root must be a real directory, not a symlink", - ); - } + const root = await fs.promises.realpath(path.resolve(skillPath)); const skillMdPath = path.join(root, "SKILL.md"); const stat = await fs.promises.stat(skillMdPath); if (!stat.isFile()) { diff --git a/packages/workspace-server/src/services/skills/skills.test.ts b/packages/workspace-server/src/services/skills/skills.test.ts index 4443d636e8..232a8c9b64 100644 --- a/packages/workspace-server/src/services/skills/skills.test.ts +++ b/packages/workspace-server/src/services/skills/skills.test.ts @@ -538,22 +538,20 @@ describe("write-path guard", () => { ).rejects.toThrow("resolves outside its repository"); }); - it("rejects a symlinked skill root outside repo roots", async () => { - // Non-repo roots have no repository anchor, so the bundler's own - // leaf-symlink check is the guard there. + it("bundles a symlinked user skill root", async () => { const target = await createSkill(root, "linked"); await mkdir(userSkillsHome.dir, { recursive: true }); const linkPath = path.join(userSkillsHome.dir, "linked"); await symlink(target, linkPath, "dir"); const service = makeService(); - await expect( - service.bundleLocalSkill({ - name: "linked", - source: "user", - path: linkPath, - }), - ).rejects.toThrow("not a symlink"); + const bundled = await service.bundleLocalSkill({ + name: "linked", + source: "user", + path: linkPath, + }); + + expect(bundled.fileName).toBe("linked.zip"); }); it.each([ From 945357fdd8332f3978f7020e9bd6e1ee95ee1d3e Mon Sep 17 00:00:00 2001 From: Richard Solomou Date: Sat, 25 Jul 2026 12:01:25 +0300 Subject: [PATCH 2/5] Restrict symlinked skills to user roots Keep repository and marketplace skill roots protected while allowing user-managed dotfile symlinks. Generated-By: PostHog Code Task-Id: 5cf8bddc-dae4-4dc9-84fb-195b06f48068 --- .../src/services/skills/skill-bundler.ts | 21 +++++++++++++--- .../src/services/skills/skills.test.ts | 24 ++++++++++++++++++- .../src/services/skills/skills.ts | 5 ++++ 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/packages/workspace-server/src/services/skills/skill-bundler.ts b/packages/workspace-server/src/services/skills/skill-bundler.ts index 7bfe2301a8..0d674c12de 100644 --- a/packages/workspace-server/src/services/skills/skill-bundler.ts +++ b/packages/workspace-server/src/services/skills/skill-bundler.ts @@ -22,8 +22,21 @@ function getSafeSkillFileName(name: string): string { return safeName.length > 0 ? safeName : "skill"; } -async function assertSkillRoot(skillPath: string): Promise { - const root = await fs.promises.realpath(path.resolve(skillPath)); +async function assertSkillRoot( + skillPath: string, + allowRootSymlink: boolean, +): Promise { + const lexical = path.resolve(skillPath); + const parentReal = await fs.promises.realpath(path.dirname(lexical)); + const root = await fs.promises.realpath(lexical); + if ( + !allowRootSymlink && + root !== path.join(parentReal, path.basename(lexical)) + ) { + throw new Error( + "Local skill bundle root must be a real directory, not a symlink", + ); + } const skillMdPath = path.join(root, "SKILL.md"); const stat = await fs.promises.stat(skillMdPath); if (!stat.isFile()) { @@ -122,12 +135,14 @@ export async function bundleLocalSkill({ name, source, skillPath, + allowRootSymlink = false, }: { name: string; source: UploadableSkillSource; skillPath: string; + allowRootSymlink?: boolean; }): Promise { - const root = await assertSkillRoot(skillPath); + const root = await assertSkillRoot(skillPath, allowRootSymlink); const acc: SkillFileAccumulator = { files: {}, totalBytes: 0 }; await collectSkillFiles(root, root, acc); const files = acc.files; diff --git a/packages/workspace-server/src/services/skills/skills.test.ts b/packages/workspace-server/src/services/skills/skills.test.ts index 232a8c9b64..6136af5f23 100644 --- a/packages/workspace-server/src/services/skills/skills.test.ts +++ b/packages/workspace-server/src/services/skills/skills.test.ts @@ -9,6 +9,7 @@ import { WatcherService } from "../watcher/service"; import { SkillsService } from "./skills"; const codexHome = vi.hoisted(() => ({ dir: "" })); +const marketplaceHome = vi.hoisted(() => ({ dir: "" })); const userSkillsHome = vi.hoisted(() => ({ dir: "" })); vi.mock("../posthog-plugin/codex-mirror", async (importOriginal) => { @@ -19,7 +20,11 @@ vi.mock("../posthog-plugin/codex-mirror", async (importOriginal) => { vi.mock("./skill-discovery", async (importOriginal) => { const actual = await importOriginal(); - return { ...actual, getUserSkillsDir: () => userSkillsHome.dir }; + return { + ...actual, + getMarketplaceInstallPaths: async () => [marketplaceHome.dir], + getUserSkillsDir: () => userSkillsHome.dir, + }; }); let root: string; @@ -54,6 +59,7 @@ beforeEach(async () => { folderPath = path.join(root, "repo"); repoSkillsDir = path.join(folderPath, ".claude", "skills"); codexHome.dir = path.join(root, "codex-skills"); + marketplaceHome.dir = path.join(root, "marketplace"); userSkillsHome.dir = path.join(root, "user-skills"); await mkdir(path.join(pluginPath, "skills"), { recursive: true }); await mkdir(repoSkillsDir, { recursive: true }); @@ -554,6 +560,22 @@ describe("write-path guard", () => { expect(bundled.fileName).toBe("linked.zip"); }); + it("rejects a symlinked marketplace skill root", async () => { + const target = await createSkill(root, "linked"); + const marketplaceSkillsDir = path.join(marketplaceHome.dir, "skills"); + await mkdir(marketplaceSkillsDir, { recursive: true }); + const linkPath = path.join(marketplaceSkillsDir, "linked"); + await symlink(target, linkPath, "dir"); + + await expect( + makeService().bundleLocalSkill({ + name: "linked", + source: "marketplace", + path: linkPath, + }), + ).rejects.toThrow("not a symlink"); + }); + it.each([ ["bundled skill", () => path.join(pluginPath, "skills", "bundled-skill")], ["arbitrary directory", () => path.join(root, "rogue")], diff --git a/packages/workspace-server/src/services/skills/skills.ts b/packages/workspace-server/src/services/skills/skills.ts index fdf67d0d65..b6473c404f 100644 --- a/packages/workspace-server/src/services/skills/skills.ts +++ b/packages/workspace-server/src/services/skills/skills.ts @@ -500,10 +500,15 @@ export class SkillsService { ): Promise { const skillDir = await this.resolveKnownSkillDir(input.path); await this.assertRepoSkillStaysInRepo(skillDir); + const parent = path.dirname(skillDir); + const allowRootSymlink = [getUserSkillsDir(), getCodexSkillsDir()].some( + (root) => path.resolve(root) === parent, + ); return bundleLocalSkill({ name: input.name, source: input.source, skillPath: skillDir, + allowRootSymlink, }); } From da39dc0b2440bceb8fca057888b76d98b20bc431 Mon Sep 17 00:00:00 2001 From: Richard Solomou Date: Sat, 25 Jul 2026 12:07:19 +0300 Subject: [PATCH 3/5] Address skill dependency review feedback Require explicit selection before dependency expansion follows a symlinked local skill. Generated-By: PostHog Code Task-Id: 5cf8bddc-dae4-4dc9-84fb-195b06f48068 --- .../src/services/skills/skills.test.ts | 36 +++++++++++++++++++ .../src/services/skills/skills.ts | 20 ++++++++--- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/packages/workspace-server/src/services/skills/skills.test.ts b/packages/workspace-server/src/services/skills/skills.test.ts index 6136af5f23..10daeb1035 100644 --- a/packages/workspace-server/src/services/skills/skills.test.ts +++ b/packages/workspace-server/src/services/skills/skills.test.ts @@ -739,6 +739,42 @@ describe("resolveSkillBundleDependencies", () => { expect(resolved.map((r) => r.path)).toEqual([primary, repoHelper]); }); + it("rejects an implicitly resolved symlinked user dependency", async () => { + const primary = await createSkill( + repoSkillsDir, + "scoped-parent", + withDeps("scoped-parent", ["helper"]), + ); + const target = await createSkill(root, "helper"); + await mkdir(userSkillsHome.dir, { recursive: true }); + await symlink(target, path.join(userSkillsHome.dir, "helper"), "dir"); + + await expect( + makeService().resolveSkillBundleDependencies([ + ref("scoped-parent", primary), + ]), + ).rejects.toThrow("Select helper explicitly"); + }); + + it("allows an explicitly selected symlinked user dependency", async () => { + const primary = await createSkill( + repoSkillsDir, + "scoped-parent", + withDeps("scoped-parent", ["helper"]), + ); + const target = await createSkill(root, "helper"); + await mkdir(userSkillsHome.dir, { recursive: true }); + const helper = path.join(userSkillsHome.dir, "helper"); + await symlink(target, helper, "dir"); + + const resolved = await makeService().resolveSkillBundleDependencies([ + ref("scoped-parent", primary), + { name: "helper", source: "user", path: helper }, + ]); + + expect(resolved.map((skill) => skill.path)).toEqual([primary, helper]); + }); + it("expands a tagged skill to include its transitive dependencies", async () => { const primary = await createSkill( repoSkillsDir, diff --git a/packages/workspace-server/src/services/skills/skills.ts b/packages/workspace-server/src/services/skills/skills.ts index b6473c404f..141e2ab6b4 100644 --- a/packages/workspace-server/src/services/skills/skills.ts +++ b/packages/workspace-server/src/services/skills/skills.ts @@ -579,6 +579,9 @@ export class SkillsService { ); const seen = new Set(); + const explicitRefs = new Set( + refs.map((ref) => `${ref.source}:${ref.path}`), + ); const resolved: SkillBundleRef[] = []; const queue: SkillBundleRef[] = [...refs]; // Sanity ceiling on the dependency closure. The `seen` set already @@ -622,10 +625,19 @@ export class SkillsService { for (const dependencyName of dependencyNames) { const dependencyRef = findUploadableByName(dependencyName, ref); - if ( - dependencyRef && - !seen.has(`${dependencyRef.source}:${dependencyRef.path}`) - ) { + const dependencyKey = dependencyRef + ? `${dependencyRef.source}:${dependencyRef.path}` + : null; + if (dependencyRef && dependencyKey && !seen.has(dependencyKey)) { + const isImplicitSymlink = + !explicitRefs.has(dependencyKey) && + (await fs.promises.lstat(dependencyRef.path)).isSymbolicLink(); + if (isImplicitSymlink) { + throw new Error( + `The ${ref.name} skill references /${dependencyRef.name}, which is a symlinked local skill. ` + + `Select ${dependencyRef.name} explicitly to include it in this cloud run.`, + ); + } queue.push(dependencyRef); } } From 5be97fbd0a74fcd773368fff28696021ab569f44 Mon Sep 17 00:00:00 2001 From: Richard Solomou Date: Mon, 27 Jul 2026 13:03:30 +0300 Subject: [PATCH 4/5] Address skill path ownership review Generated-By: PostHog Code Task-Id: 5cf8bddc-dae4-4dc9-84fb-195b06f48068 --- .../src/services/skills/skills.test.ts | 32 +++++++++++++++++ .../src/services/skills/skills.ts | 36 ++++++++++++------- 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/packages/workspace-server/src/services/skills/skills.test.ts b/packages/workspace-server/src/services/skills/skills.test.ts index 10daeb1035..130b62d80a 100644 --- a/packages/workspace-server/src/services/skills/skills.test.ts +++ b/packages/workspace-server/src/services/skills/skills.test.ts @@ -560,6 +560,38 @@ describe("write-path guard", () => { expect(bundled.fileName).toBe("linked.zip"); }); + it("bundles a user skill through a symlinked user skills directory", async () => { + const realUserSkillsDir = path.join(root, "real-user-skills"); + const target = await createSkill(root, "linked"); + await mkdir(realUserSkillsDir, { recursive: true }); + await symlink(realUserSkillsDir, userSkillsHome.dir, "dir"); + const linkPath = path.join(userSkillsHome.dir, "linked"); + await symlink(target, linkPath, "dir"); + + const bundled = await makeService().bundleLocalSkill({ + name: "linked", + source: "user", + path: linkPath, + }); + + expect(bundled.fileName).toBe("linked.zip"); + }); + + it("rejects an open workspace root reached through a user skill symlink", async () => { + await writeFile(path.join(folderPath, "SKILL.md"), "workspace"); + await mkdir(userSkillsHome.dir, { recursive: true }); + const linkPath = path.join(userSkillsHome.dir, "workspace"); + await symlink(folderPath, linkPath, "dir"); + + await expect( + makeService().bundleLocalSkill({ + name: "workspace", + source: "user", + path: linkPath, + }), + ).rejects.toThrow("resolves outside its repository"); + }); + it("rejects a symlinked marketplace skill root", async () => { const target = await createSkill(root, "linked"); const marketplaceSkillsDir = path.join(marketplaceHome.dir, "skills"); diff --git a/packages/workspace-server/src/services/skills/skills.ts b/packages/workspace-server/src/services/skills/skills.ts index 141e2ab6b4..14de1858a2 100644 --- a/packages/workspace-server/src/services/skills/skills.ts +++ b/packages/workspace-server/src/services/skills/skills.ts @@ -500,10 +500,7 @@ export class SkillsService { ): Promise { const skillDir = await this.resolveKnownSkillDir(input.path); await this.assertRepoSkillStaysInRepo(skillDir); - const parent = path.dirname(skillDir); - const allowRootSymlink = [getUserSkillsDir(), getCodexSkillsDir()].some( - (root) => path.resolve(root) === parent, - ); + const allowRootSymlink = await this.isUnderUserSkillRoot(skillDir); return bundleLocalSkill({ name: input.name, source: input.source, @@ -523,22 +520,37 @@ export class SkillsService { private async assertRepoSkillStaysInRepo(skillDir: string): Promise { const parent = path.dirname(skillDir); const folders = await this.folders.getFolders(); - const owningFolder = folders.find( - (folder) => - path.resolve(path.join(folder.path, ".claude", "skills")) === parent, + const realSkill = await fs.promises.realpath(skillDir); + const foldersWithRealPaths = await Promise.all( + folders.map(async (folder) => ({ + folder, + realPath: await fs.promises.realpath(path.resolve(folder.path)), + })), + ); + const owningFolder = foldersWithRealPaths.find( + ({ folder, realPath }) => + path.resolve(path.join(folder.path, ".claude", "skills")) === parent || + realSkill === realPath || + realSkill.startsWith(realPath + path.sep), ); if (!owningFolder) return; - const [realSkill, realFolder] = await Promise.all([ - fs.promises.realpath(skillDir), - fs.promises.realpath(path.resolve(owningFolder.path)), - ]); - if (!realSkill.startsWith(realFolder + path.sep)) { + if (!realSkill.startsWith(owningFolder.realPath + path.sep)) { throw new Error( "Access denied: repository skill resolves outside its repository", ); } } + private async isUnderUserSkillRoot(skillDir: string): Promise { + const parent = await fs.promises.realpath(path.dirname(skillDir)); + const roots = await Promise.all( + [getUserSkillsDir(), getCodexSkillsDir()].map((root) => + fs.promises.realpath(root).catch(() => path.resolve(root)), + ), + ); + return roots.includes(parent); + } + /** * Expand a set of tagged skill refs to include their transitive dependency * skills, so a skill that needs another pulls it into the same cloud run. From f8e2dc2dc41e11afceb22db35525754bf5fa7577 Mon Sep 17 00:00:00 2001 From: Richard Solomou Date: Mon, 27 Jul 2026 13:08:17 +0300 Subject: [PATCH 5/5] Fix cross-repository skill ownership Generated-By: PostHog Code Task-Id: 5cf8bddc-dae4-4dc9-84fb-195b06f48068 --- .../src/services/skills/skills.test.ts | 25 +++++++++++++++++-- .../src/services/skills/skills.ts | 24 ++++++++++++------ 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/packages/workspace-server/src/services/skills/skills.test.ts b/packages/workspace-server/src/services/skills/skills.test.ts index 130b62d80a..8feeb6a074 100644 --- a/packages/workspace-server/src/services/skills/skills.test.ts +++ b/packages/workspace-server/src/services/skills/skills.test.ts @@ -32,12 +32,16 @@ let pluginPath: string; let folderPath: string; let repoSkillsDir: string; -function makeService(): SkillsService { +function makeService(openFolders: string[] = [folderPath]): SkillsService { const plugin = { getPluginPath: () => pluginPath, } as unknown as PosthogPluginService; const folders = { - getFolders: async () => [{ path: folderPath, name: "my-repo" }], + getFolders: async () => + openFolders.map((folder, index) => ({ + path: folder, + name: `repo-${index}`, + })), } as unknown as FoldersService; return new SkillsService(plugin, folders, new WatcherService()); } @@ -526,6 +530,23 @@ describe("write-path guard", () => { ).rejects.toThrow("resolves outside its repository"); }); + it("rejects a repo skill that resolves into another open repository", async () => { + const targetRepo = path.join(root, "target-repo"); + const targetSkillsDir = path.join(targetRepo, ".claude", "skills"); + await createSkill(targetSkillsDir, "escapee"); + await rm(repoSkillsDir, { recursive: true, force: true }); + await symlink(targetSkillsDir, repoSkillsDir, "dir"); + const service = makeService([targetRepo, folderPath]); + + await expect( + service.bundleLocalSkill({ + name: "escapee", + source: "repo", + path: path.join(repoSkillsDir, "escapee"), + }), + ).rejects.toThrow("resolves outside its repository"); + }); + it("rejects a symlinked repo skill root", async () => { // A repository could commit `.claude/skills/foo` as a symlink to a // directory outside the repo; bundling must refuse to follow it rather diff --git a/packages/workspace-server/src/services/skills/skills.ts b/packages/workspace-server/src/services/skills/skills.ts index 14de1858a2..039a875935 100644 --- a/packages/workspace-server/src/services/skills/skills.ts +++ b/packages/workspace-server/src/services/skills/skills.ts @@ -527,14 +527,24 @@ export class SkillsService { realPath: await fs.promises.realpath(path.resolve(folder.path)), })), ); - const owningFolder = foldersWithRealPaths.find( - ({ folder, realPath }) => - path.resolve(path.join(folder.path, ".claude", "skills")) === parent || - realSkill === realPath || - realSkill.startsWith(realPath + path.sep), + const lexicalOwner = foldersWithRealPaths.find( + ({ folder }) => + path.resolve(path.join(folder.path, ".claude", "skills")) === parent, ); - if (!owningFolder) return; - if (!realSkill.startsWith(owningFolder.realPath + path.sep)) { + if (lexicalOwner) { + if (!realSkill.startsWith(lexicalOwner.realPath + path.sep)) { + throw new Error( + "Access denied: repository skill resolves outside its repository", + ); + } + return; + } + + const resolvedOwner = foldersWithRealPaths.find( + ({ realPath }) => + realSkill === realPath || realSkill.startsWith(realPath + path.sep), + ); + if (resolvedOwner && realSkill === resolvedOwner.realPath) { throw new Error( "Access denied: repository skill resolves outside its repository", );