From f38235b51bb1f72643fcfbdd6b30564740659a0a Mon Sep 17 00:00:00 2001
From: Dotta <bippadotta@protonmail.com>
Date: Thu, 9 Jul 2026 20:04:59 +0000
Subject: [PATCH] Fix Clip dependency and path validation gaps

Co-Authored-By: Paperclip <noreply@paperclip.ing>
---
 packages/shared/src/clip-manifest.test.ts |  4 ++++
 packages/shared/src/clip-manifest.ts      |  5 +++++
 server/src/__tests__/clips-routes.test.ts | 19 +++++++++++++++++++
 server/src/routes/clips.ts                | 14 ++++++++++++--
 4 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/packages/shared/src/clip-manifest.test.ts b/packages/shared/src/clip-manifest.test.ts
index 0442b806a55..c33d9fc325b 100644
--- a/packages/shared/src/clip-manifest.test.ts
+++ b/packages/shared/src/clip-manifest.test.ts
@@ -292,6 +292,10 @@ describe("clip manifest foundation", () => {
     expect(first.dependencies.adapters.map((entry) => entry.type)).toEqual(["claude_local", "codex_local"]);
     expect(first.dependencies.skills.map((entry) => entry.key)).toEqual(["triage"]);
     expect(first.dependencies.secrets.map((entry) => entry.key)).toContain("GH_TOKEN");
+    expect(first.dependencies.secrets.find((entry) => entry.key === "GH_TOKEN")).toMatchObject({
+      requirement: "required",
+      sourceRefs: ["agents.support", "envInputs.0"],
+    });
     expect(first.dependencies.permissions.map((entry) => entry.capability)).toContain("github.issues.write");
     expect(first.dependencies.runtime).toMatchObject({
       browser: true,
diff --git a/packages/shared/src/clip-manifest.ts b/packages/shared/src/clip-manifest.ts
index 1fcb44285f7..652a10203d2 100644
--- a/packages/shared/src/clip-manifest.ts
+++ b/packages/shared/src/clip-manifest.ts
@@ -894,6 +894,11 @@ function addMapEntry<T extends { sourceRefs: Set<string> }>(map: Map<string, T>,
     map.set(key, value);
     return;
   }
+  const existingRequirement = existing as T & { requirement?: "required" | "optional" };
+  const nextRequirement = (value as T & { requirement?: "required" | "optional" }).requirement;
+  if (existingRequirement.requirement === "optional" && nextRequirement === "required") {
+    existingRequirement.requirement = "required";
+  }
   for (const sourceRef of value.sourceRefs) {
     existing.sourceRefs.add(sourceRef);
   }
diff --git a/server/src/__tests__/clips-routes.test.ts b/server/src/__tests__/clips-routes.test.ts
index 865a978481a..c42d308042e 100644
--- a/server/src/__tests__/clips-routes.test.ts
+++ b/server/src/__tests__/clips-routes.test.ts
@@ -232,6 +232,25 @@ describe("clip routes", () => {
     expect(mockCompanyPortabilityService.previewExport).not.toHaveBeenCalled();
   });
 
+  it("rejects malformed clipId path params before clip database lookups", async () => {
+    const revisionRes = await request(createApp())
+      .post("/api/clips/not-a-uuid/revisions")
+      .send({
+        manifestChecksum: "sha256:manifest-v2",
+        artifactChecksum: "sha256:artifact-v2",
+        manifestPayload: { schema: "paperclip.clip/v1" },
+      });
+    const patchRes = await request(createApp())
+      .patch("/api/clips/not-a-uuid")
+      .send({ title: "Updated title" });
+
+    expect(revisionRes.status).toBe(400);
+    expect(revisionRes.body).toEqual({ error: "Invalid clipId path parameter." });
+    expect(patchRes.status).toBe(400);
+    expect(patchRes.body).toEqual({ error: "Invalid clipId path parameter." });
+    expect(mockClipService.getClipById).not.toHaveBeenCalled();
+  });
+
   it("strips owner-supplied trust states when creating revisions", async () => {
     const clipId = "33333333-3333-4333-8333-333333333333";
     const sourceCompanyId = "22222222-2222-4222-8222-222222222222";
diff --git a/server/src/routes/clips.ts b/server/src/routes/clips.ts
index b6ca3d49719..e102e8b829c 100644
--- a/server/src/routes/clips.ts
+++ b/server/src/routes/clips.ts
@@ -75,6 +75,14 @@ function getClipCompanyIdParam(req: Request) {
   return companyId;
 }
 
+function getClipIdParam(req: Request) {
+  const clipId = req.params.clipId as string;
+  if (!isUuidLike(clipId)) {
+    throw badRequest("Invalid clipId path parameter.");
+  }
+  return clipId;
+}
+
 function evictExpiredClipRateLimitHits(cutoff: number, now: number) {
   if (now - clipRateLimitLastEvictionAt < CLIP_RATE_LIMIT_EVICTION_INTERVAL_MS) return;
   clipRateLimitLastEvictionAt = now;
@@ -607,7 +615,8 @@ export function clipRoutes(db: Db, storage?: StorageService) {
   });
 
   router.post("/clips/:clipId/revisions", validate(createClipRevisionSchema), async (req, res) => {
-    const clip = await svc.getClipById(req.params.clipId as string);
+    const clipId = getClipIdParam(req);
+    const clip = await svc.getClipById(clipId);
     if (!clip) {
       res.status(404).json({ error: "Clip not found" });
       return;
@@ -640,7 +649,8 @@ export function clipRoutes(db: Db, storage?: StorageService) {
   });
 
   router.patch("/clips/:clipId", validate(updateClipSchema), async (req, res) => {
-    const clip = await svc.getClipById(req.params.clipId as string);
+    const clipId = getClipIdParam(req);
+    const clip = await svc.getClipById(clipId);
     if (!clip) {
       res.status(404).json({ error: "Clip not found" });
       return;