From fe9325ad29b379ec74f31833a2b95bc22fc5230f Mon Sep 17 00:00:00 2001 From: Jiahua Huang Date: Thu, 25 Jun 2026 21:27:12 +0000 Subject: [PATCH] feat(tool/cloud-storage): configure object operation parameters --- .../tools/cloud-storage-copy-object.md | 16 + .../tools/cloud-storage-delete-object.md | 13 + .../tools/cloud-storage-download-object.md | 24 +- .../tools/cloud-storage-move-object.md | 13 + .../tools/cloud-storage-upload-object.md | 13 + .../tools/cloud-storage-write-object.md | 13 + .../cloudstorage/cloudstoragecommon/paths.go | 23 ++ .../cloudstoragecommon/paths_test.go | 39 +++ .../cloudstoragecopyobject.go | 36 +- .../cloudstoragecopyobject_test.go | 120 +++++++ .../cloudstoragedeleteobject.go | 15 +- .../cloudstoragedeleteobject_test.go | 100 ++++++ .../cloudstoragedownloadobject.go | 45 ++- .../cloudstoragedownloadobject_test.go | 165 ++++++++++ .../cloudstoragemoveobject.go | 15 +- .../cloudstoragemoveobject_test.go | 103 ++++++ .../cloudstorageuploadobject.go | 15 +- .../cloudstorageuploadobject_test.go | 109 ++++++ .../cloudstoragewriteobject.go | 15 +- .../cloudstoragewriteobject_test.go | 104 ++++++ .../cloud_storage_integration_test.go | 309 +++++++++++++++++- 21 files changed, 1265 insertions(+), 40 deletions(-) diff --git a/docs/en/integrations/cloud-storage/tools/cloud-storage-copy-object.md b/docs/en/integrations/cloud-storage/tools/cloud-storage-copy-object.md index 83bee9423398..cbef91f916ff 100644 --- a/docs/en/integrations/cloud-storage/tools/cloud-storage-copy-object.md +++ b/docs/en/integrations/cloud-storage/tools/cloud-storage-copy-object.md @@ -32,6 +32,10 @@ or update the destination object. | destination_bucket | string | true | Name of the Cloud Storage bucket to copy into. | | destination_object | string | true | Full destination object name (path) within the destination bucket. | +If `source_bucket` or `destination_bucket` is configured on the tool, that field +is removed from the parameter list and the configured value is used for every +invocation. + ## Example ```yaml @@ -42,6 +46,16 @@ source: my-gcs-source description: Use this tool to copy Cloud Storage objects. ``` +```yaml +kind: tool +name: copy_reports +type: cloud-storage-copy-object +source: my-gcs-source +description: Use this tool to copy reports into the archive bucket. +source_bucket: analytics-exports +destination_bucket: analytics-archive +``` + ## Output Format The tool returns a JSON object with: @@ -62,3 +76,5 @@ The tool returns a JSON object with: | type | string | true | Must be "cloud-storage-copy-object". | | source | string | true | Name of the Cloud Storage source to copy objects in. | | description | string | true | Description of the tool that is passed to the LLM. | +| source_bucket | string | false | Source Cloud Storage bucket to use for every invocation. When set, `source_bucket` is hidden from the tool parameters. | +| destination_bucket | string | false | Destination Cloud Storage bucket to use for every invocation. When set, `destination_bucket` is hidden from the tool parameters. | diff --git a/docs/en/integrations/cloud-storage/tools/cloud-storage-delete-object.md b/docs/en/integrations/cloud-storage/tools/cloud-storage-delete-object.md index fc201f483e2e..f3243d82b7f1 100644 --- a/docs/en/integrations/cloud-storage/tools/cloud-storage-delete-object.md +++ b/docs/en/integrations/cloud-storage/tools/cloud-storage-delete-object.md @@ -26,6 +26,9 @@ The Cloud Storage credentials must be able to delete the target object. | bucket | string | true | Name of the Cloud Storage bucket containing the object to delete. | | object | string | true | Full object name (path) within the bucket, e.g. `path/to/file.txt`. | +If `bucket` is configured on the tool, it is removed from the parameter list and +the configured bucket is used for every invocation. + ## Example ```yaml @@ -36,6 +39,15 @@ source: my-gcs-source description: Use this tool to delete Cloud Storage objects. ``` +```yaml +kind: tool +name: delete_reports +type: cloud-storage-delete-object +source: my-gcs-source +description: Use this tool to delete report objects from Cloud Storage. +bucket: analytics-exports +``` + ## Output Format The tool returns a JSON object with: @@ -53,3 +65,4 @@ The tool returns a JSON object with: | type | string | true | Must be "cloud-storage-delete-object". | | source | string | true | Name of the Cloud Storage source to delete objects in. | | description | string | true | Description of the tool that is passed to the LLM. | +| bucket | string | false | Cloud Storage bucket to use for every invocation. When set, `bucket` is hidden from the tool parameters. | diff --git a/docs/en/integrations/cloud-storage/tools/cloud-storage-download-object.md b/docs/en/integrations/cloud-storage/tools/cloud-storage-download-object.md index fecb04be5714..20e21441c814 100644 --- a/docs/en/integrations/cloud-storage/tools/cloud-storage-download-object.md +++ b/docs/en/integrations/cloud-storage/tools/cloud-storage-download-object.md @@ -14,7 +14,10 @@ return the object bytes to the LLM and does not require UTF-8 text content, so i can be used for binary objects or large files. The `destination` path is interpreted on the server where Toolbox is running. -Relative paths and paths containing `..` are rejected. +By default, `destination` must be an absolute path and paths containing `..` are +rejected. When `destination_dir` is configured on the tool, `destination` remains +visible to the LLM but must be a relative path under that configured directory. +Absolute paths and paths that escape `destination_dir` are rejected. [gcs-objects]: https://cloud.google.com/storage/docs/objects @@ -38,6 +41,11 @@ destination already exists. | destination | string | true | Absolute local filesystem path where the object will be written. Relative paths and paths containing `..` are rejected. | | overwrite | boolean | false | If true, overwrite the destination when it already exists. If false (default), return an error when it exists. | +If `bucket` is configured on the tool, it is removed from the parameter list. If +`destination_dir` is configured on the tool, `destination` stays in the parameter +list but changes to a relative path under `destination_dir`. If `overwrite` is +configured on the tool, it is removed from the parameter list. + ## Example ```yaml @@ -48,6 +56,17 @@ source: my-gcs-source description: Use this tool to download a Cloud Storage object to the server filesystem. ``` +```yaml +kind: tool +name: download_reports +type: cloud-storage-download-object +source: my-gcs-source +description: Use this tool to download report objects into the reports directory. +bucket: analytics-exports +destination_dir: /var/toolbox/downloads/reports +overwrite: false +``` + ## Output Format The tool returns a JSON object with: @@ -65,3 +84,6 @@ The tool returns a JSON object with: | type | string | true | Must be "cloud-storage-download-object". | | source | string | true | Name of the Cloud Storage source to download objects from. | | description | string | true | Description of the tool that is passed to the LLM. | +| bucket | string | false | Cloud Storage bucket to use for every invocation. When set, `bucket` is hidden from the tool parameters. | +| destination_dir | string | false | Absolute local directory to contain downloaded files. When set, `destination` is a relative path under this directory. | +| overwrite | boolean | false | Whether to overwrite existing destination files for every invocation. When set, `overwrite` is hidden from the tool parameters. | diff --git a/docs/en/integrations/cloud-storage/tools/cloud-storage-move-object.md b/docs/en/integrations/cloud-storage/tools/cloud-storage-move-object.md index 100d5cb42f17..3b025653cd02 100644 --- a/docs/en/integrations/cloud-storage/tools/cloud-storage-move-object.md +++ b/docs/en/integrations/cloud-storage/tools/cloud-storage-move-object.md @@ -33,6 +33,9 @@ already exists, `storage.objects.delete` is also required. | source_object | string | true | Full source object name (path) within the bucket, e.g. `path/to/file.txt`. | | destination_object | string | true | Full destination object name (path) within the same bucket. | +If `bucket` is configured on the tool, it is removed from the parameter list and +the configured bucket is used for every invocation. + ## Example ```yaml @@ -43,6 +46,15 @@ source: my-gcs-source description: Use this tool to move or rename an object within a Cloud Storage bucket. ``` +```yaml +kind: tool +name: move_reports +type: cloud-storage-move-object +source: my-gcs-source +description: Use this tool to move report objects within the reports bucket. +bucket: analytics-exports +``` + ## Output Format The tool returns a JSON object with: @@ -62,3 +74,4 @@ The tool returns a JSON object with: | type | string | true | Must be "cloud-storage-move-object". | | source | string | true | Name of the Cloud Storage source to move objects in. | | description | string | true | Description of the tool that is passed to the LLM. | +| bucket | string | false | Cloud Storage bucket to use for every invocation. When set, `bucket` is hidden from the tool parameters. | diff --git a/docs/en/integrations/cloud-storage/tools/cloud-storage-upload-object.md b/docs/en/integrations/cloud-storage/tools/cloud-storage-upload-object.md index 274d3d7f0ad7..7eda1cf44173 100644 --- a/docs/en/integrations/cloud-storage/tools/cloud-storage-upload-object.md +++ b/docs/en/integrations/cloud-storage/tools/cloud-storage-upload-object.md @@ -38,6 +38,9 @@ file. | source | string | true | Absolute local filesystem path of the file to upload. Relative paths and paths containing `..` are rejected. | | content_type | string | false | MIME type to record on the uploaded object. When empty, it is inferred from the source file extension when possible. | +If `bucket` is configured on the tool, it is removed from the parameter list and +the configured bucket is used for every invocation. + ## Example ```yaml @@ -48,6 +51,15 @@ source: my-gcs-source description: Use this tool to upload a local file to Cloud Storage. ``` +```yaml +kind: tool +name: upload_reports +type: cloud-storage-upload-object +source: my-gcs-source +description: Use this tool to upload local report files to Cloud Storage. +bucket: analytics-exports +``` + ## Output Format The tool returns a JSON object with: @@ -66,3 +78,4 @@ The tool returns a JSON object with: | type | string | true | Must be "cloud-storage-upload-object". | | source | string | true | Name of the Cloud Storage source to upload objects to. | | description | string | true | Description of the tool that is passed to the LLM. | +| bucket | string | false | Cloud Storage bucket to use for every invocation. When set, `bucket` is hidden from the tool parameters. | diff --git a/docs/en/integrations/cloud-storage/tools/cloud-storage-write-object.md b/docs/en/integrations/cloud-storage/tools/cloud-storage-write-object.md index 980909a918ca..94bdf5e24d85 100644 --- a/docs/en/integrations/cloud-storage/tools/cloud-storage-write-object.md +++ b/docs/en/integrations/cloud-storage/tools/cloud-storage-write-object.md @@ -33,6 +33,9 @@ object. | content | string | true | Text content to write to the Cloud Storage object. | | content_type | string | false | MIME type to record on the written object. When empty, Cloud Storage auto-detects from the content. | +If `bucket` is configured on the tool, it is removed from the parameter list and +the configured bucket is used for every invocation. + ## Example ```yaml @@ -43,6 +46,15 @@ source: my-gcs-source description: Use this tool to write text content to Cloud Storage. ``` +```yaml +kind: tool +name: write_reports +type: cloud-storage-write-object +source: my-gcs-source +description: Use this tool to write generated reports to Cloud Storage. +bucket: analytics-exports +``` + ## Output Format The tool returns a JSON object with: @@ -61,3 +73,4 @@ The tool returns a JSON object with: | type | string | true | Must be "cloud-storage-write-object". | | source | string | true | Name of the Cloud Storage source to write objects to. | | description | string | true | Description of the tool that is passed to the LLM. | +| bucket | string | false | Cloud Storage bucket to use for every invocation. When set, `bucket` is hidden from the tool parameters. | diff --git a/internal/tools/cloudstorage/cloudstoragecommon/paths.go b/internal/tools/cloudstorage/cloudstoragecommon/paths.go index f21cb1433015..2dcb457e97dd 100644 --- a/internal/tools/cloudstorage/cloudstoragecommon/paths.go +++ b/internal/tools/cloudstorage/cloudstoragecommon/paths.go @@ -49,3 +49,26 @@ func ValidateLocalPath(p string) (string, error) { } return clean, nil } + +func ResolveWithinDir(dir, rel string) (string, error) { + cleanDir, err := ValidateLocalPath(dir) + if err != nil { + return "", fmt.Errorf("directory %q is invalid: %w", dir, err) + } + if rel == "" { + return "", fmt.Errorf("relative path is empty") + } + if filepath.IsAbs(rel) { + return "", fmt.Errorf("path %q must be relative", rel) + } + + cleanDest := filepath.Clean(filepath.Join(cleanDir, rel)) + within, err := filepath.Rel(cleanDir, cleanDest) + if err != nil { + return "", fmt.Errorf("path %q cannot be resolved within %q: %w", rel, cleanDir, err) + } + if within == ".." || strings.HasPrefix(within, ".."+string(filepath.Separator)) || filepath.IsAbs(within) { + return "", fmt.Errorf("path %q escapes configured directory %q", rel, cleanDir) + } + return cleanDest, nil +} diff --git a/internal/tools/cloudstorage/cloudstoragecommon/paths_test.go b/internal/tools/cloudstorage/cloudstoragecommon/paths_test.go index 4e480ca7b842..e75d75d06671 100644 --- a/internal/tools/cloudstorage/cloudstoragecommon/paths_test.go +++ b/internal/tools/cloudstorage/cloudstoragecommon/paths_test.go @@ -59,3 +59,42 @@ func TestValidateLocalPath(t *testing.T) { }) } } + +func TestResolveWithinDir(t *testing.T) { + base := t.TempDir() + + tcs := []struct { + desc string + dir string + rel string + want string + wantErr bool + }{ + {desc: "valid relative path", dir: base, rel: filepath.Join("nested", "out.bin"), want: filepath.Join(base, "nested", "out.bin")}, + {desc: "cleans in-dir path", dir: base, rel: filepath.Join("nested", ".", "out.bin"), want: filepath.Join(base, "nested", "out.bin")}, + {desc: "cleans parent segment that stays in dir", dir: base, rel: filepath.Join("nested", "..", "out.bin"), want: filepath.Join(base, "out.bin")}, + {desc: "empty dir", dir: "", rel: "out.bin", wantErr: true}, + {desc: "relative dir", dir: "relative/base", rel: "out.bin", wantErr: true}, + {desc: "empty relative path", dir: base, rel: "", wantErr: true}, + {desc: "absolute relative path rejected", dir: base, rel: filepath.Join(base, "out.bin"), wantErr: true}, + {desc: "parent escape rejected", dir: base, rel: filepath.Join("..", "escape.bin"), wantErr: true}, + {desc: "nested parent escape rejected", dir: base, rel: filepath.Join("nested", "..", "..", "escape.bin"), wantErr: true}, + } + for _, tc := range tcs { + t.Run(tc.desc, func(t *testing.T) { + got, err := ResolveWithinDir(tc.dir, tc.rel) + if tc.wantErr { + if err == nil { + t.Fatalf("expected error, got %q", got) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tc.want { + t.Fatalf("got %q, want %q", got, tc.want) + } + }) + } +} diff --git a/internal/tools/cloudstorage/cloudstoragecopyobject/cloudstoragecopyobject.go b/internal/tools/cloudstorage/cloudstoragecopyobject/cloudstoragecopyobject.go index c170e64b6e32..8a4a607c72d6 100644 --- a/internal/tools/cloudstorage/cloudstoragecopyobject/cloudstoragecopyobject.go +++ b/internal/tools/cloudstorage/cloudstoragecopyobject/cloudstoragecopyobject.go @@ -54,10 +54,12 @@ type compatibleSource interface { } type Config struct { - tools.ConfigBase `yaml:",inline"` - Type string `yaml:"type" validate:"required"` - Source string `yaml:"source" validate:"required"` - Annotations *tools.ToolAnnotations `yaml:"annotations,omitempty"` + tools.ConfigBase `yaml:",inline"` + Type string `yaml:"type" validate:"required"` + Source string `yaml:"source" validate:"required"` + Annotations *tools.ToolAnnotations `yaml:"annotations,omitempty"` + SourceBucket *string `yaml:"source_bucket,omitempty"` + DestinationBucket *string `yaml:"destination_bucket,omitempty"` } var _ tools.ToolConfig = Config{} @@ -70,12 +72,24 @@ func (cfg Config) Initialize(context.Context) (tools.Tool, error) { if cfg.Description == "" { return nil, fmt.Errorf("description is required for tool %q", cfg.Name) } + if cfg.SourceBucket != nil && *cfg.SourceBucket == "" { + return nil, fmt.Errorf("source_bucket cannot be empty for tool %q", cfg.Name) + } + if cfg.DestinationBucket != nil && *cfg.DestinationBucket == "" { + return nil, fmt.Errorf("destination_bucket cannot be empty for tool %q", cfg.Name) + } - sourceBucketParam := parameters.NewStringParameter(sourceBucketKey, "Name of the Cloud Storage bucket containing the source object.") sourceObjectParam := parameters.NewStringParameter(sourceObjectKey, "Full source object name (path) within the source bucket, e.g. 'path/to/file.txt'.") - destinationBucketParam := parameters.NewStringParameter(destinationBucketKey, "Name of the Cloud Storage bucket to copy into.") destinationObjectParam := parameters.NewStringParameter(destinationObjectKey, "Full destination object name (path) within the destination bucket, e.g. 'path/to/file.txt'.") - allParameters := parameters.Parameters{sourceBucketParam, sourceObjectParam, destinationBucketParam, destinationObjectParam} + allParameters := parameters.Parameters{} + if cfg.SourceBucket == nil { + allParameters = append(allParameters, parameters.NewStringParameter(sourceBucketKey, "Name of the Cloud Storage bucket containing the source object.")) + } + allParameters = append(allParameters, sourceObjectParam) + if cfg.DestinationBucket == nil { + allParameters = append(allParameters, parameters.NewStringParameter(destinationBucketKey, "Name of the Cloud Storage bucket to copy into.")) + } + allParameters = append(allParameters, destinationObjectParam) return Tool{ BaseTool: tools.NewBaseTool( @@ -104,16 +118,16 @@ func (t Tool) Invoke(ctx context.Context, resourceMgr tools.SourceProvider, para } mapParams := params.AsMap() - sourceBucket, ok := mapParams[sourceBucketKey].(string) - if !ok || sourceBucket == "" { + sourceBucket := cloudstoragecommon.ResolveString(t.Cfg.SourceBucket, mapParams, sourceBucketKey) + if sourceBucket == "" { return nil, util.NewAgentError(fmt.Sprintf("invalid or missing '%s' parameter; expected a non-empty string", sourceBucketKey), nil) } sourceObject, ok := mapParams[sourceObjectKey].(string) if !ok || sourceObject == "" { return nil, util.NewAgentError(fmt.Sprintf("invalid or missing '%s' parameter; expected a non-empty string", sourceObjectKey), nil) } - destinationBucket, ok := mapParams[destinationBucketKey].(string) - if !ok || destinationBucket == "" { + destinationBucket := cloudstoragecommon.ResolveString(t.Cfg.DestinationBucket, mapParams, destinationBucketKey) + if destinationBucket == "" { return nil, util.NewAgentError(fmt.Sprintf("invalid or missing '%s' parameter; expected a non-empty string", destinationBucketKey), nil) } destinationObject, ok := mapParams[destinationObjectKey].(string) diff --git a/internal/tools/cloudstorage/cloudstoragecopyobject/cloudstoragecopyobject_test.go b/internal/tools/cloudstorage/cloudstoragecopyobject/cloudstoragecopyobject_test.go index 53b2faedf613..36bfe3dd1e1f 100644 --- a/internal/tools/cloudstorage/cloudstoragecopyobject/cloudstoragecopyobject_test.go +++ b/internal/tools/cloudstorage/cloudstoragecopyobject/cloudstoragecopyobject_test.go @@ -83,6 +83,31 @@ func TestParseFromYamlCloudStorageCopyObject(t *testing.T) { }, }, }, + { + desc: "with configurable buckets", + in: ` + kind: tool + name: configured_copy + type: cloud-storage-copy-object + source: prod-gcs + description: Copy configured object + source_bucket: source-bucket + destination_bucket: destination-bucket + `, + want: server.ToolConfigs{ + "configured_copy": cloudstoragecopyobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "configured_copy", + Description: "Copy configured object", + AuthRequired: []string{}, + }, + Type: "cloud-storage-copy-object", + Source: "prod-gcs", + SourceBucket: strPtr("source-bucket"), + DestinationBucket: strPtr("destination-bucket"), + }, + }, + }, } for _, tc := range tcs { t.Run(tc.desc, func(t *testing.T) { @@ -97,6 +122,10 @@ func TestParseFromYamlCloudStorageCopyObject(t *testing.T) { } } +func strPtr(s string) *string { + return &s +} + type mockSource struct { sources.Source called bool @@ -106,6 +135,97 @@ type mockSource struct { gotDestinationObject string } +func TestConfiguredBucketsHiddenAndForwarded(t *testing.T) { + cfg := cloudstoragecopyobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "copy_tool", + Description: "Copy", + }, + Type: "cloud-storage-copy-object", + Source: "my-gcs", + SourceBucket: strPtr("source-bucket"), + DestinationBucket: strPtr("destination-bucket"), + } + tool, err := cfg.Initialize(context.Background()) + if err != nil { + t.Fatalf("failed to initialize tool: %v", err) + } + gotNames := manifestParamNames(tool.StaticManifest().Parameters) + wantNames := []string{"source_object", "destination_object"} + if diff := cmp.Diff(wantNames, gotNames); diff != "" { + t.Fatalf("manifest parameters mismatch (-want +got):\n%s", diff) + } + + src := &mockSource{} + params := parameters.ParamValues{ + {Name: "source_object", Value: "src"}, + {Name: "destination_object", Value: "dst"}, + } + if _, err := tool.Invoke(context.Background(), &mockSourceProvider{source: src}, params, ""); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if src.gotSourceBucket != "source-bucket" || src.gotSourceObject != "src" || src.gotDestinationBucket != "destination-bucket" || src.gotDestinationObject != "dst" { + t.Fatalf("forwarded params = %q/%q/%q/%q, want source-bucket/src/destination-bucket/dst", src.gotSourceBucket, src.gotSourceObject, src.gotDestinationBucket, src.gotDestinationObject) + } +} + +func TestUnsetBucketsRemainVisible(t *testing.T) { + cfg := cloudstoragecopyobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "copy_tool", + Description: "Copy", + }, + Type: "cloud-storage-copy-object", + Source: "my-gcs", + } + tool, err := cfg.Initialize(context.Background()) + if err != nil { + t.Fatalf("failed to initialize tool: %v", err) + } + gotNames := manifestParamNames(tool.StaticManifest().Parameters) + wantNames := []string{"source_bucket", "source_object", "destination_bucket", "destination_object"} + if diff := cmp.Diff(wantNames, gotNames); diff != "" { + t.Fatalf("manifest parameters mismatch (-want +got):\n%s", diff) + } +} + +func TestEmptyConfiguredBucketsRejected(t *testing.T) { + tcs := []struct { + desc string + sourceBucket *string + destinationBucket *string + wantSubstr string + }{ + {desc: "empty source bucket", sourceBucket: strPtr(""), wantSubstr: "source_bucket"}, + {desc: "empty destination bucket", destinationBucket: strPtr(""), wantSubstr: "destination_bucket"}, + } + for _, tc := range tcs { + t.Run(tc.desc, func(t *testing.T) { + cfg := cloudstoragecopyobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "copy_tool", + Description: "Copy", + }, + Type: "cloud-storage-copy-object", + Source: "my-gcs", + SourceBucket: tc.sourceBucket, + DestinationBucket: tc.destinationBucket, + } + if _, err := cfg.Initialize(context.Background()); err == nil || !strings.Contains(err.Error(), tc.wantSubstr) { + t.Fatalf("Initialize() error = %v, want %q", err, tc.wantSubstr) + } + }) + } +} + +func manifestParamNames(params []parameters.ParameterManifest) []string { + names := make([]string, 0, len(params)) + for _, p := range params { + names = append(names, p.Name) + } + return names +} + func (m *mockSource) CopyObject(ctx context.Context, sourceBucket, sourceObject, destinationBucket, destinationObject string) (map[string]any, error) { m.called = true m.gotSourceBucket = sourceBucket diff --git a/internal/tools/cloudstorage/cloudstoragedeleteobject/cloudstoragedeleteobject.go b/internal/tools/cloudstorage/cloudstoragedeleteobject/cloudstoragedeleteobject.go index 962252a9b21e..7402830adb61 100644 --- a/internal/tools/cloudstorage/cloudstoragedeleteobject/cloudstoragedeleteobject.go +++ b/internal/tools/cloudstorage/cloudstoragedeleteobject/cloudstoragedeleteobject.go @@ -56,6 +56,7 @@ type Config struct { Type string `yaml:"type" validate:"required"` Source string `yaml:"source" validate:"required"` Annotations *tools.ToolAnnotations `yaml:"annotations,omitempty"` + Bucket *string `yaml:"bucket,omitempty"` } var _ tools.ToolConfig = Config{} @@ -68,10 +69,16 @@ func (cfg Config) Initialize(context.Context) (tools.Tool, error) { if cfg.Description == "" { return nil, fmt.Errorf("description is required for tool %q", cfg.Name) } + if cfg.Bucket != nil && *cfg.Bucket == "" { + return nil, fmt.Errorf("bucket cannot be empty for tool %q", cfg.Name) + } - bucketParam := parameters.NewStringParameter(bucketKey, "Name of the Cloud Storage bucket containing the object to delete.") objectParam := parameters.NewStringParameter(objectKey, "Full object name (path) within the bucket, e.g. 'path/to/file.txt'.") - allParameters := parameters.Parameters{bucketParam, objectParam} + allParameters := parameters.Parameters{} + if cfg.Bucket == nil { + allParameters = append(allParameters, parameters.NewStringParameter(bucketKey, "Name of the Cloud Storage bucket containing the object to delete.")) + } + allParameters = append(allParameters, objectParam) return Tool{ BaseTool: tools.NewBaseTool( @@ -100,8 +107,8 @@ func (t Tool) Invoke(ctx context.Context, resourceMgr tools.SourceProvider, para } mapParams := params.AsMap() - bucket, ok := mapParams[bucketKey].(string) - if !ok || bucket == "" { + bucket := cloudstoragecommon.ResolveString(t.Cfg.Bucket, mapParams, bucketKey) + if bucket == "" { return nil, util.NewAgentError(fmt.Sprintf("invalid or missing '%s' parameter; expected a non-empty string", bucketKey), nil) } object, ok := mapParams[objectKey].(string) diff --git a/internal/tools/cloudstorage/cloudstoragedeleteobject/cloudstoragedeleteobject_test.go b/internal/tools/cloudstorage/cloudstoragedeleteobject/cloudstoragedeleteobject_test.go index 05ca92f44729..8815153b4916 100644 --- a/internal/tools/cloudstorage/cloudstoragedeleteobject/cloudstoragedeleteobject_test.go +++ b/internal/tools/cloudstorage/cloudstoragedeleteobject/cloudstoragedeleteobject_test.go @@ -83,6 +83,29 @@ func TestParseFromYamlCloudStorageDeleteObject(t *testing.T) { }, }, }, + { + desc: "with configurable bucket", + in: ` + kind: tool + name: configured_delete + type: cloud-storage-delete-object + source: prod-gcs + description: Delete configured object + bucket: baked-bucket + `, + want: server.ToolConfigs{ + "configured_delete": cloudstoragedeleteobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "configured_delete", + Description: "Delete configured object", + AuthRequired: []string{}, + }, + Type: "cloud-storage-delete-object", + Source: "prod-gcs", + Bucket: strPtr("baked-bucket"), + }, + }, + }, } for _, tc := range tcs { t.Run(tc.desc, func(t *testing.T) { @@ -97,6 +120,10 @@ func TestParseFromYamlCloudStorageDeleteObject(t *testing.T) { } } +func strPtr(s string) *string { + return &s +} + type mockSource struct { sources.Source called bool @@ -104,6 +131,79 @@ type mockSource struct { gotObject string } +func TestConfiguredBucketHiddenAndForwarded(t *testing.T) { + cfg := cloudstoragedeleteobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "delete_tool", + Description: "Delete", + }, + Type: "cloud-storage-delete-object", + Source: "my-gcs", + Bucket: strPtr("baked-bucket"), + } + tool, err := cfg.Initialize(context.Background()) + if err != nil { + t.Fatalf("failed to initialize tool: %v", err) + } + gotNames := manifestParamNames(tool.StaticManifest().Parameters) + wantNames := []string{"object"} + if diff := cmp.Diff(wantNames, gotNames); diff != "" { + t.Fatalf("manifest parameters mismatch (-want +got):\n%s", diff) + } + + src := &mockSource{} + params := parameters.ParamValues{{Name: "object", Value: "o"}} + if _, err := tool.Invoke(context.Background(), &mockSourceProvider{source: src}, params, ""); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if src.gotBucket != "baked-bucket" || src.gotObject != "o" { + t.Fatalf("forwarded params = %q/%q, want baked-bucket/o", src.gotBucket, src.gotObject) + } +} + +func TestUnsetBucketRemainsVisible(t *testing.T) { + cfg := cloudstoragedeleteobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "delete_tool", + Description: "Delete", + }, + Type: "cloud-storage-delete-object", + Source: "my-gcs", + } + tool, err := cfg.Initialize(context.Background()) + if err != nil { + t.Fatalf("failed to initialize tool: %v", err) + } + gotNames := manifestParamNames(tool.StaticManifest().Parameters) + wantNames := []string{"bucket", "object"} + if diff := cmp.Diff(wantNames, gotNames); diff != "" { + t.Fatalf("manifest parameters mismatch (-want +got):\n%s", diff) + } +} + +func TestEmptyConfiguredBucketRejected(t *testing.T) { + cfg := cloudstoragedeleteobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "delete_tool", + Description: "Delete", + }, + Type: "cloud-storage-delete-object", + Source: "my-gcs", + Bucket: strPtr(""), + } + if _, err := cfg.Initialize(context.Background()); err == nil || !strings.Contains(err.Error(), "bucket") { + t.Fatalf("Initialize() error = %v, want bucket error", err) + } +} + +func manifestParamNames(params []parameters.ParameterManifest) []string { + names := make([]string, 0, len(params)) + for _, p := range params { + names = append(names, p.Name) + } + return names +} + func (m *mockSource) DeleteObject(ctx context.Context, bucket, object string) (map[string]any, error) { m.called = true m.gotBucket = bucket diff --git a/internal/tools/cloudstorage/cloudstoragedownloadobject/cloudstoragedownloadobject.go b/internal/tools/cloudstorage/cloudstoragedownloadobject/cloudstoragedownloadobject.go index 24bd78c80eed..09824840f5e0 100644 --- a/internal/tools/cloudstorage/cloudstoragedownloadobject/cloudstoragedownloadobject.go +++ b/internal/tools/cloudstorage/cloudstoragedownloadobject/cloudstoragedownloadobject.go @@ -58,6 +58,9 @@ type Config struct { Type string `yaml:"type" validate:"required"` Source string `yaml:"source" validate:"required"` Annotations *tools.ToolAnnotations `yaml:"annotations,omitempty"` + Bucket *string `yaml:"bucket,omitempty"` + DestinationDir *string `yaml:"destination_dir,omitempty"` + Overwrite *bool `yaml:"overwrite,omitempty"` } // validate interface @@ -71,12 +74,32 @@ func (cfg Config) Initialize(context.Context) (tools.Tool, error) { if cfg.Description == "" { return nil, fmt.Errorf("description is required for tool %q", cfg.Name) } + if cfg.Bucket != nil && *cfg.Bucket == "" { + return nil, fmt.Errorf("bucket cannot be empty for tool %q", cfg.Name) + } + if cfg.DestinationDir != nil { + if *cfg.DestinationDir == "" { + return nil, fmt.Errorf("destination_dir cannot be empty for tool %q", cfg.Name) + } + if _, err := cloudstoragecommon.ValidateLocalPath(*cfg.DestinationDir); err != nil { + return nil, fmt.Errorf("destination_dir is invalid for tool %q: %w", cfg.Name, err) + } + } - bucketParam := parameters.NewStringParameter(bucketKey, "Name of the Cloud Storage bucket containing the object.") objectParam := parameters.NewStringParameter(objectKey, "Full object name (path) within the bucket, e.g. 'path/to/file.txt'.") - destinationParam := parameters.NewStringParameter(destinationKey, "Absolute local filesystem path where the object will be written. Relative paths and paths containing '..' are rejected.") - overwriteParam := parameters.NewBooleanParameter(overwriteKey, "If true, overwrite the destination when it already exists. If false (default), the tool returns an error when the destination exists.", parameters.WithBooleanDefault(false)) - allParameters := parameters.Parameters{bucketParam, objectParam, destinationParam, overwriteParam} + destinationDesc := "Absolute local filesystem path where the object will be written. Relative paths and paths containing '..' are rejected." + if cfg.DestinationDir != nil { + destinationDesc = "Relative path under the configured destination_dir where the object will be written. Absolute paths and paths that escape destination_dir are rejected." + } + allParameters := parameters.Parameters{} + if cfg.Bucket == nil { + allParameters = append(allParameters, parameters.NewStringParameter(bucketKey, "Name of the Cloud Storage bucket containing the object.")) + } + allParameters = append(allParameters, objectParam) + allParameters = append(allParameters, parameters.NewStringParameter(destinationKey, destinationDesc)) + if cfg.Overwrite == nil { + allParameters = append(allParameters, parameters.NewBooleanParameter(overwriteKey, "If true, overwrite the destination when it already exists. If false (default), the tool returns an error when the destination exists.", parameters.WithBooleanDefault(false))) + } return Tool{ BaseTool: tools.NewBaseTool( @@ -106,8 +129,8 @@ func (t Tool) Invoke(ctx context.Context, resourceMgr tools.SourceProvider, para } mapParams := params.AsMap() - bucket, ok := mapParams[bucketKey].(string) - if !ok || bucket == "" { + bucket := cloudstoragecommon.ResolveString(t.Cfg.Bucket, mapParams, bucketKey) + if bucket == "" { return nil, util.NewAgentError(fmt.Sprintf("invalid or missing '%s' parameter; expected a non-empty string", bucketKey), nil) } object, ok := mapParams[objectKey].(string) @@ -118,11 +141,17 @@ func (t Tool) Invoke(ctx context.Context, resourceMgr tools.SourceProvider, para if !ok || destination == "" { return nil, util.NewAgentError(fmt.Sprintf("invalid or missing '%s' parameter; expected a non-empty string", destinationKey), nil) } - cleanDest, pathErr := cloudstoragecommon.ValidateLocalPath(destination) + var cleanDest string + var pathErr error + if t.Cfg.DestinationDir != nil { + cleanDest, pathErr = cloudstoragecommon.ResolveWithinDir(*t.Cfg.DestinationDir, destination) + } else { + cleanDest, pathErr = cloudstoragecommon.ValidateLocalPath(destination) + } if pathErr != nil { return nil, util.NewAgentError(fmt.Sprintf("invalid '%s' parameter: %v", destinationKey, pathErr), pathErr) } - overwrite, _ := mapParams[overwriteKey].(bool) + overwrite := cloudstoragecommon.ResolveBool(t.Cfg.Overwrite, mapParams, overwriteKey) resp, err := source.DownloadObject(ctx, bucket, object, cleanDest, overwrite) if err != nil { diff --git a/internal/tools/cloudstorage/cloudstoragedownloadobject/cloudstoragedownloadobject_test.go b/internal/tools/cloudstorage/cloudstoragedownloadobject/cloudstoragedownloadobject_test.go index e32debf5d699..bb781ca27f9f 100644 --- a/internal/tools/cloudstorage/cloudstoragedownloadobject/cloudstoragedownloadobject_test.go +++ b/internal/tools/cloudstorage/cloudstoragedownloadobject/cloudstoragedownloadobject_test.go @@ -84,6 +84,33 @@ func TestParseFromYamlCloudStorageDownloadObject(t *testing.T) { }, }, }, + { + desc: "with configurable parameters", + in: ` + kind: tool + name: configured_download + type: cloud-storage-download-object + source: prod-gcs + description: Download configured object + bucket: baked-bucket + destination_dir: /tmp/downloads + overwrite: false + `, + want: server.ToolConfigs{ + "configured_download": cloudstoragedownloadobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "configured_download", + Description: "Download configured object", + AuthRequired: []string{}, + }, + Type: "cloud-storage-download-object", + Source: "prod-gcs", + Bucket: strPtr("baked-bucket"), + DestinationDir: strPtr("/tmp/downloads"), + Overwrite: boolPtr(false), + }, + }, + }, } for _, tc := range tcs { t.Run(tc.desc, func(t *testing.T) { @@ -98,15 +125,27 @@ func TestParseFromYamlCloudStorageDownloadObject(t *testing.T) { } } +func strPtr(s string) *string { + return &s +} + +func boolPtr(b bool) *bool { + return &b +} + type mockSource struct { sources.Source called bool + gotBucket string + gotObject string gotDest string gotOverwrite bool } func (m *mockSource) DownloadObject(ctx context.Context, bucket, object, destination string, overwrite bool) (map[string]any, error) { m.called = true + m.gotBucket = bucket + m.gotObject = object m.gotDest = destination m.gotOverwrite = overwrite return map[string]any{"destination": destination, "bytes": int64(11), "contentType": "text/plain"}, nil @@ -195,3 +234,129 @@ func TestInvokeValidation(t *testing.T) { }) } } + +func TestConfiguredParametersHiddenAndForwarded(t *testing.T) { + destDir := t.TempDir() + cfg := cloudstoragedownloadobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "download_tool", + Description: "Download", + }, + Type: "cloud-storage-download-object", + Source: "my-gcs", + Bucket: strPtr("baked-bucket"), + DestinationDir: strPtr(destDir), + Overwrite: boolPtr(false), + } + tool, err := cfg.Initialize(context.Background()) + if err != nil { + t.Fatalf("failed to initialize tool: %v", err) + } + gotNames := manifestParamNames(tool.StaticManifest().Parameters) + wantNames := []string{"object", "destination"} + if diff := cmp.Diff(wantNames, gotNames); diff != "" { + t.Fatalf("manifest parameters mismatch (-want +got):\n%s", diff) + } + + src := &mockSource{} + params := parameters.ParamValues{ + {Name: "object", Value: "path/to/object.txt"}, + {Name: "destination", Value: filepath.Join("nested", "out.txt")}, + } + if _, err := tool.Invoke(context.Background(), &mockSourceProvider{source: src}, params, ""); err != nil { + t.Fatalf("unexpected error: %v", err) + } + wantDest := filepath.Join(destDir, "nested", "out.txt") + if src.gotBucket != "baked-bucket" || src.gotObject != "path/to/object.txt" || src.gotDest != wantDest || src.gotOverwrite { + t.Fatalf("forwarded params = bucket %q object %q dest %q overwrite %v, want baked-bucket/path/to/object.txt/%s/false", src.gotBucket, src.gotObject, src.gotDest, src.gotOverwrite, wantDest) + } +} + +func TestUnsetParametersRemainVisible(t *testing.T) { + cfg := cloudstoragedownloadobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "download_tool", + Description: "Download", + }, + Type: "cloud-storage-download-object", + Source: "my-gcs", + } + tool, err := cfg.Initialize(context.Background()) + if err != nil { + t.Fatalf("failed to initialize tool: %v", err) + } + gotNames := manifestParamNames(tool.StaticManifest().Parameters) + wantNames := []string{"bucket", "object", "destination", "overwrite"} + if diff := cmp.Diff(wantNames, gotNames); diff != "" { + t.Fatalf("manifest parameters mismatch (-want +got):\n%s", diff) + } +} + +func TestConfiguredParameterValidation(t *testing.T) { + tcs := []struct { + desc string + bucket *string + destDir *string + wantSubstr string + }{ + {desc: "empty bucket", bucket: strPtr(""), wantSubstr: "bucket"}, + {desc: "empty destination dir", destDir: strPtr(""), wantSubstr: "destination_dir"}, + {desc: "relative destination dir", destDir: strPtr("relative/dir"), wantSubstr: "destination_dir"}, + } + for _, tc := range tcs { + t.Run(tc.desc, func(t *testing.T) { + cfg := cloudstoragedownloadobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "download_tool", + Description: "Download", + }, + Type: "cloud-storage-download-object", + Source: "my-gcs", + Bucket: tc.bucket, + DestinationDir: tc.destDir, + } + if _, err := cfg.Initialize(context.Background()); err == nil || !strings.Contains(err.Error(), tc.wantSubstr) { + t.Fatalf("Initialize() error = %v, want %q", err, tc.wantSubstr) + } + }) + } +} + +func TestConfiguredDestinationDirRejectsEscapingDestination(t *testing.T) { + destDir := t.TempDir() + cfg := cloudstoragedownloadobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "download_tool", + Description: "Download", + }, + Type: "cloud-storage-download-object", + Source: "my-gcs", + Bucket: strPtr("baked-bucket"), + DestinationDir: strPtr(destDir), + } + tool, err := cfg.Initialize(context.Background()) + if err != nil { + t.Fatalf("failed to initialize tool: %v", err) + } + src := &mockSource{} + params := parameters.ParamValues{ + {Name: "object", Value: "o"}, + {Name: "destination", Value: filepath.Join("..", "escape.txt")}, + {Name: "overwrite", Value: false}, + } + _, toolErr := tool.Invoke(context.Background(), &mockSourceProvider{source: src}, params, "") + if toolErr == nil || !strings.Contains(toolErr.Error(), "destination") { + t.Fatalf("Invoke() error = %v, want destination error", toolErr) + } + if src.called { + t.Fatalf("source called despite invalid destination") + } +} + +func manifestParamNames(params []parameters.ParameterManifest) []string { + names := make([]string, 0, len(params)) + for _, p := range params { + names = append(names, p.Name) + } + return names +} diff --git a/internal/tools/cloudstorage/cloudstoragemoveobject/cloudstoragemoveobject.go b/internal/tools/cloudstorage/cloudstoragemoveobject/cloudstoragemoveobject.go index 43719712d02f..5767625368d4 100644 --- a/internal/tools/cloudstorage/cloudstoragemoveobject/cloudstoragemoveobject.go +++ b/internal/tools/cloudstorage/cloudstoragemoveobject/cloudstoragemoveobject.go @@ -57,6 +57,7 @@ type Config struct { Type string `yaml:"type" validate:"required"` Source string `yaml:"source" validate:"required"` Annotations *tools.ToolAnnotations `yaml:"annotations,omitempty"` + Bucket *string `yaml:"bucket,omitempty"` } var _ tools.ToolConfig = Config{} @@ -69,11 +70,17 @@ func (cfg Config) Initialize(context.Context) (tools.Tool, error) { if cfg.Description == "" { return nil, fmt.Errorf("description is required for tool %q", cfg.Name) } + if cfg.Bucket != nil && *cfg.Bucket == "" { + return nil, fmt.Errorf("bucket cannot be empty for tool %q", cfg.Name) + } - bucketParam := parameters.NewStringParameter(bucketKey, "Name of the Cloud Storage bucket containing the object to move.") sourceObjectParam := parameters.NewStringParameter(sourceObjectKey, "Full source object name (path) within the bucket, e.g. 'path/to/file.txt'.") destinationObjectParam := parameters.NewStringParameter(destinationObjectKey, "Full destination object name (path) within the same bucket, e.g. 'path/to/file.txt'.") - allParameters := parameters.Parameters{bucketParam, sourceObjectParam, destinationObjectParam} + allParameters := parameters.Parameters{} + if cfg.Bucket == nil { + allParameters = append(allParameters, parameters.NewStringParameter(bucketKey, "Name of the Cloud Storage bucket containing the object to move.")) + } + allParameters = append(allParameters, sourceObjectParam, destinationObjectParam) return Tool{ BaseTool: tools.NewBaseTool( @@ -102,8 +109,8 @@ func (t Tool) Invoke(ctx context.Context, resourceMgr tools.SourceProvider, para } mapParams := params.AsMap() - bucket, ok := mapParams[bucketKey].(string) - if !ok || bucket == "" { + bucket := cloudstoragecommon.ResolveString(t.Cfg.Bucket, mapParams, bucketKey) + if bucket == "" { return nil, util.NewAgentError(fmt.Sprintf("invalid or missing '%s' parameter; expected a non-empty string", bucketKey), nil) } sourceObject, ok := mapParams[sourceObjectKey].(string) diff --git a/internal/tools/cloudstorage/cloudstoragemoveobject/cloudstoragemoveobject_test.go b/internal/tools/cloudstorage/cloudstoragemoveobject/cloudstoragemoveobject_test.go index 5f317a060fd3..77a08d4e0542 100644 --- a/internal/tools/cloudstorage/cloudstoragemoveobject/cloudstoragemoveobject_test.go +++ b/internal/tools/cloudstorage/cloudstoragemoveobject/cloudstoragemoveobject_test.go @@ -83,6 +83,29 @@ func TestParseFromYamlCloudStorageMoveObject(t *testing.T) { }, }, }, + { + desc: "with configurable bucket", + in: ` + kind: tool + name: configured_move + type: cloud-storage-move-object + source: prod-gcs + description: Move configured object + bucket: baked-bucket + `, + want: server.ToolConfigs{ + "configured_move": cloudstoragemoveobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "configured_move", + Description: "Move configured object", + AuthRequired: []string{}, + }, + Type: "cloud-storage-move-object", + Source: "prod-gcs", + Bucket: strPtr("baked-bucket"), + }, + }, + }, } for _, tc := range tcs { t.Run(tc.desc, func(t *testing.T) { @@ -97,6 +120,10 @@ func TestParseFromYamlCloudStorageMoveObject(t *testing.T) { } } +func strPtr(s string) *string { + return &s +} + type mockSource struct { sources.Source called bool @@ -105,6 +132,82 @@ type mockSource struct { gotDestinationObject string } +func TestConfiguredBucketHiddenAndForwarded(t *testing.T) { + cfg := cloudstoragemoveobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "move_tool", + Description: "Move", + }, + Type: "cloud-storage-move-object", + Source: "my-gcs", + Bucket: strPtr("baked-bucket"), + } + tool, err := cfg.Initialize(context.Background()) + if err != nil { + t.Fatalf("failed to initialize tool: %v", err) + } + gotNames := manifestParamNames(tool.StaticManifest().Parameters) + wantNames := []string{"source_object", "destination_object"} + if diff := cmp.Diff(wantNames, gotNames); diff != "" { + t.Fatalf("manifest parameters mismatch (-want +got):\n%s", diff) + } + + src := &mockSource{} + params := parameters.ParamValues{ + {Name: "source_object", Value: "src"}, + {Name: "destination_object", Value: "dst"}, + } + if _, err := tool.Invoke(context.Background(), &mockSourceProvider{source: src}, params, ""); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if src.gotBucket != "baked-bucket" || src.gotSourceObject != "src" || src.gotDestinationObject != "dst" { + t.Fatalf("forwarded params = %q/%q/%q, want baked-bucket/src/dst", src.gotBucket, src.gotSourceObject, src.gotDestinationObject) + } +} + +func TestUnsetBucketRemainsVisible(t *testing.T) { + cfg := cloudstoragemoveobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "move_tool", + Description: "Move", + }, + Type: "cloud-storage-move-object", + Source: "my-gcs", + } + tool, err := cfg.Initialize(context.Background()) + if err != nil { + t.Fatalf("failed to initialize tool: %v", err) + } + gotNames := manifestParamNames(tool.StaticManifest().Parameters) + wantNames := []string{"bucket", "source_object", "destination_object"} + if diff := cmp.Diff(wantNames, gotNames); diff != "" { + t.Fatalf("manifest parameters mismatch (-want +got):\n%s", diff) + } +} + +func TestEmptyConfiguredBucketRejected(t *testing.T) { + cfg := cloudstoragemoveobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "move_tool", + Description: "Move", + }, + Type: "cloud-storage-move-object", + Source: "my-gcs", + Bucket: strPtr(""), + } + if _, err := cfg.Initialize(context.Background()); err == nil || !strings.Contains(err.Error(), "bucket") { + t.Fatalf("Initialize() error = %v, want bucket error", err) + } +} + +func manifestParamNames(params []parameters.ParameterManifest) []string { + names := make([]string, 0, len(params)) + for _, p := range params { + names = append(names, p.Name) + } + return names +} + func (m *mockSource) MoveObject(ctx context.Context, bucket, sourceObject, destinationObject string) (map[string]any, error) { m.called = true m.gotBucket = bucket diff --git a/internal/tools/cloudstorage/cloudstorageuploadobject/cloudstorageuploadobject.go b/internal/tools/cloudstorage/cloudstorageuploadobject/cloudstorageuploadobject.go index fcb66a7420dd..77b7dd02b09c 100644 --- a/internal/tools/cloudstorage/cloudstorageuploadobject/cloudstorageuploadobject.go +++ b/internal/tools/cloudstorage/cloudstorageuploadobject/cloudstorageuploadobject.go @@ -58,6 +58,7 @@ type Config struct { Type string `yaml:"type" validate:"required"` Source string `yaml:"source" validate:"required"` Annotations *tools.ToolAnnotations `yaml:"annotations,omitempty"` + Bucket *string `yaml:"bucket,omitempty"` } // validate interface @@ -71,12 +72,18 @@ func (cfg Config) Initialize(context.Context) (tools.Tool, error) { if cfg.Description == "" { return nil, fmt.Errorf("description is required for tool %q", cfg.Name) } + if cfg.Bucket != nil && *cfg.Bucket == "" { + return nil, fmt.Errorf("bucket cannot be empty for tool %q", cfg.Name) + } - bucketParam := parameters.NewStringParameter(bucketKey, "Name of the Cloud Storage bucket to upload into.") objectParam := parameters.NewStringParameter(objectKey, "Full object name (path) within the bucket, e.g. 'path/to/file.txt'.") sourceParam := parameters.NewStringParameter(sourceKey, "Absolute local filesystem path of the file to upload. Relative paths and paths containing '..' are rejected.") contentTypeParam := parameters.NewStringParameter(contentTypeKey, "MIME type to record on the uploaded object. When empty, it is inferred from the source file's extension; if that fails, Cloud Storage auto-detects from the first 512 bytes of content.", parameters.WithStringDefault("")) - allParameters := parameters.Parameters{bucketParam, objectParam, sourceParam, contentTypeParam} + allParameters := parameters.Parameters{} + if cfg.Bucket == nil { + allParameters = append(allParameters, parameters.NewStringParameter(bucketKey, "Name of the Cloud Storage bucket to upload into.")) + } + allParameters = append(allParameters, objectParam, sourceParam, contentTypeParam) return Tool{ BaseTool: tools.NewBaseTool( @@ -106,8 +113,8 @@ func (t Tool) Invoke(ctx context.Context, resourceMgr tools.SourceProvider, para } mapParams := params.AsMap() - bucket, ok := mapParams[bucketKey].(string) - if !ok || bucket == "" { + bucket := cloudstoragecommon.ResolveString(t.Cfg.Bucket, mapParams, bucketKey) + if bucket == "" { return nil, util.NewAgentError(fmt.Sprintf("invalid or missing '%s' parameter; expected a non-empty string", bucketKey), nil) } object, ok := mapParams[objectKey].(string) diff --git a/internal/tools/cloudstorage/cloudstorageuploadobject/cloudstorageuploadobject_test.go b/internal/tools/cloudstorage/cloudstorageuploadobject/cloudstorageuploadobject_test.go index ffeb23827532..4bcf2389c478 100644 --- a/internal/tools/cloudstorage/cloudstorageuploadobject/cloudstorageuploadobject_test.go +++ b/internal/tools/cloudstorage/cloudstorageuploadobject/cloudstorageuploadobject_test.go @@ -84,6 +84,29 @@ func TestParseFromYamlCloudStorageUploadObject(t *testing.T) { }, }, }, + { + desc: "with configurable bucket", + in: ` + kind: tool + name: configured_upload + type: cloud-storage-upload-object + source: prod-gcs + description: Upload configured object + bucket: baked-bucket + `, + want: server.ToolConfigs{ + "configured_upload": cloudstorageuploadobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "configured_upload", + Description: "Upload configured object", + AuthRequired: []string{}, + }, + Type: "cloud-storage-upload-object", + Source: "prod-gcs", + Bucket: strPtr("baked-bucket"), + }, + }, + }, } for _, tc := range tcs { t.Run(tc.desc, func(t *testing.T) { @@ -98,15 +121,23 @@ func TestParseFromYamlCloudStorageUploadObject(t *testing.T) { } } +func strPtr(s string) *string { + return &s +} + type mockSource struct { sources.Source called bool + gotBucket string + gotObject string gotSource string gotContentType string } func (m *mockSource) UploadObject(ctx context.Context, bucket, object, source, contentType string) (map[string]any, error) { m.called = true + m.gotBucket = bucket + m.gotObject = object m.gotSource = source m.gotContentType = contentType return map[string]any{"bucket": bucket, "object": object, "bytes": int64(0), "contentType": contentType}, nil @@ -197,3 +228,81 @@ func TestInvokeValidation(t *testing.T) { }) } } + +func TestConfiguredBucketHiddenAndForwarded(t *testing.T) { + cfg := cloudstorageuploadobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "upload_tool", + Description: "Upload", + }, + Type: "cloud-storage-upload-object", + Source: "my-gcs", + Bucket: strPtr("baked-bucket"), + } + tool, err := cfg.Initialize(context.Background()) + if err != nil { + t.Fatalf("failed to initialize tool: %v", err) + } + gotNames := manifestParamNames(tool.StaticManifest().Parameters) + wantNames := []string{"object", "source", "content_type"} + if diff := cmp.Diff(wantNames, gotNames); diff != "" { + t.Fatalf("manifest parameters mismatch (-want +got):\n%s", diff) + } + + localSource := filepath.Join(t.TempDir(), "in.csv") + src := &mockSource{} + params := parameters.ParamValues{ + {Name: "object", Value: "o"}, + {Name: "source", Value: localSource}, + {Name: "content_type", Value: "text/csv"}, + } + if _, err := tool.Invoke(context.Background(), &mockSourceProvider{source: src}, params, ""); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if src.gotBucket != "baked-bucket" || src.gotObject != "o" || src.gotSource != localSource || src.gotContentType != "text/csv" { + t.Fatalf("forwarded params = %q/%q/%q/%q, want baked-bucket/o/%s/text/csv", src.gotBucket, src.gotObject, src.gotSource, src.gotContentType, localSource) + } +} + +func TestUnsetBucketRemainsVisible(t *testing.T) { + cfg := cloudstorageuploadobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "upload_tool", + Description: "Upload", + }, + Type: "cloud-storage-upload-object", + Source: "my-gcs", + } + tool, err := cfg.Initialize(context.Background()) + if err != nil { + t.Fatalf("failed to initialize tool: %v", err) + } + gotNames := manifestParamNames(tool.StaticManifest().Parameters) + wantNames := []string{"bucket", "object", "source", "content_type"} + if diff := cmp.Diff(wantNames, gotNames); diff != "" { + t.Fatalf("manifest parameters mismatch (-want +got):\n%s", diff) + } +} + +func TestEmptyConfiguredBucketRejected(t *testing.T) { + cfg := cloudstorageuploadobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "upload_tool", + Description: "Upload", + }, + Type: "cloud-storage-upload-object", + Source: "my-gcs", + Bucket: strPtr(""), + } + if _, err := cfg.Initialize(context.Background()); err == nil || !strings.Contains(err.Error(), "bucket") { + t.Fatalf("Initialize() error = %v, want bucket error", err) + } +} + +func manifestParamNames(params []parameters.ParameterManifest) []string { + names := make([]string, 0, len(params)) + for _, p := range params { + names = append(names, p.Name) + } + return names +} diff --git a/internal/tools/cloudstorage/cloudstoragewriteobject/cloudstoragewriteobject.go b/internal/tools/cloudstorage/cloudstoragewriteobject/cloudstoragewriteobject.go index f1a616fe406f..5730ca9b5223 100644 --- a/internal/tools/cloudstorage/cloudstoragewriteobject/cloudstoragewriteobject.go +++ b/internal/tools/cloudstorage/cloudstoragewriteobject/cloudstoragewriteobject.go @@ -58,6 +58,7 @@ type Config struct { Type string `yaml:"type" validate:"required"` Source string `yaml:"source" validate:"required"` Annotations *tools.ToolAnnotations `yaml:"annotations,omitempty"` + Bucket *string `yaml:"bucket,omitempty"` } var _ tools.ToolConfig = Config{} @@ -70,12 +71,18 @@ func (cfg Config) Initialize(context.Context) (tools.Tool, error) { if cfg.Description == "" { return nil, fmt.Errorf("description is required for tool %q", cfg.Name) } + if cfg.Bucket != nil && *cfg.Bucket == "" { + return nil, fmt.Errorf("bucket cannot be empty for tool %q", cfg.Name) + } - bucketParam := parameters.NewStringParameter(bucketKey, "Name of the Cloud Storage bucket to write into.") objectParam := parameters.NewStringParameter(objectKey, "Full object name (path) within the bucket, e.g. 'path/to/file.txt'.") contentParam := parameters.NewStringParameter(contentKey, "Text content to write to the Cloud Storage object.") contentTypeParam := parameters.NewStringParameter(contentTypeKey, "MIME type to record on the written object. When empty, Cloud Storage auto-detects from the first 512 bytes of content.", parameters.WithStringDefault("")) - allParameters := parameters.Parameters{bucketParam, objectParam, contentParam, contentTypeParam} + allParameters := parameters.Parameters{} + if cfg.Bucket == nil { + allParameters = append(allParameters, parameters.NewStringParameter(bucketKey, "Name of the Cloud Storage bucket to write into.")) + } + allParameters = append(allParameters, objectParam, contentParam, contentTypeParam) return Tool{ BaseTool: tools.NewBaseTool( @@ -104,8 +111,8 @@ func (t Tool) Invoke(ctx context.Context, resourceMgr tools.SourceProvider, para } mapParams := params.AsMap() - bucket, ok := mapParams[bucketKey].(string) - if !ok || bucket == "" { + bucket := cloudstoragecommon.ResolveString(t.Cfg.Bucket, mapParams, bucketKey) + if bucket == "" { return nil, util.NewAgentError(fmt.Sprintf("invalid or missing '%s' parameter; expected a non-empty string", bucketKey), nil) } object, ok := mapParams[objectKey].(string) diff --git a/internal/tools/cloudstorage/cloudstoragewriteobject/cloudstoragewriteobject_test.go b/internal/tools/cloudstorage/cloudstoragewriteobject/cloudstoragewriteobject_test.go index 4a2d7f9a86e5..e723d0731705 100644 --- a/internal/tools/cloudstorage/cloudstoragewriteobject/cloudstoragewriteobject_test.go +++ b/internal/tools/cloudstorage/cloudstoragewriteobject/cloudstoragewriteobject_test.go @@ -83,6 +83,29 @@ func TestParseFromYamlCloudStorageWriteObject(t *testing.T) { }, }, }, + { + desc: "with configurable bucket", + in: ` + kind: tool + name: configured_write + type: cloud-storage-write-object + source: prod-gcs + description: Write configured object + bucket: baked-bucket + `, + want: server.ToolConfigs{ + "configured_write": cloudstoragewriteobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "configured_write", + Description: "Write configured object", + AuthRequired: []string{}, + }, + Type: "cloud-storage-write-object", + Source: "prod-gcs", + Bucket: strPtr("baked-bucket"), + }, + }, + }, } for _, tc := range tcs { t.Run(tc.desc, func(t *testing.T) { @@ -97,6 +120,10 @@ func TestParseFromYamlCloudStorageWriteObject(t *testing.T) { } } +func strPtr(s string) *string { + return &s +} + type mockSource struct { sources.Source called bool @@ -106,6 +133,83 @@ type mockSource struct { gotContentType string } +func TestConfiguredBucketHiddenAndForwarded(t *testing.T) { + cfg := cloudstoragewriteobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "write_tool", + Description: "Write", + }, + Type: "cloud-storage-write-object", + Source: "my-gcs", + Bucket: strPtr("baked-bucket"), + } + tool, err := cfg.Initialize(context.Background()) + if err != nil { + t.Fatalf("failed to initialize tool: %v", err) + } + gotNames := manifestParamNames(tool.StaticManifest().Parameters) + wantNames := []string{"object", "content", "content_type"} + if diff := cmp.Diff(wantNames, gotNames); diff != "" { + t.Fatalf("manifest parameters mismatch (-want +got):\n%s", diff) + } + + src := &mockSource{} + params := parameters.ParamValues{ + {Name: "object", Value: "o"}, + {Name: "content", Value: "body"}, + {Name: "content_type", Value: "text/plain"}, + } + if _, err := tool.Invoke(context.Background(), &mockSourceProvider{source: src}, params, ""); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if src.gotBucket != "baked-bucket" || src.gotObject != "o" || src.gotContent != "body" || src.gotContentType != "text/plain" { + t.Fatalf("forwarded params = %q/%q/%q/%q, want baked-bucket/o/body/text/plain", src.gotBucket, src.gotObject, src.gotContent, src.gotContentType) + } +} + +func TestUnsetBucketRemainsVisible(t *testing.T) { + cfg := cloudstoragewriteobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "write_tool", + Description: "Write", + }, + Type: "cloud-storage-write-object", + Source: "my-gcs", + } + tool, err := cfg.Initialize(context.Background()) + if err != nil { + t.Fatalf("failed to initialize tool: %v", err) + } + gotNames := manifestParamNames(tool.StaticManifest().Parameters) + wantNames := []string{"bucket", "object", "content", "content_type"} + if diff := cmp.Diff(wantNames, gotNames); diff != "" { + t.Fatalf("manifest parameters mismatch (-want +got):\n%s", diff) + } +} + +func TestEmptyConfiguredBucketRejected(t *testing.T) { + cfg := cloudstoragewriteobject.Config{ + ConfigBase: tools.ConfigBase{ + Name: "write_tool", + Description: "Write", + }, + Type: "cloud-storage-write-object", + Source: "my-gcs", + Bucket: strPtr(""), + } + if _, err := cfg.Initialize(context.Background()); err == nil || !strings.Contains(err.Error(), "bucket") { + t.Fatalf("Initialize() error = %v, want bucket error", err) + } +} + +func manifestParamNames(params []parameters.ParameterManifest) []string { + names := make([]string, 0, len(params)) + for _, p := range params { + names = append(names, p.Name) + } + return names +} + func (m *mockSource) WriteObject(ctx context.Context, bucket, object, content, contentType string) (map[string]any, error) { m.called = true m.gotBucket = bucket diff --git a/tests/cloudstorage/cloud_storage_integration_test.go b/tests/cloudstorage/cloud_storage_integration_test.go index 1cce317006fe..17939ac32611 100644 --- a/tests/cloudstorage/cloud_storage_integration_test.go +++ b/tests/cloudstorage/cloud_storage_integration_test.go @@ -83,7 +83,8 @@ func TestCloudStorageToolEndpoints(t *testing.T) { teardown := setupCloudStorageTestData(t, ctx, client, CloudStorageProject, bucketName) defer teardown(t) - toolsFile := getCloudStorageToolsConfig(sourceConfig, bucketName) + configuredDownloadDir := t.TempDir() + toolsFile := getCloudStorageToolsConfig(sourceConfig, bucketName, configuredDownloadDir) cmd, cleanup, err := tests.StartCmd(ctx, toolsFile, "--enable-api") if err != nil { @@ -681,12 +682,165 @@ func TestCloudStorageToolEndpoints(t *testing.T) { }, }, ) + tests.RunToolGetTestByName(t, "my_download_object_configured", + map[string]any{ + "my_download_object_configured": map[string]any{ + "description": "Download a Cloud Storage object with configured storage settings.", + "authRequired": []any{}, + "parameters": []any{ + map[string]any{ + "authServices": []any{}, + "description": "Full object name (path) within the bucket, e.g. 'path/to/file.txt'.", + "name": "object", + "required": true, + "type": "string", + }, + map[string]any{ + "authServices": []any{}, + "description": "Relative path under the configured destination_dir where the object will be written. Absolute paths and paths that escape destination_dir are rejected.", + "name": "destination", + "required": true, + "type": "string", + }, + }, + }, + }, + ) + tests.RunToolGetTestByName(t, "my_upload_object_configured", + map[string]any{ + "my_upload_object_configured": map[string]any{ + "description": "Upload a local file to a configured Cloud Storage bucket.", + "authRequired": []any{}, + "parameters": []any{ + map[string]any{ + "authServices": []any{}, + "description": "Full object name (path) within the bucket, e.g. 'path/to/file.txt'.", + "name": "object", + "required": true, + "type": "string", + }, + map[string]any{ + "authServices": []any{}, + "description": "Absolute local filesystem path of the file to upload. Relative paths and paths containing '..' are rejected.", + "name": "source", + "required": true, + "type": "string", + }, + map[string]any{ + "authServices": []any{}, + "description": "MIME type to record on the uploaded object. When empty, it is inferred from the source file's extension; if that fails, Cloud Storage auto-detects from the first 512 bytes of content.", + "name": "content_type", + "required": false, + "type": "string", + "default": "", + }, + }, + }, + }, + ) + tests.RunToolGetTestByName(t, "my_write_object_configured", + map[string]any{ + "my_write_object_configured": map[string]any{ + "description": "Write text content to a configured Cloud Storage bucket.", + "authRequired": []any{}, + "parameters": []any{ + map[string]any{ + "authServices": []any{}, + "description": "Full object name (path) within the bucket, e.g. 'path/to/file.txt'.", + "name": "object", + "required": true, + "type": "string", + }, + map[string]any{ + "authServices": []any{}, + "description": "Text content to write to the Cloud Storage object.", + "name": "content", + "required": true, + "type": "string", + }, + map[string]any{ + "authServices": []any{}, + "description": "MIME type to record on the written object. When empty, Cloud Storage auto-detects from the first 512 bytes of content.", + "name": "content_type", + "required": false, + "type": "string", + "default": "", + }, + }, + }, + }, + ) + tests.RunToolGetTestByName(t, "my_copy_object_configured", + map[string]any{ + "my_copy_object_configured": map[string]any{ + "description": "Copy a Cloud Storage object between configured buckets.", + "authRequired": []any{}, + "parameters": []any{ + map[string]any{ + "authServices": []any{}, + "description": "Full source object name (path) within the source bucket, e.g. 'path/to/file.txt'.", + "name": "source_object", + "required": true, + "type": "string", + }, + map[string]any{ + "authServices": []any{}, + "description": "Full destination object name (path) within the destination bucket, e.g. 'path/to/file.txt'.", + "name": "destination_object", + "required": true, + "type": "string", + }, + }, + }, + }, + ) + tests.RunToolGetTestByName(t, "my_move_object_configured", + map[string]any{ + "my_move_object_configured": map[string]any{ + "description": "Move a Cloud Storage object within a configured bucket.", + "authRequired": []any{}, + "parameters": []any{ + map[string]any{ + "authServices": []any{}, + "description": "Full source object name (path) within the bucket, e.g. 'path/to/file.txt'.", + "name": "source_object", + "required": true, + "type": "string", + }, + map[string]any{ + "authServices": []any{}, + "description": "Full destination object name (path) within the same bucket, e.g. 'path/to/file.txt'.", + "name": "destination_object", + "required": true, + "type": "string", + }, + }, + }, + }, + ) + tests.RunToolGetTestByName(t, "my_delete_object_configured", + map[string]any{ + "my_delete_object_configured": map[string]any{ + "description": "Delete an object from a configured Cloud Storage bucket.", + "authRequired": []any{}, + "parameters": []any{ + map[string]any{ + "authServices": []any{}, + "description": "Full object name (path) within the bucket, e.g. 'path/to/file.txt'.", + "name": "object", + "required": true, + "type": "string", + }, + }, + }, + }, + ) runCloudStorageListObjectsTest(t, bucketName) runCloudStorageReadObjectTest(t, bucketName) runCloudStorageListBucketsTest(t, bucketName) runCloudStorageGetObjectMetadataTest(t, bucketName) - runCloudStorageConfiguredParamsTest(ctx, t, client, bucketName) + runCloudStorageConfiguredParamsTest(ctx, t, client, bucketName, configuredDownloadDir) bucketToolName := "toolbox-it-bucket-" + strings.ReplaceAll(uuid.New().String(), "-", "")[:17] defer cleanupGCSBucket(ctx, t, client, bucketToolName) runCloudStorageCreateBucketTest(ctx, t, client, bucketToolName) @@ -701,7 +855,7 @@ func TestCloudStorageToolEndpoints(t *testing.T) { runCloudStorageDeleteObjectTest(ctx, t, client, bucketName) } -func getCloudStorageToolsConfig(sourceConfig map[string]any, bucketName string) map[string]any { +func getCloudStorageToolsConfig(sourceConfig map[string]any, bucketName, configuredDownloadDir string) map[string]any { return map[string]any{ "sources": map[string]any{ "my_instance": sourceConfig, @@ -827,6 +981,45 @@ func getCloudStorageToolsConfig(sourceConfig map[string]any, bucketName string) "location": "US", "uniform_bucket_level_access": true, }, + "my_download_object_configured": map[string]any{ + "type": "cloud-storage-download-object", + "source": "my_instance", + "description": "Download a Cloud Storage object with configured storage settings.", + "bucket": bucketName, + "destination_dir": configuredDownloadDir, + "overwrite": true, + }, + "my_upload_object_configured": map[string]any{ + "type": "cloud-storage-upload-object", + "source": "my_instance", + "description": "Upload a local file to a configured Cloud Storage bucket.", + "bucket": bucketName, + }, + "my_write_object_configured": map[string]any{ + "type": "cloud-storage-write-object", + "source": "my_instance", + "description": "Write text content to a configured Cloud Storage bucket.", + "bucket": bucketName, + }, + "my_copy_object_configured": map[string]any{ + "type": "cloud-storage-copy-object", + "source": "my_instance", + "description": "Copy a Cloud Storage object between configured buckets.", + "source_bucket": bucketName, + "destination_bucket": bucketName, + }, + "my_move_object_configured": map[string]any{ + "type": "cloud-storage-move-object", + "source": "my_instance", + "description": "Move a Cloud Storage object within a configured bucket.", + "bucket": bucketName, + }, + "my_delete_object_configured": map[string]any{ + "type": "cloud-storage-delete-object", + "source": "my_instance", + "description": "Delete an object from a configured Cloud Storage bucket.", + "bucket": bucketName, + }, }, } } @@ -1379,7 +1572,7 @@ func runCloudStorageGetObjectMetadataTest(t *testing.T, bucket string) { // parameters (they are absent from the schema) and relies on the configured // values instead. The seeded `bucket` is the configured bucket for the // bucket-scoped tools. -func runCloudStorageConfiguredParamsTest(ctx context.Context, t *testing.T, client *storage.Client, bucket string) { +func runCloudStorageConfiguredParamsTest(ctx context.Context, t *testing.T, client *storage.Client, bucket, configuredDownloadDir string) { t.Run("list_objects uses configured bucket and prefix", func(t *testing.T) { result, status := invokeTool(t, "my_list_objects_configured", `{}`) if status != http.StatusOK { @@ -1471,6 +1664,114 @@ func runCloudStorageConfiguredParamsTest(ctx context.Context, t *testing.T, clie t.Errorf("expected uniform bucket-level access to be enabled") } }) + + t.Run("download_object uses configured bucket destination_dir and overwrite", func(t *testing.T) { + relDest := filepath.Join("configured", "downloaded.txt") + dest := filepath.Join(configuredDownloadDir, relDest) + if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil { + t.Fatalf("setup: %v", err) + } + if err := os.WriteFile(dest, []byte("pre-existing"), 0o644); err != nil { + t.Fatalf("setup: %v", err) + } + body := fmt.Sprintf(`{"object": %q, "destination": %q}`, downloadObject, relDest) + result, status := invokeTool(t, "my_download_object_configured", body) + if status != http.StatusOK { + t.Fatalf("unexpected status %d: %s", status, result) + } + got, err := os.ReadFile(dest) + if err != nil { + t.Fatalf("failed to read downloaded file: %v", err) + } + if string(got) != downloadBody { + t.Errorf("downloaded content = %q, want %q (raw %s)", string(got), downloadBody, result) + } + }) + + t.Run("upload_object uses configured bucket", func(t *testing.T) { + srcPath := filepath.Join(t.TempDir(), "configured-upload.txt") + content := "configured upload" + if err := os.WriteFile(srcPath, []byte(content), 0o644); err != nil { + t.Fatalf("setup: %v", err) + } + obj := "configured/upload.txt" + body := fmt.Sprintf(`{"object": %q, "source": %q, "content_type": "text/plain"}`, obj, srcPath) + result, status := invokeTool(t, "my_upload_object_configured", body) + if status != http.StatusOK { + t.Fatalf("unexpected status %d: %s", status, result) + } + got, attrs := readGCSObject(t, ctx, client, bucket, obj) + if got != content { + t.Errorf("uploaded content = %q, want %q", got, content) + } + if attrs.ContentType != "text/plain" { + t.Errorf("GCS ContentType = %q, want text/plain (raw %s)", attrs.ContentType, result) + } + }) + + t.Run("write_object uses configured bucket", func(t *testing.T) { + obj := "configured/write.txt" + content := "configured write" + body := fmt.Sprintf(`{"object": %q, "content": %q, "content_type": "text/plain"}`, obj, content) + result, status := invokeTool(t, "my_write_object_configured", body) + if status != http.StatusOK { + t.Fatalf("unexpected status %d: %s", status, result) + } + got, attrs := readGCSObject(t, ctx, client, bucket, obj) + if got != content { + t.Errorf("written content = %q, want %q", got, content) + } + if attrs.ContentType != "text/plain" { + t.Errorf("GCS ContentType = %q, want text/plain (raw %s)", attrs.ContentType, result) + } + }) + + t.Run("copy_object uses configured buckets", func(t *testing.T) { + dest := "configured/copied.txt" + body := fmt.Sprintf(`{"source_object": %q, "destination_object": %q}`, helloObject, dest) + result, status := invokeTool(t, "my_copy_object_configured", body) + if status != http.StatusOK { + t.Fatalf("unexpected status %d: %s", status, result) + } + got, _ := readGCSObject(t, ctx, client, bucket, dest) + if got != helloBody { + t.Errorf("copied content = %q, want %q (raw %s)", got, helloBody, result) + } + }) + + t.Run("move_object uses configured bucket", func(t *testing.T) { + src := "configured/move-source.txt" + dest := "configured/move-destination.txt" + writeGCSObject(t, ctx, client, bucket, src, "text/plain", "configured move") + body := fmt.Sprintf(`{"source_object": %q, "destination_object": %q}`, src, dest) + result, status := invokeTool(t, "my_move_object_configured", body) + if status != http.StatusOK { + t.Fatalf("unexpected status %d: %s", status, result) + } + got, _ := readGCSObject(t, ctx, client, bucket, dest) + if got != "configured move" { + t.Errorf("moved content = %q, want configured move (raw %s)", got, result) + } + if gcsObjectExists(t, ctx, client, bucket, src) { + t.Errorf("expected source object %q to be gone after move", src) + } + }) + + t.Run("delete_object uses configured bucket", func(t *testing.T) { + obj := "configured/delete.txt" + writeGCSObject(t, ctx, client, bucket, obj, "text/plain", "configured delete") + body := fmt.Sprintf(`{"object": %q}`, obj) + result, status := invokeTool(t, "my_delete_object_configured", body) + if status != http.StatusOK { + t.Fatalf("unexpected status %d: %s", status, result) + } + if !strings.Contains(result, `"deleted":true`) { + t.Errorf("expected deleted confirmation, got %s", result) + } + if gcsObjectExists(t, ctx, client, bucket, obj) { + t.Errorf("expected object %q to be deleted", obj) + } + }) } func runCloudStorageDownloadObjectTest(t *testing.T, bucket string) {