mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-05 19:28:25 +00:00
fix(napplet): grant SIGNER to the browser, and never widen a narrow decrypt grant
Two defects found auditing the NIP-44 change. The in-app browser mints its own per-origin launch token and never consulted HostProfile, so it still granted IDENTITY+RELAY. Those are exactly the surfaces that set __nappletNip07, so the shim advertised window.nostr.nip44 and the broker then denied every call -- worse than not advertising it, since apps stop falling back. The website set now lives once, in NappletCapability, and both mints read it. Second, the broker recorded a consent grant under the REQUESTED op rather than the op the grant itself carried. Nip44Decrypt is the first napplet-side request with a narrower alternative (DecryptFrom(peer)), so a user tapping "always allow for Alice" would have been stored as a broad "allow decrypt" -- every conversation, forever, from one tap. Recording now goes through NostrSignerPermissionLedger.record, which uses the grant's own op, and a standing narrow grant is honoured on later requests instead of re-prompting. This mirrors the NIP-46 authorizer, which already got both right. With the recording fixed, the consent dialog can safely name the counterparty: Nip44Decrypt now supplies it, so the prompt reads "read your private messages with Alice" and offers the scoped grant beside the broad one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Hge2jR1BPnyZse75VQ4kg
This commit is contained in:
@@ -277,7 +277,7 @@ class NappletBrokerService : Service() {
|
||||
// Bind to the account active at mint time: a browser token minted for one account must
|
||||
// never sign as another if the user switches while the page is still open.
|
||||
val mintAccount = Amethyst.instance.sessionManager.loggedInAccount() ?: return true
|
||||
val token = NappletLaunchRegistry.register(identity, setOf(NappletCapability.IDENTITY, NappletCapability.RELAY), mintAccount.pubKey)
|
||||
val token = NappletLaunchRegistry.register(identity, NappletCapability.WEBSITE_CAPABILITIES, mintAccount.pubKey)
|
||||
val response =
|
||||
Message.obtain(null, NappletIpc.MSG_BROWSER_TOKEN).apply {
|
||||
this.data =
|
||||
|
||||
@@ -27,6 +27,8 @@ import com.vitorpamplona.amethyst.commons.connectedApps.signers.NostrSignerOp
|
||||
import com.vitorpamplona.amethyst.commons.napplet.NappletCapability
|
||||
import com.vitorpamplona.amethyst.commons.napplet.NappletIdentity
|
||||
import com.vitorpamplona.amethyst.commons.napplet.protocol.NappletRequest
|
||||
import com.vitorpamplona.amethyst.commons.napplet.protocol.counterpartyPubKey
|
||||
import com.vitorpamplona.amethyst.commons.napplet.protocol.toNarrowSignerOp
|
||||
import com.vitorpamplona.amethyst.connectedApps.consent.SignerConnectInfo
|
||||
import com.vitorpamplona.amethyst.connectedApps.consent.SignerConsentInfo
|
||||
import com.vitorpamplona.amethyst.favorites.BrowserIconRegistry
|
||||
@@ -78,7 +80,13 @@ fun buildSignerConsentInfo(
|
||||
} else {
|
||||
resolveNappletMeta(identity.authorPubKey, identity.identifier, untitled)
|
||||
}
|
||||
val summary = op.label(context)
|
||||
// A decrypt grant can be scoped to one conversation: offer "always allow for Alice" next to the
|
||||
// broad "always allow", instead of only the all-conversations-forever choice. Mirrors the NIP-46
|
||||
// dialog, so the same decision reads the same way whichever surface asked.
|
||||
val narrowOp = request.toNarrowSignerOp()
|
||||
val counterparty = request.counterpartyPubKey()
|
||||
// For decrypt this names the counterparty ("read your private messages with Alice").
|
||||
val summary = (narrowOp ?: op).label(context)
|
||||
val preview =
|
||||
when (request) {
|
||||
is NappletRequest.Publish -> request.content.take(160).trim()
|
||||
@@ -140,6 +148,16 @@ fun buildSignerConsentInfo(
|
||||
rawData = rawData,
|
||||
iconUrl = iconUrl,
|
||||
previewTemplate = previewTemplate,
|
||||
counterpartyName = counterparty?.let { counterpartyLabel(it) },
|
||||
counterpartyPicture = counterparty?.let { LocalCache.getUserIfExists(it)?.profilePicture() },
|
||||
counterpartyPubKey = counterparty,
|
||||
narrowOp = narrowOp,
|
||||
// Read the pubkey off the narrow op itself: the dialog drops the button unless BOTH halves
|
||||
// are present, so deriving them from one value keeps them from disagreeing.
|
||||
narrowOpLabel =
|
||||
(narrowOp as? NostrSignerOp.DecryptFrom)?.let {
|
||||
context.getString(R.string.nip46_signer_allow_always_for, counterpartyLabel(it.counterparty))
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
+39
-31
@@ -34,6 +34,7 @@ import com.vitorpamplona.amethyst.commons.napplet.permissions.PermissionDecision
|
||||
import com.vitorpamplona.amethyst.commons.napplet.protocol.NappletRequest
|
||||
import com.vitorpamplona.amethyst.commons.napplet.protocol.NappletResponse
|
||||
import com.vitorpamplona.amethyst.commons.napplet.protocol.NappletStorageScope
|
||||
import com.vitorpamplona.amethyst.commons.napplet.protocol.toNarrowSignerOp
|
||||
import com.vitorpamplona.amethyst.commons.napplet.protocol.toSignerOp
|
||||
import com.vitorpamplona.quartz.nip01Core.core.Event
|
||||
import com.vitorpamplona.quartz.nip01Core.signers.NostrSigner
|
||||
@@ -455,51 +456,58 @@ class NappletBroker(
|
||||
): Boolean =
|
||||
signerConsentLock.withLock {
|
||||
val sl = signerLedger ?: return@withLock true
|
||||
val coordinate = signerCoordinateFor(identity)
|
||||
// A decrypt request also carries a narrower op ("decrypt messages from THIS
|
||||
// counterparty"). A standing narrow grant satisfies it without widening the broad one.
|
||||
val narrowOp = request.toNarrowSignerOp()
|
||||
|
||||
// Session grants win immediately without touching storage. Scoped to this applet: a
|
||||
// grant made for one app never authorizes another.
|
||||
if (sessionKey(signerCoordinateFor(identity), op) in sessionAllows) {
|
||||
sl.updateLastUsed(signerCoordinateFor(identity))
|
||||
if (sessionKey(coordinate, op) in sessionAllows) {
|
||||
sl.updateLastUsed(coordinate)
|
||||
return@withLock true
|
||||
}
|
||||
when (sl.decide(signerCoordinateFor(identity), op)) {
|
||||
when (sl.decide(coordinate, op)) {
|
||||
NostrOpDecision.ALLOW -> {
|
||||
sl.updateLastUsed(signerCoordinateFor(identity))
|
||||
sl.updateLastUsed(coordinate)
|
||||
true
|
||||
}
|
||||
// An explicit DENY on the broad op is final — a narrow grant never overrides it.
|
||||
NostrOpDecision.DENY -> false
|
||||
NostrOpDecision.ASK -> {
|
||||
val prompt = signerConsentPrompt ?: return@withLock true
|
||||
when (val grant = prompt.request(identity, op, request)) {
|
||||
is SignerOpGrant.AllowAll -> {
|
||||
sl.setPolicy(signerCoordinateFor(identity), AppSignerPolicy.FULL_TRUST)
|
||||
sl.updateLastUsed(signerCoordinateFor(identity))
|
||||
true
|
||||
}
|
||||
is SignerOpGrant.AllowForOp -> {
|
||||
sl.setOpDecision(signerCoordinateFor(identity), op, NostrOpDecision.ALLOW)
|
||||
sl.updateLastUsed(signerCoordinateFor(identity))
|
||||
true
|
||||
}
|
||||
is SignerOpGrant.AllowForSession -> {
|
||||
sessionAllows.add(sessionKey(signerCoordinateFor(identity), op))
|
||||
sl.updateLastUsed(signerCoordinateFor(identity))
|
||||
true
|
||||
}
|
||||
is SignerOpGrant.AllowUntil -> {
|
||||
sl.setTimedOpDecision(signerCoordinateFor(identity), op, NostrOpDecision.ALLOW, grant.expiresAt)
|
||||
sl.updateLastUsed(signerCoordinateFor(identity))
|
||||
true
|
||||
}
|
||||
is SignerOpGrant.DenyForOp -> {
|
||||
sl.setOpDecision(signerCoordinateFor(identity), op, NostrOpDecision.DENY)
|
||||
false
|
||||
}
|
||||
else -> grant.isAllowed
|
||||
if (narrowOp != null && isNarrowAllowed(sl, coordinate, narrowOp)) {
|
||||
sl.updateLastUsed(coordinate)
|
||||
return@withLock true
|
||||
}
|
||||
val prompt = signerConsentPrompt ?: return@withLock true
|
||||
val grant = prompt.request(identity, op, request)
|
||||
// Record the GRANT's own op, never the requested one: the dialog may hand back a
|
||||
// narrower op ("only from this counterparty"), and a stored grant must never be
|
||||
// wider than what the user actually tapped.
|
||||
sl.record(coordinate, grant)
|
||||
if (grant is SignerOpGrant.AllowForSession) {
|
||||
sessionAllows.add(sessionKey(coordinate, grant.op))
|
||||
}
|
||||
if (grant.isAllowed) sl.updateLastUsed(coordinate)
|
||||
grant.isAllowed
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* True when a standing or session grant exists for the narrower [narrowOp] (e.g. decrypt-from-X).
|
||||
* Only an explicit per-op override counts: [NostrSignerPermissionLedger.decide] would otherwise
|
||||
* fall through to the app's policy, and FULL_TRUST/REASONABLE would answer for an op nobody ever
|
||||
* granted. Mirrors the NIP-46 authorizer so both surfaces honour a narrow grant identically.
|
||||
*/
|
||||
private suspend fun isNarrowAllowed(
|
||||
sl: NostrSignerPermissionLedger,
|
||||
coordinate: String,
|
||||
narrowOp: NostrSignerOp,
|
||||
): Boolean =
|
||||
sessionKey(coordinate, narrowOp) in sessionAllows ||
|
||||
sl.store.loadOpDecision(coordinate, narrowOp)?.let { sl.decide(coordinate, narrowOp) == NostrOpDecision.ALLOW } ?: false
|
||||
|
||||
/**
|
||||
* The signer-ledger coordinate for [identity] under the current account. The signer permission
|
||||
* store is shared with NIP-46, which already namespaces by account
|
||||
|
||||
+12
@@ -107,6 +107,18 @@ enum class NappletCapability {
|
||||
get() = !requiresPerUseConsent
|
||||
|
||||
companion object {
|
||||
/**
|
||||
* What a page in the **website** posture (the NIP-07 `window.nostr` surface) may ask the
|
||||
* broker for. There are two independent mints of this set — the nSite host derives it from
|
||||
* `HostProfile.WEBSITE`, while the in-app browser mints a fresh per-origin token — so it
|
||||
* lives here, once: when the two drifted, the browser silently denied every call to a
|
||||
* capability the injected shim was still advertising.
|
||||
*
|
||||
* Widening this widens what any visited site can request, so it is a security decision, not
|
||||
* a convenience list.
|
||||
*/
|
||||
val WEBSITE_CAPABILITIES: Set<NappletCapability> = setOf(IDENTITY, RELAY, SIGNER)
|
||||
|
||||
/**
|
||||
* Maps a bare, currently supported NAP domain to the capability the broker enforces.
|
||||
* Returns `null` for unknown and partial/legacy domains — callers MUST treat that as
|
||||
|
||||
+28
@@ -41,3 +41,31 @@ fun NappletRequest.toSignerOp(): NostrSignerOp? =
|
||||
is NappletRequest.Nip44Decrypt -> NostrSignerOp.Decrypt
|
||||
else -> null
|
||||
}
|
||||
|
||||
/**
|
||||
* The NARROWER op a request may alternatively be granted — today only
|
||||
* [NostrSignerOp.DecryptFrom], i.e. "always allow, but only for this counterparty". Mirrors the
|
||||
* NIP-46 authorizer's `toNarrowSignerOp`.
|
||||
*
|
||||
* A single broad "always allow decrypt" hands an app every private conversation the user will ever
|
||||
* have; this is the granular alternative the consent dialog offers alongside it. `null` for every
|
||||
* request without a counterparty — signing and encryption already name the thing being granted.
|
||||
*/
|
||||
fun NappletRequest.toNarrowSignerOp(): NostrSignerOp? =
|
||||
when (this) {
|
||||
is NappletRequest.Nip44Decrypt -> NostrSignerOp.DecryptFrom(peer)
|
||||
else -> null
|
||||
}
|
||||
|
||||
/**
|
||||
* The counterparty whose conversation a decrypt request asks to read, or `null` for every other
|
||||
* request. Scoped to decryption to match the NIP-46 authorizer and what the consent dialog
|
||||
* documents: it drives "X wants to read your messages with Alice", a categorically different
|
||||
* decision from the encrypt/sign case, where the counterparty is already part of what the user
|
||||
* is composing.
|
||||
*/
|
||||
fun NappletRequest.counterpartyPubKey(): String? =
|
||||
when (this) {
|
||||
is NappletRequest.Nip44Decrypt -> peer
|
||||
else -> null
|
||||
}
|
||||
|
||||
+98
@@ -850,4 +850,102 @@ class NappletBrokerTest {
|
||||
val response = broker.handle(applet, NappletRequest.Nip44Decrypt(peer.pubKey, sealed), allDeclared)
|
||||
assertIs<NappletResponse.Denied>(response)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun theWebsiteCapabilitySetCarriesEverythingNip07Needs() {
|
||||
// Two places mint this set (the nSite HostProfile and the in-app browser's per-origin
|
||||
// token). They drifted once: the browser kept IDENTITY+RELAY while the shim advertised
|
||||
// nip44, so every call was denied by a capability the page was told it had.
|
||||
assertTrue(NappletCapability.IDENTITY in NappletCapability.WEBSITE_CAPABILITIES)
|
||||
assertTrue(NappletCapability.RELAY in NappletCapability.WEBSITE_CAPABILITIES)
|
||||
assertTrue(NappletCapability.SIGNER in NappletCapability.WEBSITE_CAPABILITIES)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun nip44WorksUnderTheWebsiteCapabilitySetAlone() =
|
||||
runTest {
|
||||
// What a browsed page actually gets — not `allDeclared`, which would hide a missing grant.
|
||||
val peer = NostrSignerInternal(KeyPair("44".repeat(32).hexToByteArray()))
|
||||
val broker = broker(ScriptedPrompt(GrantState.ALLOW_ALWAYS))
|
||||
|
||||
val response =
|
||||
broker.handle(
|
||||
applet,
|
||||
NappletRequest.Nip44Encrypt(peer.pubKey, "gm"),
|
||||
NappletCapability.WEBSITE_CAPABILITIES,
|
||||
)
|
||||
|
||||
assertIs<NappletResponse.Text>(response)
|
||||
assertEquals("gm", peer.nip44Decrypt(response.value, signer.pubKey))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun anAlwaysAllowForOneCounterpartyDoesNotUnlockTheRest() =
|
||||
runTest {
|
||||
// The dialog can hand back DecryptFrom(alice) instead of the broad Decrypt. Recording the
|
||||
// REQUESTED op there would silently upgrade "only Alice" into every conversation forever.
|
||||
val signerLedger = NostrSignerPermissionLedger(InMemoryNostrSignerPermissionStore())
|
||||
signerLedger.setPolicy("napplet:${signer.pubKey}:${applet.coordinate}", AppSignerPolicy.REASONABLE)
|
||||
|
||||
val alice = NostrSignerInternal(KeyPair("55".repeat(32).hexToByteArray()))
|
||||
val bob = NostrSignerInternal(KeyPair("66".repeat(32).hexToByteArray()))
|
||||
|
||||
val opPrompt = ScriptedSignerPrompt(SignerOpGrant.AllowForOp(NostrSignerOp.DecryptFrom(alice.pubKey)))
|
||||
val broker =
|
||||
NappletBroker(
|
||||
signer = signer,
|
||||
ledger = NappletPermissionLedger(InMemoryNappletPermissionStore()),
|
||||
consentPrompt = ScriptedPrompt(GrantState.ALLOW_ALWAYS),
|
||||
signerLedger = signerLedger,
|
||||
signerConsentPrompt = opPrompt,
|
||||
)
|
||||
|
||||
val fromAlice = alice.nip44Encrypt("hi", signer.pubKey)
|
||||
val fromBob = bob.nip44Encrypt("hi", signer.pubKey)
|
||||
|
||||
// 1. First read from Alice prompts; the user allows, but only for Alice.
|
||||
assertIs<NappletResponse.Text>(broker.handle(applet, NappletRequest.Nip44Decrypt(alice.pubKey, fromAlice), allDeclared))
|
||||
assertEquals(1, opPrompt.calls)
|
||||
|
||||
// 2. Reading Alice again rides the narrow grant — no second prompt.
|
||||
assertIs<NappletResponse.Text>(broker.handle(applet, NappletRequest.Nip44Decrypt(alice.pubKey, fromAlice), allDeclared))
|
||||
assertEquals(1, opPrompt.calls)
|
||||
|
||||
// 3. Bob is a different conversation and must ask again. If the broad Decrypt had been
|
||||
// recorded in step 1, this would sail through without the user ever agreeing to it.
|
||||
broker.handle(applet, NappletRequest.Nip44Decrypt(bob.pubKey, fromBob), allDeclared)
|
||||
assertEquals(2, opPrompt.calls)
|
||||
|
||||
// The broad grant was never written.
|
||||
assertNull(signerLedger.store.loadOpDecision("napplet:${signer.pubKey}:${applet.coordinate}", NostrSignerOp.Decrypt))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun aSessionGrantIsStoredNoWiderThanTheUserGaveIt() =
|
||||
runTest {
|
||||
val signerLedger = NostrSignerPermissionLedger(InMemoryNostrSignerPermissionStore())
|
||||
signerLedger.setPolicy("napplet:${signer.pubKey}:${applet.coordinate}", AppSignerPolicy.PARANOID)
|
||||
|
||||
val alice = NostrSignerInternal(KeyPair("77".repeat(32).hexToByteArray()))
|
||||
val bob = NostrSignerInternal(KeyPair("88".repeat(32).hexToByteArray()))
|
||||
|
||||
val opPrompt = ScriptedSignerPrompt(SignerOpGrant.AllowForSession(NostrSignerOp.DecryptFrom(alice.pubKey)))
|
||||
val broker =
|
||||
NappletBroker(
|
||||
signer = signer,
|
||||
ledger = NappletPermissionLedger(InMemoryNappletPermissionStore()),
|
||||
consentPrompt = ScriptedPrompt(GrantState.ALLOW_ALWAYS),
|
||||
signerLedger = signerLedger,
|
||||
signerConsentPrompt = opPrompt,
|
||||
)
|
||||
|
||||
broker.handle(applet, NappletRequest.Nip44Decrypt(alice.pubKey, alice.nip44Encrypt("a", signer.pubKey)), allDeclared)
|
||||
assertEquals(1, opPrompt.calls)
|
||||
|
||||
// Same counterparty rides the session grant; a different one must not.
|
||||
broker.handle(applet, NappletRequest.Nip44Decrypt(alice.pubKey, alice.nip44Encrypt("a", signer.pubKey)), allDeclared)
|
||||
assertEquals(1, opPrompt.calls)
|
||||
broker.handle(applet, NappletRequest.Nip44Decrypt(bob.pubKey, bob.nip44Encrypt("b", signer.pubKey)), allDeclared)
|
||||
assertEquals(2, opPrompt.calls)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -51,7 +51,7 @@ enum class HostProfile {
|
||||
*/
|
||||
fun declaredCapabilities(requires: List<String>): Set<NappletCapability> =
|
||||
when (this) {
|
||||
WEBSITE -> setOf(NappletCapability.IDENTITY, NappletCapability.RELAY, NappletCapability.SIGNER)
|
||||
WEBSITE -> NappletCapability.WEBSITE_CAPABILITIES
|
||||
NAPPLET -> resolveRequiredCapabilities(requires).capabilities.toSet()
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user