Replace dangerous workflow_run trigger with secure workflow_call in smoke-detector #3129
Conversation
- Update JSON schema to support workflow_call inputs and reusable workflow jobs - Add support for inputs.* expressions in expression safety validation - Update smoke-detector to use workflow_call trigger with explicit inputs - Add investigate-on-failure jobs to smoke test workflows that call smoke-detector - Replace all workflow_run context expressions with inputs parameters - Add uses/with/secrets support to Job struct and rendering logic Co-authored-by: pelikhan <[email protected]>
…moke-detector Co-authored-by: pelikhan <[email protected]>
go.mod
Outdated
| github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 | ||
| github.com/sourcegraph/conc v0.3.0 | ||
| github.com/spf13/cobra v1.9.1 | ||
| gopkg.in/yaml.v3 v3.0.1 |
- Changed smoke-detector permissions from issues:write to issues:read - Safe-outputs (add-comment, create-issue) handle all write operations - Updated investigate-on-failure jobs to use read-only permissions - Maintains security by keeping writes in controlled safe-output jobs Co-authored-by: pelikhan <[email protected]>
- Reverted go.mod and go.sum to remove gopkg.in/yaml.v3 dependency - Updated compiler_jobs.go to use github.com/goccy/go-yaml instead - No functional changes, using existing project dependency Co-authored-by: pelikhan <[email protected]>
| - Smoke Opencode | ||
| branches: | ||
| - 'copilot/*' | ||
| reaction: "eyes" |
There was a problem hiding this comment.
Added reaction: "eyes" field back to the on: section. Changes in commit 097571e.
Note: With workflow_call trigger, the reaction field is preserved in the configuration but won't have a functional effect since workflow_call doesn't have a triggering item (like an issue or PR) to add reactions to.
- Removed smoke-opencode.md and smoke-opencode.lock.yml - Workflow is no longer needed Co-authored-by: pelikhan <[email protected]>
- Restored reaction: "eyes" field in on: section - Reaction is parsed from frontmatter even though workflow_call doesn't have a triggering item to react to - Maintains consistency with original workflow configuration Co-authored-by: pelikhan <[email protected]>
|
@copilot merge main and recompile |
…ector-workflow-trigger
|
@copilot merge main and recompile |
…ector-workflow-trigger
Merged latest main branch changes and recompiled all workflows. Changes pushed in merge commit 817ae7a.
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for GitHub Actions reusable workflow calls and workflow_call triggers to the agentic workflows system. This enables workflows to be modularized and called from other workflows, improving code reuse and maintainability.
Key Changes:
- Added
workflow_calltrigger support with input/secret parameter definitions in the schema - Implemented reusable workflow call functionality using
uses,with, andsecretsproperties - Updated expression validation to allow
inputs.*pattern for workflow_call parameters - Refactored smoke detector workflow from
workflow_runtrigger toworkflow_callfor better composability
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/jobs.go | Added Uses, With, Secrets fields to Job struct and rendering logic for reusable workflow calls |
| pkg/workflow/expression_safety.go | Added workflowCallInputsRegex to allow inputs.* expressions for workflow_call parameters |
| pkg/workflow/compiler_jobs.go | Added logic to extract and process reusable workflow call properties (uses/with/secrets) and permissions from frontmatter |
| pkg/parser/schemas/main_workflow_schema.json | Added schema definitions for workflow_call trigger inputs/secrets and job-level uses/with/secrets properties |
| docs/src/content/docs/reference/frontmatter-full.md | Updated documentation to show workflow_call trigger configuration with inputs/secrets |
| .github/workflows/smoke-detector.md | Converted from workflow_run trigger to workflow_call with defined input parameters |
| .github/workflows/smoke-detector.lock.yml | Compiled workflow with workflow_call trigger and pre_activation job |
| .github/workflows/smoke-copilot.md | Added investigate-on-failure job that calls smoke-detector as reusable workflow |
| .github/workflows/smoke-copilot.lock.yml | Compiled workflow with reusable workflow call job |
| .github/workflows/smoke-codex.md | Added investigate-on-failure job that calls smoke-detector as reusable workflow |
| .github/workflows/smoke-codex.lock.yml | Compiled workflow with reusable workflow call job |
| .github/workflows/smoke-claude.md | Added investigate-on-failure job that calls smoke-detector as reusable workflow |
| .github/workflows/smoke-claude.lock.yml | Compiled workflow with reusable workflow call job |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case string: | ||
| yaml.WriteString(fmt.Sprintf(" %s: %s\n", key, v)) |
There was a problem hiding this comment.
String values in GitHub Actions YAML may need proper quoting or escaping. The current implementation directly interpolates string values without checking if they contain special characters (colons, quotes, newlines, etc.) or GitHub expressions. This could produce invalid YAML or incorrect behavior. Consider using a YAML marshaling library or adding proper escaping/quoting logic.
| switch v := value.(type) { | ||
| case string: | ||
| yaml.WriteString(fmt.Sprintf(" %s: %s\n", key, v)) | ||
| case int, int64, float64: |
There was a problem hiding this comment.
The case statement matches multiple numeric types (int, int64, float64) but doesn't handle all numeric types that could appear in map[string]any. Missing types include: int8, int16, int32, uint, uint8, uint16, uint32, uint64, float32. Consider using a type switch or checking reflect.Kind() to handle all numeric types consistently.
| case int, int64, float64: | |
| case int, int8, int16, int32, int64, | |
| uint, uint8, uint16, uint32, uint64, | |
| float32, float64: |
| // Extract permissions | ||
| if permissions, hasPermissions := configMap["permissions"]; hasPermissions { | ||
| if permsMap, ok := permissions.(map[string]any); ok { | ||
| // Use gopkg.in/yaml.v3 to marshal permissions |
There was a problem hiding this comment.
The comment references 'gopkg.in/yaml.v3' but the actual import is 'github.com/goccy/go-yaml' (line 9). Update the comment to reflect the correct YAML library being used.
| // Use gopkg.in/yaml.v3 to marshal permissions | |
| // Use github.com/goccy/go-yaml to marshal permissions |
| allowedList.WriteString(" - needs.*\n") | ||
| allowedList.WriteString(" - steps.*\n") | ||
| allowedList.WriteString(" - github.event.inputs.*\n") | ||
| allowedList.WriteString(" - inputs.* (workflow_call)\n") |
There was a problem hiding this comment.
[nitpick] The comment '(workflow_call)' in the allowed list output could be confusing since inputs.* is also valid in other contexts. Consider clarifying that this pattern is specifically for reusable workflow calls, e.g., ' - inputs.* (reusable workflow inputs)\n' to distinguish from github.event.inputs.*
| allowedList.WriteString(" - inputs.* (workflow_call)\n") | |
| allowedList.WriteString(" - inputs.* (reusable workflow inputs)\n") |
| description: 'Commit SHA that triggered the workflow run' | ||
| required: true | ||
| type: string | ||
| reaction: "eyes" |
There was a problem hiding this comment.
The reaction field is placed inside the on: trigger configuration, but according to the schema (line 916-921 in main_workflow_schema.json), it should be at the same level as other trigger properties within the on: object. The current placement appears syntactically valid but may cause confusion. Verify this is intentionally nested under workflow_call inputs or if it should be at the on: object level.
Fix dangerous workflow_run trigger in smoke-detector
Problem
The smoke-detector workflow used
workflow_runtrigger, which executes in the base branch context with full permissions even for untrusted fork PRs, enabling privilege escalation and secret extraction attacks.Solution
Converted smoke-detector to use secure
workflow_callpattern with explicit inputs passed from smoke test workflows.Changes Made
1. Schema Extensions
inputsandsecretsfields to workflow_call trigger schemauses,with,secretsproperties to job schemainputs.*pattern support for workflow_call input expressions2. Code Changes
workflowCallInputsRegexto allowinputs.*expressionsUses,With, andSecretsfields to Job structbuildCustomJobs()to handle reusable workflow calls; uses existinggoccy/go-yamlpackagerenderJob()to outputuses/with/secretsinstead ofstepsfor reusable workflows3. Workflow Updates
smoke-detector.md:
workflow_runtoworkflow_callworkflow_name,run_id,run_number,conclusion,html_url,head_sha${{ github.event.workflow_run.* }}with${{ inputs.* }}contents: read,issues: read,pull-requests: read,actions: read)add-comment,create-issue)reaction: "eyes"field from original configurationsmoke-{claude,codex,copilot}.md:
investigate-on-failurejob withuses: ./.github/workflows/smoke-detector.lock.ymlif: failure()issues: write)Cleanup: Removed obsolete smoke-opencode workflow
4. Merges with Main
Security Improvements
✅ Eliminated privilege escalation: No longer runs in base branch context for fork PRs
✅ Prevented secret exposure: Requires explicit input passing, no automatic access to repository secrets
✅ Enforced input validation: All parameters must be explicitly provided via
with:clause✅ Applied least-privilege: Read-only permissions; writes handled by safe-outputs
✅ Separation of concerns: Write operations isolated in safe-output jobs with controlled permissions
Verification
workflow_runtriggers remain in smoke-detectoruses:patternFixes #3120
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.