-
Notifications
You must be signed in to change notification settings - Fork 111
Add compile-time validation for dangerous property names in expressions #14829
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add compile-time validation for dangerous property names in expressions #14829
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds compile-time validation to block dangerous JavaScript property names in GitHub Actions expressions (e.g., constructor, __proto__) to prevent prototype-pollution style access patterns, aligning compile-time checks with existing runtime protections.
Changes:
- Introduces a
DangerousPropertyNamesblocklist constant. - Adds
validateExpressionForDangerousProps()and runs it early during single-expression validation. - Expands expression safety tests to cover blocked properties and end-to-end markdown validation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/workflow/expression_validation.go | Adds dangerous-property detection and integrates it into expression validation flow. |
| pkg/constants/constants.go | Defines the dangerous property name blocklist used by the validator. |
| pkg/workflow/expression_safety_test.go | Adds unit + integration-style tests for dangerous property validation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This reverts commit c10cefe. The commit introduced undefined regex variables (dangerousPropSplitRe, numericIndexRe) that were never added to the var block, breaking compilation. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Add compile-time validation for dangerous property names in expressions
Extends the runtime protection from #14826 by blocking dangerous JavaScript property names at compile time. Prevents prototype pollution attacks via expressions like
${{ github.constructor }}or${{ inputs.__proto__ }}.Changes
DangerousPropertyNamesinpkg/constants/constants.golists 13 blocked property names (constructor, proto, prototype, toString, valueOf, etc.)validateExpressionForDangerousProps()inpkg/workflow/expression_validation.goparses dot/bracket notation and checks property names against the blocklistvalidateSingleExpression()before other checksExample
Compilation now fails with a clear error:
Safe expressions continue to work:
${{ github.actor }} ✓ ${{ github.event.issue.number }} ✓ ${{ needs.job.outputs.result }} ✓💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.