Skip to content

fix: preserve local relative imports during gh aw update #23809

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 3 commits into from
Apr 1, 2026
Merged

fix: preserve local relative imports during gh aw update #23809

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
465986a
Initial plan
Copilot Apr 1, 2026
5a35087
fix: preserve local relative imports during gh aw update
Copilot Apr 1, 2026
4a77103
fix: add path traversal protection and improve CI imports parsing
Copilot Apr 1, 2026
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
164 changes: 164 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2587,6 +2587,170 @@ jobs:
echo "- Success count: ${{ steps.add-workflows.outputs.success_count }}"
echo "- Failure count: ${{ steps.add-workflows.outputs.failure_count }}"

integration-update:
name: Integration Update - Preserve Local Imports
runs-on: ubuntu-latest
timeout-minutes: 15
permissions:
contents: read
concurrency:
group: ci-${{ github.ref }}-integration-update
cancel-in-progress: true
steps:
- name: Checkout code
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2

- name: Set up Go
id: setup-go
uses: actions/setup-go@4dc6199c7b1a012772edbd06daecab0f50c9053c # v6
with:
go-version-file: go.mod
cache: true

- name: Report Go cache status
run: |
if [ "${{ steps.setup-go.outputs.cache-hit }}" == "true" ]; then
echo "✅ Go cache hit" >> $GITHUB_STEP_SUMMARY
else
echo "⚠️ Go cache miss" >> $GITHUB_STEP_SUMMARY
fi

- name: Download dependencies
run: go mod download

- name: Verify dependencies
run: go mod verify

- name: Build gh-aw binary
run: make build

- name: Set up isolated test workspace
run: |
mkdir -p /tmp/test-update-workspace/.github/workflows
cd /tmp/test-update-workspace
git init -q
git config user.email "test@example.com"
git config user.name "Test"
echo "✅ Test workspace initialised at /tmp/test-update-workspace"

- name: Add a workflow from githubnext/agentics
id: add-workflow
env:
GH_TOKEN: ${{ github.token }}
run: |
cd /tmp/test-update-workspace

echo "## Integration Update Test: Preserve Local Imports" >> $GITHUB_STEP_SUMMARY
echo "" >> $GITHUB_STEP_SUMMARY

# Add daily-team-status which uses shared imports in githubnext/agentics
if /home/runner/work/gh-aw/gh-aw/gh-aw add githubnext/agentics/workflows/daily-team-status.md --force 2>&1; then
echo "✅ Added workflow successfully" >> $GITHUB_STEP_SUMMARY
else
echo "❌ Failed to add workflow" >> $GITHUB_STEP_SUMMARY
exit 1
fi

WORKFLOW=".github/workflows/daily-team-status.md"
if [ ! -f "$WORKFLOW" ]; then
echo "❌ Workflow file not found after add"
exit 1
fi

# Check if the workflow has relative imports
if grep -q "^imports:" "$WORKFLOW"; then
echo "✅ Workflow has an imports field" >> $GITHUB_STEP_SUMMARY
# Collect relative import paths (those without "@" — not yet cross-repo refs).
# Python is used for robust YAML-aware parsing instead of fragile AWK patterns.
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says “Python is used for robust YAML-aware parsing”, but the script is using a regex over the raw file contents (not a YAML parser). This is misleading for future maintenance; either adjust the comment or switch to an actual YAML parser (e.g., python3 -c 'import yaml' or yq) if true YAML parsing is intended.

Suggested change
# Python is used for robust YAML-aware parsing instead of fragile AWK patterns.
# Python is used here with a regex to extract imports from the YAML text (more maintainable than inline AWK),
# but it does not perform full YAML-aware parsing.

Copilot uses AI. Check for mistakes.
RELATIVE_IMPORTS=$(python3 - "$WORKFLOW" <<'PYEOF'
import sys, re

content = open(sys.argv[1]).read()
# Find the imports: block (list items under the key, indented with 2+ spaces)
m = re.search(r'^imports:\n((?:[ \t]+-[ \t]+.+\n)+)', content, re.MULTILINE)
if m:
for line in m.group(1).splitlines():
val = line.strip().lstrip('- ').strip()
if val and '@' not in val:
print(val)
PYEOF
)
if [ -n "$RELATIVE_IMPORTS" ]; then
echo "has_relative_imports=true" >> $GITHUB_OUTPUT
echo "$RELATIVE_IMPORTS" > /tmp/relative-imports.txt
echo "Relative import paths: $RELATIVE_IMPORTS" >> $GITHUB_STEP_SUMMARY
else
echo "has_relative_imports=false" >> $GITHUB_OUTPUT
echo "⚠️ All imports already use cross-repo refs; skipping local-file preservation check" >> $GITHUB_STEP_SUMMARY
fi
else
echo "has_relative_imports=false" >> $GITHUB_OUTPUT
echo "⚠️ No imports field found in added workflow; skipping preservation check" >> $GITHUB_STEP_SUMMARY
fi

