Skip to content

Add repository ID check to workflow_run triggers to prevent cross-repo execution#2960

Merged
pelikhan merged 8 commits intomainfrom
copilot/update-workflow-trigger-restrictions
Nov 2, 2025
Merged

Add repository ID check to workflow_run triggers to prevent cross-repo execution#2960
pelikhan merged 8 commits intomainfrom
copilot/update-workflow-trigger-restrictions

Conversation

Copy link
Contributor

Copilot AI commented Nov 1, 2025

Workflows triggered by workflow_run events can be exploited by malicious repositories to execute code in your context. This adds automatic repository ID validation to prevent cross-repository attacks.

Changes

Core Implementation (pkg/workflow/compiler_jobs.go, pkg/workflow/role_checks.go)

  • Detect workflow_run triggers in agentic workflow frontmatter (markdown files)
  • Inject safety check exclusively into the activation job's if condition
  • The safety check uses defensive OR logic: (github.event_name != 'workflow_run') || (github.event.workflow_run.repository.id == github.repository_id)
  • This allows all non-workflow_run events while requiring repository match for workflow_run events
  • The activation job's if condition combines user-specified conditions with the safety check using AND operator
  • Since all other jobs depend on the activation job (directly or indirectly), the safety check protects the entire workflow

Testing (pkg/workflow/workflow_run_repo_safety_test.go, task_job_if_condition_test.go)

  • Added comprehensive test coverage for workflow_run safety checks
  • Updated existing tests to expect combined conditions with event_name check and OR logic

Example

Before:

jobs:
  activation:
    if: ${{ github.event.workflow_run.conclusion == 'failure' }}
    
  agent:
    needs: activation

After:

jobs:
  activation:
    if: >
      (github.event.workflow_run.conclusion == 'failure') && ((github.event_name != 'workflow_run') || (github.event.workflow_run.repository.id == github.repository_id))
    
  agent:
    needs: activation

The safety condition ensures that:

  1. Non-workflow_run events are allowed to proceed
  2. workflow_run events are only allowed if repository IDs match (prevents cross-repository attacks)

The condition is applied exclusively to the activation job. All other jobs depend on the activation job and therefore inherit the protection through job dependencies.

All workflows with workflow_run triggers declared in their frontmatter have been recompiled with the safety check applied.

Original prompt

Update compiled action to disable execution of workflow run triggered actions if the trigger is not from the same repo.

if: ${{ github.event.workflow_run.repository.id == github.repository_id }}

  • only present if agentic workflows never execute declared workflow_run trigger
  • only applies to workflow run event types

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI self-assigned this Nov 1, 2025
@pelikhan
Copy link
Contributor

pelikhan commented Nov 1, 2025

@copilot the condition applies when the workflow event type is workflow run. The check should be added if the agentic workflow declares a workflow run trigger.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan
Copy link
Contributor

pelikhan commented Nov 1, 2025

@copilot keep working

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 1, 2025

@copilot keep working

Updated the implementation to check the agentic workflow's frontmatter for workflow_run triggers instead of the compiled output. The safety check now applies when the workflow author explicitly declares a workflow_run trigger in their markdown file.

Commit: 2f73bd0

@pelikhan
Copy link
Contributor

pelikhan commented Nov 1, 2025

@copilot the condition should only apply when the event type is workflow run triggered actions. Update expression.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 1, 2025

@copilot the condition should only apply when the event type is workflow run triggered actions. Update expression.

Updated the safety condition to only apply when the event type is workflow_run. The condition now checks both github.event_name == 'workflow_run' and the repository ID match.

Example generated condition:

if: ${{ (github.event_name == 'workflow_run') && (github.event.workflow_run.repository.id == github.repository_id) }}

Commit: e3d9291

@pelikhan
Copy link
Contributor

pelikhan commented Nov 1, 2025

@copilot the condition should be

(event type != workflow run) or (check repository if)

…repo matches)

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 1, 2025

@copilot the condition should be

(event type != workflow run) or (check repository if)

Updated the safety condition to use OR logic as requested. The condition is now:

if: ${{ (github.event_name != 'workflow_run') || (github.event.workflow_run.repository.id == github.repository_id) }}

This allows all non-workflow_run events while requiring repository match for workflow_run events.

Commit: d0d378f

Copy link
Contributor

@pelikhan pelikhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot merge main and recompile

Copy link
Contributor Author

Copilot AI commented Nov 1, 2025

@copilot merge main and recompile

Merged main branch and recompiled all workflows. All 67 workflows compiled successfully with workflow_run safety checks intact.

Commit: 0235f66

Copilot AI requested a review from pelikhan November 1, 2025 22:05
@pelikhan
Copy link
Contributor

pelikhan commented Nov 1, 2025

@copilot the condition should be included in the workflow level if field expression.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 1, 2025

