Address review feedback: TLS ServerName/CA, Find nil-check, config fail-fast

Addresses external review comments on the valkey-store backend:

- StoresFind now rejects a nil/empty query Key before dereferencing it,
  so a malformed gRPC request can no longer panic the backend.
- TLS: derive ServerName (SNI) from the VALKEY_ADDR host so certificate
  verification works for IP-addressed endpoints, and add VALKEY_TLS_CA_CERT
  (custom CA bundle) and VALKEY_TLS_SKIP_VERIFY (testing-only) knobs.
- Config integer parsing now fails fast on a malformed value (e.g.
  VALKEY_HNSW_M=1x6) instead of silently defaulting, matching the
  fail-fast behaviour of the index-algo/distance-metric validation.
- Add VALKEY_DB (SELECT n) support for logical-DB isolation.
- Cap the human-readable part of a namespace token at 64 chars so a very
  long model name cannot produce an unbounded key prefix / index name
  (the appended short hash keeps distinct namespaces collision-free).
- Document the KNN-query injection-safety invariant (fields are constants)
  and why StoresGet uses a single aggregate DoMulti deadline for reads.
- Unit tests for the Find nil/empty-key guard, fail-fast HNSW parsing,
  and VALKEY_DB parsing/validation; docs + .env updated for the new vars.

Assisted-by: Kiro:claude-opus-4.8 golangci-lint

Author: daric93

.env

@@ -102,7 +102,10 @@
 # VALKEY_USERNAME=
 # VALKEY_PASSWORD=
 # VALKEY_TLS=false
+# VALKEY_TLS_CA_CERT=
+# VALKEY_TLS_SKIP_VERIFY=false
 # VALKEY_CLIENT_NAME=localai-valkey-store
+# VALKEY_DB=0
 # VALKEY_INDEX_ALGO=FLAT
 # VALKEY_DISTANCE_METRIC=COSINE
 # VALKEY_HNSW_M=16

backend/go/valkey-store/config.go

