Skip to content

Commit b8f37da

Browse files
hiracky16hiracky16
authored
fix(tools/looker): enforce critical safety annotations for agent tools (#2967)
## Description This PR addresses the security and safety concerns raised in #2830 regarding critical annotations in Looker agent tools. ### Changes - **Annotation Protection**: Hardcoded `readOnlyHint` and `destructiveHint` in the `Initialize` method for the following tools to ensure they cannot be overridden by user configuration: - `create_agent` (readOnly: false) - `update_agent` (readOnly: false) - `delete_agent` (readOnly: false, destructive: true) - `get_agent` (readOnly: true) - `list_agents` (readOnly: true) - **Verification Tests**: Added `TestAnnotations` to each tool's test suite to verify that these hints are strictly enforced even when a user attempts to override them in the configuration. - **Go Environment Consistency**: Kept the Go version at `1.25.7` to match the current `main` branch state while ensuring no unwanted toolchain updates were introduced. ## Verification Results Ran unit tests for all 5 tools and confirmed that all tests, including the new annotation enforcement checks, passed successfully.
1 parent 5fd2693 commit b8f37da

File tree

10 files changed

+180
-32
lines changed

10 files changed

+180
-32
lines changed

‎internal/tools/looker/lookercreateagent/lookercreateagent.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,13 +84,12 @@ func (cfg Config) Initialize(srcs map[string]sources.Source) (tools.Tool, error)
8484
codeInterpreterParameter := parameters.NewBooleanParameterWithDefault("code_interpreter", false, "Optional. Enables Code Interpreter for this Agent.")
8585
params := parameters.Parameters{nameParameter, instructionsParameter, sourcesParameter, codeInterpreterParameter}
8686

87-
annotations := cfg.Annotations
88-
if annotations == nil {
89-
readOnlyHint := false
90-
annotations = &tools.ToolAnnotations{
91-
ReadOnlyHint: &readOnlyHint,
92-
}
87+
annotations := &tools.ToolAnnotations{}
88+
if cfg.Annotations != nil {
89+
*annotations = *cfg.Annotations
9390
}
91+
readOnlyHint := false
92+
annotations.ReadOnlyHint = &readOnlyHint
9493

9594
mcpManifest := tools.GetMcpManifest(cfg.Name, cfg.Description, cfg.AuthRequired, params, annotations)
9695

‎internal/tools/looker/lookercreateagent/lookercreateagent_test.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,3 +251,32 @@ func TestMcpManifest(t *testing.T) {
251251
}
252252
}
253253
}
254+
255+
func TestAnnotations(t *testing.T) {
256+
readOnlyTrue := true
257+
cfg := lkr.Config{
258+
Name: "test_tool",
259+
Type: "looker-create-agent",
260+
Source: "my-instance",
261+
Description: "test description",
262+
Annotations: &tools.ToolAnnotations{
263+
ReadOnlyHint: &readOnlyTrue,
264+
},
265+
}
266+
267+
tool, err := cfg.Initialize(nil)
268+
if err != nil {
269+
t.Fatalf("failed to initialize tool: %v", err)
270+
}
271+
272+
mcp := tool.McpManifest()
273+
if mcp.Annotations == nil {
274+
t.Fatal("mcp manifest annotations is nil")
275+
}
276+
if mcp.Annotations.ReadOnlyHint == nil {
277+
t.Fatal("mcp manifest ReadOnlyHint is nil")
278+
}
279+
if *mcp.Annotations.ReadOnlyHint != false {
280+
t.Errorf("ReadOnlyHint should be false, got %v", *mcp.Annotations.ReadOnlyHint)
281+
}
282+
}

‎internal/tools/looker/lookerdeleteagent/lookerdeleteagent.go‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -72,15 +72,14 @@ func (cfg Config) Initialize(srcs map[string]sources.Source) (tools.Tool, error)
7272
agentIdParameter := parameters.NewStringParameterWithDefault("agent_id", "", "The ID of the agent.")
7373
params := parameters.Parameters{agentIdParameter}
7474

