Skip to content

chore: Implement automated screenshot diff reporting #182

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.

Sign up for GitHub

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

Merged
komodgn merged 4 commits into from
Apr 20, 2026
Merged

chore: Implement automated screenshot diff reporting #182

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
525f6d1
chore: Implement automated screenshot diff reporting
komodgn Apr 20, 2026
2d58322
fix: Add push trigger to develop for screenshot comparison baseline
komodgn Apr 20, 2026
5e03621
test: Increase mock photo count to validate Roborazzi diff pipeline
komodgn Apr 20, 2026
3b7ef55
test: Reset mock data to match develop baseline before merge
komodgn Apr 20, 2026
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
100 changes: 100 additions & 0 deletions .github/workflows/screenshot-comparison-comment.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
name: screenshot-comparison-comment

on:
workflow_run:
workflows: ["screenshot-comparison"]
types: [completed]

jobs:
comment:
if: github.event.workflow_run.conclusion == 'success'
runs-on: ubuntu-latest
permissions:
contents: write
pull-requests: write
actions: read

steps:
- name: Download Diff Artifact
uses: dawidd6/action-download-artifact@v3
with:
name: screenshot-diff-reports
run_id: ${{ github.event.workflow_run.id }}
path: diff-reports

- id: get-pull-request-number
run: echo "pull_request_number=$(cat diff-reports/pr/NR)" >> "$GITHUB_OUTPUT"
Comment on lines +25 to +26
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 20, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Validate pull_request_number read from the untrusted artifact.

diff-reports/pr/NR originates from the PR-triggered screenshot-comparison workflow's artifact, which is attacker-controllable content in the workflow_run threat model. Its value is then passed to peter-evans/find-comment and peter-evans/create-or-update-comment as issue-number (Lines 75, 84). A non-numeric or multi-line payload could cause the bot to comment on an arbitrary issue/PR in the repo, or fail in non-obvious ways.

Sanitize before use, e.g.:

NR=$(tr -cd '0-9' < diff-reports/pr/NR | head -c 10)
[ -n "$NR" ] || { echo "Invalid PR number"; exit 1; }
echo "pull_request_number=$NR" >> "$GITHUB_OUTPUT"
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/screenshot-comparison-comment.yml around lines 25 - 26,
The step id get-pull-request-number reads attacker-controllable
diff-reports/pr/NR and uses it as pull_request_number for
peter-evans/find-comment and create-or-update-comment; sanitize and validate
that the artifact contains only a single numeric PR id (strip non-digits,
truncate to a reasonable length, ensure non-empty and single-line) and fail the
job if validation fails so an invalid or multi-line payload cannot be passed to
the actions as issue-number.


- name: Checkout Default Branch
uses: actions/checkout@v4
with:
ref: ${{ github.event.repository.default_branch }}

- name: Push to Companion Branch
env:
BRANCH_NAME: companion_${{ github.event.workflow_run.head_branch }}
run: |
git checkout --orphan "$BRANCH_NAME"
git rm -rf .
find diff-reports -type f -name "*.png" -exec cp {} . \;

if [ -n "$(git status --porcelain)" ]; then
git config --global user.name "ScreenshotBot"
git config --global user.email "bot@github.com"
git add .
git commit -m "Add screenshot diffs"
git push origin "$BRANCH_NAME" -f
echo "exists=true" >> "$GITHUB_ENV"
fi
Comment on lines +28 to +48
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 20, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Companion-branch push step has two correctness gaps.

  1. Filename collisions on flatten. find diff-reports -type f -name "*.png" -exec cp {} . \; flattens all PNGs from every module into the branch root. Different feature modules frequently share snapshot names (e.g., HomeScreen_compare.png), so later cp calls silently overwrite earlier ones and the PR comment will show incomplete diffs. Prefer cp --parents (preserving module paths) or a module-prefixed rename.
  2. exists=true only set on subsequent runs. The flag is only written inside if [ -n "$(git status --porcelain)" ]. On the first push to a brand-new companion branch, git status --porcelain will correctly be non-empty, so that case is fine — but if someone re-runs with identical diffs the subsequent steps are skipped and the existing stale comment is never refreshed/cleaned up. Consider also handling the "no changes, branch already exists" path (e.g., still regenerate the comment from the existing branch contents, or explicitly clear the bot comment when there are no diffs).
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/screenshot-comparison-comment.yml around lines 28 - 48,
The current Push to Companion Branch step flattens PNGs (find ... -exec cp {} .
\;) causing filename collisions and only writes exists=true inside the git
status check so re-runs with no new commit won't refresh/clear stale comments;
update the step that sets BRANCH_NAME and runs git checkout --orphan to copy
files using preserved paths (use cp --parents or module-prefixed renaming when
running find diff-reports ...) so files aren’t overwritten, and change the
existence logic so exists=true is set whenever the companion branch contains any
diffs (e.g., after copying, check for any *.png in the working tree or check if
the branch already existed and contains files) and write echo "exists=true" >>
"$GITHUB_ENV" in both the "new commit created" and "no new commit but branch
contains files" paths so downstream steps always know when a companion branch
with diffs exists.


