From b82c85cf27e221089ee1c94ebd5b0fef2f8bdb70 Mon Sep 17 00:00:00 2001 From: Yufeng He <40085740+he-yufeng@users.noreply.github.com> Date: Fri, 26 Jun 2026 07:53:14 +0800 Subject: [PATCH] fix(looker): validate explore_references shape instead of panicking The looker-conversational-analytics tool read explore_references with chained, unchecked type assertions: LookmlModel: er.(map[string]any)["model"].(string), Explore: er.(map[string]any)["explore"].(string), explore_references is declared as an array of free-form maps (NewMapParameter with an empty value type), so the entries the model supplies are never schema-validated. A reference that is not an object, omits "model"/"explore", or carries a non-string value, panics the tool on the type assertion rather than returning a usable error. Models drop or mistype those fields often enough that this is reachable in normal operation. Move the parsing into a small helper that checks each element and field and returns a clear agent error, matching the validation already done in datalineagesearchlineage. Add unit tests covering the valid, empty, and malformed cases. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> --- .../lookerconversationalanalytics.go | 39 +++++++++-- ...erconversationalanalytics_internal_test.go | 69 +++++++++++++++++++ 2 files changed, 101 insertions(+), 7 deletions(-) create mode 100644 internal/tools/looker/lookerconversationalanalytics/lookerconversationalanalytics_internal_test.go diff --git a/internal/tools/looker/lookerconversationalanalytics/lookerconversationalanalytics.go b/internal/tools/looker/lookerconversationalanalytics/lookerconversationalanalytics.go index 7ab3964dae6..bce8ca214e9 100644 --- a/internal/tools/looker/lookerconversationalanalytics/lookerconversationalanalytics.go +++ b/internal/tools/looker/lookerconversationalanalytics/lookerconversationalanalytics.go @@ -179,6 +179,35 @@ func (t Tool) ToConfig() tools.ToolConfig { return t.Cfg } +// parseExploreReferences converts the raw explore_references parameter into typed +// references. The parameter is declared as an array of free-form maps, so the values +// the model supplies are not schema-validated; guard every field access instead of +// letting an unexpected shape (a missing key, a non-string value, or a non-object +// element) panic the tool. Mirrors the validation in datalineagesearchlineage. +func parseExploreReferences(raw []any, lookerInstanceURI string) ([]LookerExploreReference, util.ToolboxError) { + refs := make([]LookerExploreReference, 0, len(raw)) + for i, er := range raw { + m, ok := er.(map[string]any) + if !ok { + return nil, util.NewAgentError(fmt.Sprintf("invalid explore reference at index %d in 'explore_references': expected object, got %T", i, er), nil) + } + model, ok := m["model"].(string) + if !ok { + return nil, util.NewAgentError(fmt.Sprintf("missing or invalid 'model' (expected string) in 'explore_references' at index %d", i), nil) + } + explore, ok := m["explore"].(string) + if !ok { + return nil, util.NewAgentError(fmt.Sprintf("missing or invalid 'explore' (expected string) in 'explore_references' at index %d", i), nil) + } + refs = append(refs, LookerExploreReference{ + LookerInstanceUri: lookerInstanceURI, + LookmlModel: model, + Explore: explore, + }) + } + return refs, nil +} + func (t Tool) Invoke(ctx context.Context, resourceMgr tools.SourceProvider, params parameters.ParamValues, accessToken tools.AccessToken) (any, util.ToolboxError) { source, err := tools.GetCompatibleSource[compatibleSource](resourceMgr, t.Cfg.Source, t.Cfg.Name, t.Cfg.Type) if err != nil { @@ -206,13 +235,9 @@ func (t Tool) Invoke(ctx context.Context, resourceMgr tools.SourceProvider, para userQuery, _ := mapParams["user_query_with_context"].(string) exploreReferences, _ := mapParams["explore_references"].([]any) - ler := make([]LookerExploreReference, 0) - for _, er := range exploreReferences { - ler = append(ler, LookerExploreReference{ - LookerInstanceUri: source.LookerApiSettings().BaseUrl, - LookmlModel: er.(map[string]any)["model"].(string), - Explore: er.(map[string]any)["explore"].(string), - }) + ler, lerErr := parseExploreReferences(exploreReferences, source.LookerApiSettings().BaseUrl) + if lerErr != nil { + return nil, lerErr } oauth_creds := OAuthCredentials{} if source.UseClientAuthorization() { diff --git a/internal/tools/looker/lookerconversationalanalytics/lookerconversationalanalytics_internal_test.go b/internal/tools/looker/lookerconversationalanalytics/lookerconversationalanalytics_internal_test.go new file mode 100644 index 00000000000..39af0721cb0 --- /dev/null +++ b/internal/tools/looker/lookerconversationalanalytics/lookerconversationalanalytics_internal_test.go @@ -0,0 +1,69 @@ +// Copyright 2026 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 lookerconversationalanalytics + +import "testing" + +func TestParseExploreReferences(t *testing.T) { + const baseURL = "https://looker.example.com" + + t.Run("valid references are parsed", func(t *testing.T) { + raw := []any{ + map[string]any{"model": "m1", "explore": "e1"}, + map[string]any{"model": "m2", "explore": "e2"}, + } + got, err := parseExploreReferences(raw, baseURL) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(got) != 2 { + t.Fatalf("expected 2 references, got %d", len(got)) + } + if got[0].LookmlModel != "m1" || got[0].Explore != "e1" || got[0].LookerInstanceUri != baseURL { + t.Fatalf("unexpected first reference: %+v", got[0]) + } + }) + + t.Run("nil input yields an empty slice without error", func(t *testing.T) { + got, err := parseExploreReferences(nil, baseURL) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(got) != 0 { + t.Fatalf("expected no references, got %d", len(got)) + } + }) + + // A model that hallucinates the explore_references shape (a non-object element, + // a missing field, or a non-string value) must surface a clean agent error + // rather than panic the tool on an unchecked type assertion. + errCases := []struct { + desc string + raw []any + }{ + {"element is not an object", []any{"not-a-map"}}, + {"missing model", []any{map[string]any{"explore": "e1"}}}, + {"model is not a string", []any{map[string]any{"model": 123, "explore": "e1"}}}, + {"missing explore", []any{map[string]any{"model": "m1"}}}, + {"explore is not a string", []any{map[string]any{"model": "m1", "explore": 7}}}, + } + for _, tc := range errCases { + t.Run(tc.desc, func(t *testing.T) { + if _, err := parseExploreReferences(tc.raw, baseURL); err == nil { + t.Fatalf("expected an error for %q, got nil", tc.desc) + } + }) + } +}