From b584c943a083012615ec6f49108d43f8254df99e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Mar 2026 01:50:58 +0000 Subject: [PATCH 1/2] Initial plan From 096d5ddfaaea99d377ddc1988eb00950ba0caa87 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Mar 2026 02:17:11 +0000 Subject: [PATCH 2/2] Validate APM version string before YAML injection in compiler Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/apm_dependencies_test.go | 88 +++++++++++++++++-- pkg/workflow/compiler_orchestrator_tools.go | 5 +- .../frontmatter_extraction_metadata.go | 13 +-- 3 files changed, 92 insertions(+), 14 deletions(-) diff --git a/pkg/workflow/apm_dependencies_test.go b/pkg/workflow/apm_dependencies_test.go index 4cd81668ee0..5ea6c90821d 100644 --- a/pkg/workflow/apm_dependencies_test.go +++ b/pkg/workflow/apm_dependencies_test.go @@ -124,7 +124,8 @@ func TestExtractAPMDependenciesFromFrontmatter(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := extractAPMDependenciesFromFrontmatter(tt.frontmatter) + result, err := extractAPMDependenciesFromFrontmatter(tt.frontmatter) + require.NoError(t, err, "Should not return an error for valid frontmatter") if tt.expectedDeps == nil { assert.Nil(t, result, "Should return nil for no dependencies") } else { @@ -147,7 +148,8 @@ func TestExtractAPMDependenciesGitHubApp(t *testing.T) { }, }, } - result := extractAPMDependenciesFromFrontmatter(frontmatter) + result, err := extractAPMDependenciesFromFrontmatter(frontmatter) + require.NoError(t, err, "Should not return an error") require.NotNil(t, result, "Should return non-nil APMDependenciesInfo") assert.Equal(t, []string{"acme-org/acme-skills/plugins/dev-tools"}, result.Packages) require.NotNil(t, result.GitHubApp, "GitHubApp should be set") @@ -167,7 +169,8 @@ func TestExtractAPMDependenciesGitHubApp(t *testing.T) { }, }, } - result := extractAPMDependenciesFromFrontmatter(frontmatter) + result, err := extractAPMDependenciesFromFrontmatter(frontmatter) + require.NoError(t, err, "Should not return an error") require.NotNil(t, result, "Should return non-nil APMDependenciesInfo") require.NotNil(t, result.GitHubApp, "GitHubApp should be set") assert.Equal(t, "acme-org", result.GitHubApp.Owner) @@ -183,7 +186,8 @@ func TestExtractAPMDependenciesGitHubApp(t *testing.T) { }, }, } - result := extractAPMDependenciesFromFrontmatter(frontmatter) + result, err := extractAPMDependenciesFromFrontmatter(frontmatter) + require.NoError(t, err, "Should not return an error") require.NotNil(t, result, "Packages should still be extracted") assert.Nil(t, result.GitHubApp, "GitHubApp should be nil when app-id is missing") }) @@ -192,7 +196,8 @@ func TestExtractAPMDependenciesGitHubApp(t *testing.T) { frontmatter := map[string]any{ "dependencies": []any{"microsoft/apm-sample-package"}, } - result := extractAPMDependenciesFromFrontmatter(frontmatter) + result, err := extractAPMDependenciesFromFrontmatter(frontmatter) + require.NoError(t, err, "Should not return an error") require.NotNil(t, result, "Should return non-nil APMDependenciesInfo") assert.Nil(t, result.GitHubApp, "GitHubApp should be nil for array format") }) @@ -206,7 +211,8 @@ func TestExtractAPMDependenciesVersion(t *testing.T) { "version": "v1.0.0", }, } - result := extractAPMDependenciesFromFrontmatter(frontmatter) + result, err := extractAPMDependenciesFromFrontmatter(frontmatter) + require.NoError(t, err, "Should not return an error for valid version") require.NotNil(t, result, "Should return non-nil APMDependenciesInfo") assert.Equal(t, "v1.0.0", result.Version, "Version should be extracted from object format") }) @@ -215,7 +221,8 @@ func TestExtractAPMDependenciesVersion(t *testing.T) { frontmatter := map[string]any{ "dependencies": []any{"microsoft/apm-sample-package"}, } - result := extractAPMDependenciesFromFrontmatter(frontmatter) + result, err := extractAPMDependenciesFromFrontmatter(frontmatter) + require.NoError(t, err, "Should not return an error") require.NotNil(t, result, "Should return non-nil APMDependenciesInfo") assert.Empty(t, result.Version, "Version should be empty for array format") }) @@ -226,10 +233,75 @@ func TestExtractAPMDependenciesVersion(t *testing.T) { "packages": []any{"microsoft/apm-sample-package"}, }, } - result := extractAPMDependenciesFromFrontmatter(frontmatter) + result, err := extractAPMDependenciesFromFrontmatter(frontmatter) + require.NoError(t, err, "Should not return an error") require.NotNil(t, result, "Should return non-nil APMDependenciesInfo") assert.Empty(t, result.Version, "Version should be empty when not specified") }) + + t.Run("Invalid version with trailing quote produces error", func(t *testing.T) { + frontmatter := map[string]any{ + "dependencies": map[string]any{ + "packages": []any{"microsoft/apm-sample-package"}, + "version": `v0.8.0"`, + }, + } + result, err := extractAPMDependenciesFromFrontmatter(frontmatter) + require.Error(t, err, "Should return an error for invalid version tag") + assert.Nil(t, result, "Should return nil result on error") + assert.Contains(t, err.Error(), "dependencies.version", "Error should mention the field name") + }) + + t.Run("Invalid version without v prefix produces error", func(t *testing.T) { + frontmatter := map[string]any{ + "dependencies": map[string]any{ + "packages": []any{"microsoft/apm-sample-package"}, + "version": "1.2.3", + }, + } + result, err := extractAPMDependenciesFromFrontmatter(frontmatter) + require.Error(t, err, "Should return an error for version missing v prefix") + assert.Nil(t, result, "Should return nil result on error") + assert.Contains(t, err.Error(), "vX.Y.Z", "Error should describe expected format") + }) + + t.Run("Invalid version string 'latest' produces error", func(t *testing.T) { + frontmatter := map[string]any{ + "dependencies": map[string]any{ + "packages": []any{"microsoft/apm-sample-package"}, + "version": "latest", + }, + } + result, err := extractAPMDependenciesFromFrontmatter(frontmatter) + require.Error(t, err, "Should return an error for non-semver version string") + assert.Nil(t, result, "Should return nil result on error") + }) + + t.Run("Valid partial version v1 compiles without error", func(t *testing.T) { + frontmatter := map[string]any{ + "dependencies": map[string]any{ + "packages": []any{"microsoft/apm-sample-package"}, + "version": "v1", + }, + } + result, err := extractAPMDependenciesFromFrontmatter(frontmatter) + require.NoError(t, err, "Should not return an error for valid partial version") + require.NotNil(t, result, "Should return non-nil APMDependenciesInfo") + assert.Equal(t, "v1", result.Version, "Version should be extracted") + }) + + t.Run("Valid partial version v1.2 compiles without error", func(t *testing.T) { + frontmatter := map[string]any{ + "dependencies": map[string]any{ + "packages": []any{"microsoft/apm-sample-package"}, + "version": "v1.2", + }, + } + result, err := extractAPMDependenciesFromFrontmatter(frontmatter) + require.NoError(t, err, "Should not return an error for valid partial version") + require.NotNil(t, result, "Should return non-nil APMDependenciesInfo") + assert.Equal(t, "v1.2", result.Version, "Version should be extracted") + }) } func TestEngineGetAPMTarget(t *testing.T) { diff --git a/pkg/workflow/compiler_orchestrator_tools.go b/pkg/workflow/compiler_orchestrator_tools.go index e6b351a6cd9..ac3587cdc15 100644 --- a/pkg/workflow/compiler_orchestrator_tools.go +++ b/pkg/workflow/compiler_orchestrator_tools.go @@ -207,7 +207,10 @@ func (c *Compiler) processToolsAndMarkdown(result *parser.FrontmatterResult, cle } // Extract APM dependencies from frontmatter - apmDependencies := extractAPMDependenciesFromFrontmatter(result.Frontmatter) + apmDependencies, err := extractAPMDependenciesFromFrontmatter(result.Frontmatter) + if err != nil { + return nil, err + } if apmDependencies != nil { orchestratorToolsLog.Printf("Extracted %d APM dependencies from frontmatter", len(apmDependencies.Packages)) } diff --git a/pkg/workflow/frontmatter_extraction_metadata.go b/pkg/workflow/frontmatter_extraction_metadata.go index 9e395af4438..6581b65ac91 100644 --- a/pkg/workflow/frontmatter_extraction_metadata.go +++ b/pkg/workflow/frontmatter_extraction_metadata.go @@ -351,10 +351,10 @@ func extractPluginsFromFrontmatter(frontmatter map[string]any) *PluginInfo { // - Object format: {packages: ["org/pkg1", "org/pkg2"], isolated: true, github-app: {...}, version: "v0.8.0"} // // Returns nil if no dependencies field is present or if the field contains no packages. -func extractAPMDependenciesFromFrontmatter(frontmatter map[string]any) *APMDependenciesInfo { +func extractAPMDependenciesFromFrontmatter(frontmatter map[string]any) (*APMDependenciesInfo, error) { value, exists := frontmatter["dependencies"] if !exists { - return nil + return nil, nil } var packages []string @@ -397,17 +397,20 @@ func extractAPMDependenciesFromFrontmatter(frontmatter map[string]any) *APMDepen } if versionAny, ok := v["version"]; ok { if versionStr, ok := versionAny.(string); ok && versionStr != "" { + if !isValidVersionTag(versionStr) { + return nil, fmt.Errorf("dependencies.version %q is not a valid semver tag (expected format: vX.Y.Z)", versionStr) + } version = versionStr } } default: - return nil + return nil, nil } if len(packages) == 0 { - return nil + return nil, nil } frontmatterMetadataLog.Printf("Extracted %d APM dependency packages from frontmatter (isolated=%v, github-app=%v, version=%s)", len(packages), isolated, githubApp != nil, version) - return &APMDependenciesInfo{Packages: packages, Isolated: isolated, GitHubApp: githubApp, Version: version} + return &APMDependenciesInfo{Packages: packages, Isolated: isolated, GitHubApp: githubApp, Version: version}, nil }