From 3026aa4e0f07d55b521d70cb55f2f3020bca2c01 Mon Sep 17 00:00:00 2001 From: Mitchell Scott Date: Wed, 26 Aug 2026 09:27:54 -0600 Subject: [PATCH 1/2] fix: validate outbound fetch addresses at dial time and on each redirect --- docs/CONFIGURATION.md | 8 ++- internal/converter/reader.go | 14 +++-- internal/downloader/client.go | 6 +- internal/downloader/handler.go | 5 ++ internal/security/httpclient.go | 76 ++++++++++++++++++++++++ internal/security/httpclient_test.go | 89 ++++++++++++++++++++++++++++ internal/security/urlvalidation.go | 84 +++++++++++++++++++------- internal/webhook/handler.go | 21 +++++-- locales/da.json | 2 + locales/de.json | 2 + locales/en.json | 2 + locales/es.json | 2 + locales/fi.json | 2 + locales/fr.json | 2 + locales/it.json | 2 + locales/ja.json | 2 + locales/ko.json | 2 + locales/nl.json | 2 + locales/no.json | 2 + locales/pl.json | 2 + locales/pt.json | 2 + locales/sv.json | 2 + locales/zh-CN.json | 2 + 23 files changed, 294 insertions(+), 39 deletions(-) create mode 100644 internal/security/httpclient.go create mode 100644 internal/security/httpclient_test.go diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 19db8bfd..a2126db7 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -56,15 +56,17 @@ For more rmapi-specific configuration, see [their documentation](https://github. | Variable | Required? | Default | Description | |--------------------------|-----------|---------|-------------| -| BLOCK_PRIVATE_IPS | No | false | Set to `true` to block URLs pointing to private/local IP addresses (RFC1918, loopback, link-local) | +| BLOCK_PRIVATE_IPS | No | false | Set to `true` to block URLs pointing to private/local IP addresses (RFC1918, loopback). Link-local is always blocked. | | BLOCKED_DOMAINS | No | | Comma-separated list of domains to block (e.g., `internal.corp,local.net`) | ### Security Configuration Notes -- **BLOCK_PRIVATE_IPS**: When enabled, prevents Server-Side Request Forgery (SSRF) attacks by blocking URLs that resolve to: +- **Link-local addresses are always blocked**, whatever `BLOCK_PRIVATE_IPS` is set to: 169.254.0.0/16 and fe80::/10. This range carries the cloud instance metadata endpoints (169.254.169.254 and 169.254.170.2), and no deployment serves documents from it. Every address is checked at connection time, including each hop of a redirect, so a redirect or a DNS record that changes between check and fetch cannot reach a blocked address. +- **BLOCK_PRIVATE_IPS**: Left at the default, Aviary can fetch from your own network, which is what makes `http://192.168.1.50/doc.pdf` work. Set it to `true` to also block URLs that resolve to: - Private IPv4 ranges: 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16 - Loopback addresses: 127.0.0.0/8, ::1 - - Link-local addresses: 169.254.0.0/16, fe80::/10 + - Carrier-grade NAT: 100.64.0.0/10 + - Unique local IPv6: fc00::/7 - Other special-use addresses - **BLOCKED_DOMAINS**: Blocks specific domains and their subdomains. For example, setting `BLOCKED_DOMAINS=example.com` will block both `example.com` and `*.example.com` diff --git a/internal/converter/reader.go b/internal/converter/reader.go index 6c6f2514..00563e54 100644 --- a/internal/converter/reader.go +++ b/internal/converter/reader.go @@ -66,9 +66,8 @@ func ExtractFromURL(urlStr string) (*ArticleContent, error) { } req.Header.Set("User-Agent", downloader.PickUA()) - client := &http.Client{ - Timeout: 30 * time.Second, - } + client := security.NewHTTPClient() + client.Timeout = 30 * time.Second // codeql[go/request-forgery]: URL is validated by security.ValidateURL above resp, err := client.Do(req) if err != nil { @@ -201,15 +200,18 @@ func extractImageURLs(html string) []string { func DownloadImage(imageURL, outputPath string) error { logging.Logf("[READER] DownloadImage: fetching %s", imageURL) + if err := security.ValidateURL(imageURL); err != nil { + return fmt.Errorf("URL validation failed: %w", err) + } + req, err := http.NewRequest("GET", imageURL, nil) if err != nil { return fmt.Errorf("failed to create request: %w", err) } req.Header.Set("User-Agent", downloader.PickUA()) - client := &http.Client{ - Timeout: 30 * time.Second, - } + client := security.NewHTTPClient() + client.Timeout = 30 * time.Second resp, err := client.Do(req) if err != nil { return fmt.Errorf("failed to download image: %w", err) diff --git a/internal/downloader/client.go b/internal/downloader/client.go index a85afdfa..d0992336 100644 --- a/internal/downloader/client.go +++ b/internal/downloader/client.go @@ -1,10 +1,10 @@ package downloader import ( - "net/http" "time" "github.com/rmitchellscott/aviary/internal/config" + "github.com/rmitchellscott/aviary/internal/security" ) // Clients used for HTTP requests. Timeouts are configured via environment @@ -15,8 +15,8 @@ import ( var ( sniffTimeout = 30 * time.Second downloadTimeout = 60 * time.Second - sniffClient = &http.Client{} - downloadClient = &http.Client{} + sniffClient = security.NewHTTPClient() + downloadClient = security.NewHTTPClient() ) func init() { diff --git a/internal/downloader/handler.go b/internal/downloader/handler.go index ecaf5bdd..9833677f 100644 --- a/internal/downloader/handler.go +++ b/internal/downloader/handler.go @@ -4,6 +4,7 @@ import ( "net/http" "github.com/gin-gonic/gin" + "github.com/rmitchellscott/aviary/internal/security" ) // SniffHandler responds with the MIME type of the ?url parameter. @@ -16,6 +17,10 @@ func SniffHandler(c *gin.Context) { mt, err := SniffMime(urlStr) if err != nil { + if security.IsBlockedAddress(err) { + c.JSON(http.StatusForbidden, gin.H{"error": "backend.status.blocked_address"}) + return + } c.JSON(http.StatusInternalServerError, gin.H{"error": "backend.status.internal_error"}) return } diff --git a/internal/security/httpclient.go b/internal/security/httpclient.go new file mode 100644 index 00000000..9ce58439 --- /dev/null +++ b/internal/security/httpclient.go @@ -0,0 +1,76 @@ +package security + +import ( + "context" + "errors" + "fmt" + "net" + "net/http" + "time" +) + +const maxRedirects = 10 + +var ErrTooManyRedirects = errors.New("too many redirects") + +// NewHTTPClient returns a client that validates every address it connects to. +// Callers set Timeout themselves. Validation at dial time covers each redirect +// hop and the address the connection actually reaches, which a check against +// the requested URL alone does not. +func NewHTTPClient() *http.Client { + return &http.Client{ + Transport: newGuardedTransport(), + CheckRedirect: checkRedirect, + } +} + +func checkRedirect(req *http.Request, via []*http.Request) error { + if len(via) >= maxRedirects { + return ErrTooManyRedirects + } + return ValidateURL(req.URL.String()) +} + +func newGuardedTransport() *http.Transport { + dialer := &net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + } + + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) { + return dialGuarded(ctx, dialer, network, addr) + } + + return transport +} + +// dialGuarded resolves the host itself and connects to a validated address, so +// the connection cannot land somewhere a second resolution would have returned. +func dialGuarded(ctx context.Context, dialer *net.Dialer, network, addr string) (net.Conn, error) { + host, port, err := net.SplitHostPort(addr) + if err != nil { + return nil, err + } + + addrs, err := net.DefaultResolver.LookupIPAddr(ctx, host) + if err != nil { + return nil, fmt.Errorf("%w: %v", ErrIPResolutionFailed, err) + } + + lastErr := error(fmt.Errorf("%w: no IPs found for hostname", ErrIPResolutionFailed)) + for _, resolved := range addrs { + if err := checkIPAllowed(resolved.IP); err != nil { + lastErr = err + continue + } + + conn, err := dialer.DialContext(ctx, network, net.JoinHostPort(resolved.IP.String(), port)) + if err == nil { + return conn, nil + } + lastErr = err + } + + return nil, lastErr +} diff --git a/internal/security/httpclient_test.go b/internal/security/httpclient_test.go new file mode 100644 index 00000000..a1489661 --- /dev/null +++ b/internal/security/httpclient_test.go @@ -0,0 +1,89 @@ +package security + +import ( + "errors" + "net/http" + "net/http/httptest" + "os" + "testing" + "time" +) + +func newTestClient() *http.Client { + client := NewHTTPClient() + client.Timeout = 5 * time.Second + return client +} + +func TestRedirectToLinkLocalRefused(t *testing.T) { + os.Unsetenv("BLOCK_PRIVATE_IPS") + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, "http://169.254.169.254/latest/meta-data/", http.StatusFound) + })) + defer server.Close() + + _, err := newTestClient().Get(server.URL) + if err == nil { + t.Fatal("expected redirect to link-local address to be refused") + } + if !errors.Is(err, ErrLinkLocal) { + t.Errorf("expected ErrLinkLocal, got %v", err) + } +} + +func TestDialToLinkLocalRefused(t *testing.T) { + os.Unsetenv("BLOCK_PRIVATE_IPS") + + _, err := newTestClient().Get("http://169.254.169.254/latest/meta-data/") + if err == nil { + t.Fatal("expected link-local address to be refused at dial time") + } + if !errors.Is(err, ErrLinkLocal) { + t.Errorf("expected ErrLinkLocal, got %v", err) + } +} + +func TestRedirectLimit(t *testing.T) { + os.Unsetenv("BLOCK_PRIVATE_IPS") + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, "/again", http.StatusFound) + })) + defer server.Close() + + _, err := newTestClient().Get(server.URL) + if !errors.Is(err, ErrTooManyRedirects) { + t.Errorf("expected ErrTooManyRedirects, got %v", err) + } +} + +func TestLinkLocalBlockedRegardlessOfFlag(t *testing.T) { + os.Unsetenv("BLOCK_PRIVATE_IPS") + defer os.Unsetenv("BLOCK_PRIVATE_IPS") + + for _, rawURL := range []string{ + "http://169.254.169.254/latest/meta-data/", + "http://169.254.170.2/v2/credentials", + "http://[fe80::1]/", + } { + if err := ValidateURL(rawURL); !errors.Is(err, ErrLinkLocal) { + t.Errorf("ValidateURL(%s) = %v, want ErrLinkLocal", rawURL, err) + } + } +} + +func TestPrivateAddressesAllowedByDefault(t *testing.T) { + os.Unsetenv("BLOCK_PRIVATE_IPS") + + for _, rawURL := range []string{ + "http://192.168.1.50/doc.pdf", + "http://10.0.0.5/doc.pdf", + "http://127.0.0.1:8080/doc.pdf", + "http://100.64.0.1/doc.pdf", + } { + if err := ValidateURL(rawURL); err != nil { + t.Errorf("ValidateURL(%s) = %v, want nil", rawURL, err) + } + } +} diff --git a/internal/security/urlvalidation.go b/internal/security/urlvalidation.go index 2383ac20..1b293a38 100644 --- a/internal/security/urlvalidation.go +++ b/internal/security/urlvalidation.go @@ -14,6 +14,7 @@ var ( ErrInvalidURL = errors.New("invalid URL format") ErrInvalidScheme = errors.New("URL scheme must be http or https") ErrPrivateIP = errors.New("URL points to private/local IP address") + ErrLinkLocal = errors.New("URL points to a link-local address") ErrBlockedDomain = errors.New("domain is in blocklist") ErrEmptyURL = errors.New("URL cannot be empty") ErrIPResolutionFailed = errors.New("failed to resolve domain") @@ -49,39 +50,78 @@ func ValidateURL(rawURL string) error { } } - if config.Get("BLOCK_PRIVATE_IPS", "") == "true" { - if err := checkPrivateIP(hostname); err != nil { - return err - } + return checkHostAddresses(hostname) +} + +func blockPrivateIPs() bool { + return config.Get("BLOCK_PRIVATE_IPS", "") == "true" +} + +// checkIPAllowed rejects an address the server must never connect to. Link-local +// space carries the cloud metadata endpoints and is refused unconditionally. +// The wider private ranges are refused only when BLOCK_PRIVATE_IPS is set, so a +// self-hosted deployment can still fetch from its own network. +func checkIPAllowed(ip net.IP) error { + if isLinkLocal(ip) { + return fmt.Errorf("%w: %s", ErrLinkLocal, ip.String()) + } + + if blockPrivateIPs() && isPrivateIP(ip) { + return fmt.Errorf("%w: %s (unset BLOCK_PRIVATE_IPS to allow)", ErrPrivateIP, ip.String()) } return nil } -func checkPrivateIP(hostname string) error { - ip := net.ParseIP(hostname) - if ip == nil { - ips, err := net.LookupIP(hostname) - if err != nil { - return fmt.Errorf("%w: %v", ErrIPResolutionFailed, err) - } - if len(ips) == 0 { - return fmt.Errorf("%w: no IPs found for hostname", ErrIPResolutionFailed) - } - for _, resolvedIP := range ips { - if isPrivateIP(resolvedIP) { - return fmt.Errorf("%w: %s resolves to %s", ErrPrivateIP, hostname, resolvedIP.String()) - } - } - } else { - if isPrivateIP(ip) { - return fmt.Errorf("%w: %s", ErrPrivateIP, ip.String()) +// IsBlockedAddress reports whether err comes from an address or domain the +// server refused to connect to, rather than from a network or server fault. +func IsBlockedAddress(err error) bool { + return errors.Is(err, ErrLinkLocal) || + errors.Is(err, ErrPrivateIP) || + errors.Is(err, ErrBlockedDomain) +} + +// checkHostAddresses validates an IP literal directly. A hostname is resolved +// here only when BLOCK_PRIVATE_IPS is set; otherwise the guarded dialer applies +// checkIPAllowed to whatever address the connection actually reaches, which +// avoids a DNS lookup on every validation and closes the rebinding gap. +func checkHostAddresses(hostname string) error { + if ip := net.ParseIP(hostname); ip != nil { + return checkIPAllowed(ip) + } + + if !blockPrivateIPs() { + return nil + } + + ips, err := net.LookupIP(hostname) + if err != nil { + return fmt.Errorf("%w: %v", ErrIPResolutionFailed, err) + } + if len(ips) == 0 { + return fmt.Errorf("%w: no IPs found for hostname", ErrIPResolutionFailed) + } + for _, resolvedIP := range ips { + if err := checkIPAllowed(resolvedIP); err != nil { + return fmt.Errorf("%s resolves to %w", hostname, err) } } return nil } +func isLinkLocal(ip net.IP) bool { + if ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() { + return true + } + + if ip4 := ip.To4(); ip4 != nil { + return ip4[0] == 169 && ip4[1] == 254 + } + + return false +} + func isPrivateIP(ip net.IP) bool { if ip.IsLoopback() { return true diff --git a/internal/webhook/handler.go b/internal/webhook/handler.go index 9a4471bd..813926be 100644 --- a/internal/webhook/handler.go +++ b/internal/webhook/handler.go @@ -100,6 +100,8 @@ func keyToMessage(key string) string { var ( // urlRegex is used to find an http(s) URL within the Body string. urlRegex = regexp.MustCompile(`https?://[^\s]+`) + // markdownClient fetches raw Markdown, validating every address it reaches. + markdownClient = security.NewHTTPClient() // jobStore holds in-memory jobs for status polling. jobStore = jobs.NewStore() // supportedContentTypes lists MIME types we can process @@ -323,6 +325,15 @@ func processPDF(jobID string, form map[string]string) (string, map[string]string // (if it exists on disk) or else extracts a URL from form["Body"], downloads it, and then proceeds to // (optionally) compress, then upload/manage on the reMarkable. Returns a human-readable status message // and/or an error. +// fetchErrorKey maps a fetch failure to the message the user sees. An address the +// server refused gets its own key so it does not read as a network fault. +func fetchErrorKey(err error, fallbackKey string) string { + if security.IsBlockedAddress(err) { + return "backend.status.blocked_address" + } + return fallbackKey +} + func processPDFForUser(jobID string, form map[string]string, userID uuid.UUID) (string, map[string]string, error) { body := form["Body"] @@ -410,7 +421,7 @@ func processPDFForUser(jobID string, form map[string]string, userID uuid.UUID) ( jobStore.UpdateWithOperation(jobID, "Running", "backend.status.downloading", nil, "downloading") localPath, err = downloader.DownloadPDFForUser(match, true, prefix, userID, nil) if err != nil { - return "backend.status.download_error", nil, err + return fetchErrorKey(err, "backend.status.download_error"), nil, err } } else if contentType == "text/markdown" || contentType == "text/plain" { // Markdown URL - fetch raw content and convert @@ -418,13 +429,13 @@ func processPDFForUser(jobID string, form map[string]string, userID uuid.UUID) ( jobStore.UpdateWithOperation(jobID, "Running", "backend.status.fetching_url", nil, "fetching") if err := security.ValidateURL(match); err != nil { - return "backend.status.invalid_url", nil, fmt.Errorf("URL validation failed: %w", err) + return fetchErrorKey(err, "backend.status.invalid_url"), nil, fmt.Errorf("URL validation failed: %w", err) } // codeql[go/request-forgery]: URL is validated by security.ValidateURL above - resp, err := http.Get(match) + resp, err := markdownClient.Get(match) if err != nil { - return "backend.status.download_error", nil, fmt.Errorf("failed to fetch markdown: %w", err) + return fetchErrorKey(err, "backend.status.download_error"), nil, fmt.Errorf("failed to fetch markdown: %w", err) } defer resp.Body.Close() @@ -494,7 +505,7 @@ func processPDFForUser(jobID string, form map[string]string, userID uuid.UUID) ( // Extract readable content from URL articleContent, extractErr := converter.ExtractFromURL(match) if extractErr != nil { - return "backend.status.download_error", nil, fmt.Errorf("failed to extract article: %w", extractErr) + return fetchErrorKey(extractErr, "backend.status.download_error"), nil, fmt.Errorf("failed to extract article: %w", extractErr) } // Determine output format diff --git a/locales/da.json b/locales/da.json index aa332f07..bc63b4d8 100644 --- a/locales/da.json +++ b/locales/da.json @@ -545,6 +545,8 @@ "invalid_prefix": "Ugyldigt præfiks", "using_uploaded_file": "Bruger uploadet fil", "no_url": "Ingen URL fundet i anmodningskroppen", + "invalid_url": "Ugyldig URL", + "blocked_address": "Denne adresse er ikke tilladt. Kontakt din administrator, hvis du har brug for adgang til den.", "downloading": "Downloader", "download_error": "Download fejl", "fetching_url": "Henter artikel fra URL", diff --git a/locales/de.json b/locales/de.json index b5619b3a..4283e119 100644 --- a/locales/de.json +++ b/locales/de.json @@ -545,6 +545,8 @@ "invalid_prefix": "Ungültiges Präfix", "using_uploaded_file": "Hochgeladene Datei wird verwendet", "no_url": "Keine URL im Anfragetext gefunden", + "invalid_url": "Ungültige URL", + "blocked_address": "Diese Adresse ist nicht zulässig. Wenden Sie sich an Ihren Administrator, wenn Sie Zugriff darauf benötigen.", "downloading": "Wird heruntergeladen", "download_error": "Download-Fehler", "fetching_url": "Artikel wird von URL abgerufen", diff --git a/locales/en.json b/locales/en.json index 115cdc96..85bc7e04 100644 --- a/locales/en.json +++ b/locales/en.json @@ -545,6 +545,8 @@ "invalid_prefix": "Invalid prefix", "using_uploaded_file": "Using uploaded file", "no_url": "No URL found in request body", + "invalid_url": "Invalid URL", + "blocked_address": "This address is not allowed. Contact your administrator if you need access to it.", "downloading": "Downloading", "download_error": "Download error", "fetching_url": "Fetching article from URL", diff --git a/locales/es.json b/locales/es.json index d9a63ecc..c1ea053f 100644 --- a/locales/es.json +++ b/locales/es.json @@ -359,6 +359,8 @@ "invalid_prefix": "Prefijo inválido", "using_uploaded_file": "Usando archivo subido", "no_url": "No se encontró URL en el cuerpo de la solicitud", + "invalid_url": "URL no válida", + "blocked_address": "Esta dirección no está permitida. Ponte en contacto con tu administrador si necesitas acceder a ella.", "downloading": "Descargando", "download_error": "Error de descarga", "fetching_url": "Obteniendo artículo desde URL", diff --git a/locales/fi.json b/locales/fi.json index f0a0f99b..612b35c8 100644 --- a/locales/fi.json +++ b/locales/fi.json @@ -545,6 +545,8 @@ "invalid_prefix": "Virheellinen etuliite", "using_uploaded_file": "Käytetään ladattua tiedostoa", "no_url": "URL:ää ei löytynyt pyynnön rungosta", + "invalid_url": "Virheellinen URL", + "blocked_address": "Tämä osoite ei ole sallittu. Ota yhteyttä järjestelmänvalvojaan, jos tarvitset pääsyn siihen.", "downloading": "Ladataan", "download_error": "Latausvirhe", "fetching_url": "Haetaan artikkelia URL:stä", diff --git a/locales/fr.json b/locales/fr.json index 2ceb9904..dac06957 100644 --- a/locales/fr.json +++ b/locales/fr.json @@ -383,6 +383,8 @@ "invalid_prefix": "Préfixe invalide", "using_uploaded_file": "Utilisation du fichier téléchargé", "no_url": "Aucune URL trouvée dans le corps de la requête", + "invalid_url": "URL invalide", + "blocked_address": "Cette adresse n'est pas autorisée. Contactez votre administrateur si vous avez besoin d'y accéder.", "downloading": "Téléchargement", "download_error": "Erreur de téléchargement", "fetching_url": "Récupération de l'article depuis l'URL", diff --git a/locales/it.json b/locales/it.json index dd66d95f..db3c92d4 100644 --- a/locales/it.json +++ b/locales/it.json @@ -359,6 +359,8 @@ "invalid_prefix": "Prefisso non valido", "using_uploaded_file": "Utilizzo del file caricato", "no_url": "Nessun URL trovato nel corpo della richiesta", + "invalid_url": "URL non valido", + "blocked_address": "Questo indirizzo non è consentito. Contatta il tuo amministratore se hai bisogno di accedervi.", "downloading": "Download in corso", "download_error": "Errore di download", "fetching_url": "Recupero articolo da URL", diff --git a/locales/ja.json b/locales/ja.json index fe2af2bc..16214854 100644 --- a/locales/ja.json +++ b/locales/ja.json @@ -545,6 +545,8 @@ "invalid_prefix": "無効なプレフィックス", "using_uploaded_file": "アップロードされたファイルを使用中", "no_url": "リクエストボディにURLが見つかりません", + "invalid_url": "無効なURL", + "blocked_address": "このアドレスは許可されていません。アクセスが必要な場合は管理者にお問い合わせください。", "downloading": "ダウンロード中", "download_error": "ダウンロードエラー", "fetching_url": "URLから記事を取得中", diff --git a/locales/ko.json b/locales/ko.json index 3b1052a4..3b53a730 100644 --- a/locales/ko.json +++ b/locales/ko.json @@ -545,6 +545,8 @@ "invalid_prefix": "잘못된 접두사", "using_uploaded_file": "업로드된 파일 사용 중", "no_url": "요청 본문에서 URL을 찾을 수 없음", + "invalid_url": "잘못된 URL", + "blocked_address": "이 주소는 허용되지 않습니다. 접근이 필요한 경우 관리자에게 문의하세요.", "downloading": "다운로드 중", "download_error": "다운로드 오류", "fetching_url": "URL에서 기사 가져오는 중", diff --git a/locales/nl.json b/locales/nl.json index 26c3b25c..94291025 100644 --- a/locales/nl.json +++ b/locales/nl.json @@ -545,6 +545,8 @@ "invalid_prefix": "Ongeldige prefix", "using_uploaded_file": "Geüpload bestand gebruiken", "no_url": "Geen URL gevonden in verzoek body", + "invalid_url": "Ongeldige URL", + "blocked_address": "Dit adres is niet toegestaan. Neem contact op met uw beheerder als u er toegang toe nodig heeft.", "downloading": "Downloaden", "download_error": "Download fout", "fetching_url": "Artikel ophalen van URL", diff --git a/locales/no.json b/locales/no.json index a7164ecc..33cae485 100644 --- a/locales/no.json +++ b/locales/no.json @@ -545,6 +545,8 @@ "invalid_prefix": "Ugyldig prefiks", "using_uploaded_file": "Bruker opplastet fil", "no_url": "Ingen URL funnet i forespørsel kropp", + "invalid_url": "Ugyldig URL", + "blocked_address": "Denne adressen er ikke tillatt. Kontakt administratoren din hvis du trenger tilgang til den.", "downloading": "Laster ned", "download_error": "Nedlastingsfeil", "fetching_url": "Henter artikkel fra URL", diff --git a/locales/pl.json b/locales/pl.json index ad3b7411..0b6b3168 100644 --- a/locales/pl.json +++ b/locales/pl.json @@ -545,6 +545,8 @@ "invalid_prefix": "Nieprawidłowy prefiks", "using_uploaded_file": "Korzystanie z przesłanego pliku", "no_url": "Nie znaleziono URL w treści żądania", + "invalid_url": "Nieprawidłowy URL", + "blocked_address": "Ten adres jest niedozwolony. Skontaktuj się z administratorem, jeśli potrzebujesz do niego dostępu.", "downloading": "Pobieranie", "download_error": "Błąd pobierania", "fetching_url": "Pobieranie artykułu z URL", diff --git a/locales/pt.json b/locales/pt.json index 60a187e8..7004ca9e 100644 --- a/locales/pt.json +++ b/locales/pt.json @@ -545,6 +545,8 @@ "invalid_prefix": "Prefixo inválido", "using_uploaded_file": "Usando arquivo enviado", "no_url": "Nenhuma URL encontrada no corpo da solicitação", + "invalid_url": "URL inválida", + "blocked_address": "Este endereço não é permitido. Entre em contato com seu administrador se precisar de acesso a ele.", "downloading": "Baixando", "download_error": "Erro de download", "fetching_url": "Buscando artigo do URL", diff --git a/locales/sv.json b/locales/sv.json index 197bf801..83984d86 100644 --- a/locales/sv.json +++ b/locales/sv.json @@ -545,6 +545,8 @@ "invalid_prefix": "Ogiltigt prefix", "using_uploaded_file": "Använder uppladdad fil", "no_url": "Ingen URL hittad i begärans kropp", + "invalid_url": "Ogiltig URL", + "blocked_address": "Den här adressen är inte tillåten. Kontakta din administratör om du behöver åtkomst till den.", "downloading": "Laddar ner", "download_error": "Nedladdningsfel", "fetching_url": "Hämtar artikel från URL", diff --git a/locales/zh-CN.json b/locales/zh-CN.json index 1a9f4041..15f99252 100644 --- a/locales/zh-CN.json +++ b/locales/zh-CN.json @@ -545,6 +545,8 @@ "invalid_prefix": "无效前缀", "using_uploaded_file": "使用上传的文件", "no_url": "在请求体中未找到URL", + "invalid_url": "无效的URL", + "blocked_address": "此地址不被允许。如果您需要访问它，请联系您的管理员。", "downloading": "下载中", "download_error": "下载错误", "fetching_url": "从URL获取文章", From b0beb6f9687bb675ac0ffbd3ec8108a511772a25 Mon Sep 17 00:00:00 2001 From: Mitchell Scott Date: Wed, 26 Aug 2026 09:36:02 -0600 Subject: [PATCH 2/2] feat!: block private IP fetches by default --- docs/CONFIGURATION.md | 6 ++++-- internal/downloader/sniff_test.go | 5 +++++ internal/security/httpclient_test.go | 24 +++++++++++++++++++++--- internal/security/urlvalidation.go | 10 +++++----- internal/security/urlvalidation_test.go | 2 +- 5 files changed, 36 insertions(+), 11 deletions(-) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index a2126db7..acca97e5 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -56,18 +56,20 @@ For more rmapi-specific configuration, see [their documentation](https://github. | Variable | Required? | Default | Description | |--------------------------|-----------|---------|-------------| -| BLOCK_PRIVATE_IPS | No | false | Set to `true` to block URLs pointing to private/local IP addresses (RFC1918, loopback). Link-local is always blocked. | +| BLOCK_PRIVATE_IPS | No | true | Set to `false` to allow URLs pointing to private/local IP addresses (RFC1918, loopback). Link-local is always blocked. | | BLOCKED_DOMAINS | No | | Comma-separated list of domains to block (e.g., `internal.corp,local.net`) | ### Security Configuration Notes - **Link-local addresses are always blocked**, whatever `BLOCK_PRIVATE_IPS` is set to: 169.254.0.0/16 and fe80::/10. This range carries the cloud instance metadata endpoints (169.254.169.254 and 169.254.170.2), and no deployment serves documents from it. Every address is checked at connection time, including each hop of a redirect, so a redirect or a DNS record that changes between check and fetch cannot reach a blocked address. -- **BLOCK_PRIVATE_IPS**: Left at the default, Aviary can fetch from your own network, which is what makes `http://192.168.1.50/doc.pdf` work. Set it to `true` to also block URLs that resolve to: +- **BLOCK_PRIVATE_IPS**: Left at the default, Aviary refuses to fetch from your own network. Set it to `false` if you point Aviary at a host on your LAN, at a Docker Compose sibling by service name, or at a machine on your tailnet. With the default in place, these are refused: - Private IPv4 ranges: 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16 - Loopback addresses: 127.0.0.0/8, ::1 - Carrier-grade NAT: 100.64.0.0/10 - Unique local IPv6: fc00::/7 - Other special-use addresses + + A refused address is reported to the user as a blocked address, and the server log names the address and this variable. - **BLOCKED_DOMAINS**: Blocks specific domains and their subdomains. For example, setting `BLOCKED_DOMAINS=example.com` will block both `example.com` and `*.example.com` ## Multi-User Mode Configuration diff --git a/internal/downloader/sniff_test.go b/internal/downloader/sniff_test.go index f4adead3..740a7d9b 100644 --- a/internal/downloader/sniff_test.go +++ b/internal/downloader/sniff_test.go @@ -5,11 +5,16 @@ import ( "net/http" "net/http/httptest" "net/url" + "os" "testing" "time" ) func TestSniffMimeTimeout(t *testing.T) { + // the test server listens on loopback, which is blocked by default + os.Setenv("BLOCK_PRIVATE_IPS", "false") + defer os.Unsetenv("BLOCK_PRIVATE_IPS") + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { time.Sleep(200 * time.Millisecond) w.WriteHeader(http.StatusOK) diff --git a/internal/security/httpclient_test.go b/internal/security/httpclient_test.go index a1489661..5db9f65c 100644 --- a/internal/security/httpclient_test.go +++ b/internal/security/httpclient_test.go @@ -16,7 +16,8 @@ func newTestClient() *http.Client { } func TestRedirectToLinkLocalRefused(t *testing.T) { - os.Unsetenv("BLOCK_PRIVATE_IPS") + os.Setenv("BLOCK_PRIVATE_IPS", "false") + defer os.Unsetenv("BLOCK_PRIVATE_IPS") server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, "http://169.254.169.254/latest/meta-data/", http.StatusFound) @@ -45,7 +46,8 @@ func TestDialToLinkLocalRefused(t *testing.T) { } func TestRedirectLimit(t *testing.T) { - os.Unsetenv("BLOCK_PRIVATE_IPS") + os.Setenv("BLOCK_PRIVATE_IPS", "false") + defer os.Unsetenv("BLOCK_PRIVATE_IPS") server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, "/again", http.StatusFound) @@ -73,9 +75,25 @@ func TestLinkLocalBlockedRegardlessOfFlag(t *testing.T) { } } -func TestPrivateAddressesAllowedByDefault(t *testing.T) { +func TestPrivateAddressesBlockedByDefault(t *testing.T) { os.Unsetenv("BLOCK_PRIVATE_IPS") + for _, rawURL := range []string{ + "http://192.168.1.50/doc.pdf", + "http://10.0.0.5/doc.pdf", + "http://127.0.0.1:8080/doc.pdf", + "http://100.64.0.1/doc.pdf", + } { + if err := ValidateURL(rawURL); !errors.Is(err, ErrPrivateIP) { + t.Errorf("ValidateURL(%s) = %v, want ErrPrivateIP", rawURL, err) + } + } +} + +func TestPrivateAddressesAllowedWhenOptedOut(t *testing.T) { + os.Setenv("BLOCK_PRIVATE_IPS", "false") + defer os.Unsetenv("BLOCK_PRIVATE_IPS") + for _, rawURL := range []string{ "http://192.168.1.50/doc.pdf", "http://10.0.0.5/doc.pdf", diff --git a/internal/security/urlvalidation.go b/internal/security/urlvalidation.go index 1b293a38..2b05e327 100644 --- a/internal/security/urlvalidation.go +++ b/internal/security/urlvalidation.go @@ -54,20 +54,20 @@ func ValidateURL(rawURL string) error { } func blockPrivateIPs() bool { - return config.Get("BLOCK_PRIVATE_IPS", "") == "true" + return config.Get("BLOCK_PRIVATE_IPS", "true") != "false" } // checkIPAllowed rejects an address the server must never connect to. Link-local // space carries the cloud metadata endpoints and is refused unconditionally. -// The wider private ranges are refused only when BLOCK_PRIVATE_IPS is set, so a -// self-hosted deployment can still fetch from its own network. +// The wider private ranges are refused unless BLOCK_PRIVATE_IPS is set to false, +// which a deployment that fetches from its own network has to opt into. func checkIPAllowed(ip net.IP) error { if isLinkLocal(ip) { return fmt.Errorf("%w: %s", ErrLinkLocal, ip.String()) } if blockPrivateIPs() && isPrivateIP(ip) { - return fmt.Errorf("%w: %s (unset BLOCK_PRIVATE_IPS to allow)", ErrPrivateIP, ip.String()) + return fmt.Errorf("%w: %s (set BLOCK_PRIVATE_IPS=false to allow)", ErrPrivateIP, ip.String()) } return nil @@ -82,7 +82,7 @@ func IsBlockedAddress(err error) bool { } // checkHostAddresses validates an IP literal directly. A hostname is resolved -// here only when BLOCK_PRIVATE_IPS is set; otherwise the guarded dialer applies +// here only when private ranges are blocked; otherwise the guarded dialer applies // checkIPAllowed to whatever address the connection actually reaches, which // avoids a DNS lookup on every validation and closes the rebinding gap. func checkHostAddresses(hostname string) error { diff --git a/internal/security/urlvalidation_test.go b/internal/security/urlvalidation_test.go index edd1cd80..d65971f4 100644 --- a/internal/security/urlvalidation_test.go +++ b/internal/security/urlvalidation_test.go @@ -145,7 +145,7 @@ func TestValidateURL(t *testing.T) { if tt.blockPrivate { os.Setenv("BLOCK_PRIVATE_IPS", "true") } else { - os.Unsetenv("BLOCK_PRIVATE_IPS") + os.Setenv("BLOCK_PRIVATE_IPS", "false") } if tt.blockedDomains != "" {