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

interaction_handler: Don't put response_data on log #251

Conversation

sorah
Copy link

@sorah sorah commented May 17, 2015

interaction_handler is great. but I want keeping use debug level logging to get command's output, but when interaction_handler feature puts content of sending data to debug log with message Sending.

data sent using interaction_handler may contain sensitive data, so please reconsider logging content by default. Note that the default log level is set to debug.

Data sent using interaction_handler may contain sensitive data.
It was on debug level, but the default log level is set to debug.
@leehambley
Copy link
Member

Thanks for the patch @sorah, whilst I agree, this will change behaviour and I can't accept it as it. I suggest writing a slightly larger patch which allows the interaction handler to take a callback to be called with the various SSH callbacks (such as on_response, etc) which can then be more thoroughly tested, so as not to break the existing behaviour for people

@sorah
Copy link
Author

sorah commented May 17, 2015

okay it's fine.

@sorah
Copy link
Author

sorah commented May 17, 2015

umm I couldn't imagine that acceptable plan. could you implement it instead of me?

@robd
Copy link
Contributor

robd commented May 17, 2015

Hi @sorah

Thanks for trying out the interaction handler and giving your feedback!

I was not sure about logging of the response when I wrote this. I will give this some more thought and try to come up with a PR which allows you to see the output when testing new interaction rules, but disable the output once the rules are finalised.

Thanks,
Rob

@sorah
Copy link
Author

sorah commented May 17, 2015

try to come up with a PR which allows you to see the output when testing new interaction rules, but disable the output once the rules are finalised.

Yes, that'd be great! ✨

@robd
Copy link
Contributor

robd commented May 17, 2015

@sorah, @leehambley I think this approach might work. Basically, no longer log by default, but add support for a log_level parameter when you're first setting up the InteractionHandler or trying to debug:

# Initial run
execute(:unfamiliar_command, MappingInteractionHandler.new({}, :debug))
# => DEBUG output: Unable to find interaction handler mapping for stdout: "Some output\n" so no response was sent

# Check log, add mapping
execute(:unfamiliar_command, MappingInteractionHandler.new({
  "Some output\n"=>"Some Input\n"
}, :debug))

# Repeat until mappings are complete
execute(:unfamiliar_command, MappingInteractionHandler.new({
  "Some output\n"=>"Some Input\n",
  "Other output\n"=>"Other Input\n"
}, :debug))

# Once finalised, remove `MappingInteractionHandler` constructor / debug parameter 
execute(:unfamiliar_command, {
  "Some output\n"=>"Some Input\n",
  "Other output\n"=>"Other Input\n"
})

If happy, I will create a PR once #249 is merged

@robd
Copy link
Contributor

robd commented May 18, 2015

@sorah Does #253 help?

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