Skip to content

Conversation

mandarini
Copy link
Contributor

@mandarini mandarini commented Aug 28, 2025

What kind of change does this PR introduce?

Proactive security hardening - implementing defense-in-depth for our preview release workflow.

What is the current behavior?

The current preview-release.yml workflow is secure in practice but uses a pattern that could be theoretically vulnerable if our existing safeguards were bypassed.

Current workflow security analysis:

  • Protected by maintainer-only label requirement (trigger: preview)
  • No code injection vulnerabilities (no direct interpolation of user input)
  • Limited permission scope (only pull-requests: write)
  • ⚠️ Theoretical risk: Uses pull_request_target while checking out PR head code
  • ⚠️ Pattern concern: Executes npm ci and npm run build from forks in a context with secrets

Important: Our workflow was never vulnerable to the attacks seen in the recent incident due to our security controls. However, in light of recent supply chain attacks, we're implementing additional layers of security.

What is the new behavior?

Implementing a zero-trust architecture that makes exploitation impossible even if all other safeguards fail.

New Three-Workflow Architecture:

  1. preview-build.yml - Executes untrusted fork code in a completely isolated environment (no secrets, minimal permissions)
  2. trigger-tests.yml - Orchestrates testing using only artifacts (never touches fork code, has access to secrets)
  3. preview-comment.yml - Updates PR status (read-only operations with artifacts)

Security Improvements:

Security Layer Previous (Secure) New (Defense-in-Depth)
Maintainer Control ✅ Required label ✅ Required label
Code Injection Protection ✅ No interpolation ✅ No interpolation
Fork Code Isolation ⚠️ Runs with secrets present ✅ Complete isolation from secrets
Workflow Tampering ⚠️ Theoretical if branch protection bypassed ✅ Impossible by design
Audit Trail ✅ GitHub logs ✅ Enhanced with explicit PR author logging
Community Testing ⚠️ Requires trust ✅ Safe by architecture

Key Architectural Benefits:

  • Separation of Concerns: Each workflow has a single, well-defined responsibility
  • Artifact-Based Communication: Data passes through GitHub's artifact system, not workflow contexts
  • Fail-Safe Design: Even if GitHub Actions introduced new vulnerabilities, our architecture would remain secure
  • Industry Best Practice: Aligns with GitHub Security Lab recommendations

Additional context

Why make this change now?

Following the recent Nx supply chain attack and similar incidents in the ecosystem, we're proactively hardening our security posture. While our existing workflow was not vulnerable to the specific attack vectors used against Nx (thanks to our label requirements and lack of code injection points), we recognize that:

  1. Security is not binary - Defense-in-depth provides resilience against unknown future attacks
  2. Patterns matter - Even secure implementations of risky patterns can normalize their use
  3. Community confidence - Demonstrating security leadership builds trust in Supabase's infrastructure

Risk Assessment:

  • Previous risk level: Low (protected by multiple controls)
  • New risk level: Negligible (architecturally impossible to exploit)
  • Implementation risk: Minimal (preserves all existing functionality)

Implementation Highlights:

  • ✅ Maintains full backwards compatibility
  • ✅ No changes required to existing workflows or processes
  • ✅ Actually improves functionality (safer community contributions)
  • ✅ Sets a security standard for other Supabase repositories
  • ✅ Provides a template for secure CI/CD in the broader ecosystem

Technical Details:

This change replaces a single 167-line workflow with three focused workflows (307 lines total) that:

  • Eliminate the use of pull_request_target with fork code execution
  • Ensure secrets are never available in the same context as untrusted code
  • Use GitHub's artifact system for secure inter-workflow communication
  • Add comprehensive logging for security auditing

@coveralls
Copy link

coveralls commented Aug 28, 2025

Pull Request Test Coverage Report for Build 17293540350

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 92.391%

Totals Coverage Status
Change from base Build 17210149121: 0.0%
Covered Lines: 223
Relevant Lines: 236

💛 - Coveralls

@mandarini mandarini marked this pull request as ready for review August 28, 2025 10:52
@mandarini mandarini self-assigned this Aug 28, 2025
@mandarini mandarini enabled auto-merge (squash) August 28, 2025 11:02
@mandarini mandarini merged commit 0b11909 into main Aug 28, 2025
5 checks passed
@mandarini mandarini deleted the chore/secure-proof-workflows branch August 28, 2025 16:31
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.

3 participants