Allow github:ORG// repos to redirect to other repos in the same org

hack/bats/tests/url-github.bats

@@ -6,19 +6,33 @@ load "../helpers/load"
 # The jandubois/jandubois GitHub repo has been especially constructed to test
 # various features of the github URL scheme:
 #
-# * repo defaults to org when not specified
-# * filename defaults to .lima.yaml when only a path is specified
-# * .yaml default extension
-# * .lima.yaml files may be treated as symlinks
-# * default branch lookup when not specified
+#   * repo defaults to org when not specified
+#   * filename defaults to .lima.yaml when only a path is specified
+#   * .yaml default extension
+#   * .lima.yaml files may be treated as symlinks
+#   * default branch lookup when not specified
+#   * github:ORG// repos can redirect to another github:ORG URL in the same ORG
 #
-# The repo files are:
+# The jandubois/jandubois repo files are:
 #
-# ├── .lima.yaml -> templates/demo.yaml
-# ├── docs
-# │   └── .lima.yaml -> ../templates/demo.yaml
-# └── templates
-#     └── demo.yaml
+#   ├── .lima.yaml         -> templates/demo.yaml
+#   ├── back
+#   │   └── .lima.yaml     -> github:jandubois//loop/
+#   ├── docs
+#   │   └── .lima.yaml     -> ../templates/demo.yaml
+#   ├── invalid
+#   │   ├── org
+#   │   │   └── .lima.yaml -> github:lima-vm
+#   │   └── tag
+#   │       └── .lima.yaml -> github:jandubois//@v0.0.0
+#   ├── loop
+#   │   └── .lima.yaml     -> github:jandubois//back/
+#   ├── redirect
+#   │   └── .lima.yaml     -> github:jandubois/lima/templates/default
+#   ├── templates
+#   │   └── demo.yaml      "base: template:default"
+#   └── yaml
+#       └── .lima.yaml     "{}"
 #
 # Both the `main` branch and the `v0.0.0` tag have this layout.
 
