fix: address Copilot review comments

PR SUNET#263 Code Quality Fixes:

- jws.go: Use %v instead of %s for non-string types in error messages
- attachment.go: Use RawURLEncoding for base64url (no padding per DIDComm spec)
- attachment.go: Fix AttachmentData comment to match implementation (multiple formats allowed)
- packer.go: Return error when SignBeforeEncrypt=true but SignerKey=nil (was silent no-op)
- route.go: Document BuildRoute fallback behavior and RoutingKey usage
- ecdsa/suite.go: Add context parameter for HSM signer support
- transport/doc.go: Update example to use SendRequest struct
- message/doc.go: Fix WithTo example (variadic, not slice)
- signer_test.go: Check rand.Read errors properly

Author: leifj

pkg/didcomm/crypto/jws.go

@@ -156,10 +156,10 @@ func determineSigningAlgorithm(key jwk.Key, hint string) (jwa.SignatureAlgorithm
 			if crv.String() == "secp256k1" {
 				return jwa.ES256K(), nil
 			}
-			return jwa.ES256(), fmt.Errorf("%w: unsupported curve %s", ErrUnsupportedAlgorithm, crv)
+			return jwa.ES256(), fmt.Errorf("%w: unsupported curve %v", ErrUnsupportedAlgorithm, crv)
 		}
 	default:
-		return jwa.EdDSA(), fmt.Errorf("%w: unsupported key type %s", ErrUnsupportedAlgorithm, kty)
+		return jwa.EdDSA(), fmt.Errorf("%w: unsupported key type %v", ErrUnsupportedAlgorithm, kty)
 	}
 }
 

pkg/didcomm/message/attachment.go

@@ -39,7 +39,8 @@ type Attachment struct {
 }
 
 // AttachmentData represents the actual attachment content.
