Skip to content

Remove github.actor and github.workflow from allowed expressions#2642

Merged
pelikhan merged 7 commits into
mainfrom
copilot/remove-unsafe-expressions
Oct 28, 2025
Merged

Remove github.actor and github.workflow from allowed expressions#2642
pelikhan merged 7 commits into
mainfrom
copilot/remove-unsafe-expressions

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Oct 27, 2025

  • Remove github.actor and github.workflow from AllowedExpressions in pkg/constants/constants.go
  • Update tests in pkg/workflow/expression_safety_test.go to expect these expressions to be unauthorized
  • Update tests in pkg/workflow/expression_parser_comprehensive_test.go that use these expressions
  • Update template integration tests to use allowed expressions
  • Update workflow files in .github/workflows/ to remove these expressions from user-facing content
  • Keep github.workflow in cache keys (frontmatter) - it's safe there
  • Update documentation in .github/instructions/github-agentic-workflows.instructions.md
  • Update template documentation in pkg/cli/templates/instructions.md
  • Run code review and address feedback
  • Address maintainer feedback on workflow files:
    • Changed title-prefix values to specific workflow names (e.g., "[ci-doctor]", "[daily-perf]", "[daily-test]")
    • Removed "Triggered by" and "Actor" lines from workflow markdown content
    • Updated title references from "this workflow" to specific workflow names or removed prefix
  • Removed github.workflow from invalid expressions examples (since it's valid in cache keys)

Summary

Successfully removed github.actor and github.workflow from the allowed expressions list for user-facing markdown content. These expressions are now properly rejected during workflow compilation as they can contain user-controllable content that could be used for injection attacks.

Latest Changes

Removed github.workflow from the invalid expressions examples section in both instruction files. This makes the documentation clearer since github.workflow is actually allowed in specific contexts (cache keys in frontmatter), so it shouldn't be shown as completely invalid.

Key points:

  • github.actor - Completely blocked (remains in invalid examples)
  • github.workflow - Blocked in markdown content, allowed in cache keys (removed from invalid examples to avoid confusion)

All tests pass and workflows compile successfully.

Original prompt

The following expressions are not safe to use in an agentic workflows

  • github.workflow
  • github.actor

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits October 27, 2025 23:06
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Remove unsafe expressions in agentic workflows Remove github.actor and github.workflow from allowed expressions Oct 27, 2025
Copilot AI requested a review from pelikhan October 27, 2025 23:14
@pelikhan pelikhan marked this pull request as ready for review October 27, 2025 23:22
Copilot AI review requested due to automatic review settings October 27, 2025 23:22
@github-actions
Copy link
Copy Markdown
Contributor

Agentic Changeset Generator triggered by this pull request.

Copy link
Copy Markdown
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 removes github.actor and github.workflow from the list of allowed GitHub Actions context expressions in agentic workflows to prevent potential injection attacks. These user-controllable values have been replaced with safer alternatives like github.event.sender.id (system-generated) or static text placeholders.

Key Changes:

  • Removed github.actor and github.workflow from the allowed expressions list
  • Updated 33 test cases to expect rejection of these expressions instead of allowing them
  • Migrated 22 workflow files to use safer alternatives

Reviewed Changes

Copilot reviewed 41 out of 41 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/constants/constants.go Removed github.actor and github.workflow from AllowedExpressions array
pkg/workflow/expression_safety_test.go Updated 14 test cases to expect rejection of github.actor and github.workflow
pkg/workflow/expression_parser_comprehensive_test.go Updated 13 test cases to expect rejection in complex expressions
pkg/workflow/template_expression_integration_test.go Replaced github.actor with github.event.sender.id in template tests
.github/workflows/*.md (22 files) Replaced ${{ github.actor }} with "the user" or "@user" and ${{ github.workflow }} with "this workflow"
.github/workflows/*.lock.yml (22 files) Regenerated lock files reflecting the markdown changes
.github/instructions/github-agentic-workflows.instructions.md Updated documentation to remove blocked expressions and show them as invalid examples

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

toolset: [default, discussions]
cache-memory:
key: schema-consistency-cache-${{ github.workflow }}
key: schema-consistency-cache-this workflow
Copy link

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

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

The cache key contains a literal space in 'this workflow' which will be used as-is in the cache key. GitHub Actions cache keys should not contain spaces. Consider using a hyphenated form like 'this-workflow' or 'schema-consistency' instead.

Suggested change
key: schema-consistency-cache-this workflow
key: schema-consistency-cache-this-workflow

Copilot uses AI. Check for mistakes.
Comment thread .github/workflows/poem-bot.md Outdated
# Memory cache for persistent AI memory across runs
cache-memory:
key: poem-memory-${{ github.workflow }}-${{ github.run_id }}
key: poem-memory-this workflow-${{ github.run_id }}
Copy link

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

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

The cache key contains a literal space in 'this workflow' which will be used as-is in the cache key. GitHub Actions cache keys should not contain spaces. Consider using a hyphenated form like 'this-workflow' or 'poem-memory' instead.

Suggested change
key: poem-memory-this workflow-${{ github.run_id }}
key: poem-memory-this-workflow-${{ github.run_id }}

Copilot uses AI. Check for mistakes.
@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot Keep ${{ github.workflow }} in the cache key.

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

Copilot AI commented Oct 27, 2025

@copilot Keep ${{ github.workflow }} in the cache key.

Done in commit 95eee08. Reverted the cache keys in poem-bot.md and schema-consistency-checker.md to keep ${{ github.workflow }}. The expression is still blocked in user-facing markdown content but allowed in frontmatter cache key configuration.

Copilot AI requested a review from pelikhan October 27, 2025 23:34
Comment thread .github/workflows/ci-doctor.md Outdated
Comment thread .github/workflows/commit-changes-analyzer.md Outdated
Comment thread .github/workflows/brave.md Outdated
Comment thread .github/workflows/daily-perf-improver.md Outdated
Comment thread .github/workflows/daily-perf-improver.md Outdated
Comment thread .github/workflows/scout.md Outdated
Comment thread .github/workflows/security-fix-pr.md Outdated
Comment thread .github/workflows/unbloat-docs.md Outdated
Comment thread .github/workflows/video-analyzer.md Outdated
Comment thread .github/workflows/video-analyzer.md Outdated
…ines

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI requested a review from pelikhan October 28, 2025 00:42
@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot review instructions.md and prompt.md files and make sure the experessions are not used there

…m allowed list

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

Copilot AI commented Oct 28, 2025

@copilot review instructions.md and prompt.md files and make sure the experessions are not used there

Done in commit 793253a. Reviewed both .github/instructions/github-agentic-workflows.instructions.md and pkg/cli/templates/instructions.md. Removed github.actor and github.workflow from the allowed expressions list and fixed examples to not use them. They now only appear in:

  1. Cache key defaults (where github.workflow is safe to use)
  2. Invalid expressions examples (showing what NOT to use)

Comment thread pkg/cli/templates/instructions.md Outdated
# Environment: ${{ env.MY_VAR }}
# Complex: ${{ toJson(github.workflow) }}
# Actor: ${{ github.actor }}
# Workflow: ${{ github.workflow }}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot remove line

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in commit 597b4e5. Removed the # Workflow: ${{ github.workflow }} line from the invalid expressions examples in both instruction files. This makes it clearer that github.workflow is context-dependent - invalid in markdown content but valid in cache keys.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI requested a review from pelikhan October 28, 2025 01:09
@pelikhan pelikhan merged commit 8adb0ae into main Oct 28, 2025
4 checks passed
@pelikhan pelikhan deleted the copilot/remove-unsafe-expressions branch October 28, 2025 01:37
dsyme added a commit that referenced this pull request Oct 29, 2025
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