chore: security in-depth and a few inconsistencies between docs and manifest

Author: atilafassina

packages/appkit/src/plugins/files/helpers.ts

@@ -2,3 +2,30 @@ export {
   contentTypeFromPath,
   isTextContentType,
 } from "../../connectors/files/defaults";
+
+/**
+ * Extract the parent directory from a file or directory path.
+ *
+ * Handles edge cases such as root-level files (`"/file.txt"` → `"/"`),
+ * paths without slashes (`"file.txt"` → `""`), and trailing slashes.
+ */
+export function parentDirectory(path: string): string {
+  const normalized =
+    path.length > 1 && path.endsWith("/") ? path.slice(0, -1) : path;
+  const lastSlash = normalized.lastIndexOf("/");
+
+  if (lastSlash > 0) return normalized.substring(0, lastSlash);
+  if (normalized.startsWith("/")) return "/";
+  return "";
+}
+
+/**
+ * Sanitize a filename for use in a `Content-Disposition` HTTP header.
+ *
+ * Redundancy check – Unity Catalog is unlikely to allow filenames with
+ * quotes or control characters, but we sanitize defensively to prevent
+ * HTTP header injection if upstream constraints ever change.
+ */
+export function sanitizeFilename(raw: string): string {
+  return raw.replace(/["\\]/g, "\\$&").replace(/[\r\n]/g, "");
+}

packages/appkit/src/plugins/files/manifest.json

@@ -13,7 +13,7 @@
         "permission": "WRITE_VOLUME",
         "fields": {
           "path": {
-            "env": "DATABRICKS_VOLUME_PATH",
+            "env": "DATABRICKS_DEFAULT_VOLUME",
             "description": "Volume path (e.g. /Volumes/catalog/schema/volume_name)"
           }
         }

packages/appkit/src/plugins/files/plugin.ts

@@ -14,6 +14,7 @@ import {
   filesReadDefaults,
   filesWriteDefaults,
 } from "./defaults";
+import { parentDirectory, sanitizeFilename } from "./helpers";
 import { filesManifest } from "./manifest";
 import type { DownloadResponse, IFilesConfig } from "./types";
 
@@ -340,7 +341,7 @@ export class FilesPlugin extends Plugin {
       return;
     }
 
-    const fileName = path.split("/").pop() ?? "download";
+    const fileName = sanitizeFilename(path.split("/").pop() ?? "download");
     res.setHeader("Content-Disposition", `attachment; filename="${fileName}"`);
     res.setHeader(
       "Content-Type",
@@ -392,7 +393,7 @@ export class FilesPlugin extends Plugin {
     res.setHeader("Content-Security-Policy", "sandbox");
 
     if (!isSafeInlineContentType(resolvedType)) {
-      const fileName = path.split("/").pop() ?? "download";
+      const fileName = sanitizeFilename(path.split("/").pop() ?? "download");
       res.setHeader(
         "Content-Disposition",
         `attachment; filename="${fileName}"`,
@@ -525,8 +526,7 @@ export class FilesPlugin extends Plugin {
       return;
     }
 
-    const parentDir = path.substring(0, path.lastIndexOf("/")) || path;
-    this._invalidateListCache(this._resolvePath(parentDir));
+    this._invalidateListCache(this._resolvePath(parentDirectory(path)));
 
     logger.debug(req, "Upload complete: path=%s", path);
     res.json(result);
@@ -558,8 +558,7 @@ export class FilesPlugin extends Plugin {
       return;
     }
 
-    const parentDir = dirPath.substring(0, dirPath.lastIndexOf("/")) || dirPath;
-    this._invalidateListCache(this._resolvePath(parentDir));
+    this._invalidateListCache(this._resolvePath(parentDirectory(dirPath)));
 
     res.json(result);
   }
@@ -588,8 +587,7 @@ export class FilesPlugin extends Plugin {
       return;
     }
 
-    const parentDir = path.substring(0, path.lastIndexOf("/")) || path;
-    this._invalidateListCache(this._resolvePath(parentDir));
+    this._invalidateListCache(this._resolvePath(parentDirectory(path)));
 
     res.json(result);
   }

packages/appkit/src/plugins/files/tests/helpers.test.ts

@@ -1,5 +1,10 @@
 import { describe, expect, test } from "vitest";
-import { contentTypeFromPath, isTextContentType } from "../helpers";
+import {
+  contentTypeFromPath,
+  isTextContentType,
+  parentDirectory,
+  sanitizeFilename,
+} from "../helpers";
 
 describe("contentTypeFromPath", () => {
   test("works without reported type", () => {
@@ -119,3 +124,60 @@ describe("isTextContentType", () => {
     expect(isTextContentType("image/jpeg")).toBe(false);
   });
 });
+
+describe("parentDirectory", () => {
+  test("extracts parent from nested path", () => {
+    expect(parentDirectory("/Volumes/catalog/schema/vol/file.txt")).toBe(
+      "/Volumes/catalog/schema/vol",
+    );
+  });
+
+  test("extracts parent from two-segment path", () => {
+    expect(parentDirectory("/dir/file.txt")).toBe("/dir");
+  });
+
+  test("returns root for root-level file", () => {
+    expect(parentDirectory("/file.txt")).toBe("/");
+  });
+
+  test("returns empty string for relative path without slash", () => {
+    expect(parentDirectory("file.txt")).toBe("");
+  });
+
+  test("strips trailing slash before computing parent", () => {
+    expect(parentDirectory("/dir/subdir/")).toBe("/dir");
+  });
+
+  test("handles root path with trailing slash", () => {
+    expect(parentDirectory("/")).toBe("/");
+  });
+
+  test("handles relative nested path", () => {
+    expect(parentDirectory("subdir/file.txt")).toBe("subdir");
+  });
+});
+
+describe("sanitizeFilename", () => {
+  test("passes through clean filenames unchanged", () => {
+    expect(sanitizeFilename("report.pdf")).toBe("report.pdf");
+    expect(sanitizeFilename("my-file_v2.txt")).toBe("my-file_v2.txt");
+  });
+
+  test("escapes double quotes", () => {
+    expect(sanitizeFilename('file"name.txt')).toBe('file\\"name.txt');
+  });
+
+  test("escapes backslashes", () => {
+    expect(sanitizeFilename("file\\name.txt")).toBe("file\\\\name.txt");
+  });
+
+  test("strips carriage returns and newlines", () => {
+    expect(sanitizeFilename("file\r\nname.txt")).toBe("filename.txt");
+    expect(sanitizeFilename("file\rname.txt")).toBe("filename.txt");
+    expect(sanitizeFilename("file\nname.txt")).toBe("filename.txt");
+  });
+
+  test("handles combined special characters", () => {
+    expect(sanitizeFilename('a"b\\c\r\nd.txt')).toBe('a\\"b\\\\cd.txt');
+  });
+});

packages/appkit/src/plugins/files/tests/plugin.integration.test.ts

@@ -70,7 +70,7 @@ describe("Files Plugin Integration", () => {
 
   beforeAll(async () => {
     setupDatabricksEnv({
-      DATABRICKS_VOLUME_PATH: "/Volumes/catalog/schema/vol",
+      DATABRICKS_DEFAULT_VOLUME: "/Volumes/catalog/schema/vol",
     });
     ServiceContext.reset();
 
@@ -96,7 +96,7 @@ describe("Files Plugin Integration", () => {
   });
 
   afterAll(async () => {
-    delete process.env.DATABRICKS_VOLUME_PATH;
+    delete process.env.DATABRICKS_DEFAULT_VOLUME;
     serviceContextMock?.restore();
     if (server) {
       await new Promise<void>((resolve, reject) => {