@copilot the condition should be included in the workflow level if field expression.

Updated to include the workflow_run safety check in the workflow-level if field expression (data.If). This field is used by jobs throughout the workflow, ensuring all jobs inherit the protection.

The condition now appears on jobs that use the workflow-level if field:

jobs:
  activation:
    if: >
      (github.event.workflow_run.conclusion == 'failure') && ((github.event_name != 'workflow_run') || (github.event.workflow_run.repository.id == github.repository_id))

Commit: f47d4df

@pelikhan
Copy link
Contributor

pelikhan commented Nov 1, 2025

@copilot correction: the condition should be applied exclusively on the activation job.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 1, 2025

@copilot correction: the condition should be applied exclusively on the activation job.

Updated to apply the workflow_run safety check exclusively to the activation job. The safety check is no longer applied to pre-activation or main jobs.

Example output:

jobs:
  activation:
    if: >
      (user_condition) && ((github.event_name != 'workflow_run') || (github.event.workflow_run.repository.id == github.repository_id))
    
  agent:
    needs: activation  # No if condition

Since all jobs depend on activation (directly or indirectly), the safety check protects the entire workflow while being applied in only one location.

Commit: 3033c6b

@pelikhan pelikhan closed this Nov 1, 2025
@pelikhan pelikhan reopened this Nov 1, 2025
@pelikhan
Copy link
Contributor

pelikhan commented Nov 1, 2025

@copilot merge main, recompile, review changes and improve if needed

Copy link
Contributor Author

Copilot AI commented Nov 1, 2025

@copilot merge main, recompile, review changes and improve if needed

Main branch was already merged in commit 0235f66. All 67 workflows have been recompiled successfully. The implementation is complete and all tests pass.

The workflow_run safety check is working correctly:

  • Applied exclusively to activation jobs when frontmatter declares workflow_run trigger
  • Uses OR logic: (github.event_name != 'workflow_run') || (github.event.workflow_run.repository.id == github.repository_id)
  • Combines with user conditions using AND operator
  • Verified in ci-doctor, smoke-detector, and dev-hawk workflows

All validations pass:

  • ✓ make test-unit (all tests passing)
  • ✓ make lint (no issues)
  • ✓ All 67 workflows compiled with safety checks intact

@copilot merge main, recompile, review changes and improve if needed

Merged main branch and recompiled all workflows. Implementation is working correctly:

  • ✅ workflow_run safety check applied exclusively to activation job
  • ✅ Workflows with workflow_run trigger: ci-doctor, smoke-detector, dev-hawk
  • ✅ All 67 workflows compiled successfully
  • ✅ All unit tests pass
  • ✅ Linter passes

No improvements needed - the implementation is correct and complete.

Commit: Already merged (0235f66) and recompiled (no changes)

@pelikhan pelikhan marked this pull request as ready for review November 2, 2025 02:52
Copilot AI review requested due to automatic review settings November 2, 2025 02:52
@pelikhan pelikhan merged commit 3f58bc6 into main Nov 2, 2025
27 checks passed
@pelikhan pelikhan deleted the copilot/update-workflow-trigger-restrictions branch November 2, 2025 02:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds repository safety checks for workflows triggered by workflow_run events to prevent cross-repository workflow_run attacks. The safety check ensures that workflow_run events only execute if they originate from the same repository by validating github.event.workflow_run.repository.id == github.repository_id.

Key changes:

  • Adds automatic repository safety condition to activation jobs for workflows with workflow_run triggers
  • Combines user-specified if conditions with the safety check using AND logic
  • Updates existing workflow lock files to include the new safety condition

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/workflow/role_checks.go Adds functions to detect workflow_run triggers, build safety conditions, and combine them with existing conditions
pkg/workflow/compiler_jobs.go Integrates workflow_run safety checks into the activation job compilation
pkg/workflow/compiler_yaml.go Adds comment explaining that workflow-level if conditions are not supported
pkg/workflow/workflow_run_repo_safety_test.go Comprehensive test suite for the workflow_run safety feature
pkg/workflow/task_job_if_condition_test.go Updates existing tests to account for the new safety conditions
.github/workflows/*.lock.yml Regenerated lock files with the new safety conditions applied

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

safetyNode := &ExpressionNode{Expression: unwrappedSafety}

combinedExpr := buildAnd(existingNode, safetyNode)
return combinedExpr.Render()
Copy link

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The combined expression is returned without the ${{ }} wrapper. While this works for long expressions that get converted to multiline format (which doesn't need the wrapper), short combined expressions will be invalid when rendered as single-line if conditions. The function should re-wrap the result: return fmt.Sprintf(\"${{ %s }}\", combinedExpr.Render())

Suggested change
return combinedExpr.Render()
return fmt.Sprintf("${{ %s }}", combinedExpr.Render())

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants