fix: PR 755 follow-up regression test and code quality

pkg/signatory/signatory.go

@@ -686,58 +686,68 @@ func fixupRequests(req []string) {
 
 func fixupOperations(ops []string) {
 	for i, o := range ops {
-		switch o {
+		base, subKind, hasSubKind := strings.Cut(o, ":")
+		switch base {
 		case "endorsement":
-			ops[i] = "attestation"
+			base = "attestation"
 		case "preendorsement":
-			ops[i] = "preattestation"
+			base = "preattestation"
 		case "double_endorsement_evidence":
-			ops[i] = "double_attestation_evidence"
+			base = "double_attestation_evidence"
 		case "double_preendorsement_evidence":
-			ops[i] = "double_preattestation_evidence"
+			base = "double_preattestation_evidence"
+		}
+		if hasSubKind {
+			ops[i] = base + ":" + subKind
+		} else {
+			ops[i] = base
 		}
 	}
 	sort.Strings(ops)
 }
 
 func checkRequestKind(allowedKinds []string) error {
-	avilKinds := proto.ListSignRequests()
+	availKinds := proto.ListSignRequests()
 	for _, kind := range allowedKinds {
-		if !slices.Contains(avilKinds, kind) {
+		if !slices.Contains(availKinds, kind) {
 			return fmt.Errorf("invalid request kind `%s` in `allow` list", kind)
 		}
 	}
 	return nil
 }
 
 func checkOperationKind(allowedKinds []string) error {
-	avilKinds := append(proto.ListGenericOperations(), proto.ListPseudoOperations()...)
+	availKinds := append(proto.ListGenericOperations(), proto.ListPseudoOperations()...)
 	for _, kind := range allowedKinds {
 		// Handle ballot sub-kinds (ballot:yay, ballot:nay, ballot:pass)
 		if strings.Contains(kind, ":") {
 			parts := strings.SplitN(kind, ":", 2)
 			base := parts[0]
 			subKind := parts[1]
-			
+
 			// Only ballot operations support sub-kind syntax
 			if base != "ballot" {
 				return fmt.Errorf("invalid operation kind `%s` in `allow.generic` list", kind)
 			}
-			
+
 			// Validate the ballot sub-kind
-			validBallotKinds := []string{"yay", "nay", "pass"}
+			validBallotKinds := []string{
+				core.BallotYay.String(),
+				core.BallotNay.String(),
+				core.BallotPass.String(),
+			}
 			if !slices.Contains(validBallotKinds, subKind) {
 				return fmt.Errorf("invalid operation kind `%s` in `allow.generic` list", kind)
 			}
-			
+
 			// Ensure base "ballot" is in the valid operations list
-			if !slices.Contains(avilKinds, base) {
+			if !slices.Contains(availKinds, base) {
 				return fmt.Errorf("invalid operation kind `%s` in `allow.generic` list", kind)
 			}
 			continue
 		}
-		
-		if !slices.Contains(avilKinds, kind) {
+
+		if !slices.Contains(availKinds, kind) {
 			return fmt.Errorf("invalid operation kind `%s` in `allow.generic` list", kind)
 		}
 	}

pkg/signatory/signatory_test.go

@@ -445,6 +445,94 @@ func TestPolicy(t *testing.T) {
 	}
 }
 
+func TestBallotSubkindPolicy_EndToEnd(t *testing.T) {
+	priv, err := crypt.ParsePrivateKey([]byte(privateKey))
+	require.NoError(t, err)
+	pk := priv.Public()
+
+	// Build config with ballot:nay and ballot:pass sub-kind policy
+	cfg := hashmap.NewPublicKeyHashMap([]hashmap.PublicKeyKV[*config.TezosPolicy]{
+		{
+			Key: pk.Hash(),
+			Val: &config.TezosPolicy{
+				Allow:       map[string][]string{"generic": {"ballot:nay", "ballot:pass"}},
+				LogPayloads: true,
+			},
+		},
+	})
+
+	// PreparePolicy must accept sub-kind syntax (this is where PR 657 broke)
+	policy, err := signatory.PreparePolicy(cfg)
+	require.NoError(t, err)
+
+	conf := signatory.Config{
+		Vaults:    map[string]*config.VaultConfig{"mock": {Driver: "mock"}},
+		Watermark: watermark.Ignore{},
+		VaultFactory: vault.FactoryFunc(func(context.Context, string, *yaml.Node, config.GlobalContext) (vault.Vault, error) {
+			return memory.NewUnparsed([]*memory.UnparsedKey{{Data: privateKey}}, "Mock"), nil
+		}),
+		Policy: policy,
+	}
+
+	s, err := signatory.New(context.Background(), &conf)
+	require.NoError(t, err)
+	require.NoError(t, s.Unlock(context.Background()))
+
+	t.Run("ballot:nay accepted", func(t *testing.T) {
+		var buf bytes.Buffer
+		var req latest.SignRequest = &latest.GenericOperationSignRequest{
+			Branch: &tz.BlockHash{},
+			Contents: []latest.GenericOperationSignRequestOperationContents{
+				&latest.Ballot{
+					Source:   &tz.Ed25519PublicKeyHash{1, 2, 3},
+					Period:   0,
+					Proposal: &tz.ProtocolHash{},
+					Ballot:   core.BallotNay,
+				},
+			},
+		}
+		require.NoError(t, encoding.Encode(&buf, &req))
+		_, err := s.Sign(context.Background(), &signatory.SignRequest{PublicKeyHash: pk.Hash(), Message: buf.Bytes()})
+		require.NoError(t, err)
+	})
+
+	t.Run("ballot:pass accepted", func(t *testing.T) {
+		var buf bytes.Buffer
+		var req latest.SignRequest = &latest.GenericOperationSignRequest{
+			Branch: &tz.BlockHash{},
+			Contents: []latest.GenericOperationSignRequestOperationContents{
+				&latest.Ballot{
+					Source:   &tz.Ed25519PublicKeyHash{1, 2, 3},
+					Period:   0,
+					Proposal: &tz.ProtocolHash{},
+					Ballot:   core.BallotPass,
+				},
+			},
+		}
+		require.NoError(t, encoding.Encode(&buf, &req))
+		_, err := s.Sign(context.Background(), &signatory.SignRequest{PublicKeyHash: pk.Hash(), Message: buf.Bytes()})
+		require.NoError(t, err)
+	})
+
+	t.Run("ballot:yay rejected", func(t *testing.T) {
+		var buf bytes.Buffer
+		var req latest.SignRequest = &latest.GenericOperationSignRequest{
+			Branch: &tz.BlockHash{},
+			Contents: []latest.GenericOperationSignRequestOperationContents{
+				&latest.Ballot{
+					Source:   &tz.Ed25519PublicKeyHash{1, 2, 3},
+					Period:   0,
+					Proposal: &tz.ProtocolHash{},
+					Ballot:   core.BallotYay,
+				},
+			},
+		}
+		require.NoError(t, encoding.Encode(&buf, &req))
+		_, err := s.Sign(context.Background(), &signatory.SignRequest{PublicKeyHash: pk.Hash(), Message: buf.Bytes()})
+		require.EqualError(t, err, "operation `ballot:yay' is not allowed")
+	})
+}
+
 func TestProofOfPossession(t *testing.T) {
 	type testCase struct {
 		title    string
@@ -839,6 +927,18 @@ func TestOperationKindCheck(t *testing.T) {
 			ops:     []string{"transaction:foo"},
 			wantErr: "invalid operation kind `transaction:foo` in `allow.generic` list",
 		},
+		// Deprecated aliases with sub-kind syntax: fixup normalizes the base
+		// before validation, so errors report the canonical name.
+		{
+			name:    "endorsement:foo normalized to attestation:foo then rejected",
+			ops:     []string{"endorsement:foo"},
+			wantErr: "invalid operation kind `attestation:foo` in `allow.generic` list",
+		},
+		{
+			name:    "preendorsement:foo normalized to preattestation:foo then rejected",
+			ops:     []string{"preendorsement:foo"},
+			wantErr: "invalid operation kind `preattestation:foo` in `allow.generic` list",
+		},
 	}
 
 	invalidOps := []string{"attestation", "attestation_with_dal", "preattestation"}