adhocprofile: Validate IDs and santize names properly (#3470)

We currently allow any character to be user specified at upload. This
limits that character set. It also makes sure only IDs are listend and
retrieve with get, when they are fulfilling the character limit.

This validation will ensure that the generated identifiers are withing
the allowed set use for object store.

pkg/adhocprofiles/adhocprofiles.go

@@ -6,6 +6,7 @@ import (
 	"crypto/rand"
 	"encoding/base64"
 	"encoding/json"
+	"fmt"
 	"io"
 	"slices"
 	"strings"
@@ -41,6 +42,33 @@ type AdHocProfile struct {
 	UploadedAt time.Time `json:"uploadedAt"`
 }
 
+func validRunes(r rune) bool {
+	if r >= 'a' && r <= 'z' || r >= 'A' && r <= 'Z' || r >= '0' && r <= '9' || r == '.' || r == '-' || r == '_' {
+		return true
+	}
+	return false
+}
+
+// check if the id is valid
+func validID(id string) bool {
+	for _, r := range id {
+		if !validRunes(r) {
+			return false
+		}
+	}
+	return true
+}
+
+// replaces invalid runes in the id with underscores
+func replaceInvalidRunes(id string) string {
+	return strings.Map(func(r rune) rune {
+		if validRunes(r) {
+			return r
+		}
+		return '_'
+	}, id)
+}
+
 func NewAdHocProfiles(bucket objstore.Bucket, logger log.Logger, limits frontend.Limits) *AdHocProfiles {
 	a := &AdHocProfiles{
 		logger: logger,
@@ -68,6 +96,9 @@ func (a *AdHocProfiles) Upload(ctx context.Context, c *connect.Request[v1.AdHocP
 		UploadedAt: time.Now().UTC(),
 	}
 
+	// replace runes outside of [a-zA-Z0-9_-.] with underscores
+	adHocProfile.Name = replaceInvalidRunes(adHocProfile.Name)
+
 	// TODO: Add per-tenant upload limits (number of files, total size, etc.)
 
 	maxNodes, err := validation.ValidateMaxNodes(a.limits, []string{tenantID}, c.Msg.GetMaxNodes())
@@ -118,7 +149,12 @@ func (a *AdHocProfiles) Get(ctx context.Context, c *connect.Request[v1.AdHocProf
 
 	bucket := a.getBucket(tenantID)
 
-	reader, err := bucket.Get(ctx, c.Msg.GetId())
+	id := c.Msg.GetId()
+	if !validID(id) {
+		return nil, connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("id '%s' is invalid: can only contain [a-zA-Z0-9_-.]", id))
+	}
+
+	reader, err := bucket.Get(ctx, id)
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to get profile")
 	}
@@ -167,6 +203,11 @@ func (a *AdHocProfiles) List(ctx context.Context, c *connect.Request[v1.AdHocPro
 
 	profiles := make([]*v1.AdHocProfilesProfileMetadata, 0)
 	err = bucket.Iter(ctx, "", func(s string) error {
+		// do not list elements with invalid ids
+		if !validID(s) {
+			return nil
+		}
+
 		separatorIndex := strings.IndexRune(s, '-')
 		id, err := ulid.Parse(s[0:separatorIndex])
 		if err != nil {

pkg/adhocprofiles/adhocprofiles_test.go

@@ -6,6 +6,7 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"os"
+	"strings"
 	"testing"
 
 	"connectrpc.com/connect"
@@ -125,9 +126,10 @@ func TestAdHocProfiles_Upload(t *testing.T) {
 		c   *connect.Request[v1.AdHocProfilesUploadRequest]
 	}
 	tests := []struct {
-		name    string
-		args    args
-		wantErr bool
+		name           string
+		args           args
+		wantErr        bool
+		expectedSuffix string
 	}{
 		{
 			name: "reject requests with missing tenant id",
@@ -153,11 +155,24 @@ func TestAdHocProfiles_Upload(t *testing.T) {
 			args: args{
 				ctx: tenant.InjectTenantID(context.Background(), "tenant"),
 				c: connect.NewRequest(&v1.AdHocProfilesUploadRequest{
-					Name:    "test",
+					Name:    "test.cpu.pb.gz",
 					Profile: encodedProfile,
 				}),
 			},
-			wantErr: false,
+			wantErr:        false,
+			expectedSuffix: "-test.cpu.pb.gz",
+		},
+		{
+			name: "should limit profile names to particular character set",
+			args: args{
+				ctx: tenant.InjectTenantID(context.Background(), "tenant"),
+				c: connect.NewRequest(&v1.AdHocProfilesUploadRequest{
+					Name:    "test/../../../etc/passwd",
+					Profile: encodedProfile,
+				}),
+			},
+			wantErr:        false,
+			expectedSuffix: "-test_.._.._.._etc_passwd",
 		},
 	}
 	for _, tt := range tests {
@@ -172,6 +187,18 @@ func TestAdHocProfiles_Upload(t *testing.T) {
 				t.Errorf("Upload() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
+
+			if tt.expectedSuffix != "" {
+				found := false
+				err := bucket.Iter(tt.args.ctx, "tenant/adhoc", func(name string) error {
+					if strings.HasSuffix(name, tt.expectedSuffix) {
+						found = true
+					}
+					return nil
+				})
+				require.NoError(t, err)
+				require.True(t, found)
+			}
 		})
 	}
 }