From 35e4d70116c8a2a5b82eaf4b8c58889dc02cbe46 Mon Sep 17 00:00:00 2001
From: Craig Jennings <c@cjennings.net>
Date: Sun, 24 May 2026 04:27:54 -0500
Subject: fix(vc): harden clipboard git-clone process and path handling

cj/git-clone-clipboard-url shelled out via shell-command and derived the clone directory with file-name-nondirectory, which mishandles scp-style SSH URLs with no slash (git@host:repo.git became git@host:repo). It also ran git in default-directory and only checked whether the clone dir appeared afterward, so a failed clone was silent.

The clone now runs as a direct git process (call-process, no shell) with clone -- url dir so a URL beginning with - cannot be read as a flag. The destination path comes from cj/--git-clone-dir-name, which takes the last component splitting on / and :, handling HTTPS, scp-style and ssh:// SSH, and local paths. It validates the clipboard is non-empty and the target is a writable directory that does not already contain the destination, and surfaces a non-zero git exit as a user-error with the *git-clone* output. Tests cover the deriver across URL schemes plus the empty-clipboard and clone-failure paths.
---
 tests/test-vc-config--git-clone.el | 88 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)
 create mode 100644 tests/test-vc-config--git-clone.el

(limited to 'tests/test-vc-config--git-clone.el')

diff --git a/tests/test-vc-config--git-clone.el b/tests/test-vc-config--git-clone.el
new file mode 100644
index 00000000..3b39ece2
--- /dev/null
+++ b/tests/test-vc-config--git-clone.el
@@ -0,0 +1,88 @@
+;;; test-vc-config--git-clone.el --- Tests for clipboard git-clone hardening -*- lexical-binding: t; -*-
+
+;;; Commentary:
+;; Unit tests for cj/--git-clone-dir-name (robust repo-dir derivation across
+;; HTTPS, scp-style SSH, ssh:// and local URLs) and for cj/git-clone-clipboard-url
+;; reporting a failed clone from the process exit status instead of silently
+;; assuming the directory appeared.
+
+;;; Code:
+
+(require 'ert)
+(require 'cl-lib)
+
+(add-to-list 'load-path (expand-file-name "modules" user-emacs-directory))
+(require 'vc-config)
+
+;;; cj/--git-clone-dir-name — Normal Cases
+
+(ert-deftest test-vc-git-clone-dir-name-https-with-git-suffix ()
+  "Normal: an HTTPS URL with a .git suffix yields the bare repo name."
+  (should (equal "repo"
+                 (cj/--git-clone-dir-name "https://example.com/user/repo.git"))))
+
+(ert-deftest test-vc-git-clone-dir-name-https-without-git-suffix ()
+  "Normal: an HTTPS URL without a .git suffix yields the bare repo name."
+  (should (equal "repo"
+                 (cj/--git-clone-dir-name "https://example.com/user/repo"))))
+
+(ert-deftest test-vc-git-clone-dir-name-ssh-scp-with-user ()
+  "Normal: scp-style SSH with a user path yields the repo name."
+  (should (equal "repo"
+                 (cj/--git-clone-dir-name "git@example.com:user/repo.git"))))
+
+(ert-deftest test-vc-git-clone-dir-name-ssh-url-scheme ()
+  "Normal: an ssh:// URL yields the repo name."
+  (should (equal "repo"
+                 (cj/--git-clone-dir-name "ssh://git@example.com/user/repo.git"))))
+
+;;; Boundary Cases
+
+(ert-deftest test-vc-git-clone-dir-name-ssh-scp-without-user ()
+  "Boundary: scp-style SSH with no user path (host:repo.git) still works.
+This is the case the old file-name-nondirectory derivation got wrong,
+since there is no `/' separator."
+  (should (equal "repo"
+                 (cj/--git-clone-dir-name "git@example.com:repo.git"))))
+
+(ert-deftest test-vc-git-clone-dir-name-local-path ()
+  "Boundary: a local filesystem path yields the repo name."
+  (should (equal "repo"
+                 (cj/--git-clone-dir-name "/home/me/src/repo.git"))))
+
+(ert-deftest test-vc-git-clone-dir-name-trailing-slash ()
+  "Boundary: a trailing slash does not swallow the repo name."
+  (should (equal "repo"
+                 (cj/--git-clone-dir-name "https://example.com/user/repo.git/"))))
+
+(ert-deftest test-vc-git-clone-dir-name-surrounding-whitespace ()
+  "Boundary: clipboard whitespace around the URL is trimmed."
+  (should (equal "repo"
+                 (cj/--git-clone-dir-name "  https://example.com/user/repo.git\n"))))
+
+;;; cj/git-clone-clipboard-url — Error Cases
+
+(ert-deftest test-vc-git-clone-clipboard-url-reports-clone-failure ()
+  "Error: a nonzero git exit status surfaces a user-error, not silence.
+Uses a real writable temp dir as the target (so the file predicates run
+for real) and mocks only the clone process to fail."
+  (let ((target (make-temp-file "cj-clone-fail-" t)))
+    (unwind-protect
+        (cl-letf (((symbol-function 'call-process) (lambda (&rest _) 128))
+                  ((symbol-function 'pop-to-buffer) #'ignore)
+                  ((symbol-function 'message) #'ignore))
+          (should-error
+           (cj/git-clone-clipboard-url "https://example.com/user/repo.git" target)
+           :type 'user-error))
+      (delete-directory target t))))
+
+(ert-deftest test-vc-git-clone-clipboard-url-empty-clipboard-errors ()
+  "Error: an empty clipboard URL aborts before any clone attempt."
+  (let ((cloned nil))
+    (cl-letf (((symbol-function 'call-process)
+               (lambda (&rest _) (setq cloned t) 0)))
+      (should-error (cj/git-clone-clipboard-url "   " "/tmp") :type 'user-error))
+    (should-not cloned)))
+
+(provide 'test-vc-config--git-clone)
+;;; test-vc-config--git-clone.el ends here
-- 
cgit v1.2.3