fix(updates): preserve SSH origin pointing at the right repo

The in-app Apply Update path unconditionally ran `git remote set-url
  origin https://github.com/maziggy/bambuddy.git` before fetching, on
  the theory that systemd service users wouldn't have SSH keys. True
  in production, but it also clobbered every developer's SSH origin
  the moment they tested the upgrade flow against their own checkout.
  Next `git push` then prompted for HTTPS credentials and bounced.

  New behaviour: read `origin` first via `git remote get-url`, parse
  out the (owner, repo) pair using a small helper that handles all
  four canonical forms (git@github.com:owner/repo[.git] and
  https://github.com/owner/repo[.git]), and only rewrite if it doesn't
  already resolve to maziggy/bambuddy. Native installs with no remote
  or pointing at a fork still get reset to the canonical HTTPS URL.

  Three new regression tests in test_updates_api.py:
    - parser accepts SSH/HTTPS, with/without .git, rejects non-GitHub
    - SSH origin pointing at maziggy/bambuddy is preserved (the
      developer-footgun case)
    - origin pointing at a fork still gets rewritten to HTTPS (the
      original behaviour we don't want to lose)

backend/app/api/routes/updates.py

@@ -78,6 +78,78 @@ def _find_executable(name: str) -> str | None:
     return None
 
 
+def _parse_github_remote(url: str) -> tuple[str, str] | None:
+    """Extract `(owner, repo)` from a GitHub remote URL, or None if it isn't a
+    GitHub URL we recognise.
+
+    Handles the four forms `git remote -v` typically prints:
+      - `git@github.com:owner/repo.git`         (SSH, the dev default)
+      - `git@github.com:owner/repo`             (SSH without .git suffix)
+      - `https://github.com/owner/repo.git`     (HTTPS, what _perform_update sets)
+      - `https://github.com/owner/repo`         (HTTPS without .git)
+
+    Anything else (a fork URL, a different host, a malformed value, the empty
+    string from a missing origin) returns None so the caller treats it as
+    "not pointing at our repo" and resets it.
+    """
+    s = url.strip()
+    if not s:
+        return None
+    # SSH form: git@github.com:owner/repo[.git]
+    ssh_prefix = "git@github.com:"
+    https_prefix_a = "https://github.com/"
+    https_prefix_b = "http://github.com/"  # tolerated for legacy
+    if s.startswith(ssh_prefix):
+        path = s[len(ssh_prefix) :]
+    elif s.startswith(https_prefix_a):
+        path = s[len(https_prefix_a) :]
+    elif s.startswith(https_prefix_b):
+        path = s[len(https_prefix_b) :]
+    else:
+        return None
+    if path.endswith(".git"):
+        path = path[:-4]
+    parts = path.strip("/").split("/")
+    if len(parts) != 2 or not parts[0] or not parts[1]:
+        return None
+    return (parts[0], parts[1])
+
+
+async def _origin_points_at_repo(git_path: str, git_config: list[str], base_dir, expected_repo: str) -> bool:
+    """Return True iff the working tree's `origin` already resolves to
+    `<owner>/<repo>` matching `expected_repo` (e.g. "maziggy/bambuddy"),
+    regardless of whether it's the SSH or HTTPS form. Used to skip the
+    `git remote set-url origin https://...` rewrite when the developer's
+    SSH origin is already correct — see `_perform_update` for context."""
+    try:
+        process = await asyncio.create_subprocess_exec(
+            git_path,
+            *git_config,
+            "remote",
+            "get-url",
+            "origin",
+            cwd=str(base_dir),
+            stdout=asyncio.subprocess.PIPE,
+            stderr=asyncio.subprocess.PIPE,
+        )
+        stdout, _ = await process.communicate()
+    except (OSError, asyncio.CancelledError):
+        # Fail closed: let the caller go through the rewrite branch if we
+        # can't even invoke git. The unconditional set-url is the safer
+        # fallback, only mildly destructive.
+        return False
+    if process.returncode != 0:
+        # Most likely cause: no `origin` defined yet (fresh clone-style
+        # checkout). Caller will set it.
+        return False
+    parsed = _parse_github_remote(stdout.decode().strip())
+    if parsed is None:
+        return False
+    owner, repo = parsed
+    expected_owner, expected_repo_name = expected_repo.split("/", 1)
+    return owner == expected_owner and repo == expected_repo_name
+
+
 def parse_version(version: str) -> tuple:
     """Parse version string into tuple for comparison.
 
@@ -341,20 +413,32 @@ async def _perform_update():
             "error": None,
         }
 
-        # Ensure remote uses HTTPS (SSH may not be available)
+        # Ensure remote points at the expected repo. We previously rewrote
+        # origin to HTTPS unconditionally on the assumption that systemd
+        # service users wouldn't have SSH keys configured — which is fine
+        # for that case, but stomps on developer checkouts where origin is
+        # legitimately `git@github.com:maziggy/bambuddy.git` and the user
+        # auths via SSH keys. After the rewrite, `git push` prompts for
+        # HTTPS credentials and fails.
+        # New behaviour: read the current origin, parse out the
+        # `<owner>/<repo>` pair, and only rewrite if it doesn't already
+        # resolve to the right GitHub repo. SSH origins pointing at the
+        # correct repo are preserved; only missing / wrong / corrupted
+        # origins get reset to HTTPS.
         https_url = f"https://github.com/{GITHUB_REPO}.git"
-        process = await asyncio.create_subprocess_exec(
-            git_path,
-            *git_config,
-            "remote",
-            "set-url",
-            "origin",
-            https_url,
-            cwd=str(base_dir),
-            stdout=asyncio.subprocess.PIPE,
-            stderr=asyncio.subprocess.PIPE,
-        )
-        await process.communicate()
+        if not await _origin_points_at_repo(git_path, git_config, base_dir, GITHUB_REPO):
+            process = await asyncio.create_subprocess_exec(
+                git_path,
+                *git_config,
+                "remote",
+                "set-url",
+                "origin",
+                https_url,
+                cwd=str(base_dir),
+                stdout=asyncio.subprocess.PIPE,
+                stderr=asyncio.subprocess.PIPE,
+            )
+            await process.communicate()
 
         _update_status = {
             "status": "downloading",

backend/tests/integration/test_updates_api.py

@@ -47,6 +47,135 @@ class TestUpdatesAPI:
 
         assert is_newer_version("0.1.5", "0.1.5b7") is True
 
+    def test_parse_github_remote_recognises_ssh_https_and_dotgit(self):
+        """`_parse_github_remote` must accept the four canonical forms `git
+        remote -v` prints; anything else returns None so callers can treat
+        it as 'reset to expected URL'."""
+        from backend.app.api.routes.updates import _parse_github_remote
+
+        assert _parse_github_remote("git@github.com:maziggy/bambuddy.git") == (
+            "maziggy",
+            "bambuddy",
+        )
+        assert _parse_github_remote("git@github.com:maziggy/bambuddy") == (
+            "maziggy",
+            "bambuddy",
+        )
+        assert _parse_github_remote("https://github.com/maziggy/bambuddy.git") == (
+            "maziggy",
+            "bambuddy",
+        )
+        assert _parse_github_remote("https://github.com/maziggy/bambuddy") == (
+            "maziggy",
+            "bambuddy",
+        )
+        # Non-GitHub host → None (we don't claim ownership over arbitrary
+        # forge URLs).
+        assert _parse_github_remote("git@gitlab.com:maziggy/bambuddy.git") is None
+        # Empty / malformed → None.
+        assert _parse_github_remote("") is None
+        assert _parse_github_remote("not-a-url") is None
+        assert _parse_github_remote("https://github.com/maziggy") is None  # no /repo
+
+    @pytest.mark.asyncio
+    async def test_perform_update_preserves_ssh_origin_when_pointing_at_correct_repo(self, tmp_path):
+        """Regression for the developer-checkout footgun: if origin already
+        points at github.com/maziggy/bambuddy via SSH, the updater must
+        leave it alone instead of clobbering it with HTTPS. Pre-fix, every
+        Apply Update click rewrote `git@github.com:...` to `https://...`,
+        breaking subsequent `git push` for any developer testing the
+        upgrade flow against their own checkout."""
+        from backend.app.api.routes import updates as updates_module
+
+        app_dir = tmp_path / "app"
+        data_dir = tmp_path / "app" / "data"
+        app_dir.mkdir()
+        data_dir.mkdir()
+        (app_dir / "requirements.txt").write_text("fastapi\n")
+
+        calls: list[dict] = []
+
+        async def fake_create_subprocess_exec(*args, **kwargs):
+            calls.append({"args": args, "cwd": kwargs.get("cwd")})
+            proc = MagicMock()
+            # When the updater asks `git remote get-url origin`, return the
+            # SSH URL. Every other subprocess returns successfully with no
+            # output.
+            if "get-url" in args and "origin" in args:
+                proc.communicate = AsyncMock(return_value=(b"git@github.com:maziggy/bambuddy.git\n", b""))
+            else:
+                proc.communicate = AsyncMock(return_value=(b"", b""))
+            proc.returncode = 0
+            return proc
+
+        with (
+            patch.object(updates_module.settings, "base_dir", data_dir),
+            patch.object(updates_module.settings, "app_dir", app_dir),
+            patch.object(updates_module, "_find_executable", return_value="/usr/bin/git"),
+            patch.object(
+                updates_module.asyncio,
+                "create_subprocess_exec",
+                side_effect=fake_create_subprocess_exec,
+            ),
+        ):
+            await updates_module._perform_update()
+
+        # The updater MUST NOT have run `git remote set-url origin <https>`
+        # because origin already pointed at the right repo over SSH.
+        set_url_calls = [c for c in calls if "set-url" in c["args"] and "origin" in c["args"]]
+        assert not set_url_calls, (
+            "Updater clobbered an SSH origin pointing at the correct repo. "
+            "Captured set-url calls: " + repr([c["args"] for c in set_url_calls])
+        )
+
+    @pytest.mark.asyncio
+    async def test_perform_update_resets_origin_when_pointing_elsewhere(self, tmp_path):
+        """Defensive: if origin points at a fork or unrelated repo (or is
+        missing), the updater should still rewrite it to the canonical
+        HTTPS URL so subsequent fetch / reset works against the right
+        repo. This is the original behaviour that the SSH-preservation
+        fix above must NOT regress."""
+        from backend.app.api.routes import updates as updates_module
+        from backend.app.core.config import GITHUB_REPO
+
+        app_dir = tmp_path / "app"
+        data_dir = tmp_path / "app" / "data"
+        app_dir.mkdir()
+        data_dir.mkdir()
+        (app_dir / "requirements.txt").write_text("fastapi\n")
+
+        calls: list[dict] = []
+
+        async def fake_create_subprocess_exec(*args, **kwargs):
+            calls.append({"args": args, "cwd": kwargs.get("cwd")})
+            proc = MagicMock()
+            # origin is set to a fork — must be rewritten.
+            if "get-url" in args and "origin" in args:
+                proc.communicate = AsyncMock(return_value=(b"git@github.com:somefork/bambuddy.git\n", b""))
+            else:
+                proc.communicate = AsyncMock(return_value=(b"", b""))
+            proc.returncode = 0
+            return proc
+
+        with (
+            patch.object(updates_module.settings, "base_dir", data_dir),
+            patch.object(updates_module.settings, "app_dir", app_dir),
+            patch.object(updates_module, "_find_executable", return_value="/usr/bin/git"),
+            patch.object(
+                updates_module.asyncio,
+                "create_subprocess_exec",
+                side_effect=fake_create_subprocess_exec,
+            ),
+        ):
+            await updates_module._perform_update()
+
+        set_url_calls = [c for c in calls if "set-url" in c["args"] and "origin" in c["args"]]
+        assert set_url_calls, "Updater must rewrite origin when it points at a fork."
+        rewritten_to = set_url_calls[0]["args"][-1]
+        assert rewritten_to == f"https://github.com/{GITHUB_REPO}.git", (
+            f"Expected origin to be reset to canonical HTTPS URL; got: {rewritten_to}"
+        )
+
     @pytest.mark.asyncio
     async def test_perform_update_runs_pip_in_app_dir_not_data_dir(self, tmp_path):
         """Native install: `requirements.txt` lives at INSTALL_PATH (the source-