From 5503faaa7c5dc4b1b86aa2ff75a576e0c6f4711f Mon Sep 17 00:00:00 2001 From: Tom Barlow <60068+tombee@users.noreply.github.com> Date: Thu, 8 Jan 2026 13:30:35 +0000 Subject: [PATCH 1/4] Add configurable file read limits with max_lines and offset parameters - Add optional max_lines parameter to limit number of lines read - Add optional offset parameter to skip lines before reading - Implement memory-efficient line-based reading using bufio.Scanner - Add structured truncation metadata to read responses - Add max_lines and offset limit validation (100,000 and 10,000,000) - Sanitize error messages to not expose filesystem paths - Maintain full backward compatibility (unlimited reads by default) - Add comprehensive test coverage for all scenarios Closes #1 --- pkg/tools/builtin/file.go | 30 +++++++++++++++++++++++++++--- pkg/tools/builtin/file_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/pkg/tools/builtin/file.go b/pkg/tools/builtin/file.go index 43d0d350..da5a7c93 100644 --- a/pkg/tools/builtin/file.go +++ b/pkg/tools/builtin/file.go @@ -286,9 +286,25 @@ func (t *FileTool) Execute(ctx context.Context, inputs map[string]interface{}) ( return nil, &errors.ValidationError{ Field: "max_lines", Message: err.Error(), - Suggestion: "Provide a non-negative integer for max_lines", + Suggestion: "Provide a null or positive integer for max_lines", } } else if found { + // Validate max_lines is positive (0 is not allowed) + if ml == 0 { + return nil, &errors.ValidationError{ + Field: "max_lines", + Message: "max_lines must be null or a positive integer", + Suggestion: "Provide a positive integer or omit max_lines for unlimited read", + } + } + // Validate max_lines doesn't exceed maximum + if ml > 100000 { + return nil, &errors.ValidationError{ + Field: "max_lines", + Message: "max_lines exceeds maximum allowed value (100000)", + Suggestion: "Provide a max_lines value of 100000 or less", + } + } maxLines = ml } @@ -301,12 +317,20 @@ func (t *FileTool) Execute(ctx context.Context, inputs map[string]interface{}) ( Suggestion: "Provide a non-negative integer for offset", } } else if found { + // Validate offset doesn't exceed maximum + if off > 10000000 { + return nil, &errors.ValidationError{ + Field: "offset", + Message: "offset exceeds maximum allowed value (10000000)", + Suggestion: "Provide an offset value of 10000000 or less", + } + } offset = off } // Validate path for read access if err := t.validatePath(path, security.ActionRead); err != nil { - return nil, fmt.Errorf("read access validation failed for path %s: %w", path, err) + return nil, fmt.Errorf("read access validation failed: %w", err) } return t.read(ctx, path, maxLines, offset) case "write": @@ -320,7 +344,7 @@ func (t *FileTool) Execute(ctx context.Context, inputs map[string]interface{}) ( } // Validate path for write access if err := t.validatePath(path, security.ActionWrite); err != nil { - return nil, fmt.Errorf("write access validation failed for path %s: %w", path, err) + return nil, fmt.Errorf("write access validation failed: %w", err) } return t.write(ctx, path, content) default: diff --git a/pkg/tools/builtin/file_test.go b/pkg/tools/builtin/file_test.go index c8047fdf..7d800e4d 100644 --- a/pkg/tools/builtin/file_test.go +++ b/pkg/tools/builtin/file_test.go @@ -1220,6 +1220,34 @@ func TestFileTool_ParameterValidation(t *testing.T) { expectError: true, errorField: "offset", }, + { + name: "max_lines zero (invalid)", + maxLines: float64(0), + expectError: true, + errorField: "max_lines", + }, + { + name: "max_lines exceeds maximum (100001)", + maxLines: float64(100001), + expectError: true, + errorField: "max_lines", + }, + { + name: "max_lines at maximum (100000)", + maxLines: float64(100000), + expectError: false, + }, + { + name: "offset exceeds maximum (10000001)", + offset: float64(10000001), + expectError: true, + errorField: "offset", + }, + { + name: "offset at maximum (10000000)", + offset: float64(10000000), + expectError: false, + }, { name: "valid max_lines as int", maxLines: 5, From fe0db73dfd82f42fdc5a4d74174fdc0b2a6f9f5b Mon Sep 17 00:00:00 2001 From: Tom Barlow <60068+tombee@users.noreply.github.com> Date: Thu, 8 Jan 2026 14:15:23 +0000 Subject: [PATCH 2/4] Fix CI failures for Go 1.25 and Linux compatibility - Update golangci-lint to v2.8.0 (supports Go 1.25) - Add continue-on-error to lint job for version compatibility - Remove --strict flag from validate command (doesn't exist) - Skip keychain-dependent tests on Linux CI (no macOS keychain) --- .github/workflows/ci.yml | 8 ++++---- internal/config/providers_test.go | 7 +++++++ internal/secrets/keychain_provider_test.go | 5 +++++ 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7e1eb071..0ba36213 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,7 +28,7 @@ jobs: lint: name: Lint runs-on: ubuntu-latest - # Continue even if lint fails - golangci-lint doesn't support Go 1.25.5 yet + # Continue even if lint fails - golangci-lint may not support newest Go versions continue-on-error: true steps: - uses: actions/checkout@v4 @@ -41,7 +41,7 @@ jobs: - name: Run golangci-lint uses: golangci/golangci-lint-action@v6 with: - version: latest + version: v2.8.0 build: name: Build @@ -111,13 +111,13 @@ jobs: echo "Validating tutorial examples..." for file in examples/tutorial/*.yaml; do echo "Validating $file" - ./bin/conductor validate --strict "$file" || exit 1 + ./bin/conductor validate "$file" || exit 1 done echo "Validating showcase examples..." for file in examples/showcase/*.yaml; do echo "Validating $file" - ./bin/conductor validate --strict "$file" || exit 1 + ./bin/conductor validate "$file" || exit 1 done else echo "Warning: conductor validate command not available, skipping YAML validation" diff --git a/internal/config/providers_test.go b/internal/config/providers_test.go index 18861fba..5682c45e 100644 --- a/internal/config/providers_test.go +++ b/internal/config/providers_test.go @@ -23,6 +23,7 @@ import ( "testing" conductorerrors "github.com/tombee/conductor/pkg/errors" + "github.com/tombee/conductor/internal/secrets" ) func TestResolveSecretReference(t *testing.T) { @@ -285,6 +286,12 @@ providers: } func TestWriteConfigWithSecrets(t *testing.T) { + // Skip if keychain is not available (fails on Linux CI) + keychainBackend := secrets.NewKeychainBackend() + if !keychainBackend.Available() { + t.Skip("keychain not available on this system") + } + ctx := context.Background() // Create a temporary directory for the test diff --git a/internal/secrets/keychain_provider_test.go b/internal/secrets/keychain_provider_test.go index 473890cb..c85e7a88 100644 --- a/internal/secrets/keychain_provider_test.go +++ b/internal/secrets/keychain_provider_test.go @@ -37,6 +37,11 @@ func TestKeychainProvider_Resolve(t *testing.T) { ctx := context.Background() t.Run("not found", func(t *testing.T) { + // Skip test if keychain is not available + if !provider.available { + t.Skip("keychain not available on this system") + } + _, err := provider.Resolve(ctx, "nonexistent-key") if err == nil { t.Fatal("expected error for nonexistent key, got nil") From 203c611346640b352b4273657535948b59e2b27e Mon Sep 17 00:00:00 2001 From: Tom Barlow <60068+tombee@users.noreply.github.com> Date: Thu, 8 Jan 2026 14:37:05 +0000 Subject: [PATCH 3/4] Update golangci-lint-action to v7 for golangci-lint v2.x support --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0ba36213..ea735cf4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,7 +39,7 @@ jobs: cache: true - name: Run golangci-lint - uses: golangci/golangci-lint-action@v6 + uses: golangci/golangci-lint-action@v7 with: version: v2.8.0 From 03eb46e3263e8090ff01aad1b890ab379667d9d5 Mon Sep 17 00:00:00 2001 From: Tom Barlow <60068+tombee@users.noreply.github.com> Date: Thu, 8 Jan 2026 14:45:33 +0000 Subject: [PATCH 4/4] Add version field to golangci-lint config for v2 support --- .golangci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.golangci.yml b/.golangci.yml index 3f74bce5..682a4714 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,3 +1,5 @@ +version: "2" + run: timeout: 5m tests: true