fix: address CodeRabbit and Devin review findings from PR #74

- TLS: default allowSelfSigned=true to preserve self-signed cert workflow;
  enforce rejectUnauthorized=true when trustedCAs are supplied in client context
- Signature verification: verifySecureMessage now cryptographically verifies
  signatures using PeerIdentity.verifyHex instead of only checking fingerprints
- WebSocket: fix timeout handle leak in performClientHandshake; add error
  logging to empty catch blocks; add per-socket error handler; pre-compute
  signature once in broadcast(); add send error callbacks; reject startup if
  TLS configured but certInfo missing
- Command parsing: replace broken regex tokenizer with shared splitCommand()
  utility that correctly handles --flag="value with spaces"
- Rate limiter: prefer x-real-ip, fall back to rightmost x-forwarded-for entry
- Save route: scope IPv6 prefix checks to actual IPv6 addresses; stop leaking
  internal error messages to clients
- Publish: extract shared parseSkillFrontmatter(); fix getRepoInfo to use
  async execFileSync; fix path containment checks with separator
- Remote transport: add rejectUnauthorized option; bracket IPv6 in URLs;
  fix host:port splitting for IPv6 with lastIndexOf
- Health checks: add tls/tlsAllowSelfSigned to Host type; pass https options
- Fetcher: validate owner/repo; derive indexDir from INDEX_PATH
- .gitignore: add recursive **/.env pattern, remove redundant entries

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: rohitg00

.gitignore

@@ -24,8 +24,7 @@ npm-debug.log*
 
 # Environment
 .env
-.env.local
-.env.*.local
+**/.env
 **/.env.local
 **/.env.*.local
 

packages/api/src/middleware/rate-limit.ts

@@ -18,11 +18,12 @@ export function rateLimiter(maxRequests = 60, windowMs = 60_000) {
   }, windowMs).unref();
 
   return async (c: Context, next: Next): Promise<Response | void> => {
+    const xRealIp = c.req.header("x-real-ip");
     const forwarded = c.req.header("x-forwarded-for");
-    const ip =
-      forwarded?.split(",")[0]?.trim() ||
-      c.req.header("x-real-ip") ||
-      "unknown";
+    const forwardedIp = forwarded
+      ? forwarded.split(",").at(-1)?.trim()
+      : undefined;
+    const ip = xRealIp || forwardedIp || "unknown";
     const now = Date.now();
 
     let entry = windows.get(ip);

packages/api/src/routes/save.ts

@@ -34,14 +34,16 @@ function isAllowedUrl(url: string): boolean {
     if (bare.startsWith("10.") || bare.startsWith("192.168.")) return false;
     if (/^172\.(1[6-9]|2\d|3[01])\./.test(bare)) return false;
     if (bare.startsWith("169.254.")) return false;
-    if (
-      bare.startsWith("fe80:") ||
-      bare.startsWith("fc") ||
-      bare.startsWith("fd")
-    )
-      return false;
+    if (bare.includes(":")) {
+      if (
+        bare.startsWith("fe80:") ||
+        bare.startsWith("fc") ||
+        bare.startsWith("fd")
+      )
+        return false;
+      if (bare.startsWith("ff")) return false;
+    }
     if (/^(22[4-9]|23\d|24\d|25[0-5])\./.test(bare)) return false;
-    if (bare.startsWith("ff")) return false;
     return true;
   } catch {
     return false;
@@ -106,17 +108,17 @@ export function saveRoutes() {
         tags: tagger.detectTags(content),
       });
     } catch (err) {
-      const message = err instanceof Error ? err.message : "Unknown error";
+      console.error("Save extraction failed:", err);
       const isTimeout =
         (err instanceof DOMException && err.name === "TimeoutError") ||
         (err instanceof Error &&
           (err.name === "TimeoutError" || err.name === "AbortError"));
 
       if (isTimeout) {
-        return c.json({ error: `Fetch timeout: ${message}` }, 504);
+        return c.json({ error: "Fetch timed out" }, 504);
       }
 
-      return c.json({ error: `Extraction failed: ${message}` }, 422);
+      return c.json({ error: "Extraction failed" }, 422);
     }
   });
 

