Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions packages/host-router/src/routers/skills.router.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { publicProcedure, router } from "@posthog/host-trpc/trpc";
import { SKILLS_SERVICE } from "@posthog/workspace-server/services/skills/identifiers";
import {
bundleLocalSkillInput,
bundleLocalSkillOutput,
createSkillInput,
deleteSkillFileInput,
deleteSkillInput,
Expand Down Expand Up @@ -36,6 +38,12 @@ export const skillsRouter = router({
.query(({ ctx }) =>
ctx.container.get<SkillsService>(SKILLS_SERVICE).listSkills(),
),
bundleLocal: publicProcedure
.input(bundleLocalSkillInput)
.output(bundleLocalSkillOutput)
.query(({ ctx, input }) =>
ctx.container.get<SkillsService>(SKILLS_SERVICE).bundleLocalSkill(input),
),
contents: publicProcedure
.input(skillContentsInput)
.output(skillContentsOutput)
Expand Down
19 changes: 19 additions & 0 deletions packages/workspace-server/src/services/skills/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,28 @@ export const installTeamSkillInput = z.object({
export type ExportedSkill = z.infer<typeof exportSkillOutput>;
export type InstallTeamSkillInput = z.infer<typeof installTeamSkillInput>;

export const bundleLocalSkillInput = z.object({
name: z.string().min(1),
source: z.enum(["user", "repo", "marketplace", "codex"]),
path: z.string().min(1),
});

export const bundleLocalSkillOutput = z.object({
name: z.string(),
source: z.enum(["user", "repo", "marketplace", "codex"]),
fileName: z.string(),
contentType: z.literal("application/zip"),
contentBase64: z.string(),
contentSha256: z.string(),
size: z.number().int().positive(),
});

export type BundleLocalSkillInput = z.infer<typeof bundleLocalSkillInput>;
export type BundleLocalSkillOutput = z.infer<typeof bundleLocalSkillOutput>;
export type SkillInfo = z.infer<typeof skillInfo>;
export type SkillScope = z.infer<typeof skillScope>;
export type CreateSkillInput = z.infer<typeof createSkillInput>;
export type SkillSource = z.infer<typeof skillSource>;
export type SkillFileEntry = z.infer<typeof skillFileEntry>;
export type SkillContents = z.infer<typeof skillContentsOutput>;
export type UploadableSkillSource = BundleLocalSkillInput["source"];
173 changes: 173 additions & 0 deletions packages/workspace-server/src/services/skills/skill-bundler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
import * as crypto from "node:crypto";
import * as fs from "node:fs";
import * as path from "node:path";
import { strToU8, zipSync } from "fflate";
import type { BundleLocalSkillOutput, UploadableSkillSource } from "./schemas";

const SKILL_BUNDLE_MAX_BYTES = 30 * 1024 * 1024;
const SKILL_BUNDLE_MAX_FILES = 1000;
const IGNORED_ENTRIES = new Set([
".DS_Store",
".git",
"node_modules",
"__pycache__",
]);

function toZipPath(filePath: string): string {
return filePath.split(path.sep).join("/");
}

function getSafeSkillFileName(name: string): string {
const safeName = path.basename(name).replace(/[^\w.-]/g, "_");
return safeName.length > 0 ? safeName : "skill";
}

async function assertSkillRoot(skillPath: string): Promise<string> {
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()) {
throw new Error("Local skill bundle must contain a SKILL.md file");
}
return root;
}

function isInsideRoot(root: string, candidate: string): boolean {
const relative = path.relative(root, candidate);
return (
Boolean(relative) &&
!relative.startsWith("..") &&
!path.isAbsolute(relative)
);
}

interface SkillFileAccumulator {
files: Record<string, Uint8Array>;
totalBytes: number;
}

async function addSkillFile(
acc: SkillFileAccumulator,
relativePath: string,
sourcePath: string,
size: number,
): Promise<void> {
if (Object.keys(acc.files).length >= SKILL_BUNDLE_MAX_FILES) {
throw new Error(
`Local skill bundle contains more than ${SKILL_BUNDLE_MAX_FILES} files`,
);
}
if (acc.totalBytes + size > SKILL_BUNDLE_MAX_BYTES) {
throw new Error("Local skill bundle exceeds the 30MB cloud run limit");
}
const content = await fs.promises.readFile(sourcePath);
acc.files[toZipPath(relativePath)] = new Uint8Array(content);
acc.totalBytes += content.byteLength;
}

async function collectSkillFiles(
root: string,
currentDir: string,
acc: SkillFileAccumulator,
): Promise<void> {
const entries = await fs.promises.readdir(currentDir, {
withFileTypes: true,
});

for (const entry of entries) {
if (IGNORED_ENTRIES.has(entry.name)) {
continue;
}

const absolutePath = path.join(currentDir, entry.name);
const relativePath = path.relative(root, absolutePath);
if (
!relativePath ||
relativePath.startsWith("..") ||
path.isAbsolute(relativePath)
) {
continue;
}

if (entry.isSymbolicLink()) {
const realPath = await fs.promises
.realpath(absolutePath)
.catch(() => null);
if (!realPath || !isInsideRoot(root, realPath)) {
continue;
}
const stat = await fs.promises.stat(realPath);
if (!stat.isFile()) {
continue;
}
await addSkillFile(acc, relativePath, realPath, stat.size);
continue;
}

if (entry.isDirectory()) {
await collectSkillFiles(root, absolutePath, acc);
continue;
}

if (!entry.isFile()) {
continue;
}

const stat = await fs.promises.stat(absolutePath);
await addSkillFile(acc, relativePath, absolutePath, stat.size);
}
}

export async function bundleLocalSkill({
name,
source,
skillPath,
}: {
name: string;
source: UploadableSkillSource;
skillPath: string;
}): Promise<BundleLocalSkillOutput> {
const root = await assertSkillRoot(skillPath);
const acc: SkillFileAccumulator = { files: {}, totalBytes: 0 };
await collectSkillFiles(root, root, acc);
const files = acc.files;
const fileNames = Object.keys(files).sort();

if (!files["SKILL.md"]) {
throw new Error("Local skill bundle must contain a SKILL.md file");
}

const manifest = {
schema_version: 1,
name,
source,
};
Comment thread
tatoalo marked this conversation as resolved.

const zipInput: Record<string, Uint8Array> = {};
for (const fileName of fileNames) {
zipInput[fileName] = files[fileName];
}
zipInput["posthog-skill-bundle.json"] = strToU8(JSON.stringify(manifest));

const zipped = zipSync(zipInput, { level: 6 });
if (zipped.byteLength > SKILL_BUNDLE_MAX_BYTES) {
throw new Error(
"Local skill bundle archive exceeds the 30MB cloud run limit",
);
}

const contentSha256 = crypto
.createHash("sha256")
.update(zipped)
.digest("hex");

return {
name,
source,
fileName: `${getSafeSkillFileName(name)}.zip`,
contentType: "application/zip",
contentBase64: Buffer.from(zipped).toString("base64"),
contentSha256,
size: zipped.byteLength,
};
}
13 changes: 13 additions & 0 deletions packages/workspace-server/src/services/skills/skills.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,16 @@ import type { PosthogPluginService } from "../posthog-plugin/posthog-plugin";
import type { WatcherService } from "../watcher/service";
import { parseSkillFrontmatter } from "./parse-skill-frontmatter";
import type {
BundleLocalSkillInput,
BundleLocalSkillOutput,
CreateSkillInput,
ExportedSkill,
InstallTeamSkillInput,
SkillContents,
SkillInfo,
SkillSource,
} from "./schemas";
import { bundleLocalSkill } from "./skill-bundler";
import {
getMarketplaceInstallPaths,
getUserSkillsDir,
Expand Down Expand Up @@ -485,6 +488,16 @@ export class SkillsService {
}
return resolved;
}

bundleLocalSkill(
input: BundleLocalSkillInput,
): Promise<BundleLocalSkillOutput> {
return bundleLocalSkill({
name: input.name,
source: input.source,
skillPath: input.path,
});
Comment on lines +492 to +499

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 security Missing resolveKnownSkillDir guard allows arbitrary filesystem bundling

Every other method that accepts a skillPathgetSkillContents, readSkillFile, saveSkillManifest, etc. — first passes it through resolveKnownSkillDir (or resolveWritableSkillDir), which validates that the path sits under a known skill root. That guard's own comment says "This keeps the contents/readFile endpoints from becoming arbitrary-filesystem reads." bundleLocalSkill skips that guard entirely, so any caller can provide an arbitrary path (e.g. ~/.ssh) containing a SKILL.md and receive the full directory base64-encoded in the response. The fix is to call resolveKnownSkillDir on input.path before forwarding it to the bundler.

Rule Used: When implementing new features, ensure that owners... (source)

Learned From
PostHog/posthog#31236

}
}

export function validateSkillDirName(name: string): void {
Expand Down
Loading