From df088573fbe512c2f923bb48dce894121a94375f Mon Sep 17 00:00:00 2001 From: Dennis Date: Tue, 11 Aug 2026 23:58:11 +0800 Subject: [PATCH 1/4] fix(localio): bound all text input paths --- internal/localio/input.go | 18 ++++++-- internal/localio/input_test.go | 80 ++++++++++++++++++++++++++++++++-- 2 files changed, 91 insertions(+), 7 deletions(-) diff --git a/internal/localio/input.go b/internal/localio/input.go index 0cee5fc6..cd232152 100644 --- a/internal/localio/input.go +++ b/internal/localio/input.go @@ -15,8 +15,7 @@ const defaultTextInputLimit = int64(8 << 20) var ( readTextInputAll = io.ReadAll - readTextInputStat = os.Stat - readTextInputFile = os.ReadFile + openTextInputFile = os.Open readTextInputRel = filepath.Rel ) @@ -41,6 +40,9 @@ func ReadTextInput(spec string, stdin io.Reader, maxBytes int64) (string, error) return string(data), nil } if !strings.HasPrefix(spec, "@") { + if int64(len(spec)) > maxBytes { + return "", fmt.Errorf("LOCAL_INPUT_TOO_LARGE: 文本输入超过 %d 字节", maxBytes) + } return spec, nil } path := strings.TrimSpace(strings.TrimPrefix(spec, "@")) @@ -70,7 +72,12 @@ func ReadTextInput(spec string, stdin io.Reader, maxBytes int64) (string, error) if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) || filepath.IsAbs(rel) { return "", fmt.Errorf("LOCAL_INPUT_UNSAFE: @file 解析后逃逸工作目录") } - info, err := readTextInputStat(realPath) + file, err := openTextInputFile(realPath) + if err != nil { + return "", fmt.Errorf("LOCAL_INPUT_READ_FAILED: %w", err) + } + defer file.Close() + info, err := file.Stat() if err != nil { return "", fmt.Errorf("LOCAL_INPUT_READ_FAILED: %w", err) } @@ -80,9 +87,12 @@ func ReadTextInput(spec string, stdin io.Reader, maxBytes int64) (string, error) if info.Size() > maxBytes { return "", fmt.Errorf("LOCAL_INPUT_TOO_LARGE: 文件大小 %d 超过 %d 字节", info.Size(), maxBytes) } - data, err := readTextInputFile(realPath) + data, err := readTextInputAll(io.LimitReader(file, maxBytes+1)) if err != nil { return "", fmt.Errorf("LOCAL_INPUT_READ_FAILED: %w", err) } + if int64(len(data)) > maxBytes { + return "", fmt.Errorf("LOCAL_INPUT_TOO_LARGE: 文件超过 %d 字节", maxBytes) + } return string(data), nil } diff --git a/internal/localio/input_test.go b/internal/localio/input_test.go index 93932172..3b60e5bc 100644 --- a/internal/localio/input_test.go +++ b/internal/localio/input_test.go @@ -44,11 +44,76 @@ func TestCrossPlatformCoverageReadTextInputRejectsEscapeAndOversizeE2E(t *testin t.Fatalf("unsafe input accepted: %q", spec) } } + if _, err := ReadTextInput("too-large", nil, 2); err == nil { + t.Fatal("oversize literal accepted") + } if _, err := ReadTextInput("-", strings.NewReader("too-large"), 2); err == nil { t.Fatal("oversize stdin accepted") } } +func TestCrossPlatformCoverageReadTextInputUsesOpenedDescriptorAndBoundedReadE2E(t *testing.T) { + dir := t.TempDir() + old, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + if err := os.Chdir(dir); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chdir(old) }) + + t.Run("path replacement", func(t *testing.T) { + path := filepath.Join(dir, "input.txt") + replacement := filepath.Join(dir, "replacement.txt") + if err := os.WriteFile(path, []byte("safe"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(replacement, []byte("replacement-exceeds-limit"), 0o600); err != nil { + t.Fatal(err) + } + testseam.Swap(t, &openTextInputFile, func(candidate string) (*os.File, error) { + file, openErr := os.Open(candidate) + if openErr != nil { + return nil, openErr + } + if renameErr := os.Rename(replacement, candidate); renameErr != nil { + _ = file.Close() + return nil, renameErr + } + return file, nil + }) + got, err := ReadTextInput("@input.txt", nil, 10) + if err != nil || got != "safe" { + t.Fatalf("replacement read = %q, %v", got, err) + } + }) + + t.Run("file grows after stat", func(t *testing.T) { + path := filepath.Join(dir, "grow.txt") + if err := os.WriteFile(path, []byte("ok"), 0o600); err != nil { + t.Fatal(err) + } + testseam.Swap(t, &readTextInputAll, func(reader io.Reader) ([]byte, error) { + file, openErr := os.OpenFile(path, os.O_APPEND|os.O_WRONLY, 0) + if openErr != nil { + return nil, openErr + } + if _, writeErr := file.WriteString("-too-large"); writeErr != nil { + _ = file.Close() + return nil, writeErr + } + if closeErr := file.Close(); closeErr != nil { + return nil, closeErr + } + return io.ReadAll(reader) + }) + if _, err := ReadTextInput("@grow.txt", nil, 4); err == nil { + t.Fatal("file growth bypassed size limit") + } + }) +} + func TestCrossPlatformCoverageReadTextInputFailureBranchesE2E(t *testing.T) { if got, err := ReadTextInput("literal", nil, 0); err != nil || got != "literal" { t.Fatalf("default limit literal = %q, %v", got, err) @@ -103,11 +168,20 @@ func TestCrossPlatformCoverageReadTextInputFailureBranchesE2E(t *testing.T) { "relative": func(t *testing.T) { testseam.Swap(t, &readTextInputRel, func(string, string) (string, error) { return "", errors.New("rel") }) }, - "stat": func(t *testing.T) { - testseam.Swap(t, &readTextInputStat, func(string) (os.FileInfo, error) { return nil, errors.New("stat") }) + "open": func(t *testing.T) { + testseam.Swap(t, &openTextInputFile, func(string) (*os.File, error) { return nil, errors.New("open") }) + }, + "opened file stat": func(t *testing.T) { + testseam.Swap(t, &openTextInputFile, func(candidate string) (*os.File, error) { + file, openErr := os.Open(candidate) + if openErr == nil { + _ = file.Close() + } + return file, openErr + }) }, "read file": func(t *testing.T) { - testseam.Swap(t, &readTextInputFile, func(string) ([]byte, error) { return nil, errors.New("read file") }) + testseam.Swap(t, &readTextInputAll, func(io.Reader) ([]byte, error) { return nil, errors.New("read file") }) }, } { t.Run(name, func(t *testing.T) { From 3d67d83110525f512e481cb4a3bd005129b54248 Mon Sep 17 00:00:00 2001 From: Dennis Date: Wed, 12 Aug 2026 00:28:43 +0800 Subject: [PATCH 2/4] test(localio): isolate input boundary e2e --- internal/localio/input_bounds_test.go | 75 +++++++++++++++++++++++++++ internal/localio/input_test.go | 62 ---------------------- 2 files changed, 75 insertions(+), 62 deletions(-) create mode 100644 internal/localio/input_bounds_test.go diff --git a/internal/localio/input_bounds_test.go b/internal/localio/input_bounds_test.go new file mode 100644 index 00000000..cd3a9dc0 --- /dev/null +++ b/internal/localio/input_bounds_test.go @@ -0,0 +1,75 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 + +package localio + +import ( + "io" + "os" + "path/filepath" + "testing" + + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/testseam" +) + +func TestCrossPlatformCoverageReadTextInputUsesOpenedDescriptorAndBoundedReadE2E(t *testing.T) { + dir := t.TempDir() + old, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + if err := os.Chdir(dir); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chdir(old) }) + + t.Run("path replacement", func(t *testing.T) { + path := filepath.Join(dir, "input.txt") + replacement := filepath.Join(dir, "replacement.txt") + if err := os.WriteFile(path, []byte("safe"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(replacement, []byte("replacement-exceeds-limit"), 0o600); err != nil { + t.Fatal(err) + } + testseam.Swap(t, &openTextInputFile, func(candidate string) (*os.File, error) { + file, openErr := os.Open(candidate) + if openErr != nil { + return nil, openErr + } + if renameErr := os.Rename(replacement, candidate); renameErr != nil { + _ = file.Close() + return nil, renameErr + } + return file, nil + }) + got, err := ReadTextInput("@input.txt", nil, 10) + if err != nil || got != "safe" { + t.Fatalf("replacement read = %q, %v", got, err) + } + }) + + t.Run("file grows after stat", func(t *testing.T) { + path := filepath.Join(dir, "grow.txt") + if err := os.WriteFile(path, []byte("ok"), 0o600); err != nil { + t.Fatal(err) + } + testseam.Swap(t, &readTextInputAll, func(reader io.Reader) ([]byte, error) { + file, openErr := os.OpenFile(path, os.O_APPEND|os.O_WRONLY, 0) + if openErr != nil { + return nil, openErr + } + if _, writeErr := file.WriteString("-too-large"); writeErr != nil { + _ = file.Close() + return nil, writeErr + } + if closeErr := file.Close(); closeErr != nil { + return nil, closeErr + } + return io.ReadAll(reader) + }) + if _, err := ReadTextInput("@grow.txt", nil, 4); err == nil { + t.Fatal("file growth bypassed size limit") + } + }) +} diff --git a/internal/localio/input_test.go b/internal/localio/input_test.go index 3b60e5bc..5595db18 100644 --- a/internal/localio/input_test.go +++ b/internal/localio/input_test.go @@ -52,68 +52,6 @@ func TestCrossPlatformCoverageReadTextInputRejectsEscapeAndOversizeE2E(t *testin } } -func TestCrossPlatformCoverageReadTextInputUsesOpenedDescriptorAndBoundedReadE2E(t *testing.T) { - dir := t.TempDir() - old, err := os.Getwd() - if err != nil { - t.Fatal(err) - } - if err := os.Chdir(dir); err != nil { - t.Fatal(err) - } - t.Cleanup(func() { _ = os.Chdir(old) }) - - t.Run("path replacement", func(t *testing.T) { - path := filepath.Join(dir, "input.txt") - replacement := filepath.Join(dir, "replacement.txt") - if err := os.WriteFile(path, []byte("safe"), 0o600); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(replacement, []byte("replacement-exceeds-limit"), 0o600); err != nil { - t.Fatal(err) - } - testseam.Swap(t, &openTextInputFile, func(candidate string) (*os.File, error) { - file, openErr := os.Open(candidate) - if openErr != nil { - return nil, openErr - } - if renameErr := os.Rename(replacement, candidate); renameErr != nil { - _ = file.Close() - return nil, renameErr - } - return file, nil - }) - got, err := ReadTextInput("@input.txt", nil, 10) - if err != nil || got != "safe" { - t.Fatalf("replacement read = %q, %v", got, err) - } - }) - - t.Run("file grows after stat", func(t *testing.T) { - path := filepath.Join(dir, "grow.txt") - if err := os.WriteFile(path, []byte("ok"), 0o600); err != nil { - t.Fatal(err) - } - testseam.Swap(t, &readTextInputAll, func(reader io.Reader) ([]byte, error) { - file, openErr := os.OpenFile(path, os.O_APPEND|os.O_WRONLY, 0) - if openErr != nil { - return nil, openErr - } - if _, writeErr := file.WriteString("-too-large"); writeErr != nil { - _ = file.Close() - return nil, writeErr - } - if closeErr := file.Close(); closeErr != nil { - return nil, closeErr - } - return io.ReadAll(reader) - }) - if _, err := ReadTextInput("@grow.txt", nil, 4); err == nil { - t.Fatal("file growth bypassed size limit") - } - }) -} - func TestCrossPlatformCoverageReadTextInputFailureBranchesE2E(t *testing.T) { if got, err := ReadTextInput("literal", nil, 0); err != nil || got != "literal" { t.Fatalf("default limit literal = %q, %v", got, err) From f413db06bedad45d89127f8c2cd8bddc0ac5eac8 Mon Sep 17 00:00:00 2001 From: Dennis Date: Wed, 12 Aug 2026 00:32:56 +0800 Subject: [PATCH 3/4] fix(localio): reject special files before open --- internal/localio/input.go | 11 ++++++ internal/localio/input_bounds_test.go | 50 ++++++++++++++++++++++++ internal/localio/input_fifo_unix_test.go | 46 ++++++++++++++++++++++ internal/localio/input_test.go | 3 ++ 4 files changed, 110 insertions(+) create mode 100644 internal/localio/input_fifo_unix_test.go diff --git a/internal/localio/input.go b/internal/localio/input.go index cd232152..30cbe562 100644 --- a/internal/localio/input.go +++ b/internal/localio/input.go @@ -15,6 +15,7 @@ const defaultTextInputLimit = int64(8 << 20) var ( readTextInputAll = io.ReadAll + statTextInputPath = os.Stat openTextInputFile = os.Open readTextInputRel = filepath.Rel ) @@ -72,6 +73,16 @@ func ReadTextInput(spec string, stdin io.Reader, maxBytes int64) (string, error) if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) || filepath.IsAbs(rel) { return "", fmt.Errorf("LOCAL_INPUT_UNSAFE: @file 解析后逃逸工作目录") } + pathInfo, err := statTextInputPath(realPath) + if err != nil { + return "", fmt.Errorf("LOCAL_INPUT_READ_FAILED: %w", err) + } + if !pathInfo.Mode().IsRegular() { + return "", fmt.Errorf("LOCAL_INPUT_INVALID: @file 必须是普通文件") + } + if pathInfo.Size() > maxBytes { + return "", fmt.Errorf("LOCAL_INPUT_TOO_LARGE: 文件大小 %d 超过 %d 字节", pathInfo.Size(), maxBytes) + } file, err := openTextInputFile(realPath) if err != nil { return "", fmt.Errorf("LOCAL_INPUT_READ_FAILED: %w", err) diff --git a/internal/localio/input_bounds_test.go b/internal/localio/input_bounds_test.go index cd3a9dc0..6fab1e6b 100644 --- a/internal/localio/input_bounds_test.go +++ b/internal/localio/input_bounds_test.go @@ -72,4 +72,54 @@ func TestCrossPlatformCoverageReadTextInputUsesOpenedDescriptorAndBoundedReadE2E t.Fatal("file growth bypassed size limit") } }) + + t.Run("path becomes directory before descriptor check", func(t *testing.T) { + path := filepath.Join(dir, "becomes-directory.txt") + if err := os.WriteFile(path, []byte("safe"), 0o600); err != nil { + t.Fatal(err) + } + testseam.Swap(t, &openTextInputFile, func(candidate string) (*os.File, error) { + if removeErr := os.Remove(candidate); removeErr != nil { + return nil, removeErr + } + if mkdirErr := os.Mkdir(candidate, 0o700); mkdirErr != nil { + return nil, mkdirErr + } + return os.Open(candidate) + }) + if _, err := ReadTextInput("@becomes-directory.txt", nil, 10); err == nil { + t.Fatal("path type replacement bypassed descriptor validation") + } + }) + + t.Run("file grows before descriptor check", func(t *testing.T) { + path := filepath.Join(dir, "grows-before-stat.txt") + if err := os.WriteFile(path, []byte("ok"), 0o600); err != nil { + t.Fatal(err) + } + testseam.Swap(t, &openTextInputFile, func(candidate string) (*os.File, error) { + file, openErr := os.Open(candidate) + if openErr != nil { + return nil, openErr + } + writer, writeOpenErr := os.OpenFile(candidate, os.O_APPEND|os.O_WRONLY, 0) + if writeOpenErr != nil { + _ = file.Close() + return nil, writeOpenErr + } + if _, writeErr := writer.WriteString("-too-large"); writeErr != nil { + _ = writer.Close() + _ = file.Close() + return nil, writeErr + } + if closeErr := writer.Close(); closeErr != nil { + _ = file.Close() + return nil, closeErr + } + return file, nil + }) + if _, err := ReadTextInput("@grows-before-stat.txt", nil, 4); err == nil { + t.Fatal("pre-open growth bypassed descriptor size validation") + } + }) } diff --git a/internal/localio/input_fifo_unix_test.go b/internal/localio/input_fifo_unix_test.go new file mode 100644 index 00000000..a6b31f2f --- /dev/null +++ b/internal/localio/input_fifo_unix_test.go @@ -0,0 +1,46 @@ +//go:build unix + +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 + +package localio + +import ( + "os" + "path/filepath" + "strings" + "syscall" + "testing" + "time" +) + +func TestCrossPlatformCoverageReadTextInputRejectsUnconnectedFIFOWithoutBlockingE2E(t *testing.T) { + dir := t.TempDir() + old, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + if err := os.Chdir(dir); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chdir(old) }) + + fifo := filepath.Join(dir, "input.fifo") + if err := syscall.Mkfifo(fifo, 0o600); err != nil { + t.Fatal(err) + } + result := make(chan error, 1) + go func() { + _, readErr := ReadTextInput("@input.fifo", nil, 1024) + result <- readErr + }() + + select { + case err := <-result: + if err == nil || !strings.Contains(err.Error(), "LOCAL_INPUT_INVALID") { + t.Fatalf("FIFO rejection error = %v", err) + } + case <-time.After(time.Second): + t.Fatal("FIFO without a writer blocked text input validation") + } +} diff --git a/internal/localio/input_test.go b/internal/localio/input_test.go index 5595db18..d20b843a 100644 --- a/internal/localio/input_test.go +++ b/internal/localio/input_test.go @@ -106,6 +106,9 @@ func TestCrossPlatformCoverageReadTextInputFailureBranchesE2E(t *testing.T) { "relative": func(t *testing.T) { testseam.Swap(t, &readTextInputRel, func(string, string) (string, error) { return "", errors.New("rel") }) }, + "path stat": func(t *testing.T) { + testseam.Swap(t, &statTextInputPath, func(string) (os.FileInfo, error) { return nil, errors.New("stat") }) + }, "open": func(t *testing.T) { testseam.Swap(t, &openTextInputFile, func(string) (*os.File, error) { return nil, errors.New("open") }) }, From a7678472ab8319d13303f74a8ab8225d715df3e8 Mon Sep 17 00:00:00 2001 From: Dennis Date: Wed, 12 Aug 2026 00:40:36 +0800 Subject: [PATCH 4/4] test(localio): scope path replacement to unix --- internal/localio/input_bounds_test.go | 26 ---------------- internal/localio/input_fifo_unix_test.go | 38 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 26 deletions(-) diff --git a/internal/localio/input_bounds_test.go b/internal/localio/input_bounds_test.go index 6fab1e6b..3e26c876 100644 --- a/internal/localio/input_bounds_test.go +++ b/internal/localio/input_bounds_test.go @@ -23,32 +23,6 @@ func TestCrossPlatformCoverageReadTextInputUsesOpenedDescriptorAndBoundedReadE2E } t.Cleanup(func() { _ = os.Chdir(old) }) - t.Run("path replacement", func(t *testing.T) { - path := filepath.Join(dir, "input.txt") - replacement := filepath.Join(dir, "replacement.txt") - if err := os.WriteFile(path, []byte("safe"), 0o600); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(replacement, []byte("replacement-exceeds-limit"), 0o600); err != nil { - t.Fatal(err) - } - testseam.Swap(t, &openTextInputFile, func(candidate string) (*os.File, error) { - file, openErr := os.Open(candidate) - if openErr != nil { - return nil, openErr - } - if renameErr := os.Rename(replacement, candidate); renameErr != nil { - _ = file.Close() - return nil, renameErr - } - return file, nil - }) - got, err := ReadTextInput("@input.txt", nil, 10) - if err != nil || got != "safe" { - t.Fatalf("replacement read = %q, %v", got, err) - } - }) - t.Run("file grows after stat", func(t *testing.T) { path := filepath.Join(dir, "grow.txt") if err := os.WriteFile(path, []byte("ok"), 0o600); err != nil { diff --git a/internal/localio/input_fifo_unix_test.go b/internal/localio/input_fifo_unix_test.go index a6b31f2f..67d48f9d 100644 --- a/internal/localio/input_fifo_unix_test.go +++ b/internal/localio/input_fifo_unix_test.go @@ -12,8 +12,46 @@ import ( "syscall" "testing" "time" + + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/testseam" ) +func TestCrossPlatformCoverageReadTextInputUsesOpenedDescriptorAfterPathReplacementE2E(t *testing.T) { + dir := t.TempDir() + old, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + if err := os.Chdir(dir); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chdir(old) }) + + path := filepath.Join(dir, "input.txt") + replacement := filepath.Join(dir, "replacement.txt") + if err := os.WriteFile(path, []byte("safe"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(replacement, []byte("replacement-exceeds-limit"), 0o600); err != nil { + t.Fatal(err) + } + testseam.Swap(t, &openTextInputFile, func(candidate string) (*os.File, error) { + file, openErr := os.Open(candidate) + if openErr != nil { + return nil, openErr + } + if renameErr := os.Rename(replacement, candidate); renameErr != nil { + _ = file.Close() + return nil, renameErr + } + return file, nil + }) + got, err := ReadTextInput("@input.txt", nil, 10) + if err != nil || got != "safe" { + t.Fatalf("replacement read = %q, %v", got, err) + } +} + func TestCrossPlatformCoverageReadTextInputRejectsUnconnectedFIFOWithoutBlockingE2E(t *testing.T) { dir := t.TempDir() old, err := os.Getwd()