Fix repo-root-relative import path resolution in ResolveIncludePath#24501
Merged
Fix repo-root-relative import path resolution in ResolveIncludePath#24501
ResolveIncludePath#24501Conversation
6 tasks
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>
Copilot
AI
changed the title
[WIP] Fix repo-root-relative import path resolution in ResolveIncludePath
Fix repo-root-relative import path resolution in Apr 4, 2026
ResolveIncludePath
Copilot stopped work on behalf of
pelikhan due to an error
April 4, 2026 14:48
Collaborator
|
@copilot add tests to check that these rules work in the repo name ".github" as well |
Collaborator
|
@copilot only the .github, .agents folder are available when resolving imports. Other repo root path will be resolve |
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>
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>
Contributor
Author
Added
|
Contributor
Author
The comment appears truncated — could you clarify the intended behavior? Specifically:
If (1), I'll update the security scope logic in |
Collaborator
|
@copilot restrict workspace root to .github and .agents |
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>
… 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>
Contributor
Author
Done in commit Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes local include/import resolution so repo-root-relative paths (notably .github/... and /.github/...) resolve from the repository root instead of being incorrectly joined onto the workflow’s baseDir (e.g. .github/workflows/...). Also introduces controlled access to repo-root /.agents/... paths and adds tests for the new resolution and security rules.
Changes:
- Update
ResolveIncludePathto treat.github/...and/<repo-root>...paths as repo-root-relative with an explicit security scope. - Mirror the same path-resolution behavior in the Wasm build.
- Add unit tests covering
.github/agentsimports,/.github//.agentsimports, traversal rejection, and the special case where the repository itself is named.github.
Show a summary per file
| File | Description |
|---|---|
pkg/parser/remote_fetch.go |
Adjusts local include path resolution to support repo-root-relative .github/... and restricted /<...> imports with scoped security checks. |
pkg/parser/remote_fetch_wasm.go |
Brings the Wasm implementation in line with the updated repo-root-relative resolution and security rules. |
pkg/parser/frontmatter_utils_test.go |
Adds test coverage for repo-root-relative imports, .agents access rules, and traversal rejection (including .github-repo layout). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
pkg/parser/remote_fetch.go:174
- Repo-root-relative detection is currently hard-coded to POSIX-style prefixes (
.github/,.agents/, and leading/). If an import path contains Windows separators (e.g.,.github\\agents\\planner.md), it won’t be treated as repo-root-relative and will reintroduce the original bad join behavior. Consider normalizingfilePathwithfilepath.ToSlashfor prefix checks (or explicitly handling\\variants) to keep behavior consistent across platforms.
// 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: 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
if strings.HasPrefix(stripped, ".agents/") {
securityBase = filepath.Join(repoRoot, ".agents")
} else {
// .github/-prefixed: security scope is the .github folder.
securityBase = githubFolder
}
}
- Files reviewed: 3/3 changed files
- Comments generated: 4
Comment on lines
139
to
147
| @@ -151,14 +147,44 @@ func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, e | |||
| } | |||
There was a problem hiding this comment.
The .github-folder search loop can become non-terminating on Windows when baseDir is not under a .github directory (e.g., C:\...), because filepath.Dir("C:\\") == "C:\\" and the loop only stops on "." or "/". Add a cross-platform termination condition (e.g., break when the parent dir equals the current dir, or handle volume roots) so local includes from non-.github directories don’t hang.
This issue also appears on line 150 of the same file.
See below for a potential fix:
parent := filepath.Dir(githubFolder)
if parent == githubFolder || parent == "." || parent == "/" {
// If we can't find .github folder, use baseDir
githubFolder = baseDir
break
}
githubFolder = parent
pkg/parser/remote_fetch.go
Outdated
Comment on lines
187
to
188
| 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) |
There was a problem hiding this comment.
The security escape error message still says "must be within .github folder" even when securityBase is .agents (for /.agents/... imports). Update the error text to reflect the actual allowed base (e.g., mention .agents when applicable, or use a generic "allowed folder" message) so users aren’t misled during debugging.
Suggested change
| 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) | |
| 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 %s folder (resolves to: %s)", filePath, allowedFolder, relativePath) |
pkg/parser/remote_fetch_wasm.go
Outdated
| 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) |
There was a problem hiding this comment.
In the Wasm implementation, the security escape error message always says "must be within .github folder" even though securityBase can be .agents for /.agents/... imports. Adjust the error text to reflect the actual security scope (or use a generic "allowed folder" message) to avoid misleading errors.
Suggested change
| 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 the allowed folder (resolves to: %s)", filePath, relativePath) |
Comment on lines
+80
to
+99
| 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 { | ||
| // 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 | ||
| if strings.HasPrefix(stripped, ".agents/") { | ||
| securityBase = filepath.Join(repoRoot, ".agents") | ||
| } else { | ||
| // .github/-prefixed: security scope is the .github folder. | ||
| securityBase = githubFolder | ||
| } | ||
| } |
There was a problem hiding this comment.
Repo-root-relative detection here only recognizes POSIX-style prefixes (.github/, .agents/, leading /). If callers pass Windows-style paths (e.g., .github\\agents\\planner.md), the repo-root-relative logic won’t trigger. Consider normalizing filePath (e.g., filepath.ToSlash) for these prefix checks or handling \\ variants explicitly, consistent with other path-normalization code in the repo.
Collaborator
|
@copilot View comments |
…th 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>
Contributor
Author
Addressed all three review comments in commit
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Collaborator
|
@copilot More tests covering all paths styles |
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>
Contributor
Author
Added
|
This comment has been minimized.
This comment has been minimized.
Contributor
🟢 Contribution Check — AlignedHey Checklist
SummaryThis PR is well-aligned with the project's contribution guidelines. The fix is surgical (two mirrored files for native + Wasm), security considerations are explicitly handled, Windows path separators are normalized, and coverage is comprehensive — including the edge case of a repo named
|
This was referenced Apr 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
imports: [.github/agents/planner.md]was broken becauseResolveIncludePathunconditionally joined import paths ontobaseDir(.github/workflows/), producing.github/workflows/.github/agents/planner.md— a path that never exists.Changes
pkg/parser/remote_fetch.go/remote_fetch_wasm.go: Before resolving, detect repo-root-relative paths and adjust the resolution base accordingly:.github/-prefixed → resolve from repo root; security scope stays.github//-prefixed → strip leading slash, resolve from repo root; only.github/and.agents/subdirectories are accessible (all other paths are rejected with a security error)baseDir)parent == githubFolderas an additional termination conditionfilePathwithfilepath.ToSlashbefore prefix checks to handle Windows-style path separators correctly.githubor.agents) rather than always saying.githubpkg/parser/frontmatter_utils_test.go: Comprehensive test coverage added across three test functions:TestResolveIncludePath— core cases:.github/agents/imports,/.github/- and/.agents/-prefixed repo-root imports, backward-compat relative imports, path traversal rejection, and rejection of/-prefixed paths outside.github/or.agents/TestResolveIncludePath_DotGithubRepo— verifies all resolution rules and traversal rejection work correctly when the repository itself is named.github(e.g. an org'sorg/.githubrepo), where the on-disk layout is<parent>/.github/.github/workflows/TestResolveIncludePath_AllPathStyles— exhaustive coverage of every path form: bare filenames,./-prefixed explicit relative, subdirectory relative, multi-level.github/agents/sub/nested.md, multi-level/.github/and/.agents/paths, intra-.githubtraversal that stays in scope, traversal attacks escaping all security scopes, slash-prefix to disallowed directories, andbaseDirwithout any.githubancestor (plain relative fallback)Example