From cca681979a11689f58e34731ee6b3620bfc4b789 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Jul 2026 04:03:01 +0000 Subject: [PATCH 1/3] Initial plan From 2f744c0850c3e25c16a7b73e7d26b8c0d7d662e0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Jul 2026 04:07:53 +0000 Subject: [PATCH 2/3] =?UTF-8?q?fix:=20resolve=20final=20PR=20#337=20blocke?= =?UTF-8?q?rs=20=E2=80=94=20credential=20access,=20amount=20validation,=20?= =?UTF-8?q?wording?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Blocker 1: run nwc-lnurl.service as albyhub user/group so it can read /var/lib/albyhub/unlock-password (mode 0600, dir mode 0700). Remove the now-unused nwc-lnurl user, group, and /var/lib/nwc-lnurl state directory. ReadOnlyPaths updated to allow the whole albyhub dir. Blocker 2: require exactly one amount query parameter in the LNURL callback HTTP handler. Duplicate values now return a 400 protocol error ("Exactly one amount parameter is required") before the helper is called. The missing-amount and non-integer paths are unchanged. Blocker 3: partial-funding failure message now reads "was created successfully" instead of "already exists" to avoid confusion with a duplicate-name error, while retaining the warning not to recreate. Tests added: - LnurlHandlerAmountTests — HTTP-handler level tests for duplicate (2×, 3×), single-valid, and missing amount parameters. - test_create_partial_failure_message_says_created_successfully — asserts exact wording of the partial-funding message. - test_nwc_lnurl_service_runs_as_albyhub — asserts the Nix service block sets User/Group to albyhub. - test_nwc_module_no_separate_nwc_lnurl_user — asserts no standalone nwc-lnurl user/group is declared. All 229 Python tests pass (1 skipped). JS syntax clean. No secrets. --- app/sovran_systemsos_web/nwc_hub_manager.py | 2 +- app/sovran_systemsos_web/nwc_lnurl_service.py | 16 ++- app/tests/test_wallet_connections.py | 126 ++++++++++++++++++ modules/nwc-wallets.nix | 16 +-- 4 files changed, 144 insertions(+), 16 deletions(-) diff --git a/app/sovran_systemsos_web/nwc_hub_manager.py b/app/sovran_systemsos_web/nwc_hub_manager.py index 4ced9cc..2dd337d 100644 --- a/app/sovran_systemsos_web/nwc_hub_manager.py +++ b/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." ) diff --git a/app/sovran_systemsos_web/nwc_lnurl_service.py b/app/sovran_systemsos_web/nwc_lnurl_service.py index ab78a74..27d03fa 100644 --- a/app/sovran_systemsos_web/nwc_lnurl_service.py +++ b/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_str = amount_list[0] if amount_list else None + amount_values = qs.get("amount") + if not amount_values: + amount_str = None + elif len(amount_values) != 1: + self._send_json( + 400, + { + "status": "ERROR", + "reason": "Exactly one 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 diff --git a/app/tests/test_wallet_connections.py b/app/tests/test_wallet_connections.py index 758c63d..c4bc858 100644 --- a/app/tests/test_wallet_connections.py +++ b/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("Exactly one", 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 ───────────────────────────────── diff --git a/modules/nwc-wallets.nix b/modules/nwc-wallets.nix index 76014b8..86d8a69 100644 --- a/modules/nwc-wallets.nix +++ b/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"; - Group = "nwc-lnurl"; + User = "albyhub"; + Group = "albyhub"; ExecStart = "${config.services.sovranHub.webPackage}/bin/nwc-lnurl"; Restart = "on-failure"; RestartSec = "10s"; @@ -135,7 +125,7 @@ lib.mkIf config.sovran_systemsOS.features."nwc-wallets" { ProtectSystem = "strict"; ReadOnlyPaths = [ "/var/lib/domains/lightning" - "/var/lib/albyhub/unlock-password" + "/var/lib/albyhub" ]; }; }; From ecd83a82622a8949cc05ecbb2809faeab9571874 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Jul 2026 04:09:41 +0000 Subject: [PATCH 3/3] =?UTF-8?q?fix:=20address=20code=20review=20=E2=80=94?= =?UTF-8?q?=20restrict=20ReadOnlyPaths=20to=20unlock-password,=20clarify?= =?UTF-8?q?=20amount=20error=20message?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ReadOnlyPaths for nwc-lnurl.service now lists only the specific file /var/lib/albyhub/unlock-password (least-privilege) instead of the whole /var/lib/albyhub directory. Amount-duplicate error message changed to "A single amount parameter is required" (clearer for 2+ values than "Exactly one"). Test assertion updated to match new message text. --- app/sovran_systemsos_web/nwc_lnurl_service.py | 2 +- app/tests/test_wallet_connections.py | 2 +- modules/nwc-wallets.nix | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/sovran_systemsos_web/nwc_lnurl_service.py b/app/sovran_systemsos_web/nwc_lnurl_service.py index 27d03fa..f6d358d 100644 --- a/app/sovran_systemsos_web/nwc_lnurl_service.py +++ b/app/sovran_systemsos_web/nwc_lnurl_service.py @@ -194,7 +194,7 @@ def _make_handler(manager: "AlbyHubManager") -> type: 400, { "status": "ERROR", - "reason": "Exactly one amount parameter is required", + "reason": "A single amount parameter is required", }, ) return diff --git a/app/tests/test_wallet_connections.py b/app/tests/test_wallet_connections.py index c4bc858..5012778 100644 --- a/app/tests/test_wallet_connections.py +++ b/app/tests/test_wallet_connections.py @@ -1046,7 +1046,7 @@ class LnurlHandlerAmountTests(unittest.TestCase): code, body = sent[0] self.assertEqual(code, 400) self.assertEqual(body["status"], "ERROR") - self.assertIn("Exactly one", body["reason"]) + 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") diff --git a/modules/nwc-wallets.nix b/modules/nwc-wallets.nix index 86d8a69..e19ab5c 100644 --- a/modules/nwc-wallets.nix +++ b/modules/nwc-wallets.nix @@ -125,7 +125,7 @@ lib.mkIf config.sovran_systemsOS.features."nwc-wallets" { ProtectSystem = "strict"; ReadOnlyPaths = [ "/var/lib/domains/lightning" - "/var/lib/albyhub" + "/var/lib/albyhub/unlock-password" ]; }; };