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

fixes #610 add an optional regex parameter allowing users to compare … #611

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

aalsabag
Copy link

@aalsabag aalsabag commented Jun 15, 2023

…user commands to regex as opposed to a static string

Fixes issue #610:

Description of the changes being introduced by the pull request:
Users would be able to pass a regex to compare a user command to. An example would look like this. This would accept users using vi or /usr/bin/vi

command = ["/usr/bin/vi", "file1", "file2"] 
regex = "(/usr/bin/)?vi file1 file2"
verify_command_alignment(command, regex=regex)

Please verify and check that the pull request fulfills the following
requirements
:

  • [ x] The code follows the Code Style Guidelines
  • [ x] Tests have been added for the bug fix or new feature
  • Docs have been added for the bug fix or new feature

…ompare user commands to regex as opposed to a static string
Copy link
Member

@lukpueh lukpueh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks for the contribution, @aalsabag! I like the idea, though I am not sure, if, at this moment, we want to improve the in-toto verification flow without a related ITE. (@adityasaky, @SantiagoTorres?)

Regardless, I am no fan of mutual exclusive function arguments. Assuming that we accept the proposal, what would you think about making regex a boolean argument, which toggles whether expected_command is interpreted as regex?

@aalsabag
Copy link
Author

Many thanks for the contribution, @aalsabag! I like the idea, though I am not sure, if, at this moment, we want to improve the in-toto verification flow without a related ITE. (@adityasaky, @SantiagoTorres?)

Regardless, I am no fan of mutual exclusive function arguments. Assuming that we accept the proposal, what would you think about making regex a boolean argument, which toggles whether expected_command is interpreted as regex?

This was actually my original approach but I had abandoned it because of the complications of the expected_command being treated like a list. But I think I can make it work. I'll update the PR shortly!

With regards to the ITE, I must confess, I didn't know it existed 😂. I can fill it out if that's what @SantiagoTorres would like. I tried to make the change such that it doesn't impact existing functionality at all (i.e only warns).

@lukpueh
Copy link
Member

lukpueh commented Jun 16, 2023

Many thanks for the contribution, @aalsabag! I like the idea, though I am not sure, if, at this moment, we want to improve the in-toto verification flow without a related ITE. (@adityasaky, @SantiagoTorres?)
Regardless, I am no fan of mutual exclusive function arguments. Assuming that we accept the proposal, what would you think about making regex a boolean argument, which toggles whether expected_command is interpreted as regex?

This was actually my original approach but I had abandoned it because of the complications of the expected_command being treated like a list. But I think I can make it work. I'll update the PR shortly!

Oh right, I forgot that expected_command is a list. 😬 Then my suggestion is probably not a good idea.

@lukpueh
Copy link
Member

lukpueh commented Jun 16, 2023

But regardless of technical details, we should think this through in terms of the verification flow...

The expected_command value (also list of strings) is authenticated via layout and its interpretation is defined in the spec. How will a client know that they should treat the value as regex?

@aalsabag
Copy link
Author

I think i understand what you mean. That's a very good point that I didn't think of. Essentially, how would one know that expected_command is a regex just from looking at the layout?
I think I need to think about what that would look like in the spec

@SantiagoTorres
Copy link
Member

I quite like the idea! Indeed the list bit is to avoid any sort of IFS abuse on alignment. We could definitely collapse it back to a string to match, yet I think making it optional would be best...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants