MF3-M05: allow wallet-only revocation of session keys

docs/api.yaml

@@ -391,7 +391,10 @@ types:
           description: >
             Session-key holder's EIP-191 signature over the same payload as user_sig —
             abi.encode(SESSION_KEY_AUTH_TYPEHASH, session_key, metadata_hash) — proving possession of the key
-            being registered. Required on every submit to prevent registration of keys the submitter does not control.
+            being registered. Required when expires_at is in the future (registration, update, re-activation)
+            to prevent registration of keys the submitter does not control. Optional for revocation
+            (expires_at at or before now): a wallet may revoke unilaterally with user_sig alone so a lost or
+            uncooperative session key cannot block revocation.
 
   - app_session_key_state:
       description: Represents the state of an app session key
@@ -423,7 +426,7 @@ types:
           description: User's signature over the session key metadata to authorize the registration/update of the session key
         - name: session_key_sig
           type: string
-          description: Session-key holder's signature over the same packed state (which already binds user_address) proving possession of the key being registered. Required on every submit to prevent registration of keys the submitter does not control.
+          description: Session-key holder's signature over the same packed state (which already binds user_address) proving possession of the key being registered. Required when expires_at is in the future (registration, update, re-activation) to prevent registration of keys the submitter does not control. Optional for revocation (expires_at at or before now); a wallet may revoke unilaterally with user_sig alone so a lost or uncooperative session key cannot block revocation.
 
   - pagination_params:
       description: Pagination request parameters
@@ -601,15 +604,16 @@ api:
                 The metadata hash binds `expires_at`, so each revoke or re-activation requires a
                 fresh user signature over the new payload. Negative unix timestamps are rejected.
 
-                Both `user_sig` (wallet) and `session_key_sig` (session-key holder) are required
-                on every submit, including the revocation path — the session key must co-sign its
-                own deactivation. Wallet-only revocation (for a lost or compromised key) is not
-                supported by this method; that flow requires a separate code path and is tracked
-                as a follow-up.
+                Activation, update, and re-activation (`expires_at` in the future) require both
+                `user_sig` (wallet) and `session_key_sig` (session-key holder, proving possession).
+                Revocation (`expires_at` at or before now) requires only `user_sig`: a wallet may
+                revoke unilaterally so a lost, unavailable, or compromised session key cannot block
+                revocation of its own delegation. Any `session_key_sig` supplied on the revocation
+                path is ignored.
               request:
                 - field_name: state
                   type: channel_session_key_state
-                  description: Session key metadata and delegation information. Set `expires_at` to a past value to revoke; future to (re-)activate.
+                  description: Session key metadata and delegation information. Set `expires_at` to a value at or before now to revoke (user_sig only); future to (re-)activate (both signatures).
               response: []
               errors:
                 - message: invalid_session_key_state
@@ -792,15 +796,16 @@ api:
                 The metadata hash binds `expires_at`, so each revoke or re-activation requires a
                 fresh user signature over the new payload. Negative unix timestamps are rejected.
 
-                Both `user_sig` (wallet) and `session_key_sig` (session-key holder) are required
-                on every submit, including the revocation path — the session key must co-sign its
-                own deactivation. Wallet-only revocation (for a lost or compromised key) is not
-                supported by this method; that flow requires a separate code path and is tracked
-                as a follow-up.
+                Activation, update, and re-activation (`expires_at` in the future) require both
+                `user_sig` (wallet) and `session_key_sig` (session-key holder, proving possession).
+                Revocation (`expires_at` at or before now) requires only `user_sig`: a wallet may
+                revoke unilaterally so a lost, unavailable, or compromised session key cannot block
+                revocation of its own delegation. Any `session_key_sig` supplied on the revocation
+                path is ignored.
               request:
                 - field_name: state
                   type: app_session_key_state
