diff --git a/.github/workflows/cloclo.lock.yml b/.github/workflows/cloclo.lock.yml index 19929dd7e0..ec6d72d786 100644 --- a/.github/workflows/cloclo.lock.yml +++ b/.github/workflows/cloclo.lock.yml @@ -723,20 +723,20 @@ jobs: __GH_AW_NEEDS_ACTIVATION_OUTPUTS_TEXT__ ``` - {{#if ${{ github.event.issue.number }} }} + {{#if __GH_AW_GITHUB_EVENT_ISSUE_NUMBER__ }} ## Issue Context - **Issue Number**: __GH_AW_GITHUB_EVENT_ISSUE_NUMBER__ - **Issue State**: __GH_AW_GITHUB_EVENT_ISSUE_STATE__ {{/if}} - {{#if ${{ github.event.discussion.number }} }} + {{#if __GH_AW_GITHUB_EVENT_DISCUSSION_NUMBER__ }} ## Discussion Context - **Discussion Number**: __GH_AW_GITHUB_EVENT_DISCUSSION_NUMBER__ {{/if}} - {{#if ${{ github.event.pull_request.number }} }} + {{#if __GH_AW_GITHUB_EVENT_PULL_REQUEST_NUMBER__ }} ## Pull Request Context **IMPORTANT**: If this command was triggered from a pull request, you must capture and include the PR branch information in your processing: diff --git a/pkg/workflow/compiler_yaml.go b/pkg/workflow/compiler_yaml.go index 93897ebacf..f03c1b9717 100644 --- a/pkg/workflow/compiler_yaml.go +++ b/pkg/workflow/compiler_yaml.go @@ -183,6 +183,11 @@ func (c *Compiler) generatePrompt(yaml *strings.Builder, data *WorkflowData) { cleanedMarkdownContent = SubstituteImportInputs(cleanedMarkdownContent, data.ImportInputs) } + // Wrap GitHub expressions in template conditionals BEFORE extracting expressions + // This ensures that expressions created by wrapping (e.g., {{#if ${{ expr }} }}) + // are also extracted and replaced with environment variables + cleanedMarkdownContent = wrapExpressionsInTemplateConditionals(cleanedMarkdownContent) + // Extract expressions and create environment variable mappings for security extractor := NewExpressionExtractor() expressionMappings, err := extractor.ExtractExpressions(cleanedMarkdownContent) @@ -197,9 +202,6 @@ func (c *Compiler) generatePrompt(yaml *strings.Builder, data *WorkflowData) { cleanedMarkdownContent = extractor.ReplaceExpressionsWithEnvVars(cleanedMarkdownContent) } - // Wrap GitHub expressions in template conditionals - cleanedMarkdownContent = wrapExpressionsInTemplateConditionals(cleanedMarkdownContent) - // Split content into manageable chunks chunks := splitContentIntoChunks(cleanedMarkdownContent) compilerYamlLog.Printf("Split prompt into %d chunks", len(chunks)) diff --git a/pkg/workflow/template_expression_integration_test.go b/pkg/workflow/template_expression_integration_test.go index 3119658821..f530d38fa1 100644 --- a/pkg/workflow/template_expression_integration_test.go +++ b/pkg/workflow/template_expression_integration_test.go @@ -91,16 +91,17 @@ ${{ needs.activation.outputs.text }} t.Error("Compiled workflow should contain interpolation and template rendering step") } - // Verify GitHub expressions are properly wrapped in template conditionals - expectedWrappedExpressions := []string{ - "{{#if ${{ github.event.issue.number }} }}", - "{{#if ${{ github.event.pull_request.number }} }}", - "{{#if ${{ needs.activation.outputs.text }} }}", + // Verify GitHub expressions are properly replaced with placeholders in template conditionals + // After the fix, expressions should be replaced with __GH_AW_*__ placeholders + expectedPlaceholderExpressions := []string{ + "{{#if __GH_AW_GITHUB_EVENT_ISSUE_NUMBER__ }}", + "{{#if __GH_AW_GITHUB_EVENT_PULL_REQUEST_NUMBER__ }}", + "{{#if __GH_AW_NEEDS_ACTIVATION_OUTPUTS_TEXT__ }}", } - for _, expectedExpr := range expectedWrappedExpressions { + for _, expectedExpr := range expectedPlaceholderExpressions { if !strings.Contains(compiledStr, expectedExpr) { - t.Errorf("Compiled workflow should contain wrapped expression: %s", expectedExpr) + t.Errorf("Compiled workflow should contain placeholder expression: %s", expectedExpr) } } @@ -266,25 +267,27 @@ Steps expression - will be wrapped. compiledStr := string(compiledYAML) - // Verify all expressions are wrapped (simplified behavior) - if !strings.Contains(compiledStr, "{{#if ${{ github.event.issue.number }} }}") { - t.Error("GitHub expression should be wrapped") + // Verify all expressions are replaced with placeholders (correct behavior) + if !strings.Contains(compiledStr, "{{#if __GH_AW_GITHUB_EVENT_ISSUE_NUMBER__ }}") { + t.Error("GitHub expression should be replaced with placeholder") } - if !strings.Contains(compiledStr, "{{#if ${{ steps.my_step.outputs.value }} }}") { - t.Error("Steps expression should be wrapped") + if !strings.Contains(compiledStr, "{{#if __GH_AW_STEPS_MY_STEP_OUTPUTS_VALUE__ }}") { + t.Error("Steps expression should be replaced with placeholder") } - if !strings.Contains(compiledStr, "{{#if ${{ true }} }}") { - t.Error("Literal 'true' should be wrapped") + // Verify that literal values are also replaced with placeholders + // true and false literals get normalized to __GH_AW_TRUE__ and __GH_AW_FALSE__ + if !strings.Contains(compiledStr, "{{#if __GH_AW_TRUE__ }}") { + t.Error("Literal 'true' should be replaced with placeholder") } - if !strings.Contains(compiledStr, "{{#if ${{ false }} }}") { - t.Error("Literal 'false' should be wrapped") + if !strings.Contains(compiledStr, "{{#if __GH_AW_FALSE__ }}") { + t.Error("Literal 'false' should be replaced with placeholder") } - if !strings.Contains(compiledStr, "{{#if ${{ some_variable }} }}") { - t.Error("Unknown variable should be wrapped") + if !strings.Contains(compiledStr, "{{#if __GH_AW_SOME_VARIABLE__ }}") { + t.Error("Unknown variable should be replaced with placeholder") } // Make sure we didn't create invalid double-wrapping diff --git a/pkg/workflow/template_rendering_test.go b/pkg/workflow/template_rendering_test.go index 77c463b892..5316b64576 100644 --- a/pkg/workflow/template_rendering_test.go +++ b/pkg/workflow/template_rendering_test.go @@ -77,22 +77,23 @@ Normal content here. t.Error("Interpolation and template rendering step should use github-script action") } - // Verify that GitHub expressions are wrapped in ${{ }} - if !strings.Contains(compiledStr, "{{#if ${{ github.event.issue.number }} }}") { - t.Error("Compiled workflow should contain wrapped github.event.issue.number expression") + // Verify that GitHub expressions are replaced with placeholders + if !strings.Contains(compiledStr, "{{#if __GH_AW_GITHUB_EVENT_ISSUE_NUMBER__ }}") { + t.Error("Compiled workflow should contain placeholder for github.event.issue.number expression") } - if !strings.Contains(compiledStr, "{{#if ${{ github.actor }} }}") { - t.Error("Compiled workflow should contain wrapped github.actor expression") + if !strings.Contains(compiledStr, "{{#if __GH_AW_GITHUB_ACTOR__ }}") { + t.Error("Compiled workflow should contain placeholder for github.actor expression") } - // Verify that literal values are also wrapped (simplified behavior) - if !strings.Contains(compiledStr, "{{#if ${{ true }} }}") { - t.Error("Compiled workflow should contain wrapped literal true") + // Verify that literal values are also replaced with placeholders + // true and false literals get normalized to __GH_AW_TRUE__ and __GH_AW_FALSE__ + if !strings.Contains(compiledStr, "{{#if __GH_AW_TRUE__ }}") { + t.Error("Compiled workflow should contain placeholder for literal true") } - if !strings.Contains(compiledStr, "{{#if ${{ false }} }}") { - t.Error("Compiled workflow should contain wrapped literal false") + if !strings.Contains(compiledStr, "{{#if __GH_AW_FALSE__ }}") { + t.Error("Compiled workflow should contain placeholder for literal false") } // Verify the setupGlobals helper is used