fix: correct visual tests workflow

[pravusjif]

Visual test workflow runs fail due to a wrong configuration: https://github.com/decentraland/unity-explorer/actions/runs/26288958503

This PR corrects that, without this change, the explorer-autmation workflow can't post the github PR comments in this repo, due to Github rules this change cannot work in a PR unless it's already been merged.

[pravusjif]

/visual-tests

[pravusjif]

/visual-tests

[github-actions]

New build in progress, come back later!

[claude]

Claude finished @pravusjif's task in 2m 5s —— View job

---

Security Review — Workflow Change

Scope of change: A single permission elevation in .github/workflows/visual-regression.yml:

 permissions:
-  contents: read
+  contents: write
   pull-requests: write

• Read PR diff and workflow file
• Run workflow security review (W.1–W.10)
• Post findings & verdict

---

Findings

MEDIUM — W.10: Permissions broader than the dispatcher itself exercises

This dispatcher workflow performs only read-side operations on its own:

• gh api .../pulls/${PR_NUMBER} (read)
• gh api .../actions/runs?... (read)
• gh api .../branches/${HEAD_REF} against explorer-automation (read)
• One write op already covered by pull-requests: write: posting an eyes reaction on the trigger comment (/issues/comments/{id}/reactions).

Nothing in the dispatcher itself needs contents: write. The elevation only makes sense as a grant to the reusable workflow called via secrets: inherit at .github/workflows/visual-regression.yml:162:

uses: decentraland/explorer-automation/.github/workflows/run-visual-suite.yml@main

That's a legitimate pattern (e.g. the suite may commit baseline updates), but the PR doesn't say so. Recommend adding a one-line comment in the permissions: block explaining why contents: write is needed (e.g. "required by run-visual-suite to push baseline updates / annotated commits"). Future readers will otherwise treat it as overscoped and not know whether it's safe to drop.

If run-visual-suite.yml does NOT actually need contents: write, this should be reverted — the dispatcher itself doesn't need it.

HIGH (pre-existing, not introduced here, but amplified by this PR) — W.7a: Secret-holding reusable workflow pinned to a mutable ref

.github/workflows/visual-regression.yml:162 uses @main:

uses: decentraland/explorer-automation/.github/workflows/run-visual-suite.yml@main

combined with secrets: inherit (line 170). The inline comment justifies @main as "the merged version", but @main is still a mutable ref — if decentraland/explorer-automation's default branch is compromised, every visual-test run inherits every secret listed in lines 27–32 (Alttester license, REPOS_READ_ONLY_TOKEN, AWS access keys, S3 bucket secret).

This PR makes that exposure strictly larger: with contents: write, a compromised reusable workflow could also push commits to unity-explorer itself, not just consume secrets.

Recommend (separately from this PR) pinning to a 40-char commit SHA and using Dependabot or a manual bump cadence for updates. This is the same standard you'd apply to any third-party action; an internal-org workflow is lower risk but the same class of risk.

MEDIUM (pre-existing) — Author-association gate without TOCTOU protection

.github/workflows/visual-regression.yml:63-77 gates by OWNER|MEMBER|COLLABORATOR. Two notes:

1. W.6-adjacent: author_association == 'MEMBER' only resolves for users whose org membership is public. Members with private membership get CONTRIBUTOR / NONE. This may already match the team's intent (only public-membership trusted users), but worth confirming.
2. W.3-adjacent: The workflow resolves head.sha at run time via gh api .../pulls/${PR_NUMBER} (line 99) with no check that the head wasn't pushed after the triggering comment. A trusted commenter posting /visual-tests then having a co-author force-push between trigger and execution would have the new code run against the resolved build. Lower risk here because the gate is to write-access users and the workflow runs visual tests rather than executing arbitrary PR code, but pattern-wise worth a pr.head.repo.pushed_at vs comment.created_at check.

Neither of these is introduced by this PR.

LOW — observations

• W.1, W.4, W.5, W.8: N/A — no LLM prompt, no pull_request_target, no tool allowlist.
• W.2: $GITHUB_OUTPUT writes use simple name=value form (lines 71, 75, 128–134, 148, 151), not heredocs from external content. Safe.
• W.9: No actions/upload-artifact in this workflow. N/A.

---

Recommendations for this PR

1. Add a comment beside contents: write explaining what the reusable workflow uses it for. If it isn't actually used, revert to contents: read. Fix this →

Recommendations for follow-up (out of scope for this PR)

2. Pin decentraland/explorer-automation/.github/workflows/run-visual-suite.yml to a commit SHA.
3. Consider a TOCTOU guard comparing pr.head.repo.pushed_at to comment.created_at.

---

DEPENDENCY_REVIEW: NEEDS_ATTENTION

The permission elevation is plausibly correct as a fix, but it's undocumented and it amplifies an existing mutable-ref + secrets: inherit exposure. Worth a brief human confirmation that the reusable workflow legitimately needs contents: write before merge.
• Branch: test/trigger-visual-tests