From dc966d53c6a753dac2f6d022d2e127860f809125 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 30 Jan 2026 06:08:52 +0000 Subject: [PATCH 1/2] Initial plan From 038f8dba0e8fa536de382ae37161b8df2c59c44c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 30 Jan 2026 06:17:16 +0000 Subject: [PATCH 2/2] Add comprehensive tests and documentation for pass-through field validation Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../schema_passthrough_validation_test.go | 485 ++++++++++++++++++ pkg/parser/schema_validation.go | 10 + pkg/workflow/validation.go | 22 + 3 files changed, 517 insertions(+) create mode 100644 pkg/parser/schema_passthrough_validation_test.go diff --git a/pkg/parser/schema_passthrough_validation_test.go b/pkg/parser/schema_passthrough_validation_test.go new file mode 100644 index 0000000000..805ccd77c2 --- /dev/null +++ b/pkg/parser/schema_passthrough_validation_test.go @@ -0,0 +1,485 @@ +//go:build !integration + +package parser + +import ( + "strings" + "testing" +) + +// TestPassThroughFieldValidation tests that pass-through YAML fields +// (concurrency, container, environment, env, runs-on, services) are +// properly validated by the schema during frontmatter parsing. +// +// These fields are "pass-through" in that they are extracted from +// frontmatter and passed directly to GitHub Actions without modification, +// but they still need basic structure validation to catch obvious errors +// at compile time rather than at GitHub Actions runtime. +func TestPassThroughFieldValidation(t *testing.T) { + tests := []struct { + name string + frontmatter map[string]any + wantErr bool + errContains string + }{ + // Concurrency field tests + { + name: "valid concurrency - simple string", + frontmatter: map[string]any{ + "on": "push", + "concurrency": "my-group", + }, + wantErr: false, + }, + { + name: "valid concurrency - object with group", + frontmatter: map[string]any{ + "on": "push", + "concurrency": map[string]any{ + "group": "my-group", + }, + }, + wantErr: false, + }, + { + name: "valid concurrency - object with group and cancel-in-progress", + frontmatter: map[string]any{ + "on": "push", + "concurrency": map[string]any{ + "group": "my-group", + "cancel-in-progress": true, + }, + }, + wantErr: false, + }, + { + name: "invalid concurrency - array", + frontmatter: map[string]any{ + "on": "push", + "concurrency": []string{"invalid"}, + }, + wantErr: true, + errContains: "oneOf", + }, + { + name: "invalid concurrency - object missing required group", + frontmatter: map[string]any{ + "on": "push", + "concurrency": map[string]any{ + "cancel-in-progress": true, + }, + }, + wantErr: true, + errContains: "missing property 'group'", + }, + { + name: "invalid concurrency - object with invalid field", + frontmatter: map[string]any{ + "on": "push", + "concurrency": map[string]any{ + "group": "my-group", + "invalid": "field", + }, + }, + wantErr: true, + errContains: "additional properties 'invalid' not allowed", + }, + + // Container field tests + { + name: "valid container - simple string", + frontmatter: map[string]any{ + "on": "push", + "container": "ubuntu:latest", + }, + wantErr: false, + }, + { + name: "valid container - object with image", + frontmatter: map[string]any{ + "on": "push", + "container": map[string]any{ + "image": "ubuntu:latest", + }, + }, + wantErr: false, + }, + { + name: "valid container - object with image and credentials", + frontmatter: map[string]any{ + "on": "push", + "container": map[string]any{ + "image": "ubuntu:latest", + "credentials": map[string]any{ + "username": "user", + "password": "${{ secrets.PASSWORD }}", + }, + }, + }, + wantErr: false, + }, + { + name: "invalid container - array", + frontmatter: map[string]any{ + "on": "push", + "container": []string{"invalid"}, + }, + wantErr: true, + errContains: "got array", + }, + { + name: "invalid container - object missing required image", + frontmatter: map[string]any{ + "on": "push", + "container": map[string]any{ + "env": map[string]string{"TEST": "value"}, + }, + }, + wantErr: true, + errContains: "missing property 'image'", + }, + { + name: "invalid container - object with invalid field", + frontmatter: map[string]any{ + "on": "push", + "container": map[string]any{ + "image": "ubuntu:latest", + "invalid": "field", + }, + }, + wantErr: true, + errContains: "additional properties 'invalid' not allowed", + }, + + // Environment field tests + { + name: "valid environment - simple string", + frontmatter: map[string]any{ + "on": "push", + "environment": "production", + }, + wantErr: false, + }, + { + name: "valid environment - object with name", + frontmatter: map[string]any{ + "on": "push", + "environment": map[string]any{ + "name": "production", + }, + }, + wantErr: false, + }, + { + name: "valid environment - object with name and url", + frontmatter: map[string]any{ + "on": "push", + "environment": map[string]any{ + "name": "production", + "url": "https://prod.example.com", + }, + }, + wantErr: false, + }, + { + name: "invalid environment - array", + frontmatter: map[string]any{ + "on": "push", + "environment": []string{"invalid"}, + }, + wantErr: true, + errContains: "oneOf", + }, + { + name: "invalid environment - object missing required name", + frontmatter: map[string]any{ + "on": "push", + "environment": map[string]any{ + "url": "https://prod.example.com", + }, + }, + wantErr: true, + errContains: "missing property 'name'", + }, + { + name: "invalid environment - object with invalid field", + frontmatter: map[string]any{ + "on": "push", + "environment": map[string]any{ + "name": "production", + "invalid": "field", + }, + }, + wantErr: true, + errContains: "additional properties 'invalid' not allowed", + }, + + // Env field tests + { + name: "valid env - object with string values", + frontmatter: map[string]any{ + "on": "push", + "env": map[string]any{ + "NODE_ENV": "production", + "API_KEY": "${{ secrets.API_KEY }}", + }, + }, + wantErr: false, + }, + { + name: "valid env - string (pass-through)", + frontmatter: map[string]any{ + "on": "push", + "env": "some-string", + }, + wantErr: false, + }, + { + name: "invalid env - array", + frontmatter: map[string]any{ + "on": "push", + "env": []string{"invalid"}, + }, + wantErr: true, + errContains: "oneOf", + }, + + // Runs-on field tests + { + name: "valid runs-on - simple string", + frontmatter: map[string]any{ + "on": "push", + "runs-on": "ubuntu-latest", + }, + wantErr: false, + }, + { + name: "valid runs-on - array of strings", + frontmatter: map[string]any{ + "on": "push", + "runs-on": []string{"ubuntu-latest", "self-hosted"}, + }, + wantErr: false, + }, + { + name: "valid runs-on - object with labels", + frontmatter: map[string]any{ + "on": "push", + "runs-on": map[string]any{ + "labels": []string{"ubuntu-latest"}, + }, + }, + wantErr: false, + }, + { + name: "valid runs-on - object with group and labels", + frontmatter: map[string]any{ + "on": "push", + "runs-on": map[string]any{ + "group": "larger-runners", + "labels": []string{"ubuntu-latest-8-cores"}, + }, + }, + wantErr: false, + }, + { + name: "invalid runs-on - number", + frontmatter: map[string]any{ + "on": "push", + "runs-on": 123, + }, + wantErr: true, + errContains: "oneOf", + }, + { + name: "invalid runs-on - object with invalid field", + frontmatter: map[string]any{ + "on": "push", + "runs-on": map[string]any{ + "labels": []string{"ubuntu-latest"}, + "invalid": "field", + }, + }, + wantErr: true, + errContains: "additional properties 'invalid' not allowed", + }, + + // Services field tests + { + name: "valid services - object with service names", + frontmatter: map[string]any{ + "on": "push", + "services": map[string]any{ + "postgres": "postgres:14", + }, + }, + wantErr: false, + }, + { + name: "valid services - object with service configuration", + frontmatter: map[string]any{ + "on": "push", + "services": map[string]any{ + "postgres": map[string]any{ + "image": "postgres:14", + "env": map[string]any{ + "POSTGRES_PASSWORD": "${{ secrets.DB_PASSWORD }}", + }, + }, + }, + }, + wantErr: false, + }, + { + name: "invalid services - string", + frontmatter: map[string]any{ + "on": "push", + "services": "invalid", + }, + wantErr: true, + errContains: "got string, want object", + }, + { + name: "invalid services - array", + frontmatter: map[string]any{ + "on": "push", + "services": []string{"invalid"}, + }, + wantErr: true, + errContains: "got array, want object", + }, + { + name: "invalid services - service object missing required image", + frontmatter: map[string]any{ + "on": "push", + "services": map[string]any{ + "postgres": map[string]any{ + "env": map[string]any{ + "POSTGRES_PASSWORD": "secret", + }, + }, + }, + }, + wantErr: true, + errContains: "missing property 'image'", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateMainWorkflowFrontmatterWithSchema(tt.frontmatter) + + if tt.wantErr && err == nil { + t.Errorf("Expected error but got none") + return + } + + if !tt.wantErr && err != nil { + t.Errorf("Unexpected error: %v", err) + return + } + + if tt.wantErr && err != nil && tt.errContains != "" { + if !strings.Contains(err.Error(), tt.errContains) { + t.Errorf("Error = %v, expected to contain %q", err, tt.errContains) + } + } + }) + } +} + +// TestPassThroughFieldEdgeCases tests additional edge cases for pass-through fields +func TestPassThroughFieldEdgeCases(t *testing.T) { + tests := []struct { + name string + frontmatter map[string]any + wantErr bool + errContains string + }{ + { + name: "concurrency with expression in group", + frontmatter: map[string]any{ + "on": "push", + "concurrency": map[string]any{ + "group": "workflow-${{ github.ref }}", + }, + }, + wantErr: false, + }, + { + name: "runs-on with empty labels array is valid", + frontmatter: map[string]any{ + "on": "push", + "runs-on": map[string]any{ + "labels": []string{}, + }, + }, + wantErr: false, + }, + { + name: "container with all optional fields", + frontmatter: map[string]any{ + "on": "push", + "container": map[string]any{ + "image": "ubuntu:latest", + "env": map[string]any{ + "TEST": "value", + }, + "ports": []any{8080, "9090"}, + "volumes": []string{"/tmp:/tmp"}, + "options": "--cpus 1", + }, + }, + wantErr: false, + }, + { + name: "environment with expression in url", + frontmatter: map[string]any{ + "on": "push", + "environment": map[string]any{ + "name": "staging", + "url": "${{ steps.deploy.outputs.url }}", + }, + }, + wantErr: false, + }, + { + name: "services with credentials", + frontmatter: map[string]any{ + "on": "push", + "services": map[string]any{ + "redis": map[string]any{ + "image": "redis:alpine", + "credentials": map[string]any{ + "username": "user", + "password": "${{ secrets.PASSWORD }}", + }, + }, + }, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateMainWorkflowFrontmatterWithSchema(tt.frontmatter) + + if tt.wantErr && err == nil { + t.Errorf("Expected error but got none") + return + } + + if !tt.wantErr && err != nil { + t.Errorf("Unexpected error: %v", err) + return + } + + if tt.wantErr && err != nil && tt.errContains != "" { + if !strings.Contains(err.Error(), tt.errContains) { + t.Errorf("Error = %v, expected to contain %q", err, tt.errContains) + } + } + }) + } +} diff --git a/pkg/parser/schema_validation.go b/pkg/parser/schema_validation.go index dca8def63e..ee085c3208 100644 --- a/pkg/parser/schema_validation.go +++ b/pkg/parser/schema_validation.go @@ -42,6 +42,15 @@ func validateSharedWorkflowFields(frontmatter map[string]any) error { } // ValidateMainWorkflowFrontmatterWithSchema validates main workflow frontmatter using JSON schema +// +// This function validates all frontmatter fields including pass-through fields that are +// extracted and passed directly to GitHub Actions (concurrency, container, environment, env, +// runs-on, services). The JSON schema validation catches structural errors at compile time: +// - Invalid data types (e.g., array when object expected) +// - Missing required properties (e.g., container missing 'image') +// - Invalid additional properties (e.g., unknown fields) +// +// See pkg/parser/schema_passthrough_validation_test.go for comprehensive test coverage. func ValidateMainWorkflowFrontmatterWithSchema(frontmatter map[string]any) error { schemaValidationLog.Print("Validating main workflow frontmatter with schema") @@ -55,6 +64,7 @@ func ValidateMainWorkflowFrontmatterWithSchema(frontmatter map[string]any) error } // Then run the standard schema validation + // This validates all fields including pass-through fields (concurrency, container, etc.) if err := validateWithSchema(filtered, mainWorkflowSchema, "main workflow file"); err != nil { schemaValidationLog.Printf("Schema validation failed for main workflow: %v", err) return err diff --git a/pkg/workflow/validation.go b/pkg/workflow/validation.go index 296bb3307e..30ba0d7e32 100644 --- a/pkg/workflow/validation.go +++ b/pkg/workflow/validation.go @@ -25,6 +25,28 @@ // - bundler_script_validation.go: JavaScript script content (execSync, GitHub globals) // - bundler_runtime_validation.go: JavaScript runtime mode compatibility // +// # Pass-Through Field Validation +// +// Several workflow frontmatter fields are "pass-through" fields - they are extracted +// from frontmatter and passed directly to GitHub Actions without modification: +// - concurrency: Workflow concurrency control +// - container: Container configuration for jobs +// - environment: GitHub environment configuration +// - env: Environment variables +// - runs-on: Runner selection +// - services: Service containers +// +// These fields ARE validated during frontmatter parsing using JSON Schema validation +// (see pkg/parser/schemas/main_workflow_schema.json). The schema catches: +// - Invalid data types (e.g., array when object expected) +// - Missing required properties (e.g., container missing 'image') +// - Invalid additional properties (e.g., unknown fields in concurrency) +// - Structure violations (e.g., runs-on as number instead of string/array/object) +// +// Schema validation happens in pkg/parser/schema_validation.go during frontmatter +// parsing, so errors are caught at compile time rather than GitHub Actions runtime. +// See pkg/parser/schema_passthrough_validation_test.go for comprehensive test coverage. +// // # When to Add New Validation // // Add validation to existing domain files when: