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
20 changes: 20 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# clang-tidy config for the OneWifi CI gate. Step 0 = ADVISORY (nothing fatal).
#
# The Checks list disables the existing style and possibly false-positives members of bugprone-*,
# leaving the real-bug checks.
#
# To enable gating: move a proven-clean check into WarningsAsErrors
# (e.g. 'bugprone-not-null-terminated-result') once its sites are fixed.
Checks: >
-*,
bugprone-*,
-bugprone-easily-swappable-parameters,
-bugprone-branch-clone,
-bugprone-narrowing-conversions,
-bugprone-implicit-widening-of-multiplication-result,
-bugprone-assignment-in-if-condition,
-bugprone-reserved-identifier,
-bugprone-multi-level-implicit-pointer-conversion,
-bugprone-too-small-loop-variable,
-bugprone-switch-missing-default-case
WarningsAsErrors: ''
137 changes: 137 additions & 0 deletions .github/actions/pr-context/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
name: 'Establish trusted PR context'
description: >
Stage-2 helper for the build-check workflow_run (adding comments part). Downloads a
named artifact produced by the triggering stage-1 run, STRICTLY validates its
pr-meta.env, binds the recorded PR number back to the triggering run's head
repo/branch (values a fork cannot forge), and re-checks head-sha freshness.
No check-out or executing PR code. It only reads the artifact as passive
data. Every stage-2 posting job (clang-format / build / clang-tidy) pipes its
trust decision through this one action, so the security-critical logic lives in
one place only.

inputs:
artifact-name:
description: 'Name of the stage-1 artifact to download (must contain pr-meta.env).'
required: true
run-id:
description: 'github.event.workflow_run.id — the run that produced the artifact.'
required: true
github-token:
description: 'Token with actions:read (download) + pull-requests:read (identity/freshness).'
required: true
repository:
description: 'github.repository — owner/name of the BASE repo the comment posts to.'
required: true
expected-repo:
description: 'github.event.workflow_run.head_repository.full_name (the fork/branch source).'
required: true
expected-branch:
description: 'github.event.workflow_run.head_branch.'
required: true

outputs:
found:
description: '"true" only if the artifact was present AND fully validated.'
value: ${{ steps.gate.outputs.found }}
fresh:
description: '"true" if the PR head still points at the recorded head_sha.'
value: ${{ steps.gate.outputs.fresh }}
pr_number:
description: 'Validated PR number (bare integer). Empty when found=false.'
value: ${{ steps.gate.outputs.pr_number }}
head_sha:
description: 'Validated PR head sha (40-hex) recorded by stage 1. Empty when found=false.'
value: ${{ steps.gate.outputs.head_sha }}
dir:
description: 'Directory the artifact was extracted into (== artifact-name).'
value: ${{ inputs.artifact-name }}

runs:
using: composite
steps:
# Each artifact lands in its own directory (path: == name) so multiple
# pr-context calls in one job never clobber each other's files.
- name: Download stage-1 artifact (${{ inputs.artifact-name }})
id: dl
continue-on-error: true # a missing artifact is normal → found=false
uses: actions/download-artifact@v7
with:
name: ${{ inputs.artifact-name }}
path: ${{ inputs.artifact-name }}
run-id: ${{ inputs.run-id }}
github-token: ${{ inputs.github-token }}

- name: Validate metadata, identity, and freshness
id: gate
shell: bash
env:
GH_TOKEN: ${{ inputs.github-token }}
DL_OUTCOME: ${{ steps.dl.outcome }}
ART_DIR: ${{ inputs.artifact-name }}
REPO: ${{ inputs.repository }}
EXPECTED_REPO: ${{ inputs.expected-repo }}
EXPECTED_BRANCH: ${{ inputs.expected-branch }}
run: |
# No `-e`: this script drives its own control flow and fails CLOSED with
# explicit exits. Pipefail so a failed producer in a pipe is not masked.
set -uo pipefail
found=false; fresh=false; pr_number=; head_sha=

if [ "$DL_OUTCOME" != "success" ]; then
echo "No '$ART_DIR' artifact on the triggering run — nothing to post for this job."
else
META="$ART_DIR/pr-meta.env"
if [ ! -f "$META" ]; then
echo "::error::Artifact '$ART_DIR' present but pr-meta.env is missing — refusing."
exit 1
fi

# 1. FORMAT : accept only a bare integer (PR no.) and a sha. The file
# came from a job that may have run fork code, so it is attacker-
# controlled: never `source` it (env injection)! Parse each field instead.
pr_number="$(grep -oP '^pr_number=\K[0-9]+$' "$META" | head -1 || true)"
head_sha="$(grep -oP '^head_sha=\K[0-9a-f]{40}$' "$META" | head -1 || true)"
if [ -z "$pr_number" ] || [ -z "$head_sha" ]; then
echo "::error::Malformed pr-meta.env in '$ART_DIR'; refusing to continue."
exit 1
fi

