Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions pkg/parser/frontmatter_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,11 @@ func TestIsRepositoryImport(t *testing.T) {
importPath: "my_org/my_repo",
want: true,
},
{
name: "owner/repo with dot is repository import",
importPath: "githubnext/gh-aw.dev",
want: true,
},
{
name: "workflowspec with three parts is not repository import",
importPath: "owner/repo/path/to/file.md",
Expand Down
12 changes: 2 additions & 10 deletions pkg/parser/import_bfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func seedSingleImportSpec(importSpec ImportSpec, baseDir string, cache *ImportCa
state.acc.repositoryImports = append(state.acc.repositoryImports, importPath)
return nil
}
filePath, sectionName := splitImportPathAndSection(importPath)
filePath, sectionName := splitPathAndSection(importPath)
fullPath, err := resolveSeedImportPath(filePath, importPath, baseDir, cache, workflowFilePath, yamlContent)
if err != nil {
return err
Expand All @@ -140,14 +140,6 @@ func seedSingleImportSpec(importSpec ImportSpec, baseDir string, cache *ImportCa
return enqueueImportPath(state, importPath, fullPath, sectionName, baseDir, importSpec.Inputs, origin)
}

func splitImportPathAndSection(importPath string) (string, string) {
if strings.Contains(importPath, "#") {
parts := strings.SplitN(importPath, "#", 2)
return parts[0], parts[1]
}
return importPath, ""
}

func resolveSeedImportPath(filePath, importPath, baseDir string, cache *ImportCache, workflowFilePath string, yamlContent string) (string, error) {
fullPath, err := ResolveIncludePath(filePath, baseDir, cache)
if err != nil {
Expand Down Expand Up @@ -399,7 +391,7 @@ func nestedEntriesFromSpecs(specs []ImportSpec) []nestedImportEntry {

func enqueueNestedImportEntry(entry nestedImportEntry, item importQueueItem, baseDir string, cache *ImportCache, workflowFilePath string, yamlContent string, state *importBFSState) error {
nestedImportPath := entry.path
nestedFilePath, nestedSectionName := splitImportPathAndSection(nestedImportPath)
nestedFilePath, nestedSectionName := splitPathAndSection(nestedImportPath)
resolvedPath, nestedRemoteOrigin, err := resolveNestedImportPathAndOrigin(item, nestedFilePath)
if err != nil {
return err
Expand Down
31 changes: 3 additions & 28 deletions pkg/parser/import_topological.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"errors"
"slices"
"sort"
"strings"

"github.com/github/gh-aw/pkg/setutil"
"github.com/github/gh-aw/pkg/sliceutil"
Expand Down Expand Up @@ -74,7 +73,7 @@ func buildImportDependencies(imports []string, baseDir string, cache *ImportCach
}

func resolveNestedImportPaths(importPath, baseDir string, cache *ImportCache) ([]string, error) {
filePath := stripImportSection(importPath)
filePath, _ := splitPathAndSection(importPath)
fullPath, err := ResolveIncludePath(filePath, baseDir, cache)
if err != nil {
return nil, err
Expand All @@ -83,35 +82,11 @@ func resolveNestedImportPaths(importPath, baseDir string, cache *ImportCache) ([
if err != nil {
return nil, err
}
frontmatter, err := extractFrontmatterForTopologicalSort(fullPath, content)
result, err := extractFrontmatterForImport(fullPath, content)
if err != nil {
return nil, err
}
return extractImportPaths(frontmatter), nil
}

func stripImportSection(importPath string) string {
if strings.Contains(importPath, "#") {
parts := strings.SplitN(importPath, "#", 2)
return parts[0]
}
return importPath
}

func extractFrontmatterForTopologicalSort(fullPath string, content []byte) (map[string]any, error) {
var (
result *FrontmatterResult
err error
)
if strings.HasPrefix(fullPath, BuiltinPathPrefix) {
result, err = ExtractFrontmatterFromBuiltinFile(fullPath, content)
} else {
result, err = ExtractFrontmatterFromContent(string(content))
}
if err != nil {
return nil, err
}
return result.Frontmatter, nil
return extractImportPaths(result.Frontmatter), nil
}

func calculateInDegree(imports []string, dependencies map[string][]string, allImportsSet map[string]struct {
Expand Down
10 changes: 1 addition & 9 deletions pkg/parser/include_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func resolveDirectiveWithVisited(
extractTools bool,
visited map[string]struct {
}) (includeDirectiveResolution, bool, error) {
filePath, sectionName := splitIncludePathAndSection(directive.Path)
filePath, sectionName := splitPathAndSection(directive.Path)
fullPath, err := ResolveIncludePath(filePath, baseDir, nil)
if err != nil {
includeLog.Printf("Failed to resolve include path '%s': %v", filePath, err)
Expand Down Expand Up @@ -160,14 +160,6 @@ func resolveDirectiveWithVisited(
}, false, nil
}

func splitIncludePathAndSection(includePath string) (string, string) {
if strings.Contains(includePath, "#") {
parts := strings.SplitN(includePath, "#", 2)
return parts[0], parts[1]
}
return includePath, ""
}

// processIncludedFile processes a single included file, optionally extracting a section
// processIncludedFileWithVisited processes a single included file with cycle detection for nested includes
func processIncludedFileWithVisited(filePath, sectionName string, extractTools bool, visited map[string]struct {
Expand Down
10 changes: 10 additions & 0 deletions pkg/parser/path_section.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package parser

import "strings"

func splitPathAndSection(path string) (string, string) {
if before, after, ok := strings.Cut(path, "#"); ok {
return before, after
}
return path, ""
}
Comment thread
github-actions[bot] marked this conversation as resolved.
131 changes: 7 additions & 124 deletions pkg/parser/remote_fetch_wasm.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,53 +8,6 @@ import (
"strings"
)

func isUnderWorkflowsDirectory(filePath string) bool {
normalizedPath := filepath.ToSlash(filePath)
if !strings.Contains(normalizedPath, ".github/workflows/") {
return false
}
parts := strings.Split(normalizedPath, ".github/workflows/")
if len(parts) < 2 {
return false
}
return !strings.Contains(parts[1], "/")
}

func isCustomAgentFile(filePath string) bool {
normalizedPath := filepath.ToSlash(filePath)
return strings.Contains(normalizedPath, ".github/agents/") && strings.HasSuffix(strings.ToLower(normalizedPath), ".md")
}

func isRepositoryImport(importPath string) bool {
cleanPath := importPath
if idx := strings.Index(importPath, "#"); idx != -1 {
cleanPath = importPath[:idx]
}
pathWithoutRef := cleanPath
if idx := strings.Index(cleanPath, "@"); idx != -1 {
pathWithoutRef = cleanPath[:idx]
}
parts := strings.Split(pathWithoutRef, "/")
if len(parts) != 2 {
return false
}
if strings.HasPrefix(pathWithoutRef, ".") || strings.HasPrefix(pathWithoutRef, "/") {
return false
}
if strings.HasPrefix(pathWithoutRef, "shared/") {
return false
}
owner := parts[0]
repo := parts[1]
if owner == "" || repo == "" {
return false
}
if strings.Contains(repo, ".") {
return false
}
return true
}

func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, error) {
parserLog.Printf("ResolveIncludePath: filePath=%s, baseDir=%s", filePath, baseDir)

Expand All @@ -66,54 +19,25 @@ func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, e
return filePath, nil
}

if isWorkflowSpec(filePath) {
if IsWorkflowSpec(filePath) {
parserLog.Printf("ResolveIncludePath: rejecting remote workflowspec in Wasm build: %s", filePath)
return "", fmt.Errorf("remote imports not available in Wasm: %s", filePath)
}

githubFolder := baseDir
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)
filePathSlash := filepath.ToSlash(filePath)
if strings.HasPrefix(filePathSlash, ".github/") {
resolveBase = repoRoot
} 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 = filepath.FromSlash(stripped)
resolveBase = repoRoot
if strings.HasPrefix(stripped, ".agents/") {
securityBase = filepath.Join(repoRoot, ".agents")
} else {
// .github/-prefixed: security scope is the .github folder.
securityBase = githubFolder
}
}
resolveBase, securityBase, normalizedFilePath := computeIncludeResolveAndSecurityBases(filePath, baseDir)
if resolveBase == "" {
return "", fmt.Errorf("security: path %s must be within .github or .agents folder", normalizedFilePath)
}

fullPath := filepath.Join(resolveBase, filePath)
fullPath := filepath.Join(resolveBase, normalizedFilePath)

normalizedSecurityBase := filepath.Clean(securityBase)
normalizedFullPath := filepath.Clean(fullPath)

relativePath, err := filepath.Rel(normalizedSecurityBase, normalizedFullPath)
if err != nil || relativePath == ".." || strings.HasPrefix(relativePath, ".."+string(filepath.Separator)) || filepath.IsAbs(relativePath) {
allowedFolder := filepath.Base(normalizedSecurityBase)
parserLog.Printf("ResolveIncludePath: security boundary violation: path=%s, allowedFolder=%s", filePath, allowedFolder)
return "", fmt.Errorf("security: path %s must be within %s folder (resolves to: %s)", filePath, allowedFolder, relativePath)
parserLog.Printf("ResolveIncludePath: security boundary violation: path=%s, allowedFolder=%s", normalizedFilePath, allowedFolder)
return "", fmt.Errorf("security: path %s must be within %s folder (resolves to: %s)", normalizedFilePath, allowedFolder, relativePath)
}

// In wasm builds, check the virtual filesystem first
Expand All @@ -124,44 +48,3 @@ func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, e
parserLog.Printf("ResolveIncludePath: file not found in virtual filesystem: %s", fullPath)
return "", fmt.Errorf("file not found: %s", fullPath)
}

// IsWorkflowSpec checks if a path looks like a workflowspec (owner/repo/path[@ref]).
func IsWorkflowSpec(path string) bool {
cleanPath := path
if idx := strings.Index(path, "#"); idx != -1 {
cleanPath = path[:idx]
}
if idx := strings.Index(cleanPath, "@"); idx != -1 {
cleanPath = cleanPath[:idx]
}
parts := strings.Split(cleanPath, "/")
if len(parts) < 3 {
return false
}
// Preserve legacy behavior expected by parser tests: URL-like paths are
// currently treated as workflowspecs because downstream parsing supports
// repository/path extraction from slash-delimited remote references.
if strings.Contains(cleanPath, "://") {
return true
}
if strings.HasPrefix(cleanPath, ".") {
return false
}
if strings.HasPrefix(cleanPath, "shared/") {
return false
}
if strings.HasPrefix(cleanPath, "/") {
return false
}
// Safe indexing: len(parts) >= 3 is guaranteed above.
owner := parts[0]
repo := parts[1]
if owner == "" || repo == "" {
return false
}
return true
}

func isWorkflowSpec(path string) bool {
return IsWorkflowSpec(path)
}
Loading
Loading