- name: Create local copies of the shared import files
if: steps.add-workflow.outputs.has_relative_imports == 'true'
run: |
cd /tmp/test-update-workspace
echo "Creating local shared files..." >> $GITHUB_STEP_SUMMARY
while IFS= read -r import_path; do
local_file=".github/workflows/$import_path"
mkdir -p "$(dirname "$local_file")"
echo "# Local shared file (simulating user-copied content)" > "$local_file"
echo "✅ Created: $local_file" >> $GITHUB_STEP_SUMMARY
done < /tmp/relative-imports.txt

- name: Run gh aw update --force
if: steps.add-workflow.outputs.has_relative_imports == 'true'
env:
GH_TOKEN: ${{ github.token }}
run: |
cd /tmp/test-update-workspace
echo "Running gh aw update --force --no-compile daily-team-status ..."
/home/runner/work/gh-aw/gh-aw/gh-aw update daily-team-status --force --no-compile 2>&1
echo "✅ Update completed" >> $GITHUB_STEP_SUMMARY

- name: Verify local import paths are preserved after update
if: steps.add-workflow.outputs.has_relative_imports == 'true'
run: |
cd /tmp/test-update-workspace
WORKFLOW=".github/workflows/daily-team-status.md"

echo "" >> $GITHUB_STEP_SUMMARY
echo "### Import paths after \`gh aw update\`" >> $GITHUB_STEP_SUMMARY
echo '```' >> $GITHUB_STEP_SUMMARY
grep -A10 "^imports:" "$WORKFLOW" >> $GITHUB_STEP_SUMMARY || true
echo '```' >> $GITHUB_STEP_SUMMARY

FAILED=false
while IFS= read -r import_path; do
local_file=".github/workflows/$import_path"

# Confirm the local file still exists (sanity check)
if [ ! -f "$local_file" ]; then
echo "⚠️ Local file not found (was it deleted?): $local_file"
continue
fi

# The import entry in the workflow file must still be the relative path,
# NOT a cross-repo reference (which would contain "@" and "owner/repo/").
if grep -qF "- $import_path" "$WORKFLOW"; then
echo "✅ Relative import preserved: $import_path" >> $GITHUB_STEP_SUMMARY
else
echo "❌ FAIL: '$import_path' was rewritten even though the local file exists" >> $GITHUB_STEP_SUMMARY
echo "❌ Current imports section:"
grep -A10 "^imports:" "$WORKFLOW" || true
FAILED=true
fi
done < /tmp/relative-imports.txt

if [ "$FAILED" = "true" ]; then
echo "❌ **FAILURE**: gh aw update rewrote local relative imports to cross-repo paths" >> $GITHUB_STEP_SUMMARY
exit 1
fi

echo "✅ **SUCCESS**: All local relative import paths were preserved by gh aw update" >> $GITHUB_STEP_SUMMARY

