Skip to content
Open
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: 6 additions & 2 deletions .github/forbidden-words.json
Original file line number Diff line number Diff line change
@@ -1,22 +1,26 @@
{
"$comment": "Forbidden words/phrases checked against newly added lines in pull requests by .github/scripts/check-forbidden-words.sh. Each rule has a regex 'pattern' (extended POSIX, evaluated case-insensitively unless 'caseSensitive' is true) and a 'message' shown when matched. Add 'excludePaths' globs to skip files for a specific rule.",
"$comment": "Forbidden words/phrases checked against newly added lines in pull requests by .github/scripts/check-forbidden-words.sh. Each rule has a regex 'pattern' (extended POSIX/PCRE, evaluated case-insensitively unless 'caseSensitive' is true), a required 'replacement' string, and a 'message' shown when matched. The check still fails when a forbidden phrase is found; in addition, .github/workflows/forbidden-words-suggest.yml posts an inline PR review 'suggestion' that rewrites the offending line by substituting each matched pattern with its 'replacement'. The 'replacement' value is inserted verbatim (matching honors 'caseSensitive', but the replacement text is never case-adjusted and does not support backreferences). Add 'excludePaths' globs to skip files for a specific rule.",
"excludePaths": [
"**/*.lock.yml",
"pnpm-lock.yaml",
".github/forbidden-words.json",
".github/scripts/check-forbidden-words.sh"
".github/scripts/check-forbidden-words.sh",
".github/scripts/post-forbidden-word-suggestions.sh"
],
"rules": [
{
"pattern": "\\.NET Aspire",
"replacement": "Aspire",
"message": "Use \"Aspire\" instead of \".NET Aspire\"."
},
{
"pattern": "dotnet aspire",
"replacement": "Aspire",
"message": "Use \"Aspire\" instead of \"dotnet aspire\"."
},
{
"pattern": "\\bapp host\\b",
"replacement": "AppHost",
"message": "Use \"AppHost\" instead of \"app host\"."
}
]
Expand Down
150 changes: 126 additions & 24 deletions .github/scripts/check-forbidden-words.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,22 @@
# pre-existing text is never flagged. Rules are defined in
# .github/forbidden-words.json.
#
# In addition to failing when a forbidden phrase is found, this script records
# a machine-readable list of findings (see FINDINGS_FILE below). Each finding
# includes the corrected line produced by substituting every matched rule's
# 'pattern' with its 'replacement'. A companion workflow
# (.github/workflows/forbidden-words-suggest.yml) turns those findings into
# inline pull request review suggestions.
#
# Usage:
# check-forbidden-words.sh <base_sha> <head_sha>
#
# Environment overrides:
# CONFIG_FILE Path to the rules file (default: .github/forbidden-words.json)
# CONFIG_FILE Path to the rules file (default: .github/forbidden-words.json)
# FINDINGS_FILE When set, a JSON document of findings is written here (even
# when violations are present). Shape:
# { "findings": [ { "file", "line", "original",
# "suggestion", "messages", "patterns" } ] }
#
# Exits non-zero when at least one forbidden phrase is found.

Expand All @@ -20,23 +31,56 @@ BASE_SHA="${1:?base SHA required}"
HEAD_SHA="${2:?head SHA required}"
MERGE_BASE="$(git merge-base "$BASE_SHA" "$HEAD_SHA")" || { echo "::error::Unable to compute merge-base of $BASE_SHA and $HEAD_SHA"; exit 2; }
CONFIG_FILE="${CONFIG_FILE:-.github/forbidden-words.json}"
FINDINGS_FILE="${FINDINGS_FILE:-}"

if [[ ! -f "$CONFIG_FILE" ]]; then
echo "::error::Config file not found: $CONFIG_FILE"
exit 2
fi

command -v jq >/dev/null 2>&1 || { echo "::error::jq is required"; exit 2; }
command -v perl >/dev/null 2>&1 || { echo "::error::perl is required"; exit 2; }

# Read global exclude globs (newline separated).
mapfile -t GLOBAL_EXCLUDES < <(jq -r '.excludePaths // [] | .[]' "$CONFIG_FILE")

rule_count=$(jq '(.rules // []) | length' "$CONFIG_FILE")

# Emit an empty findings file up front so callers always have a valid document.
write_findings() {
[[ -z "$FINDINGS_FILE" ]] && return 0
if [[ -s "$FINDINGS_TMP" ]]; then
jq -s '{findings: .}' "$FINDINGS_TMP" > "$FINDINGS_FILE"
else
printf '{"findings":[]}\n' > "$FINDINGS_FILE"
fi
}

FINDINGS_TMP="$(mktemp)"
trap 'rm -f "$FINDINGS_TMP"' EXIT

if [[ "$rule_count" -eq 0 ]]; then
echo "No rules defined in $CONFIG_FILE; nothing to check."
write_findings
exit 0
fi

# Every rule must define a non-empty replacement so a suggestion can be built.
missing_replacement=$(jq '[.rules[] | select((.replacement // "") == "")] | length' "$CONFIG_FILE")
if [[ "$missing_replacement" -ne 0 ]]; then
echo "::error::Every rule in $CONFIG_FILE must define a non-empty \"replacement\". Found $missing_replacement rule(s) without one."
# Still emit an (empty) findings document so callers that always consume it
# see a valid artifact, even though we fail the check.
write_findings
exit 2
fi

# Preload rule fields to avoid re-invoking jq for every line.
mapfile -t PATTERNS < <(jq -r '.rules[].pattern' "$CONFIG_FILE")
mapfile -t REPLACEMENTS < <(jq -r '.rules[].replacement' "$CONFIG_FILE")
mapfile -t MESSAGES < <(jq -r '.rules[] | (.message // "")' "$CONFIG_FILE")
mapfile -t CASE_SENSITIVE < <(jq -r '.rules[] | (.caseSensitive // false)' "$CONFIG_FILE")

# Returns 0 if $1 matches any of the shell globs passed as remaining args.
matches_any_glob() {
local path="$1"; shift
Expand All @@ -51,6 +95,20 @@ matches_any_glob() {
return 1
}

# Substitutes every occurrence of a pattern with its replacement, inserting the
# replacement verbatim. Perl inserts the value of $r without reprocessing its
# contents, so no backreferences ($1) or escape sequences are interpreted.
# Matching honors the rule's case sensitivity.
apply_replacement() {
CONTENT="$1" PAT="$2" REP="$3" CS="$4" perl -e '
my $c = $ENV{CONTENT};
my $p = $ENV{PAT};
my $r = $ENV{REP};
if ($ENV{CS} eq "true") { $c =~ s/$p/$r/g; } else { $c =~ s/$p/$r/gi; }
print $c;
'
}

violations=0

# List the files changed between base and head (added/copied/modified/renamed).
Expand All @@ -64,6 +122,17 @@ for file in "${CHANGED_FILES[@]}"; do
continue
fi

# Precompute which rules apply to this file (per-rule excludePaths).
rule_applies=()
for ((r = 0; r < rule_count; r++)); do
mapfile -t RULE_EXCLUDES < <(jq -r ".rules[$r].excludePaths // [] | .[]" "$CONFIG_FILE")
if ((${#RULE_EXCLUDES[@]} > 0)) && matches_any_glob "$file" "${RULE_EXCLUDES[@]}"; then
rule_applies[$r]=0
else
rule_applies[$r]=1
fi
done

# Collect added lines as "<line_number>:<content>". --unified=0 keeps hunks
# tight so we can track new-file line numbers from the @@ headers.
diff_output=$(git diff --unified=0 "$MERGE_BASE" "$HEAD_SHA" -- "$file")
Expand Down Expand Up @@ -91,41 +160,74 @@ for file in "${CHANGED_FILES[@]}"; do

[[ -z "$added_lines" ]] && continue

for ((r = 0; r < rule_count; r++)); do
pattern=$(jq -r ".rules[$r].pattern" "$CONFIG_FILE")
message=$(jq -r ".rules[$r].message // \"\"" "$CONFIG_FILE")
case_sensitive=$(jq -r ".rules[$r].caseSensitive // false" "$CONFIG_FILE")

# Per-rule path excludes.
mapfile -t RULE_EXCLUDES < <(jq -r ".rules[$r].excludePaths // [] | .[]" "$CONFIG_FILE")
if ((${#RULE_EXCLUDES[@]} > 0)) && matches_any_glob "$file" "${RULE_EXCLUDES[@]}"; then
continue
fi

grep_opts=(-P)
if [[ "$case_sensitive" != "true" ]]; then
grep_opts+=(-i)
fi
# Check each added line against every applicable rule, aggregating all matches
# on the line into a single corrected line (at most one suggestion per line).
while IFS= read -r entry; do

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Something the agent found that I'm not 100% but worth double checking:

Not on this exact line — it's about the awk that builds $added_lines just above (the /^\+\+\+/ { next } rule). With --unified=0, an added line whose content starts with ++ shows up as +++... in the diff, so that rule skips it as if it were the +++ b/file header.
Two knock-on effects: the forbidden phrase on that line never gets flagged (gate silently passes), and since line++ is skipped too, every later added line in the hunk ends up off-by-one — so otherwise-valid suggestions land on the wrong line or get rejected (422). Matching the header form only should fix it, e.g. /^\+\+\+ / { next } (note the trailing space).

[[ -z "$entry" ]] && continue
lineno="${entry%%:*}"
content="${entry#*:}"

corrected="$content"
line_has_match=0
suggestion_available=1
line_messages=()
line_patterns=()

for ((r = 0; r < rule_count; r++)); do
[[ "${rule_applies[$r]}" == "1" ]] || continue

pattern="${PATTERNS[$r]}"
replacement="${REPLACEMENTS[$r]}"
message="${MESSAGES[$r]}"
case_sensitive="${CASE_SENSITIVE[$r]}"

grep_opts=(-P)
if [[ "$case_sensitive" != "true" ]]; then
grep_opts+=(-i)
fi

# Check each added line's content (line number stripped) against the rule.
while IFS= read -r entry; do
[[ -z "$entry" ]] && continue
lineno="${entry%%:*}"
content="${entry#*:}"
if printf '%s' "$content" | grep -q "${grep_opts[@]}" -- "$pattern"; then
line_has_match=1
violations=$((violations + 1))
line_messages+=("$message")
line_patterns+=("$pattern")

printf '::error file=%s,line=%s::Forbidden phrase (/%s/): %s\n' \
"$file" "$lineno" "$pattern" "$message"
printf ' %s:%s: %s\n' "$file" "$lineno" "$content"
violations=$((violations + 1))

if ! updated="$(apply_replacement "$corrected" "$pattern" "$replacement" "$case_sensitive")"; then
suggestion_available=0
echo "::warning file=$file,line=$lineno::Could not build an inline suggestion for this line because the matched pattern is not supported by the replacement engine."
else
corrected="$updated"
fi
fi
done <<< "$added_lines"
done
done

if [[ "$line_has_match" -eq 1 && "$suggestion_available" -eq 1 ]]; then
messages_json=$(printf '%s\n' "${line_messages[@]}" | jq -R . | jq -s .)
patterns_json=$(printf '%s\n' "${line_patterns[@]}" | jq -R . | jq -s .)
jq -cn \
--arg file "$file" \
--argjson line "$lineno" \
--arg original "$content" \
--arg suggestion "$corrected" \
--argjson messages "$messages_json" \
--argjson patterns "$patterns_json" \
'{file: $file, line: $line, original: $original, suggestion: $suggestion, messages: $messages, patterns: $patterns}' \
>> "$FINDINGS_TMP"
fi
done <<< "$added_lines"
done

write_findings

echo
if [[ "$violations" -gt 0 ]]; then
echo "Found $violations forbidden phrase occurrence(s) in added lines."
echo "Update the wording, or adjust .github/forbidden-words.json if a rule is wrong."
echo "Inline suggestions will be posted on the pull request where possible."
exit 1
fi

Expand Down
132 changes: 132 additions & 0 deletions .github/scripts/post-forbidden-word-suggestions.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
#!/usr/bin/env bash
#
# Posts inline pull request review suggestions for forbidden-word findings.
#
# Reads the findings document produced by check-forbidden-words.sh and, for each
# finding, posts an inline review comment containing a GitHub ```suggestion```
# block that rewrites the offending line. A suggestion is skipped when a review
# comment already exists on that (path, line), so re-runs do not pile up
# duplicates.
#
# This script only reads the findings JSON as data and calls the GitHub API; it
# never checks out or executes pull request code, so it is safe to run from the
# privileged `workflow_run` context.
#
# Required environment:
# GH_TOKEN Token with `pull-requests: write` (the Aspire bot app token).
# REPO "owner/repo".
# PR_NUMBER Pull request number.
# HEAD_SHA Commit SHA the findings were computed against.
# FINDINGS_FILE Path to the findings JSON document.

set -euo pipefail

: "${GH_TOKEN:?GH_TOKEN required}"
: "${REPO:?REPO required}"
: "${PR_NUMBER:?PR_NUMBER required}"
: "${HEAD_SHA:?HEAD_SHA required}"
: "${FINDINGS_FILE:?FINDINGS_FILE required}"

command -v jq >/dev/null 2>&1 || { echo "::error::jq is required"; exit 2; }
command -v gh >/dev/null 2>&1 || { echo "::error::gh is required"; exit 2; }

if [[ ! -f "$FINDINGS_FILE" ]]; then
echo "Findings file not found ($FINDINGS_FILE); nothing to suggest."
exit 0
fi

# The findings document originates from a PR-scoped run, so treat it as
# untrusted input: if it isn't valid JSON with a findings array, skip quietly
# instead of failing this best-effort workflow.
if ! jq -e '.findings | type == "array"' "$FINDINGS_FILE" >/dev/null 2>&1; then
echo "::warning::Findings file is missing or not a valid findings document ($FINDINGS_FILE); skipping suggestions."
exit 0
fi

total=$(jq '.findings | length' "$FINDINGS_FILE")
if [[ "$total" -eq 0 ]]; then
echo "No findings; nothing to suggest."
exit 0
fi

# Existing review comments on the PR, as {path, line} objects (line falls back to
# original_line for outdated comments). Used to avoid duplicate suggestions.
# If the listing fails (permissions, rate limiting, transient network), skip
# rather than risk posting duplicates or failing this best-effort workflow.
if ! existing_raw=$(gh api --paginate "/repos/$REPO/pulls/$PR_NUMBER/comments" \
--jq '.[] | {path: .path, line: (.line // .original_line)}' 2>/tmp/list-comments.err); then
echo "::warning::Could not list existing PR comments; skipping suggestions to avoid duplicates."
sed 's/^/ gh: /' /tmp/list-comments.err >&2 || true
exit 0
fi
existing_json=$(printf '%s' "$existing_raw" | jq -s '.')

# Build the list of comments to post: findings that don't already have a comment
# on the same (path, line), rendered as a suggestion block.
comments_json=$(jq -n \
--slurpfile f "$FINDINGS_FILE" \
--argjson existing "$existing_json" \
'
$f[0].findings
| map(select(. as $find
| ($existing | any(.path == $find.file and .line == $find.line)) | not))
| map({
path: .file,
line: .line,
side: "RIGHT",
body: ((.messages | join("\n")) + "\n\n```suggestion\n" + .suggestion + "\n```")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One thing that might still be worth hardening: messages comes straight out of the artifact, which is built by the PR's own copy of the scan script + forbidden-words.json. So a fork author fully controls it, and we post it verbatim as the Aspire bot — meaning they can slip @mentions in (bot pings arbitrary people) or drop fake "approved by maintainers"-style text into the comment body.

The coordinates are now safely re-resolved from the workflow_run identity, which is great — it's just the body content that's still trusted. Could we re-derive the message text from the trusted forbidden-words.json on main (keyed off patterns, which we already carry in the finding) instead of echoing the artifact's messages?

})
')

count=$(jq 'length' <<< "$comments_json")
if [[ "$count" -eq 0 ]]; then
echo "All $total finding(s) already have a comment on their line; nothing to post."
exit 0
fi

echo "Posting $count new inline suggestion(s) (of $total finding(s))."

# Prefer a single batched review so all suggestions appear together.
review_payload=$(jq -n \
--arg commit "$HEAD_SHA" \
--argjson comments "$comments_json" \
'{
commit_id: $commit,
event: "COMMENT",
body: "Automated wording suggestions from the forbidden-words check. Apply the suggestions to resolve them.",
comments: $comments
}')

if printf '%s' "$review_payload" \
| gh api --method POST "/repos/$REPO/pulls/$PR_NUMBER/reviews" --input - >/dev/null 2>/tmp/review.err; then
echo "Posted a review with $count inline suggestion(s)."
exit 0
fi

echo "::warning::Batched review failed; falling back to individual comments."
sed 's/^/ gh: /' /tmp/review.err >&2 || true

# Fallback: post each suggestion independently so one bad line can't block the rest.
ok=0
fail=0
while IFS= read -r c; do
[[ -z "$c" ]] && continue
path=$(jq -r '.path' <<< "$c")
line=$(jq -r '.line' <<< "$c")
if jq -n \
--arg commit "$HEAD_SHA" \
--arg path "$path" \
--argjson line "$line" \
--arg body "$(jq -r '.body' <<< "$c")" \
'{commit_id: $commit, path: $path, line: $line, side: "RIGHT", body: $body}' \
| gh api --method POST "/repos/$REPO/pulls/$PR_NUMBER/comments" --input - >/dev/null 2>&1; then
ok=$((ok + 1))
else
fail=$((fail + 1))
echo "::warning::Could not post suggestion on $path:$line."
fi
done < <(jq -c '.[]' <<< "$comments_json")

echo "Individual fallback: $ok posted, $fail failed."
# Best-effort: never fail the workflow just because suggestions couldn't post.
exit 0
Loading
Loading