Validate tenantID on single resolver (#6727)

Signed-off-by: SungJin1212 <[email protected]>

Author: SungJin1212

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## master / unreleased
 * [CHANGE] Ingester: Remove EnableNativeHistograms config flag and instead gate keep through new per-tenant limit at ingestion. #6718
 * [CHANGE] StoreGateway/Alertmanager: Add default 5s connection timeout on client. #6603
+* [CHANGE] Validate a tenantID when to use a single tenant resolver. #6727
 * [FEATURE] Query Frontend: Add dynamic interval size for query splitting. This is enabled by configuring experimental flags `querier.max-shards-per-query` and/or `querier.max-fetched-data-duration-per-query`. The split interval size is dynamically increased to maintain a number of shards and total duration fetched below the configured values. #6458
 * [FEATURE] Querier/Ruler: Add `query_partial_data` and `rules_partial_data` limits to allow queries/rules to be evaluated with data from a single zone, if other zones are not available. #6526
 * [FEATURE] Update prometheus alertmanager version to v0.28.0 and add new integration msteamsv2, jira, and rocketchat. #6590

pkg/tenant/resolver.go

@@ -2,7 +2,6 @@ package tenant
 
 import (
 	"context"
-	"errors"
 	"net/http"
 	"strings"
 
@@ -67,24 +66,18 @@ type SingleResolver struct {
 // `\` can be found.
 func containsUnsafePathSegments(id string) bool {
 	// handle the relative reference to current and parent path.
-	if id == "." || id == ".." {
-		return true
-	}
-
-	return strings.ContainsAny(id, "\\/")
+	return id == "." || id == ".."
 }
 
-var errInvalidTenantID = errors.New("invalid tenant ID")
-
 func (t *SingleResolver) TenantID(ctx context.Context) (string, error) {
 	//lint:ignore faillint wrapper around upstream method
 	id, err := user.ExtractOrgID(ctx)
 	if err != nil {
 		return "", err
 	}
 
-	if containsUnsafePathSegments(id) {
-		return "", errInvalidTenantID
+	if err := ValidTenantID(id); err != nil {
+		return "", err
 	}
 
 	return id, nil
@@ -134,9 +127,6 @@ func (t *MultiResolver) TenantIDs(ctx context.Context) ([]string, error) {
 		if err := ValidTenantID(orgID); err != nil {
 			return nil, err
 		}
-		if containsUnsafePathSegments(orgID) {
-			return nil, errInvalidTenantID
-		}
 	}
 
 	return NormalizeTenantIDs(orgIDs), nil

pkg/tenant/resolver_test.go

@@ -67,37 +67,49 @@ var commonResolverTestCases = []resolverTestCase{
 	{
 		name:         "parent-dir",
 		headerValue:  strptr(".."),
-		errTenantID:  errInvalidTenantID,
-		errTenantIDs: errInvalidTenantID,
+		errTenantID:  errTenantIDUnsafe,
+		errTenantIDs: errTenantIDUnsafe,
 	},
 	{
 		name:         "current-dir",
 		headerValue:  strptr("."),
-		errTenantID:  errInvalidTenantID,
-		errTenantIDs: errInvalidTenantID,
+		errTenantID:  errTenantIDUnsafe,
+		errTenantIDs: errTenantIDUnsafe,
+	},
+	{
+		name:         "white space",
+		headerValue:  strptr(" "),
+		errTenantID:  &errTenantIDUnsupportedCharacter{pos: 0, tenantID: " "},
+		errTenantIDs: &errTenantIDUnsupportedCharacter{pos: 0, tenantID: " "},
+	},
+	{
+		name:         "containing-forward-slash",
+		headerValue:  strptr("forward/slash"),
+		errTenantID:  &errTenantIDUnsupportedCharacter{pos: 7, tenantID: "forward/slash"},
+		errTenantIDs: &errTenantIDUnsupportedCharacter{pos: 7, tenantID: "forward/slash"},
+	},
+	{
+		name:         "containing-backward-slash",
+		headerValue:  strptr(`backward\slash`),
+		errTenantID:  &errTenantIDUnsupportedCharacter{pos: 8, tenantID: "backward\\slash"},
+		errTenantIDs: &errTenantIDUnsupportedCharacter{pos: 8, tenantID: "backward\\slash"},
 	},
 }
 
 func TestSingleResolver(t *testing.T) {
 	r := NewSingleResolver()
 	for _, tc := range append(commonResolverTestCases, []resolverTestCase{
 		{
-			name:        "multi-tenant",
-			headerValue: strptr("tenant-a|tenant-b"),
-			tenantID:    "tenant-a|tenant-b",
-			tenantIDs:   []string{"tenant-a|tenant-b"},
-		},
-		{
-			name:         "containing-forward-slash",
-			headerValue:  strptr("forward/slash"),
-			errTenantID:  errInvalidTenantID,
-			errTenantIDs: errInvalidTenantID,
+			name:         "multi-tenant",
+			headerValue:  strptr("tenant-a|tenant-b"),
+			errTenantID:  &errTenantIDUnsupportedCharacter{pos: 8, tenantID: "tenant-a|tenant-b"},
+			errTenantIDs: &errTenantIDUnsupportedCharacter{pos: 8, tenantID: "tenant-a|tenant-b"},
 		},
 		{
-			name:         "containing-backward-slash",
-			headerValue:  strptr(`backward\slash`),
-			errTenantID:  errInvalidTenantID,
-			errTenantIDs: errInvalidTenantID,
+			name:         "containing-invalid-character",
+			headerValue:  strptr("+"),
+			errTenantID:  &errTenantIDUnsupportedCharacter{pos: 0, tenantID: "+"},
+			errTenantIDs: &errTenantIDUnsupportedCharacter{pos: 0, tenantID: "+"},
 		},
 	}...) {
 		t.Run(tc.name, tc.test(r))
@@ -126,22 +138,16 @@ func TestMultiResolver(t *testing.T) {
 			tenantIDs:   []string{"tenant-a", "tenant-b"},
 		},
 		{
-			name:         "multi-tenant-with-relative-path",
-			headerValue:  strptr("tenant-a|tenant-b|.."),
-			errTenantID:  errInvalidTenantID,
-			errTenantIDs: errInvalidTenantID,
+			name:         "multi-tenant-with-unsafe-path-segment(.)",
+			headerValue:  strptr("tenant-a|tenant-b|."),
+			errTenantID:  errTenantIDUnsafe,
+			errTenantIDs: errTenantIDUnsafe,
 		},
 		{
-			name:         "containing-forward-slash",
-			headerValue:  strptr("forward/slash"),
-			errTenantID:  &errTenantIDUnsupportedCharacter{pos: 7, tenantID: "forward/slash"},
-			errTenantIDs: &errTenantIDUnsupportedCharacter{pos: 7, tenantID: "forward/slash"},
-		},
-		{
-			name:         "containing-backward-slash",
-			headerValue:  strptr(`backward\slash`),
-			errTenantID:  &errTenantIDUnsupportedCharacter{pos: 8, tenantID: "backward\\slash"},
-			errTenantIDs: &errTenantIDUnsupportedCharacter{pos: 8, tenantID: "backward\\slash"},
+			name:         "multi-tenant-with-unsafe-path-segment(..)",
+			headerValue:  strptr("tenant-a|tenant-b|.."),
+			errTenantID:  errTenantIDUnsafe,
+			errTenantIDs: errTenantIDUnsafe,
 		},
 	}...) {
 		t.Run(tc.name, tc.test(r))

pkg/tenant/tenant.go

@@ -12,6 +12,7 @@ import (
 
 var (
 	errTenantIDTooLong = errors.New("tenant ID is too long: max 150 characters")
+	errTenantIDUnsafe  = errors.New("tenant ID is '.' or '..'")
 )
 
 type errTenantIDUnsupportedCharacter struct {
@@ -65,6 +66,10 @@ func ValidTenantID(s string) error {
 		return errTenantIDTooLong
 	}
 
+	if containsUnsafePathSegments(s) {
+		return errTenantIDUnsafe
+	}
+
 	return nil
 }
 

pkg/tenant/tenant_test.go

@@ -29,6 +29,14 @@ func TestValidTenantIDs(t *testing.T) {
 			name: strings.Repeat("a", 151),
 			err:  strptr("tenant ID is too long: max 150 characters"),
 		},
+		{
+			name: ".",
+			err:  strptr("tenant ID is '.' or '..'"),
+		},
+		{
+			name: "..",
+			err:  strptr("tenant ID is '.' or '..'"),
+		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			err := ValidTenantID(tc.name)