fix(modules): validate modules at load instead of at match time

internal/modules/executor.go

@@ -622,6 +622,24 @@ func checkRegex(content string, patterns []string, condition string) bool {
 	return true
 }
 
+// validateExtractors rejects a regex extractor whose pattern fails to
+// compile, at load rather than at match time: runExtractors (and its dns/tcp
+// counterparts) compile lazily and skip a bad pattern, which would otherwise
+// leave the extractor silently and permanently empty.
+func validateExtractors(extractors []Extractor) error {
+	for i := range extractors {
+		if extractors[i].Type != "regex" {
+			continue
+		}
+		for _, pattern := range extractors[i].Regex {
+			if _, err := regexp.Compile(pattern); err != nil {
+				return fmt.Errorf("regex extractor pattern %q: %w", pattern, err)
+			}
+		}
+	}
+	return nil
+}
+
 // runExtractors extracts data from the response.
 func runExtractors(extractors []Extractor, resp *http.Response, body string) map[string]string {
 	if len(extractors) == 0 {

internal/modules/favicon.go

@@ -15,6 +15,7 @@ package modules
 import (
 	"fmt"
 	"math"
+	"regexp"
 	"strings"
 
 	"github.com/vmfunc/sif/internal/fingerprint"
@@ -67,11 +68,38 @@ func faviconEvidence(matchers []Matcher, body string) (string, bool) {
 	return fmt.Sprintf("favicon mmh3=%d", hash), true
 }
 
+// hasNonEmpty reports whether at least one entry in vals is non-empty.
+func hasNonEmpty(vals []string) bool {
+	for _, v := range vals {
+		if v != "" {
+			return true
+		}
+	}
+	return false
+}
+
 // validateMatchers fails favicon matchers that would silently never fire (no
-// hash, or one out of 32-bit range) and malformed range matchers at load
-// rather than at match time.
+// hash, or one out of 32-bit range), an unknown matcher type, an unparseable
+// regex and a malformed range matcher at load rather than at match time. an
+// empty word or regex list matches every response under the default AND
+// condition, so it is rejected here too. dns and tcp narrow the allowlist
+// further in their own validators.
 func validateMatchers(matchers []Matcher) error {
 	for i := range matchers {
+		switch matchers[i].Type {
+		case "word", "regex", "status", "favicon", "size", "range":
+		default:
+			return fmt.Errorf("matcher type %q is not supported (use word, regex, status, favicon, size, or range)", matchers[i].Type)
+		}
+
+		if matchers[i].Type == "regex" {
+			for _, pattern := range matchers[i].Regex {
+				if _, err := regexp.Compile(pattern); err != nil {
+					return fmt.Errorf("regex matcher pattern %q: %w", pattern, err)
+				}
+			}
+		}
+
 		if matchers[i].Type == "favicon" {
 			if len(matchers[i].Hash) == 0 {
 				return fmt.Errorf("favicon matcher requires at least one hash")
@@ -83,6 +111,14 @@ func validateMatchers(matchers []Matcher) error {
 			}
 		}
 
+		if matchers[i].Type == "word" && !hasNonEmpty(matchers[i].Words) {
+			return fmt.Errorf("word matcher requires at least one non-empty word")
+		}
+
+		if matchers[i].Type == "regex" && !hasNonEmpty(matchers[i].Regex) {
+			return fmt.Errorf("regex matcher requires at least one non-empty pattern")
+		}
+
 		if matchers[i].Type == "range" {
 			if matchers[i].Min == nil && matchers[i].Max == nil {
 				return fmt.Errorf("range matcher requires min or max")

internal/modules/favicon_test.go

@@ -197,6 +197,22 @@ func TestValidateMatchers(t *testing.T) {
 		{name: "favicon with no hash", matchers: []Matcher{{Type: "favicon"}}, wantErr: true},
 		{name: "out-of-range hash", matchers: []Matcher{{Type: "favicon", Hash: []int64{99999999999}}}, wantErr: true},
 		{name: "non-favicon ignored", matchers: []Matcher{{Type: "word", Words: []string{"x"}}}, wantErr: false},
+		{name: "word with no words rejected", matchers: []Matcher{{Type: "word"}}, wantErr: true},
+		{name: "word with nil words rejected", matchers: []Matcher{{Type: "word", Words: nil}}, wantErr: true},
+		{name: "word with only empty string rejected", matchers: []Matcher{{Type: "word", Words: []string{""}}}, wantErr: true},
+		{name: "word with only empty strings rejected", matchers: []Matcher{{Type: "word", Words: []string{"", ""}}}, wantErr: true},
+		{name: "word with one real word allowed", matchers: []Matcher{{Type: "word", Words: []string{"real"}}}, wantErr: false},
+		{name: "word with empty and real word allowed", matchers: []Matcher{{Type: "word", Words: []string{"", "real"}}}, wantErr: false},
+		{name: "regex with no patterns rejected", matchers: []Matcher{{Type: "regex"}}, wantErr: true},
+		{name: "regex with nil patterns rejected", matchers: []Matcher{{Type: "regex", Regex: nil}}, wantErr: true},
+		{name: "regex with only empty pattern rejected", matchers: []Matcher{{Type: "regex", Regex: []string{""}}}, wantErr: true},
+		{name: "regex with one real pattern allowed", matchers: []Matcher{{Type: "regex", Regex: []string{"real"}}}, wantErr: false},
+		{name: "unknown matcher type rejected", matchers: []Matcher{{Type: "words", Words: []string{"x"}}}, wantErr: true},
+		{name: "status matcher allowed", matchers: []Matcher{{Type: "status", Status: []int{200}}}, wantErr: false},
+		{name: "size matcher allowed", matchers: []Matcher{{Type: "size", Size: []int{100}}}, wantErr: false},
+		{name: "valid regex allowed", matchers: []Matcher{{Type: "regex", Regex: []string{`admin\d+`}}}, wantErr: false},
+		{name: "unclosed regex rejected", matchers: []Matcher{{Type: "regex", Regex: []string{"(unclosed"}}}, wantErr: true},
+		{name: "one bad pattern among good ones rejected", matchers: []Matcher{{Type: "regex", Regex: []string{`ok\d+`, "("}}}, wantErr: true},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

internal/modules/loader.go

@@ -17,6 +17,7 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/charmbracelet/log"
 	"github.com/vmfunc/sif/internal/output"
@@ -28,6 +29,10 @@ import (
 // reach a parent directory, and modules/ sits above this package). it stays nil
 // in builds and tests that don't import that package, so the loader simply falls
 // back to the filesystem as before.
+// sifModulePath is this project's go module path, used to recognise a source
+// checkout as the working directory. see inSifCheckout.
+const sifModulePath = "github.com/vmfunc/sif"
+
 var builtinFS fs.FS
 
 // SetBuiltinFS registers the embedded module filesystem. see builtinFS.
@@ -70,20 +75,32 @@ func resolveBuiltinDir() string {
 	if dir := firstExistingDir(builtinDirCandidates()); dir != "" {
 		return dir
 	}
+	if builtinFS != nil {
+		// an embedded set is the baseline; do not point the disk walk at the
+		// working directory just to have a path.
+		return ""
+	}
 	return "modules"
 }
 
 // builtinDirCandidates lists the directories to probe for built-in modules,
 // most specific first: next to the executable, the working directory (for
 // development), then the freedesktop system data dirs so packaged installs
 // (modules under /usr/share/sif) are found too.
+//
+// The bare "modules" candidate resolves against the working directory at
+// runtime, so it is only offered inside a sif checkout (or when the binary
+// carries no embedded set), keeping the edit-and-rerun flow without letting a
+// release binary trust a modules/ folder it happens to find.
 func builtinDirCandidates() []string {
 	candidates := make([]string, 0, 4)
 
 	if execPath, err := os.Executable(); err == nil {
 		candidates = append(candidates, filepath.Join(filepath.Dir(execPath), "modules"))
 	}
-	candidates = append(candidates, "modules")
+	if builtinFS == nil || inSifCheckout() {
+		candidates = append(candidates, "modules")
+	}
 
 	for _, dir := range dataDirs() {
 		candidates = append(candidates, filepath.Join(dir, "sif", "modules"))
@@ -92,6 +109,23 @@ func builtinDirCandidates() []string {
 	return candidates
 }
 
+// inSifCheckout reports whether the working directory is a sif source tree, by
+// reading the module path out of its go.mod. It is what separates "the developer
+// is running from the repo" from "the binary happens to be sitting next to some
+// other project's modules/ dir".
+func inSifCheckout() bool {
+	data, err := os.ReadFile("go.mod")
+	if err != nil {
+		return false
+	}
+	for _, line := range strings.Split(string(data), "\n") {
+		if rest, ok := strings.CutPrefix(strings.TrimSpace(line), "module "); ok {
+			return strings.TrimSpace(rest) == sifModulePath
+		}
+	}
+	return false
+}
+
 // dataDirs returns the freedesktop base data directories, honoring
 // $XDG_DATA_DIRS and falling back to the spec default when it is unset.
 func dataDirs() []string {
@@ -113,20 +147,17 @@ func firstExistingDir(candidates []string) string {
 // LoadAll discovers and loads all modules from both built-in
 // and user directories.
 func (l *Loader) LoadAll() error {
-	// Load built-in modules first, preferring an on-disk modules/ dir (dev tree
-	// or a release that ships the folder alongside the binary).
-	before := l.loaded
-	if err := l.loadDir(l.builtinDir, false); err != nil {
-		log.Debugf("No built-in modules found: %v", err)
-	}
-
-	// nothing on disk: fall back to the modules embedded in the binary so a bare
-	// `go install`ed sif still ships its built-in modules.
-	if l.loaded == before && l.embedded != nil {
+	// the embedded set is the baseline; the on-disk builtin dir layers over it.
+	// Register replaces by id rather than duplicating, so a disk module overrides
+	// only its own id and every other embedded module survives.
+	if l.embedded != nil {
 		if err := l.loadFS(l.embedded); err != nil {
 			log.Debugf("No embedded modules loaded: %v", err)
 		}
 	}
+	if err := l.loadDir(l.builtinDir, false); err != nil {
+		log.Debugf("No built-in modules found: %v", err)
+	}
 
 	// Load user modules (can override built-in)
 	if err := l.loadDir(l.userDir, true); err != nil {
@@ -143,11 +174,15 @@ func (l *Loader) LoadAll() error {
 	return nil
 }
 
-// loadDir loads modules from a directory.
+// loadDir loads modules from a directory. a per-entry error is logged and
+// skipped rather than returned: filepath.Walk aborts the whole walk on the
+// first error a walkFn returns, silently dropping every module sorted after
+// the bad entry.
 func (l *Loader) loadDir(dir string, userDefined bool) error {
 	return filepath.Walk(dir, func(path string, info os.FileInfo, err error) error {
 		if err != nil {
-			return err
+			log.Debugf("Skipping %s: %v", path, err)
+			return nil
 		}
 
 		if info.IsDir() {