Skip to content

Conversation

@leodido
Copy link
Contributor

@leodido leodido commented Nov 18, 2025

Problem

Multiple workflows building the same artifact concurrently can overwrite each other's SLSA attestations, causing verification failures. This happens when:

  1. Workflow A builds artifact (checksum A), uploads to S3, signs it, uploads attestation A
  2. Workflow B builds artifact (checksum B), finds artifact exists (skips upload), signs LOCAL artifact B, uploads attestation B
  3. Result: S3 has artifact A + attestation B → verification fails

Part of https://linear.app/ona-team/issue/CLC-2096/prevent-attestation-overwrites-in-concurrent-builds

Solution

Check both artifact and attestation existence before uploading:

  1. Both exist: Skip both uploads (prevents overwrites from concurrent builds)
  2. Only artifact exists: Upload attestation only (handles old builds before SLSA)
  3. Neither exists: Upload both (normal flow)
  4. HasFile check fails: Fallback to upload (safe default)

This ensures:

  • Concurrent builds don't overwrite each other's attestations
  • Old artifacts without attestations get attestations added
  • Safe fallback behavior when cache checks fail

Changes

Code Changes

  • Modified UploadArtifactWithAttestation to check attestation existence when artifact exists
  • Added logic to upload attestation only when artifact exists but attestation is missing
  • Updated logging to clearly indicate which scenario is occurring
  • Maintained safe fallback behavior when HasFile checks fail

Test Changes

  • Updated TestArtifactUploader_SkipsExistingArtifacts to verify both artifact and attestation exist
  • Added TestArtifactUploader_UploadsAttestationWhenMissing to test missing attestation scenario
  • Updated TestArtifactUploader_SimulatesDownloadedArtifactWorkflow to include attestation in setup
  • Updated TestArtifactUploader_PreventsAttestationOverwrite to verify race condition fix
  • Added TestArtifactUploader_AttestationCheckError to test fallback when attestation check fails

Testing

All tests pass:

  • 19 ArtifactUploader tests pass (added 2 new tests)
  • Complete signing package test suite passes (6.1s)
  • New tests verify all three scenarios work correctly

Prevents SLSA verification failures caused by attestation overwrites
in concurrent builds. When an artifact already exists in remote cache,
skip both artifact and attestation upload to preserve the existing
artifact's attestation.

Co-authored-by: Ona <no-reply@ona.com>
@leodido leodido force-pushed the fix/slsa-attestation-skip-upload branch from 44b762f to 5fe1c50 Compare November 18, 2025 14:00
Copy link
Member

@geropl geropl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

When artifact exists, check if attestation also exists:
- If both exist: skip both uploads (prevents overwrites)
- If only artifact exists: upload attestation only (handles old builds)
- If HasFile check fails: fallback to upload (safe default)

This handles the case where artifacts exist from old builds before
SLSA attestation was implemented, ensuring attestations are uploaded
for those artifacts.

Co-authored-by: Ona <no-reply@ona.com>
@leodido leodido merged commit 0c95ac4 into main Nov 18, 2025
6 checks passed
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