feat(outbound): warn at startup about allow-list patterns that grant more than intended

This commit is contained in:
Julien Neuhart
2026-09-04 19:52:41 +02:00
parent 86a013b664
commit 201e80b9d7
5 changed files with 1002 additions and 1 deletions

602
pkg/gotenberg/allowlist.go Normal file
View File

@@ -0,0 +1,602 @@
package gotenberg
import (
"strings"
)
// AllowListRisk classifies why an allow-list pattern is dangerous. A URL that
// matches an allow-list skips the private and public IP checks, so a pattern
// that matches more than its author intended silently widens outbound access.
// See [AuditAllowList].
type AllowListRisk string
const (
// AllowListRiskUnanchored marks a pattern with no leading "^". regexp2
// searches rather than matches, so the pattern hits anywhere in the URL,
// including the query string.
AllowListRiskUnanchored AllowListRisk = "unanchored"
// AllowListRiskUnanchoredBranch marks an alternation whose later branches
// have no leading "^". Anchoring only the first branch is a common slip.
AllowListRiskUnanchoredBranch AllowListRisk = "unanchored-branch"
// AllowListRiskCatchAll marks a pattern with no literal prefix, such as
// ".+", which matches every URL and disables filtering entirely.
AllowListRiskCatchAll AllowListRisk = "catch-all"
// AllowListRiskOpenHost marks a pattern whose host is not terminated, so
// it also matches attacker-chosen suffix hosts. For example
// "^https://trusted\.example\.com" matches
// "https://trusted.example.com.attacker.example/".
AllowListRiskOpenHost AllowListRisk = "open-host"
)
// AllowListFinding reports one risky entry of an allow-list.
type AllowListFinding struct {
// Index is the zero-based position of the pattern within the flag value.
Index int
// Pattern is the operator's pattern, verbatim.
Pattern string
// Risk is why the pattern is dangerous.
Risk AllowListRisk
}
// maxAuditedPatternLength bounds the patterns [AuditAllowList] inspects. A
// pathological pattern is not worth walking, and reporting nothing is better
// than reporting a partial verdict.
const maxAuditedPatternLength = 4096
// AuditAllowList reports the entries of an allow-list that match more URLs
// than their author is likely to intend. It is a lint over the pattern source,
// not a parser: it recognizes the shapes that are dangerous in practice and
// stays silent when it cannot be sure.
//
// Callers use the findings to warn operators. Never use them to reject a
// configuration: existing deployments rely on loose patterns, and a pattern
// this function does not flag is not thereby safe.
func AuditAllowList(patterns []string) []AllowListFinding {
var findings []AllowListFinding
for i, pattern := range patterns {
if pattern == "" || len(pattern) > maxAuditedPatternLength {
continue
}
risk, ok := auditPattern(pattern)
if ok {
findings = append(findings, AllowListFinding{Index: i, Pattern: pattern, Risk: risk})
}
}
return findings
}
// auditPattern classifies a single pattern, reporting the first risk found.
func auditPattern(pattern string) (AllowListRisk, bool) {
body := trimInlineFlags(pattern)
branches := splitTopLevelAlternation(body)
for i, branch := range branches {
branch = strings.TrimSpace(branch)
anchored := hasStartAnchor(branch)
rest := strings.TrimPrefix(strings.TrimPrefix(branch, `\A`), "^")
// Catch-all first: a pattern that constrains nothing matches every URL
// whether or not it is anchored, and saying so is more useful than
// telling the operator to anchor it.
if literalPrefix(rest) == "" {
return AllowListRiskCatchAll, true
}
if !anchored {
if i == 0 {
return AllowListRiskUnanchored, true
}
return AllowListRiskUnanchoredBranch, true
}
// A lookaround invalidates the token walk, so skip the host check for
// this branch rather than guess. The anchor and catch-all checks above
// still applied.
if containsLookaround(rest) {
continue
}
if hostIsOpen(rest) {
return AllowListRiskOpenHost, true
}
}
return "", false
}
// trimInlineFlags removes a leading inline flag group such as "(?i)" so that
// the anchor check sees the pattern proper.
func trimInlineFlags(pattern string) string {
if !strings.HasPrefix(pattern, "(?") {
return pattern
}
end := strings.Index(pattern, ")")
if end == -1 {
return pattern
}
// Only a flag group qualifies. "(?:", "(?=", "(?!" and "(?<" open a real
// group and must stay.
flags := pattern[2:end]
if flags == "" || strings.ContainsAny(flags, ":=!<") {
return pattern
}
for _, r := range flags {
if !strings.ContainsRune("imsUx-", r) {
return pattern
}
}
return pattern[end+1:]
}
// hasStartAnchor reports whether branch begins with a start-of-input anchor.
func hasStartAnchor(branch string) bool {
return strings.HasPrefix(branch, "^") || strings.HasPrefix(branch, `\A`)
}
// containsLookaround reports whether the pattern uses a lookaround, which the
// token walk in [hostIsOpen] cannot reason about.
func containsLookaround(s string) bool {
return strings.Contains(s, "(?=") || strings.Contains(s, "(?!") || strings.Contains(s, "(?<")
}
// splitTopLevelAlternation splits on "|" at paren depth zero, honoring escapes
// and character classes.
func splitTopLevelAlternation(s string) []string {
var (
parts []string
current strings.Builder
depth int
inClass bool
)
for i := 0; i < len(s); i++ {
c := s[i]
switch {
case c == '\\' && i+1 < len(s):
current.WriteByte(c)
current.WriteByte(s[i+1])
i++
continue
case c == '[' && !inClass:
inClass = true
case c == ']' && inClass:
inClass = false
case c == '(' && !inClass:
depth++
case c == ')' && !inClass:
depth--
case c == '|' && !inClass && depth == 0:
parts = append(parts, current.String())
current.Reset()
continue
}
current.WriteByte(c)
}
parts = append(parts, current.String())
return parts
}
// literalPrefix returns the characters a matching URL must start with. It
// stops at the first optional or non-literal token, and descends one level
// into a leading mandatory group so that "^(https|http)://" is not mistaken
// for a catch-all. An empty result means the pattern constrains nothing.
func literalPrefix(s string) string {
var prefix strings.Builder
for i := 0; i < len(s); {
// A group: descend once when it is mandatory, otherwise stop.
if s[i] == '(' {
end := matchingParen(s, i)
if end == -1 {
break
}
if isQuantified(s, end+1) {
break
}
inner := trimInlineFlags(s[i+1 : end])
branches := splitTopLevelAlternation(inner)
common := literalPrefix(branches[0])
for _, b := range branches[1:] {
common = commonPrefix(common, literalPrefix(b))
}
prefix.WriteString(common)
// Only the leading group is worth descending into.
break
}
var token string
switch {
case s[i] == '\\' && i+1 < len(s):
token = s[i : i+2]
case s[i] == '[':
end := matchingBracket(s, i)
if end == -1 {
return prefix.String()
}
token = s[i : end+1]
default:
token = s[i : i+1]
}
next := i + len(token)
if isQuantified(s, next) {
break
}
// Only a plain literal or an escaped literal contributes.
switch {
case len(token) == 2 && token[0] == '\\' && !isEscapeClass(token[1]):
prefix.WriteByte(token[1])
case len(token) == 1 && !strings.ContainsAny(token, `.[]()^$*+?{}|`):
prefix.WriteByte(token[0])
default:
return prefix.String()
}
i = next
}
return prefix.String()
}
// hostIsOpen reports whether the authority part of the pattern can be left
// without crossing a terminator, which means the pattern also matches
// attacker-chosen suffix hosts or userinfo.
//
// It walks the tokens after "://" and classifies each one. A terminator ends
// the authority, so the pattern is safe. A crosser can match "@", "?" or "#"
// and therefore lets a matching URL escape the authority, so the pattern is
// open. Reaching the end without a terminator is open too, which is the
// classic "^https://trusted\.example\.com" case.
func hostIsOpen(s string) bool {
_, after, ok := strings.Cut(s, "://")
if !ok {
// No authority to reason about, for example "^file:///tmp/".
return false
}
rest := after
for i := 0; i < len(rest); {
var token string
switch {
case rest[i] == '\\' && i+1 < len(rest):
token = rest[i : i+2]
case rest[i] == '[':
end := matchingBracket(rest, i)
if end == -1 {
return true
}
token = rest[i : end+1]
case rest[i] == '(':
end := matchingParen(rest, i)
if end == -1 {
return true
}
token = rest[i : end+1]
default:
token = rest[i : i+1]
}
next := i + len(token)
optional := isOptionalQuantifier(rest, next)
switch classifyHostToken(token) {
case hostTokenTerminator:
// An optional terminator does not end anything, since the URL may
// match without it.
if !optional {
return false
}
case hostTokenCrosser:
return true
case hostTokenNeutral:
// Part of the host itself, so keep walking.
}
i = next
for i < len(rest) && isQuantifierByte(rest[i]) {
if rest[i] == '{' {
end := strings.IndexByte(rest[i:], '}')
if end == -1 {
return true
}
i += end + 1
continue
}
i++
}
}
return true
}
// hostTokenKind is how a token affects the walk in [hostIsOpen].
type hostTokenKind int
const (
hostTokenNeutral hostTokenKind = iota
hostTokenTerminator
hostTokenCrosser
)
// hostTerminators are the characters that end the authority of a URL.
const hostTerminators = "/:#?"
// crosserClassChars are the characters that, if a class can match them, let a
// match escape the authority.
const crosserClassChars = "/@?#"
// classifyHostToken classifies one token of the authority walk.
func classifyHostToken(token string) hostTokenKind {
switch {
case token == ".":
// The wildcard matches "@", "#" and "?", so a host built on it can be
// left without ever reaching a terminator.
return hostTokenCrosser
case token == "$":
return hostTokenTerminator
case len(token) == 1 && strings.Contains(hostTerminators, token):
return hostTokenTerminator
case len(token) == 2 && token[0] == '\\':
switch token[1] {
case 'S', 'D', 'W':
return hostTokenCrosser
case 'd', 'w', 's':
return hostTokenNeutral
case 'p', 'P':
return hostTokenCrosser
}
if strings.Contains(hostTerminators, token[1:]) {
return hostTokenTerminator
}
return hostTokenNeutral
case strings.HasPrefix(token, "["):
inner := strings.TrimSuffix(strings.TrimPrefix(token, "["), "]")
if strings.HasPrefix(inner, "^") {
// A negated class almost always admits "@".
return hostTokenCrosser
}
if classContainsAny(inner, crosserClassChars) {
return hostTokenCrosser
}
return hostTokenNeutral
case strings.HasPrefix(token, "("):
return classifyGroup(token)
}
return hostTokenNeutral
}
// classifyGroup classifies a parenthesized group. A group whose every branch
// starts with a terminator ends the authority, which is what makes the
// idiomatic "(:|/|$)" safe. A group containing a crosser is a crosser.
func classifyGroup(token string) hostTokenKind {
inner := trimInlineFlags(strings.TrimSuffix(strings.TrimPrefix(token, "("), ")"))
inner = strings.TrimPrefix(inner, "?:")
branches := splitTopLevelAlternation(inner)
allTerminate := true
for _, branch := range branches {
if branch == "" {
allTerminate = false
continue
}
kind := classifyHostToken(firstToken(branch))
if kind == hostTokenCrosser {
return hostTokenCrosser
}
if kind != hostTokenTerminator {
allTerminate = false
}
// A crosser anywhere inside the branch still escapes the authority.
if branchHasCrosser(branch) {
return hostTokenCrosser
}
}
if allTerminate {
return hostTokenTerminator
}
return hostTokenNeutral
}
// branchHasCrosser reports whether any token of branch is a crosser.
func branchHasCrosser(branch string) bool {
for i := 0; i < len(branch); {
token := tokenAt(branch, i)
if token == "" {
return true
}
if classifyHostToken(token) == hostTokenCrosser {
return true
}
i += len(token)
}
return false
}
// firstToken returns the first regex token of s.
func firstToken(s string) string {
return tokenAt(s, 0)
}
// tokenAt returns the regex token starting at index i, or "" if it is
// malformed.
func tokenAt(s string, i int) string {
if i >= len(s) {
return ""
}
switch {
case s[i] == '\\' && i+1 < len(s):
return s[i : i+2]
case s[i] == '[':
end := matchingBracket(s, i)
if end == -1 {
return ""
}
return s[i : end+1]
case s[i] == '(':
end := matchingParen(s, i)
if end == -1 {
return ""
}
return s[i : end+1]
}
return s[i : i+1]
}
// classContainsAny reports whether a character class body can match any of the
// given characters, expanding simple ranges.
func classContainsAny(class, chars string) bool {
for i := 0; i < len(class); i++ {
if class[i] == '\\' && i+1 < len(class) {
// An escape class such as \S inside a class admits everything.
if strings.ContainsRune("SDW", rune(class[i+1])) {
return true
}
if strings.ContainsRune(chars, rune(class[i+1])) {
return true
}
i++
continue
}
if i+2 < len(class) && class[i+1] == '-' {
lo, hi := class[i], class[i+2]
for _, c := range []byte(chars) {
if c >= lo && c <= hi {
return true
}
}
i += 2
continue
}
if strings.ContainsRune(chars, rune(class[i])) {
return true
}
}
return false
}
// matchingParen returns the index of the ")" closing the "(" at start.
func matchingParen(s string, start int) int {
depth := 0
inClass := false
for i := start; i < len(s); i++ {
switch {
case s[i] == '\\' && i+1 < len(s):
i++
case s[i] == '[' && !inClass:
inClass = true
case s[i] == ']' && inClass:
inClass = false
case s[i] == '(' && !inClass:
depth++
case s[i] == ')' && !inClass:
depth--
if depth == 0 {
return i
}
}
}
return -1
}
// matchingBracket returns the index of the "]" closing the "[" at start.
func matchingBracket(s string, start int) int {
for i := start + 1; i < len(s); i++ {
switch {
case s[i] == '\\' && i+1 < len(s):
i++
case s[i] == ']':
// A "]" immediately after "[" or "[^" is a literal.
if i == start+1 || (i == start+2 && s[start+1] == '^') {
continue
}
return i
}
}
return -1
}
// isQuantifierByte reports whether c opens a quantifier.
func isQuantifierByte(c byte) bool {
return c == '?' || c == '*' || c == '+' || c == '{'
}
// isQuantified reports whether a quantifier starts at index i.
func isQuantified(s string, i int) bool {
return i < len(s) && isQuantifierByte(s[i])
}
// isOptionalQuantifier reports whether the quantifier at index i lets the
// preceding token match nothing.
func isOptionalQuantifier(s string, i int) bool {
if i >= len(s) {
return false
}
switch s[i] {
case '?', '*':
return true
case '{':
return strings.HasPrefix(s[i:], "{0")
}
return false
}
// isEscapeClass reports whether c after a backslash denotes a character class
// rather than a literal.
func isEscapeClass(c byte) bool {
return strings.ContainsRune("dDwWsSbBAzZpP", rune(c))
}
// commonPrefix returns the longest common prefix of a and b.
func commonPrefix(a, b string) string {
n := min(len(a), len(b))
for i := range n {
if a[i] != b[i] {
return a[:i]
}
}
return a[:n]
}

