Merge pull request #338 from naturallaw777/copilot/copiloturgent-fix-alby-hub-api

fix(nwc): credential access, strict LNURL amount validation, partial-funding wording

app/sovran_systemsos_web/nwc_hub_manager.py

@@ -505,7 +505,7 @@ class AlbyHubManager:
             except AlbyHubError as exc:
                 funding_result["error"] = exc.code
                 funding_result["message"] = (
-                    "The wallet already exists and the NWC connection secret is shown "
+                    "The wallet was created successfully and the NWC connection secret is shown "
                     "above, but initial funding failed. Save the NWC secret now. "
                     "Do not recreate this wallet."
                 )

app/sovran_systemsos_web/nwc_lnurl_service.py

@@ -186,8 +186,20 @@ def _make_handler(manager: "AlbyHubManager") -> type:
             m = re.fullmatch(r"/lnurlp/([^/]+)/callback", path)
             if m:
                 alias = urllib.parse.unquote(m.group(1))
-                amount_list = qs.get("amount")
+                amount_values = qs.get("amount")
-                amount_str = amount_list[0] if amount_list else None
+                if not amount_values:
+                    amount_str = None
+                elif len(amount_values) != 1:
+                    self._send_json(
+                        400,
+                        {
+                            "status": "ERROR",
+                            "reason": "A single amount parameter is required",
+                        },
+                    )
+                    return
+                else:
+                    amount_str = amount_values[0]
                 payload, code = _lnurl_callback(alias, amount_str, self._manager)
                 self._send_json(code, payload)
                 return

app/tests/test_wallet_connections.py

@@ -597,6 +597,33 @@ class ManagerCreateTests(unittest.TestCase):
         self.assertFalse(result["result"]["funding"]["success"])
         self.assertIn("message", result["result"]["funding"])
 
+    def test_create_partial_failure_message_says_created_successfully(self):
+        """Partial-funding message must say 'was created successfully', not 'already exists'."""
+        m = self._mgr()
+
+        def _request(method, path, **kw):
+            if method == "GET" and path.startswith("/api/apps?"):
+                return []
+            if method == "POST" and path == "/api/apps":
+                return {
+                    "id": 99,
+                    "pairingUri": "nostr+walletconnect://pubkey?relay=r&secret=S",
+                    **_make_app(id_=99, alias="new"),
+                }
+            if method == "POST" and path == "/api/transfers":
+                raise mgr.AlbyHubError("transfer_failed", "Insufficient funds")
+            if method == "GET" and "/api/v2/apps/99" in path:
+                return _make_app(id_=99, alias="new")
+            return {}
+
+        m._request = MagicMock(side_effect=_request)
+        result = m.create_wallet("W", "new", "send_receive_limited", 5000)
+
+        message = result["result"]["funding"]["message"]
+        self.assertIn("was created successfully", message)
+        self.assertNotIn("already exists", message)
+        self.assertIn("Do not recreate", message)
+
 
 class ManagerDrainTests(unittest.TestCase):
     def _mgr(self, app, transfer_ok=True):
@@ -986,6 +1013,85 @@ class LnurlCallbackTests(unittest.TestCase):
         self.assertEqual(code, 502)
 
 
