Skip to content

fix(exec): ignore URL segments in workspace guard#1384

Closed
Alix-007 wants to merge 6 commits intosipeed:mainfrom
Alix-007:fix/issue-1042-shell-url-guard
Closed

fix(exec): ignore URL segments in workspace guard#1384
Alix-007 wants to merge 6 commits intosipeed:mainfrom
Alix-007:fix/issue-1042-shell-url-guard

Conversation

@Alix-007
Copy link
Copy Markdown
Contributor

@Alix-007 Alix-007 commented Mar 11, 2026

Fixes #1042.

Summary

  • only treat absolute paths as filesystem paths when they start at a shell token boundary
  • stop flagging URL path segments like wttr.in/Beijing?T as workspace escapes
  • add regression coverage for URL commands and real outside-workspace paths

Testing

  • go test ./pkg/tools -run TestShellTool_(GuardCommand|SafePaths|WorkingDir)

@Alix-007
Copy link
Copy Markdown
Contributor Author

Follow-up update: I pushed a small forward-only commit that keeps the path-boundary guard while moving it out of the current merge hotspot in pkg/tools/shell.go. Local validation on this branch: go test ./pkg/tools -run TestShellTool_GuardCommand_IgnoresURLPathSegments -count=1 and go test ./pkg/tools -run TestShellTool_GuardCommand_BlocksAbsolutePathOutsideWorkspace -count=1 both pass.

@Alix-007
Copy link
Copy Markdown
Contributor Author

Pushed a follow-up that aligns the URL guard changes with current upstream/main while keeping the path-boundary check as the only extra behavior. Local go test ./pkg/tools -run TestShellTool_GuardCommand_ -count=1 passed, and a local merge probe against upstream/main now merges cleanly.

@Alix-007
Copy link
Copy Markdown
Contributor Author

Addressed the current red CI items on this PR. This follow-up keeps ile:// URIs inside the workspace guard after the URL-path refactor, fails closed on cross-volume absolute paths, and reshapes the long absolute-path assertion so golines stays clean. Local verification on the branch: go test ./pkg/tools -run TestShellTool_GuardCommand_IgnoresURLPathSegments -count=1, go test ./pkg/tools -run TestShellTool_GuardCommand_BlocksAbsolutePathOutsideWorkspace -count=1, and golangci-lint run --new-from-rev HEAD~1 ./pkg/tools/.... I also ran a merge probe against the latest upstream main and the merged tree passed TestShellTool_FileURISandboxing, TestShellTool_URLBypassPrevented, and TestShellTool_GuardCommand_IgnoresURLPathSegments locally.

@Alix-007
Copy link
Copy Markdown
Contributor Author

Closing this one for now because the remaining path is blocked by GitHub permissions, not by the code change itself. I already resolved the current shell/url guard regressions locally, verified the key tests against the latest upstream main, and produced a merge result that would clear the dirty state. GitHub then rejected the push because updating this branch now also updates .github/workflows/nightly.yml, and the available PAT does not have workflow scope. Reopening or recreating this contribution makes sense once workflow-capable credentials are available.

@Alix-007
Copy link
Copy Markdown
Contributor Author

Closing per current contribution constraints: the remaining push is blocked by missing workflow scope on the available credential, so this PR cannot be finished from here without stronger GitHub permissions.

@Alix-007 Alix-007 closed this Mar 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain: tool go Pull requests that update go code type: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]exec工具的guardCommand方法问题

1 participant