Adding validation for request input and added more UTs.

.github/.codecov.yml

@@ -15,4 +15,4 @@ coverage:
   status:
     project:
       default:
-        target: 70%
+        target: 80%

cli/cli.go

@@ -17,7 +17,6 @@
 // 2. Read and unmarshal input
 // 3. Execute relevant plugin functions
 // 4. marshals output
-
 package cli
 
 import (
@@ -123,7 +122,7 @@ func (c *CLI) Execute(ctx context.Context, args []string) {
 func (c *CLI) printVersion(ctx context.Context) {
 	md := c.getMetadata(ctx, c.pl)
 
-	fmt.Printf("%s - %s\nVersion: %s \n", md.Name, md.Description, md.Version)
+	fmt.Printf("%s - %s\nVersion: %s\n", md.Name, md.Description, md.Version)
 }
 
 // validateArgs validate commands/arguments passed to executable.
@@ -135,11 +134,21 @@ func (c *CLI) validateArgs(ctx context.Context, args []string) {
 }
 
 // unmarshalRequest reads input from std.in and unmarshal it into given request struct
-func (c *CLI) unmarshalRequest(request any) error {
+func (c *CLI) unmarshalRequest(request plugin.Request) error {
 	if err := json.NewDecoder(os.Stdin).Decode(request); err != nil {
 		c.logger.Errorf("%s unmarshalling error :%v", reflect.TypeOf(request), err)
 		return plugin.NewJSONParsingError(plugin.ErrorMsgMalformedInput)
 	}
+
+	if err := request.Validate(); err != nil {
+		c.logger.Errorf("%s validation error :%v", reflect.TypeOf(request), err)
+		var plError *plugin.Error
+		if errors.As(err, &plError) {
+			return plugin.NewValidationErrorf("%s: %s", plugin.ErrorMsgMalformedInput, plError.Message)
+		}
+		return plugin.NewValidationErrorf("%s", plugin.ErrorMsgMalformedInput)
+	}
+
 	return nil
 }
 

cli/cli_test.go

@@ -24,7 +24,7 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/notaryproject/notation-plugin-framework-go/cli/internal/mock"
+	"github.com/notaryproject/notation-plugin-framework-go/internal/mock"
 	"github.com/notaryproject/notation-plugin-framework-go/plugin"
 )
 
@@ -34,7 +34,7 @@ var errorCli, _ = New(mock.NewPlugin(true))
 func TestNewWithLogger(t *testing.T) {
 	_, err := NewWithLogger(nil, &discardLogger{})
 	if err == nil {
-		t.Fatalf("NewWithLogger() expected error but not found")
+		t.Errorf("NewWithLogger() expected error but not found")
 	}
 }
 
@@ -56,7 +56,6 @@ func TestMarshalResponse(t *testing.T) {
 }
 
 func TestMarshalResponseError(t *testing.T) {
-
 	_, err := cli.marshalResponse(nil, fmt.Errorf("expected error thrown"))
 	assertErr(t, err, plugin.ErrorCodeGeneric)
 
@@ -89,17 +88,17 @@ func TestUnmarshalRequestError(t *testing.T) {
 	var request plugin.DescribeKeyRequest
 	err := cli.unmarshalRequest(&request)
 	if err == nil {
-		t.Fatalf("unmarshalRequest() expected error but not found")
+		t.Errorf("unmarshalRequest() expected error but not found")
 	}
 
 	plgErr, ok := err.(*plugin.Error)
 	if !ok {
-		t.Fatalf("unmarshalRequest() expected error of type plugin.Error but found %s", reflect.TypeOf(err))
+		t.Errorf("unmarshalRequest() expected error of type plugin.Error but found %s", reflect.TypeOf(err))
 	}
 
 	expectedErrStr := "{\"errorCode\":\"VALIDATION_ERROR\",\"errorMessage\":\"Input is not a valid JSON\"}"
 	if plgErr.Error() != expectedErrStr {
-		t.Fatalf("unmarshalRequest() expected error string to be %s but found %s", expectedErrStr, plgErr.Error())
+		t.Errorf("unmarshalRequest() expected error string to be %s but found %s", expectedErrStr, plgErr.Error())
 	}
 }
 
@@ -115,48 +114,54 @@ func TestGetMetadataError(t *testing.T) {
 	if e, ok := err.(*exec.ExitError); ok && !e.Success() {
 		return
 	}
-	t.Fatalf("process ran with err %v, want exit status 1", err)
+	t.Errorf("process ran with err %v, want exit status 1", err)
 }
 
 func TestExecuteSuccess(t *testing.T) {
 	sigGenCli, _ := New(mock.NewSigGeneratorPlugin(false))
 	tests := map[string]struct {
 		c  *CLI
+		in string
 		op string
 	}{
 		string(plugin.CommandGetMetadata): {
 			c:  cli,
+			in: "{}",
 			op: "{\"name\":\"Example Plugin\",\"description\":\"This is an description of example plugin. 🍺\",\"version\":\"1.0.0\",\"url\":\"https://example.com/notation/plugin\",\"capabilities\":[\"SIGNATURE_VERIFIER.TRUSTED_IDENTITY\",\"SIGNATURE_VERIFIER.REVOCATION_CHECK\",\"SIGNATURE_GENERATOR.ENVELOPE\"]}",
 		},
 		string(plugin.Version): {
 			c:  cli,
-			op: "Example Plugin - This is an description of example plugin. 🍺\nVersion: 1.0.0 \n",
+			in: "",
+			op: "Example Plugin - This is an description of example plugin. 🍺\nVersion: 1.0.0\n",
 		},
 		string(plugin.CommandGenerateEnvelope): {
 			c:  cli,
-			op: "{\"signatureEnvelope\":\"\",\"signatureEnvelopeType\":\"\",\"annotations\":{\"manifestAnntnKey1\":\"value1\"}}",
+			in: "{\"contractVersion\":\"1.0\",\"keyId\":\"someKeyId\",\"payloadType\":\"somePT\",\"signatureEnvelopeType\":\"someSET\",\"payload\":\"em9w\"}",
+			op: "{\"signatureEnvelope\":\"\",\"signatureEnvelopeType\":\"someSET\",\"annotations\":{\"manifestAnntnKey1\":\"value1\"}}",
 		},
 		string(plugin.CommandVerifySignature): {
 			c:  cli,
+			in: "{\"contractVersion\":\"1.0\",\"signature\":{\"criticalAttributes\":{\"contentType\":\"someCT\",\"signingScheme\":\"someSigningScheme\"},\"unprocessedAttributes\":null,\"certificateChain\":[\"emFw\",\"em9w\"]},\"trustPolicy\":{\"trustedIdentities\":null,\"signatureVerification\":[\"SIGNATURE_GENERATOR.RAW\"]}}",
 			op: "{\"verificationResults\":{\"SIGNATURE_VERIFIER.REVOCATION_CHECK\":{\"success\":true,\"reason\":\"Not revoked\"},\"SIGNATURE_VERIFIER.TRUSTED_IDENTITY\":{\"success\":true,\"reason\":\"Valid trusted Identity\"}},\"processedAttributes\":[]}",
 		},
 		string(plugin.CommandGenerateSignature): {
 			c:  sigGenCli,
+			in: "{\"contractVersion\":\"1.0\",\"keyId\":\"someKeyId\",\"keySpec\":\"EC-384\",\"hashAlgorithm\":\"SHA-384\",\"payload\":\"em9w\"}",
 			op: "{\"keyId\":\"someKeyId\",\"signature\":\"YWJjZA==\",\"signingAlgorithm\":\"RSASSA-PSS-SHA-256\",\"certificateChain\":[\"YWJjZA==\",\"d3h5eg==\"]}",
 		},
 		string(plugin.CommandDescribeKey): {
 			c:  sigGenCli,
+			in: "{\"contractVersion\":\"1.0\",\"keyId\":\"someKeyId\"}",
 			op: "{\"keyId\":\"someKeyId\",\"keySpec\":\"RSA-2048\"}",
 		},
 	}
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
-			closer := setupReader("{}")
+			closer := setupReader(test.in)
 			defer closer()
 			op := captureStdOut(func() {
 				test.c.Execute(context.Background(), []string{"notation", name})
 			})
-			fmt.Println(op)
 			if op != test.op {
 				t.Errorf("Execute() with '%s' args, expected '%s' but got '%s'", name, test.op, op)
 			}
@@ -206,6 +211,5 @@ func assertErr(t *testing.T, err error, code plugin.ErrorCode) {
 		}
 		t.Errorf("mismatch in error code: \n expected: %s\n actual : %s", code, plgErr.ErrCode)
 	}
-
 	t.Errorf("expected error of type PluginError but found %s", reflect.TypeOf(err))
 }

cli/utils.go

@@ -51,7 +51,7 @@ func getValidArgs(md *plugin.GetMetadataResponse) []string {
 
 // deliverError print to standard error and then return nonzero exit code
 func deliverError(message string) {
-	_, _ = fmt.Fprintln(os.Stderr, message)
+	_, _ = fmt.Fprint(os.Stderr, message)
 	os.Exit(1)
 }
 

example/plugin.go

@@ -91,10 +91,11 @@ func (p *ExamplePlugin) VerifySignature(ctx context.Context, req *plugin.VerifyS
 
 func (p *ExamplePlugin) GetMetadata(ctx context.Context, req *plugin.GetMetadataRequest) (*plugin.GetMetadataResponse, error) {
 	return &plugin.GetMetadataResponse{
-		Name:        "Example Plugin",
-		Description: "This is an description of example plugin. 🍺",
-		URL:         "https://example.com/notation/plugin",
-		Version:     "1.0.0",
+		SupportedContractVersions: []string{plugin.ContractVersion},
+		Name:                      "com.example.plugin",
+		Description:               "This is an description of example plugin",
+		URL:                       "https://example.com/notation/plugin",
+		Version:                   "1.0.0",
 		Capabilities: []plugin.Capability{
 			plugin.CapabilityEnvelopeGenerator,
 			plugin.CapabilityTrustedIdentityVerifier,

plugin/describekey.go

@@ -24,6 +24,19 @@ func (DescribeKeyRequest) Command() Command {
 	return CommandDescribeKey
 }
 
+// Validate validates DescribeKeyRequest struct
+func (r DescribeKeyRequest) Validate() error {
+	if r.ContractVersion == "" {
+		return NewValidationError("contractVersion cannot be empty")
+	}
+
+	if r.KeyID == "" {
+		return NewValidationError("keyId cannot be empty")
+	}
+
+	return nil
+}
+
 // DescribeKeyResponse is the response of a describe-key request.
 type DescribeKeyResponse struct {
 	// The same key id as passed in the request.

plugin/describekey_test.go

@@ -0,0 +1,72 @@
+// Copyright The Notary Project Authors.
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package plugin
+
+import (
+	"fmt"
+	"testing"
+)
+
+func TestDescribeKeyRequest_Validate(t *testing.T) {
+	reqs := []DescribeKeyRequest{
+		getDescribeKeyRequest(ContractVersion, "someKeyId"),
+		{
+			ContractVersion: ContractVersion,
+			KeyID:           "someKeyId",
+			PluginConfig:    map[string]string{"someKey": "someValue"}},
+	}
+
+	for _, req := range reqs {
+		if err := req.Validate(); err != nil {
+			t.Errorf("VerifySignatureRequest#Validate failed with error: %+v", err)
+		}
+	}
+}
+
+func TestDescribeKeyRequest_Validate_Error(t *testing.T) {
+	testCases := []struct {
+		name string
+		req  DescribeKeyRequest
+	}{
+		{name: "contractVersion", req: getDescribeKeyRequest("", "someKeyId")},
+		{name: "keyId", req: getDescribeKeyRequest(ContractVersion, "")},
+	}
+
+	for _, testcase := range testCases {
+		t.Run(testcase.name, func(t *testing.T) {
+			if err := testcase.req.Validate(); err != nil {
+				expMsg := fmt.Sprintf("{\"errorCode\":\"VALIDATION_ERROR\",\"errorMessage\":\"%s cannot be empty\"}", testcase.name)
+				if err.Error() != expMsg {
+					t.Errorf("expected error message '%s' but got '%s'", expMsg, err.Error())
+				}
+			} else {
+				t.Error("DescribeKeyRequest#Validate didn't returned error")
+			}
+		})
+	}
+}
+
+func TestDescribeKeyRequest_Command(t *testing.T) {
+	req := getDescribeKeyRequest(ContractVersion, "someKeyId")
+	if cmd := req.Command(); cmd != CommandDescribeKey {
+		t.Errorf("DescribeKeyRequest#Command, expected %s but returned %s", CommandDescribeKey, cmd)
+	}
+}
+
+func getDescribeKeyRequest(cv, kid string) DescribeKeyRequest {
+	return DescribeKeyRequest{
+		ContractVersion: cv,
+		KeyID:           kid,
+	}
+}

plugin/errors.go

@@ -34,8 +34,7 @@ const (
 	ErrorMsgMalformedOutputFmt string = "Failed to generate response. Error: %s"
 )
 
-// Error is used when the signature associated is no longer
-// valid.
+// Error is used to return a well-formed error response as per NotaryProject specification.
 type Error struct {
 	ErrCode  ErrorCode         `json:"errorCode"`
 	Message  string            `json:"errorMessage,omitempty"`
@@ -53,8 +52,8 @@ func NewGenericError(msg string) *Error {
 	return NewError(ErrorCodeGeneric, msg)
 }
 
-func NewGenericErrorf(format string, msg string) *Error {
-	return NewError(ErrorCodeGeneric, fmt.Sprintf(format, msg))
+func NewGenericErrorf(format string, msg ...any) *Error {
+	return NewError(ErrorCodeGeneric, fmt.Sprintf(format, msg...))
 }
 
 func NewUnsupportedError(msg string) *Error {
@@ -65,8 +64,8 @@ func NewValidationError(msg string) *Error {
 	return NewError(ErrorCodeValidation, msg)
 }
 
-func NewValidationErrorf(format string, msg string) *Error {
-	return NewError(ErrorCodeValidation, fmt.Sprintf(format, msg))
+func NewValidationErrorf(format string, msg ...any) *Error {
+	return NewError(ErrorCodeValidation, fmt.Sprintf(format, msg...))
 }
 
 func NewUnsupportedContractVersionError(version string) *Error {

plugin/errors_test.go

@@ -0,0 +1,65 @@
+// Copyright The Notary Project Authors.
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package plugin
+
+import (
+	"testing"
+)
+
+func TestNewError(t *testing.T) {
+	msg := "someMSg"
+	err := NewError(ErrorCodeValidation, msg)
+	if err != nil {
+		if ErrorCodeValidation != err.ErrCode {
+			t.Errorf("NewError, expected errorCode '%s' but found'%s'", ErrorCodeValidation, err.ErrCode)
+		}
+
+		if msg != err.Message {
+			t.Errorf("NewError, expected message'%s' but found '%s'", msg, err.Message)
+		}
+
+		if err.Metadata != nil {
+			t.Errorf("NewError, expected metadata to be nil but found '%s'", err.Metadata)
+		}
+
+		expError := "{\"errorCode\":\"VALIDATION_ERROR\",\"errorMessage\":\"someMSg\"}"
+		if expError != err.Error() {
+			t.Errorf("NewError#Error, expected error to be '%s' but found '%s'", expError, err.Error())
+
+		}
+
+	} else {
+		t.Error("NewError didn't return an error")
+	}
+}
+
+func TestErrorCodes(t *testing.T) {
+	testCases := []struct {
+		err     *Error
+		errCode ErrorCode
+	}{
+		{err: NewValidationError(""), errCode: ErrorCodeValidation},
+		{err: NewValidationErrorf("%s", ""), errCode: ErrorCodeValidation},
+		{err: NewUnsupportedError(""), errCode: ErrorCodeValidation},
+		{err: NewGenericError(""), errCode: ErrorCodeGeneric},
+		{err: NewGenericErrorf("%s", ""), errCode: ErrorCodeGeneric},
+		{err: NewJSONParsingError(""), errCode: ErrorCodeValidation},
+		{err: NewUnsupportedContractVersionError(""), errCode: ErrorCodeUnsupportedContractVersion},
+	}
+	for _, test := range testCases {
+		if test.errCode != test.err.ErrCode {
+			t.Errorf("Expected errorCode %s but found %s", test.errCode, test.err.ErrCode)
+		}
+	}
+}

plugin/metadata.go

@@ -23,6 +23,11 @@ func (GetMetadataRequest) Command() Command {
 	return CommandGetMetadata
 }
 
+// Validate validates GetMetadataRequest struct
+func (GetMetadataRequest) Validate() error {
+	return nil
+}
+
 // GetMetadataResponse provided by the plugin.
 type GetMetadataResponse struct {
 	Name                      string       `json:"name"`