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

Inconsistent path match behaviour compared to python implementation #209

Open
cosmin79 opened this issue Feb 17, 2023 · 4 comments
Open
Labels
bug Something isn't working

Comments

@cosmin79
Copy link

cosmin79 commented Feb 17, 2023

General information:
Hey. So in the general spec https://github.com/in-toto/docs/blob/master/in-toto-spec.md#433-artifact-rules , we have this statement about the path patterns accepted in expected materials / products paths:
"The 'pattern' value is a path-pattern that will be matched against paths reported in the link metadata, including bash-style wildcards (e.g., '*')."

This is implemented differently between Golang and Python.

Could you comment on the differences?

  • Version (tag or commit):
  • Operating system (Linux/Mac/Windows):
  • Go version:
  • Go build flags (if any):

Description of the bug:
I've noticed the python implementation for in-toto uses fnmatch:
https://github.com/in-toto/in-toto/blob/develop/in_toto/verifylib.py#L676

In the Golang implementation, this is achieved via https://github.com/in-toto/in-toto-golang/blob/master/in_toto/match.go . The following test for example wouldn't be true if using fnmatch https://github.com/in-toto/in-toto-golang/blob/master/in_toto/match_test.go#L49 .

Anything special you want to tell us?

@cosmin79 cosmin79 added the bug Something isn't working label Feb 17, 2023
@adityasaky
Copy link
Member

Hi @cosmin79, thanks for flagging this. I'm wondering what the right course of action here is, as the spec doesn't mandate one way or another. Ideally, it would be better for both implementations to align. @shibumi and @lukpueh WDYT?

@adityasaky
Copy link
Member

I'd wager we should handle this and add tests via in-toto/in-toto#563 to catch regressions in future.

@cosmin79
Copy link
Author

Yea, I think a common test suite to check the different implementations against is one of the nicer way of dealing with it and catching other differences proactively :)

@adityasaky
Copy link
Member

I should also note that the spec is being updated (in-toto/specification#75) to be more explicit about the patterns. That'd make in-toto-golang non compliant.

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
Development

No branches or pull requests

2 participants