From 235433d519bdbc67594a76853a7a9af74aa6f4e4 Mon Sep 17 00:00:00 2001 From: HuaiYJ Date: Mon, 24 Aug 2026 16:21:13 +0800 Subject: [PATCH] fix: restrict file pulls to allowed roots --- cmd/ocm-agent/main.go | 7 ++++- config.yaml.example | 6 ++++ internal/config/config.go | 6 ++++ internal/filepull/puller.go | 49 +++++++++++++++++++++++++++++++- internal/filepull/puller_test.go | 44 ++++++++++++++++++++++++++-- 5 files changed, 107 insertions(+), 5 deletions(-) diff --git a/cmd/ocm-agent/main.go b/cmd/ocm-agent/main.go index 936f119..62719ed 100644 --- a/cmd/ocm-agent/main.go +++ b/cmd/ocm-agent/main.go @@ -304,8 +304,13 @@ func connectionSupervisor( AgentID: agentID, }, log) + allowedFilePullRoots := cfg.FilePull.AllowedDirs + if len(allowedFilePullRoots) == 0 { + allowedFilePullRoots = []string{cfg.DataDir} + } puller := filepull.NewFilePuller(artifactUploader, fpRep, filepull.Config{ - AgentID: agentID, + AgentID: agentID, + AllowedRoots: allowedFilePullRoots, }, log) runner, runnerErr := taskrunner.NewRunner(taskrunner.Config{ diff --git a/config.yaml.example b/config.yaml.example index 71ebe41..5cb3212 100644 --- a/config.yaml.example +++ b/config.yaml.example @@ -40,3 +40,9 @@ fingerprint: taskrunner: workdir_ttl_sec: 86400 workdir_cleanup_interval_sec: 0 + +# FILE_PULL 仅允许读取这些目录下的普通文件。留空时默认只允许 data_dir。 +file_pull: + allowed_dirs: + - /opt/ocm-agent/data + - /var/log diff --git a/internal/config/config.go b/internal/config/config.go index 0f567f8..24bc771 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -21,9 +21,15 @@ type Config struct { Collector CollectorConfig `yaml:"collector" json:"collector"` Fingerprint FingerprintConfig `yaml:"fingerprint" json:"fingerprint"` Taskrunner TaskrunnerConfig `yaml:"taskrunner" json:"taskrunner"` + FilePull FilePullConfig `yaml:"file_pull" json:"file_pull"` PluginsConfDir string `yaml:"plugins_conf_dir" json:"plugins_conf_dir,omitempty"` } +// FilePullConfig 将控制面下发的文件拉取限制在显式允许的本地目录内。 +type FilePullConfig struct { + AllowedDirs []string `yaml:"allowed_dirs" json:"allowed_dirs,omitempty"` +} + // TaskrunnerConfig 控制 run_artifact 任务的本地落盘与清理策略。 // // 每次下发任务，agent 会在 /tasks// 下写入 diff --git a/internal/filepull/puller.go b/internal/filepull/puller.go index 13a670b..fe4f7e5 100644 --- a/internal/filepull/puller.go +++ b/internal/filepull/puller.go @@ -12,6 +12,8 @@ import ( "fmt" "io/fs" "os" + "path/filepath" + "strings" "go.uber.org/zap" ) @@ -34,11 +36,14 @@ const ( StatusError = "error" ReasonFileNotFound = "file_not_found" + ReasonPathDenied = "path_denied" ReasonReadFailed = "read_failed" ReasonUploadFailed = "upload_failed" ReasonInternal = "internal_error" ) +var ErrPathDenied = errors.New("file pull path denied") + // Report 是通过 Reporter 上报的 JSON 信封，结构由 spec.md 中 // "Happy path file pull" 章节固定下来。 type Report struct { @@ -89,6 +94,8 @@ type Config struct { // HardMaxSize 是与 action.max_size 无关、强制生效的字节数上限； // NewFilePuller 会将其默认设为 DefaultMaxFileSize。 HardMaxSize int64 + // AllowedRoots 将 local_path 限制在这些目录内；为空时拒绝所有读取。 + AllowedRoots []string } // FilePuller 执行三步文件拉取流程（ReadFile → Upload → Report）。 @@ -137,6 +144,9 @@ func (p *FilePuller) Execute(ctx context.Context, taskID string, action FilePull return p.reportErr(ctx, taskID, ReasonFileNotFound, "file not found: "+action.LocalPath) } + if errors.Is(readErr, ErrPathDenied) { + return p.reportErr(ctx, taskID, ReasonPathDenied, readErr.Error()) + } return p.reportErr(ctx, taskID, ReasonReadFailed, readErr.Error()) } @@ -191,7 +201,11 @@ func (p *FilePuller) readFile(action FilePullAction) ([]byte, bool, error) { limit = action.MaxSize } - f, err := os.Open(action.LocalPath) + path, err := p.allowedPath(action.LocalPath) + if err != nil { + return nil, false, err + } + f, err := os.Open(path) if err != nil { return nil, false, err } @@ -201,6 +215,9 @@ func (p *FilePuller) readFile(action FilePullAction) ([]byte, bool, error) { if err != nil { return nil, false, err } + if !stat.Mode().IsRegular() { + return nil, false, fmt.Errorf("%w: %s is not a regular file", ErrPathDenied, action.LocalPath) + } size := stat.Size() truncated := false toRead := size @@ -216,6 +233,36 @@ func (p *FilePuller) readFile(action FilePullAction) ([]byte, bool, error) { return buf, truncated, nil } +func (p *FilePuller) allowedPath(path string) (string, error) { + if len(p.cfg.AllowedRoots) == 0 { + return "", fmt.Errorf("%w: no allowed roots configured", ErrPathDenied) + } + target, err := filepath.Abs(filepath.Clean(path)) + if err != nil { + return "", fmt.Errorf("resolve local_path: %w", err) + } + target, err = filepath.EvalSymlinks(target) + if err != nil { + return "", err + } + + for _, configuredRoot := range p.cfg.AllowedRoots { + root, err := filepath.Abs(filepath.Clean(configuredRoot)) + if err != nil { + continue + } + root, err = filepath.EvalSymlinks(root) + if err != nil { + continue + } + rel, err := filepath.Rel(root, target) + if err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return target, nil + } + } + return "", fmt.Errorf("%w: %s is outside allowed roots", ErrPathDenied, path) +} + // readFull 是 io.ReadFull 的内联简化版，避免为单次调用引入额外的包依赖。 func readFull(r interface{ Read([]byte) (int, error) }, buf []byte) (int, error) { total := 0 diff --git a/internal/filepull/puller_test.go b/internal/filepull/puller_test.go index 43d8e09..bc7fe23 100644 --- a/internal/filepull/puller_test.go +++ b/internal/filepull/puller_test.go @@ -134,7 +134,7 @@ func writeTempFile(t *testing.T, dir, name string, content []byte) string { } func newTestPuller(up ArtifactUploader, rep Reporter) *FilePuller { - return NewFilePuller(up, rep, Config{AgentID: 9}, zap.NewNop()) + return NewFilePuller(up, rep, Config{AgentID: 9, AllowedRoots: []string{os.TempDir()}}, zap.NewNop()) } func hexMD5(b []byte) string { @@ -264,8 +264,9 @@ func TestPuller_Truncation_HardCap(t *testing.T) { up := &fakeUploader{} rep := &capturingReporter{} p := NewFilePuller(up, rep, Config{ - AgentID: 9, - HardMaxSize: 1024, + AgentID: 9, + HardMaxSize: 1024, + AllowedRoots: []string{os.TempDir()}, }, zap.NewNop()) if err := p.Execute(context.Background(), "task-5", FilePullAction{ @@ -362,6 +363,43 @@ func TestPuller_EmptyLocalPathRejected(t *testing.T) { } } +func TestPuller_PathBoundary(t *testing.T) { + allowed := t.TempDir() + outside := t.TempDir() + insidePath := writeTempFile(t, allowed, "inside.log", []byte("inside")) + outsidePath := writeTempFile(t, outside, "outside.log", []byte("outside")) + puller := NewFilePuller(&fakeUploader{}, &capturingReporter{}, Config{ + AgentID: 9, AllowedRoots: []string{allowed}, + }, zap.NewNop()) + + if _, _, err := puller.readFile(FilePullAction{LocalPath: insidePath}); err != nil { + t.Fatalf("inside file rejected: %v", err) + } + if _, _, err := puller.readFile(FilePullAction{LocalPath: outsidePath}); !errors.Is(err, ErrPathDenied) { + t.Fatalf("outside file error = %v, want ErrPathDenied", err) + } + if _, _, err := puller.readFile(FilePullAction{LocalPath: allowed}); !errors.Is(err, ErrPathDenied) { + t.Fatalf("directory error = %v, want ErrPathDenied", err) + } + + symlink := filepath.Join(allowed, "escape.log") + if err := os.Symlink(outsidePath, symlink); err != nil { + t.Logf("symlink test skipped: %v", err) + return + } + if _, _, err := puller.readFile(FilePullAction{LocalPath: symlink}); !errors.Is(err, ErrPathDenied) { + t.Fatalf("escaping symlink error = %v, want ErrPathDenied", err) + } +} + +func TestPuller_NoAllowedRootsRejectsRead(t *testing.T) { + path := writeTempFile(t, t.TempDir(), "blocked.log", []byte("blocked")) + puller := NewFilePuller(&fakeUploader{}, &capturingReporter{}, Config{AgentID: 9}, zap.NewNop()) + if _, _, err := puller.readFile(FilePullAction{LocalPath: path}); !errors.Is(err, ErrPathDenied) { + t.Fatalf("readFile error = %v, want ErrPathDenied", err) + } +} + func TestPuller_SourceOverride(t *testing.T) { dir := t.TempDir() path := writeTempFile(t, dir, "a.log", []byte("z")) -- Gitee