-// Exactly one of JWS, Hash, Links, Base64, or JSON should be set.
+// At least one of JWS, Links, Base64, or JSON should be set.
+// Multiple formats are allowed per DIDComm spec.
 type AttachmentData struct {
 	// JWS is a JSON Web Signature wrapping the content.
 	// Provides integrity and optional authentication.
@@ -71,7 +72,7 @@ func (a *Attachment) Validate() error {
 // Works for Base64 and JSON encoded content.
 func (a *Attachment) GetBytes() ([]byte, error) {
 	if a.Data.Base64 != "" {
-		return base64.URLEncoding.DecodeString(a.Data.Base64)
+		return base64.RawURLEncoding.DecodeString(a.Data.Base64)
 	}
 	if a.Data.JSON != nil {
 		return json.Marshal(a.Data.JSON)
@@ -111,7 +112,7 @@ func (a *Attachment) GetJSON(v any) error {
 		return json.Unmarshal(data, v)
 	}
 	if a.Data.Base64 != "" {
-		data, err := base64.URLEncoding.DecodeString(a.Data.Base64)
+		data, err := base64.RawURLEncoding.DecodeString(a.Data.Base64)
 		if err != nil {
 			return err
 		}
@@ -144,13 +145,13 @@ func (d *AttachmentData) Validate() error {
 	return nil
 }
 
-// NewBase64Attachment creates an attachment with base64-encoded data.
+// NewBase64Attachment creates an attachment with base64url-encoded data (no padding).
 func NewBase64Attachment(id string, mediaType string, data []byte) Attachment {
 	return Attachment{
 		ID:        id,
 		MediaType: mediaType,
 		Data: AttachmentData{
-			Base64: base64.URLEncoding.EncodeToString(data),
+			Base64: base64.RawURLEncoding.EncodeToString(data),
 		},
 	}
 }

pkg/didcomm/message/doc.go

@@ -20,7 +20,7 @@
 //
 //	msg := message.New(
 //	    message.WithType("https://example.com/protocols/1.0/hello"),
-//	    message.WithTo([]string{"did:example:bob"}),
+//	    message.WithTo("did:example:bob"),
 //	    message.WithFrom("did:example:alice"),
 //	    message.WithBody(map[string]any{"greeting": "Hello, Bob!"}),
 //	)

pkg/didcomm/packer.go

@@ -75,7 +75,10 @@ func Pack(ctx context.Context, msg *message.Message, opts PackOptions) (*PackRes
 	}
 
 	// Step 1: Sign if requested
-	if opts.SignBeforeEncrypt && opts.SignerKey != nil {
+	if opts.SignBeforeEncrypt {
+		if opts.SignerKey == nil {
+			return nil, fmt.Errorf("SignBeforeEncrypt requires SignerKey to be set")
+		}
 		signedMsg, err := crypto.Sign(ctx, plaintext, opts.SignerKey, crypto.SignOptions{})
 		if err != nil {
 			return nil, fmt.Errorf("failed to sign message: %w", err)

pkg/didcomm/routing/route.go

@@ -13,10 +13,13 @@ const DefaultMaxHops = 10
 // Hop represents a single hop in a routing path.
 type Hop struct {
 	// RecipientDID is the DID that should receive the message at this hop.
+	// This is used for encryption (Encrypter interface requires DIDs).
 	RecipientDID string
 
-	// RoutingKey is the key ID to use for encryption at this hop.
-	// If empty, the recipient's default key agreement key is used.
+	// RoutingKey is the original key ID from the service endpoint.
+	// May be a DID URL (e.g., did:example:mediator#key-1) or a DID.
+	// The Encrypter uses RecipientDID (extracted from RoutingKey if needed)
+	// since the encryption interface operates on DIDs, not key IDs.
 	RoutingKey string
 
 	// ServiceEndpoint is the endpoint URL for this hop (if known).
@@ -107,7 +110,10 @@ func (rb *RouteBuilder) buildRouteRecursive(ctx context.Context, did string, dep
 	// Resolve the DID's service endpoint
 	service, err := rb.resolver.ResolveDIDCommService(ctx, did)
 	if err != nil {
-		// If we can't resolve, assume direct delivery
+		// Fallback to direct delivery when service resolution fails.
+		// This allows sending to DIDs that don't publish DIDComm services,
+		// treating them as direct recipients. Callers should handle this
+		// by attempting direct delivery or reporting the missing endpoint.
 		return &Route{
 			FinalRecipient: did,
 			Hops: []Hop{

pkg/didcomm/transport/doc.go

@@ -14,7 +14,11 @@
 // Messages are sent as POST requests with appropriate content types.
 //
 //	client := transport.NewHTTPClient()
-//	response, err := client.Send(ctx, "https://example.com/didcomm", message)
+//	response, err := client.Send(ctx, transport.SendRequest{
+//		Endpoint:  "https://example.com/didcomm",
+//		Message:   packedMessage,
+//		MediaType: "application/didcomm-encrypted+json",
+//	})
 //
 // # Server Handler
 //

pkg/openid4vp/vc20_handler.go

@@ -921,7 +921,7 @@ func (h *VC20Handler) signECDSA2019(cred *credential.RDFCredential) (*credential
 	}
 
 	suite := ecdsaSuite.NewSuite()
-	return suite.Sign(cred, key, &ecdsaSuite.SignOptions{
+	return suite.Sign(context.Background(), cred, key, &ecdsaSuite.SignOptions{
 		VerificationMethod: h.signerConfig.VerificationMethod,
 		ProofPurpose:       "assertionMethod",
 		Created:            time.Now().UTC(),

pkg/openid4vp/vc20_handler_integration_test.go

@@ -50,7 +50,7 @@ func TestVC20Handler_VerifyECDSA2019_Integration(t *testing.T) {
 	cred, err := credential.NewRDFCredentialFromJSON(credBytes, nil)
 	require.NoError(t, err)
 
-	signedCred, err := suite.Sign(cred, key, &ecdsaSuite.SignOptions{
+	signedCred, err := suite.Sign(t.Context(), cred, key, &ecdsaSuite.SignOptions{
 		VerificationMethod: "did:example:issuer#key-1",
 		Created:            time.Now(),
 		ProofPurpose:       "assertionMethod",
@@ -218,7 +218,7 @@ func TestVC20Handler_VP_Extraction(t *testing.T) {
 	cred, err := credential.NewRDFCredentialFromJSON(credBytes, nil)
 	require.NoError(t, err)
 
-	signedCred, err := suite.Sign(cred, key, &ecdsaSuite.SignOptions{
+	signedCred, err := suite.Sign(t.Context(), cred, key, &ecdsaSuite.SignOptions{
 		VerificationMethod: "did:example:issuer#key-1",
 		Created:            time.Now(),
 		ProofPurpose:       "assertionMethod",
@@ -283,7 +283,7 @@ func TestVC20Handler_TimeValidation(t *testing.T) {
 		cred, err := credential.NewRDFCredentialFromJSON(credBytes, nil)
 		require.NoError(t, err)
 
-		signedCred, err := suite.Sign(cred, key, &ecdsaSuite.SignOptions{
+		signedCred, err := suite.Sign(t.Context(), cred, key, &ecdsaSuite.SignOptions{
 			VerificationMethod: "did:example:issuer#key-1",
 			Created:            time.Now(),
 			ProofPurpose:       "assertionMethod",
@@ -323,7 +323,7 @@ func TestVC20Handler_TimeValidation(t *testing.T) {
 		cred, err := credential.NewRDFCredentialFromJSON(credBytes, nil)
 		require.NoError(t, err)
 
-		signedCred, err := suite.Sign(cred, key, &ecdsaSuite.SignOptions{
+		signedCred, err := suite.Sign(t.Context(), cred, key, &ecdsaSuite.SignOptions{
 			VerificationMethod: "did:example:issuer#key-1",
 			Created:            time.Now().Add(-time.Hour * 24 * 30),
 			ProofPurpose:       "assertionMethod",

pkg/openid4vp/vc20_vp_builder.go

@@ -3,6 +3,7 @@
 package openid4vp
 
 import (
+	"context"
 	"crypto"
 	"crypto/ecdsa"
 	"crypto/ed25519"
@@ -223,7 +224,7 @@ func (b *VPBuilder) signWithECDSA(vpBytes []byte, privateKey crypto.PrivateKey,
 
 	// Sign
 	suite := ecdsaSuite.NewSuite()
-	signedVP, err := suite.Sign(vpCred, ecKey, signOpts)
+	signedVP, err := suite.Sign(context.Background(), vpCred, ecKey, signOpts)
 	if err != nil {
 		return nil, fmt.Errorf("ECDSA signing failed: %w", err)
 	}

pkg/vc20/crypto/ecdsa/suite.go

@@ -41,18 +41,18 @@ type SignOptions struct {
 }
 
 // Sign signs a credential using ecdsa-rdfc-2019
-func (s *Suite) Sign(cred *credential.RDFCredential, key *ecdsa.PrivateKey, opts *SignOptions) (*credential.RDFCredential, error) {
+func (s *Suite) Sign(ctx context.Context, cred *credential.RDFCredential, key *ecdsa.PrivateKey, opts *SignOptions) (*credential.RDFCredential, error) {
 	if key == nil {
 		return nil, fmt.Errorf("private key is nil")
 	}
 	// Wrap the raw key and delegate to SignWithSigner
 	wrapper := vccrypto.NewECDSAKeyWrapper(key)
-	return s.SignWithSigner(cred, wrapper, opts)
+	return s.SignWithSigner(ctx, cred, wrapper, opts)
 }
 
 // SignWithSigner signs a credential using ecdsa-rdfc-2019 with a VCSigner.
-// This method supports both raw keys (via wrappers) and HSM-based keys (via pki.RawSigner).
-func (s *Suite) SignWithSigner(cred *credential.RDFCredential, signer vccrypto.VCSigner, opts *SignOptions) (*credential.RDFCredential, error) {
+// This method supports both raw keys (via wrappers) and HSM-backed keys (via pki.RawSigner).
+func (s *Suite) SignWithSigner(ctx context.Context, cred *credential.RDFCredential, signer vccrypto.VCSigner, opts *SignOptions) (*credential.RDFCredential, error) {
 	if cred == nil {
 		return nil, fmt.Errorf("credential is nil")
 	}
@@ -130,7 +130,7 @@ func (s *Suite) SignWithSigner(cred *credential.RDFCredential, signer vccrypto.V
 	combined := append(proofHashBytes[:], docHashBytes[:]...)
 
 	// 5. Sign using the VCSigner
-	signature, err := signer.SignDigest(context.Background(), combined)
+	signature, err := signer.SignDigest(ctx, combined)
 	if err != nil {
 		return nil, fmt.Errorf("failed to sign: %w", err)
 	}