75-
annotations := cfg.Annotations
76-
if annotations == nil {
77-
readOnlyHint := false
78-
destructiveHint := true
79-
annotations = &tools.ToolAnnotations{
80-
ReadOnlyHint: &readOnlyHint,
81-
DestructiveHint: &destructiveHint,
82-
}
75+
annotations := &tools.ToolAnnotations{}
76+
if cfg.Annotations != nil {
77+
*annotations = *cfg.Annotations
8378
}
79+
readOnlyHint := false
80+
destructiveHint := true
81+
annotations.ReadOnlyHint = &readOnlyHint
82+
annotations.DestructiveHint = &destructiveHint
8483

8584
mcpManifest := tools.GetMcpManifest(cfg.Name, cfg.Description, cfg.AuthRequired, params, annotations)
8685

‎internal/tools/looker/lookerdeleteagent/lookerdeleteagent_test.go‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,3 +243,40 @@ func TestMcpManifest(t *testing.T) {
243243
}
244244
}
245245
}
246+
247+
func TestAnnotations(t *testing.T) {
248+
readOnlyTrue := true
249+
destructiveFalse := false
250+
cfg := lkr.Config{
251+
Name: "test_tool",
252+
Type: "looker-delete-agent",
253+
Source: "my-instance",
254+
Description: "test description",
255+
Annotations: &tools.ToolAnnotations{
256+
ReadOnlyHint: &readOnlyTrue,
257+
DestructiveHint: &destructiveFalse,
258+
},
259+
}
260+
261+
tool, err := cfg.Initialize(nil)
262+
if err != nil {
263+
t.Fatalf("failed to initialize tool: %v", err)
264+
}
265+
266+
mcp := tool.McpManifest()
267+
if mcp.Annotations == nil {
268+
t.Fatal("mcp manifest annotations is nil")
269+
}
270+
if mcp.Annotations.ReadOnlyHint == nil {
271+
t.Fatal("mcp manifest ReadOnlyHint is nil")
272+
}
273+
if *mcp.Annotations.ReadOnlyHint != false {
274+
t.Errorf("ReadOnlyHint should be false, got %v", *mcp.Annotations.ReadOnlyHint)
275+
}
276+
if mcp.Annotations.DestructiveHint == nil {
277+
t.Fatal("mcp manifest DestructiveHint is nil")
278+
}
279+
if *mcp.Annotations.DestructiveHint != true {
280+
t.Errorf("DestructiveHint should be true, got %v", *mcp.Annotations.DestructiveHint)
281+
}
282+
}

‎internal/tools/looker/lookergetagent/lookergetagent.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,12 @@ func (cfg Config) Initialize(srcs map[string]sources.Source) (tools.Tool, error)
7272
agentIdParameter := parameters.NewStringParameterWithDefault("agent_id", "", "The ID of the agent.")
7373
params := parameters.Parameters{agentIdParameter}
7474

75-
annotations := cfg.Annotations
76-
if annotations == nil {
77-
readOnlyHint := true
78-
annotations = &tools.ToolAnnotations{
79-
ReadOnlyHint: &readOnlyHint,
80-
}
75+
annotations := &tools.ToolAnnotations{}
76+
if cfg.Annotations != nil {
77+
*annotations = *cfg.Annotations
8178
}
79+
readOnlyHint := true
80+
annotations.ReadOnlyHint = &readOnlyHint
8281

8382
mcpManifest := tools.GetMcpManifest(cfg.Name, cfg.Description, cfg.AuthRequired, params, annotations)
8483

‎internal/tools/looker/lookergetagent/lookergetagent_test.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,3 +243,32 @@ func TestMcpManifest(t *testing.T) {
243243
}
244244
}
245245
}
246+
247+
func TestAnnotations(t *testing.T) {
248+
readOnlyFalse := false
249+
cfg := lkr.Config{
250+
Name: "test_tool",
251+
Type: "looker-get-agent",
252+
Source: "my-instance",
253+
Description: "test description",
254+
Annotations: &tools.ToolAnnotations{
255+
ReadOnlyHint: &readOnlyFalse,
256+
},
257+
}
258+
259+
tool, err := cfg.Initialize(nil)
260+
if err != nil {
261+
t.Fatalf("failed to initialize tool: %v", err)
262+
}
263+
264+
mcp := tool.McpManifest()
265+
if mcp.Annotations == nil {
266+
t.Fatal("mcp manifest annotations is nil")
267+
}
268+
if mcp.Annotations.ReadOnlyHint == nil {
269+
t.Fatal("mcp manifest ReadOnlyHint is nil")
270+
}
271+
if *mcp.Annotations.ReadOnlyHint != true {
272+
t.Errorf("ReadOnlyHint should be true, got %v", *mcp.Annotations.ReadOnlyHint)
273+
}
274+
}

