[sergo] Sergo Report: New-Feature Validation Gap + YAML Injection Revisit - 2026-03-16

[github-actions[bot]]

Executive Summary

Today's Sergo run targeted the freshly-merged commit 2181d15 ("Support github-app: auth in dependencies: for cross-org APM packages"), auditing the entire new feature path from YAML parsing through code generation. The cached half revisited the YAML injection surface already explored in runs 21–22, confirming the new buildAPMAppTokenMintStep and buildGitHubAppTokenMintStep are safe from YAML injection (GitHub org/repo names are alphanumeric+hyphen only). The new exploration half traced every step of the cross-org APM auth flow and found two HIGH-severity issues that were introduced with this commit: a silent validation failure that will cause confusing runtime errors for users who misconfigure dependencies.github-app, and a missing token revocation step that leaves minted GitHub App tokens live for up to one hour after the activation job completes. A third MEDIUM finding covers a non-determinism inconsistency in the existing buildGitHubAppTokenMintStep.

---

Serena Tools Update

Tools Snapshot

• Total Tools Available: 40 (23 active, 17 inactive — JetBrains, execute_shell_command, switch_modes, etc.)
• New Tools Since Last Run: None — tool list unchanged from run 18
• Removed Tools: None
• Modified Tools: None

Tool Capabilities Used Today

Tool	Usage
activate_project	Project activation
search_for_pattern	Multi-file pattern searches across pkg/
list_dir	Package structure exploration
bash (grep + read)	Targeted line reads and cross-checks

---

Strategy Selection

Cached Reuse Component (50%)

Previous Strategy Adapted: yaml-step-generator-audit (runs 21–22, score 8/10)

• Why Reused: Runs 21–22 found that formatYAMLValue doesn't escape embedded single quotes and that copilot_engine_execution.go injects COPILOT_API_TARGET without quoting. The new commit adds two more functions that inject user-controlled strings into YAML (buildAPMAppTokenMintStep and its YAML output in apm_dependencies.go). Auditing these new paths with the same lens was a natural extension.
• Modifications: Shifted focus from runtime_step_generator.go / yaml.go to the brand-new apm_dependencies.go and safe_outputs_app_config.go functions. Checked app.AppID, app.PrivateKey, app.Owner, and app.Repositories[0] injection points.
• Result: Clean. The owner and repositories values follow GitHub naming restrictions (alphanumeric + hyphen only), so no YAML injection is possible in practice.

New Exploration Component (50%)

Novel Approach: New-feature validation-gap audit of github-app: in dependencies:

• Tools Employed: search_for_pattern, bash (grep/read), line-by-line code tracing
• Hypothesis: A feature integrating credential handling into a new code path frequently misses one of: input validation, error surfacing, or token lifecycle cleanup.
• Target Areas: pkg/workflow/frontmatter_extraction_metadata.go, pkg/workflow/apm_dependencies.go, pkg/workflow/compiler_activation_job.go
• What Was Found: All three hypothesized gap types were present — invalid config is silently dropped, no user-facing error is surfaced, and the minted token is never revoked.

Combined Strategy Rationale

The cached component provided a precise audit lens (YAML injection in token-building functions), while the new component traced the entire feature lifecycle. Together they covered both the security surface of the YAML output and the correctness/security of the runtime token lifecycle.

---

Analysis Execution

Codebase Context

• Total Go Files (non-test): 589
• Focus Areas: pkg/workflow/apm_dependencies.go, pkg/workflow/frontmatter_extraction_metadata.go, pkg/workflow/compiler_activation_job.go, pkg/workflow/safe_outputs_app_config.go
• Latest Commit: 2181d15 — "Support github-app: auth in dependencies: for cross-org APM packages"

Findings Summary

Severity	Count
High	2
Medium	1
Low	0
Total	3

---

Detailed Findings

Finding 1 — Silent drop of invalid dependencies.github-app config (HIGH)

File: pkg/workflow/frontmatter_extraction_metadata.go:388–393

When the user configures dependencies.github-app but omits a required field (app-id or private-key), the code silently sets githubApp = nil and logs only to the internal debug logger:

if githubApp.AppID == "" || githubApp.PrivateKey == "" {
    frontmatterMetadataLog.Print("dependencies.github-app missing required app-id or private-key; ignoring")
    githubApp = nil
}

