Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions .github/workflows/vale.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@ jobs:
- name: Get changed files
id: changed
run: |
FILES=$(git diff --name-only --diff-filter=d ${{ github.event.pull_request.base.sha }}...${{ github.event.pull_request.head.sha }} -- '*.md' '*.mdx' | tr '\n' ' ')
echo "files=$FILES" >> "$GITHUB_OUTPUT"
FILES=$(git diff --name-only --diff-filter=d ${{ github.event.pull_request.base.sha }}...${{ github.event.pull_request.head.sha }} -- '*.md' '*.mdx')
if [ -z "$FILES" ]; then
echo "skip=true" >> "$GITHUB_OUTPUT"
echo 'files=all' >> "$GITHUB_OUTPUT"
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

When no markdown files are changed (skip=true), the code sets files=all on line 18. However, all subsequent steps that consume this output are gated on steps.changed.outputs.skip == 'false', so this value is never actually used. The string 'all' being stored as a fallback is misleading and potentially dangerous: if the skip condition ever fails to propagate correctly or is evaluated unexpectedly, the Vale action would receive files: all (a literal string, not a valid file path) rather than the intended JSON array, causing unpredictable behavior. The files=all line should simply be omitted from the skip=true branch since it serves no purpose.

Suggested change
echo 'files=all' >> "$GITHUB_OUTPUT"

Copilot uses AI. Check for mistakes.
else
echo "skip=false" >> "$GITHUB_OUTPUT"
JSON=$(echo "$FILES" | jq -R -s -c 'split("\n") | map(select(. != ""))')
echo "files=$JSON" >> "$GITHUB_OUTPUT"
fi
- name: Enable Corepack
if: steps.changed.outputs.skip == 'false'
Expand All @@ -38,7 +40,7 @@ jobs:
if: steps.changed.outputs.skip == 'false'
run: |
sudo apt-get install ripgrep
FILES="${{ steps.changed.outputs.files }}"
FILES=$(echo '${{ steps.changed.outputs.files }}' | jq -r '.[]' | tr '\n' ' ')
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

On line 43, ${{ steps.changed.outputs.files }} is interpolated directly into the run shell script. This is a GitHub Actions security anti-pattern: any content in that output is injected verbatim into the shell command before it is executed. While the files output is derived from git diff output (which in a pull_request event is determined by the branch's file paths), it is still best practice to pass step outputs via environment variables rather than direct expression interpolation in run scripts, to prevent shell injection. The value should be passed through an environment variable (e.g., env: FILES_JSON: ${{ steps.changed.outputs.files }}) and then referenced as $FILES_JSON in the shell.

Copilot uses AI. Check for mistakes.
if [ -n "$FILES" ]; then
rg -l0 '\$\[[^\]]*\]' -- $FILES | xargs -0 perl -i -pe 's/\$\[[^\]]*\]/PICKLEVAR/g' || true
Comment on lines +43 to 45
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

On line 45, the $FILES variable is unquoted in the rg command. After tr '\n' ' ' converts newlines to spaces, the resulting space-separated list is word-split by the shell when passed to rg. If any filename contains spaces (or glob characters), it will be incorrectly split into multiple arguments, causing rg to fail or search the wrong paths. The variable should either be quoted or the file list should be processed in a way that preserves filenames with spaces (e.g., use readarray/null-delimited processing).

Suggested change
FILES=$(echo '${{ steps.changed.outputs.files }}' | jq -r '.[]' | tr '\n' ' ')
if [ -n "$FILES" ]; then
rg -l0 '\$\[[^\]]*\]' -- $FILES | xargs -0 perl -i -pe 's/\$\[[^\]]*\]/PICKLEVAR/g' || true
FILES=$(echo '${{ steps.changed.outputs.files }}' | jq -r '.[]')
if [ -n "$FILES" ]; then
printf '%s\n' "$FILES" | rg -l0 '\$\[[^\]]*\]' --files-from - | xargs -0 perl -i -pe 's/\$\[[^\]]*\]/PICKLEVAR/g' || true

Copilot uses AI. Check for mistakes.
fi
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,3 @@ spec:
## Additional resources

- [Global network policy](../../reference/resources/globalnetworkpolicy.mdx)
- [Hands-on workshop: Learn how to implement eBPF security policies and XDP](https://www.tigera.io/tutorials/?_sf_s=%27calico%20ebpf%20and%20xdp%27)
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,3 @@ spec:
## Additional resources

- [Global network policy](../../reference/resources/globalnetworkpolicy.mdx)
- [Hands-on workshop: Learn how to implement eBPF security policies and XDP](https://www.tigera.io/tutorials/?_sf_s=%27calico%20ebpf%20and%20xdp%27)
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,3 @@ spec:
## Additional resources

- [Global network policy](../../reference/resources/globalnetworkpolicy.mdx)
- [Hands-on workshop: Learn how to implement eBPF security policies and XDP](https://www.tigera.io/tutorials/?_sf_s=%27calico%20ebpf%20and%20xdp%27)
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,3 @@ spec:
## Additional resources

- [Global network policy](../../reference/resources/globalnetworkpolicy.mdx)
- [Hands-on workshop: Learn how to implement eBPF security policies and XDP](https://www.tigera.io/tutorials/?_sf_s=%27calico%20ebpf%20and%20xdp%27)
Loading