integration-unauthenticated-add:
name: Integration Unauthenticated Add (Public Repo)
runs-on: ubuntu-latest
Expand Down
60 changes: 54 additions & 6 deletions pkg/cli/imports.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,10 @@ func resolveImportPath(importPath string, workflowPath string) string {
// processImportsWithWorkflowSpec processes imports field in frontmatter and replaces local file references
// with workflowspec format (owner/repo/path@sha) for all imports found.
// Handles both array form and object form (with 'aw' subfield) of the imports field.
func processImportsWithWorkflowSpec(content string, workflow *WorkflowSpec, commitSHA string, verbose bool) (string, error) {
importsLog.Printf("Processing imports with workflowspec: repo=%s, sha=%s", workflow.RepoSlug, commitSHA)
// If localWorkflowDir is non-empty, any import path whose file exists under that directory is
// left as a local relative path rather than being rewritten to a cross-repo reference.
func processImportsWithWorkflowSpec(content string, workflow *WorkflowSpec, commitSHA string, localWorkflowDir string, verbose bool) (string, error) {
importsLog.Printf("Processing imports with workflowspec: repo=%s, sha=%s, localWorkflowDir=%s", workflow.RepoSlug, commitSHA, localWorkflowDir)
if verbose {
fmt.Fprintln(os.Stderr, console.FormatVerboseMessage("Processing imports field to replace with workflowspec"))
}
Expand All @@ -79,6 +81,10 @@ func processImportsWithWorkflowSpec(content string, workflow *WorkflowSpec, comm
}

// processImportPaths converts a list of raw import paths to workflowspec format.
// Paths that already use the workflowspec format (contain "@") are left unchanged.
// When localWorkflowDir is set, relative paths whose files exist locally are also
// preserved as-is so that consumers who have copied shared files into their own repo
// are not forced onto cross-repo references after every `gh aw update`.
processImportPaths := func(imports []string) []string {
processed := make([]string, 0, len(imports))
for _, importPath := range imports {
Expand All @@ -87,6 +93,16 @@ func processImportsWithWorkflowSpec(content string, workflow *WorkflowSpec, comm
processed = append(processed, importPath)
continue
}
// Preserve relative paths whose files exist in the local workflow directory.
// Absolute paths (starting with "/") are not checked — they are always resolved
// relative to the repo root and cannot be reliably tested here.
if localWorkflowDir != "" && !strings.HasPrefix(importPath, "/") {
if isLocalFileForUpdate(localWorkflowDir, importPath) {
importsLog.Printf("Import path exists locally, preserving relative path: %s", importPath)
processed = append(processed, importPath)
continue
}
}
resolvedPath := resolveImportPath(importPath, workflow.WorkflowPath)
importsLog.Printf("Resolved import path: %s -> %s (workflow: %s)", importPath, resolvedPath, workflow.WorkflowPath)
workflowSpec := buildWorkflowSpecRef(workflow.RepoSlug, resolvedPath, commitSHA, workflow.Version)
Expand Down Expand Up @@ -317,10 +333,12 @@ func processIncludesWithWorkflowSpec(content string, workflow *WorkflowSpec, com
}

// processIncludesInContent processes @include directives in workflow content for update command
// and also processes imports field in frontmatter
func processIncludesInContent(content string, workflow *WorkflowSpec, commitSHA string, verbose bool) (string, error) {
// and also processes imports field in frontmatter.
// If localWorkflowDir is non-empty, any relative import/include path whose file exists under
// that directory is left as-is rather than being rewritten to a cross-repo reference.
func processIncludesInContent(content string, workflow *WorkflowSpec, commitSHA string, localWorkflowDir string, verbose bool) (string, error) {
// First process imports field in frontmatter
processedImportsContent, err := processImportsWithWorkflowSpec(content, workflow, commitSHA, verbose)
processedImportsContent, err := processImportsWithWorkflowSpec(content, workflow, commitSHA, localWorkflowDir, verbose)
if err != nil {
if verbose {
fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to process imports: %v", err)))
Expand Down Expand Up @@ -367,6 +385,15 @@ func processIncludesInContent(content string, workflow *WorkflowSpec, commitSHA
continue
}

// Preserve relative @include paths whose files exist in the local workflow directory.
if localWorkflowDir != "" && !strings.HasPrefix(filePath, "/") {
if isLocalFileForUpdate(localWorkflowDir, filePath) {
importsLog.Printf("Include path exists locally, preserving: %s", filePath)
result.WriteString(line + "\n")
continue
}
}

// Resolve the file path relative to the workflow file's directory
resolvedPath := resolveImportPath(filePath, workflow.WorkflowPath)

Expand All @@ -393,7 +420,28 @@ func processIncludesInContent(content string, workflow *WorkflowSpec, commitSHA
return result.String(), scanner.Err()
}

// isWorkflowSpecFormat checks if a path already looks like a workflowspec
// isLocalFileForUpdate returns true when importPath resolves to an existing file
// within localWorkflowDir. The resolved absolute path must stay inside localWorkflowDir
// to guard against path traversal (e.g. "../../etc/passwd" in import paths).
// importPath must be a relative path — callers must not pass absolute paths here.
func isLocalFileForUpdate(localWorkflowDir, importPath string) bool {
if localWorkflowDir == "" || importPath == "" {
return false
}
localPath := filepath.Join(localWorkflowDir, importPath)
absDir, err1 := filepath.Abs(localWorkflowDir)
absPath, err2 := filepath.Abs(localPath)
if err1 != nil || err2 != nil {
return false
}
// Reject traversal attempts: the resolved path must be a child of localWorkflowDir
if !strings.HasPrefix(absPath, absDir+string(filepath.Separator)) {
return false
}
_, statErr := os.Stat(localPath)
return statErr == nil
Comment on lines +441 to +442
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isLocalFileForUpdate treats any existing path as a valid “local file”, but os.Stat will also succeed for directories. That can cause imports:/@include entries that point at a directory (or other non-regular file types) to be preserved even though they aren’t valid include targets. Consider checking the returned FileInfo and requiring a regular file (and not a directory) before returning true.

Suggested change
_, statErr := os.Stat(localPath)
return statErr == nil
fi, statErr := os.Stat(localPath)
if statErr != nil {
return false
}
return fi.Mode().IsRegular()

Copilot uses AI. Check for mistakes.
}

// A workflowspec is identified by having an @ version indicator (e.g., owner/repo/path@sha)
// Simple paths like "shared/mcp/file.md" are NOT workflowspecs and should be processed
func isWorkflowSpecFormat(path string) bool {
Expand Down
Loading
Loading