slsa-github-generator icon indicating copy to clipboard operation
slsa-github-generator copied to clipboard

[feature] Report variables in provenance

Open laurentsimon opened this issue 2 years ago • 5 comments

GitHub recently announced the GA of variables https://github.blog/2023-01-10-introducing-required-workflows-and-configuration-variables-to-github-actions/

They are available by default in the reusable workflow. We should report them as they are a source of user inputs.

For generators, this is important. For builders, it's less important since our builders do not use them.

Thanks @MarkLodato for pointing this out.

P0: generators

  • [ ] generic
  • [ ] container

P1: builders

  • [ ] go
  • [ ] docker

laurentsimon avatar Jan 19 '23 04:01 laurentsimon

Thought on milestone for this feature? Shall we target BYOB?

laurentsimon avatar Jan 19 '23 04:01 laurentsimon

I agree that it is important for generators because the "build" includes the caller's workflow. In SLSA Provenance v1 terminology, the external parameters are the top-level workflow, the inputs, and now the vars.

For the builders, I think we do not need to list it since the "build" is just the reusable workflow, and the external parameters are just the reusable workflow's inputs plus the top-level workflow (passed in via the github context). Since the builders do not use the vars, they are ignored.

Do you agree?

MarkLodato avatar Jan 19 '23 16:01 MarkLodato

That makes sense to me. The top-level workflow need not even be reported for builders since it's not an input and does not influence the builder. But because it's still useful for folks who want to enforce a policy on which workflow triggered their build, I think it's fine to keep it.

@ianlewis wdut?

laurentsimon avatar Jan 19 '23 21:01 laurentsimon

Ok, I think that makes sense. So from a "reproducibility" standpoint, for builders we only really care about what happens in the context of the reusable workflow. For generators we do care because presumably it's used and can affect the build.

Though I think it may depend on the builder. for example, with npm we planned to allow running script commands which effectively runs arbitrary code which could potentially use the vars. Though since they aren't environment variables and would need to be passed to the script explicitly we could just not support that. Some builders might ultimately need that support though depending on user needs.

ianlewis avatar Jan 20 '23 02:01 ianlewis

Though since they aren't environment variables and would need to be passed to the script explicitly we could just not support that. Some builders might ultimately need that support though depending on user needs.

I think we should record them regardless for BYOB since a TRW could technically make use of them.

ianlewis avatar May 13 '24 01:05 ianlewis