Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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. |
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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. |
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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:
Expand All @@ -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. |
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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. |
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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. |
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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. |
23 changes: 23 additions & 0 deletions internal/tools/cloudstorage/cloudstoragecommon/paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
39 changes: 39 additions & 0 deletions internal/tools/cloudstorage/cloudstoragecommon/paths_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand All @@ -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(
Expand Down Expand Up @@ -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)
Expand Down
Loading
Loading