Skip to content

Commit 5975945

Browse files
authored
Ignore symlinked pull request templates (#164)
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.
1 parent 514f5ab commit 5975945

4 files changed

Lines changed: 168 additions & 25 deletions

File tree

cmd/link_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1515,6 +1515,60 @@ func TestLink_BranchNames_UsesPRTemplate(t *testing.T) {
15151515
assert.NotContains(t, capturedBody, "GitHub Stacks CLI", "footer should not be present when template is used")
15161516
}
15171517

1518+
// TestLink_IgnoresSymlinkedPRTemplate verifies that `gh stack link`, when it
1519+
// creates missing PRs, does not follow a symlinked PR template.
1520+
func TestLink_IgnoresSymlinkedPRTemplate(t *testing.T) {
1521+
tmpDir := t.TempDir()
1522+
1523+
// The repo's PR template is a symlink to a file outside the repository;
1524+
// gh-stack must not follow it.
1525+
linked := filepath.Join(t.TempDir(), "linked.txt")
1526+
require.NoError(t, os.WriteFile(linked, []byte("LINKED_FILE_CONTENTS"), 0o600))
1527+
1528+
ghDir := filepath.Join(tmpDir, ".github")
1529+
require.NoError(t, os.MkdirAll(ghDir, 0o755))
1530+
if err := os.Symlink(linked, filepath.Join(ghDir, "pull_request_template.md")); err != nil {
1531+
t.Skipf("symlinks not supported on this platform: %v", err)
1532+
}
1533+
1534+
mock := newLinkGitMock("feat-a", "feat-b")
1535+
mock.RootDirFn = func() (string, error) { return tmpDir, nil }
1536+
restore := git.SetOps(mock)
1537+
defer restore()
1538+
1539+
var capturedBody string
1540+
cfg, _, errR := config.NewTestConfig()
1541+
cfg.GitHubClientOverride = &github.MockClient{
1542+
FindPRForBranchFn: func(string) (*github.PullRequest, error) {
1543+
return nil, nil // No existing PRs
1544+
},
1545+
CreatePRFn: func(base, head, title, body string, draft bool) (*github.PullRequest, error) {
1546+
capturedBody = body
1547+
return &github.PullRequest{
1548+
Number: 1, HeadRefName: head, BaseRefName: base,
1549+
URL: "https://github.com/o/r/pull/1",
1550+
}, nil
1551+
},
1552+
ListStacksFn: func() ([]github.RemoteStack, error) {
1553+
return []github.RemoteStack{}, nil
1554+
},
1555+
CreateStackFn: func([]int) (int, error) { return 42, nil },
1556+
}
1557+
1558+
cmd := LinkCmd(cfg)
1559+
cmd.SetArgs([]string{"feat-a", "feat-b"})
1560+
cmd.SetOut(io.Discard)
1561+
cmd.SetErr(io.Discard)
1562+
err := cmd.Execute()
1563+
1564+
cfg.Err.Close()
1565+
_, _ = io.ReadAll(errR)
1566+
1567+
assert.NoError(t, err)
1568+
assert.NotContains(t, capturedBody, "LINKED_FILE_CONTENTS", "symlinked template contents must not be included in the PR body")
1569+
assert.Contains(t, capturedBody, "GitHub Stacks CLI", "template ignored, footer fallback should be used")
1570+
}
1571+
15181572
func TestLink_PRNumbers_NoTemplateUsesFooter(t *testing.T) {
15191573
// When using PR numbers (no local repo context), no template is found
15201574
// and the footer should be present for newly created PRs.

cmd/submit_test.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2184,6 +2184,65 @@ func TestSubmit_UsesPRTemplate(t *testing.T) {
21842184
assert.NotContains(t, capturedBody, feedbackURL)
21852185
}
21862186

2187+
// TestSubmit_IgnoresSymlinkedPRTemplate verifies that `gh stack submit --auto`
2188+
// does not follow a symlinked PR template. The non-interactive and
2189+
// interactive-prefill flows share the same pr.FindTemplate chokepoint, so they
2190+
// are covered transitively.
2191+
func TestSubmit_IgnoresSymlinkedPRTemplate(t *testing.T) {
2192+
s := stack.Stack{
2193+
Trunk: stack.BranchRef{Branch: "main"},
2194+
Branches: []stack.BranchRef{
2195+
{Branch: "b1"},
2196+
},
2197+
}
2198+
2199+
tmpDir := t.TempDir()
2200+
writeStackFile(t, tmpDir, s)
2201+
2202+
// The repo's PR template is a symlink to a file outside the repository;
2203+
// gh-stack must not follow it.
2204+
linked := filepath.Join(t.TempDir(), "linked.txt")
2205+
require.NoError(t, os.WriteFile(linked, []byte("LINKED_FILE_CONTENTS"), 0o600))
2206+
2207+
ghDir := filepath.Join(tmpDir, ".github")
2208+
require.NoError(t, os.MkdirAll(ghDir, 0o755))
2209+
if err := os.Symlink(linked, filepath.Join(ghDir, "pull_request_template.md")); err != nil {
2210+
t.Skipf("symlinks not supported on this platform: %v", err)
2211+
}
2212+
2213+
var capturedBody string
2214+
2215+
mock := newSubmitMock(tmpDir, "b1")
2216+
mock.PushFn = func(string, []string, bool, bool) error { return nil }
2217+
mock.LogRangeFn = func(base, head string) ([]git.CommitInfo, error) {
2218+
return []git.CommitInfo{{Subject: "add feature", Body: "detailed commit body"}}, nil
2219+
}
2220+
restore := git.SetOps(mock)
2221+
defer restore()
2222+
2223+
cfg, _, _ := config.NewTestConfig()
2224+
cfg.GitHubClientOverride = &github.MockClient{
2225+
ListStacksFn: func() ([]github.RemoteStack, error) { return nil, nil },
2226+
FindPRForBranchFn: func(string) (*github.PullRequest, error) { return nil, nil },
2227+
CreatePRFn: func(base, head, title, body string, draft bool) (*github.PullRequest, error) {
2228+
capturedBody = body
2229+
return &github.PullRequest{Number: 1, ID: "PR_1", URL: "https://github.com/o/r/pull/1"}, nil
2230+
},
2231+
CreateStackFn: func([]int) (int, error) { return 1, nil },
2232+
}
2233+
2234+
cmd := SubmitCmd(cfg)
2235+
cmd.SetArgs([]string{"--auto"})
2236+
cmd.SetOut(io.Discard)
2237+
cmd.SetErr(io.Discard)
2238+
err := cmd.Execute()
2239+
2240+
assert.NoError(t, err)
2241+
assert.NotContains(t, capturedBody, "LINKED_FILE_CONTENTS", "symlinked template contents must not be included in the PR body")
2242+
// The template was ignored, so the standard footer fallback is used.
2243+
assert.Contains(t, capturedBody, "GitHub Stacks CLI")
2244+
}
2245+
21872246
func TestSubmit_NoTemplate_UsesFooter(t *testing.T) {
21882247
s := stack.Stack{
21892248
Trunk: stack.BranchRef{Branch: "main"},

internal/pr/template.go

Lines changed: 8 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,18 @@
11
package pr
22

33
import (
4-
"os"
5-
"path/filepath"
64
"strings"
7-
)
85

9-
// templatePaths lists the candidate locations for a pull request template.
10-
var templatePaths = []string{
11-
".github/pull_request_template.md",
12-
".github/PULL_REQUEST_TEMPLATE.md",
13-
"pull_request_template.md",
14-
"PULL_REQUEST_TEMPLATE.md",
15-
"docs/pull_request_template.md",
16-
"docs/PULL_REQUEST_TEMPLATE.md",
17-
}
6+
"github.com/cli/cli/v2/pkg/githubtemplate"
7+
)
188

199
// FindTemplate searches the repository root for a default pull request
20-
// template and returns its content. Returns an empty string if no template
21-
// is found or cannot be read.
10+
// template and returns its content with any YAML front-matter stripped.
11+
// It returns an empty string if no template is found.
2212
func FindTemplate(repoRoot string) string {
23-
for _, candidate := range templatePaths {
24-
path := filepath.Join(repoRoot, candidate)
25-
data, err := os.ReadFile(path)
26-
if err != nil {
27-
continue
28-
}
29-
content := strings.TrimSpace(string(data))
30-
if content != "" {
31-
return content
32-
}
13+
path := githubtemplate.FindLegacy(repoRoot, "pull_request_template")
14+
if path == "" {
15+
return ""
3316
}
34-
return ""
17+
return strings.TrimSpace(string(githubtemplate.ExtractContents(path)))
3518
}

internal/pr/template_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"testing"
77

88
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
910
)
1011

1112
// writeTemplate is a test helper that creates a file with the given content,
@@ -76,3 +77,49 @@ func TestFindTemplate_UpperCase(t *testing.T) {
7677
got := FindTemplate(root)
7778
assert.Equal(t, "UPPER template", got)
7879
}
80+
81+
// TestFindTemplate_IgnoresSymlink verifies that a symlinked PR template is not
82+
// followed: FindTemplate ignores it rather than reading through to the file the
83+
// symlink points at.
84+
func TestFindTemplate_IgnoresSymlink(t *testing.T) {
85+
root := t.TempDir()
86+
87+
// A file outside the repository that the symlinked template points at.
88+
linked := filepath.Join(t.TempDir(), "linked.txt")
89+
require.NoError(t, os.WriteFile(linked, []byte("LINKED_FILE_CONTENTS"), 0o600))
90+
91+
ghDir := filepath.Join(root, ".github")
92+
require.NoError(t, os.MkdirAll(ghDir, 0o755))
93+
link := filepath.Join(ghDir, "pull_request_template.md")
94+
if err := os.Symlink(linked, link); err != nil {
95+
t.Skipf("symlinks not supported on this platform: %v", err)
96+
}
97+
98+
got := FindTemplate(root)
99+
assert.Empty(t, got, "symlinked PR template must be ignored")
100+
assert.NotContains(t, got, "LINKED_FILE_CONTENTS", "symlink target contents must not be read")
101+
}
102+
103+
// TestFindTemplate_StripsFrontMatter documents that discovery now delegates to
104+
// cli/cli's githubtemplate package, which strips leading YAML front-matter from
105+
// the template (matching `gh pr create`).
106+
func TestFindTemplate_StripsFrontMatter(t *testing.T) {
107+
root := t.TempDir()
108+
writeTemplate(t, filepath.Join(root, ".github", "pull_request_template.md"),
109+
[]byte("---\nname: PR\nabout: test\n---\n\n## Description\n\nBody text."))
110+
111+
got := FindTemplate(root)
112+
assert.Equal(t, "## Description\n\nBody text.", got)
113+
assert.NotContains(t, got, "name: PR", "YAML front-matter should be stripped")
114+
}
115+
116+
// TestFindTemplate_HyphenatedName documents the broadened filename matching that
117+
// comes with reusing githubtemplate.FindLegacy (hyphenated variants are now
118+
// recognized, closer to GitHub's own acceptance).
119+
func TestFindTemplate_HyphenatedName(t *testing.T) {
120+
root := t.TempDir()
121+
writeTemplate(t, filepath.Join(root, ".github", "pull-request-template.md"), []byte("Hyphenated template"))
122+
123+
got := FindTemplate(root)
124+
assert.Equal(t, "Hyphenated template", got)
125+
}

0 commit comments

Comments
 (0)