-                  description: Session key metadata and delegation information. Set `expires_at` to a past value to revoke; future to (re-)activate.
+                  description: Session key metadata and delegation information. Set `expires_at` to a value at or before now to revoke (user_sig only); future to (re-)activate (both signatures).
               response: []
               errors:
                 - message: invalid_session_key_state

nitronode/api/app_session_v1/submit_session_key_state.go

@@ -92,19 +92,44 @@ func (h *Handler) SubmitSessionKeyState(c *rpc.Context) {
 		c.Fail(rpc.Errorf("invalid_session_key_state: user_sig is required"), "")
 		return
 	}
-	if coreState.SessionKeySig == "" {
-		c.Fail(rpc.Errorf("invalid_session_key_state: session_key_sig is required"), "")
-		return
-	}
 
-	// Validate both signatures: wallet's UserSig and session-key holder's SessionKeySig.
-	if err := app.ValidateAppSessionKeyStateV1(coreState); err != nil {
-		c.Fail(rpc.Errorf("invalid_session_key_state: %v", err), "")
-		return
+	// A submit with expires_at after now activates, extends, or rotates the key and requires
+	// both signatures. A submit with expires_at <= now is a revocation: it only deactivates an
+	// existing delegation, so the wallet's user_sig alone authorizes it. Requiring session_key_sig
+	// on the revocation path would let a lost, unavailable, or malicious session key veto its own
+	// revocation, stranding the user until the prior expires_at naturally passes. now is captured
+	// here and reused for the cap/version logic inside the transaction so the active/inactive
+	// decision is consistent across both.
+	now := time.Now()
+	if coreState.ExpiresAt.After(now) {
+		if coreState.SessionKeySig == "" {
+			c.Fail(rpc.Errorf("invalid_session_key_state: session_key_sig is required"), "")
+			return
+		}
+		// Validate both signatures: wallet's UserSig and session-key holder's SessionKeySig.
+		if err := app.ValidateAppSessionKeyStateV1(coreState); err != nil {
+			c.Fail(rpc.Errorf("invalid_session_key_state: %v", err), "")
+			return
+		}
+	} else {
+		// Revocation only deactivates an existing delegation. Version 1 means there is no prior
+		// delegation, so reject it before LockSessionKeyState can seed a permanent ownership
+		// claim for a session_key the caller never proved possession of: a legitimate revoke is
+		// always version >= 2, since registration at version 1 is future-dated and requires both
+		// signatures.
+		if coreState.Version == 1 {
+			c.Fail(rpc.Errorf("invalid_session_key_state: cannot revoke a session key with no prior delegation"), "")
+			return
+		}
+		// Validate only the wallet's UserSig. Any session_key_sig present is ignored, so clear
+		// it before persisting to keep stored revocation rows canonical.
+		if err := app.ValidateAppSessionKeyStateUserSigV1(coreState); err != nil {
+			c.Fail(rpc.Errorf("invalid_session_key_state: %v", err), "")
+			return
+		}
+		coreState.SessionKeySig = ""
 	}
 
-	// Validate version and store the session key state
-	now := time.Now()
 	// revoked is true only when this submit transitions an active key to inactive,
 	// so the revocation log is not emitted for inactive-to-inactive updates.
 	var revoked bool

nitronode/api/app_session_v1/submit_session_key_state_test.go

@@ -291,7 +291,10 @@ func TestSubmitSessionKeyState_InvalidUserAddress(t *testing.T) {
 	assert.Contains(t, respErr.Error(), "invalid user_address")
 }
 
-func TestSubmitSessionKeyState_RevokeWithPastExpiresAt(t *testing.T) {
+// A version-1 revoke has no prior delegation to deactivate; allowing it would let a wallet
+// seed a permanent (session_key, kind) ownership claim for a key it never proved possession
+// of. It must be rejected before LockSessionKeyState runs, so no seed row is ever written.
+func TestSubmitSessionKeyState_RevokeFirstSubmit_Rejected(t *testing.T) {
 	mockStore := new(MockStore)
 	userSigner := NewMockSigner()
 	userAddress := strings.ToLower(userSigner.PublicKey().Address().String())
@@ -306,14 +309,9 @@ func TestSubmitSessionKeyState_RevokeWithPastExpiresAt(t *testing.T) {
 		maxSessionKeyIDs: 10,
 	}
 
-	// expires_at in the past expresses a revoke: the same monotonic version sequence
-	// is preserved, the auth path filters expires_at > now so the key is deactivated.
 	expiresAt := time.Now().Add(-time.Hour).Truncate(time.Second)
 	reqPayload := buildSignedSessionKeyStateReq(t, userAddress, sessionKeyAddress, 1, nil, nil, expiresAt, userSigner, sessionKeySigner)
 
-	mockStore.On("LockSessionKeyState", userAddress, sessionKeyAddress, database.SessionKeyKindAppSession).Return(0, time.Time{}, nil)
-	mockStore.On("StoreAppSessionKeyState", mock.AnythingOfType("app.AppSessionKeyStateV1")).Return(nil)
-
 	payload, err := rpc.NewPayload(reqPayload)
 	require.NoError(t, err)
 
@@ -325,8 +323,11 @@ func TestSubmitSessionKeyState_RevokeWithPastExpiresAt(t *testing.T) {
 	handler.SubmitSessionKeyState(ctx)
 
 	require.NotNil(t, ctx.Response)
-	assert.Nil(t, ctx.Response.Error())
-	mockStore.AssertExpectations(t)
+	respErr := ctx.Response.Error()
+	require.NotNil(t, respErr)
+	assert.Contains(t, respErr.Error(), "no prior delegation")
+	mockStore.AssertNotCalled(t, "LockSessionKeyState", mock.Anything, mock.Anything, mock.Anything)
+	mockStore.AssertNotCalled(t, "StoreAppSessionKeyState", mock.Anything)
 }
 
 // Covers the typical revocation path: an active key (latestVersion > 0, prev expires_at in
@@ -882,6 +883,131 @@ func TestSubmitSessionKeyState_RejectsMismatchedSessionKeySig(t *testing.T) {
 	mockStore.AssertNotCalled(t, "LockSessionKeyState", mock.Anything, mock.Anything, mock.Anything)
 }
 
+// Wallet-only revocation: a submit with a past expires_at and NO session_key_sig is accepted
+// on the strength of user_sig alone. This is the core remediation — a lost, unavailable, or
+// uncooperative session key can no longer veto revocation of its own delegation.
+func TestSubmitSessionKeyState_RevokeUserSigOnly_Succeeds(t *testing.T) {
+	mockStore := new(MockStore)
+	userSigner := NewMockSigner()
+	userAddress := strings.ToLower(userSigner.PublicKey().Address().String())
+	sessionKeySigner := NewMockSigner()
+	sessionKeyAddress := strings.ToLower(sessionKeySigner.PublicKey().Address().String())
+
+	handler := &Handler{
+		useStoreInTx: func(handler StoreTxHandler) error {
+			return handler(mockStore)
+		},
+		metrics:          metrics.NewNoopRuntimeMetricExporter(),
+		maxSessionKeyIDs: 10,
+	}
+
+	expiresAt := time.Now().Add(-time.Hour).Truncate(time.Second)
+	// keySigner=nil → SessionKeySig stays empty; the revocation path must not require it.
+	reqPayload := buildSignedSessionKeyStateReq(t, userAddress, sessionKeyAddress, 2, nil, nil, expiresAt, userSigner, nil)
+
+	prevActiveExpiresAt := time.Now().Add(24 * time.Hour)
+	mockStore.On("LockSessionKeyState", userAddress, sessionKeyAddress, database.SessionKeyKindAppSession).Return(1, prevActiveExpiresAt, nil)
+	mockStore.On("StoreAppSessionKeyState", mock.AnythingOfType("app.AppSessionKeyStateV1")).Return(nil)
+
+	payload, err := rpc.NewPayload(reqPayload)
+	require.NoError(t, err)
+
+	ctx := &rpc.Context{
+		Context: context.Background(),
+		Request: rpc.NewRequest(1, rpc.AppSessionsV1SubmitSessionKeyStateMethod.String(), payload),
+	}
+
+	handler.SubmitSessionKeyState(ctx)
+
+	require.NotNil(t, ctx.Response)
+	assert.Nil(t, ctx.Response.Error())
+	mockStore.AssertExpectations(t)
+}
+
+// On the revocation path a present-but-mismatched session_key_sig is ignored, not validated.
+// The same signature would be rejected on the active path (see RejectsMismatchedSessionKeySig).
+func TestSubmitSessionKeyState_RevokeIgnoresMismatchedSessionKeySig(t *testing.T) {
+	mockStore := new(MockStore)
+	userSigner := NewMockSigner()
+	userAddress := strings.ToLower(userSigner.PublicKey().Address().String())
+	sessionKeySigner := NewMockSigner()
+	sessionKeyAddress := strings.ToLower(sessionKeySigner.PublicKey().Address().String())
+	otherSigner := NewMockSigner()
+
+	handler := &Handler{
+		useStoreInTx: func(handler StoreTxHandler) error {
+			return handler(mockStore)
+		},
+		metrics:          metrics.NewNoopRuntimeMetricExporter(),
+		maxSessionKeyIDs: 10,
+	}
+
+	expiresAt := time.Now().Add(-time.Hour).Truncate(time.Second)
+	// SessionKeySig signed by an unrelated key — would fail the active path, ignored on revoke.
+	reqPayload := buildSignedSessionKeyStateReq(t, userAddress, sessionKeyAddress, 2, nil, nil, expiresAt, userSigner, otherSigner)
+
+	prevActiveExpiresAt := time.Now().Add(24 * time.Hour)
+	mockStore.On("LockSessionKeyState", userAddress, sessionKeyAddress, database.SessionKeyKindAppSession).Return(1, prevActiveExpiresAt, nil)
+	// The ignored session_key_sig must be cleared before persisting so stored revocation
+	// rows never retain unverified client input.
+	mockStore.On("StoreAppSessionKeyState", mock.MatchedBy(func(s app.AppSessionKeyStateV1) bool {
+		return s.SessionKeySig == ""
+	})).Return(nil)
+
+	payload, err := rpc.NewPayload(reqPayload)
+	require.NoError(t, err)
+
+	ctx := &rpc.Context{
+		Context: context.Background(),
+		Request: rpc.NewRequest(1, rpc.AppSessionsV1SubmitSessionKeyStateMethod.String(), payload),
+	}
+
+	handler.SubmitSessionKeyState(ctx)
+
+	require.NotNil(t, ctx.Response)
+	assert.Nil(t, ctx.Response.Error())
+	mockStore.AssertExpectations(t)
+}
+
+// Even on the revocation path the wallet's user_sig must be valid: a revoke signed by a key
+// other than the user_address is rejected, so revocation stays a wallet-only right (not anyone's).
+func TestSubmitSessionKeyState_RevokeInvalidUserSig_Rejected(t *testing.T) {
+	mockStore := new(MockStore)
+	userSigner := NewMockSigner()
+	differentSigner := NewMockSigner()
+	userAddress := strings.ToLower(userSigner.PublicKey().Address().String())
+	sessionKeySigner := NewMockSigner()
+	sessionKeyAddress := strings.ToLower(sessionKeySigner.PublicKey().Address().String())
+
+	handler := &Handler{
+		useStoreInTx: func(handler StoreTxHandler) error {
+			return handler(mockStore)
+		},
+		metrics:          metrics.NewNoopRuntimeMetricExporter(),
+		maxSessionKeyIDs: 10,
+	}
+
+	expiresAt := time.Now().Add(-time.Hour).Truncate(time.Second)
+	// user_sig produced by a key other than userAddress; no session_key_sig.
+	reqPayload := buildSignedSessionKeyStateReq(t, userAddress, sessionKeyAddress, 2, nil, nil, expiresAt, differentSigner, nil)
+
+	payload, err := rpc.NewPayload(reqPayload)
+	require.NoError(t, err)
+
+	ctx := &rpc.Context{
+		Context: context.Background(),
+		Request: rpc.NewRequest(1, rpc.AppSessionsV1SubmitSessionKeyStateMethod.String(), payload),
+	}
+
+	handler.SubmitSessionKeyState(ctx)
+
+	require.NotNil(t, ctx.Response)
+	respErr := ctx.Response.Error()
+	require.NotNil(t, respErr)
+	assert.Contains(t, respErr.Error(), "user_sig does not match user_address")
+	mockStore.AssertNotCalled(t, "LockSessionKeyState", mock.Anything, mock.Anything, mock.Anything)
+}
+
 func TestSubmitSessionKeyState_RejectsForeignOwner(t *testing.T) {
 	mockStore := new(MockStore)
 	userSigner := NewMockSigner()

nitronode/api/channel_v1/submit_session_key_state.go

@@ -75,19 +75,44 @@ func (h *Handler) SubmitSessionKeyState(c *rpc.Context) {
 		c.Fail(rpc.Errorf("invalid_session_key_state: user_sig is required"), "")
 		return
 	}
-	if coreState.SessionKeySig == "" {
-		c.Fail(rpc.Errorf("invalid_session_key_state: session_key_sig is required"), "")
-		return
-	}
 
-	// Validate both signatures: wallet's user_sig and session-key holder's session_key_sig.
-	if err := core.ValidateChannelSessionKeyStateV1(coreState); err != nil {
-		c.Fail(rpc.Errorf("invalid_session_key_state: %v", err), "")
-		return
+	// A submit with expires_at after now activates, extends, or rotates the key and requires
+	// both signatures. A submit with expires_at <= now is a revocation: it only deactivates an
+	// existing delegation, so the wallet's user_sig alone authorizes it. Requiring session_key_sig
+	// on the revocation path would let a lost, unavailable, or malicious session key veto its own
+	// revocation, stranding the user until the prior expires_at naturally passes. now is captured
+	// here and reused for the cap/version logic inside the transaction so the active/inactive
+	// decision is consistent across both.
+	now := time.Now()
+	if coreState.ExpiresAt.After(now) {
+		if coreState.SessionKeySig == "" {
+			c.Fail(rpc.Errorf("invalid_session_key_state: session_key_sig is required"), "")
+			return
+		}
+		// Validate both signatures: wallet's user_sig and session-key holder's session_key_sig.
+		if err := core.ValidateChannelSessionKeyStateV1(coreState); err != nil {
+			c.Fail(rpc.Errorf("invalid_session_key_state: %v", err), "")
+			return
+		}
+	} else {
+		// Revocation only deactivates an existing delegation. Version 1 means there is no prior
+		// delegation, so reject it before LockSessionKeyState can seed a permanent ownership
+		// claim for a session_key the caller never proved possession of: a legitimate revoke is
+		// always version >= 2, since registration at version 1 is future-dated and requires both
+		// signatures.
+		if coreState.Version == 1 {
+			c.Fail(rpc.Errorf("invalid_session_key_state: cannot revoke a session key with no prior delegation"), "")
+			return
+		}
+		// Validate only the wallet's user_sig. Any session_key_sig present is ignored, so clear
+		// it before persisting to keep stored revocation rows canonical.
+		if err := core.ValidateChannelSessionKeyStateUserSigV1(coreState); err != nil {
+			c.Fail(rpc.Errorf("invalid_session_key_state: %v", err), "")
+			return
+		}
+		coreState.SessionKeySig = ""
 	}
 
-	// Validate version and store the session key state
-	now := time.Now()
 	// revoked is true only when this submit transitions an active key to inactive,
 	// so the revocation log is not emitted for inactive-to-inactive updates.
 	var revoked bool