From 231523d9680df3c680a33cbeeaa3deb221ce2649 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Wed, 8 Jul 2026 19:12:39 -0400 Subject: [PATCH] Ignore symlinked pull request templates Template discovery in internal/pr read candidate template paths with os.ReadFile, which follows symlinks. A pull request template that is a symlink (for example .github/pull_request_template.md pointing to a file outside the repository) would therefore be read through to its target, and that target's contents would be used as the PR body by `gh stack submit` and `gh stack link`. Reuse cli/cli's githubtemplate package (already a dependency) for template discovery instead of the hand-rolled path list. It is the same code `gh pr create` uses, and it ignores symlinked templates, so only regular template files inside the repository are read. FindTemplate keeps the same signature, so the submit and link callers are unchanged. Two behavior changes come with the switch: - YAML front-matter is stripped from the template, matching `gh pr create`. - Template filename matching is slightly broader; hyphenated and non-.md variants are now recognized. Add tests for FindTemplate and for the `gh stack submit --auto` and `gh stack link` PR-creation flows to confirm symlinked templates are not followed. --- cmd/link_test.go | 54 +++++++++++++++++++++++++++++++++ cmd/submit_test.go | 59 ++++++++++++++++++++++++++++++++++++ internal/pr/template.go | 33 +++++--------------- internal/pr/template_test.go | 47 ++++++++++++++++++++++++++++ 4 files changed, 168 insertions(+), 25 deletions(-) diff --git a/cmd/link_test.go b/cmd/link_test.go index b9adf84..e740b74 100644 --- a/cmd/link_test.go +++ b/cmd/link_test.go @@ -1515,6 +1515,60 @@ func TestLink_BranchNames_UsesPRTemplate(t *testing.T) { assert.NotContains(t, capturedBody, "GitHub Stacks CLI", "footer should not be present when template is used") } +// TestLink_IgnoresSymlinkedPRTemplate verifies that `gh stack link`, when it +// creates missing PRs, does not follow a symlinked PR template. +func TestLink_IgnoresSymlinkedPRTemplate(t *testing.T) { + tmpDir := t.TempDir() + + // The repo's PR template is a symlink to a file outside the repository; + // gh-stack must not follow it. + linked := filepath.Join(t.TempDir(), "linked.txt") + require.NoError(t, os.WriteFile(linked, []byte("LINKED_FILE_CONTENTS"), 0o600)) + + ghDir := filepath.Join(tmpDir, ".github") + require.NoError(t, os.MkdirAll(ghDir, 0o755)) + if err := os.Symlink(linked, filepath.Join(ghDir, "pull_request_template.md")); err != nil { + t.Skipf("symlinks not supported on this platform: %v", err) + } + + mock := newLinkGitMock("feat-a", "feat-b") + mock.RootDirFn = func() (string, error) { return tmpDir, nil } + restore := git.SetOps(mock) + defer restore() + + var capturedBody string + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRForBranchFn: func(string) (*github.PullRequest, error) { + return nil, nil // No existing PRs + }, + CreatePRFn: func(base, head, title, body string, draft bool) (*github.PullRequest, error) { + capturedBody = body + return &github.PullRequest{ + Number: 1, HeadRefName: head, BaseRefName: base, + URL: "https://github.com/o/r/pull/1", + }, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{}, nil + }, + CreateStackFn: func([]int) (int, error) { return 42, nil }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"feat-a", "feat-b"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + _, _ = io.ReadAll(errR) + + assert.NoError(t, err) + assert.NotContains(t, capturedBody, "LINKED_FILE_CONTENTS", "symlinked template contents must not be included in the PR body") + assert.Contains(t, capturedBody, "GitHub Stacks CLI", "template ignored, footer fallback should be used") +} + func TestLink_PRNumbers_NoTemplateUsesFooter(t *testing.T) { // When using PR numbers (no local repo context), no template is found // and the footer should be present for newly created PRs. diff --git a/cmd/submit_test.go b/cmd/submit_test.go index 67058b3..75c93b5 100644 --- a/cmd/submit_test.go +++ b/cmd/submit_test.go @@ -2184,6 +2184,65 @@ func TestSubmit_UsesPRTemplate(t *testing.T) { assert.NotContains(t, capturedBody, feedbackURL) } +// TestSubmit_IgnoresSymlinkedPRTemplate verifies that `gh stack submit --auto` +// does not follow a symlinked PR template. The non-interactive and +// interactive-prefill flows share the same pr.FindTemplate chokepoint, so they +// are covered transitively. +func TestSubmit_IgnoresSymlinkedPRTemplate(t *testing.T) { + s := stack.Stack{ + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{ + {Branch: "b1"}, + }, + } + + tmpDir := t.TempDir() + writeStackFile(t, tmpDir, s) + + // The repo's PR template is a symlink to a file outside the repository; + // gh-stack must not follow it. + linked := filepath.Join(t.TempDir(), "linked.txt") + require.NoError(t, os.WriteFile(linked, []byte("LINKED_FILE_CONTENTS"), 0o600)) + + ghDir := filepath.Join(tmpDir, ".github") + require.NoError(t, os.MkdirAll(ghDir, 0o755)) + if err := os.Symlink(linked, filepath.Join(ghDir, "pull_request_template.md")); err != nil { + t.Skipf("symlinks not supported on this platform: %v", err) + } + + var capturedBody string + + mock := newSubmitMock(tmpDir, "b1") + mock.PushFn = func(string, []string, bool, bool) error { return nil } + mock.LogRangeFn = func(base, head string) ([]git.CommitInfo, error) { + return []git.CommitInfo{{Subject: "add feature", Body: "detailed commit body"}}, nil + } + restore := git.SetOps(mock) + defer restore() + + cfg, _, _ := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + ListStacksFn: func() ([]github.RemoteStack, error) { return nil, nil }, + FindPRForBranchFn: func(string) (*github.PullRequest, error) { return nil, nil }, + CreatePRFn: func(base, head, title, body string, draft bool) (*github.PullRequest, error) { + capturedBody = body + return &github.PullRequest{Number: 1, ID: "PR_1", URL: "https://github.com/o/r/pull/1"}, nil + }, + CreateStackFn: func([]int) (int, error) { return 1, nil }, + } + + cmd := SubmitCmd(cfg) + cmd.SetArgs([]string{"--auto"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + assert.NoError(t, err) + assert.NotContains(t, capturedBody, "LINKED_FILE_CONTENTS", "symlinked template contents must not be included in the PR body") + // The template was ignored, so the standard footer fallback is used. + assert.Contains(t, capturedBody, "GitHub Stacks CLI") +} + func TestSubmit_NoTemplate_UsesFooter(t *testing.T) { s := stack.Stack{ Trunk: stack.BranchRef{Branch: "main"}, diff --git a/internal/pr/template.go b/internal/pr/template.go index d9ccfae..6c08f51 100644 --- a/internal/pr/template.go +++ b/internal/pr/template.go @@ -1,35 +1,18 @@ package pr import ( - "os" - "path/filepath" "strings" -) -// templatePaths lists the candidate locations for a pull request template. -var templatePaths = []string{ - ".github/pull_request_template.md", - ".github/PULL_REQUEST_TEMPLATE.md", - "pull_request_template.md", - "PULL_REQUEST_TEMPLATE.md", - "docs/pull_request_template.md", - "docs/PULL_REQUEST_TEMPLATE.md", -} + "github.com/cli/cli/v2/pkg/githubtemplate" +) // FindTemplate searches the repository root for a default pull request -// template and returns its content. Returns an empty string if no template -// is found or cannot be read. +// template and returns its content with any YAML front-matter stripped. +// It returns an empty string if no template is found. func FindTemplate(repoRoot string) string { - for _, candidate := range templatePaths { - path := filepath.Join(repoRoot, candidate) - data, err := os.ReadFile(path) - if err != nil { - continue - } - content := strings.TrimSpace(string(data)) - if content != "" { - return content - } + path := githubtemplate.FindLegacy(repoRoot, "pull_request_template") + if path == "" { + return "" } - return "" + return strings.TrimSpace(string(githubtemplate.ExtractContents(path))) } diff --git a/internal/pr/template_test.go b/internal/pr/template_test.go index 3f4ca7c..567d850 100644 --- a/internal/pr/template_test.go +++ b/internal/pr/template_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // writeTemplate is a test helper that creates a file with the given content, @@ -76,3 +77,49 @@ func TestFindTemplate_UpperCase(t *testing.T) { got := FindTemplate(root) assert.Equal(t, "UPPER template", got) } + +// TestFindTemplate_IgnoresSymlink verifies that a symlinked PR template is not +// followed: FindTemplate ignores it rather than reading through to the file the +// symlink points at. +func TestFindTemplate_IgnoresSymlink(t *testing.T) { + root := t.TempDir() + + // A file outside the repository that the symlinked template points at. + linked := filepath.Join(t.TempDir(), "linked.txt") + require.NoError(t, os.WriteFile(linked, []byte("LINKED_FILE_CONTENTS"), 0o600)) + + ghDir := filepath.Join(root, ".github") + require.NoError(t, os.MkdirAll(ghDir, 0o755)) + link := filepath.Join(ghDir, "pull_request_template.md") + if err := os.Symlink(linked, link); err != nil { + t.Skipf("symlinks not supported on this platform: %v", err) + } + + got := FindTemplate(root) + assert.Empty(t, got, "symlinked PR template must be ignored") + assert.NotContains(t, got, "LINKED_FILE_CONTENTS", "symlink target contents must not be read") +} + +// TestFindTemplate_StripsFrontMatter documents that discovery now delegates to +// cli/cli's githubtemplate package, which strips leading YAML front-matter from +// the template (matching `gh pr create`). +func TestFindTemplate_StripsFrontMatter(t *testing.T) { + root := t.TempDir() + writeTemplate(t, filepath.Join(root, ".github", "pull_request_template.md"), + []byte("---\nname: PR\nabout: test\n---\n\n## Description\n\nBody text.")) + + got := FindTemplate(root) + assert.Equal(t, "## Description\n\nBody text.", got) + assert.NotContains(t, got, "name: PR", "YAML front-matter should be stripped") +} + +// TestFindTemplate_HyphenatedName documents the broadened filename matching that +// comes with reusing githubtemplate.FindLegacy (hyphenated variants are now +// recognized, closer to GitHub's own acceptance). +func TestFindTemplate_HyphenatedName(t *testing.T) { + root := t.TempDir() + writeTemplate(t, filepath.Join(root, ".github", "pull-request-template.md"), []byte("Hyphenated template")) + + got := FindTemplate(root) + assert.Equal(t, "Hyphenated template", got) +}