app/vmbackup: introduce new flag type URL (#6152)

The new flag type is supposed to be used for specifying URL values which
could contain sensitive information such as auth tokens in GET params or
HTTP basic authentication.

The URL flag also allows loading its value from files if `file://`
prefix is specified. As example, the new flag type was used in
app/vmbackup as it requires specifying `authKey` param for making the
snapshot.

See related issue
https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5973

Thanks to @wasim-nihal for initial implementation
https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6060

---------

Signed-off-by: hagen1778 <roman@victoriametrics.com>

app/vmbackup/main.go

@@ -3,7 +3,6 @@ package main
 import (
 	"flag"
 	"fmt"
-	"net/url"
 	"os"
 	"path/filepath"
 	"strings"
@@ -27,9 +26,9 @@ var (
 	httpListenAddr    = flag.String("httpListenAddr", ":8420", "TCP address for exporting metrics at /metrics page")
 	storageDataPath   = flag.String("storageDataPath", "victoria-metrics-data", "Path to VictoriaMetrics data. Must match -storageDataPath from VictoriaMetrics or vmstorage")
 	snapshotName      = flag.String("snapshotName", "", "Name for the snapshot to backup. See https://docs.victoriametrics.com/single-server-victoriametrics/#how-to-work-with-snapshots. There is no need in setting -snapshotName if -snapshot.createURL is set")
-	snapshotCreateURL = flag.String("snapshot.createURL", "", "VictoriaMetrics create snapshot url. When this is given a snapshot will automatically be created during backup. "+
+	snapshotCreateURL = flagutil.NewURL("snapshot.createURL", "VictoriaMetrics create snapshot url. When this is given a snapshot will automatically be created during backup. "+
 		"Example: http://victoriametrics:8428/snapshot/create . There is no need in setting -snapshotName if -snapshot.createURL is set")
-	snapshotDeleteURL = flag.String("snapshot.deleteURL", "", "VictoriaMetrics delete snapshot url. Optional. Will be generated from -snapshot.createURL if not provided. "+
+	snapshotDeleteURL = flagutil.NewURL("snapshot.deleteURL", "VictoriaMetrics delete snapshot url. Optional. Will be generated from -snapshot.createURL if not provided. "+
 		"All created snapshots will be automatically deleted. Example: http://victoriametrics:8428/snapshot/delete")
 	dst = flag.String("dst", "", "Where to put the backup on the remote storage. "+
 		"Example: gs://bucket/path/to/backup, s3://bucket/path/to/backup, azblob://container/path/to/backup or fs:///path/to/local/backup/dir\n"+
@@ -55,31 +54,23 @@ func main() {
 	// See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2055
 	deleteSnapshot := func() {}
 
-	if len(*snapshotCreateURL) > 0 {
-		// create net/url object
-		createURL, err := url.Parse(*snapshotCreateURL)
-		if err != nil {
-			logger.Fatalf("cannot parse snapshotCreateURL: %s", err)
-		}
+	if len(snapshotCreateURL.String()) > 0 {
 		if len(*snapshotName) > 0 {
 			logger.Fatalf("-snapshotName shouldn't be set if -snapshot.createURL is set, since snapshots are created automatically in this case")
 		}
-		logger.Infof("Snapshot create url %s", createURL.Redacted())
-		if len(*snapshotDeleteURL) <= 0 {
-			err := flag.Set("snapshot.deleteURL", strings.Replace(*snapshotCreateURL, "/create", "/delete", 1))
+
+		logger.Infof("Snapshot create url %s", snapshotCreateURL.String())
+		if len(snapshotDeleteURL.String()) <= 0 {
+			err := flag.Set("snapshot.deleteURL", strings.Replace(snapshotCreateURL.Get(), "/create", "/delete", 1))
 			if err != nil {
 				logger.Fatalf("Failed to set snapshot.deleteURL flag: %v", err)
 			}
 		}
-		deleteURL, err := url.Parse(*snapshotDeleteURL)
-		if err != nil {
-			logger.Fatalf("cannot parse snapshotDeleteURL: %s", err)
-		}
-		logger.Infof("Snapshot delete url %s", deleteURL.Redacted())
 
-		name, err := snapshot.Create(createURL.String())
+		logger.Infof("Snapshot delete url %s", snapshotDeleteURL)
+		name, err := snapshot.Create(snapshotCreateURL.Get())
 		if err != nil {
-			logger.Fatalf("cannot create snapshot: %s", err)
+			logger.Fatalf("cannot create snapshot via %q: %s", snapshotCreateURL, err)
 		}
 		err = flag.Set("snapshotName", name)
 		if err != nil {
@@ -87,9 +78,9 @@ func main() {
 		}
 
 		deleteSnapshot = func() {
-			err := snapshot.Delete(deleteURL.String(), name)
+			err := snapshot.Delete(snapshotDeleteURL.Get(), name)
 			if err != nil {
-				logger.Fatalf("cannot delete snapshot: %s", err)
+				logger.Fatalf("cannot delete snapshot via %q: %s", snapshotDeleteURL, err)
 			}
 		}
 	}

docs/CHANGELOG.md

@@ -41,6 +41,8 @@ See also [LTS releases](https://docs.victoriametrics.com/lts-releases/).
 * FEATURE: [vmauth](https://docs.victoriametrics.com/vmauth/): support regex matching when routing incoming requests based on HTTP [query args](https://en.wikipedia.org/wiki/Query_string) via `src_query_args` option at `url_map`. See [these docs](https://docs.victoriametrics.com/vmauth/#generic-http-proxy-for-different-backends) and [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6070).
 * FEATURE: [vmui](https://docs.victoriametrics.com/#vmui): optimize auto-suggestion performance for metric names when the database contains big number of unique time series.
 * FEATURE: [vmui](https://docs.victoriametrics.com/#vmui): in the Select component, user-entered values are now preserved on blur if they match options in the list.
+* FEATURE: [vmbackup](https://docs.victoriametrics.com/vmbackup): support loading values for flags `-snapshot.createURL` and `-snapshot.deleteURL` from files for security reasons. To load value from file use the following format `-snapshot.createURL=file:///abs/path/to/file` or `-snapshot.deleteURL=file://./relative/path/to/file`. See related [issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5973).
+* FEATURE: [vmbackup](https://docs.victoriametrics.com/vmbackup): hide sensitive information for  flags `-snapshot.createURL` and `-snapshot.deleteURL` when printing logs. The change masks HTTP basic authentication user and password values, as well as GET params matching `auth`, `pass`, `key`, `secret`, `token` words. See related [issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5973). Thanks to @wasim-nihal for [initial implementation](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6060).
 
 * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert/): supported any status codes from the range 200-299 from alertmanager. Previously, only 200 status code considered a successful action. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6110).
 * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert/): avoid blocking `/api/v1/rules`, `/api/v1/alerts`, `/metrics` APIs when alerting rule uses template functions `query`, which could takes a while to execute. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6079).

lib/flagutil/url.go

@@ -0,0 +1,159 @@
+package flagutil
+
+import (
+	"flag"
+	"fmt"
+	"log"
+	"net/url"
+	"os"
+	"regexp"
+	"strings"
+	"sync/atomic"
+	"unicode"
+
+	"github.com/VictoriaMetrics/VictoriaMetrics/lib/fasttime"
+)
+
+// NewURL returns new `url` flag with the given name and description.
+//
+// The url value is redacted when calling URL.String() in the following way:
+// 1. Basic Auth username and password are replaced with "xxxxx"
+// 2. Values of GET params matching `secretWordsRe` expression are replaced with "xxxxx".
+//
+// Call URL.Get() for obtaining original URL address.
+func NewURL(name, description string) *URL {
+	description += fmt.Sprintf("\nFlag value can be read from the given file when using -%s=file:///abs/path/to/file or -%s=file://./relative/path/to/file . ", name, name)
+	u := &URL{
+		flagname: name,
+	}
+	ru := &redactedURL{
+		URL:      &url.URL{},
+		redacted: "",
+	}
+	u.value.Store(&ru)
+	flag.Var(u, name, description)
+	return u
+}
+
+// URL is a flag holding URL address
+//
+// If the flag value is file:///path/to/file,
+// then its contents is automatically re-read from the given file on disk.
+type URL struct {
+	nextRefreshTimestamp atomic.Uint64
+
+	value atomic.Pointer[*redactedURL]
+
+	// flagname is the name of the flag
+	flagname string
+
+	// sourcePath contains either url or path to file with the url
+	sourcePath string
+}
+
+type redactedURL struct {
+	*url.URL
+	redacted string
+}
+
+// Get returns the current u address.
+//
+// It re-reads u value from the file:///path/to/file
+// if they were passed to URL.Set.
+func (u *URL) Get() string {
+	u.maybeRereadURL()
+	ru := *u.value.Load()
+	return ru.URL.String()
+}
+
+// Get returns the current u redacted address.
+//
+// It re-reads u value from the file:///path/to/file
+// if they were passed to URL.Set.
+func (u *URL) String() string {
+	u.maybeRereadURL()
+	ru := *u.value.Load()
+	return ru.redacted
+}
+
+func (u *URL) maybeRereadURL() {
+	if u.sourcePath == "" {
+		// Fast path - nothing to re-read
+		return
+	}
+	tsCurr := fasttime.UnixTimestamp()
+	tsNext := u.nextRefreshTimestamp.Load()
+	if tsCurr < tsNext {
+		// Fast path - nothing to re-read
+		return
+	}
+
+	// Re-read value from s.sourcePath
+	u.nextRefreshTimestamp.Store(tsCurr + 2)
+	data, err := os.ReadFile(u.sourcePath)
+	if err != nil {
+		// cannot use lib/logger, since it can be uninitialized yet
+		log.Printf("flagutil: fall back to the previous url for -%s, since failed to re-read it from %q: cannot read %q: %s\n", u.flagname, u.sourcePath, u.sourcePath, err.Error())
+	} else {
+		addr := strings.TrimRightFunc(string(data), unicode.IsSpace)
+		res, err := newRedactedURL(addr)
+		if err != nil {
+			log.Printf("flagutil: cannot parse %q: %s\n", u.flagname, err.Error())
+			return
+		}
+		u.value.Store(&res)
+	}
+}
+
+// Set implements flag.Value interface.
+func (u *URL) Set(value string) error {
+	u.nextRefreshTimestamp.Store(0)
+	var s string
+	switch {
+	case strings.HasPrefix(value, "file://"):
+		u.sourcePath = strings.TrimPrefix(value, "file://")
+		data, err := os.ReadFile(u.sourcePath)
+		if err != nil {
+			// cannot use lib/logger, since it can be uninitialized yet
+			return fmt.Errorf("cannot read %q: %w", u.sourcePath, err)
+		}
+		s = strings.TrimRightFunc(string(data), unicode.IsSpace)
+	default:
+		u.sourcePath = ""
+		s = value
+	}
+
+	res, err := newRedactedURL(s)
+	if err != nil {
+		return fmt.Errorf("cannot parse %q: %s", u.flagname, err)
+	}
+	u.value.Store(&res)
+	return nil
+}
+
+var secretWordsRe = regexp.MustCompile("auth|pass|key|secret|token")
+
+func newRedactedURL(s string) (*redactedURL, error) {
+	u, err := url.Parse(s)
+	if err != nil {
+		return nil, fmt.Errorf("cannot parse URL: %s", err)
+	}
+	ru := &redactedURL{URL: u}
+
+	// copy URL before mutating query params
+	u2 := *u
+	values := u2.Query()
+	for k, vs := range values {
+		if secretWordsRe.MatchString(k) {
+			for i := range vs {
+				vs[i] = "xxxxx"
+			}
+		}
+	}
+	u2.RawQuery = values.Encode()
+	if _, has := u2.User.Password(); has {
+		u2.User = url.UserPassword("xxxxx", "xxxxx")
+	}
+	ru.redacted = u2.String()
+	return ru, nil
+}

lib/flagutil/url_test.go

@@ -0,0 +1,45 @@
+package flagutil
+
+import (
+	"os"
+	"testing"
+)
+
+func TestNewURL(t *testing.T) {
+	u := &URL{}
+	f := func(s, exp string) {
+		t.Helper()
+		if err := u.Set(s); err != nil {
+			t.Fatalf("failed to set %q value: %s", s, err)
+		}
+		if u.String() != exp {
+			t.Fatalf("expected to get %q; got %q instead", exp, u.String())
+		}
+	}
+
+	f("", "")
+	f("http://foo:8428", "http://foo:8428")
+	f("http://username:password@foo:8428", "http://xxxxx:xxxxx@foo:8428")
+	f("http://foo:8428?authToken=bar", "http://foo:8428?authToken=xxxxx")
+	f("http://username:password@foo:8428?authToken=bar", "http://xxxxx:xxxxx@foo:8428?authToken=xxxxx")
+
+	file, err := os.CreateTemp("", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() { _ = os.Remove(file.Name()) }()
+
+	writeToFile(t, file.Name(), "http://foo:8428")
+	f("file://"+file.Name(), "http://foo:8428")
+
+	writeToFile(t, file.Name(), "http://xxxxx:password@foo:8428?authToken=bar")
+	f("file://"+file.Name(), "http://xxxxx:xxxxx@foo:8428?authToken=xxxxx")
+}
+
+func writeToFile(t *testing.T, file, b string) {
+	t.Helper()
+	err := os.WriteFile(file, []byte(b), 0644)
+	if err != nil {
+		t.Fatal(err)
+	}
+}

lib/snapshot/snapshot.go

@@ -51,13 +51,13 @@ func Create(createSnapshotURL string) (string, error) {
 		return "", err
 	}
 	if resp.StatusCode != http.StatusOK {
-		return "", fmt.Errorf("unexpected status code returned from %q: %d; expecting %d; response body: %q", u.Redacted(), resp.StatusCode, http.StatusOK, body)
+		return "", fmt.Errorf("unexpected status code: %d; expecting %d; response body: %q", resp.StatusCode, http.StatusOK, body)
 	}
 
 	snap := snapshot{}
 	err = json.Unmarshal(body, &snap)
 	if err != nil {
-		return "", fmt.Errorf("cannot parse JSON response from %q: %w; response body: %q", u.Redacted(), err, body)
+		return "", fmt.Errorf("cannot parse JSON response: %w; response body: %q", err, body)
 	}
 
 	if snap.Status == "ok" {
@@ -67,7 +67,7 @@ func Create(createSnapshotURL string) (string, error) {
 	if snap.Status == "error" {
 		return "", errors.New(snap.Msg)
 	}
-	return "", fmt.Errorf("Unkown status: %v", snap.Status)
+	return "", fmt.Errorf("unknown status: %v", snap.Status)
 }
 
 // Delete deletes a snapshot via the provided api endpoint
@@ -95,13 +95,13 @@ func Delete(deleteSnapshotURL string, snapshotName string) error {
 		return err
 	}
 	if resp.StatusCode != http.StatusOK {
-		return fmt.Errorf("unexpected status code returned from %q: %d; expecting %d; response body: %q", u.Redacted(), resp.StatusCode, http.StatusOK, body)
+		return fmt.Errorf("unexpected status code: %d; expecting %d; response body: %q", resp.StatusCode, http.StatusOK, body)
 	}
 
 	snap := snapshot{}
 	err = json.Unmarshal(body, &snap)
 	if err != nil {
-		return fmt.Errorf("cannot parse JSON response from %q: %w; response body: %q", u.Redacted(), err, body)
+		return fmt.Errorf("cannot parse JSON response: %w; response body: %q", err, body)
 	}
 
 	if snap.Status == "ok" {
@@ -111,5 +111,5 @@ func Delete(deleteSnapshotURL string, snapshotName string) error {
 	if snap.Status == "error" {
 		return errors.New(snap.Msg)
 	}
-	return fmt.Errorf("Unkown status: %v", snap.Status)
+	return fmt.Errorf("unknown status: %v", snap.Status)
 }