diff --git a/docker/skills-init/Dockerfile b/docker/skills-init/Dockerfile index 9faed4bcb..d26216e61 100644 --- a/docker/skills-init/Dockerfile +++ b/docker/skills-init/Dockerfile @@ -14,5 +14,5 @@ RUN CGO_ENABLED=0 go build -trimpath -ldflags="-s -w" -o /build/krane . FROM alpine:3.21 -RUN apk add --no-cache git +RUN apk add --no-cache git openssh-client COPY --from=krane-builder /build/krane /usr/local/bin/krane diff --git a/go/core/internal/controller/translator/agent/adk_api_translator.go b/go/core/internal/controller/translator/agent/adk_api_translator.go index d14de599c..1512cffa0 100644 --- a/go/core/internal/controller/translator/agent/adk_api_translator.go +++ b/go/core/internal/controller/translator/agent/adk_api_translator.go @@ -1568,6 +1568,55 @@ func gitSkillName(ref v1alpha2.GitRepo) string { return path.Base(u) } +var ( + scpLikeGitURLRegex = regexp.MustCompile(`^(?:[^@/]+@)?([^:/]+):.+$`) + // validHostPattern rejects shell metacharacters in hostnames and ports to prevent + // command injection when values are interpolated into ssh-keyscan shell commands. + validHostPattern = regexp.MustCompile(`^[A-Za-z0-9.\-]+$`) + validPortPattern = regexp.MustCompile(`^[0-9]+$`) +) + +func gitSSHHost(rawURL string) (sshHostData, bool) { + parsed, err := url.Parse(rawURL) + if err == nil { + switch parsed.Scheme { + case "ssh", "git+ssh": + host := parsed.Hostname() + if host == "" || !validHostPattern.MatchString(host) { + return sshHostData{}, false + } + port := parsed.Port() + if port == "22" { + port = "" // 22 is the SSH default; omit to avoid redundant -p flag + } + if port != "" && !validPortPattern.MatchString(port) { + return sshHostData{}, false + } + return sshHostData{ + Host: host, + Port: port, + }, true + case "http", "https": + return sshHostData{}, false + } + } + + if strings.Contains(rawURL, "://") { + return sshHostData{}, false + } + + matches := scpLikeGitURLRegex.FindStringSubmatch(rawURL) + if len(matches) != 2 { + return sshHostData{}, false + } + host := matches[1] + if !validHostPattern.MatchString(host) { + return sshHostData{}, false + } + + return sshHostData{Host: host}, true +} + // validateSubPath rejects subPath values that are absolute or contain ".." traversal segments. func validateSubPath(p string) error { if p == "" { @@ -1658,35 +1707,10 @@ func prepareSkillsInitData( if authSecretRef != nil { data.AuthMountPath = "/git-auth" - seenHosts := make(map[string]bool) - hostPattern := regexp.MustCompile(`^[A-Za-z0-9\.\-:]+$`) - portPattern := regexp.MustCompile(`^[0-9]+$`) - for _, ref := range gitRefs { - u, err := url.Parse(ref.URL) - if err != nil || u.Scheme != "ssh" { - continue - } - host := u.Hostname() - if host == "" || !hostPattern.MatchString(host) { - continue - } - port := u.Port() - if port == "22" { - port = "" // 22 is the SSH default; omit to avoid -p flag - } - if port != "" && !portPattern.MatchString(port) { - continue - } - key := host + ":" + port - if seenHosts[key] { - continue - } - seenHosts[key] = true - data.SSHHosts = append(data.SSHHosts, sshHostData{Host: host, Port: port}) - } } seen := make(map[string]bool) + seenSSHHosts := make(map[sshHostData]bool) for _, ref := range gitRefs { subPath := strings.TrimSuffix(ref.Path, "/") @@ -1706,6 +1730,13 @@ func prepareSkillsInitData( } seen[name] = true + if authSecretRef != nil { + if sshHost, ok := gitSSHHost(ref.URL); ok && !seenSSHHosts[sshHost] { + seenSSHHosts[sshHost] = true + data.SSHHosts = append(data.SSHHosts, sshHost) + } + } + data.GitRefs = append(data.GitRefs, gitRefData{ URL: ref.URL, Ref: gitRef, @@ -1728,6 +1759,13 @@ func prepareSkillsInitData( }) } + slices.SortFunc(data.SSHHosts, func(a, b sshHostData) int { + if cmp := strings.Compare(a.Host, b.Host); cmp != 0 { + return cmp + } + return strings.Compare(a.Port, b.Port) + }) + return data, nil } diff --git a/go/core/internal/controller/translator/agent/git_skills_test.go b/go/core/internal/controller/translator/agent/git_skills_test.go index a9205f87a..5081903ec 100644 --- a/go/core/internal/controller/translator/agent/git_skills_test.go +++ b/go/core/internal/controller/translator/agent/git_skills_test.go @@ -2,7 +2,6 @@ package agent_test import ( "context" - "fmt" "testing" "github.com/stretchr/testify/assert" @@ -45,14 +44,15 @@ func Test_AdkApiTranslator_Skills(t *testing.T) { name string agent *v1alpha2.Agent // assertions - wantSkillsInit bool - wantSkillsVolume bool - wantContainsBranch string - wantContainsCommit string - wantContainsPath string - wantContainsKrane bool - wantAuthVolume bool - wantSSHKeyscanHosts []string // substrings expected in the ssh-keyscan lines + wantSkillsInit bool + wantSkillsVolume bool + wantContainsBranch string + wantContainsCommit string + wantContainsPath string + wantContainsKrane bool + wantAuthVolume bool + wantAuthSecretName string + wantScriptContains []string }{ { name: "no skills - no init containers", @@ -213,14 +213,18 @@ func Test_AdkApiTranslator_Skills(t *testing.T) { }, }, }, - wantSkillsInit: true, - wantSkillsVolume: true, - wantAuthVolume: true, + wantSkillsInit: true, + wantSkillsVolume: true, + wantAuthVolume: true, + wantAuthSecretName: "github-token", + wantScriptContains: []string{ + "credential.helper", + }, }, { name: "git skills with SSH URL and auth secret scans custom host", agent: &v1alpha2.Agent{ - ObjectMeta: metav1.ObjectMeta{Name: "agent-ssh", Namespace: namespace}, + ObjectMeta: metav1.ObjectMeta{Name: "agent-ssh-auth", Namespace: namespace}, Spec: v1alpha2.AgentSpec{ Type: v1alpha2.AgentType_Declarative, Declarative: &v1alpha2.DeclarativeAgentSpec{ @@ -233,17 +237,22 @@ func Test_AdkApiTranslator_Skills(t *testing.T) { }, GitRefs: []v1alpha2.GitRepo{ { - URL: "ssh://git@gitea-ssh.gitea:22/gitops/ssh-skills-repo.git", - Ref: "main", + URL: "ssh://git@gitea-ssh.gitea:22/gitops/ssh-skills-repo.git", + Ref: "main", + Name: "ssh-skill", }, }, }, }, }, - wantSkillsInit: true, - wantSkillsVolume: true, - wantAuthVolume: true, - wantSSHKeyscanHosts: []string{"gitea-ssh.gitea"}, + wantSkillsInit: true, + wantSkillsVolume: true, + wantAuthVolume: true, + wantAuthSecretName: "gitea-ssh-credentials", + wantScriptContains: []string{ + "ssh-keyscan", + "gitea-ssh.gitea", + }, }, { name: "git skill with custom name", @@ -353,6 +362,10 @@ func Test_AdkApiTranslator_Skills(t *testing.T) { assert.Contains(t, script, "krane export") } + for _, want := range tt.wantScriptContains { + assert.Contains(t, script, want) + } + // Verify /skills volume mount exists hasSkillsMount := false for _, vm := range skillsInitContainer.VolumeMounts { @@ -388,7 +401,7 @@ func Test_AdkApiTranslator_Skills(t *testing.T) { for _, v := range deployment.Spec.Template.Spec.Volumes { if v.Secret != nil && v.Name == "git-auth" { hasAuthVolume = true - assert.Equal(t, tt.agent.Spec.Skills.GitAuthSecretRef.Name, v.Secret.SecretName, "auth volume should reference the correct secret") + assert.Equal(t, tt.wantAuthSecretName, v.Secret.SecretName, "auth volume should reference the correct secret") } } assert.True(t, hasAuthVolume, "git-auth volume should exist") @@ -402,20 +415,6 @@ func Test_AdkApiTranslator_Skills(t *testing.T) { } } assert.True(t, hasAuthMount, "skills-init container should mount auth secret") - - // Verify script contains credential helper setup - script := skillsInitContainer.Command[2] - assert.Contains(t, script, "credential.helper") - } - - // Verify custom SSH hosts are scanned - if len(tt.wantSSHKeyscanHosts) > 0 { - require.NotNil(t, skillsInitContainer) - script := skillsInitContainer.Command[2] - for _, host := range tt.wantSSHKeyscanHosts { - expected := fmt.Sprintf("ssh-keyscan %s", host) - assert.Contains(t, script, expected, "script should ssh-keyscan custom host %q", host) - } } // Verify insecure flag for OCI skills diff --git a/go/core/internal/controller/translator/agent/skills-init.sh.tmpl b/go/core/internal/controller/translator/agent/skills-init.sh.tmpl index 9cac78b5f..e2b67ddd6 100644 --- a/go/core/internal/controller/translator/agent/skills-init.sh.tmpl +++ b/go/core/internal/controller/translator/agent/skills-init.sh.tmpl @@ -6,15 +6,17 @@ ENDVAL )" if [ -f "${_auth_mount}/ssh-privatekey" ]; then mkdir -p ~/.ssh + chmod 700 ~/.ssh cp "${_auth_mount}/ssh-privatekey" ~/.ssh/id_rsa chmod 600 ~/.ssh/id_rsa - ssh-keyscan github.com gitlab.com bitbucket.org >> ~/.ssh/known_hosts + touch ~/.ssh/known_hosts + chmod 600 ~/.ssh/known_hosts {{- range .SSHHosts }} - {{- if .Port }} - ssh-keyscan -p {{ .Port }} {{ .Host }} >> ~/.ssh/known_hosts - {{- else }} - ssh-keyscan {{ .Host }} >> ~/.ssh/known_hosts - {{- end }} +{{- if .Port }} + ssh-keyscan -H -p "{{ .Port }}" "{{ .Host }}" >> ~/.ssh/known_hosts 2>/dev/null || echo "WARNING: ssh-keyscan failed for {{ .Host }}:{{ .Port }}" >&2 +{{- else }} + ssh-keyscan -H "{{ .Host }}" >> ~/.ssh/known_hosts 2>/dev/null || echo "WARNING: ssh-keyscan failed for {{ .Host }}" >&2 +{{- end }} {{- end }} elif [ -f "${_auth_mount}/token" ]; then git config --global credential.helper "!f() { echo username=x-access-token; echo password=\$(cat ${_auth_mount}/token); }; f" diff --git a/go/core/internal/controller/translator/agent/skills_unit_test.go b/go/core/internal/controller/translator/agent/skills_unit_test.go index b4fefd3fa..a45b97a59 100644 --- a/go/core/internal/controller/translator/agent/skills_unit_test.go +++ b/go/core/internal/controller/translator/agent/skills_unit_test.go @@ -85,6 +85,84 @@ func Test_gitSkillName(t *testing.T) { } } +func Test_gitSSHHost(t *testing.T) { + tests := []struct { + name string + rawURL string + want sshHostData + wantOK bool + }{ + { + name: "https repo is not ssh", + rawURL: "https://github.com/org/repo.git", + wantOK: false, + }, + { + name: "scp-style ssh repo", + rawURL: "git@github.com:org/repo.git", + want: sshHostData{Host: "github.com"}, + wantOK: true, + }, + { + name: "ssh url with non-default port", + rawURL: "ssh://git@gitea-ssh.gitea:2222/gitops/repo.git", + want: sshHostData{Host: "gitea-ssh.gitea", Port: "2222"}, + wantOK: true, + }, + { + name: "ssh url without explicit port", + rawURL: "ssh://git@gitea-ssh.gitea/gitops/repo.git", + want: sshHostData{Host: "gitea-ssh.gitea"}, + wantOK: true, + }, + { + name: "git+ssh url with port", + rawURL: "git+ssh://git@example.com:2222/org/repo.git", + want: sshHostData{Host: "example.com", Port: "2222"}, + wantOK: true, + }, + { + name: "ssh url with default port 22 normalizes to empty", + rawURL: "ssh://git@gitea-ssh.gitea:22/gitops/repo.git", + want: sshHostData{Host: "gitea-ssh.gitea"}, + wantOK: true, + }, + { + name: "invalid ssh-like string", + rawURL: "not-a-git-url", + wantOK: false, + }, + { + name: "scp-style with shell injection in host is rejected", + rawURL: "git@foo$(id):repo.git", + wantOK: false, + }, + { + name: "scp-style with semicolon injection in host is rejected", + rawURL: `git@bad";id;echo ":repo.git`, + wantOK: false, + }, + { + name: "ssh url with shell injection in host is rejected", + rawURL: "ssh://git@host$(whoami)/org/repo.git", + wantOK: false, + }, + { + name: "ssh url with backtick injection in host is rejected", + rawURL: "ssh://git@`id`.evil.com/org/repo.git", + wantOK: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, ok := gitSSHHost(tt.rawURL) + assert.Equal(t, tt.wantOK, ok) + assert.Equal(t, tt.want, got) + }) + } +} + func Test_validateSubPath(t *testing.T) { tests := []struct { name string @@ -210,3 +288,52 @@ func Test_prepareSkillsInitData_authMountPath(t *testing.T) { require.NoError(t, err) assert.Equal(t, "/git-auth", data.AuthMountPath) } + +func Test_prepareSkillsInitData_sshHosts(t *testing.T) { + data, err := prepareSkillsInitData( + []v1alpha2.GitRepo{ + {URL: "https://github.com/org/https-repo", Ref: "main"}, + {URL: "git@github.com:org/scp-repo.git", Ref: "main"}, + {URL: "ssh://git@gitea-ssh.gitea:22/gitops/ssh-repo.git", Ref: "main", Name: "ssh-repo"}, + {URL: "ssh://git@gitea-ssh.gitea:22/gitops/another-ssh-repo.git", Ref: "main", Name: "another-ssh-repo"}, + }, + &corev1.LocalObjectReference{Name: "ssh-secret"}, + nil, + false, + ) + require.NoError(t, err) + assert.Equal(t, []sshHostData{ + {Host: "gitea-ssh.gitea"}, + {Host: "github.com"}, + }, data.SSHHosts) +} + +func Test_prepareSkillsInitData_sshHostsDedupesDefaultPort(t *testing.T) { + data, err := prepareSkillsInitData( + []v1alpha2.GitRepo{ + {URL: "git@github.com:org/scp-repo.git", Ref: "main"}, + {URL: "ssh://git@github.com:22/org/ssh-repo.git", Ref: "main", Name: "ssh-repo"}, + }, + &corev1.LocalObjectReference{Name: "ssh-secret"}, + nil, + false, + ) + require.NoError(t, err) + assert.Equal(t, []sshHostData{ + {Host: "github.com"}, + }, data.SSHHosts) +} + +func Test_prepareSkillsInitData_noAuthSkipsSSHHosts(t *testing.T) { + data, err := prepareSkillsInitData( + []v1alpha2.GitRepo{ + {URL: "git@github.com:org/scp-repo.git", Ref: "main"}, + {URL: "ssh://git@gitea-ssh.gitea/gitops/ssh-repo.git", Ref: "main", Name: "ssh-repo"}, + }, + nil, // no auth secret + nil, + false, + ) + require.NoError(t, err) + assert.Empty(t, data.SSHHosts, "SSH hosts should not be collected when authSecretRef is nil") +} diff --git a/go/core/internal/controller/translator/agent/testdata/outputs/agent_with_git_skills.json b/go/core/internal/controller/translator/agent/testdata/outputs/agent_with_git_skills.json index 435c5c2b7..e67edb1f3 100644 --- a/go/core/internal/controller/translator/agent/testdata/outputs/agent_with_git_skills.json +++ b/go/core/internal/controller/translator/agent/testdata/outputs/agent_with_git_skills.json @@ -235,7 +235,7 @@ "command": [ "/bin/sh", "-c", - "set -e\n_auth_mount=\"$(cat \u003c\u003c'ENDVAL'\n/git-auth\nENDVAL\n)\"\nif [ -f \"${_auth_mount}/ssh-privatekey\" ]; then\n mkdir -p ~/.ssh\n cp \"${_auth_mount}/ssh-privatekey\" ~/.ssh/id_rsa\n chmod 600 ~/.ssh/id_rsa\n ssh-keyscan github.com gitlab.com bitbucket.org \u003e\u003e ~/.ssh/known_hosts\nelif [ -f \"${_auth_mount}/token\" ]; then\n git config --global credential.helper \"!f() { echo username=x-access-token; echo password=\\$(cat ${_auth_mount}/token); }; f\"\nfi\n_url=\"$(cat \u003c\u003c'ENDVAL'\nhttps://github.com/org/my-skills\nENDVAL\n)\"\n_ref=\"$(cat \u003c\u003c'ENDVAL'\nv2.0.0\nENDVAL\n)\"\n_dest=\"$(cat \u003c\u003c'ENDVAL'\n/skills/k8s-skill\nENDVAL\n)\"\necho \"Cloning ${_url} (ref ${_ref}) into ${_dest}\"\ngit clone --depth 1 --branch \"$_ref\" -- \"$_url\" \"$_dest\"\n_subpath=\"$(cat \u003c\u003c'ENDVAL'\nskills/k8s\nENDVAL\n)\"\n_tmp=\"$(mktemp -d)\"\ncp -a \"${_dest}/${_subpath}/.\" \"$_tmp/\"\nrm -rf \"$_dest\"\nmv \"$_tmp\" \"$_dest\"\n_url=\"$(cat \u003c\u003c'ENDVAL'\nhttps://github.com/org/another-skill\nENDVAL\n)\"\n_ref=\"$(cat \u003c\u003c'ENDVAL'\nabc123def456abc123def456abc123def456abc1\nENDVAL\n)\"\n_dest=\"$(cat \u003c\u003c'ENDVAL'\n/skills/another-skill\nENDVAL\n)\"\necho \"Cloning ${_url} (commit ${_ref}) into ${_dest}\"\ngit clone -- \"$_url\" \"$_dest\"\ncd \"$_dest\" \u0026\u0026 git checkout \"$_ref\"\n_url=\"$(cat \u003c\u003c'ENDVAL'\nhttps://github.com/org/private-skill\nENDVAL\n)\"\n_ref=\"$(cat \u003c\u003c'ENDVAL'\nmain\nENDVAL\n)\"\n_dest=\"$(cat \u003c\u003c'ENDVAL'\n/skills/private-skill\nENDVAL\n)\"\necho \"Cloning ${_url} (ref ${_ref}) into ${_dest}\"\ngit clone --depth 1 --branch \"$_ref\" -- \"$_url\" \"$_dest\"\n_image=\"$(cat \u003c\u003c'ENDVAL'\nghcr.io/org/oci-skill:v1.0\nENDVAL\n)\"\n_dest=\"$(cat \u003c\u003c'ENDVAL'\n/skills/oci-skill\nENDVAL\n)\"\necho \"Exporting OCI image ${_image} into ${_dest}\"\n_uname=\"$(uname -m)\"\ncase \"$_uname\" in\n x86_64|amd64)\n _arch=\"amd64\"\n ;;\n aarch64|arm64)\n _arch=\"arm64\"\n ;;\n *)\n echo \"Unsupported architecture for OCI export: ${_uname}\" \u003e\u00262\n exit 1\n ;;\nesac\nkrane export --platform \"linux/${_arch}\" \"$_image\" '/tmp/oci-skill.tar'\nmkdir -p \"$_dest\"\ntar xf '/tmp/oci-skill.tar' -C \"$_dest\"\nrm -f '/tmp/oci-skill.tar'\n" + "set -e\n_auth_mount=\"$(cat \u003c\u003c'ENDVAL'\n/git-auth\nENDVAL\n)\"\nif [ -f \"${_auth_mount}/ssh-privatekey\" ]; then\n mkdir -p ~/.ssh\n chmod 700 ~/.ssh\n cp \"${_auth_mount}/ssh-privatekey\" ~/.ssh/id_rsa\n chmod 600 ~/.ssh/id_rsa\n touch ~/.ssh/known_hosts\n chmod 600 ~/.ssh/known_hosts\nelif [ -f \"${_auth_mount}/token\" ]; then\n git config --global credential.helper \"!f() { echo username=x-access-token; echo password=\\$(cat ${_auth_mount}/token); }; f\"\nfi\n_url=\"$(cat \u003c\u003c'ENDVAL'\nhttps://github.com/org/my-skills\nENDVAL\n)\"\n_ref=\"$(cat \u003c\u003c'ENDVAL'\nv2.0.0\nENDVAL\n)\"\n_dest=\"$(cat \u003c\u003c'ENDVAL'\n/skills/k8s-skill\nENDVAL\n)\"\necho \"Cloning ${_url} (ref ${_ref}) into ${_dest}\"\ngit clone --depth 1 --branch \"$_ref\" -- \"$_url\" \"$_dest\"\n_subpath=\"$(cat \u003c\u003c'ENDVAL'\nskills/k8s\nENDVAL\n)\"\n_tmp=\"$(mktemp -d)\"\ncp -a \"${_dest}/${_subpath}/.\" \"$_tmp/\"\nrm -rf \"$_dest\"\nmv \"$_tmp\" \"$_dest\"\n_url=\"$(cat \u003c\u003c'ENDVAL'\nhttps://github.com/org/another-skill\nENDVAL\n)\"\n_ref=\"$(cat \u003c\u003c'ENDVAL'\nabc123def456abc123def456abc123def456abc1\nENDVAL\n)\"\n_dest=\"$(cat \u003c\u003c'ENDVAL'\n/skills/another-skill\nENDVAL\n)\"\necho \"Cloning ${_url} (commit ${_ref}) into ${_dest}\"\ngit clone -- \"$_url\" \"$_dest\"\ncd \"$_dest\" \u0026\u0026 git checkout \"$_ref\"\n_url=\"$(cat \u003c\u003c'ENDVAL'\nhttps://github.com/org/private-skill\nENDVAL\n)\"\n_ref=\"$(cat \u003c\u003c'ENDVAL'\nmain\nENDVAL\n)\"\n_dest=\"$(cat \u003c\u003c'ENDVAL'\n/skills/private-skill\nENDVAL\n)\"\necho \"Cloning ${_url} (ref ${_ref}) into ${_dest}\"\ngit clone --depth 1 --branch \"$_ref\" -- \"$_url\" \"$_dest\"\n_image=\"$(cat \u003c\u003c'ENDVAL'\nghcr.io/org/oci-skill:v1.0\nENDVAL\n)\"\n_dest=\"$(cat \u003c\u003c'ENDVAL'\n/skills/oci-skill\nENDVAL\n)\"\necho \"Exporting OCI image ${_image} into ${_dest}\"\n_uname=\"$(uname -m)\"\ncase \"$_uname\" in\n x86_64|amd64)\n _arch=\"amd64\"\n ;;\n aarch64|arm64)\n _arch=\"arm64\"\n ;;\n *)\n echo \"Unsupported architecture for OCI export: ${_uname}\" \u003e\u00262\n exit 1\n ;;\nesac\nkrane export --platform \"linux/${_arch}\" \"$_image\" '/tmp/oci-skill.tar'\nmkdir -p \"$_dest\"\ntar xf '/tmp/oci-skill.tar' -C \"$_dest\"\nrm -f '/tmp/oci-skill.tar'\n" ], "image": "cr.kagent.dev/kagent-dev/kagent/skills-init:dev", "name": "skills-init",