# 2. IDENTITY : a valid-looking number could still be a different PR,
# letting a fork post onto some unrelated PR. Bind pr_number back to the
# head repo+branch GitHub recorded for this workflow_run, as this can't be
# forged. (Whether head_sha is fresh is checked below.)
if ! gh api "/repos/$REPO/pulls/$pr_number" > "$ART_DIR/pr.json"; then
echo "::error::Could not read PR #$pr_number from the API."
exit 1
fi
pr_repo="$(jq -r '.head.repo.full_name' "$ART_DIR/pr.json")"
pr_branch="$(jq -r '.head.ref' "$ART_DIR/pr.json")"
if [ "$pr_repo" != "$EXPECTED_REPO" ] || [ "$pr_branch" != "$EXPECTED_BRANCH" ]; then
echo "::error::PR #$pr_number ($pr_repo:$pr_branch) does not belong to the triggering run ($EXPECTED_REPO:$EXPECTED_BRANCH); refusing."
exit 1
fi
found=true

# 3. FRESHNESS : concurrency only cancels an inflight run. Run that
# already moved past, can still post outdated content. Fail
# on an empty read (which means transient API failure), and mark
# stale only on if different sha is observed. Callers decide
# whether to skip a stale post.
current_sha="$(jq -r '.head.sha' "$ART_DIR/pr.json")"
if [ -z "$current_sha" ] || [ "$current_sha" = "null" ]; then
echo "::error::Could not read current PR head sha."
exit 1
fi
if [ "$current_sha" = "$head_sha" ]; then
fresh=true
else
echo "PR #$pr_number moved past $head_sha (now $current_sha) — callers should skip stale posts."
fi
fi