The compiler then proceeds normally: it emits the "Using experimental feature: dependencies (APM)" warning (from compiler.go:260), compiles successfully, and generates an APM pack step without the env: GITHUB_TOKEN: override. The workflow deploys successfully but fails at runtime with a cryptic 404 / authentication error when APM tries to pull from the cross-org private package registry.

Contrast with checkout.github-app — checkout_manager.go:809-810 returns a hard compiler error for the identical condition:

if cfg.GitHubApp.AppID == "" || cfg.GitHubApp.PrivateKey == "" {
    return nil, errors.New("checkout.github-app requires both app-id and private-key")
}

This is a clear inconsistency. The checkout path fails fast at compile time; the dependencies path silently degrades to unauthenticated access.

Impact: Users configuring cross-org APM auth get a compile-time success, a deployed workflow, and a confusing runtime failure — with no actionable clue that their github-app config was ignored.

Recommendation: Emit a user-visible compiler warning (or error, to match checkout behavior) from extractAPMDependenciesFromFrontmatter or its caller in compiler_orchestrator_tools.go.

// Before (current):
if githubApp.AppID == "" || githubApp.PrivateKey == "" {
    frontmatterMetadataLog.Print("dependencies.github-app missing required app-id or private-key; ignoring")
    githubApp = nil
}

// After (recommended — match checkout_manager.go behaviour):
if githubApp.AppID == "" || githubApp.PrivateKey == "" {
    return &APMDependenciesInfo{},
        errors.New("dependencies.github-app requires both app-id and private-key")
}

Or, if a non-fatal path is preferred, bubble up a warning to compiler_orchestrator_tools.go where fmt.Fprintln(os.Stderr, console.FormatWarningMessage(...)) + c.IncrementWarningCount() can be called.

---

Finding 2 — APM GitHub App token never revoked after use (HIGH — Security)

Files: pkg/workflow/apm_dependencies.go + pkg/workflow/compiler_activation_job.go

The new buildAPMAppTokenMintStep mints a token with step ID apm-app-token:

const apmAppTokenStepID = "apm-app-token"
// ...
steps = append(steps, fmt.Sprintf("        id: %s\n", apmAppTokenStepID))

In compiler_activation_job.go:414–424, this mint step is appended to the activation job and the APM pack step uses the token. But there is no corresponding revocation step after the pack step completes. The token remains valid for up to one hour.

Every other GitHub App token mint in the codebase has a paired revocation step:

Token Step ID	Invalidation Called?
safe-outputs-app-token	✅ compiler_safe_outputs_job.go:303, notify_comment.go:357
checkout-app-token	✅ checkout_manager.go:323
mcp-github-app-token	✅ mcp_github_config.go:494
activation-app-token	✅ compiler_activation_job.go:335
apm-app-token	❌ missing

buildGitHubAppTokenInvalidationStep is hardcoded to reference safe-outputs-app-token in its if: condition and env: block, so it cannot simply be reused as-is.

Recommendation: Add a dedicated buildAPMAppTokenInvalidationStep function (analogous to buildGitHubAppTokenInvalidationStep) that references apmAppTokenStepID, and call it in compiler_activation_job.go after the upload-artifact step:

// After the APM artifact upload step in compiler_activation_job.go:
if data.APMDependencies.GitHubApp != nil {
    compilerActivationJobLog.Print("Adding APM GitHub App token invalidation step")
    steps = append(steps, buildAPMAppTokenInvalidationStep()...)
}

New function:

// buildAPMAppTokenInvalidationStep generates the step to revoke the APM GitHub App token.
// Runs with always() to ensure cleanup even on failure.
func buildAPMAppTokenInvalidationStep() []string {
    var steps []string
    steps = append(steps, "      - name: Invalidate APM GitHub App token\n")
    steps = append(steps, fmt.Sprintf("        if: always() && steps.%s.outputs.token != ''\n", apmAppTokenStepID))
    steps = append(steps, "        env:\n")
    steps = append(steps, fmt.Sprintf("          TOKEN: $\{\{ steps.%s.outputs.token }}\n", apmAppTokenStepID))
    steps = append(steps, "        run: |\n")
    steps = append(steps, "          gh api --method DELETE -H \"Authorization: token $TOKEN\" /installation/token || echo \"Token may already be expired.\"\n")
    return steps
}

Validation: Add a test to apm_dependencies_compilation_test.go asserting the invalidation step is present in the compiled output when GitHubApp != nil.

