From 21d1153b55c50359587d261959d5b547f9332c47 Mon Sep 17 00:00:00 2001 From: bobby-smedley Date: Mon, 20 Apr 2026 11:52:36 -0400 Subject: [PATCH] feat: consistent input validation with tool execution errors --- go.mod | 10 +-- go.sum | 26 +++++--- pkg/chip/server_test.go | 47 ++++++++++++++ pkg/tools/add_business_term/tool.go | 11 ++++ pkg/tools/add_business_term/tool_test.go | 16 ++--- .../add_data_classification_match/tool.go | 28 ++------ .../tool_test.go | 30 ++------- pkg/tools/create_asset/tool.go | 13 ++++ pkg/tools/create_asset/tool_test.go | 38 +++++------ pkg/tools/get_asset_details/tool.go | 7 +- pkg/tools/get_business_term_data/tool.go | 5 +- pkg/tools/get_column_semantics/tool.go | 5 +- pkg/tools/get_measure_data/tool.go | 5 +- pkg/tools/get_table_semantics/tool.go | 5 +- pkg/tools/pull_data_contract_manifest/tool.go | 10 ++- .../pull_data_contract_manifest/tool_test.go | 14 +--- .../remove_data_classification_match/tool.go | 18 +----- .../tool_test.go | 14 +--- pkg/tools/search_asset_keyword/tool.go | 14 ++++ pkg/tools/search_lineage_entities/tool.go | 5 ++ pkg/tools/validation/uuid.go | 39 +++++++++++ pkg/tools/validation/uuid_test.go | 64 +++++++++++++++++++ 22 files changed, 283 insertions(+), 141 deletions(-) create mode 100644 pkg/tools/validation/uuid.go create mode 100644 pkg/tools/validation/uuid_test.go diff --git a/go.mod b/go.mod index cc85499..75e64f0 100644 --- a/go.mod +++ b/go.mod @@ -4,8 +4,9 @@ go 1.25.0 require ( github.com/google/go-querystring v1.1.0 + github.com/google/jsonschema-go v0.4.2 github.com/google/uuid v1.6.0 - github.com/modelcontextprotocol/go-sdk v1.1.0 + github.com/modelcontextprotocol/go-sdk v1.5.0 github.com/spf13/pflag v1.0.10 github.com/spf13/viper v1.21.0 ) @@ -13,17 +14,18 @@ require ( require ( github.com/fsnotify/fsnotify v1.9.0 // indirect github.com/go-viper/mapstructure/v2 v2.4.0 // indirect - github.com/google/jsonschema-go v0.3.0 // indirect github.com/pelletier/go-toml/v2 v2.2.4 // indirect github.com/rogpeppe/go-internal v1.10.0 // indirect github.com/sagikazarmark/locafero v0.12.0 // indirect + github.com/segmentio/asm v1.1.3 // indirect + github.com/segmentio/encoding v0.5.4 // indirect github.com/spf13/afero v1.15.0 // indirect github.com/spf13/cast v1.10.0 // indirect github.com/subosito/gotenv v1.6.0 // indirect github.com/yosida95/uritemplate/v3 v3.0.2 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect - golang.org/x/oauth2 v0.34.0 // indirect - golang.org/x/sys v0.39.0 // indirect + golang.org/x/oauth2 v0.35.0 // indirect + golang.org/x/sys v0.41.0 // indirect golang.org/x/text v0.32.0 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect ) diff --git a/go.sum b/go.sum index 2e29b79..c47ee63 100644 --- a/go.sum +++ b/go.sum @@ -6,13 +6,15 @@ github.com/fsnotify/fsnotify v1.9.0 h1:2Ml+OJNzbYCTzsxtv8vKSFD9PbJjmhYF14k/jKC7S github.com/fsnotify/fsnotify v1.9.0/go.mod h1:8jBTzvmWwFyi3Pb8djgCCO5IBqzKJ/Jwo8TRcHyHii0= github.com/go-viper/mapstructure/v2 v2.4.0 h1:EBsztssimR/CONLSZZ04E8qAkxNYq4Qp9LvH92wZUgs= github.com/go-viper/mapstructure/v2 v2.4.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= +github.com/golang-jwt/jwt/v5 v5.3.1 h1:kYf81DTWFe7t+1VvL7eS+jKFVWaUnK9cB1qbwn63YCY= +github.com/golang-jwt/jwt/v5 v5.3.1/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE= github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8= github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU= -github.com/google/jsonschema-go v0.3.0 h1:6AH2TxVNtk3IlvkkhjrtbUc4S8AvO0Xii0DxIygDg+Q= -github.com/google/jsonschema-go v0.3.0/go.mod h1:r5quNTdLOYEz95Ru18zA0ydNbBuYoo9tgaYcxEYhJVE= +github.com/google/jsonschema-go v0.4.2 h1:tmrUohrwoLZZS/P3x7ex0WAVknEkBZM46iALbcqoRA8= +github.com/google/jsonschema-go v0.4.2/go.mod h1:r5quNTdLOYEz95Ru18zA0ydNbBuYoo9tgaYcxEYhJVE= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= @@ -22,8 +24,8 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= -github.com/modelcontextprotocol/go-sdk v1.1.0 h1:Qjayg53dnKC4UZ+792W21e4BpwEZBzwgRW6LrjLWSwA= -github.com/modelcontextprotocol/go-sdk v1.1.0/go.mod h1:6fM3LCm3yV7pAs8isnKLn07oKtB0MP9LHd3DfAcKw10= +github.com/modelcontextprotocol/go-sdk v1.5.0 h1:CHU0FIX9kpueNkxuYtfYQn1Z0slhFzBZuq+x6IiblIU= +github.com/modelcontextprotocol/go-sdk v1.5.0/go.mod h1:gggDIhoemhWs3BGkGwd1umzEXCEMMvAnhTrnbXJKKKA= github.com/pelletier/go-toml/v2 v2.2.4 h1:mye9XuhQ6gvn5h28+VilKrrPoQVanw5PMw/TB0t5Ec4= github.com/pelletier/go-toml/v2 v2.2.4/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= @@ -32,6 +34,10 @@ github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjR github.com/rogpeppe/go-internal v1.10.0/go.mod h1:UQnix2H7Ngw/k4C5ijL5+65zddjncjaFoBhdsK/akog= github.com/sagikazarmark/locafero v0.12.0 h1:/NQhBAkUb4+fH1jivKHWusDYFjMOOKU88eegjfxfHb4= github.com/sagikazarmark/locafero v0.12.0/go.mod h1:sZh36u/YSZ918v0Io+U9ogLYQJ9tLLBmM4eneO6WwsI= +github.com/segmentio/asm v1.1.3 h1:WM03sfUOENvvKexOLp+pCqgb/WDjsi7EK8gIsICtzhc= +github.com/segmentio/asm v1.1.3/go.mod h1:Ld3L4ZXGNcSLRg4JBsZ3//1+f/TjYl0Mzen/DQy1EJg= +github.com/segmentio/encoding v0.5.4 h1:OW1VRern8Nw6ITAtwSZ7Idrl3MXCFwXHPgqESYfvNt0= +github.com/segmentio/encoding v0.5.4/go.mod h1:HS1ZKa3kSN32ZHVZ7ZLPLXWvOVIiZtyJnO1gPH1sKt0= github.com/spf13/afero v1.15.0 h1:b/YBCLWAJdFWJTN9cLhiXXcD7mzKn9Dm86dNnfyQw1I= github.com/spf13/afero v1.15.0/go.mod h1:NC2ByUVxtQs4b3sIUphxK0NioZnmxgyCrfzeuq8lxMg= github.com/spf13/cast v1.10.0 h1:h2x0u2shc1QuLHfxi+cTJvs30+ZAHOGRic8uyGTDWxY= @@ -48,14 +54,14 @@ github.com/yosida95/uritemplate/v3 v3.0.2 h1:Ed3Oyj9yrmi9087+NczuL5BwkIc4wvTb5zI github.com/yosida95/uritemplate/v3 v3.0.2/go.mod h1:ILOh0sOhIJR3+L/8afwt/kE++YT040gmv5BQTMR2HP4= go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= -golang.org/x/oauth2 v0.34.0 h1:hqK/t4AKgbqWkdkcAeI8XLmbK+4m4G5YeQRrmiotGlw= -golang.org/x/oauth2 v0.34.0/go.mod h1:lzm5WQJQwKZ3nwavOZ3IS5Aulzxi68dUSgRHujetwEA= -golang.org/x/sys v0.39.0 h1:CvCKL8MeisomCi6qNZ+wbb0DN9E5AATixKsvNtMoMFk= -golang.org/x/sys v0.39.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= +golang.org/x/oauth2 v0.35.0 h1:Mv2mzuHuZuY2+bkyWXIHMfhNdJAdwW3FuWeCPYN5GVQ= +golang.org/x/oauth2 v0.35.0/go.mod h1:lzm5WQJQwKZ3nwavOZ3IS5Aulzxi68dUSgRHujetwEA= +golang.org/x/sys v0.41.0 h1:Ivj+2Cp/ylzLiEU89QhWblYnOE9zerudt9Ftecq2C6k= +golang.org/x/sys v0.41.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= golang.org/x/text v0.32.0 h1:ZD01bjUt1FQ9WJ0ClOL5vxgxOI/sVCNgX1YtKwcY0mU= golang.org/x/text v0.32.0/go.mod h1:o/rUWzghvpD5TXrTIBuJU77MTaN0ljMWE47kxGJQ7jY= -golang.org/x/tools v0.39.0 h1:ik4ho21kwuQln40uelmciQPp9SipgNDdrafrYA4TmQQ= -golang.org/x/tools v0.39.0/go.mod h1:JnefbkDPyD8UU2kI5fuf8ZX4/yUeh9W877ZeBONxUqQ= +golang.org/x/tools v0.42.0 h1:uNgphsn75Tdz5Ji2q36v/nsFSfR/9BRFvqhGBaJGd5k= +golang.org/x/tools v0.42.0/go.mod h1:Ma6lCIwGZvHK6XtgbswSoWroEkhugApmsXyrUmBhfr0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= diff --git a/pkg/chip/server_test.go b/pkg/chip/server_test.go index 655a390..ee722e8 100644 --- a/pkg/chip/server_test.go +++ b/pkg/chip/server_test.go @@ -3,6 +3,7 @@ package chip import ( "context" "log" + "strings" "testing" "github.com/modelcontextprotocol/go-sdk/mcp" @@ -30,6 +31,52 @@ func handleTool() ToolHandlerFunc[toolInput, toolOutput] { } } +func TestTool_SchemaValidationReturnsToolExecutionError(t *testing.T) { + chipServer := NewServer() + RegisterTool[toolInput, toolOutput](chipServer, newTool()) + chipSession := newChipSession(t.Context(), chipServer) + defer closeSilently(chipSession) + + res, err := chipSession.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "the_tool", + Arguments: map[string]any{}, + }) + if err != nil { + t.Fatalf("expected tool execution error, got protocol error: %v", err) + } + if !res.IsError { + t.Fatal("expected isError: true for missing required field") + } + if len(res.Content) == 0 { + t.Fatal("expected content describing the validation failure") + } + text, ok := res.Content[0].(*mcp.TextContent) + if !ok { + t.Fatalf("expected TextContent, got %T", res.Content[0]) + } + if !strings.Contains(text.Text, "input") { + t.Fatalf("expected error mentioning missing field 'input', got %q", text.Text) + } +} + +func TestTool_WrongTypeReturnsToolExecutionError(t *testing.T) { + chipServer := NewServer() + RegisterTool[toolInput, toolOutput](chipServer, newTool()) + chipSession := newChipSession(t.Context(), chipServer) + defer closeSilently(chipSession) + + res, err := chipSession.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "the_tool", + Arguments: map[string]any{"input": 123}, + }) + if err != nil { + t.Fatalf("expected tool execution error, got protocol error: %v", err) + } + if !res.IsError { + t.Fatal("expected isError: true for wrong-typed field") + } +} + func TestTool_IgnoreUnknownFields(t *testing.T) { chipServer := NewServer() RegisterTool[toolInput, toolOutput](chipServer, newTool()) diff --git a/pkg/tools/add_business_term/tool.go b/pkg/tools/add_business_term/tool.go index e0a3366..4ad31b2 100644 --- a/pkg/tools/add_business_term/tool.go +++ b/pkg/tools/add_business_term/tool.go @@ -2,10 +2,12 @@ package add_business_term import ( "context" + "fmt" "net/http" "github.com/collibra/chip/pkg/chip" "github.com/collibra/chip/pkg/clients" + "github.com/collibra/chip/pkg/tools/validation" "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -48,6 +50,15 @@ func NewTool(collibraClient *http.Client) *chip.Tool[Input, Output] { func handler(collibraClient *http.Client) chip.ToolHandlerFunc[Input, Output] { return func(ctx context.Context, input Input) (Output, error) { + if err := validation.UUID("domainId", input.DomainId); err != nil { + return Output{}, err + } + for i, attr := range input.Attributes { + if err := validation.UUID(fmt.Sprintf("attributes[%d].typeId", i), attr.TypeId); err != nil { + return Output{}, err + } + } + // Step 1: Create the business term asset assetResp, err := clients.CreateBusinessTermAsset(ctx, collibraClient, clients.AddBusinessTermAssetRequest{ Name: input.Name, diff --git a/pkg/tools/add_business_term/tool_test.go b/pkg/tools/add_business_term/tool_test.go index 90a6339..cce21c0 100644 --- a/pkg/tools/add_business_term/tool_test.go +++ b/pkg/tools/add_business_term/tool_test.go @@ -20,8 +20,8 @@ func TestAddBusinessTermSuccess(t *testing.T) { if req.TypePublicId != "BusinessTerm" { t.Errorf("expected typePublicId 'BusinessTerm', got '%s'", req.TypePublicId) } - if req.DomainId != "domain-uuid-123" { - t.Errorf("expected domainId 'domain-uuid-123', got '%s'", req.DomainId) + if req.DomainId != "00000000-0000-0000-0000-000000000123" { + t.Errorf("expected domainId '00000000-0000-0000-0000-000000000123', got '%s'", req.DomainId) } return http.StatusCreated, clients.AddBusinessTermAssetResponse{Id: "new-asset-uuid-456"} })) @@ -45,7 +45,7 @@ func TestAddBusinessTermSuccess(t *testing.T) { client := testutil.NewClient(server) output, err := add_business_term.NewTool(client).Handler(t.Context(), add_business_term.Input{ Name: "Revenue", - DomainId: "domain-uuid-123", + DomainId: "00000000-0000-0000-0000-000000000123", Definition: "Total income generated from sales", }) if err != nil { @@ -97,7 +97,7 @@ func TestAddBusinessTermNoDefinitionNoAttributes(t *testing.T) { client := testutil.NewClient(server) output, err := add_business_term.NewTool(client).Handler(t.Context(), add_business_term.Input{ Name: "Simple Term", - DomainId: "domain-uuid-456", + DomainId: "00000000-0000-0000-0000-000000000456", }) if err != nil { t.Fatalf("expected no error, got: %v", err) @@ -132,11 +132,11 @@ func TestAddBusinessTermWithAdditionalAttributes(t *testing.T) { client := testutil.NewClient(server) output, err := add_business_term.NewTool(client).Handler(t.Context(), add_business_term.Input{ Name: "Complex Term", - DomainId: "domain-uuid-789", + DomainId: "00000000-0000-0000-0000-000000000789", Definition: "A complex business term", Attributes: []add_business_term.InputAttribute{ - {TypeId: "custom-type-1", Value: "custom value 1"}, - {TypeId: "custom-type-2", Value: "custom value 2"}, + {TypeId: "00000000-0000-0000-0000-0000000000c1", Value: "custom value 1"}, + {TypeId: "00000000-0000-0000-0000-0000000000c2", Value: "custom value 2"}, }, }) if err != nil { @@ -169,7 +169,7 @@ func TestAddBusinessTermAttributeCreationError(t *testing.T) { client := testutil.NewClient(server) _, err := add_business_term.NewTool(client).Handler(t.Context(), add_business_term.Input{ Name: "Failing Term", - DomainId: "domain-uuid-123", + DomainId: "00000000-0000-0000-0000-000000000123", Definition: "This should fail on attribute creation", }) if err == nil { diff --git a/pkg/tools/add_data_classification_match/tool.go b/pkg/tools/add_data_classification_match/tool.go index 49fe87b..08bab7d 100644 --- a/pkg/tools/add_data_classification_match/tool.go +++ b/pkg/tools/add_data_classification_match/tool.go @@ -4,10 +4,10 @@ import ( "context" "fmt" "net/http" - "strings" "github.com/collibra/chip/pkg/chip" "github.com/collibra/chip/pkg/clients" + "github.com/collibra/chip/pkg/tools/validation" "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -34,9 +34,11 @@ func NewTool(collibraClient *http.Client) *chip.Tool[Input, Output] { func handler(collibraClient *http.Client) chip.ToolHandlerFunc[Input, Output] { return func(ctx context.Context, input Input) (Output, error) { - output, isNotValid := validateInput(input) - if isNotValid { - return output, nil + if err := validation.UUID("assetId", input.AssetID); err != nil { + return Output{}, err + } + if err := validation.UUID("classificationId", input.ClassificationID); err != nil { + return Output{}, err } request := clients.AddDataClassificationMatchRequest{ @@ -58,21 +60,3 @@ func handler(collibraClient *http.Client) chip.ToolHandlerFunc[Input, Output] { }, nil } } - -func validateInput(input Input) (Output, bool) { - if strings.TrimSpace(input.AssetID) == "" { - return Output{ - Success: false, - Error: "Asset ID is required", - }, true - } - - if strings.TrimSpace(input.ClassificationID) == "" { - return Output{ - Success: false, - Error: "Classification ID is required", - }, true - } - - return Output{}, false -} diff --git a/pkg/tools/add_data_classification_match/tool_test.go b/pkg/tools/add_data_classification_match/tool_test.go index 67ae6aa..402b720 100644 --- a/pkg/tools/add_data_classification_match/tool_test.go +++ b/pkg/tools/add_data_classification_match/tool_test.go @@ -81,18 +81,9 @@ func TestAddClassificationMatch_MissingAssetID(t *testing.T) { ClassificationID: "be45c001-b173-48ff-ac91-3f6e45868c8b", } - output, err := tools.NewTool(client).Handler(t.Context(), input) - - if err != nil { - t.Fatalf("Expected no error, got %v", err) - } - - if output.Success { - t.Error("Expected success=false for missing asset ID") - } - - if output.Error == "" { - t.Error("Expected error message for missing asset ID") + _, err := tools.NewTool(client).Handler(t.Context(), input) + if err == nil { + t.Fatal("Expected UUID validation error, got nil") } } @@ -103,18 +94,9 @@ func TestAddClassificationMatch_MissingClassificationID(t *testing.T) { AssetID: "9179b887-04ef-4ce5-ab3a-b5bbd39ea3c8", } - output, err := tools.NewTool(client).Handler(t.Context(), input) - - if err != nil { - t.Fatalf("Expected no error, got %v", err) - } - - if output.Success { - t.Error("Expected success=false for missing classification ID") - } - - if output.Error == "" { - t.Error("Expected error message for missing classification ID") + _, err := tools.NewTool(client).Handler(t.Context(), input) + if err == nil { + t.Fatal("Expected UUID validation error, got nil") } } diff --git a/pkg/tools/create_asset/tool.go b/pkg/tools/create_asset/tool.go index 879b6f0..1d743b1 100644 --- a/pkg/tools/create_asset/tool.go +++ b/pkg/tools/create_asset/tool.go @@ -7,6 +7,7 @@ import ( "github.com/collibra/chip/pkg/chip" "github.com/collibra/chip/pkg/clients" + "github.com/collibra/chip/pkg/tools/validation" "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -37,6 +38,18 @@ func NewTool(collibraClient *http.Client) *chip.Tool[Input, Output] { func handler(collibraClient *http.Client) chip.ToolHandlerFunc[Input, Output] { return func(ctx context.Context, input Input) (Output, error) { + if err := validation.UUID("assetTypeId", input.AssetTypeID); err != nil { + return Output{}, err + } + if err := validation.UUID("domainId", input.DomainID); err != nil { + return Output{}, err + } + for typeID := range input.Attributes { + if err := validation.UUID(fmt.Sprintf("attributes[%q]", typeID), typeID); err != nil { + return Output{}, err + } + } + // Create the asset assetReq := clients.CreateAssetRequest{ Name: input.Name, diff --git a/pkg/tools/create_asset/tool_test.go b/pkg/tools/create_asset/tool_test.go index efa00f5..713069a 100644 --- a/pkg/tools/create_asset/tool_test.go +++ b/pkg/tools/create_asset/tool_test.go @@ -35,8 +35,8 @@ func TestCreateAssetSuccess(t *testing.T) { client := testutil.NewClient(server) output, err := create_asset.NewTool(client).Handler(t.Context(), create_asset.Input{ Name: "My New Asset", - AssetTypeID: "type-uuid-456", - DomainID: "domain-uuid-789", + AssetTypeID: "00000000-0000-0000-0000-000000000456", + DomainID: "00000000-0000-0000-0000-000000000789", }) if err != nil { t.Fatalf("Expected no error, got: %v", err) @@ -87,11 +87,11 @@ func TestCreateAssetWithAttributes(t *testing.T) { client := testutil.NewClient(server) output, err := create_asset.NewTool(client).Handler(t.Context(), create_asset.Input{ Name: "Asset With Attrs", - AssetTypeID: "type-uuid-456", - DomainID: "domain-uuid-789", + AssetTypeID: "00000000-0000-0000-0000-000000000456", + DomainID: "00000000-0000-0000-0000-000000000789", Attributes: map[string]string{ - "attr-type-1": "Description value", - "attr-type-2": "Definition value", + "00000000-0000-0000-0000-0000000000a1": "Description value", + "00000000-0000-0000-0000-0000000000a2": "Definition value", }, }) if err != nil { @@ -134,8 +134,8 @@ func TestCreateAssetWithDisplayName(t *testing.T) { client := testutil.NewClient(server) _, err := create_asset.NewTool(client).Handler(t.Context(), create_asset.Input{ Name: "My Asset", - AssetTypeID: "type-uuid-456", - DomainID: "domain-uuid-789", + AssetTypeID: "00000000-0000-0000-0000-000000000456", + DomainID: "00000000-0000-0000-0000-000000000789", DisplayName: "My Display Name", }) if err != nil { @@ -161,8 +161,8 @@ func TestCreateAssetBadRequest(t *testing.T) { client := testutil.NewClient(server) _, err := create_asset.NewTool(client).Handler(t.Context(), create_asset.Input{ Name: "Duplicate Asset", - AssetTypeID: "type-uuid-456", - DomainID: "domain-uuid-789", + AssetTypeID: "00000000-0000-0000-0000-000000000456", + DomainID: "00000000-0000-0000-0000-000000000789", }) if err == nil { t.Fatal("Expected error for bad request, got nil") @@ -188,8 +188,8 @@ func TestCreateAssetNotFound(t *testing.T) { client := testutil.NewClient(server) _, err := create_asset.NewTool(client).Handler(t.Context(), create_asset.Input{ Name: "Test Asset", - AssetTypeID: "invalid-type-id", - DomainID: "domain-uuid-789", + AssetTypeID: "00000000-0000-0000-0000-000000000999", + DomainID: "00000000-0000-0000-0000-000000000789", }) if err == nil { t.Fatal("Expected error for not found, got nil") @@ -215,8 +215,8 @@ func TestCreateAssetForbidden(t *testing.T) { client := testutil.NewClient(server) _, err := create_asset.NewTool(client).Handler(t.Context(), create_asset.Input{ Name: "Test Asset", - AssetTypeID: "type-uuid-456", - DomainID: "domain-uuid-789", + AssetTypeID: "00000000-0000-0000-0000-000000000456", + DomainID: "00000000-0000-0000-0000-000000000789", }) if err == nil { t.Fatal("Expected error for forbidden, got nil") @@ -251,8 +251,8 @@ func TestCreateAssetEmptyAttributes(t *testing.T) { client := testutil.NewClient(server) output, err := create_asset.NewTool(client).Handler(t.Context(), create_asset.Input{ Name: "Asset No Attrs", - AssetTypeID: "type-uuid-456", - DomainID: "domain-uuid-789", + AssetTypeID: "00000000-0000-0000-0000-000000000456", + DomainID: "00000000-0000-0000-0000-000000000789", Attributes: map[string]string{}, }) if err != nil { @@ -292,10 +292,10 @@ func TestCreateAssetAttributeFailure(t *testing.T) { client := testutil.NewClient(server) _, err := create_asset.NewTool(client).Handler(t.Context(), create_asset.Input{ Name: "Asset With Bad Attr", - AssetTypeID: "type-uuid-456", - DomainID: "domain-uuid-789", + AssetTypeID: "00000000-0000-0000-0000-000000000456", + DomainID: "00000000-0000-0000-0000-000000000789", Attributes: map[string]string{ - "bad-attr-type": "some value", + "00000000-0000-0000-0000-0000000000bb": "some value", }, }) if err == nil { diff --git a/pkg/tools/get_asset_details/tool.go b/pkg/tools/get_asset_details/tool.go index c4567a5..e1abd0a 100644 --- a/pkg/tools/get_asset_details/tool.go +++ b/pkg/tools/get_asset_details/tool.go @@ -10,6 +10,7 @@ import ( "github.com/collibra/chip/pkg/chip" "github.com/collibra/chip/pkg/clients" + "github.com/collibra/chip/pkg/tools/validation" "github.com/google/uuid" "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -49,10 +50,10 @@ func NewTool(collibraClient *http.Client) *chip.Tool[Input, Output] { func handler(collibraClient *http.Client) chip.ToolHandlerFunc[Input, Output] { return func(ctx context.Context, input Input) (Output, error) { - assetUUID, err := uuid.Parse(input.AssetID) - if err != nil { - return Output{Error: fmt.Sprintf("Invalid asset ID format: %s", err.Error()), Found: false}, nil + if err := validation.UUID("assetId", input.AssetID); err != nil { + return Output{}, err } + assetUUID, _ := uuid.Parse(input.AssetID) assets, err := clients.GetAssetSummary(ctx, collibraClient, assetUUID, input.OutgoingRelationsCursor, input.IncomingRelationsCursor) if err != nil { diff --git a/pkg/tools/get_business_term_data/tool.go b/pkg/tools/get_business_term_data/tool.go index 16d3f83..44c320e 100644 --- a/pkg/tools/get_business_term_data/tool.go +++ b/pkg/tools/get_business_term_data/tool.go @@ -6,6 +6,7 @@ import ( "github.com/collibra/chip/pkg/chip" "github.com/collibra/chip/pkg/clients" + "github.com/collibra/chip/pkg/tools/validation" "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -54,8 +55,8 @@ func NewTool(collibraClient *http.Client) *chip.Tool[Input, Output] { func handler(collibraClient *http.Client) chip.ToolHandlerFunc[Input, Output] { return func(ctx context.Context, input Input) (Output, error) { - if input.BusinessTermID == "" { - return Output{Error: "businessTermId is required"}, nil + if err := validation.UUID("businessTermId", input.BusinessTermID); err != nil { + return Output{}, err } dataAttributes, err := clients.FindConnectedAssets(ctx, collibraClient, input.BusinessTermID, clients.BusinessAssetRepresentsDataAssetRelID) diff --git a/pkg/tools/get_column_semantics/tool.go b/pkg/tools/get_column_semantics/tool.go index 330d09c..08bfc6b 100644 --- a/pkg/tools/get_column_semantics/tool.go +++ b/pkg/tools/get_column_semantics/tool.go @@ -6,6 +6,7 @@ import ( "github.com/collibra/chip/pkg/chip" "github.com/collibra/chip/pkg/clients" + "github.com/collibra/chip/pkg/tools/validation" "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -47,8 +48,8 @@ func NewTool(collibraClient *http.Client) *chip.Tool[Input, Output] { func handler(collibraClient *http.Client) chip.ToolHandlerFunc[Input, Output] { return func(ctx context.Context, input Input) (Output, error) { - if input.ColumnID == "" { - return Output{Error: "columnId is required"}, nil + if err := validation.UUID("columnId", input.ColumnID); err != nil { + return Output{}, err } dataAttributes, err := clients.FindColumnsForDataAttribute(ctx, collibraClient, input.ColumnID) diff --git a/pkg/tools/get_measure_data/tool.go b/pkg/tools/get_measure_data/tool.go index eddd5c1..f4a194f 100644 --- a/pkg/tools/get_measure_data/tool.go +++ b/pkg/tools/get_measure_data/tool.go @@ -6,6 +6,7 @@ import ( "github.com/collibra/chip/pkg/chip" "github.com/collibra/chip/pkg/clients" + "github.com/collibra/chip/pkg/tools/validation" "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -53,8 +54,8 @@ func NewTool(collibraClient *http.Client) *chip.Tool[Input, Output] { func handler(collibraClient *http.Client) chip.ToolHandlerFunc[Input, Output] { return func(ctx context.Context, input Input) (Output, error) { - if input.MeasureID == "" { - return Output{Error: "measureId is required"}, nil + if err := validation.UUID("measureId", input.MeasureID); err != nil { + return Output{}, err } dataAttributes, err := clients.FindConnectedAssets(ctx, collibraClient, input.MeasureID, clients.MeasureIsCalculatedUsingDataElementRelID) diff --git a/pkg/tools/get_table_semantics/tool.go b/pkg/tools/get_table_semantics/tool.go index d888e9e..5122c7b 100644 --- a/pkg/tools/get_table_semantics/tool.go +++ b/pkg/tools/get_table_semantics/tool.go @@ -6,6 +6,7 @@ import ( "github.com/collibra/chip/pkg/chip" "github.com/collibra/chip/pkg/clients" + "github.com/collibra/chip/pkg/tools/validation" "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -54,8 +55,8 @@ func NewTool(collibraClient *http.Client) *chip.Tool[Input, Output] { func handler(collibraClient *http.Client) chip.ToolHandlerFunc[Input, Output] { return func(ctx context.Context, input Input) (Output, error) { - if input.TableID == "" { - return Output{Error: "tableId is required"}, nil + if err := validation.UUID("tableId", input.TableID); err != nil { + return Output{}, err } rawColumns, err := clients.FindConnectedAssets(ctx, collibraClient, input.TableID, clients.ColumnIsPartOfTableRelID) diff --git a/pkg/tools/pull_data_contract_manifest/tool.go b/pkg/tools/pull_data_contract_manifest/tool.go index d0de293..6b987c7 100644 --- a/pkg/tools/pull_data_contract_manifest/tool.go +++ b/pkg/tools/pull_data_contract_manifest/tool.go @@ -7,6 +7,7 @@ import ( "github.com/collibra/chip/pkg/chip" "github.com/collibra/chip/pkg/clients" + "github.com/collibra/chip/pkg/tools/validation" "github.com/google/uuid" "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -33,13 +34,10 @@ func NewTool(collibraClient *http.Client) *chip.Tool[Input, Output] { func handler(collibraClient *http.Client) chip.ToolHandlerFunc[Input, Output] { return func(ctx context.Context, input Input) (Output, error) { - dataContractUUID, err := uuid.Parse(input.DataContractID) - if err != nil { - return Output{ - Error: fmt.Sprintf("Invalid data contract ID format: %s", err.Error()), - Found: false, - }, nil + if err := validation.UUID("dataContractId", input.DataContractID); err != nil { + return Output{}, err } + dataContractUUID, _ := uuid.Parse(input.DataContractID) manifest, err := clients.PullActiveDataContractManifest(ctx, collibraClient, dataContractUUID.String()) if err != nil { diff --git a/pkg/tools/pull_data_contract_manifest/tool_test.go b/pkg/tools/pull_data_contract_manifest/tool_test.go index 15e4b96..e589c1f 100644 --- a/pkg/tools/pull_data_contract_manifest/tool_test.go +++ b/pkg/tools/pull_data_contract_manifest/tool_test.go @@ -54,19 +54,11 @@ func TestPullDataContractManifestInvalidUUID(t *testing.T) { defer server.Close() client := testutil.NewClient(server) - output, err := tools.NewTool(client).Handler(t.Context(), tools.Input{ + _, err := tools.NewTool(client).Handler(t.Context(), tools.Input{ DataContractID: "invalid-uuid", }) - if err != nil { - t.Fatalf("Expected no error, got: %v", err) - } - - if output.Found { - t.Fatal("Expected manifest not to be found") - } - - if output.Error == "" { - t.Fatal("Expected error message for invalid UUID") + if err == nil { + t.Fatal("Expected UUID validation error, got nil") } } diff --git a/pkg/tools/remove_data_classification_match/tool.go b/pkg/tools/remove_data_classification_match/tool.go index 5982ee3..44d4cbf 100644 --- a/pkg/tools/remove_data_classification_match/tool.go +++ b/pkg/tools/remove_data_classification_match/tool.go @@ -4,10 +4,10 @@ import ( "context" "fmt" "net/http" - "strings" "github.com/collibra/chip/pkg/chip" "github.com/collibra/chip/pkg/clients" + "github.com/collibra/chip/pkg/tools/validation" "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -32,9 +32,8 @@ func NewTool(collibraClient *http.Client) *chip.Tool[Input, Output] { func handler(collibraClient *http.Client) chip.ToolHandlerFunc[Input, Output] { return func(ctx context.Context, input Input) (Output, error) { - output, isNotValid := validateInput(input) - if isNotValid { - return output, nil + if err := validation.UUID("classificationMatchId", input.ClassificationMatchID); err != nil { + return Output{}, err } err := clients.RemoveDataClassificationMatch(ctx, collibraClient, input.ClassificationMatchID) @@ -50,14 +49,3 @@ func handler(collibraClient *http.Client) chip.ToolHandlerFunc[Input, Output] { }, nil } } - -func validateInput(input Input) (Output, bool) { - if strings.TrimSpace(input.ClassificationMatchID) == "" { - return Output{ - Success: false, - Error: "Classification Match ID is required", - }, true - } - - return Output{}, false -} diff --git a/pkg/tools/remove_data_classification_match/tool_test.go b/pkg/tools/remove_data_classification_match/tool_test.go index 1411d1a..d7c3601 100644 --- a/pkg/tools/remove_data_classification_match/tool_test.go +++ b/pkg/tools/remove_data_classification_match/tool_test.go @@ -43,18 +43,10 @@ func TestRemoveClassificationMatch_MissingClassificationMatchID(t *testing.T) { input := tools.Input{} - output, err := tools.NewTool(client).Handler(t.Context(), input) - - if err != nil { - t.Fatalf("Expected no error, got %v", err) - } + _, err := tools.NewTool(client).Handler(t.Context(), input) - if output.Success { - t.Error("Expected success=false for missing classification match ID") - } - - if output.Error == "" { - t.Error("Expected error message for missing classification match ID") + if err == nil { + t.Fatal("Expected UUID validation error, got nil") } } diff --git a/pkg/tools/search_asset_keyword/tool.go b/pkg/tools/search_asset_keyword/tool.go index 802dfaa..9964c22 100644 --- a/pkg/tools/search_asset_keyword/tool.go +++ b/pkg/tools/search_asset_keyword/tool.go @@ -7,6 +7,7 @@ import ( "github.com/collibra/chip/pkg/chip" "github.com/collibra/chip/pkg/clients" + "github.com/collibra/chip/pkg/tools/validation" "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -49,6 +50,19 @@ func NewTool(collibraClient *http.Client) *chip.Tool[Input, Output] { func handler(collibraClient *http.Client) chip.ToolHandlerFunc[Input, Output] { return func(ctx context.Context, input Input) (Output, error) { + for field, vals := range map[string][]string{ + "communityFilter": input.CommunityFilter, + "domainFilter": input.DomainFilter, + "domainTypeFilter": input.DomainTypeFilter, + "assetTypeFilter": input.AssetTypeFilter, + "statusFilter": input.StatusFilter, + "createdByFilter": input.CreatedByFilter, + } { + if err := validation.UUIDs(field, vals); err != nil { + return Output{}, err + } + } + if input.Limit == 0 { input.Limit = 50 } diff --git a/pkg/tools/search_lineage_entities/tool.go b/pkg/tools/search_lineage_entities/tool.go index 78beb4c..ab48749 100644 --- a/pkg/tools/search_lineage_entities/tool.go +++ b/pkg/tools/search_lineage_entities/tool.go @@ -6,6 +6,7 @@ import ( "github.com/collibra/chip/pkg/chip" "github.com/collibra/chip/pkg/clients" + "github.com/collibra/chip/pkg/tools/validation" "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -29,6 +30,10 @@ func NewTool(collibraClient *http.Client) *chip.Tool[Input, clients.SearchLineag func handler(collibraClient *http.Client) chip.ToolHandlerFunc[Input, clients.SearchLineageEntitiesOutput] { return func(ctx context.Context, input Input) (clients.SearchLineageEntitiesOutput, error) { + if err := validation.UUIDOptional("dgcId", input.DgcId); err != nil { + return clients.SearchLineageEntitiesOutput{}, err + } + result, err := clients.SearchLineageEntities(ctx, collibraClient, input.NameContains, input.Type, input.DgcId, input.Limit, input.Cursor) if err != nil { return clients.SearchLineageEntitiesOutput{}, err diff --git a/pkg/tools/validation/uuid.go b/pkg/tools/validation/uuid.go new file mode 100644 index 0000000..c1aa9f0 --- /dev/null +++ b/pkg/tools/validation/uuid.go @@ -0,0 +1,39 @@ +// Package validation provides shared input validation helpers for CHIP tools. +// Helpers return Go errors that the MCP SDK wraps as tool execution errors +// (isError: true), letting the calling model see the problem and self-correct. +package validation + +import ( + "fmt" + + "github.com/google/uuid" +) + +// UUID verifies value is a well-formed UUID. fieldName is surfaced in the +// returned error so the calling model can identify which field failed. +func UUID(fieldName, value string) error { + if _, err := uuid.Parse(value); err != nil { + return fmt.Errorf("invalid UUID for %q: %s", fieldName, err.Error()) + } + return nil +} + +// UUIDOptional verifies value is a well-formed UUID when non-empty. Empty +// strings pass. Use for optional UUID fields. +func UUIDOptional(fieldName, value string) error { + if value == "" { + return nil + } + return UUID(fieldName, value) +} + +// UUIDs verifies every entry in values is a well-formed UUID. An empty slice +// passes. The error identifies the offending element's index. +func UUIDs(fieldName string, values []string) error { + for i, v := range values { + if _, err := uuid.Parse(v); err != nil { + return fmt.Errorf("invalid UUID for %q[%d]: %s", fieldName, i, err.Error()) + } + } + return nil +} diff --git a/pkg/tools/validation/uuid_test.go b/pkg/tools/validation/uuid_test.go new file mode 100644 index 0000000..c8ad219 --- /dev/null +++ b/pkg/tools/validation/uuid_test.go @@ -0,0 +1,64 @@ +package validation + +import ( + "strings" + "testing" +) + +const validUUID = "9179b887-04ef-4ce5-ab3a-b5bbd39ea3c8" + +func TestUUID_Valid(t *testing.T) { + if err := UUID("assetId", validUUID); err != nil { + t.Fatalf("expected valid UUID to pass, got %v", err) + } +} + +func TestUUID_Malformed(t *testing.T) { + err := UUID("assetId", "not-a-uuid") + if err == nil { + t.Fatal("expected error for malformed UUID") + } + if !strings.Contains(err.Error(), "assetId") { + t.Fatalf("expected error to mention field name, got %v", err) + } +} + +func TestUUID_Empty(t *testing.T) { + if err := UUID("assetId", ""); err == nil { + t.Fatal("expected error for empty UUID on required field") + } +} + +func TestUUIDOptional_Empty(t *testing.T) { + if err := UUIDOptional("dgcId", ""); err != nil { + t.Fatalf("expected empty string to pass for optional UUID, got %v", err) + } +} + +func TestUUIDOptional_Invalid(t *testing.T) { + if err := UUIDOptional("dgcId", "nope"); err == nil { + t.Fatal("expected error for malformed optional UUID") + } +} + +func TestUUIDs_AllValid(t *testing.T) { + if err := UUIDs("assetIds", []string{validUUID, validUUID}); err != nil { + t.Fatalf("expected all-valid slice to pass, got %v", err) + } +} + +func TestUUIDs_Empty(t *testing.T) { + if err := UUIDs("assetIds", nil); err != nil { + t.Fatalf("expected empty slice to pass, got %v", err) + } +} + +func TestUUIDs_OneInvalid(t *testing.T) { + err := UUIDs("assetIds", []string{validUUID, "nope"}) + if err == nil { + t.Fatal("expected error when a slice entry is malformed") + } + if !strings.Contains(err.Error(), "assetIds") || !strings.Contains(err.Error(), "[1]") { + t.Fatalf("expected error identifying field and index, got %v", err) + } +}