Skip to content

fix: support ssh auth for git-based skills #1529

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.

Sign up for GitHub

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

Open
jsonmp-k8 wants to merge 1 commit into
base: main
Choose a base branch
from
+234 −68
Open

fix: support ssh auth for git-based skills #1529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 docker/skills-init/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
90 changes: 64 additions & 26 deletions go/core/internal/controller/translator/agent/adk_api_translator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 == "" {
Expand Down Expand Up @@ -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, "/")
Expand All @@ -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,
Expand All @@ -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
}

Expand Down
67 changes: 33 additions & 34 deletions go/core/internal/controller/translator/agent/git_skills_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package agent_test

import (
"context"
"fmt"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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{
Expand All @@ -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",
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
Expand All @@ -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
Expand Down
14 changes: 8 additions & 6 deletions go/core/internal/controller/translator/agent/skills-init.sh.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Loading
Loading