From f4e0bcf73ddb864d6a24b84e5cd5fdf07d5e5d34 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 10 Jun 2026 16:44:00 +0000 Subject: [PATCH] fix(clink): spec-conformance hardening (k1 length, frequency, description, GFY detail) Follow-ups from the line-by-line spec audit, scoped to the consume-only client: - NDebit.parse rejects a TLV-3 session id that isn't exactly 32 bytes (64 hex), per clink-debits: a wrong-length k1 is a malformed session pointer. - DebitClient.requestBudget validates frequency.unit is one of day/week/month (DebitFrequency.VALID_UNITS) instead of sending a unit a node service will GFY. - OfferClient caps the invoice description at 100 chars per clink-offers. - DebitResponse.failureDetail() composes the GFY error with its actionable extra (allowed range for code 5, retry_after for code 4); the debit zap path now surfaces that instead of the bare error string. Adds regression tests for each (malformed-k1 rejection, invalid-unit throw, description truncation, failureDetail range/retry_after). https://claude.ai/code/session_01NM2TyJtosLdY5ycjyabSRS --- .../amethyst/service/ZapPaymentHandler.kt | 2 +- .../experimental/clink/client/DebitClient.kt | 5 ++++ .../experimental/clink/client/OfferClient.kt | 4 +++- .../clink/debits/DebitMessages.kt | 22 +++++++++++++++++ .../experimental/clink/pointers/NDebit.kt | 4 ++++ .../clink/ClinkClientServerTest.kt | 24 +++++++++++++++++++ .../experimental/clink/ClinkWireShapeTest.kt | 13 ++++++++++ .../clink/pointers/ClinkPointerTest.kt | 9 +++++++ 8 files changed, 81 insertions(+), 2 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/ZapPaymentHandler.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/ZapPaymentHandler.kt index e25e8946a7..9c250325d5 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/ZapPaymentHandler.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/ZapPaymentHandler.kt @@ -416,7 +416,7 @@ class ZapPaymentHandler( if (!paid) { onError( stringRes(context, R.string.error_dialog_pay_invoice_error), - response?.error?.takeIf { it.isNotBlank() } + response?.failureDetail() ?: stringRes(context, R.string.clink_debit_no_response), payable.info.user, ) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/clink/client/DebitClient.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/clink/client/DebitClient.kt index 8ede27b411..b6b5fd2704 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/clink/client/DebitClient.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/clink/client/DebitClient.kt @@ -69,6 +69,11 @@ class DebitClient( description: String? = null, createdAt: Long = TimeUtils.now(), ): DebitEvent { + // Per spec the recurring cadence unit is one of day/week/month; reject anything else + // rather than sending a request a node service will GFY. + require(frequency == null || frequency.unit in DebitFrequency.VALID_UNITS) { + "Invalid budget frequency unit: ${frequency?.unit}" + } val request = DebitRequest( pointer = pointer.pointer, diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/clink/client/OfferClient.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/clink/client/OfferClient.kt index 6fad6ba34a..cd7fc87edf 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/clink/client/OfferClient.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/clink/client/OfferClient.kt @@ -71,7 +71,9 @@ class OfferClient( payer_data = payerData, zap = zap, expires_in_seconds = expiresInSeconds, - description = description, + // The spec caps the invoice description at 100 chars; trim so an over-long + // value doesn't get the whole request rejected by the service. + description = description?.take(100), ) return OfferEvent.createRequest(request, servicePubKey, signer, createdAt) } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/clink/debits/DebitMessages.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/clink/debits/DebitMessages.kt index 32af44338c..8d4fd67381 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/clink/debits/DebitMessages.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/clink/debits/DebitMessages.kt @@ -47,6 +47,9 @@ class DebitFrequency( const val UNIT_DAY = "day" const val UNIT_WEEK = "week" const val UNIT_MONTH = "month" + + /** The only cadence units the spec defines. */ + val VALID_UNITS = setOf(UNIT_DAY, UNIT_WEEK, UNIT_MONTH) } } @@ -67,6 +70,25 @@ class DebitResponse( ) : OptimizedSerializable { fun isOk(): Boolean = res == OK + /** + * A failure detail string for a GFY response: the service's [error] plus the actionable + * structured extra the spec attaches per code — the allowed [range] (code 5) or + * [retry_after] timestamp (code 4). Returns null for a success ([isOk]) response. The + * text is protocol-neutral (English, like the service's own `error`), meant to be shown + * as the detail line under a localized "payment failed" title. + */ + fun failureDetail(): String? { + if (isOk()) return null + val base = error?.takeIf { it.isNotBlank() } + val extra = + when { + range != null -> "allowed ${range?.min ?: "?"} to ${range?.max ?: "?"} sats" + retry_after != null -> "retry after $retry_after" + else -> null + } + return listOfNotNull(base, extra?.let { "($it)" }).joinToString(" ").ifBlank { null } + } + companion object { const val OK = "ok" const val GFY = "GFY" diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/clink/pointers/NDebit.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/clink/pointers/NDebit.kt index c8458f0bd7..3e884ed7a9 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/clink/pointers/NDebit.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/clink/pointers/NDebit.kt @@ -71,6 +71,10 @@ data class NDebit( val relays = tlv.asString(ClinkTlv.RELAY)?.mapNotNull { RelayUrlNormalizer.normalizeOrNull(it) } ?: emptyList() val pointer = tlv.firstAsString(ClinkTlv.POINTER) val k1 = tlv.firstAsHex(ClinkTlv.K1) + // A session id (TLV 3) MUST be exactly 32 bytes (64 hex chars) per the spec; a + // wrong-length value means a malformed session pointer, so reject the whole thing + // rather than silently treating it as static or sending a bad k1. + if (k1 != null && k1.length != 64) return null return NDebit(pubKey, relays, pointer, k1) } diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/experimental/clink/ClinkClientServerTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/experimental/clink/ClinkClientServerTest.kt index 4480ffe728..9caef1d4f4 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/experimental/clink/ClinkClientServerTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/experimental/clink/ClinkClientServerTest.kt @@ -20,10 +20,13 @@ */ package com.vitorpamplona.quartz.experimental.clink +import com.vitorpamplona.quartz.experimental.clink.client.DebitClient import com.vitorpamplona.quartz.experimental.clink.client.OfferClient import com.vitorpamplona.quartz.experimental.clink.debits.DebitEvent +import com.vitorpamplona.quartz.experimental.clink.debits.DebitFrequency import com.vitorpamplona.quartz.experimental.clink.offers.OfferEvent import com.vitorpamplona.quartz.experimental.clink.offers.OfferReceipt +import com.vitorpamplona.quartz.experimental.clink.pointers.NDebit import com.vitorpamplona.quartz.experimental.clink.pointers.NOffer import com.vitorpamplona.quartz.experimental.clink.server.ClinkServer import com.vitorpamplona.quartz.experimental.clink.server.K1Tracker @@ -71,6 +74,27 @@ class ClinkClientServerTest { assertEquals("ab".repeat(32), receipt.preimage) } + @Test + fun requestBudgetRejectsInvalidFrequencyUnit() = + kotlinx.coroutines.test.runTest { + val client = DebitClient(NDebit(servicePubKey, listOf(relay), null, null), signer) + kotlin.test.assertFailsWith { + client.requestBudget(1000, DebitFrequency(1, "fortnight")) + } + // valid units do not throw + client.requestBudget(1000, DebitFrequency(1, DebitFrequency.UNIT_MONTH)) + } + + @Test + fun offerRequestTruncatesDescriptionTo100Chars() = + kotlinx.coroutines.test.runTest { + val service = NostrSignerInternal(KeyPair()) + val client = OfferClient(NOffer(service.pubKey, listOf(relay), "o", null, null), signer) + val event = client.requestInvoice(amountSats = 100, description = "x".repeat(150)) + val decrypted = event.decryptRequest(service) + assertEquals(100, decrypted.description?.length) + } + @Test fun serverRequestFilterTargetsRecipientByPTag() { val filter = ClinkServer.debitRequestFilter(servicePubKey, since = 100L) diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/experimental/clink/ClinkWireShapeTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/experimental/clink/ClinkWireShapeTest.kt index d3a2215b77..11182bf76a 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/experimental/clink/ClinkWireShapeTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/experimental/clink/ClinkWireShapeTest.kt @@ -197,6 +197,19 @@ class ClinkWireShapeTest { assertEquals(500000L, res.range?.max) } + @Test + fun debitFailureDetailSurfacesRangeAndRetryAfter() { + val range = parse("""{"res":"GFY","code":5,"error":"Invalid Amount","range":{"min":1000,"max":500000}}""").failureDetail()!! + assertTrue(range.contains("Invalid Amount")) + assertTrue(range.contains("1000") && range.contains("500000")) + + val rateLimited = parse("""{"res":"GFY","code":4,"error":"Rate Limited","retry_after":1750000000}""").failureDetail()!! + assertTrue(rateLimited.contains("Rate Limited")) + assertTrue(rateLimited.contains("1750000000")) + + assertNull(parse("""{"res":"ok"}""").failureDetail()) + } + @Test fun debitGfyInvalidRequest() { val res = parse("""{"res":"GFY","code":6,"error":"Invalid Request: K1 already processed"}""") diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/experimental/clink/pointers/ClinkPointerTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/experimental/clink/pointers/ClinkPointerTest.kt index 1a9204e68d..8b4d2617d0 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/experimental/clink/pointers/ClinkPointerTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/experimental/clink/pointers/ClinkPointerTest.kt @@ -95,6 +95,15 @@ class ClinkPointerTest { assertEquals(debit, parsed) } + @Test + fun debitRejectsMalformedK1Length() { + // A session id (TLV 3) must be exactly 32 bytes; a wrong-length k1 is a malformed + // session pointer and the parser must reject it rather than accept a bad session. + val shortK1 = "abcd" + val encoded = NDebit(pubKey, listOf(relay), null, shortK1).encode() + assertNull(ClinkPointerParser.parse(encoded)) + } + @Test fun manageRoundTrip() { val manage = NManage(pubKey, listOf(relay), null)