fix(plugins): terminate git clone args so git: specs cannot inject options (#102398)

* fix(plugins): terminate git clone args so git: specs cannot inject options

A git: install spec whose base ends in .git or is scp-form was passed to
git clone as a bare positional. A spec beginning with a dash, such as
git:--upload-pack=/path/pwn.git, was parsed by git as an option rather than
a repository, reaching a command-execution primitive. The sibling
git switch --detach -- <ref> in the same files was already hardened; the
clone sites were missed.

Add a -- option terminator before the URL at both clone call sites
(installPluginFromGitSpec and installSkillFromSource) and reject a
leading-dash URL in isGitUrl as defense in depth.

* test(plugins): prove git option rejection boundary

Co-authored-by: yetval <yetvald@gmail.com>

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>
(cherry picked from commit 9706aae10a)

src/plugins/git-install.test.ts

@@ -128,6 +128,11 @@ describe("parseGitPluginSpec", () => {
     expect(parsed.ref).toBe("feature/foo");
     expect(parsed.label).toBe("git@github.com:acme/demo");
   });
+
+  it("rejects option-injection specs whose url would start with a dash", () => {
+    expect(parseGitPluginSpec("git:--upload-pack=/tmp/pwn.git")).toBeNull();
+    expect(parseGitPluginSpec("git:-oProxyCommand=payload@example.com:acme/demo.git")).toBeNull();
+  });
 });
 
 describe("isImmutableGitCommitRef", () => {
@@ -167,6 +172,19 @@ describe("installPluginFromGitSpec", () => {
     );
   });
 
+  it("rejects option-leading clone sources before invoking git", async () => {
+    const result = await installPluginFromGitSpec({
+      spec: "git:--upload-pack=/tmp/pwn.git",
+      dryRun: true,
+    });
+
+    expect(result).toEqual({
+      ok: false,
+      error: "unsupported git: plugin spec: git:--upload-pack=/tmp/pwn.git",
+    });
+    expect(runCommandWithTimeoutMock).not.toHaveBeenCalled();
+  });
+
   it("clones, checks out refs, installs from the clone, and returns commit metadata", async () => {
     runCommandWithTimeoutMock
       .mockResolvedValueOnce({ code: 0, stdout: "", stderr: "" })
@@ -206,8 +224,13 @@ describe("installPluginFromGitSpec", () => {
     expect(result.git.ref).toBe("v1.2.3");
     expect(result.git.commit).toBe("abc123");
     const cloneArgv = commandArgvAt(0);
-    expect(cloneArgv.slice(0, 3)).toEqual(["git", "clone", "https://github.com/acme/demo.git"]);
-    expect(cloneArgv[3]).toContain("/repo");
+    expect(cloneArgv.slice(0, 4)).toEqual([
+      "git",
+      "clone",
+      "--",
+      "https://github.com/acme/demo.git",
+    ]);
+    expect(cloneArgv[4]).toContain("/repo");
     expect(commandArgvAt(1)).toEqual(["git", "switch", "--detach", "--", "v1.2.3"]);
     expect(commandArgvAt(3)).toEqual([
       "npm",
@@ -310,14 +333,15 @@ describe("installPluginFromGitSpec", () => {
     }
 
     const cloneArgv = commandArgvAt(0);
-    expect(cloneArgv.slice(0, 5)).toEqual([
+    expect(cloneArgv.slice(0, 6)).toEqual([
       "git",
       "clone",
       "--depth",
       "1",
+      "--",
       "https://github.com/acme/demo.git",
     ]);
-    expect(cloneArgv[5]).toContain("/repo");
+    expect(cloneArgv[6]).toContain("/repo");
   });
 
   it("runs install policy preflight before npm installs git dependencies", async () => {
@@ -347,11 +371,12 @@ describe("installPluginFromGitSpec", () => {
       expect(result.error).toContain("git installs disabled");
     }
     expect(runCommandWithTimeoutMock).toHaveBeenCalledTimes(2);
-    expect(commandArgvAt(0).slice(0, 5)).toEqual([
+    expect(commandArgvAt(0).slice(0, 6)).toEqual([
       "git",
       "clone",
       "--depth",
       "1",
+      "--",
       "https://github.com/acme/demo.git",
     ]);
     expect(commandArgvAt(1)).toEqual(["git", "rev-parse", "HEAD"]);

src/plugins/git-install.ts

@@ -116,6 +116,9 @@ function isHttpUrl(value: string): boolean {
 }
 
 function isGitUrl(value: string): boolean {
+  if (value.startsWith("-")) {
+    return false;
+  }
   return (
     /^(?:ssh|git|file):\/\//i.test(value) || looksLikeScpGitUrl(value) || value.endsWith(".git")
   );
@@ -345,8 +348,8 @@ export async function installPluginFromGitSpec(
       `Cloning ${sanitizeForLog(redactSensitiveUrlLikeString(parsed.label))}...`,
     );
     const cloneArgs = parsed.ref
-      ? ["git", "clone", parsed.url, repoDir]
-      : ["git", "clone", "--depth", "1", parsed.url, repoDir];
+      ? ["git", "clone", "--", parsed.url, repoDir]
+      : ["git", "clone", "--depth", "1", "--", parsed.url, repoDir];
     const clone = await runGitCommand({
       argv: cloneArgs,
       action: "clone",

src/skills/lifecycle/source-install.test.ts

@@ -436,4 +436,21 @@ describe("installSkillFromSource", () => {
       });
     });
   });
+
+  it("refuses git specs whose url would be consumed as a git clone option", async () => {
+    await withTempDir({ prefix: "openclaw-skill-source-opt-inject-" }, async (root) => {
+      const workspaceDir = path.join(root, "workspace");
+      const payload = path.join(root, "payload.git");
+
+      const result = await installSkillFromSource({
+        workspaceDir,
+        spec: `git:--upload-pack=${payload}`,
+      });
+
+      expect(result).toMatchObject({
+        ok: false,
+        error: expect.stringContaining("Unsupported git skill spec"),
+      });
+    });
+  });
 });

src/skills/lifecycle/source-install.ts

@@ -283,8 +283,8 @@ async function installGitSkill(params: {
       `Cloning ${sanitizeForLog(redactSensitiveUrlLikeString(parsed.label))}...`,
     );
     const cloneArgs = parsed.ref
-      ? ["git", "clone", parsed.url, repoDir]
-      : ["git", "clone", "--depth", "1", parsed.url, repoDir];
+      ? ["git", "clone", "--", parsed.url, repoDir]
+      : ["git", "clone", "--depth", "1", "--", parsed.url, repoDir];
     const clone = await runGitCommand({
       argv: cloneArgs,
       action: "clone",