Skip to content

Commit 6cfe66f

Browse files
authored
Fix repo-root-relative import path resolution in ResolveIncludePath (#24501)
1 parent 8dd7053 commit 6cfe66f

File tree

3 files changed

+450
-22
lines changed

3 files changed

+450
-22
lines changed

pkg/parser/frontmatter_utils_test.go

Lines changed: 373 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,30 @@ func TestResolveIncludePath(t *testing.T) {
126126
err = os.WriteFile(regularFile, []byte("test"), 0644)
127127
require.NoError(t, err, "should write regular file")
128128

129+
// Create a repo-like structure: <repoRoot>/.github/workflows/, .github/agents/ and .agents/
130+
repoRoot := filepath.Join(tempDir, "repo")
131+
workflowsDir := filepath.Join(repoRoot, ".github", "workflows")
132+
agentsDir := filepath.Join(repoRoot, ".github", "agents")
133+
dotAgentsDir := filepath.Join(repoRoot, ".agents")
134+
err = os.MkdirAll(workflowsDir, 0755)
135+
require.NoError(t, err, "should create workflows dir")
136+
err = os.MkdirAll(agentsDir, 0755)
137+
require.NoError(t, err, "should create agents dir")
138+
err = os.MkdirAll(dotAgentsDir, 0755)
139+
require.NoError(t, err, "should create .agents dir")
140+
141+
workflowFile := filepath.Join(workflowsDir, "workflow.md")
142+
err = os.WriteFile(workflowFile, []byte("test"), 0644)
143+
require.NoError(t, err, "should write workflow file")
144+
145+
agentFile := filepath.Join(agentsDir, "planner.md")
146+
err = os.WriteFile(agentFile, []byte("test"), 0644)
147+
require.NoError(t, err, "should write agent file")
148+
149+
dotAgentFile := filepath.Join(dotAgentsDir, "agent.md")
150+
err = os.WriteFile(dotAgentFile, []byte("test"), 0644)
151+
require.NoError(t, err, "should write .agents file")
152+
129153
tests := []struct {
130154
name string
131155
filePath string
@@ -151,6 +175,355 @@ func TestResolveIncludePath(t *testing.T) {
151175
baseDir: tempDir,
152176
wantErr: true,
153177
},
178+
{
179+
name: "dotgithub-prefixed path resolves from repo root",
180+
filePath: ".github/agents/planner.md",
181+
baseDir: workflowsDir,
182+
expected: agentFile,
183+
},
184+
{
185+
name: "slash-dotgithub-prefixed path resolves from repo root",
186+
filePath: "/.github/agents/planner.md",
187+
baseDir: workflowsDir,
188+
expected: agentFile,
189+
},
190+
{
191+
name: "slash-dotagents-prefixed path resolves from repo root",
192+
filePath: "/.agents/agent.md",
193+
baseDir: workflowsDir,
194+
expected: dotAgentFile,
195+
},
196+
{
197+
name: "slash-prefixed path outside .github or .agents is rejected",
198+
filePath: "/agents/agent.md",
199+
baseDir: workflowsDir,
200+
wantErr: true,
201+
},
202+
{
203+
name: "relative path in workflows dir still works unchanged",
204+
filePath: "workflow.md",
205+
baseDir: workflowsDir,
206+
expected: workflowFile,
207+
},
208+
{
209+
name: "dotgithub-prefixed path that escapes repo root is rejected",
210+
filePath: ".github/../../../etc/passwd",
211+
baseDir: workflowsDir,
212+
wantErr: true,
213+
},
214+
{
215+
name: "slash-prefixed path that escapes repo root is rejected",
216+
filePath: "/../../../etc/passwd",
217+
baseDir: workflowsDir,
218+
wantErr: true,
219+
},
220+
}
221+
222+
for _, tt := range tests {
223+
t.Run(tt.name, func(t *testing.T) {
224+
result, err := ResolveIncludePath(tt.filePath, tt.baseDir, nil)
225+
226+
if tt.wantErr {
227+
assert.Error(t, err, "ResolveIncludePath(%q, %q) should return error", tt.filePath, tt.baseDir)
228+
return
229+
}
230+
231+
require.NoError(t, err, "ResolveIncludePath(%q, %q) should not error", tt.filePath, tt.baseDir)
232+
assert.Equal(t, tt.expected, result, "ResolveIncludePath(%q, %q) result", tt.filePath, tt.baseDir)
233+
})
234+
}
235+
}
236+
237+
// TestResolveIncludePath_DotGithubRepo tests import path resolution when the repository
238+
// itself is named ".github" (e.g. an org's `org/.github` repository). In that case the
239+
// on-disk layout is <parent>/.github/.github/workflows/ and the traversal logic must
240+
// correctly treat the inner ".github" directory as the special folder and
241+
// <parent>/.github/ as the repository root.
242+
func TestResolveIncludePath_DotGithubRepo(t *testing.T) {
243+
tempDir, err := os.MkdirTemp("", "test_resolve_dotgithub_repo")
244+
require.NoError(t, err, "should create temp dir")
245+
defer os.RemoveAll(tempDir)
246+
247+
// Simulate a repo whose name is ".github": the repo root is <tempDir>/.github
248+
// and the GitHub Actions folder lives at <tempDir>/.github/.github/workflows/.
249+
dotGithubRepoRoot := filepath.Join(tempDir, ".github")
250+
workflowsDir := filepath.Join(dotGithubRepoRoot, ".github", "workflows")
251+
agentsDir := filepath.Join(dotGithubRepoRoot, ".github", "agents")
252+
dotAgentsDir := filepath.Join(dotGithubRepoRoot, ".agents")
253+
254+
for _, dir := range []string{workflowsDir, agentsDir, dotAgentsDir} {
255+
require.NoError(t, os.MkdirAll(dir, 0755), "should create dir %s", dir)
256+
}
257+
258+
workflowFile := filepath.Join(workflowsDir, "workflow.md")
259+
agentFile := filepath.Join(agentsDir, "planner.md")
260+
dotAgentFile := filepath.Join(dotAgentsDir, "agent.md")
261+
262+
require.NoError(t, os.WriteFile(workflowFile, []byte("workflow"), 0644), "should write workflow file")
263+
require.NoError(t, os.WriteFile(agentFile, []byte("planner"), 0644), "should write agent file")
264+
require.NoError(t, os.WriteFile(dotAgentFile, []byte("dot-agents"), 0644), "should write .agents file")
265+
266+
tests := []struct {
267+
name string
268+
filePath string
269+
baseDir string
270+
expected string
271+
wantErr bool
272+
}{
273+
{
274+
name: "relative path still resolves within workflows dir",
275+
filePath: "workflow.md",
276+
baseDir: workflowsDir,
277+
expected: workflowFile,
278+
},
279+
{
280+
name: "dotgithub-prefixed path resolves from repo root inside .github repo",
281+
filePath: ".github/agents/planner.md",
282+
baseDir: workflowsDir,
283+
expected: agentFile,
284+
},
285+
{
286+
name: "slash-dotgithub-prefixed path resolves from repo root inside .github repo",
287+
filePath: "/.github/agents/planner.md",
288+
baseDir: workflowsDir,
289+
expected: agentFile,
290+
},
291+
{
292+
name: "slash-dotagents-prefixed path resolves from repo root inside .github repo",
293+
filePath: "/.agents/agent.md",
294+
baseDir: workflowsDir,
295+
expected: dotAgentFile,
296+
},
297+
{
298+
name: "slash-prefixed path outside .github or .agents is rejected inside .github repo",
299+
filePath: "/agents/agent.md",
300+
baseDir: workflowsDir,
301+
wantErr: true,
302+
},
303+
{
304+
name: "dotgithub-prefixed traversal is rejected inside .github repo",
305+
filePath: ".github/../../../etc/passwd",
306+
baseDir: workflowsDir,
307+
wantErr: true,
308+
},
309+
{
310+
name: "slash-prefixed traversal is rejected inside .github repo",
311+
filePath: "/../../../etc/passwd",
312+
baseDir: workflowsDir,
313+
wantErr: true,
314+
},
315+
}
316+
317+
for _, tt := range tests {
318+
t.Run(tt.name, func(t *testing.T) {
319+
result, err := ResolveIncludePath(tt.filePath, tt.baseDir, nil)
320+
321+
if tt.wantErr {
322+
assert.Error(t, err, "ResolveIncludePath(%q, %q) should return error", tt.filePath, tt.baseDir)
323+
return
324+
}
325+
326+
require.NoError(t, err, "ResolveIncludePath(%q, %q) should not error", tt.filePath, tt.baseDir)
327+
assert.Equal(t, tt.expected, result, "ResolveIncludePath(%q, %q) result", tt.filePath, tt.baseDir)
328+
})
329+
}
330+
}
331+
332+
// TestResolveIncludePath_AllPathStyles exercises every path style that
333+
// ResolveIncludePath must handle:
334+
//
335+
// - Explicit current-dir-relative ("./file.md")
336+
// - Standard relative ("file.md", "subdir/file.md")
337+
// - .github/-prefixed repo-root (".github/agents/planner.md")
338+
// - /.github/-prefixed repo-root ("/.github/agents/planner.md")
339+
// - /.agents/-prefixed repo-root ("/.agents/agent.md")
340+
// - Multi-level nested paths (".github/agents/sub/nested.md", "/.agents/sub/nested.md")
341+
// - Intra-.github traversal (".github/agents/../workflows/workflow.md")
342+
// - Traversal that escapes scope (".github/../../../etc/passwd")
343+
// - / prefix outside .github/.agents (rejected)
344+
// - baseDir without .github parent (plain relative fallback)
345+
// - Windows-style backslash paths (normalized via filepath.ToSlash on Windows)
346+
func TestResolveIncludePath_AllPathStyles(t *testing.T) {
347+
tempDir, err := os.MkdirTemp("", "test_all_path_styles")
348+
require.NoError(t, err, "should create temp dir")
349+
defer os.RemoveAll(tempDir)
350+
351+
// Build a full repo layout:
352+
// <tempDir>/repo/
353+
// .github/
354+
// workflows/
355+
// workflow.md
356+
// sub/
357+
// nested.md
358+
// agents/
359+
// planner.md
360+
// sub/
361+
// nested.md
362+
// .agents/
363+
// agent.md
364+
// sub/
365+
// nested.md
366+
repoRoot := filepath.Join(tempDir, "repo")
367+
workflowsDir := filepath.Join(repoRoot, ".github", "workflows")
368+
workflowsSubDir := filepath.Join(workflowsDir, "sub")
369+
agentsDir := filepath.Join(repoRoot, ".github", "agents")
370+
agentsSubDir := filepath.Join(agentsDir, "sub")
371+
dotAgentsDir := filepath.Join(repoRoot, ".agents")
372+
dotAgentsSubDir := filepath.Join(dotAgentsDir, "sub")
373+
374+
for _, dir := range []string{workflowsSubDir, agentsSubDir, dotAgentsSubDir} {
375+
require.NoError(t, os.MkdirAll(dir, 0755), "should create dir %s", dir)
376+
}
377+
378+
workflowFile := filepath.Join(workflowsDir, "workflow.md")
379+
workflowNestedFile := filepath.Join(workflowsSubDir, "nested.md")
380+
agentFile := filepath.Join(agentsDir, "planner.md")
381+
agentNestedFile := filepath.Join(agentsSubDir, "nested.md")
382+
dotAgentFile := filepath.Join(dotAgentsDir, "agent.md")
383+
dotAgentNestedFile := filepath.Join(dotAgentsSubDir, "nested.md")
384+
385+
for _, f := range []string{workflowFile, workflowNestedFile, agentFile, agentNestedFile, dotAgentFile, dotAgentNestedFile} {
386+
require.NoError(t, os.WriteFile(f, []byte("test"), 0644), "should write %s", f)
387+
}
388+
389+
// A directory outside any .github tree for the "no ancestor" tests.
390+
noGithubDir := filepath.Join(tempDir, "standalone")
391+
require.NoError(t, os.MkdirAll(noGithubDir, 0755), "should create standalone dir")
392+
standaloneFile := filepath.Join(noGithubDir, "helper.md")
393+
require.NoError(t, os.WriteFile(standaloneFile, []byte("test"), 0644), "should write standalone file")
394+
395+
tests := []struct {
396+
name string
397+
filePath string
398+
baseDir string
399+
expected string
400+
wantErr bool
401+
}{
402+
// ── Standard relative paths ─────────────────────────────────────────────
403+
{
404+
name: "bare filename relative to baseDir",
405+
filePath: "workflow.md",
406+
baseDir: workflowsDir,
407+
expected: workflowFile,
408+
},
409+
{
410+
name: "explicit dot-slash prefix",
411+
filePath: "./workflow.md",
412+
baseDir: workflowsDir,
413+
expected: workflowFile,
414+
},
415+
{
416+
name: "relative subdir path",
417+
filePath: "sub/nested.md",
418+
baseDir: workflowsDir,
419+
expected: workflowNestedFile,
420+
},
421+
422+
// ── .github/-prefixed repo-root paths ───────────────────────────────────
423+
{
424+
name: ".github/agents/planner.md resolves from repo root",
425+
filePath: ".github/agents/planner.md",
426+
baseDir: workflowsDir,
427+
expected: agentFile,
428+
},
429+
{
430+
name: ".github/agents/sub/nested.md resolves from repo root",
431+
filePath: ".github/agents/sub/nested.md",
432+
baseDir: workflowsDir,
433+
expected: agentNestedFile,
434+
},
435+
{
436+
name: ".github/workflows/workflow.md accessible via .github prefix",
437+
filePath: ".github/workflows/workflow.md",
438+
baseDir: workflowsDir,
439+
expected: workflowFile,
440+
},
441+
{
442+
name: "intra-.github traversal stays within scope",
443+
filePath: ".github/agents/../workflows/workflow.md",
444+
baseDir: workflowsDir,
445+
expected: workflowFile,
446+
},
447+
448+
// ── /.github/-prefixed repo-root paths ──────────────────────────────────
449+
{
450+
name: "/.github/agents/planner.md resolves from repo root",
451+
filePath: "/.github/agents/planner.md",
452+
baseDir: workflowsDir,
453+
expected: agentFile,
454+
},
455+
{
456+
name: "/.github/agents/sub/nested.md resolves from repo root",
457+
filePath: "/.github/agents/sub/nested.md",
458+
baseDir: workflowsDir,
459+
expected: agentNestedFile,
460+
},
461+
{
462+
name: "/.github/workflows/workflow.md accessible via slash prefix",
463+
filePath: "/.github/workflows/workflow.md",
464+
baseDir: workflowsDir,
465+
expected: workflowFile,
466+
},
467+
468+
// ── /.agents/-prefixed repo-root paths ──────────────────────────────────
469+
{
470+
name: "/.agents/agent.md resolves from repo root",
471+
filePath: "/.agents/agent.md",
472+
baseDir: workflowsDir,
473+
expected: dotAgentFile,
474+
},
475+
{
476+
name: "/.agents/sub/nested.md resolves from repo root",
477+
filePath: "/.agents/sub/nested.md",
478+
baseDir: workflowsDir,
479+
expected: dotAgentNestedFile,
480+
},
481+
482+
// ── Security: traversal attempts ────────────────────────────────────────
483+
{
484+
name: ".github prefix that escapes repo root is rejected",
485+
filePath: ".github/../../../etc/passwd",
486+
baseDir: workflowsDir,
487+
wantErr: true,
488+
},
489+
{
490+
name: "/.github prefix that escapes repo root is rejected",
491+
filePath: "/.github/../../../etc/passwd",
492+
baseDir: workflowsDir,
493+
wantErr: true,
494+
},
495+
{
496+
name: "/.agents prefix that escapes scope is rejected",
497+
filePath: "/.agents/../../../etc/passwd",
498+
baseDir: workflowsDir,
499+
wantErr: true,
500+
},
501+
{
502+
name: "slash prefix to disallowed top-level directory is rejected",
503+
filePath: "/src/main.go",
504+
baseDir: workflowsDir,
505+
wantErr: true,
506+
},
507+
{
508+
name: "slash prefix to /etc/passwd is rejected",
509+
filePath: "/etc/passwd",
510+
baseDir: workflowsDir,
511+
wantErr: true,
512+
},
513+
514+
// ── baseDir without a .github ancestor (plain relative fallback) ─────────
515+
{
516+
name: "relative path from non-.github baseDir resolves locally",
517+
filePath: "helper.md",
518+
baseDir: noGithubDir,
519+
expected: standaloneFile,
520+
},
521+
{
522+
name: "missing file from non-.github baseDir returns error",
523+
filePath: "missing.md",
524+
baseDir: noGithubDir,
525+
wantErr: true,
526+
},
154527
}
155528

156529
for _, tt := range tests {

0 commit comments

Comments
 (0)