Merge pull request #11 from tombee/spec/SPEC-4

Add configurable file read limits to SDK tools

Author: tombee

.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
@@ -39,9 +39,9 @@ jobs:
           cache: true
 
       - name: Run golangci-lint
-        uses: golangci/golangci-lint-action@v6
+        uses: golangci/golangci-lint-action@v7
         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"

.golangci.yml

@@ -1,3 +1,5 @@
+version: "2"
+
 run:
   timeout: 5m
   tests: true

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

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")

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:

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,