---

Finding 3 — buildGitHubAppTokenMintStep multi-repo list unsorted (MEDIUM)

File: pkg/workflow/safe_outputs_app_config.go:158–163

When safe-outputs.github-app.repositories contains more than one entry, buildGitHubAppTokenMintStep iterates them in the order they appear in the frontmatter YAML:

// safe_outputs_app_config.go:158-163
} else if len(app.Repositories) > 1 {
    steps = append(steps, "          repositories: |-\n")
    for _, repo := range app.Repositories {
        steps = append(steps, fmt.Sprintf("            %s\n", repo))
    }
}

By contrast, the new buildAPMAppTokenMintStep (apm_dependencies.go:54–60) correctly sorts before iterating:

reposCopy := make([]string, len(app.Repositories))
copy(reposCopy, app.Repositories)
sort.Strings(reposCopy)
for _, repo := range reposCopy {
    steps = append(steps, fmt.Sprintf("            %s\n", repo))
}

The inconsistency means that a user who reorders safe-outputs.github-app.repositories in their workflow frontmatter without changing the set will produce a different compiled YAML and lock file, triggering unnecessary review diffs.

Recommendation: Apply the same sort in buildGitHubAppTokenMintStep:

} else if len(app.Repositories) > 1 {
    steps = append(steps, "          repositories: |-\n")
    reposCopy := make([]string, len(app.Repositories))
    copy(reposCopy, app.Repositories)
    sort.Strings(reposCopy)
    for _, repo := range reposCopy {
        steps = append(steps, fmt.Sprintf("            %s\n", repo))
    }
}

Estimated Effort: Small (3 lines, mirrors the APM implementation exactly).

---

Improvement Tasks Generated

Task 1: Emit user-visible error for invalid dependencies.github-app config

Issue Type: Validation Gap / API Consistency

Problem: frontmatter_extraction_metadata.go:391–393 silently drops a malformed dependencies.github-app config with only a debug log, producing a compile-time success followed by a confusing runtime authentication failure. checkout_manager.go:809–810 returns a hard error for the identical condition.

Locations:

• pkg/workflow/frontmatter_extraction_metadata.go:388–393 — silent ignore path
• pkg/workflow/compiler_orchestrator_tools.go:210–214 — caller that gets the degraded result

Impact:

• Severity: High
• Affected Files: 2
• Risk: Users cannot tell from compilation output that their cross-org APM auth will be silently unauthenticated

Recommendation: Change extractAPMDependenciesFromFrontmatter to return an error on invalid github-app config, or add a console.FormatWarningMessage + c.IncrementWarningCount() call in compiler_orchestrator_tools.go when packages are returned but GitHubApp is nil despite github-app: being present in frontmatter.

Validation:

• Add test in apm_dependencies_test.go asserting that missing app-id produces a user-visible error/warning (not just silent nil)
• Verify behavior matches checkout_manager.go error message style

Estimated Effort: Small

---

Task 2: Add APM GitHub App token revocation step

Issue Type: Security — Token Lifecycle

Problem: The APM GitHub App token minted with id: apm-app-token in compiler_activation_job.go is never revoked. Every other token mint path in the codebase has a buildGitHubAppTokenInvalidationStep counterpart.

Locations:

• pkg/workflow/apm_dependencies.go — add buildAPMAppTokenInvalidationStep() function
• pkg/workflow/compiler_activation_job.go:424+ — call it after the artifact upload step

Impact:

• Severity: High
• Affected Files: 2
• Risk: Minted token remains valid for ≤1 hour after the activation job finishes; it could be used by a compromised runner

Before: No revocation step after APM pack + upload.

After: always()-gated invalidation step referencing steps.apm-app-token.outputs.token.

Validation:

• Add test to apm_dependencies_compilation_test.go asserting invalidation step present when GitHubApp != nil
• Assert if: always() condition is generated
• Run existing APM compilation tests to confirm no regressions

Estimated Effort: Small–Medium (new function + test)

---

Task 3: Sort repository list in buildGitHubAppTokenMintStep

Issue Type: Non-deterministic Output / Code Inconsistency

Problem: safe_outputs_app_config.go:158–163 iterates app.Repositories in declaration order for the multi-repo block-scalar case; apm_dependencies.go:54–60 (written later) correctly sorts. Reordering repos in frontmatter causes spurious lock file diffs.