- name: Generate Report Body
if: env.exists == 'true'
id: generate_report
run: |
BRANCH="companion_${{ github.event.workflow_run.head_branch }}"
REPO="${{ github.repository }}"

echo "REPORT_BODY<<EOF" >> $GITHUB_OUTPUT
echo "### Roborazzi Snapshot Diff Report" >> $GITHUB_OUTPUT
echo "" >> $GITHUB_OUTPUT
echo "| File Name | Comparison |" >> $GITHUB_OUTPUT
echo "|---|---|" >> $GITHUB_OUTPUT

for file in $(ls *.png | grep _compare.png); do
URL="https://github.com/${REPO}/blob/${BRANCH}/${file}?raw=true"
DISPLAY_NAME=$(echo $file | sed -r 's/(.{20})/\1<br>/g')
echo "| $DISPLAY_NAME | ![]($URL) |" >> $GITHUB_OUTPUT
done
echo "EOF" >> $GITHUB_OUTPUT
Comment on lines +50 to +68
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 20, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

GitHub Actions workflow_run script injection head_branch security

💡 Result:

GitHub Actions workflows triggered by the workflow_run event carry significant security risks, particularly for script injection vulnerabilities involving potentially untrusted data like head_branch. Official GitHub documentation warns that running untrusted code on workflow_run can lead to cache poisoning, unintended write access, and secret exposure. The workflow_run event provides access to the triggering workflow's details via github.event.workflow_run, including fields like head_branch, head_sha, event (e.g., 'pull_request'), ref, and head_repository. If the triggering workflow was executed from a pull request (especially from a fork via pull_request trigger), head_branch reflects the PR's source branch, which attackers can control to inject malicious payloads (e.g., branch names like zzz";echo${IFS}"hello";# or fooidbar). Script injection occurs when expressions like ${{ github.event.workflow_run.head_branch }} are interpolated directly into run: steps without sanitization. GitHub evaluates ${{ }} expressions before shell execution, substituting untrusted values into scripts, enabling command injection. This is exacerbated in privileged workflow_run workflows (with secrets/write permissions) triggered by unprivileged ones compromised via PRs. Examples from GitHub Security Lab and advisories (e.g., GHSL-2024-326) show branch name injection in workflows, analogous to head_branch. Tools like OSS-F Scorecard flag workflow_run usage due to chaining risks allowing PRs to trigger privileged execution. Mitigations from GitHub docs: - Treat github.event.* (especially ending in ref, head_ref, head_branch) as untrusted. - Use env: for intermediate variables: env: BRANCH: ${{ github.event.workflow_run.head_branch }} then run: echo "$BRANCH". - Prefer JavaScript actions for processing. - Add guards: if: github.event_name != 'pull_request' && github.repository == github.event.workflow_run.head_repository.full_name. - Pin branches: to trusted ones, limit permissions: { contents: read }. - Use branch filters and verify event origin. No specific CVE for workflow_run.head_branch, but pattern matches documented script injection risks in privileged triggers.

Citations:

🏁 Script executed:

# Locate the workflow file
git ls-files | grep -E '\.github/workflows.*screenshot.*\.yml'

Repository: komodgn/meta-android

Length of output: 159

🏁 Script executed:

cat -n .github/workflows/screenshot-comparison-comment.yml | head -80

Repository: komodgn/meta-android

Length of output: 3374

Script injection vulnerability: workflow_run.head_branch interpolated directly into shell at line 54.

BRANCH="companion_${{ github.event.workflow_run.head_branch }}" expands untrusted branch names directly into the bash script. Since this workflow runs with contents: write and pull-requests: write, a branch name crafted with shell metacharacters (valid git refs allow many characters like ;, $(), etc.) executes arbitrary commands with elevated permissions. The same step block already demonstrates the correct mitigation at lines 34–37, using an env: variable to safely pass the value. Apply the same pattern here:

Fix 
      - name: Generate Report Body
        if: env.exists == 'true'
        id: generate_report
+       env:
+         HEAD_BRANCH: ${{ github.event.workflow_run.head_branch }}
+         REPO: ${{ github.repository }}
        run: |
-         BRANCH="companion_${{ github.event.workflow_run.head_branch }}"
-         REPO="${{ github.repository }}"
+         BRANCH="companion_${HEAD_BRANCH}"
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/screenshot-comparison-comment.yml around lines 50 - 68,
The generate_report step currently injects untrusted
github.event.workflow_run.head_branch directly into the shell via
BRANCH="companion_${{ github.event.workflow_run.head_branch }}", which enables
command injection; move the branch construction into the step's env mapping
(e.g., set an env variable like BRANCH: "companion_${{
github.event.workflow_run.head_branch }}") and remove direct interpolation from
the run block, then reference $BRANCH inside the run script; update the step
with id generate_report to use env: BRANCH and use $BRANCH in the loop so the
branch value is passed safely to the shell.


