[feature] Add source:URI under externalParameters

[laurentsimon]

We need to add source for our BYOB builders.

In https://slsa.dev/provenance/v1 "Migrating from 0.2":

"source": old.invocation.configSource.uri,

which seems to indicate that source is a URI of type string.

In slsa-framework/slsa-verifier#621, the "source" field is currently treated as a resourceDescriptor.

@asraa @ianlewis wdut?

[asraa]

I think it should be a resource descriptor - we get the URI and the hash explicitly, and can also add new fields.

I think they should rephrase the migration as

"source": {
   "uri": old.invocation.configSource.uri,
}

That migration note was made before they switched to ResourceDescriptors.

[kpk47]

I agree with @asraa that source should be a resourceDescriptor. The provenance spec recommends string values in externalParameters, but they aren't required.

The externalParameters and internalParameters are the top-level inputs to the template, meaning inputs not derived from another input. Each is an arbitrary JSON object, though it is RECOMMENDED to keep the structure simple with string values to aid verification. The same field name SHOULD NOT be used for both externalParameters and internalParameters.

Since there's a benefit to using a resourceDescriptor, go for it. Just make sure to keep the fields in sync between the generator and the verifier.

[laurentsimon]

Can we update the SLSA specs? The source is part of the specs but is under-defined, ie our decision here is not specific to GitHub. I'm on board with making it a resourceDescriptor.

[laurentsimon]

"source": {
   "uri": old.invocation.configSource.uri,
}

and maybe also add the digest:

"source": {
    "uri": old.invocation.configSource.uri,
    "digest": old.invocation.configSource.digest,
}

[kpk47]

I support that change. I've opened an issue on the SLSA spec repo to get input.

[ianlewis]

Just to be clear, this is a totally separate Resource Descriptor that's separate from the entry in resolvedDependencies?

[asraa]

Just to be clear, this is a totally separate Resource Descriptor that's separate from the entry in resolvedDependencies?

I understood it to be duplicated, since it is (1) a user input and (2) a resolved dependency pulled into the environment by the builder.

But yeah, actually just the source URI might be a user input, while the resolved dependency may contain the extra info like the digest that's resolved when the builder pulls it. So dupes doesn't really feel right either.

[ianlewis]

I understood it to be duplicated, since it is (1) a user input and (2) a resolved dependency pulled into the environment by the builder.

But yeah, actually just the source URI might be a user input, while the resolved dependency may contain the extra info like the digest that's resolved when the builder pulls it. So dupes doesn't really feel right either.

Ok, so if the source URI only contains the repo as a user input, we might need to look for the digest in the resolvedDependencies.

So something like:

// let p = <predicate>
let sourceURI = p.externalParameters?.source?.uri;
let sourceDigest = p.externalParameters?.source?.digest;
if (sourceURI && !sourceDigest && p.resolvedDependencies) {
  for (let dep of p.resolvedDependencies) {
    if (dep.uri == sourceURI) { // Maybe ignore ref if not present in externalParameters?
      sourceDigest = dep.digest;
    }
  }
}
// verify sourceURI and sourceDigest

[laurentsimon]

well, the user does not provide the source URI as input either on a typical trigger, it's done by the platform, no? My reading of the format (which may be entirely wrong) is that source is set by the builder to simplify verification and because it's an "implicit" user input :D - if that makes any sense

[laurentsimon]

This is related to #2077.