mirror of
https://github.com/netbirdio/netbird.git
synced 2026-09-15 11:19:08 +02:00
[client] Strip URLs out of debug bundle upload errors
Redacting the URL in the log line left the error itself untouched, and Go's
*url.Error prints the URL whole. That error does not stay local: it becomes
UploadFailureReason for the CLI and the desktop UI, and for a remote job it is
stored in the management server's job record and shown in the dashboard. A
sample from a failed job:
Client error: 'upload debug bundle: get presigned URL: Get
"https://helloworld.asda1234:2356?id=eb23d149..."'
The service URL can carry userinfo or a query token, and the presigned URL the
service hands back carries credentials in its query by design, so the second
step leaks more than the first.
UploadDebugBundle now rewrites every URL in its error down to scheme://host, on
the way out, which covers the daemon, both mobile SDKs and the job runner at
once. The original error stays reachable through Unwrap.
Reported by cubic on #7514.
This commit is contained in:
@@ -10,6 +10,8 @@ import (
|
||||
"net/http"
|
||||
neturl "net/url"
|
||||
"os"
|
||||
"regexp"
|
||||
"strings"
|
||||
|
||||
"github.com/netbirdio/netbird/upload-server/types"
|
||||
)
|
||||
@@ -65,6 +67,13 @@ func rejectInsecureRedirect(req *http.Request, via []*http.Request) error {
|
||||
}
|
||||
|
||||
func UploadDebugBundle(ctx context.Context, url, managementURL, filePath string, insecure bool) (key string, err error) {
|
||||
// Every error out of here is surfaced somewhere durable: the daemon log, the
|
||||
// CLI, and — for a remote job — the management server's job record and the
|
||||
// dashboard. Go's *url.Error prints the URL whole, and the presigned URL the
|
||||
// service hands back can carry credentials in its query, so nothing leaves
|
||||
// this function with a URL longer than scheme://host.
|
||||
defer func() { err = redactURLsInError(err) }()
|
||||
|
||||
if !insecure {
|
||||
if err := requireHTTPS("upload service URL", url); err != nil {
|
||||
return "", err
|
||||
@@ -168,3 +177,35 @@ func getUploadURL(ctx context.Context, serviceURL string, managementURL string,
|
||||
func getURLHash(url string) string {
|
||||
return fmt.Sprintf("%x", sha256.Sum256([]byte(url)))
|
||||
}
|
||||
|
||||
// urlInText matches an absolute http(s) URL inside a free-form message.
|
||||
var urlInText = regexp.MustCompile(`https?://[^\s"']+`)
|
||||
|
||||
// redactedError keeps the original error reachable for errors.Is/As while
|
||||
// presenting a message with every URL cut down to scheme://host.
|
||||
type redactedError struct {
|
||||
msg string
|
||||
err error
|
||||
}
|
||||
|
||||
func (e *redactedError) Error() string { return e.msg }
|
||||
func (e *redactedError) Unwrap() error { return e.err }
|
||||
|
||||
func redactURLsInError(err error) error {
|
||||
if err == nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
msg := err.Error()
|
||||
redacted := urlInText.ReplaceAllStringFunc(msg, func(raw string) string {
|
||||
parsed, perr := neturl.Parse(strings.TrimRight(raw, `.,;:)]}"'`))
|
||||
if perr != nil || parsed.Host == "" {
|
||||
return "(redacted URL)"
|
||||
}
|
||||
return parsed.Scheme + "://" + parsed.Host
|
||||
})
|
||||
if redacted == msg {
|
||||
return err
|
||||
}
|
||||
return &redactedError{msg: redacted, err: err}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,72 @@
|
||||
package debug
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/url"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestRedactURLsInError(t *testing.T) {
|
||||
sentinel := errors.New("boom")
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
err error
|
||||
want string
|
||||
}{
|
||||
{name: "nil stays nil"},
|
||||
{
|
||||
name: "no URL is left alone",
|
||||
err: errors.New("file too large"),
|
||||
want: "file too large",
|
||||
},
|
||||
{
|
||||
// What a failed GET actually looks like: *url.Error prints the URL
|
||||
// whole, query included.
|
||||
name: "service URL loses its query",
|
||||
err: fmt.Errorf("get presigned URL: %w", &url.Error{
|
||||
Op: "Get",
|
||||
URL: "https://upload.example.com/upload-url?id=deadbeef",
|
||||
Err: errors.New("no such host"),
|
||||
}),
|
||||
want: `get presigned URL: Get "https://upload.example.com": no such host`,
|
||||
},
|
||||
{
|
||||
// The presigned PUT URL is the one that carries credentials.
|
||||
name: "presigned URL loses its credentials",
|
||||
err: errors.New(`upload failed: Put "https://bucket.s3.amazonaws.com/k?X-Amz-Signature=abc123&X-Amz-Credential=AKIA": timeout`),
|
||||
want: `upload failed: Put "https://bucket.s3.amazonaws.com": timeout`,
|
||||
},
|
||||
{
|
||||
name: "userinfo does not survive",
|
||||
err: errors.New(`Get "https://user:hunter2@upload.example.com/upload-url": refused`),
|
||||
want: `Get "https://upload.example.com": refused`,
|
||||
},
|
||||
{
|
||||
name: "two URLs are both cut",
|
||||
err: errors.New(`redirect from https://a.example.com/x?t=1 to https://b.example.com/y?t=2`),
|
||||
want: `redirect from https://a.example.com to https://b.example.com`,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
got := redactURLsInError(tc.err)
|
||||
if tc.err == nil {
|
||||
assert.NoError(t, got)
|
||||
return
|
||||
}
|
||||
require.Error(t, got)
|
||||
assert.Equal(t, tc.want, got.Error())
|
||||
})
|
||||
}
|
||||
|
||||
t.Run("the original error stays reachable", func(t *testing.T) {
|
||||
wrapped := fmt.Errorf(`Get "https://upload.example.com/x?t=1": %w`, sentinel)
|
||||
assert.ErrorIs(t, redactURLsInError(wrapped), sentinel)
|
||||
})
|
||||
}
|
||||
Reference in New Issue
Block a user