Skip to content

refactor(workflow): streamline payload validation and enhance release…#24811

Merged
MansurBesleney merged 2 commits intodevfrom
skoc/action
Feb 5, 2026
Merged

refactor(workflow): streamline payload validation and enhance release…#24811
MansurBesleney merged 2 commits intodevfrom
skoc/action

Conversation

@skoc10
Copy link
Contributor

@skoc10 skoc10 commented Feb 5, 2026

No description provided.

… notes processing in update-studio-docs workflow
@MansurBesleney MansurBesleney merged commit 0c8409f into dev Feb 5, 2026
2 checks passed
@MansurBesleney MansurBesleney deleted the skoc/action branch February 5, 2026 11:38
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 refactors a GitHub workflow that updates ABP Studio documentation by streamlining payload validation, improving release notes handling, and making the AI formatting step optional with a fallback mechanism.

Changes:

  • Replaced individual payload field validation with a loop-based approach using jq to read from GITHUB_EVENT_PATH
  • Made checkout dynamic by using client_payload.target_branch instead of hardcoded 'dev' branch
  • Simplified release notes processing with AI formatting as optional (continue-on-error) and added a fallback to prefix raw notes with bullets
  • Removed intermediate PR URL output and simplified the auto-merge command to use branch name directly

exit 1
fi
done

Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The jq command reads from GITHUB_EVENT_PATH which contains untrusted user input from the client_payload. While this is safer than direct string interpolation, the current implementation doesn't validate the format or content of the values beyond checking if they're empty or null. Consider adding validation for expected formats (e.g., version should match a version pattern, target_branch should be a valid branch name) to prevent potential injection or unexpected behavior in later steps.

