Skip to content

Fixes provenance always enabled #13066

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.

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
wants to merge 2 commits into from
Closed

Conversation

suwakei
Copy link
Contributor

@suwakei suwakei commented Jul 17, 2025

Problem:

Currently, the build option Provenance is hard-coded to true in the runBuild function.

// cmd/compose/build.go L170-L171
func runBuild(...) {
    // ...
	apiBuildOptions, err := opts.toAPIBuildOptions(services)
	apiBuildOptions.Provenance = true // Here's the problem.
    // ...
}

This prevents users from disabling the generation of SLSA provenance metadata(used for software supply chain security and build traceability) at build time.

Since the provenance flag already exists in the buildOptions structure, this hard-coded value should be removed, and users should be able to control it via a command-line flag.

Suggestion for improvement:

Add --provenance flag

Introduce a new --provenance flag to the build command. This will give users explicit control over whether provenance metadata should be generated, making the tool more flexible and intuitive.

As a security best practice, the default value of this flag should be true with true as the default, so provenance is generated unless users explicitly disable it.

Benefits

Improved control: Users can disable provenance generation by specifying --provenance=false.

Code clarity: Removes hard-coding and delegates control to existing buildOptions structure.

Security best practice: Enabling provenance by default supports secure software supply chains.

@suwakei suwakei requested a review from a team as a code owner July 17, 2025 14:06
@suwakei suwakei requested review from ndeloof and glours July 17, 2025 14:06
Copy link
Contributor

@glours glours left a comment

Choose a reason for hiding this comment

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

👍 LGTM
For legal reasons you need to sing-off your commits

@glours
Copy link
Contributor

glours commented Jul 17, 2025

Ho and you will need to run make docs to update the doc files

@ndeloof
Copy link
Contributor

ndeloof commented Jul 17, 2025

this is somehow contradictory with #12853

Also, usage of this option is a bit obscure to me, as setting this to false enables provenance attestation: https://github.com/docker/compose/blob/main/pkg/compose/build.go#L474-L476

@ndeloof
Copy link
Contributor

ndeloof commented Jul 17, 2025

This also makes me wonder we don't have attestation (or provenance?) attribute in the compose-spec

@ndeloof
Copy link
Contributor

ndeloof commented Jul 17, 2025

.. also need to check we correctly behave when BUILDX_NO_DEFAULT_ATTESTATIONS is set

@glours
Copy link
Contributor

glours commented Jul 17, 2025

Good catches @ndeloof, my bad, my review was too fast 🤦‍♂️

@ndeloof
Copy link
Contributor

ndeloof commented Jul 17, 2025

Not strictly speaking a solution to this issue, but I created compose-spec/compose-go#809 to offer better control over attestation generation.

@suwakei
Copy link
Contributor Author

suwakei commented Jul 18, 2025

@ndeloof @glours
Thank you for the ongoing work on this!
I see that #13067 offers a more comprehensive solution by updating the compose-spec and aligning with the attestations documentation.

Since that PR supersedes this one and addresses the same goal in a more extensible way,
I intend to go ahead and close this PR, and I’d be happy to contribute to #13067 if that’s helpful.

Happy to review or help refine anything there.
Really appreciate the feedback and learning opportunity — it’s been very helpful!

@ndeloof
Copy link
Contributor

ndeloof commented Jul 21, 2025

Closing as #13067 was merged

@ndeloof ndeloof closed this Jul 21, 2025
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