-
Notifications
You must be signed in to change notification settings - Fork 26
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
Fix intoto statement marshal/unmarshal #326
base: main
Are you sure you want to change the base?
Fix intoto statement marshal/unmarshal #326
Conversation
4ed4f90
to
f749b9c
Compare
Signed-off-by: Andrew Gillis <gillis.andrew@gmail.com>
f749b9c
to
4f3ea5e
Compare
For clarity, here is a diff of the output from the example command: go run cmd/sigstore-go/main.go \
-artifact-digest 76176ffa33808b54602c7c35de5c6e9a4deb96066dba6533f50ac234f4f1f4c6b3527515dc17c06fbe2860030f410eee69ea20079bd3a2c6f3dcf3b329b10751 \
-artifact-digest-algorithm sha512 \
-expectedIssuer https://token.actions.githubusercontent.com \
-expectedSAN https://github.com/sigstore/sigstore-js/.github/workflows/release.yml@refs/heads/main \
examples/bundle-provenance.json diff before.json after.json
4c4
< "type": "https://in-toto.io/Statement/v0.1",
---
> "_type": "https://in-toto.io/Statement/v0.1",
13c13
< "predicate_type": "https://slsa.dev/provenance/v0.2",
---
> "predicateType": "https://slsa.dev/provenance/v0.2", |
Hrm! Thanks for this. Can we get this fixed in intoto/attestation/go, I wonder? Or is this one of those "protobuf code gen knows no gods nor masters" kind of deals? I'm tickled that we never noticed this, but also would prefer to avoid having custom serialization funcs if we can avoid it. As a stop gap this might be fine 🤔. |
Oof, protojson is a pain... I don't think it can be easily fixed upstream due to how the protobuf types are compiled. One thing we can do that could simplify this though: What if we wrap type Statement struct{
in_toto.Statement
}
func (s *Statement) MarshalJSON() ([]byte, error) {
return protojson.Marshal(s.Statement)
}
func (s *Statement) UnmarshalJSON(data []byte) error {
...
}
type VerificationResult struct {
MediaType string `json:"mediaType"`
Statement *Statement `json:"statement,omitempty"`
... That would simplify the code here a bit. Could you also add tests? |
@codysoyland I started off by wrapping it but that makes it a breaking change and involves updating types all over. I'll add some tests tomorrow or over the weekend. |
16c52cd
to
ed182c3
Compare
Signed-off-by: Andrew Gillis <gillis.andrew@gmail.com>
ed182c3
to
be320de
Compare
What @codysoyland proppses makes a lot of sense, I like that idea. |
Signed-off-by: Andrew Gillis <gillis.andrew@gmail.com>
Thank you @gillisandrew! I like this better, but before merging, I'd like to see if the in_toto maintainers would be open to adding these marshal/unmarshal methods in the upstream. Otherwise this will continue to bite other folks... It looks like in-toto/attestation#363 was closed erroneously... perhaps one of us can add those methods in a PR to this file? I think that would be ideal and I bet they would accept it. |
@codysoyland Sounds good. I'll open one there and see what they say. |
…e-go#326 Signed-off-by: Andrew Gillis <gillis.andrew@gmail.com>
This pull request addresses a bug in the JSON marshaling of the in-toto statement in verification results. It currently outputs "predicate_type" instead of "predicateType" required by the spec.
The issue is related to in-toto/attestation#363, this applies the fix mentioned there, defining custom marshaler/unmarshaler on the VerificationResult struct and selectively marshaling/unmarshaling the statement using google.golang.org/protobuf/encoding/protojson