policy: add recursive provenance material resolution#3662
policy: add recursive provenance material resolution#3662crazy-max merged 4 commits intodocker:masterfrom
Conversation
tonistiigi
force-pushed
the
policy-recursive-materials
branch
from
58c88a4 to
9f8d762
Compare
tonistiigi
force-pushed
the
policy-recursive-materials
branch
from
9f8d762 to
b71956d
Compare
| return dockerMaterialSource(uri, dgst) | ||
| } | ||
|
|
||
| if gu, err := gitutil.ParseURL(uri); err == nil { |
There was a problem hiding this comment.
Was looking at our gitutil and it only accepts http|https|ssh|git so common SLSA-style git+https://... URIs fail parse and then fail HTTP detection, so they are treated as unsupported and never resolved: https://slsa.dev/spec/v1.2/build-provenance#builddefinition
"externalParameters": {
"repository": "https://github.com/octocat/hello-world",
"ref": "refs/heads/main"
},
"resolvedDependencies": [{
"uri": "git+https://github.com/octocat/hello-world@refs/heads/main",
"digest": {"gitCommit": "7fd1a60b01f91b314f59955a4e4d4e80d8edf11d"}
}]There was a problem hiding this comment.
I don't think buildkit can produce git+https:// atm and there isn't specific git definition for purl. Just https URLs are used, and gitutil figures out if the URL is pointing to a repo.
| if _, _, err := parseSLSAMaterial(rd); err != nil { | ||
| if logf != nil { | ||
| logf(logrus.WarnLevel, fmt.Sprintf("skipping unsupported provenance material %q: %v", m.URI, err)) | ||
| } | ||
| continue | ||
| } |
There was a problem hiding this comment.
iiuc malformed material entries are silently filtered out and that includes digest mismatch errors, so invalid/tampered materials are omitted from input.image.provenance.materials instead of causing evaluation failure. Could that potentially hide the material records policy is supposed to inspect?
There was a problem hiding this comment.
This should be preferred as we don't want images to fail automatically when new material types are added. Instead atm it will fail based on policy rules. So if the user has a policy rule expecting a specific material, but it fails to parse in here, then that policy rule will fail the build.
|
Lint issue with slsa1: https://github.com/docker/buildx/actions/runs/22424348729/job/64929097618?pr=3662#step:3:431 Needs similar golangci-lint rule: https://github.com/moby/buildkit/blob/373d0adc5c452f65d2e42300a47f328b0a55c40b/.golangci.yml#L127-L129 |
Unify root/material unknown resolution with recursive Input traversal. Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Track fields reloaded during eval --print resolution loops and filter final invalid-field warnings against that set. Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
tonistiigi
force-pushed
the
policy-recursive-materials
branch
from
1527c6a to
88cba2c
Compare
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Allow child refs from partial evaluation to match allowed parent keys on path boundaries and return canonical unknowns for metadata resolution. Add tests for parent-child matching and boundary safety cases. Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
|
I did some more testing and unfortunately this does not behave quite as well as I would have hoped. The properties of materials that are not initially present are only loaded if the material is accessed via a static index. If it is accessed dynamically, eg. or Then the rego library can not figure out what fields of the materials are actually required to be loaded. So while this is working (and maybe more useful in the I added some new examples to https://github.com/tonistiigi/buildx-rego-examples |
crazy-max
left a comment
crazy-max
left a comment
There was a problem hiding this comment.
Ok iiuc recursive material loading is currently reliable only for statically indexed material access, while dynamic iteration (every m in input.image.provenance.materials) doesn't give enough hints for lazy resolution.
As follow-up I agree that we should either extend unknown-field discovery in the Rego lib or add a dedicated helper that pre-resolves/iterates material inputs in a policy-friendly way.
2 participants
closes #3639