Skip to content

fix: return digest in attestation state save response to prevent OCC conflicts #2910

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sonarly wants to merge 1 commit into from
Closed

fix: return digest in attestation state save response to prevent OCC conflicts #2910

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion app/controlplane/api/controlplane/v1/attestation_state.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ message AttestationStateServiceSaveRequest {
string base_digest = 3;
}

message AttestationStateServiceSaveResponse {}
message AttestationStateServiceSaveResponse {
// digest of the newly saved attestation state
string digest = 1;
}

message AttestationStateServiceReadRequest {
string workflow_run_id = 1 [(buf.validate.field).string = {min_len: 1}];
Expand Down
4 changes: 2 additions & 2 deletions app/controlplane/internal/service/attestationstate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func (s *AttestationStateService) Save(ctx context.Context, req *cpAPI.Attestati
return nil, errors.Forbidden("forbidden", "failed to authenticate request")
}

err = s.attestationStateUseCase.Save(ctx, wf.ID.String(), req.WorkflowRunId, req.AttestationState, encryptionPassphrase, biz.WithAttStateBaseDigest(req.GetBaseDigest()))
digest, err := s.attestationStateUseCase.Save(ctx, wf.ID.String(), req.WorkflowRunId, req.AttestationState, encryptionPassphrase, biz.WithAttStateBaseDigest(req.GetBaseDigest()))
if err != nil {
if biz.IsErrAttestationStateConflict(err) {
return nil, cpAPI.ErrorAttestationStateErrorConflict("saving attestation: %s", err.Error())
Expand All @@ -114,7 +114,7 @@ func (s *AttestationStateService) Save(ctx context.Context, req *cpAPI.Attestati
return nil, handleUseCaseErr(err, s.log)
}

return &cpAPI.AttestationStateServiceSaveResponse{}, nil
return &cpAPI.AttestationStateServiceSaveResponse{Digest: digest}, nil
}

func (s *AttestationStateService) Read(ctx context.Context, req *cpAPI.AttestationStateServiceReadRequest) (*cpAPI.AttestationStateServiceReadResponse, error) {
Expand Down
20 changes: 11 additions & 9 deletions app/controlplane/pkg/biz/attestationstate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ type AttestationState struct {

type AttestationStateRepo interface {
Initialized(ctx context.Context, workflowRunID uuid.UUID) (bool, error)
Save(ctx context.Context, workflowRunID uuid.UUID, state []byte, baseDigest string) error
Save(ctx context.Context, workflowRunID uuid.UUID, state []byte, baseDigest string) (string, error)
Read(ctx context.Context, workflowRunID uuid.UUID) ([]byte, string, error)
Reset(ctx context.Context, workflowRunID uuid.UUID) error
}
Expand Down Expand Up @@ -79,7 +79,8 @@ func WithAttStateBaseDigest(digest string) SaveOption {
}
}

func (uc *AttestationStateUseCase) Save(ctx context.Context, workflowID, runID string, state *v1.CraftingState, passphrase string, opts ...SaveOption) error {
// Save persists the attestation state and returns the digest of the newly saved state.
func (uc *AttestationStateUseCase) Save(ctx context.Context, workflowID, runID string, state *v1.CraftingState, passphrase string, opts ...SaveOption) (string, error) {
opt := &AttestationStateSaveOpts{}

for _, o := range opts {
Expand All @@ -88,28 +89,29 @@ func (uc *AttestationStateUseCase) Save(ctx context.Context, workflowID, runID s

runUUID, err := uc.checkWorkflowRunInWorkflow(ctx, workflowID, runID)
if err != nil {
return fmt.Errorf("failed to check workflow run: %w", err)
return "", fmt.Errorf("failed to check workflow run: %w", err)
}

rawState, err := proto.Marshal(state)
if err != nil {
return fmt.Errorf("failed to marshal attestation state: %w", err)
return "", fmt.Errorf("failed to marshal attestation state: %w", err)
}

if passphrase == "" {
return NewErrValidationStr("passphrase is required")
return "", NewErrValidationStr("passphrase is required")
}

encryptedState, err := encrypt(rawState, passphrase)
if err != nil {
return fmt.Errorf("failed to encrypt attestation state: %w", err)
return "", fmt.Errorf("failed to encrypt attestation state: %w", err)
}

if err := uc.repo.Save(ctx, *runUUID, encryptedState, opt.BaseDigest); err != nil {
return fmt.Errorf("failed to save attestation state: %w", err)
digest, err := uc.repo.Save(ctx, *runUUID, encryptedState, opt.BaseDigest)
if err != nil {
return "", fmt.Errorf("failed to save attestation state: %w", err)
}

return nil
return digest, nil
}

func (uc *AttestationStateUseCase) Read(ctx context.Context, workflowID, runID, passphrase string) (*AttestationState, error) {
Expand Down
58 changes: 42 additions & 16 deletions app/controlplane/pkg/biz/attestationstate_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (s *attestationStateTestSuite) TestInitialized() {
})

s.T().Run("the run is initialized", func(t *testing.T) {
err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase)
_, _, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase)
s.NoError(err)
ok, err := s.AttestationState.Initialized(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String())
s.NoError(err)
Expand All @@ -70,19 +70,19 @@ func (s *attestationStateTestSuite) TestInitialized() {
func (s *attestationStateTestSuite) TestSave() {
ctx := context.Background()
s.Run("run in different workflow causes error", func() {
err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg2.ID.String(), s.testState, s.passphrase)
_, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg2.ID.String(), s.testState, s.passphrase)
s.Error(err)
s.True(biz.IsNotFound(err))
})

s.Run("the run doesn't exist", func() {
err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), uuid.NewString(), s.testState, s.passphrase)
_, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), uuid.NewString(), s.testState, s.passphrase)
s.Error(err)
s.True(biz.IsNotFound(err))
})

s.Run("the run exists", func() {
err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase)
_, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase)
s.NoError(err)

got, err := s.AttestationState.Read(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.passphrase)
Expand All @@ -93,7 +93,7 @@ func (s *attestationStateTestSuite) TestSave() {
})

s.Run("it can be overridden", func() {
err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase)
_, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase)
s.NoError(err)

got, err := s.AttestationState.Read(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.passphrase)
Expand All @@ -103,7 +103,7 @@ func (s *attestationStateTestSuite) TestSave() {
}

newState := &v1.CraftingState{}
err = s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), newState, s.passphrase)
_, err = s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), newState, s.passphrase)
s.NoError(err)

got, err = s.AttestationState.Read(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.passphrase)
Expand All @@ -120,7 +120,7 @@ func (s *attestationStateTestSuite) TestSave() {

// Write again passing the base digest
update := &v1.CraftingState{Attestation: &v1.Attestation{Annotations: map[string]string{"key": "updated"}}}
err = s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), update, s.passphrase, biz.WithAttStateBaseDigest(state.Digest))
_, err = s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), update, s.passphrase, biz.WithAttStateBaseDigest(state.Digest))
s.NoError(err)

// The digest now should be different
Expand All @@ -130,21 +130,47 @@ func (s *attestationStateTestSuite) TestSave() {
s.NotEqual(state.Digest, updatedState.Digest)

// trying to update passing the wrong base digest should fail with a conflict error
err = s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), update, s.passphrase, biz.WithAttStateBaseDigest(state.Digest))
_, err = s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), update, s.passphrase, biz.WithAttStateBaseDigest(state.Digest))
s.Error(err)
s.True(biz.IsErrAttestationStateConflict(err))

// but will not fail if base digest is not passed
// trying to update passing the wrong base digest should fail with a conflict error
err = s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), update, s.passphrase, biz.WithAttStateBaseDigest(""))
_, err = s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), update, s.passphrase, biz.WithAttStateBaseDigest(""))
s.NoError(err)
})

s.Run("save returns digest that can be used for consecutive writes", func() {
// First save
firstDigest, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase)
s.NoError(err)
s.NotEmpty(firstDigest)

// Second save using the returned digest as base_digest should succeed
update1 := &v1.CraftingState{Attestation: &v1.Attestation{Annotations: map[string]string{"key": "update1"}}}
secondDigest, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), update1, s.passphrase, biz.WithAttStateBaseDigest(firstDigest))
s.NoError(err)
s.NotEmpty(secondDigest)
s.NotEqual(firstDigest, secondDigest)

// Third save using the second returned digest should also succeed
update2 := &v1.CraftingState{Attestation: &v1.Attestation{Annotations: map[string]string{"key": "update2"}}}
thirdDigest, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), update2, s.passphrase, biz.WithAttStateBaseDigest(secondDigest))
s.NoError(err)
s.NotEmpty(thirdDigest)
s.NotEqual(secondDigest, thirdDigest)

// Verify the returned digest matches what Read returns
readState, err := s.AttestationState.Read(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.passphrase)
s.NoError(err)
s.Equal(thirdDigest, readState.Digest)
})
}

func (s *attestationStateTestSuite) TestRead() {
ctx := context.Background()
s.T().Run("can be retrieved with same passphrase", func(t *testing.T) {
err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase)
_, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase)
s.NoError(err)

got, err := s.AttestationState.Read(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.passphrase)
Expand All @@ -155,7 +181,7 @@ func (s *attestationStateTestSuite) TestRead() {
})

s.T().Run("it fails if they are different passphrases", func(t *testing.T) {
err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase)
_, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase)
s.NoError(err)

got, err := s.AttestationState.Read(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), "wrong-passphrase")
Expand All @@ -164,11 +190,11 @@ func (s *attestationStateTestSuite) TestRead() {
})

s.T().Run("it fails if the content has been tampered with", func(t *testing.T) {
err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase)
_, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase)
s.NoError(err)

// tamper directly with the database
err = s.Repos.AttestationState.Save(ctx, s.runOrg1.ID, []byte("tampered data modified directly in the DB"), "")
_, err = s.Repos.AttestationState.Save(ctx, s.runOrg1.ID, []byte("tampered data modified directly in the DB"), "")
s.NoError(err)