Location:

• pkg/workflow/safe_outputs_app_config.go:158–163

Impact:

• Severity: Medium
• Affected Files: 1

Recommendation: Apply sort.Strings on a copy of app.Repositories before generating the block scalar (mirrors the APM implementation exactly).

Validation:

• Add table test in compiler_safe_outputs_job_test.go verifying that [repo-b, repo-a] and [repo-a, repo-b] produce identical compiled output
• Run existing safe-outputs compilation tests

Estimated Effort: Small (3 lines)

---

Success Metrics

This Run

Metric	Value
Findings	3
Tasks Created	3
Files Analyzed	~8
Success Score	8/10

Score Reasoning: +4 findings quality (2 HIGH + 1 MEDIUM, both high-quality with concrete evidence and clear fix paths); +2 coverage (focused on new commit, 100% coverage of new APM github-app code path); +2 task generation (all 3 tasks are actionable, scoped, and include validation checklists).

---

Historical Context

Strategy Performance (last 8 runs)

Date	Strategy	Score	Findings
2026-03-16	yaml-injection-revisit + new-feature-validation-gap	8/10	3
2026-03-15	yaml-singlequote-deepdive + config-rename-backcompat	8/10	3
2026-03-13	context-propagation-revisit + dead-code-audit	7/10	3
2026-03-12	http-network-audit + dispatch-context	8/10	3
2026-03-11	http-client-loop + utf8-truncation	8/10	3
2026-03-10	os-state-revisit + http-response-size	8/10	3
2026-03-09	regexp-deepdive-revisit + chdir-leak	8/10	3
2026-03-08	regexp-compilation-deepdive + context-free-git	8/10	3

Cumulative Statistics

Metric	Value
Total Runs	23
Total Findings	69
Total Tasks Generated	69
Average Success Score	8.0/10
Most Successful Strategy	error-patterns-plus-field-registry-audit (9/10)

---

Recommendations

Immediate Actions

1. Add APM token revocation (apm_dependencies.go + compiler_activation_job.go) — HIGH security, small effort, follows an established pattern already used 4× elsewhere in the codebase
2. Surface dependencies.github-app validation error to user (frontmatter_extraction_metadata.go or compiler_orchestrator_tools.go) — HIGH UX, ensures consistency with checkout.github-app behavior
3. Sort repositories in buildGitHubAppTokenMintStep (safe_outputs_app_config.go:158–163) — MEDIUM, 3-line fix, eliminates spurious lock file diffs

Long-term Improvements

• Token lifecycle pattern: The codebase now has 5 token mint sites, each with bespoke invalidation logic. Consider extracting a generic buildAppTokenMintStep(id, app, permissions, fallback) + buildAppTokenInvalidationStep(id) pair to make future token minting impossible to implement incorrectly.
• extractAPMDependenciesFromFrontmatter error propagation: The function currently returns (result, nil) — adding error to the return type would allow the extraction path to match the validation contract of checkout_manager.go.

---

Next Run Preview

Suggested Focus Areas

• Check whether add_integration_test.go:985 SHA pin @c93eec8 TODO has been updated following PR Merge safe-outputs.allowed-domains and allowed-url-domains; rename default-redaction to default-safe-outputs #21114 merger (from run 22, MEDIUM, still unfixed)
• Audit the safe_outputs_config.go:174 backward-compat gap for the renamed allowed-url-domains → allowed-domains key (from run 22, MEDIUM, still unfixed)
• Audit the new codemod suite added in this commit (codemod_github_app.go, codemod_mcp_scripts.go, etc.) for edge cases in YAML line-by-line transforms

Strategy Evolution

The cached half of run 24 should use the new-feature validation-gap approach (score 8) from today to check the other new codemods and changesets in commit 2181d15. New exploration could target the changeset minor-preserve-branch-name-in-safe-outputs.md or minor-decouple-status-comment.md to find similar validation and lifecycle gaps.

---

References:

• §23168542999

Generated by Sergo — The Serena Go Expert | Run ID: 23168542999 | Strategy: yaml-injection-revisit-plus-new-feature-validation-gap

AI generated by Sergo - Serena Go Expert · history

• expires on Mar 17, 2026, 10:28 PM UTC

[github-actions[bot]]

This discussion has been marked as outdated by Sergo - Serena Go Expert.

A newer discussion is available at Discussion #21454.