packages/cli/src/commands/publish.ts

@@ -6,7 +6,7 @@ import {
   readdirSync,
   statSync,
 } from "node:fs";
-import { join, basename, dirname, resolve } from "node:path";
+import { join, basename, dirname, resolve, sep } from "node:path";
 import chalk from "chalk";
 import { Command, Option } from "clipanion";
 import {
@@ -41,6 +41,59 @@ interface SkillFrontmatter {
   version?: string;
 }
 
+function parseSkillFrontmatter(content: string): SkillFrontmatter {
+  const match = content.match(/^---\r?\n([\s\S]*?)\r?\n---/);
+  if (!match) return {};
+
+  const frontmatter: SkillFrontmatter = {};
+  const lines = match[1].split(/\r?\n/);
+  let inTagsList = false;
+
+  for (const line of lines) {
+    if (inTagsList) {
+      const tagMatch = line.match(/^\s*-\s*(.+)$/);
+      if (tagMatch) {
+        frontmatter.tags ??= [];
+        frontmatter.tags.push(tagMatch[1].trim().replace(/^["']|["']$/g, ""));
+        continue;
+      }
+      if (line.trim() === "") continue;
+      inTagsList = false;
+    }
+
+    const colonIdx = line.indexOf(":");
+    if (colonIdx === -1) continue;
+
+    const key = line.slice(0, colonIdx).trim();
+    const value = line.slice(colonIdx + 1).trim();
+
+    switch (key) {
+      case "name":
+        frontmatter.name = value.replace(/^["']|["']$/g, "");
+        break;
+      case "description":
+        frontmatter.description = value.replace(/^["']|["']$/g, "");
+        break;
+      case "version":
+        frontmatter.version = value.replace(/^["']|["']$/g, "");
+        break;
+      case "tags":
+        if (value.startsWith("[")) {
+          frontmatter.tags = value
+            .slice(1, -1)
+            .split(",")
+            .map((t) => t.trim().replace(/^["']|["']$/g, ""))
+            .filter((t) => t.length > 0);
+        } else if (value === "") {
+          inTagsList = true;
+          frontmatter.tags = [];
+        }
+        break;
+    }
+  }
+  return frontmatter;
+}
+
 export class PublishCommand extends Command {
   static override paths = [["publish"]];
 
@@ -186,7 +239,7 @@ export class PublishCommand extends Command {
           skill.safeName,
         );
         const resolvedDir = resolve(mintlifyDir);
-        if (!resolvedDir.startsWith(resolvedOutput)) {
+        if (!resolvedDir.startsWith(resolvedOutput + sep)) {
           console.log(
             chalk.red(`Skipping ${skill.safeName} (path traversal detected)`),
           );
@@ -255,7 +308,7 @@ export class PublishCommand extends Command {
       const resolvedSkillDir = resolve(skillDir);
       const resolvedWellKnownDir = resolve(wellKnownDir);
 
-      if (!resolvedSkillDir.startsWith(resolvedWellKnownDir)) {
+      if (!resolvedSkillDir.startsWith(resolvedWellKnownDir + sep)) {
         console.log(
           chalk.yellow(`  Skipping "${skill.name}" (path traversal detected)`),
         );
@@ -360,8 +413,7 @@ export class PublishCommand extends Command {
     for (const entry of entries) {
       const entryPath = join(skillPath, entry);
       if (statSync(entryPath).isFile()) {
-        if (entry.startsWith(".") || entry === ".skillkit-metadata.json")
-          continue;
+        if (entry.startsWith(".")) continue;
         files.push(entry);
       }
     }
@@ -374,57 +426,7 @@ export class PublishCommand extends Command {
   }
 
   private parseFrontmatter(content: string): SkillFrontmatter {
-    const match = content.match(/^---\r?\n([\s\S]*?)\r?\n---/);
-    if (!match) return {};
-
-    const frontmatter: SkillFrontmatter = {};
-    const lines = match[1].split(/\r?\n/);
-    let inTagsList = false;
-
-    for (const line of lines) {
-      if (inTagsList) {
-        const tagMatch = line.match(/^\s*-\s*(.+)$/);
-        if (tagMatch) {
-          frontmatter.tags ??= [];
-          frontmatter.tags.push(tagMatch[1].trim().replace(/^["']|["']$/g, ""));
-          continue;
-        }
-        if (line.trim() === "") continue;
-        inTagsList = false;
-      }
-
-      const colonIdx = line.indexOf(":");
-      if (colonIdx === -1) continue;
-
-      const key = line.slice(0, colonIdx).trim();
-      const value = line.slice(colonIdx + 1).trim();
-
-      switch (key) {
-        case "name":
-          frontmatter.name = value.replace(/^["']|["']$/g, "");
-          break;
-        case "description":
-          frontmatter.description = value.replace(/^["']|["']$/g, "");
-          break;
-        case "version":
-          frontmatter.version = value.replace(/^["']|["']$/g, "");
-          break;
-        case "tags":
-          if (value.startsWith("[")) {
-            frontmatter.tags = value
-              .slice(1, -1)
-              .split(",")
-              .map((t) => t.trim().replace(/^["']|["']$/g, ""))
-              .filter((t) => t.length > 0);
-          } else if (value === "") {
-            inTagsList = true;
-            frontmatter.tags = [];
-          }
-          break;
-      }
-    }
-
-    return frontmatter;
+    return parseSkillFrontmatter(content);
   }
 }
 
@@ -468,7 +470,7 @@ export class PublishSubmitCommand extends Command {
     const skillName =
       this.name || frontmatter.name || basename(dirname(skillMdPath));
 
-    const repoInfo = this.getRepoInfo(dirname(skillMdPath));
+    const repoInfo = await this.getRepoInfo(dirname(skillMdPath));
     if (!repoInfo) {
       console.error(chalk.red("Not a git repository or no remote configured"));
       console.error(
@@ -567,42 +569,7 @@ export class PublishSubmitCommand extends Command {
   }
 
   private parseFrontmatter(content: string): SkillFrontmatter {
-    const match = content.match(/^---\r?\n([\s\S]*?)\r?\n---/);
-    if (!match) return {};
-
-    const frontmatter: SkillFrontmatter = {};
-    const lines = match[1].split(/\r?\n/);
-
-    for (const line of lines) {
-      const colonIdx = line.indexOf(":");
-      if (colonIdx === -1) continue;
-
-      const key = line.slice(0, colonIdx).trim();
-      const value = line.slice(colonIdx + 1).trim();
-
-      switch (key) {
-        case "name":
-          frontmatter.name = value.replace(/^["']|["']$/g, "");
-          break;
-        case "description":
-          frontmatter.description = value.replace(/^["']|["']$/g, "");
-          break;
-        case "version":
-          frontmatter.version = value.replace(/^["']|["']$/g, "");
-          break;
-        case "tags":
-          if (value.startsWith("[")) {
-            frontmatter.tags = value
-              .slice(1, -1)
-              .split(",")
-              .map((t) => t.trim().replace(/^["']|["']$/g, ""))
-              .filter((t) => t.length > 0);
-          }
-          break;
-      }
-    }
-
-    return frontmatter;
+    return parseSkillFrontmatter(content);
   }
 
   private slugify(name: string): string {
@@ -613,10 +580,12 @@ export class PublishSubmitCommand extends Command {
       .replace(/^-+|-+$/g, "");
   }
 
-  private getRepoInfo(dir: string): { owner: string; repo: string } | null {
+  private async getRepoInfo(
+    dir: string,
+  ): Promise<{ owner: string; repo: string } | null> {
     try {
-      const { execSync } = require("node:child_process");
-      const remote = execSync("git remote get-url origin", {
+      const { execFileSync } = await import("node:child_process");
+      const remote = execFileSync("git", ["remote", "get-url", "origin"], {
         cwd: dir,
         encoding: "utf-8",
         stdio: ["pipe", "pipe", "ignore"],

packages/core/src/executor/engine.ts

@@ -6,6 +6,7 @@
 
 import { execFileSync } from "node:child_process";
 import { randomUUID } from "node:crypto";
+import { splitCommand } from "../utils/shell.js";
 import type {
   ExecutableSkill,
   ExecutableTask,
@@ -526,13 +527,9 @@ export class SkillExecutionEngine {
       for (const rule of task.verify.automated) {
         if (rule.command) {
           try {
-            const cmdParts =
-              rule.command.match(/(?:[^\s"']+|"[^"]*"|'[^']*')+/g) || [];
+            const cmdParts = splitCommand(rule.command);
             if (cmdParts.length === 0) return false;
-            const [cmd, ...cmdArgs] = cmdParts as [string, ...string[]];
-            const sanitizedArgs = cmdArgs.map((a) =>
-              a.replace(/^["']|["']$/g, ""),
-            );
+            const [cmd, ...sanitizedArgs] = cmdParts;
             const output = execFileSync(cmd, sanitizedArgs, {
               cwd: this.projectPath,
               encoding: "utf-8",

packages/core/src/index.ts

@@ -132,3 +132,6 @@ export * from './agents-md/index.js';
 
 // Save (Content Extraction & Skill Generation)
 export * from './save/index.js';
+
+// Utilities
+export { splitCommand } from './utils/shell.js';

packages/core/src/plan/executor.ts

@@ -14,6 +14,8 @@ import type {
   PlanEvent,
   PlanEventListener,
 } from "./types.js";
+import { execFileSync } from "node:child_process";
+import { splitCommand } from "../utils/shell.js";
 
 /**
  * Step executor function type
@@ -499,13 +501,11 @@ export const shellExecutor: StepExecutor = async (step, _task, _plan) => {
   }
 
   try {
-    const { execFileSync } = await import("node:child_process");
-    const parts = step.command.match(/(?:[^\s"']+|"[^"]*"|'[^']*')+/g) || [];
+    const parts = splitCommand(step.command);
     if (parts.length === 0) {
       return { success: false, output: "", error: "Empty command" };
     }
-    const [cmd, ...rawArgs] = parts as [string, ...string[]];
-    const args = rawArgs.map((a) => a.replace(/^["']|["']$/g, ""));
+    const [cmd, ...args] = parts;
     const output = execFileSync(cmd, args, {
       encoding: "utf-8",
       timeout: 60000,

packages/core/src/recommend/fetcher.ts

@@ -6,7 +6,7 @@ import {
   writeFileSync,
   mkdirSync,
 } from "node:fs";
-import { join } from "node:path";
+import { join, dirname } from "node:path";
 import { tmpdir, homedir } from "node:os";
 import { randomUUID } from "node:crypto";
 import type { SkillIndex, SkillSummary, IndexSource } from "./types.js";
@@ -46,6 +46,13 @@ export async function fetchSkillsFromRepo(
   owner: string,
   repo: string,
 ): Promise<{ skills: SkillSummary[]; error?: string }> {
+  if (!/^[\w.-]+$/.test(owner) || !/^[\w.-]+$/.test(repo)) {
+    return {
+      skills: [],
+      error: `Invalid owner or repo name: ${owner}/${repo}`,
+    };
+  }
+
   const cloneUrl = `https://github.com/${owner}/${repo}.git`;
   const tempDir = join(tmpdir(), `skillkit-fetch-${randomUUID()}`);
 
@@ -179,7 +186,7 @@ export async function buildSkillIndex(
  * Save skill index to cache
  */
 export function saveIndex(index: SkillIndex): void {
-  const indexDir = join(homedir(), ".skillkit");
+  const indexDir = dirname(INDEX_PATH);
   if (!existsSync(indexDir)) {
     mkdirSync(indexDir, { recursive: true });
   }