[DRAFT] etherform integration

[rubydusa]

duplicate of #72,
testing new solution that includes upgrade safety validation among other features

[tbsoc]

🤖 AI Code Review

Code Review: Foundry CI/CD Integration

1. Summary

This PR introduces a GitHub Actions workflow that delegates CI/CD to a reusable workflow from the etherform repository. It enables Foundry testing, coverage reporting, and upgrade safety validation for the project's smart contracts. The workflow triggers on PRs to main, pushes to main, and manual dispatch.

2. Code Quality

Strengths:

• Clean separation of concerns via reusable workflow
• Explicit boolean flags make configuration readable
• workflow_dispatch enables manual testing/debugging

Concerns:

• Branch reference for reusable workflow: @Rubydusa/upgrade-safety-validation pins to a feature branch, not a stable release or commit SHA. This introduces unpredictability—the workflow behavior can change without this repo being modified.
• No version pinning: Without a tag or SHA, you're trusting that the upstream workflow won't introduce breaking changes.

3. Security

High Priority:

• deploy-on-pr: true: This allows any pull request (including from forks, depending on repo settings) to trigger a deployment. Combined with PRIVATE_KEY, this is a significant attack surface.
• PRIVATE_KEY exposure: Storing a private key in GitHub secrets for CI is risky. If the reusable workflow or any step logs incorrectly, or if there's a supply chain attack on the upstream workflow, this key could be exfiltrated. Ensure this is a dedicated burner key with no production funds.
• pull-requests: write permission: Verify what the reusable workflow does with this. If it posts comments or updates PRs, ensure the upstream workflow doesn't misuse this permission.

Medium Priority:

• RPC_URL in secrets: If this is a private/infra endpoint, be aware it will be accessible to the workflow. Consider if this should be behind a VPN or proxy.

4. Suggestions

Priority	Suggestion
Critical	Pin the reusable workflow to a specific commit SHA or tag (e.g., @v1.2.3 or @abc123def) instead of a branch
High	Document the PRIVATE_KEY usage: what network, what funds are at risk, rotation policy
High	Add a CODEOWNERS requirement for workflow changes to prevent malicious PRs from modifying CI
Medium	Consider deploy-on-pr: false or add an environment protection rule requiring manual approval for PR deployments
Medium	Enable run-slither: true for static analysis alongside upgrade safety
Low	Add a comment explaining why workflow_dispatch is needed (emergency re-runs?)

Quick Fix Example

# Instead of:
uses: BreadchainCoop/etherform/.github/workflows/_foundry-cicd.yml@Rubydusa/upgrade-safety-validation

# Use:
uses: BreadchainCoop/etherform/.github/workflows/_foundry-cicd.yml@e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 # v1.0.0

Overall: The structure is sound, but the security posture needs tightening before merging—especially around the branch reference and deploy-on-pr behavior.

[github-actions]

Coverage Report

Metric	Coverage
Lines	100.00% (0/0)
Statements	100.00% (0/0)
Branches	100.00% (0/0)
Functions	100.00% (0/0)

Coverage by file

File	Lines	Statements	Branches	Functions