diff --git a/.changeset/patch-add-dependencies-env-apm.md b/.changeset/patch-add-dependencies-env-apm.md new file mode 100644 index 0000000000..d45c929cdb --- /dev/null +++ b/.changeset/patch-add-dependencies-env-apm.md @@ -0,0 +1,7 @@ +--- +"gh-aw": patch +--- + +Added `dependencies.env` support for APM dependencies so workflows can pass environment variables to the `microsoft/apm-action` pack step (for example private registry auth), while keeping deterministic env ordering in generated workflow steps and skipping duplicate `GITHUB_TOKEN` entries when `github-app` is configured. + +Upgraded default APM versions to `microsoft/apm@v0.8.2` and `microsoft/apm-action@v1.3.4`. diff --git a/.github/aw/actions-lock.json b/.github/aw/actions-lock.json index bebe5b19fb..6ca9158024 100644 --- a/.github/aw/actions-lock.json +++ b/.github/aw/actions-lock.json @@ -155,10 +155,10 @@ "version": "v2.10.3", "sha": "9cd1b7bf3f36d5a3c3b17abc3545bfb5481912ea" }, - "microsoft/apm-action@v1.3.1": { + "microsoft/apm-action@v1.3.4": { "repo": "microsoft/apm-action", - "version": "v1.3.1", - "sha": "5eac264e08ed8db603fe2c40983794f94cab49d8" + "version": "v1.3.4", + "sha": "83d54a6c7941049210433b16c8dfac573665b12a" }, "oven-sh/setup-bun@v2.1.3": { "repo": "oven-sh/setup-bun", diff --git a/.github/workflows/smoke-claude.lock.yml b/.github/workflows/smoke-claude.lock.yml index 4d212f7336..b6f87b5058 100644 --- a/.github/workflows/smoke-claude.lock.yml +++ b/.github/workflows/smoke-claude.lock.yml @@ -68,7 +68,7 @@ jobs: issues: write pull-requests: write env: - GH_AW_INFO_APM_VERSION: v0.8.0 + GH_AW_INFO_APM_VERSION: v0.8.2 outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} @@ -107,7 +107,7 @@ jobs: GH_AW_INFO_FIREWALL_ENABLED: "true" GH_AW_INFO_AWF_VERSION: "v0.24.3" GH_AW_INFO_AWMG_VERSION: "" - GH_AW_INFO_APM_VERSION: "v0.8.0" + GH_AW_INFO_APM_VERSION: "v0.8.2" GH_AW_INFO_FIREWALL_TYPE: "squid" GH_AW_COMPILED_STRICT: "true" uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 @@ -646,7 +646,7 @@ jobs: run: bash ${RUNNER_TEMP}/gh-aw/actions/print_prompt_summary.sh - name: Install and pack APM dependencies id: apm_pack - uses: microsoft/apm-action@5eac264e08ed8db603fe2c40983794f94cab49d8 # v1.3.1 + uses: microsoft/apm-action@83d54a6c7941049210433b16c8dfac573665b12a # v1.3.4 with: dependencies: | - microsoft/apm-sample-package @@ -687,7 +687,7 @@ jobs: GH_AW_ASSETS_ALLOWED_EXTS: "" GH_AW_ASSETS_BRANCH: "" GH_AW_ASSETS_MAX_SIZE_KB: 0 - GH_AW_INFO_APM_VERSION: v0.8.0 + GH_AW_INFO_APM_VERSION: v0.8.2 GH_AW_MCP_LOG_DIR: /tmp/gh-aw/mcp-logs/safeoutputs GH_AW_WORKFLOW_ID_SANITIZED: smokeclaude outputs: @@ -813,7 +813,7 @@ jobs: name: apm path: /tmp/gh-aw/apm-bundle - name: Restore APM dependencies - uses: microsoft/apm-action@5eac264e08ed8db603fe2c40983794f94cab49d8 # v1.3.1 + uses: microsoft/apm-action@83d54a6c7941049210433b16c8dfac573665b12a # v1.3.4 with: bundle: /tmp/gh-aw/apm-bundle/*.tar.gz apm-version: ${{ env.GH_AW_INFO_APM_VERSION }} diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index 337df875f6..f2a663c0ff 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -407,7 +407,7 @@ var SerenaLanguageSupport = map[string][]string{ } // DefaultAPMVersion is the default version of the microsoft/APM (Agent Package Manager) CLI -const DefaultAPMVersion Version = "v0.8.0" +const DefaultAPMVersion Version = "v0.8.2" // DefaultPlaywrightMCPVersion is the default version of the @playwright/mcp package const DefaultPlaywrightMCPVersion Version = "0.0.68" diff --git a/pkg/workflow/apm_dependencies.go b/pkg/workflow/apm_dependencies.go index 0122831fc4..fc4c427151 100644 --- a/pkg/workflow/apm_dependencies.go +++ b/pkg/workflow/apm_dependencies.go @@ -124,12 +124,32 @@ func GenerateAPMPackStep(apmDeps *APMDependenciesInfo, target string, data *Work " uses: " + actionRef, } - // Inject the minted GitHub App token as GITHUB_TOKEN so APM can access cross-org private repos - if apmDeps.GitHubApp != nil { - lines = append(lines, - " env:", - fmt.Sprintf(" GITHUB_TOKEN: ${{ steps.%s.outputs.token }}", apmAppTokenStepID), - ) + // Build env block: GitHub App token (if configured) + user-provided env vars. + // If github-app is configured, GITHUB_TOKEN is set from the minted app token, so any + // user-supplied GITHUB_TOKEN key is skipped to avoid a duplicate / conflicting entry. + hasGitHubAppToken := apmDeps.GitHubApp != nil + hasUserEnv := len(apmDeps.Env) > 0 + if hasGitHubAppToken || hasUserEnv { + lines = append(lines, " env:") + if hasGitHubAppToken { + lines = append(lines, + fmt.Sprintf(" GITHUB_TOKEN: ${{ steps.%s.outputs.token }}", apmAppTokenStepID), + ) + } + if hasUserEnv { + keys := make([]string, 0, len(apmDeps.Env)) + for k := range apmDeps.Env { + // Skip GITHUB_TOKEN when github-app provides it to avoid duplicate keys + if hasGitHubAppToken && k == "GITHUB_TOKEN" { + continue + } + keys = append(keys, k) + } + sort.Strings(keys) + for _, k := range keys { + lines = append(lines, fmt.Sprintf(" %s: %s", k, apmDeps.Env[k])) + } + } } lines = append(lines, diff --git a/pkg/workflow/apm_dependencies_compilation_test.go b/pkg/workflow/apm_dependencies_compilation_test.go index e9f0c5d945..2f3378f080 100644 --- a/pkg/workflow/apm_dependencies_compilation_test.go +++ b/pkg/workflow/apm_dependencies_compilation_test.go @@ -232,3 +232,45 @@ Test with Claude engine target inference assert.Contains(t, lockContent, "target: claude", "Lock file should use claude target for claude engine") } + +func TestAPMDependenciesCompilationWithEnv(t *testing.T) { + tmpDir := testutil.TempDir(t, "apm-deps-env-test") + + workflow := `--- +engine: copilot +on: workflow_dispatch +permissions: + issues: read + pull-requests: read +dependencies: + packages: + - microsoft/apm-sample-package + env: + MY_TOKEN: ${{ secrets.MY_TOKEN }} + REGISTRY: https://registry.example.com +--- + +Test with env vars on APM pack step +` + + testFile := filepath.Join(tmpDir, "test-apm-env.md") + err := os.WriteFile(testFile, []byte(workflow), 0644) + require.NoError(t, err, "Failed to write test file") + + compiler := NewCompiler() + err = compiler.CompileWorkflow(testFile) + require.NoError(t, err, "Compilation should succeed") + + lockFile := strings.Replace(testFile, ".md", ".lock.yml", 1) + content, err := os.ReadFile(lockFile) + require.NoError(t, err, "Failed to read lock file") + + lockContent := string(content) + + assert.Contains(t, lockContent, "Install and pack APM dependencies", + "Lock file should contain APM pack step") + assert.Contains(t, lockContent, "MY_TOKEN:", + "Lock file should include MY_TOKEN env var on pack step") + assert.Contains(t, lockContent, "REGISTRY: https://registry.example.com", + "Lock file should include REGISTRY env var on pack step") +} diff --git a/pkg/workflow/apm_dependencies_test.go b/pkg/workflow/apm_dependencies_test.go index 352324a916..ce3d4fcbc0 100644 --- a/pkg/workflow/apm_dependencies_test.go +++ b/pkg/workflow/apm_dependencies_test.go @@ -631,3 +631,155 @@ func TestBuildAPMAppTokenInvalidationStep(t *testing.T) { assert.Contains(t, combined, "always()", "Must run even if prior steps fail to ensure token cleanup") }) } + +func TestExtractAPMDependenciesEnv(t *testing.T) { + t.Run("Object format with env field extracts env vars", func(t *testing.T) { + frontmatter := map[string]any{ + "dependencies": map[string]any{ + "packages": []any{"microsoft/apm-sample-package"}, + "env": map[string]any{ + "MY_TOKEN": "${{ secrets.MY_TOKEN }}", + "REGISTRY": "https://registry.example.com", + }, + }, + } + 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.Env, "Env map should be set") + assert.Equal(t, "${{ secrets.MY_TOKEN }}", result.Env["MY_TOKEN"], "MY_TOKEN should be extracted") + assert.Equal(t, "https://registry.example.com", result.Env["REGISTRY"], "REGISTRY should be extracted") + }) + + t.Run("Object format without env field has nil Env", func(t *testing.T) { + frontmatter := map[string]any{ + "dependencies": map[string]any{ + "packages": []any{"microsoft/apm-sample-package"}, + }, + } + 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.Env, "Env should be nil when not specified") + }) + + t.Run("Array format has nil Env", func(t *testing.T) { + frontmatter := map[string]any{ + "dependencies": []any{"microsoft/apm-sample-package"}, + } + 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.Env, "Env should be nil for array format") + }) + + t.Run("Non-string env values are skipped", func(t *testing.T) { + frontmatter := map[string]any{ + "dependencies": map[string]any{ + "packages": []any{"microsoft/apm-sample-package"}, + "env": map[string]any{ + "VALID": "value", + "INVALID": 42, + }, + }, + } + 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.Env, "Env map should be set") + assert.Equal(t, "value", result.Env["VALID"], "String value should be extracted") + assert.NotContains(t, result.Env, "INVALID", "Non-string value should be skipped") + }) +} + +func TestGenerateAPMPackStepWithEnv(t *testing.T) { + t.Run("Pack step includes user env vars", func(t *testing.T) { + apmDeps := &APMDependenciesInfo{ + Packages: []string{"microsoft/apm-sample-package"}, + Env: map[string]string{ + "MY_TOKEN": "${{ secrets.MY_TOKEN }}", + "REGISTRY": "https://registry.example.com", + }, + } + data := &WorkflowData{Name: "test-workflow"} + step := GenerateAPMPackStep(apmDeps, "copilot", data) + + require.NotEmpty(t, step, "Step should not be empty") + combined := combineStepLines(step) + + assert.Contains(t, combined, "env:", "Should have env section") + assert.Contains(t, combined, "MY_TOKEN: ${{ secrets.MY_TOKEN }}", "Should include MY_TOKEN env var") + assert.Contains(t, combined, "REGISTRY: https://registry.example.com", "Should include REGISTRY env var") + assert.NotContains(t, combined, "GITHUB_TOKEN:", "Should not have GITHUB_TOKEN without github-app") + }) + + t.Run("Pack step with env vars and github-app includes both", func(t *testing.T) { + apmDeps := &APMDependenciesInfo{ + Packages: []string{"acme-org/acme-skills"}, + GitHubApp: &GitHubAppConfig{ + AppID: "${{ vars.APP_ID }}", + PrivateKey: "${{ secrets.APP_PRIVATE_KEY }}", + }, + Env: map[string]string{ + "EXTRA": "value", + }, + } + data := &WorkflowData{Name: "test-workflow"} + step := GenerateAPMPackStep(apmDeps, "copilot", data) + + require.NotEmpty(t, step, "Step should not be empty") + combined := combineStepLines(step) + + assert.Contains(t, combined, "GITHUB_TOKEN: ${{ steps.apm-app-token.outputs.token }}", "Should have GITHUB_TOKEN from app") + assert.Contains(t, combined, "EXTRA: value", "Should include user env var") + }) + + t.Run("Env vars are output in sorted order for determinism", func(t *testing.T) { + apmDeps := &APMDependenciesInfo{ + Packages: []string{"microsoft/apm-sample-package"}, + Env: map[string]string{ + "Z_VAR": "z", + "A_VAR": "a", + "M_VAR": "m", + }, + } + data := &WorkflowData{Name: "test-workflow"} + step := GenerateAPMPackStep(apmDeps, "copilot", data) + + require.NotEmpty(t, step, "Step should not be empty") + combined := combineStepLines(step) + + aPos := strings.Index(combined, "A_VAR:") + mPos := strings.Index(combined, "M_VAR:") + zPos := strings.Index(combined, "Z_VAR:") + require.NotEqual(t, -1, aPos, "A_VAR should be present in output") + require.NotEqual(t, -1, mPos, "M_VAR should be present in output") + require.NotEqual(t, -1, zPos, "Z_VAR should be present in output") + assert.True(t, aPos < mPos && mPos < zPos, "Env vars should be sorted alphabetically") + }) + + t.Run("GITHUB_TOKEN in user env is skipped when github-app is configured", func(t *testing.T) { + apmDeps := &APMDependenciesInfo{ + Packages: []string{"acme-org/acme-skills"}, + GitHubApp: &GitHubAppConfig{ + AppID: "${{ vars.APP_ID }}", + PrivateKey: "${{ secrets.APP_PRIVATE_KEY }}", + }, + Env: map[string]string{ + "GITHUB_TOKEN": "should-be-skipped", + "OTHER_VAR": "kept", + }, + } + data := &WorkflowData{Name: "test-workflow"} + step := GenerateAPMPackStep(apmDeps, "copilot", data) + + require.NotEmpty(t, step, "Step should not be empty") + combined := combineStepLines(step) + + assert.Contains(t, combined, "GITHUB_TOKEN: ${{ steps.apm-app-token.outputs.token }}", "Should have GITHUB_TOKEN from app token, not user env") + assert.NotContains(t, combined, "should-be-skipped", "User-supplied GITHUB_TOKEN value should be absent") + assert.Contains(t, combined, "OTHER_VAR: kept", "Other user env vars should be present") + count := strings.Count(combined, "GITHUB_TOKEN:") + assert.Equal(t, 1, count, "GITHUB_TOKEN should appear exactly once") + }) +} diff --git a/pkg/workflow/data/action_pins.json b/pkg/workflow/data/action_pins.json index bebe5b19fb..6ca9158024 100644 --- a/pkg/workflow/data/action_pins.json +++ b/pkg/workflow/data/action_pins.json @@ -155,10 +155,10 @@ "version": "v2.10.3", "sha": "9cd1b7bf3f36d5a3c3b17abc3545bfb5481912ea" }, - "microsoft/apm-action@v1.3.1": { + "microsoft/apm-action@v1.3.4": { "repo": "microsoft/apm-action", - "version": "v1.3.1", - "sha": "5eac264e08ed8db603fe2c40983794f94cab49d8" + "version": "v1.3.4", + "sha": "83d54a6c7941049210433b16c8dfac573665b12a" }, "oven-sh/setup-bun@v2.1.3": { "repo": "oven-sh/setup-bun", diff --git a/pkg/workflow/frontmatter_extraction_metadata.go b/pkg/workflow/frontmatter_extraction_metadata.go index 6581b65ac9..0199fad563 100644 --- a/pkg/workflow/frontmatter_extraction_metadata.go +++ b/pkg/workflow/frontmatter_extraction_metadata.go @@ -361,6 +361,7 @@ func extractAPMDependenciesFromFrontmatter(frontmatter map[string]any) (*APMDepe var isolated bool var githubApp *GitHubAppConfig var version string + var env map[string]string switch v := value.(type) { case []any: @@ -403,6 +404,18 @@ func extractAPMDependenciesFromFrontmatter(frontmatter map[string]any) (*APMDepe version = versionStr } } + if envAny, ok := v["env"]; ok { + if envMap, ok := envAny.(map[string]any); ok && len(envMap) > 0 { + env = make(map[string]string, len(envMap)) + for k, val := range envMap { + if s, ok := val.(string); ok { + env[k] = s + } else { + frontmatterMetadataLog.Printf("Skipping non-string env value for key '%s'", k) + } + } + } + } default: return nil, nil } @@ -411,6 +424,6 @@ func extractAPMDependenciesFromFrontmatter(frontmatter map[string]any) (*APMDepe 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}, nil + frontmatterMetadataLog.Printf("Extracted %d APM dependency packages from frontmatter (isolated=%v, github-app=%v, version=%s, env=%d)", len(packages), isolated, githubApp != nil, version, len(env)) + return &APMDependenciesInfo{Packages: packages, Isolated: isolated, GitHubApp: githubApp, Version: version, Env: env}, nil } diff --git a/pkg/workflow/frontmatter_types.go b/pkg/workflow/frontmatter_types.go index 56592bcd72..d0b75821ac 100644 --- a/pkg/workflow/frontmatter_types.go +++ b/pkg/workflow/frontmatter_types.go @@ -140,10 +140,11 @@ type PluginsConfig struct { // Supports simple array format and object format with packages, isolated, github-app, and version fields. // When present, a pack step is emitted in the activation job and a restore step in the agent job. type APMDependenciesInfo struct { - Packages []string // APM package slugs to install (e.g., "org/package") - Isolated bool // If true, agent restore step clears primitive dirs before unpacking - GitHubApp *GitHubAppConfig // Optional GitHub App for cross-org private package access - Version string // Optional APM CLI version override (e.g., "v0.8.0"); defaults to DefaultAPMVersion + Packages []string // APM package slugs to install (e.g., "org/package") + Isolated bool // If true, agent restore step clears primitive dirs before unpacking + GitHubApp *GitHubAppConfig // Optional GitHub App for cross-org private package access + Version string // Optional APM CLI version override (e.g., "v0.8.0"); defaults to DefaultAPMVersion + Env map[string]string // Optional environment variables to set on the APM pack step } // RateLimitConfig represents rate limiting configuration for workflow triggers