Skip to content

[codex] fix security alerts#56

Merged
barnabasbusa merged 1 commit into
masterfrom
codex/fix-security-alerts
May 21, 2026
Merged

[codex] fix security alerts#56
barnabasbusa merged 1 commit into
masterfrom
codex/fix-security-alerts

Conversation

@parithosh

Copy link
Copy Markdown
Member

Summary

Mirrors the workflow hardening landed in ethpandaops/assertoor#183.

  • Drops caller-controlled ref inputs from reusable check/build workflows so reusable jobs check out the event SHA instead of arbitrary PR-provided refs.
  • Moves the PR build workflow (build-dev.yml) from pull_request_target to pull_request and adds explicit read-only default permissions.
  • Splits PR binary builds from Docker publishing so fork PRs do not receive DockerHub secrets, while same-repository / manual trusted Docker publishing remains available.
  • Adds explicit workflow permissions and switches internal artifact handoffs to artifact IDs to avoid artifact-name poisoning.

Validation

  • actionlint clean

🤖 Generated with Claude Code

Mirrors the workflow hardening from ethpandaops/assertoor#183:
- Drop caller-controlled ref inputs from reusable check/build workflows
- Move PR build trigger from pull_request_target to pull_request
- Add explicit read-only default permissions to workflows
- Split fork-PR binary builds from Docker publishing so secrets never reach untrusted code
- Switch internal artifact handoffs to artifact IDs to avoid name poisoning

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@parithosh parithosh requested a review from pk910 as a code owner May 20, 2026 16:16
@qu0b-reviewer

qu0b-reviewer Bot commented May 20, 2026

Copy link
Copy Markdown

🤖 qu0b-reviewer

Summary

The PR addresses GitHub Actions security alerts by: (1) removing the pull_request_target trigger (which ran with write permissions on the base branch), replacing it with pull_request which only needs read permissions; (2) adding explicit permissions: contents: read declarations (principle of least privilege); (3) removing the ref input to reusable workflows (which was being used to parameterize actions/checkout ref — eliminating an unnecessary indirection point); (4) switching from hardcoded artifact names to artifact IDs for inter-job artifact passing. The changes are well-scoped and the security reasoning is sound.

Issues

No blockers. The changes are procedurally correct.

Suggestions

  • .github/workflows/build-dev.yml:52build_binaries (the no-docker job) passes no release: input to the reusable workflow. Versus the docker job at line 63, which does pass it (empty string is the default, so it's functionally identical — but it's inconsistent and could break silently if the shared workflow's default ever changed). Underscore as a concern, not a blocker.

NO_REVIEW_NEEDED


Reviewed @ f9633f09
"Hofstadter's Law: It always takes longer than you expect, even when you take into account Hofstadter's Law." — Douglas Hofstadter

@barnabasbusa barnabasbusa merged commit 42f9d58 into master May 21, 2026
1 check passed
@barnabasbusa barnabasbusa deleted the codex/fix-security-alerts branch May 21, 2026 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants