fix(tools/firestore): add document/collection path validation (#1229)

This change introduces robust validation for Firestore document and
collection paths across various Firestore tools.

Key changes include:
* **Path Validation:** Ensures that all Firestore paths used in tools
are relative and adhere to correct formatting, preventing issues with
absolute paths or malformed segments.
* **Improved Parameter Descriptions:** Updates the descriptions for
Firestore tool parameters to clearly specify the expectation of relative
paths (e.g., `users/userId` or `users/userId/posts/postId`) instead of
absolute paths.
* **New Utility:** Adds `internal/tools/firestore/util/validator.go` and
its corresponding test file for path validation logic.

---------

Co-authored-by: prernakakkar-google <158031829+prernakakkar-google@users.noreply.github.com>

internal/tools/firestore/firestoreadddocuments/firestoreadddocuments.go

@@ -85,7 +85,7 @@ func (cfg Config) Initialize(srcs map[string]sources.Source) (tools.Tool, error)
 	// Create parameters
 	collectionPathParameter := tools.NewStringParameter(
 		collectionPathKey,
-		"The path of the collection where the document will be added to",
+		"The relative path of the collection where the document will be added to (e.g., 'users' or 'users/userId/posts'). Note: This is a relative path, NOT an absolute path like 'projects/{project_id}/databases/{database_id}/documents/...'",
 	)
 
 	documentDataParameter := tools.NewMapParameter(
@@ -159,6 +159,11 @@ func (t Tool) Invoke(ctx context.Context, params tools.ParamValues, accessToken
 		return nil, fmt.Errorf("invalid or missing '%s' parameter", collectionPathKey)
 	}
 
+	// Validate collection path
+	if err := util.ValidateCollectionPath(collectionPath); err != nil {
+		return nil, fmt.Errorf("invalid collection path: %w", err)
+	}
+
 	// Get document data
 	documentDataRaw, ok := mapParams[documentDataKey]
 	if !ok {

internal/tools/firestore/firestoredeletedocuments/firestoredeletedocuments.go

@@ -23,6 +23,7 @@ import (
 	"github.com/googleapis/genai-toolbox/internal/sources"
 	firestoreds "github.com/googleapis/genai-toolbox/internal/sources/firestore"
 	"github.com/googleapis/genai-toolbox/internal/tools"
+	"github.com/googleapis/genai-toolbox/internal/tools/firestore/util"
 )
 
 const kind string = "firestore-delete-documents"
@@ -79,7 +80,7 @@ func (cfg Config) Initialize(srcs map[string]sources.Source) (tools.Tool, error)
 		return nil, fmt.Errorf("invalid source for %q tool: source kind must be one of %q", kind, compatibleSources)
 	}
 
-	documentPathsParameter := tools.NewArrayParameter(documentPathsKey, "Array of document paths to delete from Firestore.", tools.NewStringParameter("item", "Document path"))
+	documentPathsParameter := tools.NewArrayParameter(documentPathsKey, "Array of relative document paths to delete from Firestore (e.g., 'users/userId' or 'users/userId/posts/postId'). Note: These are relative paths, NOT absolute paths like 'projects/{project_id}/databases/{database_id}/documents/...'", tools.NewStringParameter("item", "Relative document path"))
 	parameters := tools.Parameters{documentPathsParameter}
 
 	mcpManifest := tools.McpManifest{
@@ -137,6 +138,13 @@ func (t Tool) Invoke(ctx context.Context, params tools.ParamValues, accessToken
 		return nil, fmt.Errorf("unexpected type conversion error for document paths")
 	}
 
+	// Validate each document path
+	for i, path := range documentPaths {
+		if err := util.ValidateDocumentPath(path); err != nil {
+			return nil, fmt.Errorf("invalid document path at index %d: %w", i, err)
+		}
+	}
+
 	// Create a BulkWriter to handle multiple deletions efficiently
 	bulkWriter := t.Client.BulkWriter(ctx)
 

internal/tools/firestore/firestoregetdocuments/firestoregetdocuments.go

@@ -23,6 +23,7 @@ import (
 	"github.com/googleapis/genai-toolbox/internal/sources"
 	firestoreds "github.com/googleapis/genai-toolbox/internal/sources/firestore"
 	"github.com/googleapis/genai-toolbox/internal/tools"
+	"github.com/googleapis/genai-toolbox/internal/tools/firestore/util"
 )
 
 const kind string = "firestore-get-documents"
@@ -79,7 +80,7 @@ func (cfg Config) Initialize(srcs map[string]sources.Source) (tools.Tool, error)
 		return nil, fmt.Errorf("invalid source for %q tool: source kind must be one of %q", kind, compatibleSources)
 	}
 
-	documentPathsParameter := tools.NewArrayParameter(documentPathsKey, "Array of document paths to retrieve from Firestore.", tools.NewStringParameter("item", "Document path"))
+	documentPathsParameter := tools.NewArrayParameter(documentPathsKey, "Array of relative document paths to retrieve from Firestore (e.g., 'users/userId' or 'users/userId/posts/postId'). Note: These are relative paths, NOT absolute paths like 'projects/{project_id}/databases/{database_id}/documents/...'", tools.NewStringParameter("item", "Relative document path"))
 	parameters := tools.Parameters{documentPathsParameter}
 
 	mcpManifest := tools.McpManifest{
@@ -137,6 +138,13 @@ func (t Tool) Invoke(ctx context.Context, params tools.ParamValues, accessToken
 		return nil, fmt.Errorf("unexpected type conversion error for document paths")
 	}
 
+	// Validate each document path
+	for i, path := range documentPaths {
+		if err := util.ValidateDocumentPath(path); err != nil {
+			return nil, fmt.Errorf("invalid document path at index %d: %w", i, err)
+		}
+	}
+
 	// Create document references from paths
 	docRefs := make([]*firestoreapi.DocumentRef, len(documentPaths))
 	for i, path := range documentPaths {

internal/tools/firestore/firestorelistcollections/firestorelistcollections.go

@@ -23,6 +23,7 @@ import (
 	"github.com/googleapis/genai-toolbox/internal/sources"
 	firestoreds "github.com/googleapis/genai-toolbox/internal/sources/firestore"
 	"github.com/googleapis/genai-toolbox/internal/tools"
+	"github.com/googleapis/genai-toolbox/internal/tools/firestore/util"
 )
 
 const kind string = "firestore-list-collections"
@@ -80,7 +81,7 @@ func (cfg Config) Initialize(srcs map[string]sources.Source) (tools.Tool, error)
 	}
 
 	emptyString := ""
-	parentPathParameter := tools.NewStringParameterWithDefault(parentPathKey, emptyString, "Parent document path to list subcollections from. If not provided, lists root collections.")
+	parentPathParameter := tools.NewStringParameterWithDefault(parentPathKey, emptyString, "Relative parent document path to list subcollections from (e.g., 'users/userId'). If not provided, lists root collections. Note: This is a relative path, NOT an absolute path like 'projects/{project_id}/databases/{database_id}/documents/...'")
 	parameters := tools.Parameters{parentPathParameter}
 
 	mcpManifest := tools.McpManifest{
@@ -126,6 +127,11 @@ func (t Tool) Invoke(ctx context.Context, params tools.ParamValues, accessToken
 	parentPath, hasParent := mapParams[parentPathKey].(string)
 
 	if hasParent && parentPath != "" {
+		// Validate parent document path
+		if err := util.ValidateDocumentPath(parentPath); err != nil {
+			return nil, fmt.Errorf("invalid parent document path: %w", err)
+		}
+		
 		// List subcollections of the specified document
 		docRef := t.Client.Doc(parentPath)
 		collectionRefs, err = docRef.Collections(ctx).GetAll()

internal/tools/firestore/firestorequerycollection/firestorequerycollection.go

@@ -25,6 +25,7 @@ import (
 	"github.com/googleapis/genai-toolbox/internal/sources"
 	firestoreds "github.com/googleapis/genai-toolbox/internal/sources/firestore"
 	"github.com/googleapis/genai-toolbox/internal/tools"
+	"github.com/googleapis/genai-toolbox/internal/tools/firestore/util"
 )
 
 // Constants for tool configuration
@@ -152,7 +153,7 @@ func (cfg Config) Initialize(srcs map[string]sources.Source) (tools.Tool, error)
 func createParameters() tools.Parameters {
 	collectionPathParameter := tools.NewStringParameter(
 		collectionPathKey,
-		"The path to the Firestore collection to query",
+		"The relative path to the Firestore collection to query (e.g., 'users' or 'users/userId/posts'). Note: This is a relative path, NOT an absolute path like 'projects/{project_id}/databases/{database_id}/documents/...'",
 	)
 
 	filtersDescription := `Array of filter objects to apply to the query. Each filter is a JSON string with:
@@ -303,6 +304,11 @@ func (t Tool) parseQueryParameters(params tools.ParamValues) (*queryParameters,
 		return nil, fmt.Errorf(errMissingCollectionPath, collectionPathKey)
 	}
 
+	// Validate collection path
+	if err := util.ValidateCollectionPath(collectionPath); err != nil {
+		return nil, fmt.Errorf("invalid collection path: %w", err)
+	}
+
 	result := &queryParameters{
 		CollectionPath: collectionPath,
 		Limit:          defaultLimit,

internal/tools/firestore/firestoreupdatedocument/firestoreupdatedocument.go

@@ -87,7 +87,7 @@ func (cfg Config) Initialize(srcs map[string]sources.Source) (tools.Tool, error)
 	// Create parameters
 	documentPathParameter := tools.NewStringParameter(
 		documentPathKey,
-		"The path of the document which needs to be updated",
+		"The relative path of the document which needs to be updated (e.g., 'users/userId' or 'users/userId/posts/postId'). Note: This is a relative path, NOT an absolute path like 'projects/{project_id}/databases/{database_id}/documents/...'",
 	)
 
 	documentDataParameter := tools.NewMapParameter(
@@ -169,6 +169,11 @@ func (t Tool) Invoke(ctx context.Context, params tools.ParamValues, accessToken
 		return nil, fmt.Errorf("invalid or missing '%s' parameter", documentPathKey)
 	}
 
+	// Validate document path
+	if err := util.ValidateDocumentPath(documentPath); err != nil {
+		return nil, fmt.Errorf("invalid document path: %w", err)
+	}
+
 	// Get document data
 	documentDataRaw, ok := mapParams[documentDataKey]
 	if !ok {

internal/tools/firestore/util/validator.go

@@ -0,0 +1,137 @@
+// Copyright 2025 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package util
+
+import (
+	"fmt"
+	"regexp"
+	"strings"
+)
+
+// Regular expressions for validating Firestore paths
+var (
+	// Pattern to detect absolute paths (those starting with "projects/")
+	absolutePathRegex = regexp.MustCompile(`^projects/[^/]+/databases/[^/]+/documents/`)
+)
+
+// PathType represents the type of Firestore path
+type PathType int
+
+const (
+	CollectionPath PathType = iota
+	DocumentPath
+)
+
+// ValidateCollectionPath validates that a path is a valid Firestore collection path.
+// Collection paths must have an odd number of segments (collection/doc/collection)
+func ValidateCollectionPath(path string) error {
+	return validatePath(path, CollectionPath)
+}
+
+// ValidateDocumentPath validates that a path is a valid Firestore document path.
+// Document paths must have an even number of segments (collection/doc or collection/doc/collection/doc)
+func ValidateDocumentPath(path string) error {
+	return validatePath(path, DocumentPath)
+}
+
+// validatePath is the common validation function for both collection and document paths
+func validatePath(path string, pathType PathType) error {
+	pathTypeName := "document"
+	if pathType == CollectionPath {
+		pathTypeName = "collection"
+	}
+
+	// Check for empty path
+	if path == "" {
+		return fmt.Errorf("%s path cannot be empty", pathTypeName)
+	}
+
+	// Check if it's an absolute path
+	if absolutePathRegex.MatchString(path) {
+		example := "users/userId"
+		if pathType == CollectionPath {
+			example = "users"
+		}
+		return fmt.Errorf("path must be relative (e.g., '%s'), not absolute (matching pattern: ^projects/[^/]+/databases/[^/]+/documents/)", example)
+	}
+
+	// Split the path using strings.Split to preserve empty segments
+	segments := strings.Split(path, "/")
+
+	// Check for empty result
+	if len(segments) == 0 {
+		return fmt.Errorf("%s path cannot be empty or contain only slashes", pathTypeName)
+	}
+
+	// Check segment count based on path type
+	segmentCount := len(segments)
+	if pathType == CollectionPath && segmentCount%2 == 0 {
+		// Collection paths must have an odd number of segments
+		return fmt.Errorf("invalid collection path: must have an odd number of segments (e.g., 'collection' or 'collection/doc/subcollection'), got %d segments", segmentCount)
+	} else if pathType == DocumentPath && segmentCount%2 != 0 {
+		// Document paths must have an even number of segments
+		return fmt.Errorf("invalid document path: must have an even number of segments (e.g., 'collection/doc'), got %d segments", segmentCount)
+	}
+
+	// Validate each segment
+	for i, segment := range segments {
+		isCollectionSegment := (i % 2) == 0
+		if err := validateSegment(segment, isCollectionSegment); err != nil {
+			return fmt.Errorf("invalid segment at position %d (%s): %w", i+1, segment, err)
+		}
+	}
+
+	return nil
+}
+
+// validateSegment validates a single path segment
+func validateSegment(segment string, isCollection bool) error {
+	segmentType := "document ID"
+	if isCollection {
+		segmentType = "collection ID"
+	}
+
+	// Check for empty segment
+	if segment == "" {
+		return fmt.Errorf("segment cannot be empty")
+	}
+
+	// Check for whitespace-only segment
+	if strings.TrimSpace(segment) == "" {
+		return fmt.Errorf("segment cannot be only whitespace")
+	}
+
+	// Check for single or double period
+	if segment == "." || segment == ".." {
+		return fmt.Errorf("segment cannot be '.' or '..'")
+	}
+
+	// Check for reserved prefix
+	if strings.HasPrefix(segment, "__") {
+		return fmt.Errorf("%s cannot start with '__' (reserved prefix)", segmentType)
+	}
+
+	return nil
+}
+
+// IsAbsolutePath checks if a path is an absolute Firestore path
+func IsAbsolutePath(path string) bool {
+	return absolutePathRegex.MatchString(path)
+}
+
+// IsRelativePath checks if a path is a relative Firestore path
+func IsRelativePath(path string) bool {
+	return path != "" && !IsAbsolutePath(path)
+}

internal/tools/firestore/util/validator_test.go

@@ -0,0 +1,218 @@
+// Copyright 2025 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package util
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestValidateCollectionPath(t *testing.T) {
+	tests := []struct {
+		name    string
+		path    string
+		wantErr bool
+		errMsg  string
+	}{
+		// Valid cases
+		{
+			name:    "valid root collection",
+			path:    "users",
+			wantErr: false,
+		},
+		{
+			name:    "valid subcollection",
+			path:    "users/user123/posts",
+			wantErr: false,
+		},
+		{
+			name:    "valid deeply nested",
+			path:    "users/user123/posts/post456/comments",
+			wantErr: false,
+		},
+
+		// Invalid cases
+		{
+			name:    "empty path",
+			path:    "",
+			wantErr: true,
+			errMsg:  "collection path cannot be empty",
+		},
+		{
+			name:    "even segments (document path)",
+			path:    "users/user123",
+			wantErr: true,
+			errMsg:  "must have an odd number of segments",
+		},
+		{
+			name:    "absolute path",
+			path:    "projects/my-project/databases/(default)/documents/users",
+			wantErr: true,
+			errMsg:  "path must be relative",
+		},
+		{
+			name:    "reserved prefix __",
+			path:    "__users",
+			wantErr: true,
+			errMsg:  "collection ID cannot start with '__'",
+		},
+		{
+			name:    "dot segment",
+			path:    "users/./posts",
+			wantErr: true,
+			errMsg:  "segment cannot be '.'",
+		},
+		{
+			name:    "double slashes",
+			path:    "users//posts",
+			wantErr: true,
+			errMsg:  "segment cannot be empty",
+		},
+		{
+			name:    "trailing slash",
+			path:    "users/",
+			wantErr: true,
+			errMsg:  "must have an odd number of segments",
+		},
+		{
+			name:    "whitespace only segment",
+			path:    "users/   /posts",
+			wantErr: true,
+			errMsg:  "segment cannot be only whitespace",
+		},
+		{
+			name:    "tab whitespace segment",
+			path:    "users/\t/posts",
+			wantErr: true,
+			errMsg:  "segment cannot be only whitespace",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := ValidateCollectionPath(tt.path)
+			if tt.wantErr {
+				if err == nil {
+					t.Errorf("ValidateCollectionPath(%q) expected error but got none", tt.path)
+				} else if tt.errMsg != "" && !strings.Contains(err.Error(), tt.errMsg) {
+					t.Errorf("ValidateCollectionPath(%q) error = %v, want error containing %q", tt.path, err, tt.errMsg)
+				}
+			} else {
+				if err != nil {
+					t.Errorf("ValidateCollectionPath(%q) unexpected error: %v", tt.path, err)
+				}
+			}
+		})
+	}
+}
+
+func TestValidateDocumentPath(t *testing.T) {
+	tests := []struct {
+		name    string
+		path    string
+		wantErr bool
+		errMsg  string
+	}{
+		// Valid cases
+		{
+			name:    "valid root document",
+			path:    "users/user123",
+			wantErr: false,
+		},
+		{
+			name:    "valid nested document",
+			path:    "users/user123/posts/post456",
+			wantErr: false,
+		},
+		{
+			name:    "valid deeply nested",
+			path:    "users/user123/posts/post456/comments/comment789",
+			wantErr: false,
+		},
+
+		// Invalid cases
+		{
+			name:    "empty path",
+			path:    "",
+			wantErr: true,
+			errMsg:  "document path cannot be empty",
+		},
+		{
+			name:    "odd segments (collection path)",
+			path:    "users",
+			wantErr: true,
+			errMsg:  "must have an even number of segments",
+		},
+		{
+			name:    "absolute path",
+			path:    "projects/my-project/databases/(default)/documents/users/user123",
+			wantErr: true,
+			errMsg:  "path must be relative",
+		},
+		{
+			name:    "reserved prefix __",
+			path:    "users/__user123",
+			wantErr: true,
+			errMsg:  "document ID cannot start with '__'",
+		},
+		{
+			name:    "double dot segment",
+			path:    "users/..",
+			wantErr: true,
+			errMsg:  "segment cannot be '.'",
+		},
+		{
+			name:    "double slashes in document path",
+			path:    "users//user123",
+			wantErr: true,
+			errMsg:  "must have an even number of segments",
+		},
+		{
+			name:    "trailing slash document",
+			path:    "users/user123/",
+			wantErr: true,
+			errMsg:  "must have an even number of segments",
+		},
+		{
+			name:    "whitespace only document ID",
+			path:    "users/   ",
+			wantErr: true,
+			errMsg:  "segment cannot be only whitespace",
+		},
+		{
+			name:    "whitespace in middle segment",
+			path:    "users/user123/posts/ \t ",
+			wantErr: true,
+			errMsg:  "segment cannot be only whitespace",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := ValidateDocumentPath(tt.path)
+			if tt.wantErr {
+				if err == nil {
+					t.Errorf("ValidateDocumentPath(%q) expected error but got none", tt.path)
+				} else if tt.errMsg != "" && !strings.Contains(err.Error(), tt.errMsg) {
+					t.Errorf("ValidateDocumentPath(%q) error = %v, want error containing %q", tt.path, err, tt.errMsg)
+				}
+			} else {
+				if err != nil {
+					t.Errorf("ValidateDocumentPath(%q) unexpected error: %v", tt.path, err)
+				}
+			}
+		})
+	}
+}