‎internal/tools/looker/lookerlistagents/lookerlistagents.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,13 +71,12 @@ func (cfg Config) ToolConfigType() string {
7171
func (cfg Config) Initialize(srcs map[string]sources.Source) (tools.Tool, error) {
7272
params := parameters.Parameters{}
7373

74-
annotations := cfg.Annotations
75-
if annotations == nil {
76-
readOnlyHint := true
77-
annotations = &tools.ToolAnnotations{
78-
ReadOnlyHint: &readOnlyHint,
79-
}
74+
annotations := &tools.ToolAnnotations{}
75+
if cfg.Annotations != nil {
76+
*annotations = *cfg.Annotations
8077
}
78+
readOnlyHint := true
79+
annotations.ReadOnlyHint = &readOnlyHint
8180

8281
mcpManifest := tools.GetMcpManifest(cfg.Name, cfg.Description, cfg.AuthRequired, params, annotations)
8382

‎internal/tools/looker/lookerlistagents/lookerlistagents_test.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,3 +204,32 @@ func TestMcpManifest(t *testing.T) {
204204
}
205205
}
206206
}
207+
208+
func TestAnnotations(t *testing.T) {
209+
readOnlyFalse := false
210+
cfg := lkr.Config{
211+
Name: "test_tool",
212+
Type: "looker-list-agents",
213+
Source: "my-instance",
214+
Description: "test description",
215+
Annotations: &tools.ToolAnnotations{
216+
ReadOnlyHint: &readOnlyFalse,
217+
},
218+
}
219+
220+
tool, err := cfg.Initialize(nil)
221+
if err != nil {
222+
t.Fatalf("failed to initialize tool: %v", err)
223+
}
224+
225+
mcp := tool.McpManifest()
226+
if mcp.Annotations == nil {
227+
t.Fatal("mcp manifest annotations is nil")
228+
}
229+
if mcp.Annotations.ReadOnlyHint == nil {
230+
t.Fatal("mcp manifest ReadOnlyHint is nil")
231+
}
232+
if *mcp.Annotations.ReadOnlyHint != true {
233+
t.Errorf("ReadOnlyHint should be true, got %v", *mcp.Annotations.ReadOnlyHint)
234+
}
235+
}

‎internal/tools/looker/lookerupdateagent/lookerupdateagent.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,12 @@ func (cfg Config) Initialize(srcs map[string]sources.Source) (tools.Tool, error)
8585
codeInterpreterParameter := parameters.NewBooleanParameterWithDefault("code_interpreter", false, "Optional. Enables Code Interpreter for this Agent.")
8686
params := parameters.Parameters{agentIdParameter, nameParameter, instructionsParameter, sourcesParameter, codeInterpreterParameter}
8787

88-
annotations := cfg.Annotations
89-
if annotations == nil {
90-
readOnlyHint := false
91-
annotations = &tools.ToolAnnotations{
92-
ReadOnlyHint: &readOnlyHint,
93-
}
88+
annotations := &tools.ToolAnnotations{}
89+
if cfg.Annotations != nil {
90+
*annotations = *cfg.Annotations
9491
}
92+
readOnlyHint := false
93+
annotations.ReadOnlyHint = &readOnlyHint
9594

9695
mcpManifest := tools.GetMcpManifest(cfg.Name, cfg.Description, cfg.AuthRequired, params, annotations)
9796

‎internal/tools/looker/lookerupdateagent/lookerupdateagent_test.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,3 +251,32 @@ func TestMcpManifest(t *testing.T) {
251251
}
252252
}
253253
}
254+
255+
func TestAnnotations(t *testing.T) {
256+
readOnlyTrue := true
257+
cfg := lkr.Config{
258+
Name: "test_tool",
259+
Type: "looker-update-agent",
260+
Source: "my-instance",
261+
Description: "test description",
262+
Annotations: &tools.ToolAnnotations{
263+
ReadOnlyHint: &readOnlyTrue,
264+
},
265+
}
266+
267+
tool, err := cfg.Initialize(nil)
268+
if err != nil {
269+
t.Fatalf("failed to initialize tool: %v", err)
270+
}
271+
272+
mcp := tool.McpManifest()
273+
if mcp.Annotations == nil {
274+
t.Fatal("mcp manifest annotations is nil")
275+
}
276+
if mcp.Annotations.ReadOnlyHint == nil {
277+
t.Fatal("mcp manifest ReadOnlyHint is nil")
278+
}
279+
if *mcp.Annotations.ReadOnlyHint != false {
280+
t.Errorf("ReadOnlyHint should be false, got %v", *mcp.Annotations.ReadOnlyHint)
281+
}
282+
}

0 commit comments

Comments
 (0)