fix(skills-sandbox): reject inputs that poison a sandbox's replay log

platform/archestra-rs/sandbox-core/src/validation.rs

@@ -32,7 +32,10 @@ pub(crate) fn validate_snapshot_file_path(path: &str) -> Result<()> {
 }
 
 pub(crate) fn validate_artifact_path(path: &str) -> Result<()> {
-    if path.contains('\0') || path.split('/').any(|segment| segment == "..") {
+    if path.contains('\0')
+        || path.split('/').any(|segment| segment == "..")
+        || resolves_to_directory(path)
+    {
         return Err(SandboxError::InvalidInput(format!(
             "invalid artifact path: {path:?}"
         )));
@@ -58,7 +61,14 @@ pub(crate) fn validate_artifact_path(path: &str) -> Result<()> {
 /// absolute file under the sandbox roots, free of traversal, null bytes, and
 /// shell metacharacters (defense in depth on top of the single-quoting).
 pub(crate) fn validate_upload_path(path: &str) -> Result<()> {
-    if path.contains('\0') || path.split('/').any(|segment| segment == "..") {
+    // a path whose final component is `.` (e.g. `/home/sandbox/.`) resolves to a
+    // directory, so the replayed `base64 -d > <path>` redirect fails on every
+    // run and permanently wedges the sandbox. a non-terminal `.` (`a/./b`)
+    // resolves to a regular file and is left alone.
+    if path.contains('\0')
+        || path.split('/').any(|segment| segment == "..")
+        || resolves_to_directory(path)
+    {
         return Err(SandboxError::InvalidInput(format!(
             "invalid upload path: {path:?}"
         )));
@@ -153,6 +163,14 @@ fn within_sandbox_roots(path: &str) -> bool {
         .any(|root| path == *root || path.strip_prefix(root).is_some_and(|r| r.starts_with('/')))
 }
 
+/// true when the path's final component makes it resolve to a directory rather
+/// than a file — a terminal `.` (`/home/sandbox/.` or a bare `.`). Such a path
+/// breaks the `> <path>` redirect that writes an uploaded/exported file. a
+/// non-terminal `.` (`a/./b`) normalizes to a regular file and is left alone.
+fn resolves_to_directory(path: &str) -> bool {
+    path == "." || path.ends_with("/.")
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -198,6 +216,11 @@ mod tests {
         ("/home/sandbox/../etc/passwd", false, false),
         // directory, not a file
         ("/home/sandbox/", false, false),
+        // terminal `.` resolves to a directory: rejected before it can be
+        // persisted as a replay event that fails `base64 -d > <dir>` forever
+        ("/home/sandbox/.", false, false),
+        // non-terminal `.` normalizes to a regular file: accepted
+        ("/home/sandbox/./x", true, true),
         // a root itself: replay would fail on the existing directory, so the
         // TS layer rejects it before it is persisted as an unreplayable event
         ("/home/sandbox", true, false),
@@ -220,6 +243,10 @@ mod tests {
         ("/etc/passwd", false, false),
         // traversal
         ("a/../b.txt", false, false),
+        // terminal `.` resolves to a directory
+        ("/skills/alpha/.", false, false),
+        // non-terminal `.` normalizes to a regular file: accepted
+        ("/skills/alpha/./x", true, true),
         // null byte
         ("a\0b.txt", false, false),
         // shell metacharacters: only this boundary rejects them; the TS layer

platform/backend/src/routes/skill/create.skill.route.test.ts

@@ -130,6 +130,36 @@ describe("POST /api/skills", () => {
     expect(response.statusCode).toBe(400);
   });
 
+  test("rejects a resource path that would break the sandbox mount with a 400", async () => {
+    // only paths that traverse or resolve to a directory are rejected.
+    for (const path of ["scripts/", "run.py/.", "../escape", "a/b/"]) {
+      const response = await ctx.app.inject({
+        method: "POST",
+        url: "/api/skills",
+        payload: {
+          content: MANIFEST,
+          files: [{ path, content: "print(1)" }],
+        },
+      });
+      expect(response.statusCode, `path: ${JSON.stringify(path)}`).toBe(400);
+    }
+  });
+
+  test("accepts a resource path with a non-terminal `.` or `//` alias", async () => {
+    const response = await ctx.app.inject({
+      method: "POST",
+      url: "/api/skills",
+      payload: {
+        content: MANIFEST,
+        files: [
+          { path: "./scripts/run.py", content: "print(1)" },
+          { path: "references//API.md", content: "# API" },
+        ],
+      },
+    });
+    expect(response.statusCode).toBe(200);
+  });
+
   test("rejects a manifest larger than the size cap", async () => {
     const response = await ctx.app.inject({
       method: "POST",

platform/backend/src/routes/skill/skill.routes.ts

@@ -931,7 +931,7 @@ const skillRoutes: FastifyPluginAsyncZod = async (fastify) => {
                 }),
               )
               .describe(
-                "Per created skill, resource paths not imported: oversized, beyond the per-skill file cap, or unfetchable",
+                "Per created skill, resource paths not imported: oversized, beyond the per-skill file cap, unfetchable, or unsafe to persist",
               ),
           }),
         ),

platform/backend/src/skills-sandbox/skill-sandbox-runtime-service.test.ts

@@ -380,6 +380,24 @@ describe("__internals", () => {
     }
   });
 
+  test("validateSkillMountName rejects names that collapse or escape the mount root", () => {
+    // covers names that skip parseSkillManifest (e.g. a built-in skill seeded
+    // with a white-label app name containing "/") and would otherwise persist
+    // an unreplayable mount.
+    for (const bad of ["", ".", "a/b", "..", "a..b", "skills/../x"]) {
+      expect(
+        () => __internals.validateSkillMountName(bad),
+        `name: ${JSON.stringify(bad)}`,
+      ).toThrow();
+    }
+    for (const ok of ["alpha", "my-skill", "Data Analysis"]) {
+      expect(
+        () => __internals.validateSkillMountName(ok),
+        `name: ${JSON.stringify(ok)}`,
+      ).not.toThrow();
+    }
+  });
+
   test("sanitizeAttachmentName strips unsafe chars and directory/leading dots", () => {
     const { sanitizeAttachmentName } = __internals;
     expect(sanitizeAttachmentName("pi mc.gif", "id")).toBe("pi_mc.gif");
@@ -761,6 +779,11 @@ describe("path validation vectors (mirrored with sandbox-core)", () => {
     ["/home/sandbox/../etc/passwd", false, false],
     // directory, not a file
     ["/home/sandbox/", false, false],
+    // terminal `.` resolves to a directory: rejected before it can be persisted
+    // as a replay event that fails `base64 -d > <dir>` forever
+    ["/home/sandbox/.", false, false],
+    // non-terminal `.` normalizes to a regular file: accepted
+    ["/home/sandbox/./x", true, true],
     // a root itself: rejected here before it is persisted as an unreplayable
     // event; the boundary alone would accept it
     ["/home/sandbox", false, true],
@@ -782,6 +805,10 @@ describe("path validation vectors (mirrored with sandbox-core)", () => {
     ["/etc/passwd", false, false],
     // traversal
     ["a/../b.txt", false, false],
+    // terminal `.` resolves to a directory
+    ["/skills/alpha/.", false, false],
+    // non-terminal `.` normalizes to a regular file: accepted
+    ["/skills/alpha/./x", true, true],
     // null byte
     ["a\0b.txt", false, false],
     // shell metacharacters: pass through here; the Rust boundary rejects them

platform/backend/src/skills-sandbox/skill-sandbox-runtime-service.ts

@@ -462,6 +462,7 @@ class SkillSandboxRuntimeService {
     this.ensureEnabled();
 
     return this.runWithSandbox(params.sandboxId, async (sandbox) => {
+      validateSkillMountName(params.skill.skillName);
       const files = await SkillVersionModel.findFiles(
         params.skill.skillVersionId,
       );
@@ -885,6 +886,16 @@ function resolveArtifactPath(params: {
       `invalid artifact path: ${JSON.stringify(params.path)}`,
     );
   }
+  // a terminal `.` (`/home/sandbox/.`, or a bare `.` against the cwd) resolves
+  // to a directory, so a persisted upload event would fail `base64 -d > <dir>`
+  // on every later replay and wedge the sandbox permanently. a non-terminal `.`
+  // (`a/./b`) normalizes to a regular file and is left alone.
+  if (params.path === "." || params.path.endsWith("/.")) {
+    rejectPath(
+      "artifact_path_directory",
+      `artifact path must be a file, not a directory: ${JSON.stringify(params.path)}`,
+    );
+  }
   if (params.path.startsWith("/")) {
     const allowedRoots = [SKILL_SANDBOX_ROOT, SKILL_SANDBOX_HOME];
     const isAllowed = allowedRoots.some(
@@ -956,6 +967,28 @@ function validateSkillMountFilePath(skillName: string, path: string): void {
   }
 }
 
+/**
+ * Reject a skill name that cannot become the mount root `/skills/<name>` before
+ * it is persisted as a mount event. Skill names authored through create/update
+ * are already gated at `parseSkillManifest`, but other sources reach the mount
+ * unvalidated — built-in skills seeded with a white-label app name, for one —
+ * so this mount-boundary check mirrors the Rust `skill_root_path` boundary
+ * (archestra-rs/sandbox-core/src/validation.rs) to keep an unreplayable mount
+ * out of the log regardless of where the name came from.
+ */
+function validateSkillMountName(skillName: string): void {
+  if (
+    skillName === "" ||
+    skillName === "." ||
+    skillName.includes("/") ||
+    skillName.includes("..")
+  ) {
+    throw new SkillSandboxError(
+      `Skill "${skillName}" has a name that cannot be mounted into a sandbox (it must not be "." or contain "/" or ".."). Rename the skill, then load it again.`,
+    );
+  }
+}
+
 /**
  * One install command per `requirements.txt` the version ships — root or
  * nested (skills commonly keep tool deps in `tools/requirements.txt`) — in
@@ -1158,6 +1191,7 @@ export const __internals = {
   resolveArtifactPath,
   validateUploadPath,
   validateSkillMountFilePath,
+  validateSkillMountName,
   requirementsInstallCommands,
   stageConversationAttachments,
   planAttachmentStaging,

platform/backend/src/skills/github-import.ts

@@ -6,6 +6,7 @@ import logger from "@/logging";
 import type { SkillFileEncoding, SkillFileKind } from "@/types";
 import {
   deriveSkillFileKind,
+  isSafeSkillResourcePath,
   type ParsedSkill,
   parseSkillManifest,
   SKILL_MANIFEST_FILENAME,
@@ -86,8 +87,9 @@ interface ImportedSkill {
   }[];
   /**
    * Resource paths (relative to the skill dir) that were not imported:
-   * oversized files, files beyond the per-skill cap, and files whose fetch
-   * failed. Surfaced to the caller so drops are never silent.
+   * oversized files, files beyond the per-skill cap, files whose fetch failed,
+   * and paths that would not be safe to persist (see `isSafeSkillResourcePath`).
+   * Surfaced to the caller so drops are never silent.
    */
   skippedFiles: string[];
   /** Provenance string, e.g. `owner/repo@main:skills/pdf`. */
@@ -312,7 +314,10 @@ export async function importSkills(params: {
       const fetched = fetchedFiles[cursor];
       cursor += 1;
       const relativePath = plan.toRelative(absolutePath);
-      if (fetched === null) {
+      // git blob paths are structurally clean, but this keeps the importer
+      // behind the same resource-path boundary as create/update so a path that
+      // would wedge the sandbox on replay is never persisted from any source.
+      if (fetched === null || !isSafeSkillResourcePath(relativePath)) {
         plan.skippedFiles.push(relativePath);
         continue;
       }

platform/backend/src/skills/parser.test.ts

@@ -134,6 +134,22 @@ describe("parseSkillManifest", () => {
     expect(() => parseSkillManifest(raw)).toThrow(/description/);
   });
 
+  test("rejects a name that breaks the sandbox mount root", () => {
+    for (const name of ["foo/bar", "..", "a/../b", ".", "skills/../x"]) {
+      const raw = [
+        "---",
+        `name: "${name}"`,
+        "description: A skill.",
+        "---",
+        "Body.",
+      ].join("\n");
+      expect(
+        () => parseSkillManifest(raw),
+        `name: ${JSON.stringify(name)}`,
+      ).toThrow(SkillParseError);
+    }
+  });
+
   test("throws on invalid YAML frontmatter", () => {
     const raw = ["---", "name: : :", "  bad", "---", "Body."].join("\n");
     expect(() => parseSkillManifest(raw)).toThrow(SkillParseError);

platform/backend/src/skills/parser.ts

@@ -75,6 +75,15 @@ export function parseSkillManifest(raw: string): ParsedSkill {
   if (!name) {
     throw new SkillParseError("SKILL.md frontmatter is missing `name`");
   }
+  // the name becomes the sandbox mount root `/skills/<name>`; a name that is
+  // `.` or carries `/` or `..` collapses or escapes that root and makes every
+  // later mount replay-fail, permanently wedging the sandbox. mirror the Rust
+  // `skill_root_path` boundary (archestra-rs/sandbox-core/src/validation.rs).
+  if (name === "." || name.includes("/") || name.includes("..")) {
+    throw new SkillParseError(
+      "SKILL.md `name` must not be `.` or contain `/` or `..`",
+    );
+  }
   if (!description) {
     throw new SkillParseError("SKILL.md frontmatter is missing `description`");
   }
@@ -117,6 +126,24 @@ export function deriveSkillFileKind(path: string): SkillFileKind {
   return /\.(md|mdx|txt|markdown)$/.test(normalized) ? "reference" : "asset";
 }
 
+/**
+ * A skill resource path is safe to persist when it is relative (no leading
+ * `/`), carries no `..` traversal segment, and does not resolve to a directory
+ * — its final segment is neither empty (a trailing slash) nor `.`. A path that
+ * resolves to a directory makes the Rust replay writer's `base64 -d > <path>`
+ * redirect fail on every run, permanently wedging the sandbox. Non-terminal
+ * `.`/empty segments (`a/./b`, `a//b`) normalize to a regular file and are
+ * allowed. Shared by the input schema and the GitHub importer so every
+ * persistence path applies the same boundary.
+ */
+export function isSafeSkillResourcePath(path: string): boolean {
+  if (path.startsWith("/")) return false;
+  const segments = path.split("/");
+  if (segments.some((s) => s === "..")) return false;
+  const last = segments[segments.length - 1];
+  return last !== "" && last !== ".";
+}
+
 // ===== Internal helpers =====
 
 function readString(value: unknown): string {

platform/backend/src/skills/validation.ts

@@ -3,7 +3,11 @@ import {
   MAX_SKILL_FILE_BYTES,
   MAX_SKILL_FILE_CONTENT_CHARS,
 } from "@/skills/github-import";
-import { deriveSkillFileKind, type ParsedSkill } from "@/skills/parser";
+import {
+  deriveSkillFileKind,
+  isSafeSkillResourcePath,
+  type ParsedSkill,
+} from "@/skills/parser";
 import {
   type InsertSkill,
   SkillFileEncodingSchema,
@@ -19,13 +23,10 @@ export const SkillFileInputSchema = z.object({
   path: z
     .string()
     .min(1)
-    .refine(
-      (p) => !p.startsWith("/") && !p.split("/").some((s) => s === ".."),
-      {
-        message:
-          "path must be relative and must not contain directory traversal sequences",
-      },
-    )
+    .refine(isSafeSkillResourcePath, {
+      message:
+        "path must be relative, must not traverse (`..`), and must not resolve to a directory (no trailing slash or `.`)",
+    })
     .describe("Resource path, e.g. references/API.md or scripts/run.py"),
   content: z
     .string()