Validate external safe-output secrets before activation - #60267
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Staged handlers are incorrectly blocked, Linear overrides are mishandled, and failure guidance can be misleading.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds early credential validation for external safe-output providers and integrates failures into activation reporting.
Changes:
- Validates Jira, Linear, and Azure DevOps credentials during activation.
- Supports credential overrides and injects a default Azure DevOps PAT.
- Updates validation messages, tests, and the smoke workflow lock file.
File summaries
| File | Description |
|---|---|
pkg/workflow/safe_outputs_secret_validation.go |
Defines external credential requirements. |
pkg/workflow/safe_outputs_secret_validation_test.go |
Tests requirement detection and step IDs. |
pkg/workflow/safe_outputs_azure_devops.go |
Injects the default Azure DevOps PAT. |
pkg/workflow/safe_outputs_azure_devops_test.go |
Tests PAT injection and overrides. |
pkg/workflow/notify_comment_conclusion_helpers.go |
Includes safe-output validation in failure handling. |
pkg/workflow/engine_helpers.go |
Supports caller-defined validation step IDs. |
pkg/workflow/compiler_safe_outputs_job.go |
Applies Azure DevOps credential injection. |
pkg/workflow/compiler_activation_steps.go |
Aggregates secret-validation outcomes. |
pkg/workflow/compiler_activation_steps_test.go |
Tests generated external validation steps. |
actions/setup/sh/validate_multi_secret.sh |
Generalizes validation error messages. |
actions/setup/sh/validate_multi_secret_test.go |
Tests requirement messaging. |
.github/workflows/smoke-issues.lock.yml |
Regenerates the smoke workflow with checks. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| if hasLinearSafeOutputs(config) && | ||
| strings.TrimSpace(config.LinearToken) == "" && | ||
| strings.TrimSpace(config.Env["GH_AW_LINEAR_TOKEN"]) == "" { |
| if hasSecretValidationStep(engine, data) { | ||
| envVars = append(envVars, fmt.Sprintf(" GH_AW_SECRET_VERIFICATION_RESULT: ${{ needs.%s.outputs.secret_verification_result }}\n", constants.ActivationJobName)) | ||
| if msg := engine.GetSecretFailureMessage(data); msg != "" { | ||
| envVars = append(envVars, fmt.Sprintf(" GH_AW_ENGINE_SECRET_FAILURE_MESSAGE: %q\n", msg)) | ||
| if EngineHasValidateSecretStep(engine, data) { | ||
| if msg := engine.GetSecretFailureMessage(data); msg != "" { | ||
| envVars = append(envVars, fmt.Sprintf(" GH_AW_ENGINE_SECRET_FAILURE_MESSAGE: %q\n", msg)) | ||
| } | ||
| } |
| var requirements []safeOutputSecretRequirement | ||
| if hasAnyJiraSafeOutputEnabled(config) { |
| # Join secret names with " or " | ||
| secret_or_list=$(IFS=" or "; echo "${SECRET_NAMES[*]}") | ||
| requirement_msg="The $ENGINE_NAME engine requires either $secret_or_list secret to be configured." | ||
| requirement_msg="$secret_or_list is required by $ENGINE_NAME." |
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
|
🧠 Matt Pocock Skills Reviewer failed during the skills-based review. Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
|
|
Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details.
|
|
|
✅ Ponytail Reviewer completed successfully! Lean already. Ship. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "ab.chatgpt.com"See Network Configuration for more information.
|
🏗️ ADR required — draft added for PR #60267ResultA draft ADR has been added to this PR because ADR enforcement applies here and no existing ADR was found in the PR body or Evidence used
Draft ADR added
Inferred architectural decisionThis PR makes the architectural decision to validate external safe-output credentials during activation, rather than waiting for safe-output processing after agent execution. Next actionPlease review and refine the draft ADR so it accurately reflects the intended long-term decision and trade-offs for activation-time safe-output secret validation.
|
|
🎉 This pull request is included in a new release. Release: |
Jira, Linear, and Azure DevOps safe outputs could fail after agent execution when required credentials were missing. This adds early activation checks with actionable configuration errors.
Changes
Secret requirements
Frontmatter overrides
safe-outputs.env,linear-token, or a deployment environment.Azure DevOps
${{ secrets.AZURE_DEVOPS_EXT_PAT }}to the safe-output processor by default.SYSTEM_ACCESSTOKENandAZURE_DEVOPS_EXT_PAToverrides.Failure reporting