Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
65 changes: 64 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ jobs:
github.repository == 'docker/docker-agent' &&
github.event.repository.fork == false && (
(github.event_name == 'pull_request' &&
github.event.pull_request.base.repo.full_name == github.repository) ||
github.event.pull_request.base.repo.full_name == github.repository &&
github.event.pull_request.head.repo.full_name == github.repository) ||
(github.event_name == 'workflow_dispatch' &&
startsWith(github.ref, 'refs/heads/') &&
(github.ref != 'refs/heads/main' || inputs.image_mode == 'build-only') &&
Expand Down Expand Up @@ -303,6 +304,68 @@ jobs:
GIT_TAG=pr
GIT_COMMIT=dev

# Fork PRs cannot use repository variables or OIDC; build without credentials.
build-image-fork:
if: >-
github.repository == 'docker/docker-agent' &&
github.event_name == 'pull_request' &&
github.event.pull_request.base.repo.full_name == github.repository &&
github.event.pull_request.head.repo.full_name != github.repository
permissions:
contents: read
timeout-minutes: 30
strategy:
fail-fast: false
matrix:
include:
- platform: linux/amd64
runner: ubuntu-latest
- platform: linux/arm64
runner: ubuntu-24.04-arm
runs-on: ${{ matrix.runner }}
steps:
- name: Checkout
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3.12.0
with:
driver: docker-container
cache-binary: false

- name: Build image
uses: docker/build-push-action@10e90e3645eae34f1e60eeb005ba3a3d33f178e8 # v6.19.2
with:
context: .
platforms: ${{ matrix.platform }}
outputs: type=cacheonly
push: false
load: false
sbom: false
provenance: false
github-token: ''
build-args: |
GIT_TAG=pr
GIT_COMMIT=dev

- name: Build template
uses: docker/build-push-action@10e90e3645eae34f1e60eeb005ba3a3d33f178e8 # v6.19.2
with:
context: .
target: template
platforms: ${{ matrix.platform }}
outputs: type=cacheonly
push: false
load: false
sbom: false
provenance: false
github-token: ''
build-args: |
GIT_TAG=pr
GIT_COMMIT=dev

build-and-push-image:
if: >-
github.repository == 'docker/docker-agent' &&
Expand Down
4 changes: 2 additions & 2 deletions e2e/testdata/cassettes/TestExec_Anthropic_ToolCall.yaml

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions e2e/testdata/cassettes/TestExec_Gemini_ToolCall.yaml

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions e2e/testdata/cassettes/TestExec_Mistral_ToolCall.yaml

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions e2e/testdata/cassettes/TestExec_OpenAI_HideToolCalls.yaml

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions e2e/testdata/cassettes/TestExec_OpenAI_ToolCall.yaml

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ interactions:
proto_minor: 1
content_length: 0
host: api.openai.com
body: '{"input":[{"content":[{"text":"You are a knowledgeable assistant that can write test files.","type":"input_text"}],"role":"system"},{"content":[{"text":"## Filesystem Tools\n\n- The working directory is \"/tmp/wd\"; relative paths resolve from it\n- Absolute paths must match the host OS (e.g. C:\\... on Windows, /... on Unix)\n- Prefer read_multiple_files over sequential read_file calls\n- Use search_files_content to locate code or text across files\n- Use exclude patterns in searches and max_depth in directory_tree to limit output","type":"input_text"}],"role":"system"},{"content":"Create a hello.txt file with \"Hello, World!\" content. Try only once. On error, exit without further message.","role":"user"}],"model":"gpt-5-mini","reasoning":{"summary":"detailed"},"tools":[{"strict":true,"parameters":{"additionalProperties":false,"properties":{"content":{"description":"File content","type":"string"},"path":{"description":"File to write","type":"string"}},"required":["content","path"],"type":"object"},"name":"write_file","description":"Create a new file or completely overwrite an existing file with new content.","type":"function"}],"stream":true}'
body: '{"input":[{"content":[{"text":"You are a knowledgeable assistant that can write test files.","type":"input_text"}],"role":"system"},{"content":[{"text":"## Filesystem Tools\n\n- Relative paths resolve from the working directory; absolute paths and \"..\" work as expected\n- Prefer read_multiple_files over sequential read_file calls\n- Use search_files_content to locate code or text across files\n- Use exclude patterns in searches and max_depth in directory_tree to limit output","type":"input_text"}],"role":"system"},{"content":"Create a hello.txt file with \"Hello, World!\" content. Try only once. On error, exit without further message.","role":"user"}],"model":"gpt-5-mini","reasoning":{"summary":"detailed"},"tools":[{"strict":true,"parameters":{"additionalProperties":false,"properties":{"content":{"description":"File content","type":"string"},"path":{"description":"File to write","type":"string"}},"required":["content","path"],"type":"object"},"name":"write_file","description":"Create a new file or completely overwrite an existing file with new content.","type":"function"}],"stream":true}'
url: https://api.openai.com/v1/responses
method: POST
response:
Expand Down Expand Up @@ -622,7 +622,7 @@ interactions:
proto_minor: 1
content_length: 0
host: api.openai.com
body: '{"input":[{"content":[{"text":"You are a knowledgeable assistant that can write test files.","type":"input_text"}],"role":"system"},{"content":[{"text":"## Filesystem Tools\n\n- The working directory is \"/tmp/wd\"; relative paths resolve from it\n- Absolute paths must match the host OS (e.g. C:\\... on Windows, /... on Unix)\n- Prefer read_multiple_files over sequential read_file calls\n- Use search_files_content to locate code or text across files\n- Use exclude patterns in searches and max_depth in directory_tree to limit output","type":"input_text"}],"role":"system"},{"content":"Create a hello.txt file with \"Hello, World!\" content. Try only once. On error, exit without further message.","role":"user"},{"arguments":"{\"content\":\"Hello, World!\",\"path\":\"hello.txt\"}","call_id":"call_GdIrV330GhotzkZcKWdlNbPV","name":"write_file","type":"function_call"},{"output":"The user rejected the tool call.","call_id":"call_GdIrV330GhotzkZcKWdlNbPV","type":"function_call_output"}],"model":"gpt-5-mini","reasoning":{"summary":"detailed"},"tools":[{"strict":true,"parameters":{"additionalProperties":false,"properties":{"content":{"description":"File content","type":"string"},"path":{"description":"File to write","type":"string"}},"required":["content","path"],"type":"object"},"name":"write_file","description":"Create a new file or completely overwrite an existing file with new content.","type":"function"}],"stream":true}'
body: '{"input":[{"content":[{"text":"You are a knowledgeable assistant that can write test files.","type":"input_text"}],"role":"system"},{"content":[{"text":"## Filesystem Tools\n\n- Relative paths resolve from the working directory; absolute paths and \"..\" work as expected\n- Prefer read_multiple_files over sequential read_file calls\n- Use search_files_content to locate code or text across files\n- Use exclude patterns in searches and max_depth in directory_tree to limit output","type":"input_text"}],"role":"system"},{"content":"Create a hello.txt file with \"Hello, World!\" content. Try only once. On error, exit without further message.","role":"user"},{"arguments":"{\"content\":\"Hello, World!\",\"path\":\"hello.txt\"}","call_id":"call_GdIrV330GhotzkZcKWdlNbPV","name":"write_file","type":"function_call"},{"output":"The user rejected the tool call.","call_id":"call_GdIrV330GhotzkZcKWdlNbPV","type":"function_call_output"}],"model":"gpt-5-mini","reasoning":{"summary":"detailed"},"tools":[{"strict":true,"parameters":{"additionalProperties":false,"properties":{"content":{"description":"File content","type":"string"},"path":{"description":"File to write","type":"string"}},"required":["content","path"],"type":"object"},"name":"write_file","description":"Create a new file or completely overwrite an existing file with new content.","type":"function"}],"stream":true}'
url: https://api.openai.com/v1/responses
method: POST
response:
Expand Down
5 changes: 0 additions & 5 deletions pkg/fake/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,6 @@ func DefaultMatcher(onError func(err error)) recorder.MatcherFunc {
toolChoiceRegex := regexp.MustCompile(`"tool_choice":"[^"]*",?`)
// Normalize prompt-file paths (they are machine-specific absolute paths).
promptFileRegex := regexp.MustCompile(`Instructions from: (?:[^\\"\r\n]|\\\\)+`)
// Normalize the working directory stated in the filesystem toolset
// instructions (machine-specific absolute path).
workingDirRegex := regexp.MustCompile(`The working directory is \\"(?:[^\\"\r\n]|\\\\)+\\"`)

return func(r *http.Request, i cassette.Request) bool {
if r.Body == nil || r.Body == http.NoBody {
Expand Down Expand Up @@ -328,14 +325,12 @@ func DefaultMatcher(onError func(err error)) recorder.MatcherFunc {
normalizedReq = reasoningRegex.ReplaceAllString(normalizedReq, "")
normalizedReq = toolChoiceRegex.ReplaceAllString(normalizedReq, "")
normalizedReq = promptFileRegex.ReplaceAllString(normalizedReq, "Instructions from: FILE")
normalizedReq = workingDirRegex.ReplaceAllString(normalizedReq, "The working directory is WD")
normalizedCassette := callIDRegex.ReplaceAllString(i.Body, "call_ID")
normalizedCassette = maxTokensRegex.ReplaceAllString(normalizedCassette, "")
normalizedCassette = thinkingConfigRegex.ReplaceAllString(normalizedCassette, "")
normalizedCassette = reasoningRegex.ReplaceAllString(normalizedCassette, "")
normalizedCassette = toolChoiceRegex.ReplaceAllString(normalizedCassette, "")
normalizedCassette = promptFileRegex.ReplaceAllString(normalizedCassette, "Instructions from: FILE")
normalizedCassette = workingDirRegex.ReplaceAllString(normalizedCassette, "The working directory is WD")

return normalizedReq == normalizedCassette
}
Expand Down
73 changes: 8 additions & 65 deletions pkg/tools/builtin/filesystem/filesystem.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"os"
"path/filepath"
"regexp"
"runtime"
"strings"
"sync"
"unicode/utf8"
Expand Down Expand Up @@ -250,12 +249,9 @@ func (t *ToolSet) Close() error {

func (t *ToolSet) Instructions() string {
var b strings.Builder
b.WriteString("## Filesystem Tools\n")
if t.workingDir != "" {
fmt.Fprintf(&b, "\n- The working directory is %q; relative paths resolve from it", t.workingDir)
}
b.WriteString(`
- Absolute paths must match the host OS (e.g. C:\... on Windows, /... on Unix)
b.WriteString(`## Filesystem Tools

- Relative paths resolve from the working directory; absolute paths and ".." work as expected
- Prefer read_multiple_files over sequential read_file calls
- Use search_files_content to locate code or text across files
- Use exclude patterns in searches and max_depth in directory_tree to limit output`)
Expand Down Expand Up @@ -638,9 +634,9 @@ func (t *ToolSet) executePostEditCommands(ctx context.Context, filePath string)
// resolvePath resolves a path relative to the working directory.
// A leading "~" or "~/" is expanded to the user's home directory so that
// LLM-supplied paths like "~/file.txt" work without the agent having to
// know the user's home directory upfront. Absolute paths are used as-is;
// everything else (including "." and "..") is joined with the working
// directory.
// know the user's home directory upfront. Relative paths (including ".")
// are joined with the working directory. Absolute paths and paths
// starting with ".." are used as-is.
//
// resolvePath does NOT enforce the allow- or deny-lists; callers should use
// [resolveAndCheckPath] when those checks are required (i.e. for any path
Expand All @@ -656,43 +652,6 @@ func (t *ToolSet) resolvePath(path string) string {
return filepath.Clean(filepath.Join(t.workingDir, path))
}

// checkForeignPath rejects paths that are absolute in another OS's syntax
// (e.g. "/mnt/c/..." on Windows, "C:\..." on Unix). filepath.IsAbs does not
// recognize them, so resolvePath would silently join them onto the working
// directory and the caller would get a baffling not-found for a mangled
// path. An explicit "./" prefix bypasses the check for the rare legitimate
// file whose name matches one of these shapes.
func checkForeignPath(path, goos string) error {
if goos == "windows" {
switch {
// "//server/share" and "\\server\share" are valid UNC paths.
case strings.HasPrefix(path, "/") && !strings.HasPrefix(path, "//"):
return fmt.Errorf("path %q looks like a POSIX absolute path, but this host runs Windows; use a Windows path (e.g. C:\\...) or a path relative to the working directory", path)
case strings.HasPrefix(path, `\`) && !strings.HasPrefix(path, `\\`):
return fmt.Errorf("path %q is rooted but has no drive letter; use a full Windows path (e.g. C:\\...) or a path relative to the working directory", path)
case isDriveRelative(path):
return fmt.Errorf("path %q is drive-relative; use a full Windows path (e.g. C:\\...) or a path relative to the working directory", path)
}
return nil
}
if hasDrivePrefix(path) && len(path) > 2 && (path[2] == '/' || path[2] == '\\') {
return fmt.Errorf("path %q looks like a Windows absolute path, but this host runs %s; use a POSIX path (e.g. /home/...) or a path relative to the working directory", path, goos)
}
return nil
}

// hasDrivePrefix reports whether path starts with a drive letter and colon.
func hasDrivePrefix(path string) bool {
return len(path) >= 2 && path[1] == ':' &&
('a' <= path[0] && path[0] <= 'z' || 'A' <= path[0] && path[0] <= 'Z')
}

// isDriveRelative reports whether path is like "C:" or "C:foo" — relative
// to the current directory of drive C:, which filepath.Join cannot represent.
func isDriveRelative(path string) bool {
return hasDrivePrefix(path) && (len(path) == 2 || path[2] != '/' && path[2] != '\\')
}

// resolveAndCheckPath is the canonical entry point used by every filesystem
// handler that operates on a user-supplied path. It resolves the path against
// the working directory and validates the result against the allow- and
Expand Down Expand Up @@ -736,9 +695,6 @@ func (t *ToolSet) resolveAndCheckPath(path string) (string, error) {
if t.sandboxBroken {
return "", errors.New("filesystem toolset is disabled due to invalid allow/deny list configuration")
}
if err := checkForeignPath(path, runtime.GOOS); err != nil {
return "", err
}

resolved := t.resolvePath(path)
if t.agentsIgnore.Match(resolved) {
Expand All @@ -760,12 +716,6 @@ func (t *ToolSet) resolveAndCheckPath(path string) (string, error) {
return resolved, nil
}

// resolutionHint tells the model where a failed path actually pointed, so
// it can correct a wrong base instead of retrying the same path.
func (t *ToolSet) resolutionHint(resolved string) string {
return fmt.Sprintf(" (resolved to %q; relative paths resolve from the working directory %q)", resolved, t.workingDir)
}

// rootedAccess returns the [*os.Root] handle and rooted (slash-separated)
// name for resolved when the allow-list is configured.
//
Expand Down Expand Up @@ -1019,9 +969,6 @@ func (t *ToolSet) handleEditFile(ctx context.Context, args EditFileArgs) (*tools

content, err := t.readFile(resolvedPath)
if err != nil {
if errors.Is(err, fs.ErrNotExist) {
return tools.ResultError("not found" + t.resolutionHint(resolvedPath)), nil
}
return tools.ResultError(fmt.Sprintf("Error reading file: %s", err)), nil
}

Expand Down Expand Up @@ -1066,9 +1013,6 @@ func (t *ToolSet) handleListDirectory(ctx context.Context, args ListDirectoryArg

entries, err := t.readDir(resolvedPath)
if err != nil {
if errors.Is(err, fs.ErrNotExist) {
return tools.ResultError("not found" + t.resolutionHint(resolvedPath)), nil
}
return tools.ResultError(fmt.Sprintf("Error reading directory: %s", err)), nil
}

Expand Down Expand Up @@ -1138,7 +1082,7 @@ func (t *ToolSet) handleReadFile(ctx context.Context, args ReadFileArgs) (*tools
if err != nil {
var errMsg string
if errors.Is(err, fs.ErrNotExist) {
errMsg = "not found" + t.resolutionHint(resolvedPath)
errMsg = "not found"
} else {
errMsg = err.Error()
}
Expand All @@ -1147,7 +1091,6 @@ func (t *ToolSet) handleReadFile(ctx context.Context, args ReadFileArgs) (*tools
Output: errMsg,
IsError: true,
Meta: ReadFileMeta{
Path: args.Path,
Error: errMsg,
},
}, nil
Expand Down Expand Up @@ -1375,7 +1318,7 @@ func (t *ToolSet) handleReadMultipleFiles(ctx context.Context, args ReadMultiple
if err != nil {
errMsg := err.Error()
if errors.Is(err, fs.ErrNotExist) {
errMsg = "not found" + t.resolutionHint(resolvedPath)
errMsg = "not found"
}
contents = append(contents, PathContent{
Path: path,
Expand Down
62 changes: 1 addition & 61 deletions pkg/tools/builtin/filesystem/filesystem_paths_test.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
package filesystem

import (
"fmt"
"os"
"path/filepath"
"runtime"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -48,59 +46,6 @@ func TestFilesystemTool_DefaultIsUnrestricted(t *testing.T) {
assert.Equal(t, want, resolved)
}

func TestCheckForeignPath(t *testing.T) {
t.Parallel()
tests := []struct {
path string
goos string
reject bool
}{
{"/mnt/c/Users/x/compose.yml", "windows", true},
{`\foo\bar`, "windows", true},
{"C:foo", "windows", true},
{"C:", "windows", true},
{`C:\Users\x`, "windows", false},
{"C:/Users/x", "windows", false},
{"//server/share", "windows", false},
{`\\server\share`, "windows", false},
{"./mnt/c/x", "windows", false},
{"rel/path", "windows", false},
{"~/file.txt", "windows", false},
{`C:\Users\x`, "linux", true},
{"C:/Users/x", "darwin", true},
{"/mnt/c/Users/x", "linux", false},
{`./C:\weird`, "linux", false},
{"C:foo", "linux", false},
{"rel/path", "linux", false},
}
for _, tt := range tests {
t.Run(tt.goos+" "+tt.path, func(t *testing.T) {
err := checkForeignPath(tt.path, tt.goos)
if tt.reject {
assert.Error(t, err)
} else {
assert.NoError(t, err)
}
})
}
}

// A foreign-OS absolute path must be rejected with guidance instead of
// silently joining onto the working directory (issue seen on Windows with
// "/mnt/c/..." resolving to "<wd>\mnt\c\...").
func TestResolveAndCheckPath_RejectsForeignPath(t *testing.T) {
t.Parallel()
tool := New(t.TempDir())

foreign := `C:\Users\x\compose.yml`
if runtime.GOOS == "windows" {
foreign = "/mnt/c/Users/x/compose.yml"
}
_, err := tool.resolveAndCheckPath(foreign)
require.Error(t, err)
assert.Contains(t, err.Error(), "relative to the working directory")
}

func TestFilesystemTool_AllowList_DotMeansWorkingDir(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()
Expand Down Expand Up @@ -406,16 +351,11 @@ func TestFilesystemTool_Instructions_MentionsRestrictions(t *testing.T) {
t.Parallel()
wd := t.TempDir()

// Default instructions: working directory stated (%q-quoted), no
// restriction text.
// Default instructions: no restriction text.
plain := New(wd).Instructions()
assert.Contains(t, plain, fmt.Sprintf("%q", wd))
assert.NotContains(t, plain, "restricted")
assert.NotContains(t, plain, "must not access")

// No working directory configured: no dangling `The working directory is ""`.
assert.NotContains(t, New("").Instructions(), "working directory is")

// With an allow-list: instructions mention the restriction.
allowed := newTestToolSet(t, wd, WithAllowList([]string{".", "~"})).Instructions()
assert.Contains(t, allowed, "restricted")
Expand Down
Loading
Loading