+# ── LNURL HTTP handler tests ──────────────────────────────────────
+
+
+class LnurlHandlerAmountTests(unittest.TestCase):
+    """Tests the HTTP handler layer for amount parameter validation.
+
+    Uses the handler's do_GET directly with _send_json patched on the instance
+    so no real socket is needed.
+    """
+
+    def _run_handler(self, path: str, manager=None) -> list[tuple[int, dict]]:
+        """Invoke do_GET for *path* and return all (code, body) pairs sent."""
+        from sovran_systemsos_web.nwc_lnurl_service import _make_handler
+
+        if manager is None:
+            manager = _fresh_manager()
+        handler_class = _make_handler(manager)
+        sent: list[tuple[int, dict]] = []
+        handler = handler_class.__new__(handler_class)
+        handler._manager = manager
+        handler.path = path
+        # Intercept output without a real socket
+        handler._send_json = lambda code, body: sent.append((code, body))
+        handler.do_GET()
+        return sent
+
+    def test_duplicate_amount_returns_400_with_protocol_error(self):
+        """Two amount values must be rejected at the HTTP handler level."""
+        sent = self._run_handler("/lnurlp/alice/callback?amount=1000&amount=500000")
+        self.assertEqual(len(sent), 1)
+        code, body = sent[0]
+        self.assertEqual(code, 400)
+        self.assertEqual(body["status"], "ERROR")
+        self.assertIn("single amount", body["reason"])
+
+    def test_three_amount_values_returns_400(self):
+        sent = self._run_handler("/lnurlp/alice/callback?amount=1000&amount=2000&amount=3000")
+        self.assertEqual(len(sent), 1)
+        code, body = sent[0]
+        self.assertEqual(code, 400)
+        self.assertEqual(body["status"], "ERROR")
+
+    def test_single_amount_passes_to_callback(self):
+        """A single valid amount must reach the callback helper (not short-circuit)."""
+        app = _make_app(alias="alice")
+        m = _fresh_manager()
+        m._token = "tok"
+
+        def _request(method, path, **kw):
+            if path.startswith("/api/apps"):
+                return {"apps": [app], "totalCount": 1}
+            if path == "/api/invoices":
+                body = kw.get("body") or {}
+                return {"invoice": "lnbc1000n1pfake", "appId": body.get("appId")}
+            return {}
+
+        m._request = MagicMock(side_effect=_request)
+        with patch("sovran_systemsos_web.nwc_lnurl_service._read_domain", return_value="pay.example.com"):
+            sent = self._run_handler("/lnurlp/alice/callback?amount=1000", manager=m)
+        self.assertEqual(len(sent), 1)
+        code, _ = sent[0]
+        self.assertEqual(code, 200)
+
+    def test_missing_amount_returns_400_missing_reason(self):
+        """No amount parameter must produce a 'Missing amount' error."""
+        app = _make_app(alias="alice")
+        m = _fresh_manager()
+        m._token = "tok"
+        m._request = MagicMock(side_effect=lambda method, path, **kw: (
+            {"apps": [app], "totalCount": 1} if path.startswith("/api/apps") else {}
+        ))
+        with patch("sovran_systemsos_web.nwc_lnurl_service._read_domain", return_value="pay.example.com"):
+            sent = self._run_handler("/lnurlp/alice/callback", manager=m)
+        self.assertEqual(len(sent), 1)
+        code, body = sent[0]
+        self.assertEqual(code, 400)
+        self.assertIn("Missing", body["reason"])
+
+
 # ── Nix/Patch contract tests ───────────────────────────────────────
 
 
@@ -1020,6 +1126,26 @@ class NixPatchContractTests(unittest.TestCase):
         self.assertIn("AppId       *uint  `json:\"appId\"`", text)
         self.assertIn("CreateInvoice(ctx context.Context, amount uint64, description string, appId *uint)", text)
 
+    def test_nwc_lnurl_service_runs_as_albyhub(self):
+        """nwc-lnurl.service must run as albyhub to read /var/lib/albyhub/unlock-password."""
+        repo_root = Path(__file__).resolve().parents[2]
+        module_path = repo_root / "modules" / "nwc-wallets.nix"
+        text = module_path.read_text()
+        # Locate the nwc-lnurl service block
+        service_idx = text.find("systemd.services.nwc-lnurl")
+        self.assertNotEqual(service_idx, -1, "nwc-lnurl service declaration not found")
+        service_section = text[service_idx:]
+        self.assertIn('User = "albyhub"', service_section)
+        self.assertIn('Group = "albyhub"', service_section)
+
+    def test_nwc_module_no_separate_nwc_lnurl_user(self):
+        """No standalone nwc-lnurl user or group should exist; albyhub identity is reused."""
+        repo_root = Path(__file__).resolve().parents[2]
+        module_path = repo_root / "modules" / "nwc-wallets.nix"
+        text = module_path.read_text()
+        self.assertNotIn("users.users.nwc-lnurl", text)
+        self.assertNotIn("users.groups.nwc-lnurl", text)
+
 
 # ── Server API integration tests ─────────────────────────────────
 

modules/nwc-wallets.nix

@@ -44,18 +44,8 @@ lib.mkIf config.sovran_systemsOS.features."nwc-wallets" {
     extraGroups = [ ];
   };
 
-  users.groups.nwc-lnurl = { };
-  users.users.nwc-lnurl = {
-    isSystemUser = true;
-    group = "nwc-lnurl";
-    home = "/var/lib/nwc-lnurl";
-    createHome = false;
-    extraGroups = [ "albyhub" ];
-  };
-
   systemd.tmpfiles.rules = [
     "d /var/lib/albyhub       0700 albyhub albyhub -"
-    "d /var/lib/nwc-lnurl     0750 nwc-lnurl nwc-lnurl -"
   ];
 
   services.lnd.macaroons.albyhub = {
@@ -123,8 +113,8 @@ lib.mkIf config.sovran_systemsOS.features."nwc-wallets" {
 
     serviceConfig = {
       Type = "simple";
-      User = "nwc-lnurl";
+      User = "albyhub";
-      Group = "nwc-lnurl";
+      Group = "albyhub";
       ExecStart = "${config.services.sovranHub.webPackage}/bin/nwc-lnurl";
       Restart = "on-failure";
       RestartSec = "10s";