Skip to content
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
15 changes: 13 additions & 2 deletions pkg/cli/download_workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,25 @@ func downloadWorkflowContentViaGit(ctx context.Context, repo, path, ref string,

downloadLog.Printf("Attempting git fallback for downloading workflow content: %s/%s@%s", repo, path, ref)

if err := gitutil.ValidateGitRef(ref); err != nil {
return nil, fmt.Errorf("refusing git fallback: %w", err)
}
if err := gitutil.ValidateGitPath(path); err != nil {
return nil, fmt.Errorf("refusing git fallback: %w", err)
}

// Use git archive to get the file content without cloning
githubHost := getGitHubHostForRepo(repo)
repoURL := fmt.Sprintf("%s/%s.git", githubHost, repo)

// git archive command: git archive --remote=<repo> <ref> <path>
// git archive command: git archive --remote=<repo> <ref> -- <path>
// The '--' end-of-options separator ensures path is never parsed as a git flag
// even if it begins with '-' (argument injection, CWE-88).
// ValidateGitRef/ValidateGitPath above guard against leading '-' and '..' at
// this layer; '--' is kept as defence-in-depth per the git(1) specification.
// #nosec G204 -- repoURL, ref, and path are from workflow import configuration authored by the
// developer; exec.CommandContext with separate args (not shell execution) prevents shell injection.
cmd := exec.CommandContext(ctx, "git", "archive", "--remote="+repoURL, ref, path)
cmd := exec.CommandContext(ctx, "git", "archive", "--remote="+repoURL, ref, "--", path)
archiveOutput, err := cmd.Output()
if err != nil {
downloadLog.Printf("git archive failed, falling back to git clone: repo=%s, ref=%s, err=%v", repo, ref, err)
Expand Down
65 changes: 65 additions & 0 deletions pkg/cli/download_workflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cli

import (
"context"
"encoding/base64"
"strings"
"testing"
Expand Down Expand Up @@ -75,3 +76,67 @@ func TestDecodeBase64FileContent(t *testing.T) {
})
}
}

// TestDownloadWorkflowContentViaGitValidation ensures that downloadWorkflowContentViaGit
// rejects dangerous ref/path values before spawning any git subprocess (CWE-88).
func TestDownloadWorkflowContentViaGitValidation(t *testing.T) {
tests := []struct {
name string
repo string
path string
ref string
errContains string
}{
{
name: "path starting with dash is rejected",
repo: "owner/repo",
path: "--output=/tmp/evil",
ref: "abc123",
errContains: "refusing git fallback",
},
{
name: "ref starting with dash is rejected",
repo: "owner/repo",
path: "workflow.md",
ref: "--upload-pack=evil",
errContains: "refusing git fallback",
},
{
name: "path with traversal is rejected",
repo: "owner/repo",
path: "../../../etc/passwd",
ref: "abc123",
errContains: "refusing git fallback",
},
{
name: "ref with dotdot is rejected",
repo: "owner/repo",
path: "workflow.md",
ref: "main..evil",
errContains: "refusing git fallback",
},
{
name: "empty ref is rejected",
repo: "owner/repo",
path: "workflow.md",
ref: "",
errContains: "refusing git fallback",
},
{
name: "empty path is rejected",
repo: "owner/repo",
path: "",
ref: "abc123",
errContains: "refusing git fallback",
},
}

ctx := context.Background()
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, err := downloadWorkflowContentViaGit(ctx, tt.repo, tt.path, tt.ref, false)
require.Error(t, err, "expected validation error for invalid input")
assert.Contains(t, err.Error(), tt.errContains)
})
}
}
Loading