{
echo "found=$found"
echo "fresh=$fresh"
echo "pr_number=$pr_number"
echo "head_sha=$head_sha"
} >> "$GITHUB_OUTPUT"
98 changes: 98 additions & 0 deletions .github/actions/sticky-comment/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
name: 'Upsert a sticky PR comment'
description: >
Create-or-update a single PR issue comment identified by a hidden HTML marker,
so re-runs replace the previous body instead of adding new comments. Used for
the gcc build summary and the clang-tidy summary (the two "unify as one sticky
comment" payloads). Trusted stage-2 only: the caller must already have
established a validated pr-number via the pr-context action.

inputs:
github-token:
description: 'Token with pull-requests:write.'
required: true
repository:
description: 'github.repository — owner/name of the base repo.'
required: true
pr-number:
description: 'Validated PR number (from pr-context).'
required: true
marker:
description: 'Stable id for this comment slot, e.g. "build-summary" or "clang-tidy". One sticky comment per marker.'
required: true
body-file:
description: 'Path to a file holding the Markdown body to post.'
required: true
bot-login:
description: 'Login of the identity that owns our comments (for the ownership match). Change this if you switch to a GitHub App token.'
required: false
default: 'github-actions[bot]'

runs:
using: composite
steps:
- name: Upsert sticky comment (${{ inputs.marker }})
shell: bash
env:
GH_TOKEN: ${{ inputs.github-token }}
REPO: ${{ inputs.repository }}
PR: ${{ inputs.pr-number }}
MARKER: ${{ inputs.marker }}
BODY_FILE: ${{ inputs.body-file }}
BOT_LOGIN: ${{ inputs.bot-login }}
run: |
set -uo pipefail
if [ ! -f "$BODY_FILE" ]; then
echo "::error::body-file '$BODY_FILE' not found."
exit 1
fi
# Hidden marker: lets the next run find our comment for this slot without
# relying on any actor field that crossed the trust boundary.
TAG="<!-- onewifi-ci:${MARKER} -->"
{ printf '%s\n\n' "$TAG"; cat "$BODY_FILE"; } > sticky-body.md

# Cap the body. GitHub rejects issue comments over ~65536 bytes, and the
# POST/PATCH would fail after everything else succeeded. Truncate with a
# note. if the cut lands inside a ``` code fence, close it first so the
# note isn't swallowed into the block.
MAXB=60000
if [ "$(wc -c < sticky-body.md)" -gt "$MAXB" ]; then
# iconv -c drops any invalid/incomplete UTF-8 the byte-cut may leave
# (the summaries contain emoji) so the JSON body stays valid.
head -c "$MAXB" sticky-body.md | iconv -f utf-8 -t utf-8 -c > sticky-body.cut
mv sticky-body.cut sticky-body.md
if [ $(( $(grep -c '```' sticky-body.md) % 2 )) -ne 0 ]; then
printf '\n```\n' >> sticky-body.md
fi
printf '\n\n_… truncated at %s bytes; see workflow run summary for full output._\n' "$MAXB" >> sticky-body.md
fi

# Find an existing comment we own for this marker. "Ours" here means the bot
# identity (login + type == Bot) AND our marker. Login-matched so we
# can't patch some other bot's comment that only quoted our marker.
# $BOT_LOGIN and $TAG are workflow-trusted (not fork data), so shell
# interpolation into the filter is safe.
existing="$(gh api "/repos/$REPO/issues/$PR/comments" --paginate \
--jq ".[] | select(.user.type == \"Bot\" and .user.login == \"$BOT_LOGIN\") | select(.body | contains(\"$TAG\")) | .id" \
2>/dev/null | head -1 || true)"

# -f (raw-field) sends the body as a plain string — no gh type-inference,
# no dependence on the '@file' loader. Bodies are small (capped upstream),
# so passing via one argv is safe.
body="$(cat sticky-body.md)"
if [ -n "$existing" ]; then
if gh api --method PATCH "/repos/$REPO/issues/comments/$existing" \
-f body="$body" >/dev/null; then
echo "Updated sticky '$MARKER' comment ($existing) on PR #$PR."
else
echo "::error::Failed to update sticky '$MARKER' comment $existing."
exit 1
fi
else
if gh api --method POST "/repos/$REPO/issues/$PR/comments" \
-f body="$body" >/dev/null; then
echo "Created sticky '$MARKER' comment on PR #$PR."
else
echo "::error::Failed to create sticky '$MARKER' comment."
exit 1
fi
fi
120 changes: 120 additions & 0 deletions .github/scripts/diff_to_suggestions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
#!/usr/bin/env python3
"""Convert a git-clang-format unified diff (read from stdin) into GitHub PR review
suggestions.

Writes two files consumed by the pr-comments.yml `format` job:
/tmp/comments.json - the (capped) list of review comments
/tmp/review.json - the full review payload for POST .../pulls/{n}/reviews

Environment:
HEAD_SHA required - commit the review is posted against.
MAX_COMMENTS optional - cap on comments per review (default 25). Large
reviews trigger GitHub rate limiting and fail as a misleading 404 error.

A "Commit suggestion" button is just a review comment whose body is a
```suggestion block. In the diff the 'old' side (-) is the text currently in the
PR head (what we comment on); the 'new' side (+) is the replacement text.

This lives in a checked-out file (not a YAML heredoc) so it can be unit-tested
and reviewed. The 'format' job runs it from the trusted base-branch checkout.
"""
import json
import os
import re
import sys

HUNK = re.compile(r"^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@")


def parse(diff_text):
comments, path, old_ln = [], None, 0
removed, added, start = [], [], None

def flush():
nonlocal removed, added, start
if start is None or (not removed and not added):
removed, added, start = [], [], None
return
if not removed:
# Pure-insertion hunk (only + lines): a ```suggestion anchored here
# replaces the anchor line and silently deletes its original content
# on a one-click apply. clang-format's real edits always touch an
# existing line (a line split shows a removed line too), so drop
# these rather than risk corrupting code.
removed, added, start = [], [], None
return
body = "```suggestion\n" + "".join(line + "\n" for line in added) + "```"
c = {
"path": path,
"side": "RIGHT",
"body": body,
"line": start + len(removed) - 1,
}
if len(removed) > 1:
c["start_line"] = start
c["start_side"] = "RIGHT"
comments.append(c)
removed, added, start = [], [], None

for raw in diff_text.splitlines():
if raw.startswith("diff --git"):
flush(); path = None; continue
if raw.startswith("--- "):
continue
if raw.startswith("+++ "):
p = raw[4:].strip()
path = p[2:] if p.startswith("b/") else p
continue
m = HUNK.match(raw)
if m:
flush(); old_ln = int(m.group(1)); continue
if path is None:
continue
if raw.startswith("-"):
if start is None:
start = old_ln
removed.append(raw[1:]); old_ln += 1
elif raw.startswith("+"):
if start is None:
start = old_ln
added.append(raw[1:])
elif raw.startswith("\\"):
continue
else:
flush(); old_ln += 1
flush()
return comments


def main():
comments = parse(sys.stdin.read())
total = len(comments)
cap = int(os.environ.get("MAX_COMMENTS", "25"))
if total > cap:
comments = comments[:cap]
with open("/tmp/comments.json", "w") as fh:
json.dump(comments, fh)

body = ("`clang-format` suggests the formatting changes below. "
"Use **Commit suggestion** to apply them.")
if total > cap:
body += (f"\n\n> **Note:** showing the first {cap} of {total} suggestions. "
"Apply these and push, and the rest post on the next run — "
"or fix them all at once locally:\n"
"> ```\n"
"> pip install clang-format==18.1.8\n"
"> git-clang-format --style=file --extensions c,h,cpp <merge-base>\n"
"> ```")
review = {
"commit_id": os.environ["HEAD_SHA"],
"event": "COMMENT",
"body": body,
"comments": comments,
}
with open("/tmp/review.json", "w") as fh:
json.dump(review, fh)
print(f"{total} suggestion(s) parsed; prepared {len(comments)} to post")


if __name__ == "__main__":
main()
Loading
Loading