fix(chromium): better default security and DX for allow / deny lists

This commit is contained in:
Julien Neuhart
2026-03-27 09:47:21 +01:00
parent bd6d92be9b
commit 06b2b2e10c
13 changed files with 358 additions and 56 deletions

View File

@@ -12,40 +12,68 @@ import (
// ErrFiltered happens if a value is filtered by the [FilterDeadline] function.
var ErrFiltered = errors.New("value filtered")
// FilterDeadline checks if given value is allowed and not denied according to
// regex patterns. It returns a [context.DeadlineExceeded] if it takes too long
// to process.
func FilterDeadline(allowed, denied *regexp2.Regexp, s string, deadline time.Time) error {
// FIXME: not ideal to compile everytime, but is there another way to create a clone?
if allowed.String() != "" {
allow := regexp2.MustCompile(allowed.String(), 0)
allow.MatchTimeout = time.Until(deadline)
// RegexpToSlice wraps a single [regexp2.Regexp] into a slice suitable for
// [FilterDeadline]. If the regexp pattern is empty, it returns nil (meaning no
// filtering).
func RegexpToSlice(r *regexp2.Regexp) []*regexp2.Regexp {
if r == nil || r.String() == "" {
return nil
}
ok, err := allow.MatchString(s)
if err != nil {
if time.Now().After(deadline) {
return context.DeadlineExceeded
return []*regexp2.Regexp{r}
}
// FilterDeadline checks if the given value is allowed and not denied according
// to regex patterns. The allowed list uses OR semantics (value must match at
// least one pattern). The denied list uses OR semantics (value is denied if it
// matches any pattern). It returns a [context.DeadlineExceeded] if it takes
// too long to process.
func FilterDeadline(allowed, denied []*regexp2.Regexp, s string, deadline time.Time) error {
if len(allowed) > 0 {
matched := false
for _, pattern := range allowed {
// FIXME: not ideal to compile everytime, but is there another way to create a clone?
clone := regexp2.MustCompile(pattern.String(), 0)
clone.MatchTimeout = time.Until(deadline)
ok, err := clone.MatchString(s)
if err != nil {
if time.Now().After(deadline) {
return context.DeadlineExceeded
}
return fmt.Errorf("'%s' cannot handle '%s': %w", clone.String(), s, err)
}
if ok {
matched = true
break
}
return fmt.Errorf("'%s' cannot handle '%s': %w", allow.String(), s, err)
}
if !ok {
return fmt.Errorf("'%s' does not match the expression from the allowed list: %w", s, ErrFiltered)
if !matched {
return fmt.Errorf("'%s' does not match any expression from the allowed list: %w", s, ErrFiltered)
}
}
if denied.String() != "" {
deny := regexp2.MustCompile(denied.String(), 0)
deny.MatchTimeout = time.Until(deadline)
if len(denied) > 0 {
for _, pattern := range denied {
clone := regexp2.MustCompile(pattern.String(), 0)
clone.MatchTimeout = time.Until(deadline)
ok, err := deny.MatchString(s)
if err != nil {
if time.Now().After(deadline) {
return context.DeadlineExceeded
ok, err := clone.MatchString(s)
if err != nil {
if time.Now().After(deadline) {
return context.DeadlineExceeded
}
return fmt.Errorf("'%s' cannot handle '%s': %w", clone.String(), s, err)
}
if ok {
return fmt.Errorf("'%s' matches the expression from the denied list: %w", s, ErrFiltered)
}
return fmt.Errorf("'%s' cannot handle '%s': %w", deny.String(), s, err)
}
if ok {
return fmt.Errorf("'%s' matches the expression from the denied list: %w", s, ErrFiltered)
}
}

View File

@@ -12,8 +12,8 @@ import (
func TestFilterDeadline(t *testing.T) {
for _, tc := range []struct {
scenario string
allowed *regexp2.Regexp
denied *regexp2.Regexp
allowed []*regexp2.Regexp
denied []*regexp2.Regexp
s string
deadline time.Time
expectError bool
@@ -21,17 +21,17 @@ func TestFilterDeadline(t *testing.T) {
}{
{
scenario: "DeadlineExceeded (allowed)",
allowed: regexp2.MustCompile("foo", 0),
denied: regexp2.MustCompile("", 0),
allowed: []*regexp2.Regexp{regexp2.MustCompile("foo", 0)},
denied: nil,
s: "foo",
deadline: time.Now().Add(time.Duration(-1) * time.Hour),
expectError: true,
expectedError: context.DeadlineExceeded,
},
{
scenario: "ErrFiltered (allowed)",
allowed: regexp2.MustCompile("foo", 0),
denied: regexp2.MustCompile("", 0),
scenario: "ErrFiltered (allowed, no match)",
allowed: []*regexp2.Regexp{regexp2.MustCompile("foo", 0)},
denied: nil,
s: "bar",
deadline: time.Now().Add(time.Duration(5) * time.Second),
expectError: true,
@@ -39,8 +39,8 @@ func TestFilterDeadline(t *testing.T) {
},
{
scenario: "DeadlineExceeded (denied)",
allowed: regexp2.MustCompile("", 0),
denied: regexp2.MustCompile("foo", 0),
allowed: nil,
denied: []*regexp2.Regexp{regexp2.MustCompile("foo", 0)},
s: "foo",
deadline: time.Now().Add(time.Duration(-1) * time.Hour),
expectError: true,
@@ -48,21 +48,55 @@ func TestFilterDeadline(t *testing.T) {
},
{
scenario: "ErrFiltered (denied)",
allowed: regexp2.MustCompile("", 0),
denied: regexp2.MustCompile("foo", 0),
allowed: nil,
denied: []*regexp2.Regexp{regexp2.MustCompile("foo", 0)},
s: "foo",
deadline: time.Now().Add(time.Duration(5) * time.Second),
expectError: true,
expectedError: ErrFiltered,
},
{
scenario: "success",
allowed: regexp2.MustCompile("", 0),
denied: regexp2.MustCompile("", 0),
scenario: "success (empty lists)",
allowed: nil,
denied: nil,
s: "foo",
deadline: time.Now().Add(time.Duration(5) * time.Second),
expectError: false,
},
{
scenario: "multi-pattern allow list, second matches",
allowed: []*regexp2.Regexp{regexp2.MustCompile("^https://", 0), regexp2.MustCompile("^file:///tmp/", 0)},
denied: nil,
s: "file:///tmp/abc/index.html",
deadline: time.Now().Add(time.Duration(5) * time.Second),
expectError: false,
},
{
scenario: "multi-pattern allow list, none matches",
allowed: []*regexp2.Regexp{regexp2.MustCompile("^https://", 0), regexp2.MustCompile("^ftp://", 0)},
denied: nil,
s: "file:///tmp/abc/index.html",
deadline: time.Now().Add(time.Duration(5) * time.Second),
expectError: true,
expectedError: ErrFiltered,
},
{
scenario: "multi-pattern deny list, second matches",
allowed: nil,
denied: []*regexp2.Regexp{regexp2.MustCompile("^ftp://", 0), regexp2.MustCompile("^file:.*", 0)},
s: "file:///etc/passwd",
deadline: time.Now().Add(time.Duration(5) * time.Second),
expectError: true,
expectedError: ErrFiltered,
},
{
scenario: "https URL passes deny list targeting file://",
allowed: nil,
denied: []*regexp2.Regexp{regexp2.MustCompile("^file:.*", 0)},
s: "https://example.com",
deadline: time.Now().Add(time.Duration(5) * time.Second),
expectError: false,
},
} {
t.Run(tc.scenario, func(t *testing.T) {
err := FilterDeadline(tc.allowed, tc.denied, tc.s, tc.deadline)
@@ -81,3 +115,45 @@ func TestFilterDeadline(t *testing.T) {
})
}
}
func TestRegexpToSlice(t *testing.T) {
for _, tc := range []struct {
scenario string
input *regexp2.Regexp
expectNil bool
expectLen int
}{
{
scenario: "nil regexp",
input: nil,
expectNil: true,
},
{
scenario: "empty regexp",
input: regexp2.MustCompile("", 0),
expectNil: true,
},
{
scenario: "non-empty regexp",
input: regexp2.MustCompile("^file:.*", 0),
expectNil: false,
expectLen: 1,
},
} {
t.Run(tc.scenario, func(t *testing.T) {
result := RegexpToSlice(tc.input)
if tc.expectNil && result != nil {
t.Fatalf("expected nil but got: %v", result)
}
if !tc.expectNil && result == nil {
t.Fatal("expected non-nil but got nil")
}
if !tc.expectNil && len(result) != tc.expectLen {
t.Fatalf("expected length %d but got %d", tc.expectLen, len(result))
}
})
}
}

View File

@@ -222,3 +222,32 @@ func (f *ParsedFlags) MustDeprecatedRegexp(deprecated string, newName string) *r
return f.MustRegexp(newName)
}
// MustRegexpSlice returns a slice of compiled regular expressions from a
// string-slice flag given by name. Empty strings are skipped.
// It panics if an error occurs.
func (f *ParsedFlags) MustRegexpSlice(name string) []*regexp2.Regexp {
vals := f.MustStringSlice(name)
var regexps []*regexp2.Regexp
for _, val := range vals {
if val == "" {
continue
}
regexps = append(regexps, regexp2.MustCompile(val, 0))
}
return regexps
}
// MustDeprecatedRegexpSlice returns the slice of compiled regular expressions
// of a deprecated flag if it was explicitly set or the slice of the new flag.
// It panics if an error occurs.
func (f *ParsedFlags) MustDeprecatedRegexpSlice(deprecated string, newName string) []*regexp2.Regexp {
if f.Changed(deprecated) {
return f.MustRegexpSlice(deprecated)
}
return f.MustRegexpSlice(newName)
}

View File

@@ -6,6 +6,7 @@ import (
"testing"
"time"
"github.com/dlclark/regexp2"
flag "github.com/spf13/pflag"
)
@@ -833,3 +834,120 @@ func TestParsedFlags_MustDeprecatedRegexp(t *testing.T) {
})
}
}
func TestParsedFlags_MustRegexpSlice(t *testing.T) {
fs := flag.NewFlagSet("tests", flag.ContinueOnError)
fs.StringSlice("foo", []string{}, "")
err := fs.Parse([]string{"--foo=^file:.*", "--foo=^ftp://.*"})
if err != nil {
t.Fatalf("expected no error but got: %v", err)
}
parsedFlags := ParsedFlags{FlagSet: fs}
for _, tc := range []struct {
scenario string
name string
expectPanic bool
expectLen int
}{
{
scenario: "success with multiple patterns",
name: "foo",
expectPanic: false,
expectLen: 2,
},
{
scenario: "non-existing flag",
name: "bar",
expectPanic: true,
},
} {
t.Run(tc.scenario, func(t *testing.T) {
if tc.expectPanic {
defer func() {
if r := recover(); r == nil {
t.Fatal("expected panic but got none")
}
}()
}
if !tc.expectPanic {
defer func() {
if r := recover(); r != nil {
t.Fatalf("expected no panic but got: %v", r)
}
}()
}
result := parsedFlags.MustRegexpSlice(tc.name)
if !tc.expectPanic && len(result) != tc.expectLen {
t.Errorf("expected %d regexps but got %d", tc.expectLen, len(result))
}
})
}
// Test empty strings are skipped.
fs2 := flag.NewFlagSet("tests2", flag.ContinueOnError)
fs2.StringSlice("baz", []string{""}, "")
err = fs2.Parse([]string{})
if err != nil {
t.Fatalf("expected no error but got: %v", err)
}
parsedFlags2 := ParsedFlags{FlagSet: fs2}
result := parsedFlags2.MustRegexpSlice("baz")
if len(result) != 0 {
t.Errorf("expected 0 regexps for empty strings but got %d", len(result))
}
}
func TestParsedFlags_MustDeprecatedRegexpSlice(t *testing.T) {
for _, tc := range []struct {
scenario string
rawFlags []string
expectPattern string
}{
{
scenario: "deprecated flag value",
rawFlags: []string{"--foo=^file:.*"},
expectPattern: "^file:.*",
},
{
scenario: "non-deprecated flag value",
rawFlags: []string{"--bar=^ftp://.*"},
expectPattern: "^ftp://.*",
},
{
scenario: "deprecated flag value > non-deprecated flag value",
rawFlags: []string{"--foo=^file:.*", "--bar=^ftp://.*"},
expectPattern: "^file:.*",
},
} {
t.Run(tc.scenario, func(t *testing.T) {
fs := flag.NewFlagSet("tests", flag.ContinueOnError)
fs.StringSlice("foo", []string{}, "")
fs.StringSlice("bar", []string{}, "")
parsedFlags := ParsedFlags{FlagSet: fs}
err := parsedFlags.Parse(tc.rawFlags)
if err != nil {
t.Fatalf("expected no error but got: %v", err)
}
actual := parsedFlags.MustDeprecatedRegexpSlice("foo", "bar")
if len(actual) != 1 {
t.Fatalf("expected 1 regexp but got %d", len(actual))
}
if actual[0].String() != tc.expectPattern {
t.Errorf("expected pattern '%s' but got '%s'", tc.expectPattern, actual[0].String())
}
})
}
_ = regexp2.None // Keep import alive.
}