@@ -52,7 +66,7 @@ URLS=(
 )
 
 url() {
-	run_e "$1" limactl template url "$2"
+	run_e "$1" limactl --debug template url "$2"
 }
 
 test_jandubois_url() {
@@ -71,9 +85,14 @@ for url in "${URLS[@]}"; do
 	bats_test_function --description "$url" -- test_jandubois_url "$url"
 done
 
-@test '.lima.yaml is retained when it is not a symlink' {
-	url -0 'github:jandubois//test/'
-	assert_output 'https://raw.githubusercontent.com/jandubois/jandubois/main/test/.lima.yaml'
+@test '.lima.yaml is retained when it exits and is not a symlink' {
+	url -0 'github:jandubois//yaml/'
+	assert_output 'https://raw.githubusercontent.com/jandubois/jandubois/main/yaml/.lima.yaml'
+}
+
+@test 'non-existing .lima.yaml returns an error' {
+	url -1 'github:jandubois//missing/'
+	assert_fatal 'file "https://raw.githubusercontent.com/jandubois/jandubois/main/missing/.lima.yaml" not found or inaccessible: status 404'
 }
 
 @test 'hidden files without an extension get a .yaml extension' {
@@ -88,15 +107,62 @@ done
 
 @test 'github: URLs are EXPERIMENTAL' {
 	url -0 'github:jandubois'
-	assert_stderr --regexp 'warning.+GitHub locator .* replaced with .* EXPERIMENTAL'
+	assert_warning "The github: scheme is still EXPERIMENTAL"
 }
 
-@test 'Empty github: url returns an error' {
+# Invalid URLs
+@test 'empty github: url returns an error' {
 	url -1 'github:'
 	assert_fatal 'github: URL must contain at least an ORG, got ""'
 }
 
-@test 'Missing org returns an error' {
+@test 'missing org returns an error' {
 	url -1 'github:/jandubois'
 	assert_fatal 'github: URL must contain at least an ORG, got ""'
 }
+
+# github: redirects in github:ORG// repos
+@test 'org redirects can point to different repo and may switch the branch name' {
+	url -0 'github:jandubois//redirect/'
+	# Note that the default branch in jandubois/jandubois is main, but in jandubois/lima it is master
+	assert_debug 'Locator "github:jandubois//redirect/" replaced with "github:jandubois/lima/templates/default"'
+	assert_debug 'Locator "github:jandubois/lima/templates/default" replaced with "https://raw.githubusercontent.com/jandubois/lima/master/templates/default.yaml"'
+	assert_output 'https://raw.githubusercontent.com/jandubois/lima/master/templates/default.yaml'
+}
+
+@test 'org redirects propagate an explicit branch/tag to the other repo' {
+	url -0 'github:jandubois//redirect/@v1.2.1'
+	assert_debug 'Locator "github:jandubois//redirect/@v1.2.1" replaced with "github:jandubois/lima/templates/[email protected]"'
+	assert_debug 'Locator "github:jandubois/lima/templates/[email protected]" replaced with "https://raw.githubusercontent.com/jandubois/lima/v1.2.1/templates/default.yaml"'
+	assert_output 'https://raw.githubusercontent.com/jandubois/lima/v1.2.1/templates/default.yaml'
+}
+
+@test 'org redirects cannot point to another org' {
+	url -1 'github:jandubois//invalid/org/'
+	assert_fatal 'redirect "github:lima-vm" is not a "github:jandubois" URL…'
+}
+
+@test 'org redirects with branch cannot point to another org' {
+	url -1 'github:jandubois//invalid/org/@main'
+	assert_fatal 'redirect "github:lima-vm" is not a "github:jandubois" URL…'
+}
+
+@test 'org redirects cannot include a branch or tag' {
+	url -1 'github:jandubois//invalid/tag/'
+	assert_fatal 'redirect "github:jandubois//@v0.0.0" must not include a branch/tag/sha…'
+}
+
+@test 'org redirects with tag cannot include a branch or tag' {
+	url -1 'github:jandubois//invalid/tag/@v0.0.0'
+	assert_fatal 'redirect "github:jandubois//@v0.0.0" must not include a branch/tag/sha…'
+}
+
+@test 'org redirects must not create circular redirects' {
+	url -1 'github:jandubois//loop/'
+	assert_fatal 'custom locator "github:jandubois//loop/" has a redirect loop'
+}
+
+@test 'org redirects with branch must not create circular redirects' {
+	url -1 'github:jandubois//back/@main'
+	assert_fatal 'custom locator "github:jandubois//back/@main" has a redirect loop'
+}

pkg/limatmpl/github.go

@@ -16,6 +16,8 @@ import (
 	"strings"
 )
 
+const defaultFilename = ".lima.yaml"
+
 // transformGitHubURL transforms a github: URL to a raw.githubusercontent.com URL.
 // Input format: ORG/REPO[/PATH][@BRANCH]
 //
@@ -25,43 +27,37 @@ import (
 // If PATH is just a directory (trailing slash), it will be set to .lima.yaml
 // IF FILE is .lima.yaml and contents looks like a symlink, it will be replaced by the symlink target.
 func transformGitHubURL(ctx context.Context, input string) (string, error) {
-	// Check for explicit branch specification with @ at the end
-	var branch string
-	if idx := strings.LastIndex(input, "@"); idx != -1 {
-		branch = input[idx+1:]
-		input = input[:idx]
-	}
+	input, origBranch, _ := strings.Cut(input, "@")
 
 	parts := strings.Split(input, "/")
 	for len(parts) < 2 {
 		parts = append(parts, "")
 	}
-
 	org := parts[0]
 	if org == "" {
 		return "", fmt.Errorf("github: URL must contain at least an ORG, got %q", input)
 	}
-
 	// If REPO is omitted (github:ORG or github:ORG//PATH), default it to ORG
 	repo := cmp.Or(parts[1], org)
-	pathPart := strings.Join(parts[2:], "/")
+	filePath := strings.Join(parts[2:], "/")
 
-	if pathPart == "" {
-		pathPart = ".lima.yaml"
+	if filePath == "" {
+		filePath = defaultFilename
 	} else {
-		// If path ends with /, it's a directory, so append .lima
-		if strings.HasSuffix(pathPart, "/") {
-			pathPart += ".lima"
+		// If path ends with / then it's a directory, so append .lima
+		if strings.HasSuffix(filePath, "/") {
+			filePath += defaultFilename
 		}
 
 		// If the filename (excluding first char for hidden files) has no extension, add .yaml
-		filename := path.Base(pathPart)
+		filename := path.Base(filePath)
 		if !strings.Contains(filename[1:], ".") {
-			pathPart += ".yaml"
+			filePath += ".yaml"
 		}
 	}
 
 	// Query default branch if no branch was specified
+	branch := origBranch
 	if branch == "" {
 		var err error
 		branch, err = getGitHubDefaultBranch(ctx, org, repo)
@@ -71,13 +67,24 @@ func transformGitHubURL(ctx context.Context, input string) (string, error) {
 	}
 
 	// If filename is .lima.yaml, check if it's a symlink/redirect to another file
-	if path.Base(pathPart) == ".lima.yaml" {
-		if redirectPath, err := resolveGitHubSymlink(ctx, org, repo, branch, pathPart); err == nil {
-			pathPart = redirectPath
-		}
+	if path.Base(filePath) == defaultFilename {
+		return resolveGitHubSymlink(ctx, org, repo, branch, filePath, origBranch)
 	}
+	return githubUserContentURL(org, repo, branch, filePath), nil
+}
 
-	return fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/%s", org, repo, branch, pathPart), nil
+func githubUserContentURL(org, repo, branch, filePath string) string {
+	return fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/%s", org, repo, branch, filePath)
+}
+
+func getGitHubUserContent(ctx context.Context, org, repo, branch, filePath string) (*http.Response, error) {
+	url := githubUserContentURL(org, repo, branch, filePath)
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, http.NoBody)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create request: %w", err)
+	}
+	req.Header.Set("User-Agent", "lima")
+	return http.DefaultClient.Do(req)
 }
 
 // getGitHubDefaultBranch queries the GitHub API to get the default branch for a repository.
@@ -108,61 +115,96 @@ func getGitHubDefaultBranch(ctx context.Context, org, repo string) (string, erro
 	if err != nil {
 		return "", fmt.Errorf("failed to read GitHub API response: %w", err)
 	}
-
 	if resp.StatusCode != http.StatusOK {
 		return "", fmt.Errorf("GitHub API returned status %d: %s", resp.StatusCode, string(body))
 	}
 
 	var repoData struct {
 		DefaultBranch string `json:"default_branch"`
 	}
-
 	if err := json.Unmarshal(body, &repoData); err != nil {
 		return "", fmt.Errorf("failed to parse GitHub API response: %w", err)
 	}
-
 	if repoData.DefaultBranch == "" {
 		return "", fmt.Errorf("repository %s/%s has no default branch", org, repo)
 	}
-
 	return repoData.DefaultBranch, nil
 }
 
 // resolveGitHubSymlink checks if a file at the given path is a symlink/redirect to another file.
-// If the file contains a single line without YAML content, it's treated as a path to the actual file.
-// Returns the redirect path if found, or the original path otherwise.
-func resolveGitHubSymlink(ctx context.Context, org, repo, branch, filePath string) (string, error) {
-	url := fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/%s", org, repo, branch, filePath)
-
-	req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, http.NoBody)
-	if err != nil {
-		return "", fmt.Errorf("failed to create request: %w", err)
-	}
-
-	req.Header.Set("User-Agent", "lima")
-
-	resp, err := http.DefaultClient.Do(req)
+// If the file contains a single line without newline, space, or colon then it's treated as a path to the actual file.
+// Returns a URL to the redirect path if found, or a URL for original path otherwise.
+func resolveGitHubSymlink(ctx context.Context, org, repo, branch, filePath, origBranch string) (string, error) {
+	resp, err := getGitHubUserContent(ctx, org, repo, branch, filePath)
 	if err != nil {
 		return "", fmt.Errorf("failed to fetch file: %w", err)
 	}
 	defer resp.Body.Close()
 
+	// Special rule for branch/tag propagation for github:ORG// requests.
+	if resp.StatusCode == http.StatusNotFound && repo == org {
+		defaultBranch, err := getGitHubDefaultBranch(ctx, org, repo)
+		if err == nil {
+			return resolveGitHubRedirect(ctx, org, repo, defaultBranch, filePath, branch)
+		}
+	}
 	if resp.StatusCode != http.StatusOK {
-		return "", fmt.Errorf("file not found or inaccessible: status %d", resp.StatusCode)
+		return "", fmt.Errorf("file %q not found or inaccessible: status %d", resp.Request.URL, resp.StatusCode)
 	}
 
 	// Read first 1KB to check the file content
 	buf := make([]byte, 1024)
 	n, err := resp.Body.Read(buf)
 	if err != nil && !errors.Is(err, io.EOF) {
-		return "", fmt.Errorf("failed to read file content: %w", err)
+		return "", fmt.Errorf("failed to read %q content: %w", resp.Request.URL, err)
 	}
 	content := string(buf[:n])
 
+	// Symlink can also be a github: redirect if we are in a github:ORG// repo.
+	if repo == org && strings.HasPrefix(content, "github:") {
+		return validateGitHubRedirect(content, org, origBranch, resp.Request.URL.String())
+	}
+
 	// A symlink must be a single line (without trailing newline), no spaces, no colons
 	if !(content == "" || strings.ContainsAny(content, "\n :")) {
 		// symlink is relative to the directory of filePath
-		return path.Join(path.Dir(filePath), content), nil
+		filePath = path.Join(path.Dir(filePath), content)
+	}
+	return githubUserContentURL(org, repo, branch, filePath), nil
+}
+
+// resolveGitHubRedirect checks if a file at the given path is a github: URL to another file within the same repo.
+// Returns the URL, or an error if the file doesn't exist, or doesn't start with github:ORG.
+func resolveGitHubRedirect(ctx context.Context, org, repo, defaultBranch, filePath, origBranch string) (string, error) {
+	// Refetch the filepath from the defaultBranch
+	resp, err := getGitHubUserContent(ctx, org, repo, defaultBranch, filePath)
+	if err != nil {
+		return "", fmt.Errorf("failed to fetch file: %w", err)
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		return "", fmt.Errorf("file %q not found or inaccessible: status %d", resp.Request.URL, resp.StatusCode)
+	}
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return "", fmt.Errorf("failed to read %q content: %w", resp.Request.URL, err)
+	}
+	return validateGitHubRedirect(string(body), org, origBranch, resp.Request.URL.String())
+}
+
+func validateGitHubRedirect(body, org, origBranch, url string) (string, error) {
+	redirect, _, _ := strings.Cut(body, "\n")
+	redirect = strings.TrimSpace(redirect)
+
+	if !strings.HasPrefix(redirect, "github:"+org+"/") {
+		return "", fmt.Errorf(`redirect %q is not a "github:%s" URL (from %q)`, redirect, org, url)
+	}
+	if strings.ContainsRune(redirect, '@') {
+		return "", fmt.Errorf("redirect %q must not include a branch/tag/sha (from %q)", redirect, url)
+	}
+	// If the origBranch is empty, then we need to look up the default branch in the redirect
+	if origBranch != "" {
+		redirect += "@" + origBranch
 	}
-	return filePath, nil
+	return redirect, nil
 }