Skip to content

ssh: explicitly set ~ for home directory unless using cmd.exe #1

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

Merged
merged 6 commits into from
Mar 17, 2025
Merged
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
# Build output
/build/
.idea/
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ If you'd like to contribute to Mutagen, please see the

Mutagen takes security very seriously. If you believe you have found a security
issue with Mutagen, please practice responsible disclosure practices and send an
email directly to [security@mutagen.io](mailto:security@mutagen.io) instead of
email directly to [security@docker.com](mailto:security@docker.com) instead of
opening a GitHub issue. For more information, please see the
[security documentation](SECURITY.md).

Expand Down
7 changes: 3 additions & 4 deletions SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@
If you find a security issue with Mutagen or one of its related projects, please
DO NOT submit it via the issue tracker! Instead, please follow responsible
disclosure practices and send information about security issues directly to
[[email protected]](mailto:[email protected]) so that a proper assessment
can be made and a fix prepared before a wide announcement. You will receive an
acknowledgement within 24 hours. If you do not, please contact the project
maintainer directly at [[email protected]](mailto:[email protected]).
[[email protected]](mailto:[email protected]) so that a proper assessment
can be made and a fix prepared before a wide announcement. Please note that the
`@docker.com` email address is correct — Docker acquired Mutagen in 2023.

Even in cases where you have limited or incomplete information, or you're not
sure whether or not a problem constitutes a security issue, please make contact
Expand Down
85 changes: 51 additions & 34 deletions pkg/agent/dial.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,39 +34,7 @@ const (
// installation should be attempted and whether or not the remote environment is
// cmd.exe-based.
func connect(logger *logging.Logger, transport Transport, mode, prompter string, cmdExe bool) (io.ReadWriteCloser, bool, bool, error) {
// Compute the agent invocation command, relative to the user's home
// directory on the remote. Unless we have reason to assume that this is a
// cmd.exe environment, we construct a path using forward slashes. This will
// work for all POSIX systems and POSIX-like environments on Windows. If we
// know we're hitting a cmd.exe environment, then we use backslashes,
// otherwise the invocation won't work. Watching for cmd.exe to fail on
// commands with forward slashes is actually the way that we detect cmd.exe
// environments.
//
// HACK: We're assuming that none of these path components have spaces in
// them, but since we control all of them, this is probably okay.
//
// HACK: When invoking on Windows systems (whether inside a POSIX
// environment or cmd.exe), we can leave the "exe" suffix off the target
// name. Fortunately this allows us to also avoid having to try the
// combination of forward slashes + ".exe" for Windows POSIX environments.
pathSeparator := "/"
if cmdExe {
pathSeparator = "\\"
}
dataDirectoryName := filesystem.MutagenDataDirectoryName
if mutagen.DevelopmentModeEnabled {
dataDirectoryName = filesystem.MutagenDataDirectoryDevelopmentName
}
agentInvocationPath := strings.Join([]string{
dataDirectoryName,
filesystem.MutagenAgentsDirectoryName,
mutagen.Version,
BaseName,
}, pathSeparator)

// Compute the command to invoke.
command := fmt.Sprintf("%s %s --%s=%s", agentInvocationPath, mode, FlagLogLevel, logger.Level())
command := fmt.Sprintf("%s %s --%s=%s", agentInvocationPath(cmdExe), mode, FlagLogLevel, logger.Level())

// Set up (but do not start) an agent process.
message := "Connecting to agent (POSIX)..."
Expand Down Expand Up @@ -176,6 +144,55 @@ func connect(logger *logging.Logger, transport Transport, mode, prompter string,
return stream, false, false, nil
}

// agentInvocationPath computes the agent invocation path, relative to the user's home
// directory on the remote.
func agentInvocationPath(cmdExe bool) string {
dataDirectoryName := filesystem.MutagenDataDirectoryName
if mutagen.DevelopmentModeEnabled {
dataDirectoryName = filesystem.MutagenDataDirectoryDevelopmentName
}
return remotePathFromHome(cmdExe,
dataDirectoryName,
filesystem.MutagenAgentsDirectoryName,
mutagen.Version,
BaseName,
)
}

// remotePathFromHome constructs a path string from the given components as a list of relative path components from
// the user's home directory. If cmdExe is true, construct a path cmd.exe will understand.
func remotePathFromHome(cmdExe bool, components ...string) string {
// Unless we have reason to assume that this is a cmd.exe environment, we
// construct a path using forward slashes. This will work for all POSIX
// systems and POSIX-like environments on Windows. If we know we're hitting
// a cmd.exe environment, then we use backslashes, otherwise the invocation
// won't work. Watching for cmd.exe to fail on commands with forward slashes
// is actually the way that we detect cmd.exe environments.
//
// HACK: We're assuming that none of these path components have spaces in
// them, but since we control all of them, this is probably okay.
//
// HACK: When invoking on Windows systems (whether inside a POSIX
// environment or cmd.exe), we can leave the "exe" suffix off the target
// name. Fortunately this allows us to also avoid having to try the
// combination of forward slashes + ".exe" for Windows POSIX environments.
//
// HACK: When invoking on cmd.exe, we leave off the ~ prefix, since cmd.exe
// doesn't recognize it. In most cases the initial working directory for SSH
// commands is the home directory, but when possible we try to be explicit,
// to work around systems that use a different directory, such as Coder
// workspaces, which allow different initial working directories to be
// configured.
pathSeparator := "/"
pathComponents := []string{filesystem.HomeDirectorySpecial}
if cmdExe {
pathSeparator = "\\"
pathComponents = nil
}
pathComponents = append(pathComponents, components...)
return strings.Join(pathComponents, pathSeparator)
}

// Dial connects to an agent-based endpoint using the specified transport,
// connection mode, and prompter.
func Dial(logger *logging.Logger, transport Transport, mode, prompter string) (io.ReadWriteCloser, error) {
Expand Down Expand Up @@ -204,7 +221,7 @@ func Dial(logger *logging.Logger, transport Transport, mode, prompter string) (i
}

// Attempt to install.
if err := install(logger, transport, prompter); err != nil {
if err := install(logger, transport, prompter, cmdExe); err != nil {
return nil, fmt.Errorf("unable to install agent: %w", err)
}

Expand Down
20 changes: 19 additions & 1 deletion pkg/agent/dial_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,21 @@
package agent

// TODO: Implement.
import (
"fmt"
"github.com/mutagen-io/mutagen/pkg/mutagen"
"testing"
)

func TestAgentInvocationPath(t *testing.T) {
path := agentInvocationPath(false)
expected := fmt.Sprintf("~/.mutagen/agents/%s/mutagen-agent", mutagen.Version)
if path != expected {
t.Errorf("Invocation path cmdExe=false, expected %s, got %s", expected, path)
}

path = agentInvocationPath(true)
expected = fmt.Sprintf(".mutagen\\agents\\%s\\mutagen-agent", mutagen.Version)
if path != expected {
t.Errorf("Invocation path cmdExe=true, expected %s, got %s", expected, path)
}
}
21 changes: 9 additions & 12 deletions pkg/agent/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func Install() error {

// install attempts to probe an endpoint and install the appropriate agent
// binary over the specified transport.
func install(logger *logging.Logger, transport Transport, prompter string) error {
func install(logger *logging.Logger, transport Transport, prompter string, cmdExe bool) error {
// Detect the target platform.
goos, goarch, posix, err := probe(transport, prompter)
if err != nil {
Expand Down Expand Up @@ -68,14 +68,16 @@ func install(logger *logging.Logger, transport Transport, prompter string) error
if err != nil {
return fmt.Errorf("unable to generate UUID for agent copying: %w", err)
}
destination := BaseName + randomUUID.String()
remoteFileName := BaseName + randomUUID.String()
if goos == "windows" {
destination += ".exe"
remoteFileName += ".exe"
}
if posix {
destination = "." + destination
remoteFileName = "." + remoteFileName
}
if err = transport.Copy(agentExecutable, destination); err != nil {
fullRemotePath := remotePathFromHome(cmdExe, remoteFileName)

if err = transport.Copy(agentExecutable, fullRemotePath); err != nil {
return fmt.Errorf("unable to copy agent binary: %w", err)
}

Expand All @@ -89,7 +91,7 @@ func install(logger *logging.Logger, transport Transport, prompter string) error
if err := prompting.Message(prompter, "Setting agent executability..."); err != nil {
return fmt.Errorf("unable to message prompter: %w", err)
}
executabilityCommand := fmt.Sprintf("chmod +x %s", destination)
executabilityCommand := fmt.Sprintf("chmod +x %s", fullRemotePath)
if err := run(transport, executabilityCommand); err != nil {
return fmt.Errorf("unable to set agent executability: %w", err)
}
Expand All @@ -99,12 +101,7 @@ func install(logger *logging.Logger, transport Transport, prompter string) error
if err := prompting.Message(prompter, "Installing agent..."); err != nil {
return fmt.Errorf("unable to message prompter: %w", err)
}
var installCommand string
if posix {
installCommand = fmt.Sprintf("./%s %s", destination, CommandInstall)
} else {
installCommand = fmt.Sprintf("%s %s", destination, CommandInstall)
}
installCommand := fmt.Sprintf("%s %s", fullRemotePath, CommandInstall)
if err := run(transport, installCommand); err != nil {
return fmt.Errorf("unable to invoke agent installation: %w", err)
}
Expand Down
11 changes: 11 additions & 0 deletions pkg/agent/transport/ssh/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"os/exec"
"path/filepath"
"strconv"
"strings"

"github.com/mutagen-io/mutagen/pkg/agent"
"github.com/mutagen-io/mutagen/pkg/agent/transport"
Expand Down Expand Up @@ -192,6 +193,12 @@ func (t *sshTransport) Command(command string) (*exec.Cmd, error) {

// ClassifyError implements the ClassifyError method of agent.Transport.
func (t *sshTransport) ClassifyError(processState *os.ProcessState, errorOutput string) (bool, bool, error) {
// Windows Powershell introduces line breaks in the errorOutput, which get
// escaped before arriving at this function into a literal `\r` followed by
// a newline. Strip these out so that line breaks don't screw with our
// matching.
errorOutput = strings.ReplaceAll(errorOutput, "\\r\n", "")

// SSH faithfully returns exit codes and error output, so we can use direct
// methods for testing and classification. Note that we may get POSIX-like
// error codes back even from Windows remotes, but that indicates a POSIX
Expand Down Expand Up @@ -228,6 +235,10 @@ func (t *sshTransport) ClassifyError(processState *os.ProcessState, errorOutput
return false, true, nil
} else if process.OutputIsWindowsCommandNotFound(errorOutput) {
return true, true, nil
} else if process.OutputIsWindowsPowershellCommandNotFound(errorOutput) {
// It's Windows Powershell, not cmd.exe, so try (re)installing, but set cmdExe
// to false.
return true, false, nil
}

// Just bail if we weren't able to determine the nature of the error.
Expand Down
2 changes: 2 additions & 0 deletions pkg/filesystem/mutagen.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ const (
// MutagenLicensingDirectoryName is the name of the licensing data directory
// within the Mutagen data directory.
MutagenLicensingDirectoryName = "licensing"

HomeDirectorySpecial = "~"
)

// Mutagen computes (and optionally creates) subdirectories inside the Mutagen
Expand Down
6 changes: 6 additions & 0 deletions pkg/integration/internal_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,9 @@ func (t *testWindowsDockerTransportPrompter) Prompt(_ string) (string, error) {
}

func TestSynchronizationGOROOTSrcToBetaOverDocker(t *testing.T) {
// CODER: we don't care about Docker-based transport, and our SSH changes break it on Linux
t.Skip()

// If Docker test support isn't available, then skip this test.
if os.Getenv("MUTAGEN_TEST_DOCKER") != "true" {
t.Skip()
Expand Down Expand Up @@ -422,6 +425,9 @@ func init() {
}

func TestForwardingToHTTPDemo(t *testing.T) {
// CODER: we don't care about Docker-based transport, and our SSH changes break it on Linux
t.Skip()

// If Docker test support isn't available, then skip this test.
if os.Getenv("MUTAGEN_TEST_DOCKER") != "true" {
t.Skip()
Expand Down
11 changes: 11 additions & 0 deletions pkg/process/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ const (
// windowsCommandNotFoundFragment is a fragment of the error output returned
// on Windows systems when a command cannot be found.
windowsCommandNotFoundFragment = "The system cannot find the path specified"
// windowsPowershellCommandNotFoundFragment is a fragment of the error output
// returned on Windows systems running Powershell when a command cannot be
// found.
windowsPowershellCommandNotFoundFragment = "is not recognized as the name of a cmdlet, function, script file, or operable program."
Copy link
Member

Choose a reason for hiding this comment

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

This seems like it would break if you have a different OS language, but I guess mutagen was already doing similar error string checks previously and I can't really think of a better way to accomplish this. So it's probably fine for now

)

// OutputIsPOSIXCommandNotFound returns whether or not a process' error output
Expand All @@ -38,6 +42,13 @@ func OutputIsWindowsInvalidCommand(output string) bool {
// represents a command not found error on Windows.
func OutputIsWindowsCommandNotFound(output string) bool {
return strings.Contains(output, windowsCommandNotFoundFragment)

}

// OutputIsWindowsPowershellCommandNotFound returns whether or not a process' error
// output represents a command not found error from Windows running Powershell.
func OutputIsWindowsPowershellCommandNotFound(output string) bool {
return strings.Contains(output, windowsPowershellCommandNotFoundFragment)
}

// ExtractExitErrorMessage is a utility function that will attempt to extract
Expand Down
Loading