rusackas commented on PR #40719: URL: https://github.com/apache/superset/pull/40719#issuecomment-4617263544
Great catch, and you're right — this is a real fork-PR exfiltration vector, not a hypothetical. Switching `pull_request` → `workflow_run` drops the implicit fork secrets block: the gated run executes in the base-repo context with full secrets **and** a read-write `GITHUB_TOKEN`, and since we check out and run the fork's code (`cypress-run-all` at L166), a fork PR could lift `CYPRESS_RECORD_KEY` *and* the token once pre-commit passes. Broader than just the record key. Your `head_repository.full_name == github.repository` guard is the correct fix, but it skips forks entirely — which means fork PRs (most of our external contributions) would lose E2E coverage they have today. That tradeoff, plus the other `workflow_run` downsides (default-branch-only workflow file, branch-protection rework, forks losing change-detection), tips the cost/benefit against this approach. Since #40717 (shard cut) and #40718 (job-level change-gating) already capture the bulk of the E2E savings with no security change, I'm going to **close this one** rather than bolt on guards that negate its value. Thanks for the careful review — this was the right call to flag. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