- name: Find Old Comment
if: env.exists == 'true'
uses: peter-evans/find-comment@v3
id: fc
with:
issue-number: ${{ steps.get-pull-request-number.outputs.pull_request_number }}
comment-author: 'github-actions[bot]'
body-includes: 'Roborazzi Snapshot Diff Report'

- name: Add or Update Comment
if: env.exists == 'true'
uses: peter-evans/create-or-update-comment@v4
with:
comment-id: ${{ steps.fc.outputs.comment-id }}
issue-number: ${{ steps.get-pull-request-number.outputs.pull_request_number }}
body: ${{ steps.generate_report.outputs.REPORT_BODY }}
edit-mode: replace

- name: Cleanup outdated companion branches
shell: bash
run: |
git branch -r --format="%(refname:lstrip=3)" | grep companion_ | while read -r branch; do
last_commit_date_timestamp=$(git log -1 --format=%ct "origin/$branch")
now_timestamp=$(date +%s)

# 30 days
if [ $((now_timestamp - last_commit_date_timestamp)) -gt 2592000 ]; then
echo "Deleting outdated branch: $branch"
git push origin --delete "$branch"
fi
done
Comment on lines +88 to +100
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 20, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Cleanup step won't see remote branches due to shallow checkout.

actions/checkout@v4 defaults to fetch-depth: 1 and fetches only the checked-out ref, so git branch -r in this step typically lists just origin/<default_branch> — no companion_* refs are present, and git log -1 origin/$branch would fail for any it did find. The 30-day cleanup is effectively a no-op.

Either set fetch-depth: 0 on the checkout at Line 29 (and ensure remote branches are fetched, e.g., add git fetch --prune origin '+refs/heads/companion_*:refs/remotes/origin/companion_*' before this loop), or implement the cleanup via the GitHub REST API (gh api repos/{owner}/{repo}/branches + gh api -X DELETE .../git/refs/heads/...) which doesn't depend on local refs.

Also consider adding set -euo pipefail and quoting $((now_timestamp - last_commit_date_timestamp)) guards so a single bad ref doesn't abort the loop.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/screenshot-comparison-comment.yml around lines 88 - 100,
The cleanup step fails to see remote companion_* branches because
actions/checkout@v4 uses fetch-depth: 1; update the workflow so the checkout
uses fetch-depth: 0 or explicitly fetch the companion refs before running the
loop (e.g., run a git fetch --prune origin
'+refs/heads/companion_*:refs/remotes/origin/companion_*'), or replace the loop
with GitHub REST/gh API calls to list and delete branches; also enable safer
shell settings (set -euo pipefail) at the top of the step and ensure the
arithmetic and variables are quoted (e.g., quote $((now_timestamp -
last_commit_date_timestamp)) and guard empty last_commit_date_timestamp) so a
missing ref won't break the loop.

