fix(outbound): return 403 instead of 500 for an unresolvable outbound host

This commit is contained in:
Julien Neuhart
2026-08-12 20:36:14 +02:00
parent 91f2587fef
commit 357c3b4a59
2 changed files with 44 additions and 1 deletions

View File

@@ -338,8 +338,18 @@ func DecideOutbound(ctx context.Context, rawURL string, allowList, denyList []*r
return OutboundDecision{}, fmt.Errorf("'%s' targets a non-public address: %w", normalized, ErrFiltered)
case errors.Is(err, ErrPublicIP):
return OutboundDecision{}, fmt.Errorf("'%s' targets a public address: %w", normalized, ErrFiltered)
default:
case errors.Is(err, context.Canceled), errors.Is(err, context.DeadlineExceeded):
// A cancellation or timeout is not a policy decision; surface it
// as-is so callers do not report it as a filtered request.
return OutboundDecision{}, fmt.Errorf("validate '%s' host: %w", normalized, err)
default:
// The host could not be resolved, so its address class cannot be
// verified. Fail closed and treat it as filtered, the same as a
// host that resolves to a blocked address, so clients get a
// generic 403 rather than a 500. This also denies alternate IP
// encodings such as http://2130706433/ that the resolver rejects
// as a hostname but Chromium would read as a private IP.
return OutboundDecision{}, fmt.Errorf("validate '%s' host: %v: %w", normalized, err, ErrFiltered)
}
}

View File

@@ -311,6 +311,39 @@ func TestFilterOutboundURL(t *testing.T) {
}
}
func TestDecideOutbound_UnresolvableHostFailsClosed(t *testing.T) {
withStubResolver(t, func(string) ([]netip.Addr, error) {
return nil, errors.New("no such host")
})
// An alternate IP encoding (decimal for 127.0.0.1) that the resolver
// rejects as a hostname must fail closed as filtered, not surface as a
// server error, so clients receive a generic 403.
_, err := DecideOutbound(context.Background(), "http://2130706433/", nil, nil, time.Now().Add(5*time.Second), WithDenyPrivateIPs(true))
if !errors.Is(err, ErrFiltered) {
t.Fatalf("expected ErrFiltered, got: %v", err)
}
}
func TestDecideOutbound_ResolverCancellationNotFiltered(t *testing.T) {
withStubResolver(t, func(string) ([]netip.Addr, error) {
return nil, context.Canceled
})
// A cancellation or timeout is not a policy decision and must not be
// reported as a filtered request.
_, err := DecideOutbound(context.Background(), "http://example.com/", nil, nil, time.Now().Add(5*time.Second), WithDenyPrivateIPs(true))
if err == nil {
t.Fatal("expected error, got nil")
}
if errors.Is(err, ErrFiltered) {
t.Fatalf("cancellation must not be filtered, got: %v", err)
}
if !errors.Is(err, context.Canceled) {
t.Fatalf("expected context.Canceled, got: %v", err)
}
}
func TestResolveAndCheckPublic_IPLiteralLoopback(t *testing.T) {
withStubResolver(t, func(host string) ([]netip.Addr, error) {
t.Fatalf("unexpected DNS lookup for %q", host)