View File

@@ -0,0 +1,177 @@
package gotenberg
import (
"testing"
"github.com/dlclark/regexp2"
)
func TestAuditAllowList(t *testing.T) {
for _, tc := range []struct {
scenario string
pattern string
want AllowListRisk
}{
// Safe: the host is terminated before anything can leave it.
{"idiomatic terminator group", `^https?://internal\.svc(:|/|$)`, ""},
{"trailing slash", `^https://trusted\.example\.com/`, ""},
{"optional port then terminator", `^https://example\.com(:[0-9]+)?(/|$)`, ""},
{"positive class cannot leave authority", `^https://[a-z0-9.-]+\.s3\.amazonaws\.com/`, ""},
{"leading mandatory group", `^(https|http)://a\.example\.com/`, ""},
{"optional subdomain group", `^https://(www\.)?example\.com/`, ""},
{"port terminator", `^https://example\.com:8443/`, ""},
{"end anchor", `^https://example\.com$`, ""},
{"alternation both anchored and terminated", `^https://a\.example/|^https://b\.example/`, ""},
{"no authority to check", `^file:///tmp/`, ""},
{"digit class in host", `^https://node\d+\.example\.com/`, ""},
// Unanchored: regexp2 searches, so these match anywhere in the URL.
{"no anchor", `trusted\.example\.com`, AllowListRiskUnanchored},
{"no anchor with scheme", `https://trusted\.example\.com/`, AllowListRiskUnanchored},
// Only the first branch anchored.
{"second branch unanchored", `^http://a\.example/|http://b\.example/`, AllowListRiskUnanchoredBranch},
// Catch-all: matches every URL.
{"dot plus", `.+`, AllowListRiskCatchAll},
{"dot star", `.*`, AllowListRiskCatchAll},
{"anchored dot star", `^.*`, AllowListRiskCatchAll},
{"anchored dot plus", `^.+`, AllowListRiskCatchAll},
// Open host: the reported vulnerability class.
{"advisory pattern", `^http://trusted\.example\.com`, AllowListRiskOpenHost},
{"gotenberg.dev internet-facing recipe", `^https?://[^/]+\.internal\.example\.com`, AllowListRiskOpenHost},
{"gotenberg.dev strict whitelist recipe", `^https://(api|cdn|images)\.internal\.example\.com`, AllowListRiskOpenHost},
{"gotenberg.dev hooks recipe", `^https?://hooks\.internal\.example\.com`, AllowListRiskOpenHost},
{"feature file pattern", `^https?://host.docker.internal.*`, AllowListRiskOpenHost},
{"scheme only", `^https?://`, AllowListRiskOpenHost},
{"wildcard subdomain", `^https://.+\.example\.com/`, AllowListRiskOpenHost},
{"escaped dot is not a terminator", `^https?://example\.com\.`, AllowListRiskOpenHost},
{"negated class in host", `^https://[^.]+\.example\.com/`, AllowListRiskOpenHost},
} {
t.Run(tc.scenario, func(t *testing.T) {
findings := AuditAllowList([]string{tc.pattern})
if tc.want == "" {
if len(findings) != 0 {
t.Fatalf("AuditAllowList(%q) = %+v, want no finding", tc.pattern, findings)
}
return
}
if len(findings) != 1 {
t.Fatalf("AuditAllowList(%q) returned %d findings, want 1", tc.pattern, len(findings))
}
if findings[0].Risk != tc.want {
t.Fatalf("AuditAllowList(%q) risk = %q, want %q", tc.pattern, findings[0].Risk, tc.want)
}
})
}
}
// TestAuditAllowList_FlaggedPatternsAreActuallyExploitable proves the audit is
// not merely syntactic: every pattern it flags as open-host really does admit
// a host the operator did not intend.
func TestAuditAllowList_FlaggedPatternsAreActuallyExploitable(t *testing.T) {
for _, tc := range []struct {
pattern string
attack string
}{
{`^http://trusted\.example\.com`, "http://trusted.example.com.attacker.example/"},
{`^https?://[^/]+\.internal\.example\.com`, "http://a.internal.example.com.attacker.example/"},
{`^https://(api|cdn|images)\.internal\.example\.com`, "https://api.internal.example.com.attacker.example/"},
{`^https?://hooks\.internal\.example\.com`, "http://hooks.internal.example.com.attacker.example/"},
{`^https?://host.docker.internal.*`, "http://host.docker.internal.attacker.example/"},
{`^https://.+\.example\.com/`, "https://attacker.example/#x.example.com/"},
{`^https?://example\.com\.`, "http://example.com.attacker.example/"},
} {
t.Run(tc.pattern, func(t *testing.T) {
findings := AuditAllowList([]string{tc.pattern})
if len(findings) == 0 {
t.Fatalf("pattern %q was not flagged", tc.pattern)
}
ok, err := regexp2.MustCompile(tc.pattern, 0).MatchString(tc.attack)
if err != nil {
t.Fatalf("match %q: %v", tc.attack, err)
}
if !ok {
t.Fatalf("pattern %q does not match %q, so the finding is a false positive", tc.pattern, tc.attack)
}
})
}
}
// TestAuditAllowList_SafePatternsRejectTheAttacks is the converse: the shapes
// the audit stays silent about really do reject the same attacks.
func TestAuditAllowList_SafePatternsRejectTheAttacks(t *testing.T) {
safe := []string{
`^https?://internal\.svc(:|/|$)`,
`^https://trusted\.example\.com/`,
`^https://example\.com(:[0-9]+)?(/|$)`,
`^https://[a-z0-9.-]+\.s3\.amazonaws\.com/`,
}
attacks := []string{
"https://internal.svc.attacker.example/",
"https://trusted.example.com.attacker.example/",
"https://trusted.example.com@169.254.169.254/",
"https://example.com.attacker.example/",
"https://example.com@10.0.0.5/",
"https://bucket.s3.amazonaws.com.attacker.example/",
"https://bucket.s3.amazonaws.com@127.0.0.1/",
}
for _, pattern := range safe {
t.Run(pattern, func(t *testing.T) {
if findings := AuditAllowList([]string{pattern}); len(findings) != 0 {
t.Fatalf("safe pattern %q was flagged as %q", pattern, findings[0].Risk)
}
re := regexp2.MustCompile(pattern, 0)
for _, attack := range attacks {
ok, err := re.MatchString(attack)
if err != nil {
t.Fatalf("match %q: %v", attack, err)
}
if ok {
t.Fatalf("pattern %q matches attack %q but was not flagged", pattern, attack)
}
}
})
}
}
func TestAuditAllowList_SkipsEmptyAndOversized(t *testing.T) {
oversized := make([]byte, maxAuditedPatternLength+1)
for i := range oversized {
oversized[i] = 'a'
}
findings := AuditAllowList([]string{"", string(oversized)})
if len(findings) != 0 {
t.Fatalf("AuditAllowList returned %+v, want no finding", findings)
}
}
func TestAuditAllowList_ReportsIndex(t *testing.T) {
findings := AuditAllowList([]string{
`^https://ok\.example\.com/`,
`^https://open\.example\.com`,
})
if len(findings) != 1 {
t.Fatalf("got %d findings, want 1", len(findings))
}
if findings[0].Index != 1 {
t.Fatalf("findings[0].Index = %d, want 1", findings[0].Index)
}
}
// TestAuditAllowList_ShippedChromiumDenyListIsNotAudited guards the rule that
// deny-lists are never audited. The shipped Chromium deny-list uses a
// lookaround and has no authority, so auditing it would produce noise.
func TestAuditAllowList_LookaroundIsNotFlaggedForHost(t *testing.T) {
findings := AuditAllowList([]string{`^file:(?!//\/tmp/).*`})
if len(findings) != 0 {
t.Fatalf("AuditAllowList returned %+v, want no finding", findings)
}
}