66 changes: 66 additions & 0 deletions .github/workflows/screenshot-comparison.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
name: screenshot-comparison

on:
pull_request:
paths-ignore:
- '**.md'

concurrency:
group: ${{ github.workflow }}-${{ github.head_ref }}
cancel-in-progress: true

jobs:
screenshot-comparison:
runs-on: ubuntu-latest
permissions:
contents: read
actions: write
checks: read

steps:
- name: Checkout PR Branch
uses: actions/checkout@v4

- name: Get base branch HEAD commit
id: get_base_branch_head
run: |
git fetch origin ${{ github.base_ref }}
echo "sha=$(git rev-parse origin/${{ github.base_ref }})" >> "$GITHUB_OUTPUT"

- name: Setup Android
uses: ./.github/actions/setup-android
env:
LOCAL_PROPERTIES: ${{ secrets.LOCAL_PROPERTIES }}

- name: Wait for base unit-test
uses: lewagon/wait-on-check-action@v1.3.4
with:
ref: ${{ steps.get_base_branch_head.outputs.sha }}
check-name: 'unit-test'
repo-token: ${{ github.token }}
wait-interval: 10

- name: Download Base Screenshot
uses: dawidd6/action-download-artifact@v3
continue-on-error: true
with:
name: test-reports
workflow: unit-test.yml
commit: ${{ steps.get_base_branch_head.outputs.sha }}

- name: Run Comparison
run: ./gradlew compareRoborazziDebug --stacktrace
Comment on lines +43 to +52
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 20, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Handle missing base-branch artifact explicitly.

Download Base Screenshot is marked continue-on-error: true, so when the base branch has no test-reports artifact yet (e.g., first run after this PR merges, retention expired, or base unit-test was skipped via skip-ci), the next step still runs ./gradlew compareRoborazziDebug against an empty baseline. Depending on Roborazzi's behavior this will either fail the job or silently report every screenshot as changed, which is then posted to the PR as a misleading "diff report."

Consider detecting the download outcome (id: + steps.<id>.outcome == 'success') and either skipping the compare/upload steps or posting a clear "no baseline available" message instead.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/screenshot-comparison.yml around lines 43 - 52, The
workflow should detect when the download artifact step fails and skip or change
behavior of the compare/upload steps: add an id to the "Download Base
Screenshot" step (e.g., id: download_base) and change the following "Run
Comparison" (and any upload/report steps) to run only when
steps.download_base.outcome == 'success' (or post a clear "no baseline
available" message when outcome != 'success'); reference the "Download Base
Screenshot" step id and the Gradle task compareRoborazziDebug to gate or alter
downstream steps accordingly so the compare is not executed against an empty
baseline.


- name: Save PR number for comment
run: |
mkdir -p ./pr
echo ${{ github.event.number }} > ./pr/NR

- name: Upload Diff Results
if: always()
uses: actions/upload-artifact@v4
with:
name: screenshot-diff-reports
path: |
**/build/outputs/roborazzi
./pr/NR
7 changes: 7 additions & 0 deletions .github/workflows/unit-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ jobs:
id: test
run: ./gradlew testDebugUnitTest --parallel --stacktrace

- name: Save PR number
if: github.event_name == 'pull_request'
run: |
mkdir -p ./pr
echo ${{ github.event.number }} > ./pr/NR

- name: Upload Test Reports and Screenshots
if: always()
uses: actions/upload-artifact@v4
Expand All @@ -39,4 +45,5 @@ jobs:
path: |
**/build/reports/tests
**/build/outputs/roborazzi
./pr/NR
retention-days: 7
1 change: 1 addition & 0 deletions gradle.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,4 @@ android.nonTransitiveRClass=true
roborazzi.test.record=true
roborazzi.record.filePathStrategy=relativePathFromRoborazziContextOutputDirectory
robolectric.graphicsMode=NATIVE
roborazzi.test.compare=true
Loading