Skip to content

Review 5594

Cindy Zhang edited this page Aug 27, 2026 · 1 revision

#5594 — fix(ci): authorize visual acceptance by capability

cixzhang · merged · view on GitHub

Verdict: CLEAR — semantic approve (fresh full review; no review posted by this run)

PR

#5594 fix(ci): authorize visual acceptance by capability by cixzhang (bucket: the maintainer)

HEAD REVIEWED

e743f44f10eadb5b112fba835aac851df239bd9e — every claim below is about this exact commit. It merged as 1d94f85fd07420c6b55fd001234081f7444255f8.

VERSIONS

LOOP VERSION: 1.5.0
AUDIT RUBRIC: 1.13

LANE

LANE: full
WHY: this changes a privileged workflow authorization boundary and immutable acceptance-record validation.

PROBLEM

#5592 normalized GitHub's collaborator-permission response but also allowed the reported role name to grant authority. Role names are provenance, not a stable capability boundary. Endpoint authorization must use strict effective capability data, while legacy record compatibility is a separate trust domain.

VERDICT: clear

SOLUTION

Endpoint normalization accepts only nested admin === true or maintain === true. It rejects explicit false, strings, null, arrays, missing capability fields, malformed capability objects, raw top-level permission fallback, role-only identity, ordinary write, unknown users, and 404s.

Record validation is separate: a new record must carry valid effectivePermission; only an old record with no such field may fall back to raw maintain or admin. Reported permission and role remain archived as provenance only.

VERDICT: clear

ARCHITECTURE

OWNER: .github/scripts/visual-gate/authorization.mjs
SEAMS: GitHub endpoint response → strict endpoint predicate → trusted outputs → archive CLI → separate record predicate.

The authorize checkout uses trusted default-branch code with no explicit ref. author_association is absent. #5593's dependency setup and the dependency-free accept path remain intact.

VERDICT: clear

IMPACT

Maintainers with effective maintain/admin capability retain acceptance access. Role-only identities, ordinary writers, malformed endpoint data, and raw endpoint permission fallback fail closed. Valid legacy maintain/admin records remain readable; legacy write records fail.

VERDICT: clear

API

No public package API change. Internal authorization exports split one mixed predicate into explicit endpoint and record policies.

VERDICT: clear

THEMING

No styles, tokens, theme variables, targets, or overrides changed.

VERDICT: clear

BREAKING

  • API: no public API change.
  • Visual: no rendered output changes.
  • Theme: no target or override changes.
  • Behavior: intentionally removes role-name authorization without changing the effective maintain/admin capability set.

VERDICT: clear

PERFORMANCE & RESOURCES

Zero Effects, listeners, observers, layout reads, dependencies, or bundle-facing changes. Authorization remains constant-time property checks and set membership.

VERDICT: clear

VISUAL EVIDENCE

VISUAL CHECK: not applicable
WHY: only CI workflow and Node authorization files changed. Exact-head visual status reported “No stable visual scope.”

VERDICT: clear

A11Y & I18N

No rendered elements, ARIA, strings, locale data, or direction behavior changed. Component a11y and RTL jobs correctly skipped for no component scope.

VERDICT: clear

JUDGEMENT

slot verdict
PROBLEM clear
SOLUTION clear
ARCHITECTURE clear
IMPACT clear
API clear
THEMING clear
BREAKING clear
PERFORMANCE clear
VISUAL clear
A11Y & I18N clear

GOAL: met — the adversarial endpoint matrix, separate legacy-record policy, exact-head CI, and recovered 124-frame artifact all passed.
DISPOSITION: no negative findings.
ADVICE: omitted — no defect found.
AUTHOR CAN PROCEED: yes — all requested claims were verified.
WORST OUTCOME: none found.
JUDGEMENT NEEDED: none — defect fix against the existing security contract.

CLEAR — semantic approve.

REVIEW

Thanks, this is clean. The endpoint and record trust boundaries both hold, including the recovered 124-frame bundle.

INLINE

None.

EVIDENCE

  • Exact-head CI run 33054031543: success, including the full test job.
  • Focused suite: 3 files, 52 tests passed.
  • Full visual-gate suite: 14 files, 149 tests passed.
  • actionlint and check:repo passed.
  • Recovered pre-cleanup gh-pages commit be6f3351c823f6b62c59eb29507cb4518024eacc: #5162 exact head 66ef14d69ed5f32c469c78cb2e8395bfe23ec633, run 33041576630/2, 124/124 deltas archived and success:accepted.
  • The same recovered artifact refused both role-only authority and raw top-level admin without effective capability.
  • No live acceptance command was replayed and nothing was pushed to gh-pages by the review.

REVIEW LOOP

Full lane: draft → blind critic → rewrite → critic. Two critic passes. Final verdict CLEAR.

Clone this wiki locally