Suggested change
# Validate version format (e.g., 1.2.3 or 1.2.3-rc.1)
VERSION="$(jq -r '.client_payload.version' "$GITHUB_EVENT_PATH")"
if ! [[ "$VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+([.-][A-Za-z0-9.+-]+)?$ ]]; then
echo "Invalid version format: $VERSION"
exit 1
fi
# Validate target_branch format (conservative allowed characters)
TARGET_BRANCH="$(jq -r '.client_payload.target_branch' "$GITHUB_EVENT_PATH")"
if ! [[ "$TARGET_BRANCH" =~ ^[A-Za-z0-9._\/-]+$ ]]; then
echo "Invalid target_branch format (contains forbidden characters): $TARGET_BRANCH"
exit 1
fi
# Additional safety checks for branch name
if [[ "$TARGET_BRANCH" == /* ]] || [[ "$TARGET_BRANCH" == -* ]] || [[ "$TARGET_BRANCH" == *..* ]]; then
echo "Invalid target_branch format: $TARGET_BRANCH"
exit 1
fi

Copilot uses AI. Check for mistakes.
- uses: actions/checkout@v4
with:
ref: dev
ref: ${{ github.event.client_payload.target_branch }}
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

Using the client_payload.target_branch directly for checkout without validation creates a security risk. An attacker could potentially use special git references or inject commands. The validation step only checks if the field is non-empty, but doesn't verify it's a valid, safe branch name. Consider validating that target_branch matches an expected pattern (e.g., ^[a-zA-Z0-9/_-]+$) and/or restricting to an allowed list of branches before using it in the checkout action.

Copilot uses AI. Check for mistakes.
# -----------------------------
- name: Decide final release notes
run: |
if [ -n "${{ steps.ai.outputs.response }}" ]; then
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The condition checks if steps.ai.outputs.response is non-empty using -n, but if the AI step fails with continue-on-error, the output might be undefined rather than empty. This could lead to unexpected behavior. Consider explicitly checking the step outcome (e.g., steps.ai.outcome == 'success') or verifying the output exists and is valid before using it.

Suggested change
if [ -n "${{ steps.ai.outputs.response }}" ]; then
if [ "${{ steps.ai.outcome }}" = "success" ] && [ -n "${{ steps.ai.outputs.response }}" ]; then

Copilot uses AI. Check for mistakes.
echo "${{ steps.ai.outputs.response }}" > .tmp/final-notes.txt
else
echo "AI unavailable – using raw release notes"
sed 's/^/- /' .tmp/raw-notes.txt > .tmp/final-notes.txt
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The sed command 's/^/- /' will prefix every line with "- ", but this doesn't handle cases where the raw notes might already have bullet points or other formatting. This could result in malformed markdown (e.g., "- - item" or "- # Heading"). Consider checking if the raw notes already have formatting, or use a more robust fallback that handles various input formats.

Suggested change
sed 's/^/- /' .tmp/raw-notes.txt > .tmp/final-notes.txt
awk '{
# Blank or whitespace-only lines
if ($0 ~ /^[[:space:]]*$/) {
print $0;
}
# Lines already starting with common Markdown list or heading patterns:
# - unordered bullets: -, *, +
# - ordered bullets: 1., 2., etc.
# - headings: #, ##, etc.
else if ($0 ~ /^[[:space:]]*([-*+] |\d+\. |#)/) {
print $0;
}
# Otherwise, prefix the line with "- "
else {
print "- " $0;
}
}' .tmp/raw-notes.txt > .tmp/final-notes.txt

Copilot uses AI. Check for mistakes.
Comment on lines +103 to +118
URL="${{ github.event.client_payload.url }}"

mkdir -p docs/en/studio
touch "$FILE"

if ! grep -q "## Version $VERSION" "$FILE"; then
{
echo "## Version $VERSION - $NAME"
echo ""
echo "${{ steps.ai.outputs.response }}"
echo ""
echo "[Release Link](${{ github.event.client_payload.url }})"
echo ""
echo "---"
echo ""
} | cat - "$FILE" > "$FILE.new"
mv "$FILE.new" "$FILE"
else
echo "Release notes already exist"
if grep -q "## Version $VERSION" "$FILE"; then
echo "Release notes already contain $VERSION, skipping"
exit 0
fi

{
echo "## Version $VERSION - $NAME"
echo ""
cat .tmp/final-notes.txt
echo ""
echo "[Release Link]($URL)"
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The URL variable is extracted from client_payload.url and used directly in markdown link syntax without validation or sanitization. While markdown links are generally safe, malicious URLs could include JavaScript protocols (javascript:) or other unexpected schemes that might be problematic depending on where the documentation is rendered. Consider validating that the URL starts with https:// or http:// before using it.

Copilot uses AI. Check for mistakes.
--squash \
--auto \
--admin
gh pr merge "$BRANCH" --squash --auto --admin
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The command 'gh pr merge "$BRANCH"' uses the branch name instead of the PR URL or number. While GitHub CLI can accept branch names, this approach is less explicit and could be ambiguous if multiple PRs exist from the same branch. The previous implementation using the PR URL from the output was more reliable. Consider either: 1) restoring the PR URL output and using it, or 2) using 'gh pr merge --head "$BRANCH"' to be more explicit, or 3) capturing and using the PR number.

Suggested change
gh pr merge "$BRANCH" --squash --auto --admin
gh pr merge --head "$BRANCH" --squash --auto --admin

Copilot uses AI. Check for mistakes.
cat format.txt >> $GITHUB_OUTPUT
echo "EOF" >> $GITHUB_OUTPUT
mkdir -p .tmp
jq -r '.client_payload.notes' "$GITHUB_EVENT_PATH" > .tmp/raw-notes.txt
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The jq command extracts client_payload.notes which could contain arbitrary content from an external source. While saving it to a file is generally safer than direct interpolation, the content is later used in shell commands (line 92) and passed to AI. Consider sanitizing or validating the notes content to ensure it doesn't contain malicious content that could be exploited in later steps.

Suggested change
jq -r '.client_payload.notes' "$GITHUB_EVENT_PATH" > .tmp/raw-notes.txt
# Extract notes and remove non-printable control characters (except tab/newline/carriage return)
jq -r '.client_payload.notes' "$GITHUB_EVENT_PATH" \
| tr -d '\000-\010\013\014\016-\037\177' \
> .tmp/raw-notes.txt

Copilot uses AI. Check for mistakes.
Name: ${{ github.event.client_payload.name }}
Raw notes:
Release notes:
${{ github.event.client_payload.notes }}
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The step passes untrusted client_payload.notes directly to the AI prompt via GitHub expression interpolation. This could allow prompt injection attacks where malicious notes content manipulates the AI's behavior or extracts sensitive information. Consider reading the notes from the file created in line 59 instead, or sanitize the content before passing it to the AI prompt.

Copilot uses AI. Check for mistakes.
Comment on lines +110 to +125
exit 0
fi

{
echo "## Version $VERSION - $NAME"
echo ""
cat .tmp/final-notes.txt
echo ""
echo "[Release Link]($URL)"
echo ""
echo "---"
echo ""
} | cat - "$FILE" > "$FILE.new"

mv "$FILE.new" "$FILE"

Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

In a multi-line shell script within a GitHub Actions step, 'exit 0' terminates only that step's script, not the entire job. This means the code after 'exit 0' (lines 113-124) will never execute when the version is found, which is the intended behavior. However, this creates dead code after the exit statement in the same step. Consider restructuring to use conditional logic (if/else) or splitting into separate steps with conditional execution for better clarity and maintainability.

Suggested change
exit 0
fi
{
echo "## Version $VERSION - $NAME"
echo ""
cat .tmp/final-notes.txt
echo ""
echo "[Release Link]($URL)"
echo ""
echo "---"
echo ""
} | cat - "$FILE" > "$FILE.new"
mv "$FILE.new" "$FILE"
else
{
echo "## Version $VERSION - $NAME"
echo ""
cat .tmp/final-notes.txt
echo ""
echo "[Release Link]($URL)"
echo ""
echo "---"
echo ""
} | cat - "$FILE" > "$FILE.new"
mv "$FILE.new" "$FILE"
fi

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants