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

[sonarly]

Automated fix for bug 17198

Severity: high

Summary

The AttestationStateServiceSaveResponse proto message is empty and does not return the digest of the newly saved state. After a successful save, the client's UpdateCheckSum remains stale, causing subsequent writes to send an outdated base_digest that triggers an OCC conflict error.

User Impact

Any CI/CD pipeline using remote attestation state with multiple collectors (PR metadata, AI agent config) enabled will fail during attestation initialization. The second collector's state save will always fail with a 409 conflict, preventing attestation completion.

Root Cause

The AttestationStateServiceSaveResponse proto message at app/controlplane/api/controlplane/v1/attestation_state.proto:56 is defined as empty:

message AttestationStateServiceSaveResponse {}

When the remote state manager calls Save, it discards the response and never updates the client-side UpdateCheckSum:

// pkg/attestation/crafter/statemanager/remote/remote.go:70-74
if _, err := r.client.Save(ctx, &pb.AttestationStateServiceSaveRequest{
    WorkflowRunId: key, AttestationState: state.CraftingState, BaseDigest: state.UpdateCheckSum,
}); err != nil {
    return fmt.Errorf("failed to save state: %w", err)
}
return nil
// UpdateCheckSum is NEVER updated after a successful save

Meanwhile, the server-side data layer at app/controlplane/pkg/data/attestationstate.go:56-88 computes a SHA-256 digest of the stored state on each save and validates it against the base_digest on subsequent saves. The digest changes every time the state is written because the encrypted payload includes random salt/IV.

Failure sequence during RunCollectors (crafter.go:144-155):

1. LoadCraftingState → Read() returns state + digest D1 → UpdateCheckSum = D1
2. First collector calls AddMaterial* → Write() sends base_digest=D1 → server accepts, new state gets digest D2 → response is empty, UpdateCheckSum stays D1
3. Second collector calls AddMaterial* → Write() sends base_digest=D1 → server sees current digest is D2 → base_digest != storedDigest → returns ErrAttestationStateConflict

This bug was introduced in commit dc1eb2ef which added the OCC mechanism. The OCC design correctly added a digest field to the Read response but omitted it from the Save response, creating an asymmetry where the client cannot track state versions across consecutive writes without performing an explicit Read between each Write.

Triggering cause: This bug manifests whenever multiple collectors are registered (e.g., PR metadata collector + AI agent config collector) that each add materials during initialization. The collectors feature has been expanded over time (PR metadata in earlier releases, AI agent config more recently in commits like 9c664501), making the multi-write scenario increasingly common and the bug more visible.

The fix requires three changes:

1. Add a string digest = 1 field to AttestationStateServiceSaveResponse
2. Return the computed digest from the data/biz/service layers after save
3. Update remote.Write() to capture the response and set state.UpdateCheckSum to the returned digest

Introduced by: migmartri on 2024-07-25 in commit dc1eb2e

feat: add support for optimistic concurrency check (#1133)

Suggested Fix

The fix addresses the OCC conflict bug across all layers of the stack:

1. Proto definition (attestation_state.proto): Added string digest = 1 field to AttestationStateServiceSaveResponse, which was previously empty. This is symmetric with the existing digest field in AttestationStateServiceReadResponse.

2. Data layer (data/attestationstate.go): Changed Save return type from error to (string, error). The digest of the newly saved state is computed before the transaction and returned on success.

3. Business layer (biz/attestationstate.go): Updated AttestationStateRepo interface and AttestationStateUseCase.Save to propagate the digest return value.

4. Service layer (service/attestationstate.go): Captures the returned digest and sets it in the AttestationStateServiceSaveResponse.

5. Remote state manager (remote/remote.go): The critical client-side fix — after a successful Save, updates state.UpdateCheckSum with the digest from the response. This ensures subsequent writes send the correct base_digest for OCC validation.

6. Integration tests: Updated all Save call sites to handle the new return value. Added a new test case "save returns digest that can be used for consecutive writes" that validates three consecutive saves using returned digests succeed, and the final digest matches what Read returns.

NOTE: After merging these changes, make api must be run to regenerate the protobuf files (.pb.go, .ts, etc.) before the code will compile. This is the standard team workflow per CLAUDE.md.

---

Generated by Sonarly

[cubic-dev-ai]

1 issue found across 6 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="pkg/attestation/crafter/statemanager/remote/remote.go">

<violation number="1" location="pkg/attestation/crafter/statemanager/remote/remote.go:77">
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.) [FEEDBACK_USED]</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

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