From f0e44d681008bab9602c541022b7de7497f94f77 Mon Sep 17 00:00:00 2001
From: Ben Vincent <ben@unkin.net>
Date: Thu, 2 Jul 2026 20:19:27 +1000
Subject: [PATCH] fix: blocklist fails open when a regex fails to compile (#87)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes #72

## Why
`compilePatterns` silently discards any pattern that fails to compile. A typo in a blocklist entry therefore turns a deny rule into a no-op — a fail-open with security impact.

## Changes
- Add `Remote.ValidatePatterns`, which compiles every pattern list (patterns, blocklist, mutable/immutable patterns, ban_tags) and returns an error on the first invalid regex.
- Reject invalid patterns with 400 at remote create and update time.
- Unit test for valid and invalid patterns.

## Validation
- `go test ./pkg/models/` and `make e2e` pass.

Reviewed-on: https://git.unkin.net/unkin/artifactapi/pulls/87
Co-authored-by: Ben Vincent <ben@unkin.net>
Co-committed-by: Ben Vincent <ben@unkin.net>
---
 internal/api/v2/remotes.go |  8 ++++++++
 pkg/models/remote.go       | 25 +++++++++++++++++++++++++
 pkg/models/remote_test.go  | 19 +++++++++++++++++++
 3 files changed, 52 insertions(+)
 create mode 100644 pkg/models/remote_test.go

diff --git a/internal/api/v2/remotes.go b/internal/api/v2/remotes.go
index 7b927ad..6ebd8ba 100644
--- a/internal/api/v2/remotes.go
+++ b/internal/api/v2/remotes.go
@@ -69,6 +69,10 @@ func (h *RemotesHandler) create(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "base_url is required for remote repositories", http.StatusBadRequest)
 		return
 	}
+	if err := remote.ValidatePatterns(); err != nil {
+		http.Error(w, err.Error(), http.StatusBadRequest)
+		return
+	}
 	if err := h.db.CreateRemote(r.Context(), &remote); err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
@@ -84,6 +88,10 @@ func (h *RemotesHandler) update(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	remote.Name = name
+	if err := remote.ValidatePatterns(); err != nil {
+		http.Error(w, err.Error(), http.StatusBadRequest)
+		return
+	}
 	if err := h.db.UpdateRemote(r.Context(), &remote); err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
diff --git a/pkg/models/remote.go b/pkg/models/remote.go
index 6e380e8..953336d 100644
--- a/pkg/models/remote.go
+++ b/pkg/models/remote.go
@@ -2,6 +2,7 @@ package models
 
 import (
 	"fmt"
+	"regexp"
 	"time"
 )
 
@@ -66,6 +67,30 @@ type Remote struct {
 	UpdatedAt time.Time `json:"updated_at"`
 }
 
+// ValidatePatterns ensures every configured regex compiles. Storing an
+// invalid pattern would otherwise be silently dropped at match time, which
+// for the blocklist is a fail-open: a mistyped deny rule becomes a no-op.
+func (r *Remote) ValidatePatterns() error {
+	groups := []struct {
+		field    string
+		patterns []string
+	}{
+		{"patterns", r.Patterns},
+		{"blocklist", r.Blocklist},
+		{"mutable_patterns", r.MutablePatterns},
+		{"immutable_patterns", r.ImmutablePatterns},
+		{"ban_tags", r.BanTags},
+	}
+	for _, g := range groups {
+		for _, p := range g.patterns {
+			if _, err := regexp.Compile(p); err != nil {
+				return fmt.Errorf("invalid regex in %s: %q: %w", g.field, p, err)
+			}
+		}
+	}
+	return nil
+}
+
 type RemoteWithStats struct {
 	Remote
 	Stats RemoteStats `json:"stats"`
diff --git a/pkg/models/remote_test.go b/pkg/models/remote_test.go
new file mode 100644
index 0000000..e46de7b
--- /dev/null
+++ b/pkg/models/remote_test.go
@@ -0,0 +1,19 @@
+package models
+
+import "testing"
+
+func TestRemote_ValidatePatterns(t *testing.T) {
+	valid := &Remote{
+		Patterns:          []string{`.*\.tar\.gz$`},
+		Blocklist:         []string{`^secret/`},
+		ImmutablePatterns: []string{`\.rpm$`},
+	}
+	if err := valid.ValidatePatterns(); err != nil {
+		t.Fatalf("expected valid patterns, got %v", err)
+	}
+
+	bad := &Remote{Blocklist: []string{`[unterminated`}}
+	if err := bad.ValidatePatterns(); err == nil {
+		t.Fatal("expected error for invalid blocklist regex, got nil")
+	}
+}