View File

@@ -1,11 +1,17 @@
package gotenberg
import (
"context"
"fmt"
"log/slog"
"strings"
"time"
"github.com/dlclark/regexp2"
"github.com/labstack/gommon/bytes"
flag "github.com/spf13/pflag"
"github.com/gotenberg/gotenberg/v8/pkg/gotenberg/internal/log"
)
// ParsedFlags wraps a [flag.FlagSet] so that retrieving the typed values is
@@ -226,9 +232,14 @@ func (f *ParsedFlags) MustDeprecatedRegexp(deprecated string, newName string) *r
// 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.
//
// Every allow-list and deny-list in Gotenberg is read through this method, so
// it is also where allow-list patterns are audited. See [AuditAllowList].
func (f *ParsedFlags) MustRegexpSlice(name string) []*regexp2.Regexp {
vals := f.MustStringSlice(name)
f.warnRiskyAllowList(name, vals)
var regexps []*regexp2.Regexp
for _, val := range vals {
if val == "" {
@@ -241,6 +252,114 @@ func (f *ParsedFlags) MustRegexpSlice(name string) []*regexp2.Regexp {
return regexps
}
// allowListFlagSuffix identifies the flags whose patterns grant an IP-check
// bypass. Deny-lists are never audited: they always apply, cannot be bypassed,
// and a loose deny-list is safe rather than dangerous.
const allowListFlagSuffix = "-allow-list"
// warnRiskyAllowList logs one warning per allow-list entry that matches more
// URLs than its author is likely to intend.
//
// It warns and never fails: operators depend on loose patterns today, and
// rejecting them at startup would break running deployments.
func (f *ParsedFlags) warnRiskyAllowList(name string, vals []string) {
if !strings.HasSuffix(name, allowListFlagSuffix) {
return
}
findings := AuditAllowList(vals)
if len(findings) == 0 {
return
}
// The logger is nil until the entry point initializes it, which happens
// before any module is provisioned. Tests and embedders that call this
// method directly get no logger, and must not panic for it.
logger := log.Logger()
if logger == nil {
return
}
for _, finding := range findings {
// Provision has no context.Context to propagate, so the trace-aware
// logging convention is satisfied with a background context.
logger.WarnContext(
context.Background(),
f.allowListWarning(name, finding),
slog.String("flag", "--"+name),
slog.String("env", EnvVarName(name)),
slog.Int("entry", finding.Index+1),
slog.String("reason", string(finding.Risk)),
)
}
}
// allowListWarning builds the operator-facing message for a finding. It names
// the flag and its environment variable, and, when they exist, the IP-check
// flags the entry silently disables.
func (f *ParsedFlags) allowListWarning(name string, finding AllowListFinding) string {
var b strings.Builder
// Print the pattern raw rather than quoted: %q escapes every backslash, so
// the operator would not recognize the value they set.
fmt.Fprintf(&b, "--%s (%s) entry %d '%s' ", name, EnvVarName(name), finding.Index+1, finding.Pattern)
switch finding.Risk {
case AllowListRiskUnanchored:
b.WriteString("is not anchored with ^, so it matches anywhere in the URL and a URL such as http://attacker.example/?u=trusted.example.com passes. ")
case AllowListRiskUnanchoredBranch:
b.WriteString("has an alternation branch that is not anchored with ^, and that branch matches anywhere in the URL. ")
case AllowListRiskCatchAll:
b.WriteString("matches every URL. ")
case AllowListRiskOpenHost:
b.WriteString("does not terminate the host, so it also matches suffix hosts such as http://trusted.example.com.attacker.example/. ")
}
b.WriteString(f.bypassSentence(name))
switch finding.Risk {
case AllowListRiskUnanchored, AllowListRiskUnanchoredBranch:
b.WriteString("Anchor every branch with ^ and end the host with /, :, or $.")
case AllowListRiskCatchAll:
b.WriteString("Restrict the entry to the hosts you trust, or unset the flag.")
case AllowListRiskOpenHost:
b.WriteString("End the host with /, :, $, or a group such as (:|/|$).")
}
return b.String()
}
// bypassSentence names the IP-check flags an allow-list match skips, when the
// module registers them.
func (f *ParsedFlags) bypassSentence(name string) string {
prefix := strings.TrimSuffix(name, allowListFlagSuffix)
private, public := prefix+"-deny-private-ips", prefix+"-deny-public-ips"
if f.Lookup(private) == nil || f.Lookup(public) == nil {
// A deprecated alias such as webhook-error-allow-list carries an extra
// segment that the IP-check flags do not have.
if i := strings.LastIndex(prefix, "-"); i != -1 {
private, public = prefix[:i]+"-deny-private-ips", prefix[:i]+"-deny-public-ips"
}
}
if f.Lookup(private) == nil || f.Lookup(public) == nil {
return "A URL that matches the allow-list skips the private and public IP checks. "
}
return fmt.Sprintf(
"A URL that matches the allow-list skips --%s (%s) and --%s (%s). ",
private, EnvVarName(private), public, EnvVarName(public),
)
}
// EnvVarName returns the environment variable that overrides the flag given by
// name. The entry point derives the same name when it applies environment
// overrides, so operator-facing messages can name both without drifting.
func EnvVarName(name string) string {
return strings.ToUpper(strings.ReplaceAll(name, "-", "_"))
}
// 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.

View File

@@ -3,6 +3,7 @@ package gotenberg
import (
"reflect"
"regexp"
"strings"
"testing"
"time"
@@ -951,3 +952,105 @@ func TestParsedFlags_MustDeprecatedRegexpSlice(t *testing.T) {
_ = regexp2.None // Keep import alive.
}
func TestParsedFlags_AllowListWarning(t *testing.T) {
fs := flag.NewFlagSet("tests", flag.ContinueOnError)
fs.StringSlice("chromium-allow-list", []string{}, "")
fs.Bool("chromium-deny-private-ips", false, "")
fs.Bool("chromium-deny-public-ips", false, "")
fs.StringSlice("standalone-allow-list", []string{}, "")
parsedFlags := ParsedFlags{FlagSet: fs}
for _, tc := range []struct {
scenario string
name string
finding AllowListFinding
contains []string
}{
{
scenario: "open host names both IP-check flags and their env vars",
name: "chromium-allow-list",
finding: AllowListFinding{Index: 0, Pattern: `^https://trusted\.example\.com`, Risk: AllowListRiskOpenHost},
contains: []string{
"--chromium-allow-list (CHROMIUM_ALLOW_LIST)",
"entry 1",
`^https://trusted\.example\.com`,
"does not terminate the host",
"--chromium-deny-private-ips (CHROMIUM_DENY_PRIVATE_IPS)",
"--chromium-deny-public-ips (CHROMIUM_DENY_PUBLIC_IPS)",
"End the host with",
},
},
{
scenario: "catch-all tells the operator to restrict or unset",
name: "chromium-allow-list",
finding: AllowListFinding{Index: 2, Pattern: ".+", Risk: AllowListRiskCatchAll},
contains: []string{"entry 3", "matches every URL", "Restrict the entry"},
},
{
scenario: "unanchored explains the search semantics",
name: "chromium-allow-list",
finding: AllowListFinding{Index: 0, Pattern: `trusted\.example\.com`, Risk: AllowListRiskUnanchored},
contains: []string{"is not anchored with ^", "Anchor every branch with ^"},
},
{
scenario: "module without IP-check flags falls back to a generic sentence",
name: "standalone-allow-list",
finding: AllowListFinding{Index: 0, Pattern: `^https://a\.example\.com`, Risk: AllowListRiskOpenHost},
contains: []string{"skips the private and public IP checks"},
},
} {
t.Run(tc.scenario, func(t *testing.T) {
msg := parsedFlags.allowListWarning(tc.name, tc.finding)
for _, want := range tc.contains {
if !strings.Contains(msg, want) {
t.Fatalf("message %q does not contain %q", msg, want)
}
}
if strings.Contains(msg, "—") {
t.Fatalf("message must not contain an em dash: %q", msg)
}
})
}
}
func TestParsedFlags_WarnRiskyAllowList_SkipsDenyLists(t *testing.T) {
fs := flag.NewFlagSet("tests", flag.ContinueOnError)
fs.StringSlice("chromium-deny-list", []string{}, "")
parsedFlags := ParsedFlags{FlagSet: fs}
// A deny-list is never audited: it always applies and cannot be bypassed,
// so a loose one is safe. This must also not panic on a nil logger.
parsedFlags.warnRiskyAllowList("chromium-deny-list", []string{".+", `^file:(?!//\/tmp/).*`})
}
func TestParsedFlags_WarnRiskyAllowList_NilLoggerDoesNotPanic(t *testing.T) {
fs := flag.NewFlagSet("tests", flag.ContinueOnError)
fs.StringSlice("chromium-allow-list", []string{}, "")
parsedFlags := ParsedFlags{FlagSet: fs}
// Provision runs after the entry point initializes the logger, but tests
// and embedders reach this path with no logger at all.
parsedFlags.warnRiskyAllowList("chromium-allow-list", []string{".+"})
}
func TestEnvVarName(t *testing.T) {
for _, tc := range []struct {
name string
want string
}{
{"chromium-allow-list", "CHROMIUM_ALLOW_LIST"},
{"api-download-from-deny-private-ips", "API_DOWNLOAD_FROM_DENY_PRIVATE_IPS"},
{"log-level", "LOG_LEVEL"},
} {
t.Run(tc.name, func(t *testing.T) {
if got := EnvVarName(tc.name); got != tc.want {
t.Fatalf("EnvVarName(%q) = %q, want %q", tc.name, got, tc.want)
}
})
}
}