From 4e296159423f9feffe60c1a29878c114db604621 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 4 Apr 2026 14:21:20 +0000 Subject: [PATCH 1/8] Initial plan From c292b496ab0e14b1ebd7d9cb7ea1cba0b59d165a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 4 Apr 2026 14:45:27 +0000 Subject: [PATCH 2/8] Fix repo-root-relative import path resolution in ResolveIncludePath Agent-Logs-Url: https://github.com/github/gh-aw/sessions/843b9e6e-9109-441e-bcc1-9f4671d34d54 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/parser/frontmatter_utils_test.go | 54 ++++++++++++++++++++++++++++ pkg/parser/remote_fetch.go | 35 +++++++++++++----- pkg/parser/remote_fetch_wasm.go | 21 ++++++++--- 3 files changed, 97 insertions(+), 13 deletions(-) diff --git a/pkg/parser/frontmatter_utils_test.go b/pkg/parser/frontmatter_utils_test.go index c317adbe67e..e5d9a3dbee6 100644 --- a/pkg/parser/frontmatter_utils_test.go +++ b/pkg/parser/frontmatter_utils_test.go @@ -126,6 +126,30 @@ func TestResolveIncludePath(t *testing.T) { err = os.WriteFile(regularFile, []byte("test"), 0644) require.NoError(t, err, "should write regular file") + // Create a repo-like structure: /.github/workflows/ and .github/agents/ + repoRoot := filepath.Join(tempDir, "repo") + workflowsDir := filepath.Join(repoRoot, ".github", "workflows") + agentsDir := filepath.Join(repoRoot, ".github", "agents") + rootAgentsDir := filepath.Join(repoRoot, "agents") + err = os.MkdirAll(workflowsDir, 0755) + require.NoError(t, err, "should create workflows dir") + err = os.MkdirAll(agentsDir, 0755) + require.NoError(t, err, "should create agents dir") + err = os.MkdirAll(rootAgentsDir, 0755) + require.NoError(t, err, "should create root agents dir") + + workflowFile := filepath.Join(workflowsDir, "workflow.md") + err = os.WriteFile(workflowFile, []byte("test"), 0644) + require.NoError(t, err, "should write workflow file") + + agentFile := filepath.Join(agentsDir, "planner.md") + err = os.WriteFile(agentFile, []byte("test"), 0644) + require.NoError(t, err, "should write agent file") + + rootAgentFile := filepath.Join(rootAgentsDir, "agent.md") + err = os.WriteFile(rootAgentFile, []byte("test"), 0644) + require.NoError(t, err, "should write root agent file") + tests := []struct { name string filePath string @@ -151,6 +175,36 @@ func TestResolveIncludePath(t *testing.T) { baseDir: tempDir, wantErr: true, }, + { + name: "dotgithub-prefixed path resolves from repo root", + filePath: ".github/agents/planner.md", + baseDir: workflowsDir, + expected: agentFile, + }, + { + name: "slash-prefixed path resolves from repo root", + filePath: "/agents/agent.md", + baseDir: workflowsDir, + expected: rootAgentFile, + }, + { + name: "relative path in workflows dir still works unchanged", + filePath: "workflow.md", + baseDir: workflowsDir, + expected: workflowFile, + }, + { + name: "dotgithub-prefixed path that escapes repo root is rejected", + filePath: ".github/../../../etc/passwd", + baseDir: workflowsDir, + wantErr: true, + }, + { + name: "slash-prefixed path that escapes repo root is rejected", + filePath: "/../../../etc/passwd", + baseDir: workflowsDir, + wantErr: true, + }, } for _, tt := range tests { diff --git a/pkg/parser/remote_fetch.go b/pkg/parser/remote_fetch.go index f7de2025ee6..64c786a4130 100644 --- a/pkg/parser/remote_fetch.go +++ b/pkg/parser/remote_fetch.go @@ -135,13 +135,9 @@ func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, e } remoteLog.Printf("Using local file resolution for: %s", filePath) - // Regular path, resolve relative to base directory - fullPath := filepath.Join(baseDir, filePath) - // Security check: ensure the resolved path is within the .github folder - // baseDir should be .github or a subdirectory within it - githubFolder := baseDir // Find the .github folder by traversing up from baseDir + githubFolder := baseDir for !strings.HasSuffix(githubFolder, ".github") && githubFolder != "." && githubFolder != "/" { githubFolder = filepath.Dir(githubFolder) if githubFolder == "." || githubFolder == "/" { @@ -151,14 +147,35 @@ func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, e } } + // Determine resolution base and security scope for the file path. + // Paths starting with ".github/" or "/" are repo-root-relative and are resolved + // from the repository root rather than from baseDir. + resolveBase := baseDir + securityBase := githubFolder + if strings.HasSuffix(githubFolder, ".github") { + repoRoot := filepath.Dir(githubFolder) + if strings.HasPrefix(filePath, ".github/") { + // .github/-prefixed path: resolve from repo root, security scope stays .github/ + resolveBase = repoRoot + } else if stripped, ok := strings.CutPrefix(filePath, "/"); ok { + // Repo-root-absolute path: strip leading slash, resolve from repo root + filePath = stripped + resolveBase = repoRoot + securityBase = repoRoot + } + } + + // Resolve path relative to resolveBase + fullPath := filepath.Join(resolveBase, filePath) + // Normalize paths for comparison - normalizedGithubFolder := filepath.Clean(githubFolder) + normalizedSecurityBase := filepath.Clean(securityBase) normalizedFullPath := filepath.Clean(fullPath) - // Check if fullPath is within githubFolder - relativePath, err := filepath.Rel(normalizedGithubFolder, normalizedFullPath) + // Check if fullPath is within the security scope + relativePath, err := filepath.Rel(normalizedSecurityBase, normalizedFullPath) if err != nil || relativePath == ".." || strings.HasPrefix(relativePath, ".."+string(filepath.Separator)) || filepath.IsAbs(relativePath) { - remoteLog.Printf("Security: Path escapes .github folder: %s (resolves to: %s)", filePath, relativePath) + remoteLog.Printf("Security: Path escapes allowed folder: %s (resolves to: %s)", filePath, relativePath) return "", fmt.Errorf("security: path %s must be within .github folder (resolves to: %s)", filePath, relativePath) } diff --git a/pkg/parser/remote_fetch_wasm.go b/pkg/parser/remote_fetch_wasm.go index 2561295018a..3417e48dcee 100644 --- a/pkg/parser/remote_fetch_wasm.go +++ b/pkg/parser/remote_fetch_wasm.go @@ -68,8 +68,6 @@ func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, e return "", fmt.Errorf("remote imports not available in Wasm: %s", filePath) } - fullPath := filepath.Join(baseDir, filePath) - githubFolder := baseDir for !strings.HasSuffix(githubFolder, ".github") && githubFolder != "." && githubFolder != "/" { githubFolder = filepath.Dir(githubFolder) @@ -79,10 +77,25 @@ func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, e } } - normalizedGithubFolder := filepath.Clean(githubFolder) + resolveBase := baseDir + securityBase := githubFolder + if strings.HasSuffix(githubFolder, ".github") { + repoRoot := filepath.Dir(githubFolder) + if strings.HasPrefix(filePath, ".github/") { + resolveBase = repoRoot + } else if stripped, ok := strings.CutPrefix(filePath, "/"); ok { + filePath = stripped + resolveBase = repoRoot + securityBase = repoRoot + } + } + + fullPath := filepath.Join(resolveBase, filePath) + + normalizedSecurityBase := filepath.Clean(securityBase) normalizedFullPath := filepath.Clean(fullPath) - relativePath, err := filepath.Rel(normalizedGithubFolder, normalizedFullPath) + relativePath, err := filepath.Rel(normalizedSecurityBase, normalizedFullPath) if err != nil || relativePath == ".." || strings.HasPrefix(relativePath, ".."+string(filepath.Separator)) || filepath.IsAbs(relativePath) { return "", fmt.Errorf("security: path %s must be within .github folder (resolves to: %s)", filePath, relativePath) } From c893e1a3e8e6fee9087c6a7e4f2c1e40b03b73a9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 4 Apr 2026 15:05:42 +0000 Subject: [PATCH 3/8] Add tests for ResolveIncludePath with repo named .github Agent-Logs-Url: https://github.com/github/gh-aw/sessions/37e61afa-60fa-43f7-a2f5-02b7d928cc53 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/parser/frontmatter_utils_test.go | 87 ++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/pkg/parser/frontmatter_utils_test.go b/pkg/parser/frontmatter_utils_test.go index e5d9a3dbee6..d50ae63a3f6 100644 --- a/pkg/parser/frontmatter_utils_test.go +++ b/pkg/parser/frontmatter_utils_test.go @@ -222,6 +222,93 @@ func TestResolveIncludePath(t *testing.T) { } } +// TestResolveIncludePath_DotGithubRepo tests import path resolution when the repository +// itself is named ".github" (e.g. an org's `org/.github` repository). In that case the +// on-disk layout is /.github/.github/workflows/ and the traversal logic must +// correctly treat the inner ".github" directory as the special folder and +// /.github/ as the repository root. +func TestResolveIncludePath_DotGithubRepo(t *testing.T) { + tempDir, err := os.MkdirTemp("", "test_resolve_dotgithub_repo") + require.NoError(t, err, "should create temp dir") + defer os.RemoveAll(tempDir) + + // Simulate a repo whose name is ".github": the repo root is /.github + // and the GitHub Actions folder lives at /.github/.github/workflows/. + dotGithubRepoRoot := filepath.Join(tempDir, ".github") + workflowsDir := filepath.Join(dotGithubRepoRoot, ".github", "workflows") + agentsDir := filepath.Join(dotGithubRepoRoot, ".github", "agents") + rootAgentsDir := filepath.Join(dotGithubRepoRoot, "agents") + + for _, dir := range []string{workflowsDir, agentsDir, rootAgentsDir} { + require.NoError(t, os.MkdirAll(dir, 0755), "should create dir %s", dir) + } + + workflowFile := filepath.Join(workflowsDir, "workflow.md") + agentFile := filepath.Join(agentsDir, "planner.md") + rootAgentFile := filepath.Join(rootAgentsDir, "agent.md") + + for path, content := range map[string]string{ + workflowFile: "workflow", + agentFile: "planner", + rootAgentFile: "root-agent", + } { + require.NoError(t, os.WriteFile(path, []byte(content), 0644), "should write %s", path) + } + + tests := []struct { + name string + filePath string + baseDir string + expected string + wantErr bool + }{ + { + name: "relative path still resolves within workflows dir", + filePath: "workflow.md", + baseDir: workflowsDir, + expected: workflowFile, + }, + { + name: "dotgithub-prefixed path resolves from repo root inside .github repo", + filePath: ".github/agents/planner.md", + baseDir: workflowsDir, + expected: agentFile, + }, + { + name: "slash-prefixed path resolves from repo root inside .github repo", + filePath: "/agents/agent.md", + baseDir: workflowsDir, + expected: rootAgentFile, + }, + { + name: "dotgithub-prefixed traversal is rejected inside .github repo", + filePath: ".github/../../../etc/passwd", + baseDir: workflowsDir, + wantErr: true, + }, + { + name: "slash-prefixed traversal is rejected inside .github repo", + filePath: "/../../../etc/passwd", + baseDir: workflowsDir, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := ResolveIncludePath(tt.filePath, tt.baseDir, nil) + + if tt.wantErr { + assert.Error(t, err, "ResolveIncludePath(%q, %q) should return error", tt.filePath, tt.baseDir) + return + } + + require.NoError(t, err, "ResolveIncludePath(%q, %q) should not error", tt.filePath, tt.baseDir) + assert.Equal(t, tt.expected, result, "ResolveIncludePath(%q, %q) result", tt.filePath, tt.baseDir) + }) + } +} + func TestIsWorkflowSpec(t *testing.T) { tests := []struct { name string From 2da3602a5b156b3af65f4ce704446330c4dcbe30 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 4 Apr 2026 15:06:20 +0000 Subject: [PATCH 4/8] Use sequential file creation in TestResolveIncludePath_DotGithubRepo Agent-Logs-Url: https://github.com/github/gh-aw/sessions/37e61afa-60fa-43f7-a2f5-02b7d928cc53 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/parser/frontmatter_utils_test.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/pkg/parser/frontmatter_utils_test.go b/pkg/parser/frontmatter_utils_test.go index d50ae63a3f6..7fde0de8d4a 100644 --- a/pkg/parser/frontmatter_utils_test.go +++ b/pkg/parser/frontmatter_utils_test.go @@ -247,13 +247,9 @@ func TestResolveIncludePath_DotGithubRepo(t *testing.T) { agentFile := filepath.Join(agentsDir, "planner.md") rootAgentFile := filepath.Join(rootAgentsDir, "agent.md") - for path, content := range map[string]string{ - workflowFile: "workflow", - agentFile: "planner", - rootAgentFile: "root-agent", - } { - require.NoError(t, os.WriteFile(path, []byte(content), 0644), "should write %s", path) - } + require.NoError(t, os.WriteFile(workflowFile, []byte("workflow"), 0644), "should write workflow file") + require.NoError(t, os.WriteFile(agentFile, []byte("planner"), 0644), "should write agent file") + require.NoError(t, os.WriteFile(rootAgentFile, []byte("root-agent"), 0644), "should write root agent file") tests := []struct { name string From 4e3ed13b325abc79c434fef09c21bba7849936a4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 4 Apr 2026 15:49:09 +0000 Subject: [PATCH 5/8] Restrict workspace-root imports to .github/ and .agents/ only Agent-Logs-Url: https://github.com/github/gh-aw/sessions/47a10fd4-da79-49c0-92e9-2bd1a3a5b5ce Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/parser/frontmatter_utils_test.go | 54 ++++++++++++++++++++-------- pkg/parser/remote_fetch.go | 11 ++++-- pkg/parser/remote_fetch_wasm.go | 9 ++++- 3 files changed, 56 insertions(+), 18 deletions(-) diff --git a/pkg/parser/frontmatter_utils_test.go b/pkg/parser/frontmatter_utils_test.go index 7fde0de8d4a..a4d1c25f8bb 100644 --- a/pkg/parser/frontmatter_utils_test.go +++ b/pkg/parser/frontmatter_utils_test.go @@ -126,17 +126,17 @@ func TestResolveIncludePath(t *testing.T) { err = os.WriteFile(regularFile, []byte("test"), 0644) require.NoError(t, err, "should write regular file") - // Create a repo-like structure: /.github/workflows/ and .github/agents/ + // Create a repo-like structure: /.github/workflows/, .github/agents/ and .agents/ repoRoot := filepath.Join(tempDir, "repo") workflowsDir := filepath.Join(repoRoot, ".github", "workflows") agentsDir := filepath.Join(repoRoot, ".github", "agents") - rootAgentsDir := filepath.Join(repoRoot, "agents") + dotAgentsDir := filepath.Join(repoRoot, ".agents") err = os.MkdirAll(workflowsDir, 0755) require.NoError(t, err, "should create workflows dir") err = os.MkdirAll(agentsDir, 0755) require.NoError(t, err, "should create agents dir") - err = os.MkdirAll(rootAgentsDir, 0755) - require.NoError(t, err, "should create root agents dir") + err = os.MkdirAll(dotAgentsDir, 0755) + require.NoError(t, err, "should create .agents dir") workflowFile := filepath.Join(workflowsDir, "workflow.md") err = os.WriteFile(workflowFile, []byte("test"), 0644) @@ -146,9 +146,9 @@ func TestResolveIncludePath(t *testing.T) { err = os.WriteFile(agentFile, []byte("test"), 0644) require.NoError(t, err, "should write agent file") - rootAgentFile := filepath.Join(rootAgentsDir, "agent.md") - err = os.WriteFile(rootAgentFile, []byte("test"), 0644) - require.NoError(t, err, "should write root agent file") + dotAgentFile := filepath.Join(dotAgentsDir, "agent.md") + err = os.WriteFile(dotAgentFile, []byte("test"), 0644) + require.NoError(t, err, "should write .agents file") tests := []struct { name string @@ -182,10 +182,22 @@ func TestResolveIncludePath(t *testing.T) { expected: agentFile, }, { - name: "slash-prefixed path resolves from repo root", + name: "slash-dotgithub-prefixed path resolves from repo root", + filePath: "/.github/agents/planner.md", + baseDir: workflowsDir, + expected: agentFile, + }, + { + name: "slash-dotagents-prefixed path resolves from repo root", + filePath: "/.agents/agent.md", + baseDir: workflowsDir, + expected: dotAgentFile, + }, + { + name: "slash-prefixed path outside .github or .agents is rejected", filePath: "/agents/agent.md", baseDir: workflowsDir, - expected: rootAgentFile, + wantErr: true, }, { name: "relative path in workflows dir still works unchanged", @@ -237,19 +249,19 @@ func TestResolveIncludePath_DotGithubRepo(t *testing.T) { dotGithubRepoRoot := filepath.Join(tempDir, ".github") workflowsDir := filepath.Join(dotGithubRepoRoot, ".github", "workflows") agentsDir := filepath.Join(dotGithubRepoRoot, ".github", "agents") - rootAgentsDir := filepath.Join(dotGithubRepoRoot, "agents") + dotAgentsDir := filepath.Join(dotGithubRepoRoot, ".agents") - for _, dir := range []string{workflowsDir, agentsDir, rootAgentsDir} { + for _, dir := range []string{workflowsDir, agentsDir, dotAgentsDir} { require.NoError(t, os.MkdirAll(dir, 0755), "should create dir %s", dir) } workflowFile := filepath.Join(workflowsDir, "workflow.md") agentFile := filepath.Join(agentsDir, "planner.md") - rootAgentFile := filepath.Join(rootAgentsDir, "agent.md") + dotAgentFile := filepath.Join(dotAgentsDir, "agent.md") require.NoError(t, os.WriteFile(workflowFile, []byte("workflow"), 0644), "should write workflow file") require.NoError(t, os.WriteFile(agentFile, []byte("planner"), 0644), "should write agent file") - require.NoError(t, os.WriteFile(rootAgentFile, []byte("root-agent"), 0644), "should write root agent file") + require.NoError(t, os.WriteFile(dotAgentFile, []byte("dot-agents"), 0644), "should write .agents file") tests := []struct { name string @@ -271,10 +283,22 @@ func TestResolveIncludePath_DotGithubRepo(t *testing.T) { expected: agentFile, }, { - name: "slash-prefixed path resolves from repo root inside .github repo", + name: "slash-dotgithub-prefixed path resolves from repo root inside .github repo", + filePath: "/.github/agents/planner.md", + baseDir: workflowsDir, + expected: agentFile, + }, + { + name: "slash-dotagents-prefixed path resolves from repo root inside .github repo", + filePath: "/.agents/agent.md", + baseDir: workflowsDir, + expected: dotAgentFile, + }, + { + name: "slash-prefixed path outside .github or .agents is rejected inside .github repo", filePath: "/agents/agent.md", baseDir: workflowsDir, - expected: rootAgentFile, + wantErr: true, }, { name: "dotgithub-prefixed traversal is rejected inside .github repo", diff --git a/pkg/parser/remote_fetch.go b/pkg/parser/remote_fetch.go index 64c786a4130..fa25297e05c 100644 --- a/pkg/parser/remote_fetch.go +++ b/pkg/parser/remote_fetch.go @@ -158,10 +158,17 @@ func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, e // .github/-prefixed path: resolve from repo root, security scope stays .github/ resolveBase = repoRoot } else if stripped, ok := strings.CutPrefix(filePath, "/"); ok { - // Repo-root-absolute path: strip leading slash, resolve from repo root + // Repo-root-absolute path: only .github/ and .agents/ subdirectories are accessible. + if !strings.HasPrefix(stripped, ".github/") && !strings.HasPrefix(stripped, ".agents/") { + remoteLog.Printf("Security: Path not within .github or .agents: %s", filePath) + return "", fmt.Errorf("security: path %s must be within .github or .agents folder", filePath) + } filePath = stripped resolveBase = repoRoot - securityBase = repoRoot + if strings.HasPrefix(stripped, ".agents/") { + securityBase = filepath.Join(repoRoot, ".agents") + } + // For .github/-prefixed: securityBase remains githubFolder (already set above). } } diff --git a/pkg/parser/remote_fetch_wasm.go b/pkg/parser/remote_fetch_wasm.go index 3417e48dcee..da7aeb56d86 100644 --- a/pkg/parser/remote_fetch_wasm.go +++ b/pkg/parser/remote_fetch_wasm.go @@ -84,9 +84,16 @@ func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, e if strings.HasPrefix(filePath, ".github/") { resolveBase = repoRoot } else if stripped, ok := strings.CutPrefix(filePath, "/"); ok { + // Repo-root-absolute path: only .github/ and .agents/ subdirectories are accessible. + if !strings.HasPrefix(stripped, ".github/") && !strings.HasPrefix(stripped, ".agents/") { + return "", fmt.Errorf("security: path %s must be within .github or .agents folder", filePath) + } filePath = stripped resolveBase = repoRoot - securityBase = repoRoot + if strings.HasPrefix(stripped, ".agents/") { + securityBase = filepath.Join(repoRoot, ".agents") + } + // For .github/-prefixed: securityBase remains githubFolder (already set above). } } From cd2422655cd4ac9625c964c92509aaa0489ecab3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 4 Apr 2026 15:50:28 +0000 Subject: [PATCH 6/8] Make securityBase assignment explicit in .github/-prefixed slash path branch Agent-Logs-Url: https://github.com/github/gh-aw/sessions/47a10fd4-da79-49c0-92e9-2bd1a3a5b5ce Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/parser/remote_fetch.go | 4 +++- pkg/parser/remote_fetch_wasm.go | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/parser/remote_fetch.go b/pkg/parser/remote_fetch.go index fa25297e05c..5f7ff9f56cb 100644 --- a/pkg/parser/remote_fetch.go +++ b/pkg/parser/remote_fetch.go @@ -167,8 +167,10 @@ func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, e resolveBase = repoRoot if strings.HasPrefix(stripped, ".agents/") { securityBase = filepath.Join(repoRoot, ".agents") + } else { + // .github/-prefixed: security scope is the .github folder. + securityBase = githubFolder } - // For .github/-prefixed: securityBase remains githubFolder (already set above). } } diff --git a/pkg/parser/remote_fetch_wasm.go b/pkg/parser/remote_fetch_wasm.go index da7aeb56d86..63965ea377f 100644 --- a/pkg/parser/remote_fetch_wasm.go +++ b/pkg/parser/remote_fetch_wasm.go @@ -92,8 +92,10 @@ func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, e resolveBase = repoRoot if strings.HasPrefix(stripped, ".agents/") { securityBase = filepath.Join(repoRoot, ".agents") + } else { + // .github/-prefixed: security scope is the .github folder. + securityBase = githubFolder } - // For .github/-prefixed: securityBase remains githubFolder (already set above). } } From 1367012c9ce358b2988041e1990abad693735bec Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 4 Apr 2026 16:31:45 +0000 Subject: [PATCH 7/8] Address review comments: fix loop termination, error messages, and path normalization Agent-Logs-Url: https://github.com/github/gh-aw/sessions/08030f89-48e0-4052-9704-8e8327ff2d94 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/parser/remote_fetch.go | 20 ++++++++++++-------- pkg/parser/remote_fetch_wasm.go | 17 ++++++++++------- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/pkg/parser/remote_fetch.go b/pkg/parser/remote_fetch.go index 5f7ff9f56cb..1a8e85f214c 100644 --- a/pkg/parser/remote_fetch.go +++ b/pkg/parser/remote_fetch.go @@ -138,32 +138,35 @@ func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, e // Find the .github folder by traversing up from baseDir githubFolder := baseDir - for !strings.HasSuffix(githubFolder, ".github") && githubFolder != "." && githubFolder != "/" { - githubFolder = filepath.Dir(githubFolder) - if githubFolder == "." || githubFolder == "/" { - // If we can't find .github folder, use baseDir + for !strings.HasSuffix(githubFolder, ".github") { + parent := filepath.Dir(githubFolder) + if parent == githubFolder || parent == "." || parent == "/" { + // Reached filesystem root without finding .github; fall back to baseDir githubFolder = baseDir break } + githubFolder = parent } // Determine resolution base and security scope for the file path. // Paths starting with ".github/" or "/" are repo-root-relative and are resolved // from the repository root rather than from baseDir. + // Normalize path separators for reliable prefix matching across platforms. resolveBase := baseDir securityBase := githubFolder if strings.HasSuffix(githubFolder, ".github") { repoRoot := filepath.Dir(githubFolder) - if strings.HasPrefix(filePath, ".github/") { + filePathSlash := filepath.ToSlash(filePath) + if strings.HasPrefix(filePathSlash, ".github/") { // .github/-prefixed path: resolve from repo root, security scope stays .github/ resolveBase = repoRoot - } else if stripped, ok := strings.CutPrefix(filePath, "/"); ok { + } else if stripped, ok := strings.CutPrefix(filePathSlash, "/"); ok { // Repo-root-absolute path: only .github/ and .agents/ subdirectories are accessible. if !strings.HasPrefix(stripped, ".github/") && !strings.HasPrefix(stripped, ".agents/") { remoteLog.Printf("Security: Path not within .github or .agents: %s", filePath) return "", fmt.Errorf("security: path %s must be within .github or .agents folder", filePath) } - filePath = stripped + filePath = filepath.FromSlash(stripped) resolveBase = repoRoot if strings.HasPrefix(stripped, ".agents/") { securityBase = filepath.Join(repoRoot, ".agents") @@ -184,8 +187,9 @@ func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, e // Check if fullPath is within the security scope relativePath, err := filepath.Rel(normalizedSecurityBase, normalizedFullPath) if err != nil || relativePath == ".." || strings.HasPrefix(relativePath, ".."+string(filepath.Separator)) || filepath.IsAbs(relativePath) { + allowedFolder := filepath.Base(normalizedSecurityBase) remoteLog.Printf("Security: Path escapes allowed folder: %s (resolves to: %s)", filePath, relativePath) - return "", fmt.Errorf("security: path %s must be within .github folder (resolves to: %s)", filePath, relativePath) + return "", fmt.Errorf("security: path %s must be within %s folder (resolves to: %s)", filePath, allowedFolder, relativePath) } if _, err := os.Stat(fullPath); os.IsNotExist(err) { diff --git a/pkg/parser/remote_fetch_wasm.go b/pkg/parser/remote_fetch_wasm.go index 63965ea377f..0218124851f 100644 --- a/pkg/parser/remote_fetch_wasm.go +++ b/pkg/parser/remote_fetch_wasm.go @@ -69,26 +69,28 @@ func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, e } githubFolder := baseDir - for !strings.HasSuffix(githubFolder, ".github") && githubFolder != "." && githubFolder != "/" { - githubFolder = filepath.Dir(githubFolder) - if githubFolder == "." || githubFolder == "/" { + for !strings.HasSuffix(githubFolder, ".github") { + parent := filepath.Dir(githubFolder) + if parent == githubFolder || parent == "." || parent == "/" { githubFolder = baseDir break } + githubFolder = parent } resolveBase := baseDir securityBase := githubFolder if strings.HasSuffix(githubFolder, ".github") { repoRoot := filepath.Dir(githubFolder) - if strings.HasPrefix(filePath, ".github/") { + filePathSlash := filepath.ToSlash(filePath) + if strings.HasPrefix(filePathSlash, ".github/") { resolveBase = repoRoot - } else if stripped, ok := strings.CutPrefix(filePath, "/"); ok { + } else if stripped, ok := strings.CutPrefix(filePathSlash, "/"); ok { // Repo-root-absolute path: only .github/ and .agents/ subdirectories are accessible. if !strings.HasPrefix(stripped, ".github/") && !strings.HasPrefix(stripped, ".agents/") { return "", fmt.Errorf("security: path %s must be within .github or .agents folder", filePath) } - filePath = stripped + filePath = filepath.FromSlash(stripped) resolveBase = repoRoot if strings.HasPrefix(stripped, ".agents/") { securityBase = filepath.Join(repoRoot, ".agents") @@ -106,7 +108,8 @@ func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, e relativePath, err := filepath.Rel(normalizedSecurityBase, normalizedFullPath) if err != nil || relativePath == ".." || strings.HasPrefix(relativePath, ".."+string(filepath.Separator)) || filepath.IsAbs(relativePath) { - return "", fmt.Errorf("security: path %s must be within .github folder (resolves to: %s)", filePath, relativePath) + allowedFolder := filepath.Base(normalizedSecurityBase) + return "", fmt.Errorf("security: path %s must be within %s folder (resolves to: %s)", filePath, allowedFolder, relativePath) } // In wasm builds, check the virtual filesystem first From 70433658f5ad74989f3ffd77b6e52557897dea51 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 4 Apr 2026 16:50:06 +0000 Subject: [PATCH 8/8] Add TestResolveIncludePath_AllPathStyles covering all path forms Agent-Logs-Url: https://github.com/github/gh-aw/sessions/27052e8d-ccd8-4c31-a535-dc7a096dcd3e Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/parser/frontmatter_utils_test.go | 212 +++++++++++++++++++++++++++ 1 file changed, 212 insertions(+) diff --git a/pkg/parser/frontmatter_utils_test.go b/pkg/parser/frontmatter_utils_test.go index a4d1c25f8bb..223a7d746a1 100644 --- a/pkg/parser/frontmatter_utils_test.go +++ b/pkg/parser/frontmatter_utils_test.go @@ -329,6 +329,218 @@ func TestResolveIncludePath_DotGithubRepo(t *testing.T) { } } +// TestResolveIncludePath_AllPathStyles exercises every path style that +// ResolveIncludePath must handle: +// +// - Explicit current-dir-relative ("./file.md") +// - Standard relative ("file.md", "subdir/file.md") +// - .github/-prefixed repo-root (".github/agents/planner.md") +// - /.github/-prefixed repo-root ("/.github/agents/planner.md") +// - /.agents/-prefixed repo-root ("/.agents/agent.md") +// - Multi-level nested paths (".github/agents/sub/nested.md", "/.agents/sub/nested.md") +// - Intra-.github traversal (".github/agents/../workflows/workflow.md") +// - Traversal that escapes scope (".github/../../../etc/passwd") +// - / prefix outside .github/.agents (rejected) +// - baseDir without .github parent (plain relative fallback) +// - Windows-style backslash paths (normalized via filepath.ToSlash on Windows) +func TestResolveIncludePath_AllPathStyles(t *testing.T) { + tempDir, err := os.MkdirTemp("", "test_all_path_styles") + require.NoError(t, err, "should create temp dir") + defer os.RemoveAll(tempDir) + + // Build a full repo layout: + // /repo/ + // .github/ + // workflows/ + // workflow.md + // sub/ + // nested.md + // agents/ + // planner.md + // sub/ + // nested.md + // .agents/ + // agent.md + // sub/ + // nested.md + repoRoot := filepath.Join(tempDir, "repo") + workflowsDir := filepath.Join(repoRoot, ".github", "workflows") + workflowsSubDir := filepath.Join(workflowsDir, "sub") + agentsDir := filepath.Join(repoRoot, ".github", "agents") + agentsSubDir := filepath.Join(agentsDir, "sub") + dotAgentsDir := filepath.Join(repoRoot, ".agents") + dotAgentsSubDir := filepath.Join(dotAgentsDir, "sub") + + for _, dir := range []string{workflowsSubDir, agentsSubDir, dotAgentsSubDir} { + require.NoError(t, os.MkdirAll(dir, 0755), "should create dir %s", dir) + } + + workflowFile := filepath.Join(workflowsDir, "workflow.md") + workflowNestedFile := filepath.Join(workflowsSubDir, "nested.md") + agentFile := filepath.Join(agentsDir, "planner.md") + agentNestedFile := filepath.Join(agentsSubDir, "nested.md") + dotAgentFile := filepath.Join(dotAgentsDir, "agent.md") + dotAgentNestedFile := filepath.Join(dotAgentsSubDir, "nested.md") + + for _, f := range []string{workflowFile, workflowNestedFile, agentFile, agentNestedFile, dotAgentFile, dotAgentNestedFile} { + require.NoError(t, os.WriteFile(f, []byte("test"), 0644), "should write %s", f) + } + + // A directory outside any .github tree for the "no ancestor" tests. + noGithubDir := filepath.Join(tempDir, "standalone") + require.NoError(t, os.MkdirAll(noGithubDir, 0755), "should create standalone dir") + standaloneFile := filepath.Join(noGithubDir, "helper.md") + require.NoError(t, os.WriteFile(standaloneFile, []byte("test"), 0644), "should write standalone file") + + tests := []struct { + name string + filePath string + baseDir string + expected string + wantErr bool + }{ + // ── Standard relative paths ───────────────────────────────────────────── + { + name: "bare filename relative to baseDir", + filePath: "workflow.md", + baseDir: workflowsDir, + expected: workflowFile, + }, + { + name: "explicit dot-slash prefix", + filePath: "./workflow.md", + baseDir: workflowsDir, + expected: workflowFile, + }, + { + name: "relative subdir path", + filePath: "sub/nested.md", + baseDir: workflowsDir, + expected: workflowNestedFile, + }, + + // ── .github/-prefixed repo-root paths ─────────────────────────────────── + { + name: ".github/agents/planner.md resolves from repo root", + filePath: ".github/agents/planner.md", + baseDir: workflowsDir, + expected: agentFile, + }, + { + name: ".github/agents/sub/nested.md resolves from repo root", + filePath: ".github/agents/sub/nested.md", + baseDir: workflowsDir, + expected: agentNestedFile, + }, + { + name: ".github/workflows/workflow.md accessible via .github prefix", + filePath: ".github/workflows/workflow.md", + baseDir: workflowsDir, + expected: workflowFile, + }, + { + name: "intra-.github traversal stays within scope", + filePath: ".github/agents/../workflows/workflow.md", + baseDir: workflowsDir, + expected: workflowFile, + }, + + // ── /.github/-prefixed repo-root paths ────────────────────────────────── + { + name: "/.github/agents/planner.md resolves from repo root", + filePath: "/.github/agents/planner.md", + baseDir: workflowsDir, + expected: agentFile, + }, + { + name: "/.github/agents/sub/nested.md resolves from repo root", + filePath: "/.github/agents/sub/nested.md", + baseDir: workflowsDir, + expected: agentNestedFile, + }, + { + name: "/.github/workflows/workflow.md accessible via slash prefix", + filePath: "/.github/workflows/workflow.md", + baseDir: workflowsDir, + expected: workflowFile, + }, + + // ── /.agents/-prefixed repo-root paths ────────────────────────────────── + { + name: "/.agents/agent.md resolves from repo root", + filePath: "/.agents/agent.md", + baseDir: workflowsDir, + expected: dotAgentFile, + }, + { + name: "/.agents/sub/nested.md resolves from repo root", + filePath: "/.agents/sub/nested.md", + baseDir: workflowsDir, + expected: dotAgentNestedFile, + }, + + // ── Security: traversal attempts ──────────────────────────────────────── + { + name: ".github prefix that escapes repo root is rejected", + filePath: ".github/../../../etc/passwd", + baseDir: workflowsDir, + wantErr: true, + }, + { + name: "/.github prefix that escapes repo root is rejected", + filePath: "/.github/../../../etc/passwd", + baseDir: workflowsDir, + wantErr: true, + }, + { + name: "/.agents prefix that escapes scope is rejected", + filePath: "/.agents/../../../etc/passwd", + baseDir: workflowsDir, + wantErr: true, + }, + { + name: "slash prefix to disallowed top-level directory is rejected", + filePath: "/src/main.go", + baseDir: workflowsDir, + wantErr: true, + }, + { + name: "slash prefix to /etc/passwd is rejected", + filePath: "/etc/passwd", + baseDir: workflowsDir, + wantErr: true, + }, + + // ── baseDir without a .github ancestor (plain relative fallback) ───────── + { + name: "relative path from non-.github baseDir resolves locally", + filePath: "helper.md", + baseDir: noGithubDir, + expected: standaloneFile, + }, + { + name: "missing file from non-.github baseDir returns error", + filePath: "missing.md", + baseDir: noGithubDir, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := ResolveIncludePath(tt.filePath, tt.baseDir, nil) + + if tt.wantErr { + assert.Error(t, err, "ResolveIncludePath(%q, %q) should return error", tt.filePath, tt.baseDir) + return + } + + require.NoError(t, err, "ResolveIncludePath(%q, %q) should not error", tt.filePath, tt.baseDir) + assert.Equal(t, tt.expected, result, "ResolveIncludePath(%q, %q) result", tt.filePath, tt.baseDir) + }) + } +} + func TestIsWorkflowSpec(t *testing.T) { tests := []struct { name string