fix(workflow)!: load tags steps when a subtemplate passes the filters

With a template filter set, a workflow step selected by tags loaded only
the templates that had its tags and passed the filter. When the filter
selected one of the step's subtemplates instead, the step loaded
nothing, and since subtemplates run only after their step matches, the
selected subtemplate never ran. Steps selected by template already load
for a loaded subtemplate (#7965), but the tag lookup never got that
signal, and it matched the step's tags and the filters in a single call,
so it could not skip the filters without also dropping the tags.

The tag lookup now selects templates by the step's tags first and then
filters them. A step loaded for its subtemplate used to skip every
filter that rejected a template without an error, so -eid, -es and -ept
did not apply to it while -etags and -et did. Both lookups now apply
only the exclusion filters to such a step, so every exclusion holds, as
it does against -itags since #7536.

The tag filter also let -id lift -eid for the same template, so a
workflow ran a template that a scan with the same flags skips. -eid now
wins, as in the metadata index filter.

BREAKING-CHANGE: WorkflowLoader.GetTemplatePathsByTags takes noValidate
Fixes: #7966
Refs: #7534
Signed-off-by: Dwi Siswanto <[email protected]>

DESIGN.md

@@ -36,7 +36,7 @@ It also specifies a `WorkflowLoader` interface that is used during workflow load
 
 ```go
 type WorkflowLoader interface {
-	GetTemplatePathsByTags(tags []string) []string
+	GetTemplatePathsByTags(tags []string, noValidate bool) []string
 	GetTemplatePaths(templatesList []string, noValidate bool) []string
 }
 ```

pkg/loader/workflow/workflow_loader.go

@@ -16,6 +16,8 @@ import (
 type workflowLoader struct {
 	pathFilter *filter.PathFilter
 	tagFilter  *templates.TagFilter
+	// the exclusions of tagFilter alone, see filters
+	exclusions *templates.TagFilter
 	options    *protocols.ExecutorOptions
 
 	// every workflow step that selects templates by tag searches the whole
@@ -45,12 +47,30 @@ func NewLoader(options *protocols.ExecutorOptions) (model.WorkflowLoader, error)
 	if err != nil {
 		return nil, err
 	}
+	// IncludeTags is not an inclusion filter: it lifts the exclusion of the
+	// same tags
+	exclusions, err := templates.NewTagFilter(&templates.TagFilterConfig{
+		ExcludeTags:       options.Options.ExcludeTags,
+		IncludeTags:       options.Options.IncludeTags,
+		ExcludeIds:        options.Options.ExcludeIds,
+		ExcludeSeverities: options.Options.ExcludeSeverities,
+		ExcludeProtocols:  options.Options.ExcludeProtocols,
+	})
+	if err != nil {
+		return nil, err
+	}
 	pathFilter := filter.NewPathFilter(&filter.PathFilterConfig{
 		IncludedTemplates: options.Options.IncludeTemplates,
 		ExcludedTemplates: options.Options.ExcludedTemplates,
 	}, options.Catalog)
 
-	loader := &workflowLoader{pathFilter: pathFilter, tagFilter: tagFilter, options: options, parsed: make(map[string]*templates.Template)}
+	loader := &workflowLoader{
+		pathFilter: pathFilter,
+		tagFilter:  tagFilter,
+		exclusions: exclusions,
+		options:    options,
+		parsed:     make(map[string]*templates.Template),
+	}
 	loader.templatesDirectoryPaths = sync.OnceValue(loader.listTemplatesDirectory)
 	return loader, nil
 }
@@ -85,20 +105,34 @@ func (w *workflowLoader) parseTemplate(path string) *templates.Template {
 	return template
 }
 
-func (w *workflowLoader) GetTemplatePathsByTags(templateTags []string) []string {
+func (w *workflowLoader) GetTemplatePathsByTags(templateTags []string, noValidate bool) []string {
 	var loadedTemplates []string
 	for _, path := range w.templatesDirectoryPaths() {
+		// the step's tags select the templates and the filters then decide
+		// which of them load, as for the templates of a template step
 		template := w.parseTemplate(path)
-		if template == nil {
+		if template == nil || !selectedByTags(template, templateTags) {
 			continue
 		}
-		if loaded, _ := templates.MatchLoadFilters(template, path, w.tagFilter, templateTags); loaded {
+		if matched, _ := templates.MatchLoadFilters(template, path, w.filters(noValidate), nil); matched {
 			loadedTemplates = append(loadedTemplates, path)
 		}
 	}
 	return loadedTemplates
 }
 
+// selectedByTags reports whether a workflow step that selects templates by
+// tags selects template, which it does when template has one of tags.
+// Workflows are never selected, as a workflow cannot run as a step.
+func selectedByTags(template *templates.Template, tags []string) bool {
+	if len(template.Workflows) > 0 {
+		return false
+	}
+	return slices.ContainsFunc(template.Info.Tags.ToSlice(), func(tag string) bool {
+		return slices.Contains(tags, tag)
+	})
+}
+
 func (w *workflowLoader) GetTemplatePaths(templatesList []string, noValidate bool) []string {
 	includedTemplates, errs := w.options.Catalog.GetTemplatesPath(templatesList)
 	for template, err := range errs {
@@ -108,12 +142,23 @@ func (w *workflowLoader) GetTemplatePaths(templatesList []string, noValidate boo
 
 	loadedTemplates := make([]string, 0, len(templatesPathMap))
 	for templatePath := range templatesPathMap {
-		matched, err := w.options.Parser.LoadTemplate(templatePath, w.tagFilter, nil, w.options.Catalog)
+		matched, err := w.options.Parser.LoadTemplate(templatePath, w.filters(noValidate), nil, w.options.Catalog)
 		if err != nil && !matched {
 			gologger.Warning().Msg(err.Error())
-		} else if matched || noValidate {
+		} else if matched {
 			loadedTemplates = append(loadedTemplates, templatePath)
 		}
 	}
 	return loadedTemplates
 }
+
+// filters returns the filters that decide which templates a workflow step
+// loads. With noValidate, set for a step whose subtemplates loaded, only the
+// exclusions apply: the step must load for its subtemplates to run, but an
+// excluded template never runs.
+func (w *workflowLoader) filters(noValidate bool) *templates.TagFilter {
+	if noValidate {
+		return w.exclusions
+	}
+	return w.tagFilter
+}

pkg/loader/workflow/workflow_loader_test.go

@@ -1,6 +1,8 @@
 package workflow
 
 import (
+	"maps"
+	"slices"
 	"testing"
 
 	"github.com/projectdiscovery/nuclei/v3/pkg/catalog"
@@ -8,8 +10,12 @@ import (
 	"github.com/projectdiscovery/nuclei/v3/pkg/model/types/severity"
 	"github.com/projectdiscovery/nuclei/v3/pkg/model/types/stringslice"
 	"github.com/projectdiscovery/nuclei/v3/pkg/protocols"
+	"github.com/projectdiscovery/nuclei/v3/pkg/protocols/dns"
 	"github.com/projectdiscovery/nuclei/v3/pkg/templates"
+	templateTypes "github.com/projectdiscovery/nuclei/v3/pkg/templates/types"
 	"github.com/projectdiscovery/nuclei/v3/pkg/types"
+	"github.com/projectdiscovery/nuclei/v3/pkg/utils/stats"
+	"github.com/projectdiscovery/nuclei/v3/pkg/workflows"
 	"github.com/stretchr/testify/require"
 )
 
@@ -63,9 +69,87 @@ func TestGetTemplatePathsByTagsReadsTemplatesDirectoryOnce(t *testing.T) {
 	require.NoError(t, err)
 	walksBeforeLookups := cat.walks
 
-	require.Equal(t, []string{"wordpress.yaml"}, loader.GetTemplatePathsByTags([]string{"wordpress"}))
-	require.Equal(t, []string{"jira.yaml"}, loader.GetTemplatePathsByTags([]string{"jira"}))
+	require.Equal(t, []string{"wordpress.yaml"}, loader.GetTemplatePathsByTags([]string{"wordpress"}, false))
+	require.Equal(t, []string{"jira.yaml"}, loader.GetTemplatePathsByTags([]string{"jira"}, false))
 
 	require.Equal(t, 1, cat.walks-walksBeforeLookups, "templates directory walked per lookup")
 	require.Equal(t, map[string]int{"wordpress.yaml": 1, "jira.yaml": 1}, parser.parses, "templates parsed per lookup")
 }
+
+func TestGetTemplatePathsByTagsNoValidateAppliesOnlyExclusions(t *testing.T) {
+	template := func(id string, tags ...string) *templates.Template {
+		return &templates.Template{ID: id, Info: model.Info{
+			Name:           id,
+			Authors:        stringslice.StringSlice{Value: "author"},
+			Tags:           stringslice.StringSlice{Value: tags},
+			SeverityHolder: severity.Holder{Severity: severity.Info},
+		}}
+	}
+	excludedSeverity := template("excluded-severity", "step")
+	excludedSeverity.Info.SeverityHolder.Severity = severity.Low
+	excludedProtocol := template("excluded-protocol", "step")
+	excludedProtocol.RequestsDNS = []*dns.Request{{}}
+	workflow := template("workflow", "step")
+	workflow.Workflows = []*workflows.WorkflowTemplate{{Template: "selected.yaml"}}
+	parser := &countingParser{
+		templates: map[string]*templates.Template{
+			"selected.yaml":          template("selected", "step"),
+			"included.yaml":          template("included", "step", "included"),
+			"excluded-tag.yaml":      template("excluded-tag", "step", "excluded"),
+			"excluded-id.yaml":       template("excluded-id", "step"),
+			"excluded-severity.yaml": excludedSeverity,
+			"excluded-protocol.yaml": excludedProtocol,
+			"untagged.yaml":          template("untagged", "other"),
+			"workflow.yaml":          workflow,
+		},
+		parses: make(map[string]int),
+	}
+	options := types.DefaultOptions()
+	options.IncludeIds = []string{"untagged"}
+	options.ExcludeTags = []string{"excluded", "included"}
+	options.IncludeTags = []string{"included"}
+	options.ExcludeIds = []string{"excluded-id"}
+	options.ExcludeSeverities = severity.Severities{severity.Low}
+	options.ExcludeProtocols = templateTypes.ProtocolTypes{templateTypes.DNSProtocol}
+	cat := &countingCatalog{paths: slices.Collect(maps.Keys(parser.templates))}
+	loader, err := NewLoader(&protocols.ExecutorOptions{Options: options, Catalog: cat, Parser: parser})
+	require.NoError(t, err)
+
+	require.Empty(t, loader.GetTemplatePathsByTags([]string{"step"}, false))
+	loaded := loader.GetTemplatePathsByTags([]string{"step"}, true)
+	require.ElementsMatch(t, []string{"selected.yaml", "included.yaml"}, loaded)
+}
+
+func TestGetTemplatePathsByTagsCountsSyntaxWarningsOfLoadedTemplates(t *testing.T) {
+	// a template without severity has a syntax warning
+	unrated := &templates.Template{ID: "unrated", Info: model.Info{
+		Name:    "unrated",
+		Authors: stringslice.StringSlice{Value: "author"},
+		Tags:    stringslice.StringSlice{Value: "step"},
+	}}
+	tests := []struct {
+		name     string
+		ids      []string
+		warnings int64
+	}{
+		{name: "loaded", warnings: 1},
+		{name: "filtered out", ids: []string{"other"}, warnings: 0},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			parser := &countingParser{
+				templates: map[string]*templates.Template{"unrated.yaml": unrated},
+				parses:    make(map[string]int),
+			}
+			options := types.DefaultOptions()
+			options.IncludeIds = tc.ids
+			cat := &countingCatalog{paths: []string{"unrated.yaml"}}
+			loader, err := NewLoader(&protocols.ExecutorOptions{Options: options, Catalog: cat, Parser: parser})
+			require.NoError(t, err)
+
+			before := stats.GetValue(templates.TemplateSyntaxWarningStats)
+			loader.GetTemplatePathsByTags([]string{"step"}, false)
+			require.Equal(t, tc.warnings, stats.GetValue(templates.TemplateSyntaxWarningStats)-before)
+		})
+	}
+}

pkg/model/workflow_loader.go

@@ -4,9 +4,13 @@ package model
 
 // WorkflowLoader is a loader interface required for workflow initialization.
 type WorkflowLoader interface {
-	// GetTemplatePathsByTags returns a list of template paths based on the provided tags from the templates directory
-	GetTemplatePathsByTags(tags []string) []string
+	// GetTemplatePathsByTags returns the paths of the templates in the
+	// templates directory that have one of tags. noValidate is as in
+	// GetTemplatePaths.
+	GetTemplatePathsByTags(tags []string, noValidate bool) []string
 
-	// GetTemplatePaths takes a list of templates and returns paths for them
+	// GetTemplatePaths takes a list of templates and returns paths for them.
+	// With noValidate, only the exclusion filters apply, such as excluded
+	// tags, IDs, severities, protocols and paths.
 	GetTemplatePaths(templatesList []string, noValidate bool) []string
 }

pkg/templates/compile_test.go

@@ -36,6 +36,7 @@ import (
 	"github.com/projectdiscovery/nuclei/v3/pkg/protocols/http"
 	"github.com/projectdiscovery/nuclei/v3/pkg/templates"
 	templatesigner "github.com/projectdiscovery/nuclei/v3/pkg/templates/signer"
+	"github.com/projectdiscovery/nuclei/v3/pkg/types"
 	"github.com/projectdiscovery/nuclei/v3/pkg/utils/stats"
 	"github.com/projectdiscovery/nuclei/v3/pkg/workflows"
 	"github.com/projectdiscovery/ratelimit"
@@ -439,6 +440,119 @@ workflows:
 	}
 }
 
+func Test_ParseWorkflowLoadsStepsOfFilteredSubtemplatesUnlessExcluded(t *testing.T) {
+	// tags steps search the templates directory
+	templatesDir := t.TempDir()
+	cfg := config.DefaultConfig
+	previousTemplatesDir := cfg.TemplatesDirectory
+	t.Cleanup(func() { cfg.SetTemplatesDir(previousTemplatesDir) })
+	cfg.SetTemplatesDir(templatesDir)
+
+	path := func(id string) string { return filepath.Join(templatesDir, id+".yaml") }
+	for _, tpl := range []struct{ id, severity, tag string }{
+		{"BA", "low", "ba-tag"},
+		{"L1", "info", "l1-tag"},
+		{"other", "info", "other-tag"},
+	} {
+		err := os.WriteFile(path(tpl.id), fmt.Appendf(nil, `id: %s
+
+info:
+  name: %s
+  author: pdteam
+  severity: %s
+  tags: %s
+
+http:
+  - method: GET
+    path:
+      - "{{BaseURL}}/%s"
+`, tpl.id, tpl.id, tpl.severity, tpl.tag, tpl.id), 0o600)
+		require.NoError(t, err)
+	}
+	workflowPath := filepath.Join(t.TempDir(), "workflow.yaml")
+	err := os.WriteFile(workflowPath, fmt.Appendf(nil, `id: workflow-step-filters
+
+info:
+  name: Workflow Step Filters
+  author: pdteam
+  severity: info
+
+workflows:
+  - tags: ba-tag
+    subtemplates:
+      - template: %q
+  - template: %q
+    subtemplates:
+      - template: %q
+`, path("L1"), path("BA"), path("L1")), 0o600)
+	require.NoError(t, err)
+
+	tests := []struct {
+		name   string
+		filter func(options *types.Options)
+		// executers of the tags step, its subtemplate, the template step and
+		// its subtemplate
+		loaded []int
+	}{
+		{name: "no filters", loaded: []int{1, 1, 1, 1}},
+		{name: "id of the subtemplate", filter: func(o *types.Options) {
+			o.IncludeIds = []string{"L1"}
+		}, loaded: []int{1, 1, 1, 1}},
+		{name: "tag of the subtemplate", filter: func(o *types.Options) {
+			o.Tags = []string{"l1-tag"}
+		}, loaded: []int{1, 1, 1, 1}},
+		{name: "id of the step template", filter: func(o *types.Options) {
+			o.IncludeIds = []string{"BA"}
+		}, loaded: []int{1, 0, 1, 0}},
+		{name: "excluded tag of the step template", filter: func(o *types.Options) {
+			o.IncludeIds, o.ExcludeTags = []string{"L1"}, []string{"ba-tag"}
+		}, loaded: []int{0, 1, 0, 1}},
+		{name: "included excluded tag of the step template", filter: func(o *types.Options) {
+			o.IncludeIds, o.ExcludeTags, o.IncludeTags = []string{"L1"}, []string{"ba-tag"}, []string{"ba-tag"}
+		}, loaded: []int{1, 1, 1, 1}},
+		{name: "excluded id of the step template", filter: func(o *types.Options) {
+			o.IncludeIds, o.ExcludeIds = []string{"L1"}, []string{"BA"}
+		}, loaded: []int{0, 1, 0, 1}},
+		{name: "excluded id of the step template and its subtemplate", filter: func(o *types.Options) {
+			o.IncludeIds, o.ExcludeIds = []string{"BA", "L1"}, []string{"BA"}
+		}, loaded: []int{0, 1, 0, 1}},
+		{name: "excluded id of the included step template", filter: func(o *types.Options) {
+			o.IncludeIds, o.ExcludeIds = []string{"BA"}, []string{"BA"}
+		}, loaded: []int{0, 0, 0, 0}},
+		{name: "excluded severity of the step template", filter: func(o *types.Options) {
+			o.IncludeIds, o.ExcludeSeverities = []string{"L1"}, severity.Severities{severity.Low}
+		}, loaded: []int{0, 1, 0, 1}},
+		{name: "excluded path of the step template", filter: func(o *types.Options) {
+			o.IncludeIds, o.ExcludedTemplates = []string{"L1"}, []string{path("BA")}
+		}, loaded: []int{0, 1, 0, 1}},
+		{name: "author of no template", filter: func(o *types.Options) {
+			o.Authors = []string{"nobody"}
+		}, loaded: []int{0, 0, 0, 0}},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			setup()
+			options := executerOpts.Options.Copy()
+			if tc.filter != nil {
+				tc.filter(options)
+			}
+			executerOpts.Options = options
+			workflowLoader, err := workflow.NewLoader(executerOpts)
+			require.NoError(t, err)
+			executerOpts.WorkflowLoader = workflowLoader
+
+			got, err := templates.Parse(workflowPath, nil, executerOpts)
+			require.NoError(t, err)
+			require.NotNil(t, got.CompiledWorkflow)
+			loaded := make([]int, 0, 2*len(got.CompiledWorkflow.Workflows))
+			for _, step := range got.CompiledWorkflow.Workflows {
+				loaded = append(loaded, len(step.Executers), len(step.Subtemplates[0].Executers))
+			}
+			require.Equal(t, tc.loaded, loaded)
+		})
+	}
+}
+
 func Test_ParseWorkflowLoadsTemplateWithUnknownMatcherCondition(t *testing.T) {
 	setup()
 	dir := t.TempDir()

pkg/templates/tag_filter.go

@@ -436,12 +436,13 @@ func NewTagFilter(config *TagFilterConfig) (*TagFilter, error) {
 			}
 		}
 	}
+	// an included id does not lift its exclusion: exclusions apply however a
+	// template is selected
 	for _, id := range config.IncludeIds {
 		for _, val := range splitCommaTrim(id) {
 			if _, ok := filter.allowedIds[val]; !ok {
 				filter.allowedIds[val] = struct{}{}
 			}
-			delete(filter.excludeIds, val)
 		}
 	}
 	for _, includeCondition := range config.IncludeConditions {

pkg/templates/tag_filter_test.go

@@ -185,6 +185,16 @@ func TestTagBasedFilter(t *testing.T) {
 		matched, _ = filter.Match(dummyTemplate, nil)
 		require.False(t, matched, "could not get correct match")
 	})
+	t.Run("match-exclude-id-over-include-id", func(t *testing.T) {
+		filter, err := NewTagFilter(&TagFilterConfig{
+			IncludeIds: []string{"cve-test"},
+			ExcludeIds: []string{"cve-test"},
+		})
+		require.NoError(t, err)
+		dummyTemplate := newDummyTemplate("cve-test", nil, nil, severity.Low, types.HTTPProtocol)
+		matched, _ := filter.Match(dummyTemplate, nil)
+		require.False(t, matched, "an excluded id must not match when it is also included")
+	})
 	t.Run("match-exclude-type", func(t *testing.T) {
 		filter, err := NewTagFilter(&TagFilterConfig{
 			ExcludeProtocols: []types.ProtocolType{types.HTTPProtocol},

pkg/templates/workflows.go

@@ -66,7 +66,7 @@ func parseWorkflowTemplate(workflow *workflows.WorkflowTemplate, preprocessor Pr
 
 	subTemplateTags := workflow.Tags
 	if !subTemplateTags.IsEmpty() {
-		paths = loader.GetTemplatePathsByTags(subTemplateTags.ToSlice())
+		paths = loader.GetTemplatePathsByTags(subTemplateTags.ToSlice(), noValidate)
 	} else {
 		paths = loader.GetTemplatePaths([]string{workflow.Template}, noValidate)
 	}