Skip to content
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

Oversight in the SLAS v1 field invocationID? #260

Closed
chuangw6 opened this issue Aug 24, 2023 · 7 comments · Fixed by #328
Closed

Oversight in the SLAS v1 field invocationID? #260

chuangw6 opened this issue Aug 24, 2023 · 7 comments · Fixed by #328
Labels
bug Something isn't working

Comments

@chuangw6
Copy link
Contributor

Description of the bug:

In the SLSA v1 provenance, the json tag for the field invocation id should be invocationId instead of invocationID per slsa v1 spec on the official website and the proto.

But the tag in in-toto-golang is invocationID

InvocationID string `json:"invocationID,omitempty"`

@chuangw6 chuangw6 added the bug Something isn't working label Aug 24, 2023
@chuangw6
Copy link
Contributor Author

cc @adityasaky

@adityasaky
Copy link
Member

I think it's preferable to fix this by directly using the proto. I know @marcelamelara was interested in getting this done too, unfortunately I've been a little swamped. @marcelamelara do you have any cycles to help out here?

@chuangw6
Copy link
Contributor Author

I think it's preferable to fix this by directly using the proto.

Agreed.

@marcelamelara
Copy link
Contributor

+1 to switching to using the protobufs in this implementation. IIRC, the last time I looked at making this change happen, it wasn't quite as simple as replacing one struct for the other, but I'll have some cycles in the next week or two, so I can take a look.

@msuozzo
Copy link
Contributor

msuozzo commented May 14, 2024

The aforementioned change is still not submitted so the proto version is effectively unusable. Can we merge the fix for this issue in the meantime? #328

@msuozzo
Copy link
Contributor

msuozzo commented May 15, 2024

Is this worth a patch-release? This is technically a bug fix but I can pin to the commit hash if necessary.

@adityasaky
Copy link
Member

I'll try and tag a release tomorrow. There are a couple pending updates etc we should probably merge as well, I'll check if someone has a few to take a pass at fixing some of the CI issues!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants