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

Safe logging #47

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ project/boot
project/build/target
project/plugins/lib_managed
project/plugins/src_managed
project/plugins/target
project/plugins/target
9 changes: 6 additions & 3 deletions src/main/scala/com/decodified/scalassh/Command.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,14 @@ package com.decodified.scalassh

import java.io.{ByteArrayInputStream, File, FileInputStream, InputStream}
import net.schmizz.sshj.connection.channel.direct.Session

final case class Command(command: String, input: CommandInput = CommandInput.NoInput, timeout: Option[Int] = None)
final case class Command(command: String,
input: CommandInput = CommandInput.NoInput,
timeout: Option[Int] = None,
safeToLog: Boolean = true)

object Command {
implicit def fromString(cmd: String): Command = Command(cmd)
def apply(cmd: String, safeToLog: Boolean): Command = new Command(cmd, safeToLog = safeToLog)
def apply(cmd: String): Command = new Command(cmd, safeToLog = true)
Copy link
Author

Choose a reason for hiding this comment

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

This is more for people looking at the change, it can removed, there is no need to supply safeToLog here. Obviously this can also just be added to the readme.

}

final case class CommandInput(inputStream: Option[InputStream])
Expand Down
12 changes: 8 additions & 4 deletions src/main/scala/com/decodified/scalassh/SshClient.scala
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import net.schmizz.sshj.SSHClient
import net.schmizz.sshj.connection.channel.direct.Session
import net.schmizz.sshj.userauth.keyprovider.KeyProvider
import net.schmizz.sshj.userauth.method.AuthMethod

import scala.io.Source
import scala.util.{Failure, Success, Try}
import scala.util.control.NonFatal
Expand All @@ -36,7 +35,7 @@ final class SshClient(val config: HostConfig) extends ScpTransferable {
lazy val endpoint = config.hostName + ':' + config.port
lazy val authenticatedClient = connect(client).flatMap(authenticate)
val client = createClient(config)

val unsafeLogMsg = "Sensitive data, not logged"
def exec(command: Command): Try[CommandResult] =
authenticatedClient.flatMap { client =>
startSession(client).flatMap { session =>
Expand All @@ -55,7 +54,9 @@ final class SshClient(val config: HostConfig) extends ScpTransferable {
}

def execWithSession(command: Command, session: Session): Try[CommandResult] = {
log.info("Executing SSH command on {}: \"{}\"", Seq(endpoint, command.command): _*)
log.info(
"Executing SSH command on {}: \"{}\"",
Seq(endpoint, if (command.safeToLog) command.command else unsafeLogMsg): _*)
protect("Could not execute SSH command on") {
val channel = session.exec(command.command)
command.input.inputStream.foreach(new StreamCopier().copy(_, channel.getOutputStream))
Expand Down Expand Up @@ -155,7 +156,10 @@ final class SshClient(val config: HostConfig) extends ScpTransferable {

protected def protect[T](errorMsg: => String)(f: => T): Try[T] =
try Success(f)
catch { case NonFatal(e) => Failure(SSH.Error(errorMsg + " " + endpoint, e)) }
catch {
case NonFatal(e) =>
Failure(SSH.Error(s"$errorMsg executing on $endpoint - message: ${e.getMessage}", e))
}
}

object SshClient {
Expand Down