diff --git a/pkg/workflow/block_scalar_expression_size_test.go b/pkg/workflow/block_scalar_expression_size_test.go index 4439107cc8a..2b2b7284464 100644 --- a/pkg/workflow/block_scalar_expression_size_test.go +++ b/pkg/workflow/block_scalar_expression_size_test.go @@ -115,4 +115,194 @@ func TestValidateBlockScalarExpressionSizes(t *testing.T) { // Ensure the constant is exactly 21000 as documented assert.Equal(t, 21000, MaxExpressionSize, "MaxExpressionSize must be 21000 to match GitHub Actions limit") }) + + t.Run("keep chomping (|+) block with expression fails", func(t *testing.T) { + var sb strings.Builder + sb.WriteString("jobs:\n test:\n steps:\n - name: Keep\n run: |+\n") + for range 50 { + sb.WriteString(" echo " + strings.Repeat("c", 10) + "\n") + } + sb.WriteString(" echo ${{ github.actor }}\n") + lines := strings.Split(sb.String(), "\n") + err := validateBlockScalarExpressionSizes(lines, smallMax) + require.Error(t, err, "keep-chomping block (|+) with expression exceeding limit should fail") + assert.Contains(t, err.Error(), "exceeds maximum allowed size") + }) + + t.Run("indentation indicator block (|2) with expression fails", func(t *testing.T) { + var sb strings.Builder + sb.WriteString("jobs:\n test:\n steps:\n - name: Indent\n run: |2\n") + for range 50 { + sb.WriteString(" echo " + strings.Repeat("d", 10) + "\n") + } + sb.WriteString(" echo ${{ github.actor }}\n") + lines := strings.Split(sb.String(), "\n") + err := validateBlockScalarExpressionSizes(lines, smallMax) + require.Error(t, err, "explicit-indent block (|2) with expression exceeding limit should fail") + assert.Contains(t, err.Error(), "exceeds maximum allowed size") + }) + + t.Run("combined indicator block (|2-) with expression fails", func(t *testing.T) { + var sb strings.Builder + sb.WriteString("jobs:\n test:\n steps:\n - name: Combined\n run: |2-\n") + for range 50 { + sb.WriteString(" echo " + strings.Repeat("e", 10) + "\n") + } + sb.WriteString(" echo ${{ github.actor }}\n") + lines := strings.Split(sb.String(), "\n") + err := validateBlockScalarExpressionSizes(lines, smallMax) + require.Error(t, err, "combined indicator block (|2-) with expression exceeding limit should fail") + assert.Contains(t, err.Error(), "exceeds maximum allowed size") + }) +} + +func TestValidateExpressionSizesSinglePass(t *testing.T) { + const smallMax = 100 // tiny limit to keep tests readable + + t.Run("empty YAML passes", func(t *testing.T) { + err := validateExpressionSizesSinglePass("", smallMax) + assert.NoError(t, err, "empty content should pass") + }) + + t.Run("block scalar under limit without expression passes", func(t *testing.T) { + yaml := `jobs: + test: + steps: + - name: Small block + run: | + echo hello + echo world +` + err := validateExpressionSizesSinglePass(yaml, smallMax) + assert.NoError(t, err, "small block without expression should pass") + }) + + t.Run("block scalar under limit with expression passes", func(t *testing.T) { + yaml := `jobs: + test: + steps: + - name: Small block with expression + run: | + echo ${{ github.actor }} +` + err := validateExpressionSizesSinglePass(yaml, smallMax) + assert.NoError(t, err, "small block with expression should pass (under limit)") + }) + + t.Run("large block scalar without expression passes", func(t *testing.T) { + var sb strings.Builder + sb.WriteString("jobs:\n test:\n steps:\n - name: Large block\n run: |\n") + for range 50 { + sb.WriteString(" echo " + strings.Repeat("x", 10) + "\n") + } + err := validateExpressionSizesSinglePass(sb.String(), smallMax) + assert.NoError(t, err, "large block without any expression should pass") + }) + + t.Run("large block scalar with expression fails", func(t *testing.T) { + var sb strings.Builder + sb.WriteString("jobs:\n test:\n steps:\n - name: Large block with expression\n run: |\n") + for range 50 { + sb.WriteString(" echo " + strings.Repeat("x", 10) + "\n") + } + sb.WriteString(" echo ${{ github.actor }}\n") + err := validateExpressionSizesSinglePass(sb.String(), smallMax) + require.Error(t, err, "large block with expression should fail") + assert.Contains(t, err.Error(), "exceeds maximum allowed size") + assert.Contains(t, err.Error(), "run") + }) + + t.Run("single line exceeding limit fails", func(t *testing.T) { + // A single YAML line longer than maxSize should be caught by the per-line check. + line := " value: " + strings.Repeat("x", smallMax+1) + "\n" + err := validateExpressionSizesSinglePass(line, smallMax) + require.Error(t, err, "single line over limit should fail") + assert.Contains(t, err.Error(), "exceeds maximum allowed") + }) + + t.Run("expression at beginning of large block fails", func(t *testing.T) { + var sb strings.Builder + sb.WriteString("jobs:\n test:\n steps:\n - name: Block\n run: |\n") + sb.WriteString(" echo ${{ github.ref_name }}\n") + for range 50 { + sb.WriteString(" echo " + strings.Repeat("z", 10) + "\n") + } + err := validateExpressionSizesSinglePass(sb.String(), smallMax) + require.Error(t, err, "block with expression at start should fail when total exceeds limit") + }) + + t.Run("multiple blocks: only large expression block fails", func(t *testing.T) { + var sb strings.Builder + // First block: small with expression - should pass + sb.WriteString("jobs:\n test:\n steps:\n - name: Small\n run: |\n") + sb.WriteString(" echo ${{ github.actor }}\n") + // Second block: large without expression - should pass + sb.WriteString(" - name: Large no expr\n run: |\n") + for range 50 { + sb.WriteString(" echo " + strings.Repeat("a", 10) + "\n") + } + err := validateExpressionSizesSinglePass(sb.String(), smallMax) + assert.NoError(t, err, "only large-with-expression blocks should fail") + }) + + t.Run("folded block (>) with expression fails", func(t *testing.T) { + var sb strings.Builder + sb.WriteString("jobs:\n test:\n steps:\n - name: Folded\n run: >\n") + for range 50 { + sb.WriteString(" echo " + strings.Repeat("b", 10) + "\n") + } + sb.WriteString(" echo ${{ github.actor }}\n") + err := validateExpressionSizesSinglePass(sb.String(), smallMax) + require.Error(t, err, "folded block (>) with expression exceeding limit should fail") + assert.Contains(t, err.Error(), "exceeds maximum allowed size") + }) + + t.Run("keep chomping (|+) with expression fails", func(t *testing.T) { + var sb strings.Builder + sb.WriteString("jobs:\n test:\n steps:\n - name: Keep\n run: |+\n") + for range 50 { + sb.WriteString(" echo " + strings.Repeat("c", 10) + "\n") + } + sb.WriteString(" echo ${{ github.actor }}\n") + err := validateExpressionSizesSinglePass(sb.String(), smallMax) + require.Error(t, err, "keep-chomping block (|+) with expression exceeding limit should fail") + assert.Contains(t, err.Error(), "exceeds maximum allowed size") + }) + + t.Run("indentation indicator (|2) with expression fails", func(t *testing.T) { + var sb strings.Builder + sb.WriteString("jobs:\n test:\n steps:\n - name: Indent\n run: |2\n") + for range 50 { + sb.WriteString(" echo " + strings.Repeat("d", 10) + "\n") + } + sb.WriteString(" echo ${{ github.actor }}\n") + err := validateExpressionSizesSinglePass(sb.String(), smallMax) + require.Error(t, err, "explicit-indent block (|2) with expression exceeding limit should fail") + assert.Contains(t, err.Error(), "exceeds maximum allowed size") + }) + + t.Run("combined indicator (|2-) with expression fails", func(t *testing.T) { + var sb strings.Builder + sb.WriteString("jobs:\n test:\n steps:\n - name: Combined\n run: |2-\n") + for range 50 { + sb.WriteString(" echo " + strings.Repeat("e", 10) + "\n") + } + sb.WriteString(" echo ${{ github.actor }}\n") + err := validateExpressionSizesSinglePass(sb.String(), smallMax) + require.Error(t, err, "combined indicator block (|2-) with expression exceeding limit should fail") + assert.Contains(t, err.Error(), "exceeds maximum allowed size") + }) + + t.Run("block open at EOF is checked", func(t *testing.T) { + var sb strings.Builder + sb.WriteString("jobs:\n test:\n steps:\n - name: EOF block\n run: |\n") + for range 50 { + sb.WriteString(" echo " + strings.Repeat("f", 10) + "\n") + } + sb.WriteString(" echo ${{ github.actor }}") + // No trailing newline — block is still open at EOF + err := validateExpressionSizesSinglePass(sb.String(), smallMax) + require.Error(t, err, "block still open at EOF with expression exceeding limit should fail") + assert.Contains(t, err.Error(), "exceeds maximum allowed size") + }) } diff --git a/pkg/workflow/runtime_validation.go b/pkg/workflow/runtime_validation.go index ef01b4251ef..cf0be29dc01 100644 --- a/pkg/workflow/runtime_validation.go +++ b/pkg/workflow/runtime_validation.go @@ -40,7 +40,6 @@ package workflow import ( - "errors" "fmt" "os" "strings" @@ -61,47 +60,140 @@ var runtimeValidationLog = newValidationLogger("runtime") // many lines but are parsed by GitHub Actions as a single string value. When such a // block contains at least one ${{ }} expression AND its total length exceeds 21,000 // characters, GitHub Actions rejects the workflow with "Exceeded max expression length". +// +// Performance: uses a single strings.SplitSeq pass to avoid allocating a []string slice +// for the YAML content and to eliminate the double-pass overhead of the original +// implementation. func (c *Compiler) validateExpressionSizes(yamlContent string) error { - lines := strings.Split(yamlContent, "\n") - runtimeValidationLog.Printf("Validating expression sizes: yaml_lines=%d, max_size=%d", len(lines), MaxExpressionSize) maxSize := MaxExpressionSize + runtimeValidationLog.Printf("Validating expression sizes: max_size=%d", maxSize) + return validateExpressionSizesSinglePass(yamlContent, maxSize) +} + +// validateExpressionSizesSinglePass combines the per-line size check and the block-scalar +// size check into a single strings.SplitSeq pass over the YAML content. This avoids +// allocating a []string slice for the YAML lines and eliminates the redundant second +// scan that validateBlockScalarExpressionSizes would otherwise perform. +func validateExpressionSizesSinglePass(yamlContent string, maxSize int) error { + // Block scalar tracking state (mirrors validateBlockScalarExpressionSizes). + inBlock := false + blockKey := "" + blockStartLine := 0 + blockIndent := -1 + blockSize := 0 + blockHasExpression := false + + checkBlock := func() error { + if inBlock && blockHasExpression && blockSize > maxSize { + actualSize := console.FormatFileSize(int64(blockSize)) + maxSizeFormatted := console.FormatFileSize(int64(maxSize)) + return fmt.Errorf("expression value for %q (%s) exceeds maximum allowed size (%s) starting at line %d. "+ + "GitHub Actions has a 21KB limit for YAML values that contain template expressions (${{ }}). "+ + "Split the step into separate run: blocks so that no single block containing ${{ }} expressions exceeds the limit", + blockKey, actualSize, maxSizeFormatted, blockStartLine+1) + } + return nil + } - for lineNum, line := range lines { - // Check the line length (actual content that will be in the YAML) + lineNum := 0 + for line := range strings.SplitSeq(yamlContent, "\n") { + lineNum++ + + // Per-line size check: a single YAML line value exceeding maxSize is rejected. if len(line) > maxSize { - // Extract the key/value for better error message trimmed := strings.TrimSpace(line) key := "" if colonIdx := strings.Index(trimmed, ":"); colonIdx > 0 { key = strings.TrimSpace(trimmed[:colonIdx]) } - - // Format sizes for display actualSize := console.FormatFileSize(int64(len(line))) maxSizeFormatted := console.FormatFileSize(int64(maxSize)) - - var errorMsg string if key != "" { - errorMsg = fmt.Sprintf("expression value for %q (%s) exceeds maximum allowed size (%s) at line %d. GitHub Actions has a 21KB limit for expression values including environment variables. Consider chunking the content or using artifacts instead.", - key, actualSize, maxSizeFormatted, lineNum+1) - } else { - errorMsg = fmt.Sprintf("line %d (%s) exceeds maximum allowed expression size (%s). GitHub Actions has a 21KB limit for expression values.", - lineNum+1, actualSize, maxSizeFormatted) + return fmt.Errorf("expression value for %q (%s) exceeds maximum allowed size (%s) at line %d. GitHub Actions has a 21KB limit for expression values including environment variables. Consider chunking the content or using artifacts instead", + key, actualSize, maxSizeFormatted, lineNum) } + return fmt.Errorf("line %d (%s) exceeds maximum allowed expression size (%s). GitHub Actions has a 21KB limit for expression values", + lineNum, actualSize, maxSizeFormatted) + } + + // Block scalar tracking: detect multi-line block scalars and accumulate their + // sizes so that run: | blocks containing ${{ }} can be checked against maxSize. + trimmed := strings.TrimLeft(line, " \t") + indent := len(line) - len(trimmed) - return errors.New(errorMsg) + if inBlock { + if strings.TrimSpace(line) == "" { + blockSize += len(line) + 1 // +1 for the newline + continue + } + if indent > blockIndent { + blockSize += len(line) + 1 + if strings.Contains(line, "${{") { + blockHasExpression = true + } + continue + } + // Indentation dropped back – the block has ended. + if err := checkBlock(); err != nil { + return err + } + inBlock = false + blockKey = "" + blockSize = 0 + blockHasExpression = false + blockIndent = -1 } - } - // Check multi-line YAML block scalars that contain template expressions. - // A run: | or any other block-scalar value is a single string from GitHub Actions' - // perspective; if it contains ${{ }} AND is longer than 21,000 characters the - // runner rejects it with "Exceeded max expression length". - if err := validateBlockScalarExpressionSizes(lines, maxSize); err != nil { - return err + // Detect the start of a block scalar: a YAML key whose value is | or > + // e.g. " run: |" or " script: >" or " run: |+" or " run: |2-" + if colonIdx := strings.Index(trimmed, ":"); colonIdx > 0 { + afterColon := strings.TrimSpace(trimmed[colonIdx+1:]) + if isBlockScalarHeader(afterColon) { + inBlock = true + blockKey = strings.TrimSpace(trimmed[:colonIdx]) + blockStartLine = lineNum - 1 // 0-indexed; checkBlock reports it as blockStartLine+1 + blockIndent = indent + blockSize = 0 + blockHasExpression = false + } + } } - return nil + // Check any block that was still open at EOF. + return checkBlock() +} + +// isBlockScalarHeader reports whether s (the trimmed string after a YAML colon) +// is a valid YAML block scalar indicator, covering all YAML-spec variants: +// +// bare: "|" ">" +// strip chomping: "|-" ">-" +// keep chomping: "|+" ">+" +// indent only: "|2" ">3" (digit 1–9) +// indent+strip: "|2-" ">3-" (also "|-2", ">-3") +// indent+keep: "|2+" ">3+" (also "|+2", ">+3") +func isBlockScalarHeader(s string) bool { + if len(s) == 0 || (s[0] != '|' && s[0] != '>') { + return false + } + rest := s[1:] + switch rest { + case "", "-", "+": + return true + } + if len(rest) == 1 { + return rest[0] >= '1' && rest[0] <= '9' + } + if len(rest) == 2 { + r0, r1 := rest[0], rest[1] + if r0 >= '1' && r0 <= '9' { + return r1 == '-' || r1 == '+' + } + if r0 == '-' || r0 == '+' { + return r1 >= '1' && r1 <= '9' + } + } + return false } // validateBlockScalarExpressionSizes scans the YAML lines for multi-line block scalars @@ -160,10 +252,10 @@ func validateBlockScalarExpressionSizes(lines []string, maxSize int) error { } // Detect the start of a block scalar: a YAML key whose value is | or > - // e.g. " run: |" or " script: >" + // e.g. " run: |" or " script: >" or " run: |+" or " run: |2-" if colonIdx := strings.Index(trimmed, ":"); colonIdx > 0 { afterColon := strings.TrimSpace(trimmed[colonIdx+1:]) - if afterColon == "|" || afterColon == ">" || strings.HasPrefix(afterColon, "|-") || strings.HasPrefix(afterColon, ">-") { + if isBlockScalarHeader(afterColon) { inBlock = true blockKey = strings.TrimSpace(trimmed[:colonIdx]) blockStartLine = i