@@ -73,7 +73,10 @@ type Config struct {
 	Username       string
 	Password       string
 	UseTLS         bool
+	TLSSkipVerify  bool
+	TLSCACert      string
 	ClientName     string
+	DB             int
 	IndexAlgo      string
 	DistanceMetric string
 	HNSW           hnswParams
@@ -84,20 +87,47 @@ type Config struct {
 // It fails fast on an unknown index algorithm or distance metric so a
 // misconfiguration surfaces at Load() rather than silently degrading search.
 func loadConfig() (Config, error) {
+	// intOr parses an integer env var, failing fast on a malformed value the
+	// same way an invalid index algo or distance metric does. A typo like
+	// VALKEY_HNSW_M=1x6 must surface at Load() rather than silently degrading
+	// to the default and producing subtly wrong (and hard-to-diagnose) index
+	// behaviour. The first parse error wins and is returned below.
+	var parseErr error
+	intOr := func(key string, fallback int) int {
+		v := os.Getenv(key)
+		if v == "" {
+			return fallback
+		}
+		n, err := strconv.Atoi(v)
+		if err != nil {
+			if parseErr == nil {
+				parseErr = fmt.Errorf("valkey-store: invalid %s %q: %w", key, v, err)
+			}
+			return fallback
+		}
+		return n
+	}
+
 	cfg := Config{
 		Addr:           envOr("VALKEY_ADDR", _defaultAddr),
 		Username:       os.Getenv("VALKEY_USERNAME"),
 		Password:       os.Getenv("VALKEY_PASSWORD"),
 		UseTLS:         envBool("VALKEY_TLS", false),
+		TLSSkipVerify:  envBool("VALKEY_TLS_SKIP_VERIFY", false),
+		TLSCACert:      os.Getenv("VALKEY_TLS_CA_CERT"),
 		ClientName:     envOr("VALKEY_CLIENT_NAME", _defaultClientName),
+		DB:             intOr("VALKEY_DB", 0),
 		IndexAlgo:      strings.ToUpper(envOr("VALKEY_INDEX_ALGO", _defaultIndexAlgo)),
 		DistanceMetric: strings.ToUpper(envOr("VALKEY_DISTANCE_METRIC", _defaultDistanceMetric)),
 		HNSW: hnswParams{
-			M:              envInt("VALKEY_HNSW_M", _defaultHNSWM),
-			EFConstruction: envInt("VALKEY_HNSW_EF_CONSTRUCTION", _defaultHNSWEFConstruction),
-			EFRuntime:      envInt("VALKEY_HNSW_EF_RUNTIME", _defaultHNSWEFRuntime),
+			M:              intOr("VALKEY_HNSW_M", _defaultHNSWM),
+			EFConstruction: intOr("VALKEY_HNSW_EF_CONSTRUCTION", _defaultHNSWEFConstruction),
+			EFRuntime:      intOr("VALKEY_HNSW_EF_RUNTIME", _defaultHNSWEFRuntime),
 		},
-		RequestTimeout: time.Duration(envInt("VALKEY_REQUEST_TIMEOUT_MS", _defaultRequestTimeoutMS)) * time.Millisecond,
+		RequestTimeout: time.Duration(intOr("VALKEY_REQUEST_TIMEOUT_MS", _defaultRequestTimeoutMS)) * time.Millisecond,
+	}
+	if parseErr != nil {
+		return Config{}, parseErr
 	}
 
 	// ClientName is mandatory. Restore the default if the operator blanked it,
@@ -106,6 +136,10 @@ func loadConfig() (Config, error) {
 		cfg.ClientName = _defaultClientName
 	}
 
+	if cfg.DB < 0 {
+		return Config{}, fmt.Errorf("valkey-store: invalid VALKEY_DB %d (must be >= 0)", cfg.DB)
+	}
+
 	switch cfg.IndexAlgo {
 	case indexAlgoFlat, indexAlgoHNSW:
 	default:
@@ -146,16 +180,3 @@ func envBool(key string, fallback bool) bool {
 	}
 	return b
 }
-
-func envInt(key string, fallback int) int {
-	v := os.Getenv(key)
-	if v == "" {
-		return fallback
-	}
-	n, err := strconv.Atoi(v)
-	if err != nil {
-		xlog.Warn("valkey-store: ignoring unparseable env var, using default", "key", key, "value", v, "default", fallback)
-		return fallback
-	}
-	return n
-}

backend/go/valkey-store/store.go

@@ -28,8 +28,11 @@ import (
 	"context"
 	"crypto/sha256"
 	"crypto/tls"
+	"crypto/x509"
 	"encoding/hex"
 	"fmt"
+	"net"
+	"os"
 	"strconv"
 	"strings"
 
@@ -59,6 +62,12 @@ const (
 	// it is in-memory; a networked backend wants a guard. Callers asking for
 	// more than this get the top _maxTopK results.
 	_maxTopK = 10000
+
+	// _maxNsTokenLen bounds the human-readable portion of a namespace token so
+	// a very long model name cannot produce an unbounded key prefix / index
+	// name. The appended short hash keeps distinct namespaces collision-free
+	// even when their sanitized prefixes are truncated to the same value.
+	_maxNsTokenLen = 64
 )
 
 // ValkeyStore implements the gRPC store Backend against Valkey Search.
@@ -122,12 +131,20 @@ func (s *ValkeyStore) Load(opts *pb.ModelOptions) error {
 		Username:    cfg.Username,
 		Password:    cfg.Password,
 		ClientName:  cfg.ClientName,
+		// SelectDB picks a logical Valkey DB (SELECT n) for deployments that use
+		// numbered DBs for isolation. Defaults to 0; namespace prefixing already
+		// isolates keyspaces on a shared DB.
+		SelectDB: cfg.DB,
 		// Disable client-side caching: values are opaque blobs written once and
 		// read rarely, so tracking invalidations would only add overhead.
 		DisableCache: true,
 	}
 	if cfg.UseTLS {
-		clientOpt.TLSConfig = &tls.Config{}
+		tlsCfg, err := buildTLSConfig(cfg)
+		if err != nil {
+			return err
+		}
+		clientOpt.TLSConfig = tlsCfg
 	}
 
 	// Close any client from a previous Load so a re-entrant Load does not leak
@@ -255,6 +272,35 @@ func (s *ValkeyStore) Free() error {
 	return nil
 }
 
+// buildTLSConfig assembles the tls.Config for a VALKEY_TLS=true connection.
+// Go only auto-derives ServerName (SNI) from the dial address for hostnames;
+// for an IP-addressed endpoint (e.g. 10.0.0.5:6379) SNI is left empty and the
+// certificate's SANs won't match the raw IP, so verification fails. We set it
+// explicitly from the configured host so both hostname and IP endpoints verify.
+// A custom CA bundle (VALKEY_TLS_CA_CERT) and an explicit insecure-skip escape
+// hatch (VALKEY_TLS_SKIP_VERIFY) are supported for enterprise/self-signed setups.
+func buildTLSConfig(cfg Config) (*tls.Config, error) {
+	tlsCfg := &tls.Config{}
+	if host, _, err := net.SplitHostPort(cfg.Addr); err == nil && host != "" {
+		tlsCfg.ServerName = host
+	}
+	if cfg.TLSSkipVerify {
+		tlsCfg.InsecureSkipVerify = true
+	}
+	if cfg.TLSCACert != "" {
+		pem, err := os.ReadFile(cfg.TLSCACert)
+		if err != nil {
+			return nil, fmt.Errorf("valkey-store: read VALKEY_TLS_CA_CERT %q: %w", cfg.TLSCACert, err)
+		}
+		pool := x509.NewCertPool()
+		if !pool.AppendCertsFromPEM(pem) {
+			return nil, fmt.Errorf("valkey-store: VALKEY_TLS_CA_CERT %q: no valid certificate found", cfg.TLSCACert)
+		}
+		tlsCfg.RootCAs = pool
+	}
+	return tlsCfg, nil
+}
+
 // ctx returns a request-scoped context bounded by the configured timeout. We
 // never rely on the client's built-in write timeout because index back-fill
 // and large KNN queries can legitimately exceed a short default.
@@ -332,6 +378,13 @@ func (s *ValkeyStore) StoresGet(opts *pb.StoresGetOptions) (pb.StoresGetResult,
 		return pb.StoresGetResult{}, err
 	}
 
+	// Reads pipeline the whole batch via DoMulti under ONE aggregate deadline,
+	// unlike Set/Delete which use a per-command timeout. That asymmetry is
+	// deliberate: HGET is non-mutating, so exhausting the deadline mid-batch
+	// only truncates the result (surfaced as an error) — it can never leave a
+	// partial write behind, which is the specific hazard the per-command timeout
+	// guards against for Set/Delete. Pipelining is also safe here because these
+	// are non-indexed reads (the indexed-DoMulti deadlock only affects writes).
 	ctx, cancel := s.ctx()
 	defer cancel()
 
@@ -394,6 +447,11 @@ func (s *ValkeyStore) StoresDelete(opts *pb.StoresDeleteOptions) error {
 // metric, ordered most-similar first. An empty/uncreated index returns empty
 // slices and no error, matching local-store's empty-store behaviour.
 func (s *ValkeyStore) StoresFind(opts *pb.StoresFindOptions) (pb.StoresFindResult, error) {
+	// Guard against a malformed gRPC request with a nil/empty Key before
+	// dereferencing it — a nil opts.Key would otherwise panic the backend.
+	if opts.Key == nil || len(opts.Key.Floats) == 0 {
+		return pb.StoresFindResult{}, fmt.Errorf("valkey-store: Find: query key is empty")
+	}
 	query := opts.Key.Floats
 	topK := int(opts.TopK)
 	if topK < 1 {
@@ -428,6 +486,12 @@ func (s *ValkeyStore) StoresFind(opts *pb.StoresFindOptions) (pb.StoresFindResul
 	// SORTABLE schema attribute). LIMIT 0 topK caps the result and DIALECT 2 is
 	// required for the =>[KNN ...] vector syntax. The __score field is still
 	// returned in each document and read back for the similarity conversion.
+	//
+	// Injection-safety: the only caller-controlled value interpolated here is
+	// topK (an int, already bounded above). _vecField and _scoreField are
+	// compile-time constants, so this Sprintf cannot be used to inject query
+	// syntax. Do NOT make those fields operator-configurable without sanitizing
+	// them first — the KNN query string is otherwise built only from constants.
 	q := fmt.Sprintf("*=>[KNN %d @%s $q AS %s]", topK, _vecField, _scoreField)
 	cmd := s.client.B().FtSearch().Index(s.indexName).Query(q).
 		Return("3").Identifier(_vecField).Identifier(_valField).Identifier(_scoreField).
@@ -586,7 +650,15 @@ func indexName(namespace string) string {
 // persisted index is found again after a restart.
 func nsToken(namespace string) string {
 	sum := sha256.Sum256([]byte(namespace))
-	return sanitize(namespace) + "-" + hex.EncodeToString(sum[:])[:8]
+	// Cap the human-readable part so a pathologically long model name can't
+	// produce an unbounded key prefix / index name (which would degrade Valkey
+	// performance). The 8-char hash suffix below already guarantees collision
+	// resistance regardless of truncation, so trimming the readable part is safe.
+	readable := sanitize(namespace)
+	if len(readable) > _maxNsTokenLen {
+		readable = readable[:_maxNsTokenLen]
+	}
+	return readable + "-" + hex.EncodeToString(sum[:])[:8]
 }
 
 // sanitize maps a namespace to a safe token for keys/index names: alphanumeric,

backend/go/valkey-store/store_test.go

@@ -47,7 +47,8 @@ var _ = Describe("loadConfig", func() {
 	// nothing, so there's no error to check).
 	envKeys := []string{
 		"VALKEY_ADDR", "VALKEY_CLIENT_NAME", "VALKEY_INDEX_ALGO", "VALKEY_DISTANCE_METRIC",
-		"VALKEY_REQUEST_TIMEOUT_MS", "VALKEY_TLS", "VALKEY_HNSW_M",
+		"VALKEY_REQUEST_TIMEOUT_MS", "VALKEY_TLS", "VALKEY_TLS_SKIP_VERIFY", "VALKEY_TLS_CA_CERT",
+		"VALKEY_DB", "VALKEY_HNSW_M", "VALKEY_HNSW_EF_CONSTRUCTION", "VALKEY_HNSW_EF_RUNTIME",
 	}
 
 	BeforeEach(func() {
@@ -97,6 +98,26 @@ var _ = Describe("loadConfig", func() {
 		_, err := loadConfig()
 		Expect(err).To(HaveOccurred())
 	})
+
+	It("fails fast on a malformed HNSW integer instead of silently defaulting", func() {
+		GinkgoT().Setenv("VALKEY_HNSW_M", "1x6")
+		_, err := loadConfig()
+		Expect(err).To(HaveOccurred())
+		Expect(err.Error()).To(ContainSubstring("VALKEY_HNSW_M"))
+	})
+
+	It("honours a valid VALKEY_DB override", func() {
+		GinkgoT().Setenv("VALKEY_DB", "3")
+		cfg, err := loadConfig()
+		Expect(err).NotTo(HaveOccurred())
+		Expect(cfg.DB).To(Equal(3))
+	})
+
+	It("rejects a negative VALKEY_DB", func() {
+		GinkgoT().Setenv("VALKEY_DB", "-1")
+		_, err := loadConfig()
+		Expect(err).To(HaveOccurred())
+	})
 })
 
 var _ = Describe("StoresSet", func() {
@@ -254,6 +275,22 @@ var _ = Describe("StoresFind", func() {
 		Expect(err).To(HaveOccurred())
 	})
 
+	It("rejects a nil Key without panicking", func() {
+		s, _ := newMockStore(testCfg())
+		s.keyLen = 3
+		s.indexCreated = true
+		_, err := s.StoresFind(&pb.StoresFindOptions{TopK: 5})
+		Expect(err).To(HaveOccurred())
+	})
+
+	It("rejects an empty query vector", func() {
+		s, _ := newMockStore(testCfg())
+		s.keyLen = 3
+		s.indexCreated = true
+		_, err := s.StoresFind(&pb.StoresFindOptions{Key: &pb.StoresKey{Floats: []float32{}}, TopK: 5})
+		Expect(err).To(HaveOccurred())
+	})
+
 	It("rejects query dimension mismatch", func() {
 		s, _ := newMockStore(testCfg())
 		s.keyLen = 3

docs/content/features/stores.md

@@ -80,7 +80,10 @@ The connection and index are configured through environment variables read by th
 | `VALKEY_USERNAME` | *(empty)* | Optional ACL username. |
 | `VALKEY_PASSWORD` | *(empty)* | Optional password / ACL secret. |
 | `VALKEY_TLS` | `false` | Enable TLS (required by many managed deployments). |
+| `VALKEY_TLS_CA_CERT` | *(empty)* | Path to a PEM CA bundle used to verify the server certificate (self-signed / private CA). |
+| `VALKEY_TLS_SKIP_VERIFY` | `false` | Skip TLS certificate verification. Insecure — for testing only. |
 | `VALKEY_CLIENT_NAME` | `localai-valkey-store` | Connection name reported by `CLIENT LIST`. Always set. |
+| `VALKEY_DB` | `0` | Logical Valkey DB index (`SELECT n`). Namespace prefixing already isolates keyspaces on a shared DB. |
 | `VALKEY_INDEX_ALGO` | `FLAT` | `FLAT` (exact, default) or `HNSW` (approximate ANN for large corpora). |
 | `VALKEY_HNSW_M` | `16` | HNSW graph degree (only when `VALKEY_INDEX_ALGO=HNSW`). |
 | `VALKEY_HNSW_EF_CONSTRUCTION` | `200` | HNSW build-time candidate list (HNSW only). |
@@ -109,7 +112,11 @@ backend.
 {{% notice warning %}}
 `VALKEY_TLS` defaults to `false` (plaintext). Set `VALKEY_TLS=true` whenever the Valkey server is
 not on `localhost` or a `VALKEY_PASSWORD`/`VALKEY_USERNAME` is configured, otherwise the credentials
-and the stored vectors travel the network unencrypted.
+and the stored vectors travel the network unencrypted. The TLS `ServerName` (SNI) is derived from
+the host portion of `VALKEY_ADDR`, so certificate verification works for both hostname and
+IP-addressed endpoints. For a self-signed / private CA, point `VALKEY_TLS_CA_CERT` at the PEM
+bundle; `VALKEY_TLS_SKIP_VERIFY=true` disables verification entirely and should only be used for
+local testing.
 {{% /notice %}}
 
 ## Set