Skip to content

refactor(api): remove deprecated attestation and bundle fields from Store#3056

Merged
migmartri merged 6 commits intochainloop-dev:mainfrom
migmartri:fix/attestation-store-remove-deprecated-fields
Apr 17, 2026
Merged

refactor(api): remove deprecated attestation and bundle fields from Store#3056
migmartri merged 6 commits intochainloop-dev:mainfrom
migmartri:fix/attestation-store-remove-deprecated-fields

Conversation

@migmartri
Copy link
Copy Markdown
Member

Summary

  • Remove the deprecated attestation and bundle fields from AttestationServiceStoreRequest, keeping only attestation_bundle.
  • Drop the envelope/legacy-bundle fallback paths in the CLI, controlplane service/biz/data layers, and the pkg/attestation helper.
  • The CLI now sends a single serialized bundle per Store RPC instead of three, roughly tripling the maximum attestation size that fits under the server's 16 MB limit.

Closes #3055

…tore

Closes chainloop-dev#3055

The AttestationServiceStoreRequest previously carried three representations
of the same payload: the raw DSSE envelope, the original Sigstore bundle
(with the signature bug from chainloop-dev#1832), and the fixed attestation bundle. Only
attestation_bundle is consumed today; the other two have been deprecated
since June 2025 and the fixed bundle has been available since February 2025.

This removes the deprecated fields and their fallback paths in the CLI,
controlplane service/biz/data layers, and the attestation helper package,
cutting the gRPC request size by ~3x for the same logical payload.

Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 13 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="app/controlplane/pkg/biz/workflowrun.go">

<violation number="1" location="app/controlplane/pkg/biz/workflowrun.go:342">
P1: Parsing bundle bytes here can panic on malformed-but-valid JSON because DSSE extraction assumes at least one signature. Validate DSSE envelope/signatures before extracting to avoid server crashes.</violation>
</file>

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

Comment thread app/controlplane/pkg/biz/workflowrun.go
Ignore the `FIELD_NO_DELETE` breaking-change rule for
`workflow_run.proto`. The deprecated `attestation` and `bundle` fields
are removed in this change; their tag numbers and names are reserved in
the proto to prevent accidental reuse.

Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
@migmartri migmartri requested a review from a team April 17, 2026 17:02
Validate that the incoming bundle carries a DSSE envelope with at least
one signature before extracting it, so malformed-but-valid-JSON bundles
return a typed error instead of panicking on out-of-range access inside
DSSEEnvelopeFromBundle.

Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
…ngle transaction

Collapse the separate `SaveAttestation` and `SaveBundle` repo calls into
a single `SaveAttestationBundle` that updates the workflow run and
inserts the bundle row inside one ent transaction. The previous
two-call sequence could leave the workflow run with an attestation
digest set but no bundle row (or vice versa) if the second call failed.

Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
ent's UpdateOneID returns a NotFoundError (never a nil run alongside a
nil error) when the row is missing, so the extra `run == nil` check was
dead code carried over from the previous SaveAttestation implementation.
Switch to Exec since the returned row was unused.

Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
@migmartri migmartri merged commit 66fd617 into chainloop-dev:main Apr 17, 2026
15 checks passed
_, envBytes := testEnvelope(t, path)
var env dsse.Envelope
require.NoError(t, json.Unmarshal(envBytes, &env))
b, err := attestation.BundleFromDSSEEnvelope(&env)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This method won't be needed anymore, right?

return DSSEEnvelopeFromBundle(&attBundle), nil
}

// TODO: remove this fix once `AttestationServiceStoreRequest.Bundle` is fully removed,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What about this? I think we can remove this logic as well already. Or at least leave the comment as it is

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.

CLI attestation push sends the payload three times in a single Store request

3 participants