Provide error message when allow_stdio_access creates and undecideable error #1662
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…e error If two containers are indistinguishable at create_container or exec_external time except for their `allow_stdio_access` value, then we are unable to proceed as we don't know if access should be allowed or not. Prior to this commit, no error message at all was displayed when this was hit making it almost impossible for someone not "in the know" to diagnose. This first version gives a very simple error message that will be improved with more information in later commits. Signed-off-by: Sean T. Allen <seanallen@microsoft.com>
anmaxvl
reviewed
Feb 15, 2023
| input.rule == "create_container" | ||
|
|
||
| not container_started | ||
| possible_containers := [container | |
There was a problem hiding this comment.
it seems like a lot of the logic here is also used in create_container. Can we define a function that will return the matching containers (minus stdio) and reuse it here?
There was a problem hiding this comment.
Yes but this was intentional as when we make this a better error message (coming in the not so distant future) this might no longer share all the logic. So I intentionally decided to not do that now given additional changes coming soon.
If you want it refactored now knowing that it might be undone when this is addressed further in March, I can do it. If come the changes in March, these still are rather similar, I was going to do the refactor to remove duplication then.
| errors["external processes only distinguishable by allow_stdio_access"] { | ||
| input.rule == "exec_external" | ||
|
|
||
| possible_processes := [process | |
anmaxvl
approved these changes
Feb 15, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If two containers are indistinguishable at create_container or exec_external time except for their
allow_stdio_accessvalue, then we are unable to proceed as we don't know if access should be allowed or not.Prior to this commit, no error message at all was displayed when this was hit making it almost impossible for someone not "in the know" to diagnose.
This first version gives a very simple error message that will be improved with more information in later commits.
Signed-off-by: Sean T. Allen seanallen@microsoft.com