diff --git a/db_lib/AnsibleApp.go b/db_lib/AnsibleApp.go index b9ecc05882..e8905ede8a 100644 --- a/db_lib/AnsibleApp.go +++ b/db_lib/AnsibleApp.go @@ -4,11 +4,14 @@ import ( "crypto/md5" "fmt" "io" + "net/url" "os" "path" + "strings" "github.com/semaphoreui/semaphore/db" "github.com/semaphoreui/semaphore/pkg/galaxy" + "github.com/semaphoreui/semaphore/pkg/ssh" "github.com/semaphoreui/semaphore/pkg/task_logger" ) @@ -54,6 +57,11 @@ type AnsibleApp struct { Playbook *AnsiblePlaybook Template db.Template Repository db.Repository + + // Set for the duration of InstallRequirements. The key is installed on the + // first galaxy run rather than up front, see galaxyGitEnvForRun. + galaxyInstaller AccessKeyInstaller + galaxyKey *ssh.AccessKeyInstallation } func (t *AnsibleApp) SetLogger(logger task_logger.Logger) task_logger.Logger { @@ -90,6 +98,9 @@ func (t *AnsibleApp) InstallRequirements(args LocalAppInstallingArgs) error { return err } + t.galaxyInstaller = args.Installer + defer t.destroyGalaxyKey() + err = t.installCollectionsRequirements(args.EnvironmentVars, collectionArgs) if err != nil { return err @@ -97,6 +108,36 @@ func (t *AnsibleApp) InstallRequirements(args LocalAppInstallingArgs) error { return t.installRolesRequirements(args.EnvironmentVars, roleArgs) } +// galaxyGitEnvForRun returns the git credentials galaxy's clones need. The +// repository key is installed into an agent here rather than in +// InstallRequirements: most tasks have no requirements file to install, and +// installing up front would decrypt the key and start an agent — one more thing +// that can fail — for every task. The installation is reused across files. +func (t *AnsibleApp) galaxyGitEnvForRun() ([]string, error) { + env := galaxyGitEnv(t.Repository) + + if t.galaxyInstaller == nil { + return env, nil + } + + if t.galaxyKey == nil { + installation, err := t.galaxyInstaller.Install(t.Repository.SSHKey, db.AccessKeyRoleGit, t.Logger) + if err != nil { + return nil, err + } + t.galaxyKey = &installation + } + + return append(env, t.galaxyKey.GetGitEnv()...), nil +} + +func (t *AnsibleApp) destroyGalaxyKey() { + if t.galaxyKey != nil { + _ = t.galaxyKey.Destroy() + t.galaxyKey = nil + } +} + // skipGalaxyInstall reports whether the Galaxy install step must be skipped. // The template-level flag provides the default; when the template allows // overriding it, the task-level flag takes precedence. @@ -218,7 +259,61 @@ func (t *AnsibleApp) installCollectionsRequirements(environmentVars, extraArgs [ } func (t *AnsibleApp) runGalaxy(args []string, environmentVars []string) error { - return t.Playbook.RunGalaxy(args, environmentVars) + gitEnv, err := t.galaxyGitEnvForRun() + if err != nil { + return err + } + + // Task variables come last so a manually configured GIT_* var still wins. + return t.Playbook.RunGalaxy(args, append(gitEnv, environmentVars...)) +} + +// sqQuote quotes s for GIT_CONFIG_PARAMETERS: the value is wrapped in single +// quotes, and any single quote inside it is escaped the way sh requires. +func sqQuote(s string) string { + return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'" +} + +// galaxyGitEnv lets ansible-galaxy authenticate to the repository's own git server. +// +// Galaxy shells out to `git clone` for `scm: git` requirements, and those clones +// inherit no credentials, so roles hosted next to the repository fail with +// "could not read Username" (GitHub #3677). Credentials go through +// GIT_CONFIG_PARAMETERS so they stay out of `ps` output and out of the task log, +// which only ever shows the pre-rewrite URL. +func galaxyGitEnv(repo db.Repository) (env []string) { + // Without this git prompts on /dev/tty and the task hangs instead of failing. + env = append(env, "GIT_TERMINAL_PROMPT=0") + + if repo.GetType() != db.RepositoryHTTP || repo.SSHKey.Type != db.AccessKeyLoginPassword { + return + } + + plain, err := url.Parse(repo.GitURL) + if err != nil || plain.Host == "" { + return + } + + // Scoped to this exact scheme://host[:port] so no other server named in + // requirements.yml is ever offered the credential. + plain.Path, plain.RawQuery, plain.Fragment, plain.User = "/", "", "", nil + + withAuth := *plain + if login := repo.SSHKey.LoginPassword.Login; login == "" { + withAuth.User = url.User(repo.SSHKey.LoginPassword.Password) + } else { + withAuth.User = url.UserPassword(login, repo.SSHKey.LoginPassword.Password) + } + + // git splits each GIT_CONFIG_PARAMETERS entry at its first "=", and net/url + // leaves "=" unescaped in userinfo, so a credential containing one would cut + // the key short and abort the clone with "error: invalid key". git decodes + // the escape again when it authenticates. Only the credential can hold one: + // the path, query and fragment are cleared above. + authURL := strings.ReplaceAll(withAuth.String(), "=", "%3D") + + return append(env, "GIT_CONFIG_PARAMETERS="+sqQuote( + "url."+authURL+".insteadOf="+plain.String())) } // galaxyExtraArgs returns the template-configured arguments for one galaxy diff --git a/db_lib/GalaxyGitEnv_test.go b/db_lib/GalaxyGitEnv_test.go new file mode 100644 index 0000000000..f56a14c826 --- /dev/null +++ b/db_lib/GalaxyGitEnv_test.go @@ -0,0 +1,303 @@ +package db_lib + +import ( + "errors" + "os" + "os/exec" + "path" + "strings" + "testing" + + "github.com/semaphoreui/semaphore/db" + "github.com/semaphoreui/semaphore/pkg/ssh" + "github.com/semaphoreui/semaphore/pkg/task_logger" + "github.com/semaphoreui/semaphore/util" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func httpRepo(gitURL, login, password string) db.Repository { + return db.Repository{ + GitURL: gitURL, + SSHKey: db.AccessKey{ + Type: db.AccessKeyLoginPassword, + LoginPassword: db.LoginPassword{Login: login, Password: password}, + }, + } +} + +func TestSqQuote(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + {"plain", "abc", "'abc'"}, + {"empty", "", "''"}, + {"embedded quote", "a'b", `'a'\''b'`}, + {"only quote", "'", `''\'''`}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, sqQuote(tt.input)) + }) + } +} + +func TestGalaxyGitEnv_AlwaysDisablesTerminalPrompt(t *testing.T) { + for _, repo := range []db.Repository{ + {GitURL: "git@github.com:acme/roles.git"}, + {GitURL: "https://git.private.repo/acme/roles.git"}, + httpRepo("https://git.private.repo/acme/roles.git", "u", "p"), + } { + assert.Contains(t, galaxyGitEnv(repo), "GIT_TERMINAL_PROMPT=0") + } +} + +func TestGalaxyGitEnv_HTTPSWithLoginPassword(t *testing.T) { + env := galaxyGitEnv(httpRepo("https://git.private.repo/acme/roles.git", "semuser", "sempass")) + + require.Len(t, env, 2) + assert.Equal(t, + `GIT_CONFIG_PARAMETERS='url.https://semuser:sempass@git.private.repo/.insteadOf=https://git.private.repo/'`, + env[1]) +} + +// The rewrite must match host and port, or git will not apply it. +func TestGalaxyGitEnv_KeepsPort(t *testing.T) { + env := galaxyGitEnv(httpRepo("http://127.0.0.1:3300/semuser/main-repo.git", "semuser", "sempass")) + + require.Len(t, env, 2) + assert.Contains(t, env[1], "url.http://semuser:sempass@127.0.0.1:3300/.insteadOf=http://127.0.0.1:3300/") +} + +// Matches GetGitURL: an empty login means the password is the whole credential. +func TestGalaxyGitEnv_TokenOnlyKeyUsesPasswordAsUser(t *testing.T) { + env := galaxyGitEnv(httpRepo("https://git.private.repo/acme/roles.git", "", "gho_token")) + + require.Len(t, env, 2) + assert.Contains(t, env[1], "url.https://gho_token@git.private.repo/.insteadOf=") +} + +// A password containing '@' or '/' would otherwise corrupt the URL. +func TestGalaxyGitEnv_EncodesCredentials(t *testing.T) { + env := galaxyGitEnv(httpRepo("https://git.private.repo/acme/roles.git", "user@corp", "p@ss/w:rd")) + + require.Len(t, env, 2) + assert.Contains(t, env[1], "user%40corp:p%40ss%2Fw%3Ard@git.private.repo") + assert.NotContains(t, env[1], "p@ss/w:rd") +} + +func TestGalaxyGitEnv_NoCredentialsForOtherRepoTypes(t *testing.T) { + tests := []struct { + name string + repo db.Repository + }{ + {"ssh url", db.Repository{ + GitURL: "git@github.com:acme/roles.git", + SSHKey: db.AccessKey{Type: db.AccessKeyLoginPassword, + LoginPassword: db.LoginPassword{Login: "u", Password: "p"}}, + }}, + {"https url but ssh key", db.Repository{ + GitURL: "https://git.private.repo/acme/roles.git", + SSHKey: db.AccessKey{Type: db.AccessKeySSH}, + }}, + {"https url but no key", db.Repository{ + GitURL: "https://git.private.repo/acme/roles.git", + SSHKey: db.AccessKey{Type: db.AccessKeyNone}, + }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + env := galaxyGitEnv(tt.repo) + assert.Equal(t, []string{"GIT_TERMINAL_PROMPT=0"}, env) + }) + } +} + +// requirements.yml may name several servers; only the repository's own host +// may ever be offered its credential. +func TestGalaxyGitEnv_ScopesCredentialToOneHost(t *testing.T) { + env := galaxyGitEnv(httpRepo("https://git.private.repo/acme/roles.git", "semuser", "sempass")) + + require.Len(t, env, 2) + assert.Contains(t, env[1], ".insteadOf=https://git.private.repo/") + assert.NotContains(t, env[1], "acme/roles.git") +} + +type fakeInstaller struct { + key db.AccessKey + usage db.AccessKeyRole + env []string + err error + calls int +} + +func (f *fakeInstaller) Install(key db.AccessKey, usage db.AccessKeyRole, _ task_logger.Logger) (ssh.AccessKeyInstallation, error) { + f.key, f.usage = key, usage + f.calls++ + return ssh.AccessKeyInstallation{}, f.err +} + +// setupGalaxyConfig gives the package-level util.Config a temp dir to resolve +// repository paths against, and restores it afterwards. +func setupGalaxyConfig(t *testing.T) { + original := util.Config + t.Cleanup(func() { util.Config = original }) + util.Config = &util.ConfigType{TmpPath: t.TempDir(), Process: &util.ConfigProcess{}} +} + +// stubGalaxy puts a succeeding ansible-galaxy first on PATH and returns a +// function reporting how many times it ran. Without it the real binary runs and +// fails, so InstallRequirements returns before the second requirements file and +// nothing is reused. +func stubGalaxy(t *testing.T) func() int { + t.Helper() + + dir := t.TempDir() + runLog := path.Join(dir, "runs") + + // The path is baked into the script, quoted: makeCmd builds cmd.Env from + // scratch, so an env var set here would not reach the stub. + script := "#!/bin/sh\nprintf 'run\\n' >> " + sqQuote(runLog) + "\n" + require.NoError(t, os.WriteFile(path.Join(dir, "ansible-galaxy"), []byte(script), 0o755)) + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + + return func() int { + content, err := os.ReadFile(runLog) + if err != nil { + return 0 + } + return strings.Count(string(content), "run") + } +} + +// newGalaxyApp builds the app the way AppFactory does, so that runGalaxy has a +// Playbook to run. +func newGalaxyApp(repo db.Repository) *AnsibleApp { + logger := task_logger.NopLogger{} + + return &AnsibleApp{ + Logger: logger, + Repository: repo, + Playbook: &AnsiblePlaybook{Repository: repo, Logger: logger}, + } +} + +// writeRequirements puts a requirements.yml where the app looks for it, so that +// galaxy actually runs. Without one every install is skipped. +func writeRequirements(t *testing.T, app *AnsibleApp) { + t.Helper() + + dir := app.getRepoPath() + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(path.Join(dir, "requirements.yml"), []byte("collections: []\n"), 0o644)) +} + +// The repository's own key must be the one galaxy gets, under the git role. +func TestInstallRequirements_InstallsRepositoryKey(t *testing.T) { + setupGalaxyConfig(t) + + inst := &fakeInstaller{} + app := newGalaxyApp(db.Repository{SSHKey: db.AccessKey{ID: 42, Type: db.AccessKeySSH}}) + writeRequirements(t, app) + galaxyRuns := stubGalaxy(t) + + require.NoError(t, app.InstallRequirements(LocalAppInstallingArgs{Installer: inst})) + + require.Greater(t, galaxyRuns(), 1, "reuse is only meaningful across more than one galaxy run") + assert.Equal(t, 42, inst.key.ID) + assert.Equal(t, db.AccessKeyRole(db.AccessKeyRoleGit), inst.usage) + assert.Equal(t, 1, inst.calls, "one installation must be reused across requirements files") +} + +// Nothing for galaxy to install means no key is decrypted and no agent started. +func TestInstallRequirements_NoRequirementsFileInstallsNoKey(t *testing.T) { + setupGalaxyConfig(t) + + inst := &fakeInstaller{} + app := newGalaxyApp(db.Repository{SSHKey: db.AccessKey{ID: 42, Type: db.AccessKeySSH}}) + + require.NoError(t, app.InstallRequirements(LocalAppInstallingArgs{Installer: inst})) + + assert.Zero(t, inst.calls) +} + +func TestInstallRequirements_FailsWhenKeyInstallFails(t *testing.T) { + setupGalaxyConfig(t) + + app := newGalaxyApp(db.Repository{}) + writeRequirements(t, app) + + err := app.InstallRequirements(LocalAppInstallingArgs{ + Installer: &fakeInstaller{err: errors.New("agent unavailable")}, + }) + + assert.ErrorContains(t, err, "agent unavailable") +} + +// A nil installer is the remote-runner path; it must not panic. +func TestInstallRequirements_NilInstaller(t *testing.T) { + setupGalaxyConfig(t) + + app := newGalaxyApp(db.Repository{}) + + assert.NoError(t, app.InstallRequirements(LocalAppInstallingArgs{})) +} + +// TestGalaxyGitEnv_EscapesEqualsInCredentials covers a credential containing an +// "=", which is common in tokens. git splits a GIT_CONFIG_PARAMETERS entry at +// the first "=", so an unescaped one truncates the key and aborts the clone. +func TestGalaxyGitEnv_EscapesEqualsInCredentials(t *testing.T) { + tests := []struct { + name string + login string + password string + expected string + }{ + {"equals in password", "bob", "tok=en", "bob:tok%3Den@git.private.repo"}, + {"equals in token only login", "", "ghp_ab=cd", "ghp_ab%3Dcd@git.private.repo"}, + {"equals in login", "us=er", "pw", "us%3Der:pw@git.private.repo"}, + {"equals at both ends", "a=b", "c=d", "a%3Db:c%3Dd@git.private.repo"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + env := galaxyGitEnv(httpRepo("https://git.private.repo/acme/roles.git", tt.login, tt.password)) + + require.Len(t, env, 2) + params := strings.TrimPrefix(env[1], "GIT_CONFIG_PARAMETERS=") + assert.Contains(t, params, tt.expected) + + // The key is everything before the first "=", so the rewrite must + // still be the whole url..insteadOf key. + key, _, found := strings.Cut(strings.Trim(params, "'"), "=") + require.True(t, found) + assert.True(t, strings.HasSuffix(key, ".insteadOf"), + "the config key must not be cut short by a credential: %q", key) + }) + } +} + +// TestGalaxyGitEnv_ParsedByGit hands the generated value to the real git binary, +// which is the only thing that decides whether the rewrite installs. +func TestGalaxyGitEnv_ParsedByGit(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git is not installed") + } + + env := galaxyGitEnv(httpRepo("https://git.private.repo/acme/roles.git", "bob", "tok=en")) + require.Len(t, env, 2) + + cmd := exec.Command("git", "config", "--get-regexp", "^url\\.") + cmd.Dir = t.TempDir() + // A clean environment: the developer's own ~/.gitconfig also holds url.* + // rewrites, which would make this pass for the wrong reason. + cmd.Env = []string{env[1], "HOME=" + cmd.Dir, "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null"} + + out, err := cmd.CombinedOutput() + + require.NoError(t, err, "git could not parse the config: %s", out) + assert.Contains(t, string(out), "bob:tok%3Den@git.private.repo") + assert.Contains(t, string(out), "insteadof https://git.private.repo/") +} diff --git a/pkg/ssh/agent.go b/pkg/ssh/agent.go index 7c4b190850..960963e8d8 100644 --- a/pkg/ssh/agent.go +++ b/pkg/ssh/agent.go @@ -178,7 +178,9 @@ func gitHostKeyCheckingOpts() string { case util.SshStrictHostKeyCheckingYes: return fmt.Sprintf("-o StrictHostKeyChecking=yes -o UserKnownHostsFile=%s", util.Config.Ssh.KnownHostsFile) case util.SshStrictHostKeyCheckingNo: - return "ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null" + // No leading "ssh": the caller prepends it, and a second one is taken by + // ssh as the host to connect to ("Could not resolve hostname ssh"). + return "-o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null" case util.SshStrictHostKeyCheckingAcceptNew: return fmt.Sprintf("-o StrictHostKeyChecking=accept-new -o UserKnownHostsFile=%s", util.Config.Ssh.KnownHostsFile) default: diff --git a/pkg/ssh/agent_test.go b/pkg/ssh/agent_test.go index e6ab54605a..dafd138366 100644 --- a/pkg/ssh/agent_test.go +++ b/pkg/ssh/agent_test.go @@ -3,8 +3,11 @@ package ssh import ( "os" "path/filepath" + "strings" "testing" + "github.com/semaphoreui/semaphore/util" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -90,3 +93,45 @@ func TestAgent_Close_FailedInitialization(t *testing.T) { t.Errorf("Expected no error when closing incomplete agent, got: %v", err) } } + +// TestGetGitEnv_SshCommandHasOneSshPrefix covers every host-key checking mode: +// gitHostKeyCheckingOpts must return options only, because GetGitEnv prepends +// "ssh". A second one is read by ssh as the host to connect to and every SSH +// repository fails with "Could not resolve hostname ssh". +func TestGetGitEnv_SshCommandHasOneSshPrefix(t *testing.T) { + original := util.Config + t.Cleanup(func() { util.Config = original }) + + modes := []util.SshStrictHostKeyChecking{ + util.SshStrictHostKeyCheckingYes, + util.SshStrictHostKeyCheckingNo, + util.SshStrictHostKeyCheckingAcceptNew, + } + + for _, mode := range modes { + t.Run(string(mode), func(t *testing.T) { + util.Config = &util.ConfigType{Ssh: &util.SshConfig{ + StrictHostKeyChecking: mode, + KnownHostsFile: filepath.Join(t.TempDir(), "known_hosts"), + }} + + key := AccessKeyInstallation{SSHAgent: &Agent{SocketFile: "/tmp/agent.sock"}} + + var cmd string + for _, env := range key.GetGitEnv() { + if strings.HasPrefix(env, "GIT_SSH_COMMAND=") { + cmd = strings.TrimPrefix(env, "GIT_SSH_COMMAND=") + } + } + + require.NotEmpty(t, cmd) + assert.NotContains(t, cmd, "ssh ssh") + + fields := strings.Fields(cmd) + require.Greater(t, len(fields), 1) + assert.Equal(t, "ssh", fields[0]) + assert.True(t, strings.HasPrefix(fields[1], "-"), + "the argument after ssh must be an option, got %q", fields[1]) + }) + } +}