got, err := s.AttestationState.Read(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.passphrase)
Expand All @@ -180,7 +206,7 @@ func (s *attestationStateTestSuite) TestRead() {
func (s *attestationStateTestSuite) TestWorkflowRunLifecycle() {
ctx := context.Background()
s.T().Run("the state gets cleared when workflow run is set as finished", func(t *testing.T) {
err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase)
_, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase)
s.NoError(err)

err = s.WorkflowRun.MarkAsFinished(ctx, s.runOrg1.ID.String(), biz.WorkflowRunSuccess, "finished")
Expand All @@ -192,7 +218,7 @@ func (s *attestationStateTestSuite) TestWorkflowRunLifecycle() {
})

s.T().Run("or it expires", func(t *testing.T) {
err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase)
_, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase)
s.NoError(err)

err = s.Repos.WorkflowRunRepo.Expire(ctx, s.runOrg1.ID)
Expand All @@ -208,7 +234,7 @@ func (s *attestationStateTestSuite) TestReset() {
ctx := context.Background()

s.T().Run("the run is initialized", func(t *testing.T) {
err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase)
_, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase)
s.NoError(err)

ok, err := s.AttestationState.Initialized(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String())
Expand Down
15 changes: 13 additions & 2 deletions app/controlplane/pkg/data/attestationstate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,14 @@ func (r *AttestationStateRepo) Initialized(ctx context.Context, runID uuid.UUID)

// baseDigest, when provided will be used to check that it matches the digest of the state currently in the DB
// if the digests do not match, the state has been modified and the caller should retry
func (r *AttestationStateRepo) Save(ctx context.Context, runID uuid.UUID, state []byte, baseDigest string) error {
return WithTx(ctx, r.data.DB, func(tx *ent.Tx) error {
// It returns the digest of the newly saved state.
func (r *AttestationStateRepo) Save(ctx context.Context, runID uuid.UUID, state []byte, baseDigest string) (string, error) {
newDigest, err := digest(state)
if err != nil {
return "", fmt.Errorf("failed to calculate digest of new state: %w", err)
}

err = WithTx(ctx, r.data.DB, func(tx *ent.Tx) error {
// compared the provided digest with the digest of the state in the DB
// TODO: make digest check mandatory on updates
if baseDigest != "" {
Expand Down Expand Up @@ -86,6 +92,11 @@ func (r *AttestationStateRepo) Save(ctx context.Context, runID uuid.UUID, state
}
return nil
})
if err != nil {
return "", err
}

return newDigest, nil
}

func (r *AttestationStateRepo) Read(ctx context.Context, runID uuid.UUID) ([]byte, string, error) {
Expand Down
7 changes: 6 additions & 1 deletion pkg/attestation/crafter/statemanager/remote/remote.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,15 @@
}

r.logger.Debug().Str("key", key).Str("baseDigest", state.UpdateCheckSum).Msg("Writing state to remote")
if _, err := r.client.Save(ctx, &pb.AttestationStateServiceSaveRequest{WorkflowRunId: key, AttestationState: state.CraftingState, BaseDigest: state.UpdateCheckSum}); err != nil {
resp, err := r.client.Save(ctx, &pb.AttestationStateServiceSaveRequest{WorkflowRunId: key, AttestationState: state.CraftingState, BaseDigest: state.UpdateCheckSum})
if err != nil {
return fmt.Errorf("failed to save state: %w", err)
}

// Update the checksum with the digest of the newly saved state
// so subsequent writes use the correct base digest for OCC
state.UpdateCheckSum = resp.GetDigest()

Check failure on line 77 in pkg/attestation/crafter/statemanager/remote/remote.go

View workflow job for this annotation

GitHub Actions / lint (main-module) 

resp.GetDigest undefined (type *"github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1".AttestationStateServiceSaveResponse has no field or method GetDigest) (typecheck)

@cubic-dev-ai cubic-dev-ai Bot Mar 21, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: Avoid overwriting UpdateCheckSum with an empty digest; it can disable OCC on the next write by sending an empty base_digest.

(Based on your team's feedback about cross-component and version compatibility.)

View Feedback

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/attestation/crafter/statemanager/remote/remote.go, line 77:

<comment>Avoid overwriting `UpdateCheckSum` with an empty digest; it can disable OCC on the next write by sending an empty `base_digest`.

(Based on your team's feedback about cross-component and version compatibility.) </comment>

<file context>
@@ -67,10 +67,15 @@ func (r *Remote) Write(ctx context.Context, key string, state *crafter.Versioned
 
+	// Update the checksum with the digest of the newly saved state
+	// so subsequent writes use the correct base digest for OCC
+	state.UpdateCheckSum = resp.GetDigest()
+
 	return nil
</file context>
Suggested change
state.UpdateCheckSum = resp.GetDigest()
digest := resp.GetDigest()
if digest == "" {
return fmt.Errorf("failed to save state: empty digest in response")
}
state.UpdateCheckSum = digest
Fix with Cubic


return nil
}

Expand Down
Loading