-
Notifications
You must be signed in to change notification settings - Fork 36
Validate safe-outputs target field at compile time #11112
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
Conversation
…puts target field Co-authored-by: eaftan <4733401+eaftan@users.noreply.github.com>
- Validates target values during compilation to catch errors early - Valid values: "triggering", "*", positive integers, GitHub expressions - Rejects invalid values like "event", "0", negative numbers, random strings - Includes comprehensive unit and integration tests Co-authored-by: eaftan <4733401+eaftan@users.noreply.github.com>
Co-authored-by: eaftan <4733401+eaftan@users.noreply.github.com>
- Make suggestion for "event" conditional (only shown for event-related invalid values) - Improve GitHub expression validation to check for proper bracket order - Add comprehensive tests for edge cases in expression validation Co-authored-by: eaftan <4733401+eaftan@users.noreply.github.com>
|
|
||
| // isGitHubExpression checks if a string is a valid GitHub Actions expression | ||
| // A valid expression must have properly balanced ${{ and }} markers | ||
| func isGitHubExpression(s string) bool { |
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.
@copilot use expression parser to validate correct expression, there should be a validator already
|
@copilot look for existing isPositiveInteger helper otherwise move to stringutils package, move tests too |
|
https://gh.io/copilot-coding-agent-docs look for existing isPositiveInteger helper otherwise move to stringutils package, move tests too |
|
@copilot look for existing isPositiveInteger helper otherwise move to stringutils package, move tests too |
- Moved IsPositiveInteger function from workflow validation to pkg/stringutil - Moved associated tests to stringutil_test.go - Updated safe_outputs_target_validation.go to use stringutil.IsPositiveInteger - Removed strconv import from validation file (no longer needed) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot format the code, then merge |
- Resolved merge conflicts in pkg/stringutil files - Kept both IsPositiveInteger and StripANSIEscapeCodes functions - Recompiled all workflows successfully - All tests passing after merge Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
No existing helper found. Moved |
Add compile-time validation for safe-outputs target field
Problem
The compiler previously accepted invalid values for the
targetfield in safe-outputs configuration (e.g.,target: event), which failed at runtime instead of during compilation.Solution
Added compile-time validation in the compiler to catch invalid target values early:
Valid target values:
"triggering"(default) - targets the triggering issue/PR/discussion"*"- targets any item specified in the output"123""${{ github.event.issue.number }}"Invalid values rejected:
"event","invalid")"0")"-1")"007")Implementation Details
safe_outputs_target_validation.gowith validation logicIsPositiveIntegerutility function topkg/stringutilpackagesafe_outputs_target_validation_test.goandstringutil_test.gosafe_outputs_target_validation_integration_test.gocompiler.goExample Error Message
This provides clear, actionable guidance to users when they make a mistake.
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.