From 0755df499837cf13c5f0bab0b8f7f7d1de136c4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Ho=C3=A0ng=20Th=E1=BB=A9c?= Date: Mon, 23 Mar 2026 23:26:01 +0700 Subject: [PATCH] fix(test): align TestResolveMediaPath with effectiveRestrict hardening (#381) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues fixed: 1. effectiveRestrict() now always returns true (multi-tenant security), so the "unrestricted" test case expected wrong results — paths outside workspace are now blocked regardless of the restrict parameter. 2. Tests used bare Unix paths (/etc/passwd) which are not absolute on Windows (no drive letter), causing them to resolve inside the workspace and pass incorrectly. Added outsidePath() helper that prepends the volume name for cross-platform absolute paths. --- internal/tools/message_test.go | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/internal/tools/message_test.go b/internal/tools/message_test.go index 9f8c19a0..a7c79b78 100644 --- a/internal/tools/message_test.go +++ b/internal/tools/message_test.go @@ -7,6 +7,15 @@ import ( "testing" ) +// outsidePath returns an absolute path that is guaranteed to be outside the +// given workspace and temp directories on any OS. On Windows bare "/etc/..." +// is relative (no drive letter), so we prepend the volume name of the workspace +// to ensure filepath.IsAbs returns true. +func outsidePath(workspace, segments string) string { + vol := filepath.VolumeName(workspace) + return filepath.Join(vol+string(filepath.Separator), segments) +} + func TestResolveMediaPath(t *testing.T) { tmpDir := os.TempDir() @@ -52,7 +61,7 @@ func TestResolveMediaPath(t *testing.T) { {"just MEDIA", "MEDIA", "", false}, // Outside workspace + outside /tmp/ → blocked - {"outside workspace", "MEDIA:/etc/passwd", "", false}, + {"outside workspace", "MEDIA:" + outsidePath(workspaceCanonical, "etc/passwd"), "", false}, {"traversal attack", "MEDIA:" + filepath.Join(workspaceCanonical, "..", "etc", "passwd"), "", false}, } @@ -69,8 +78,10 @@ func TestResolveMediaPath(t *testing.T) { } }) - t.Run("unrestricted", func(t *testing.T) { - tool := NewMessageTool(workspace, false) + // effectiveRestrict() always returns true (multi-tenant security hardening), + // so even tools created with restrict=false behave as restricted. + t.Run("unrestricted_tool_still_restricted", func(t *testing.T) { + tool := NewMessageTool(workspaceCanonical, false) ctx := context.Background() tests := []struct { @@ -78,8 +89,11 @@ func TestResolveMediaPath(t *testing.T) { input string wantOK bool }{ - {"any absolute path", "MEDIA:/etc/hostname", true}, + // Outside workspace → blocked (effectiveRestrict overrides to true) + {"absolute outside workspace", "MEDIA:" + outsidePath(workspaceCanonical, "etc/hostname"), false}, + // Workspace-relative → allowed {"workspace relative", "MEDIA:docs/report.pdf", true}, + // /tmp/ → allowed (temp dir exception in restricted mode) {"temp file", "MEDIA:" + filepath.Join(tmpDir, "test.png"), true}, } @@ -118,7 +132,7 @@ func TestIsInTempDir(t *testing.T) { {"in tmp", filepath.Join(tmpDir, "test.png"), true}, {"nested in tmp", filepath.Join(tmpDir, "sub", "file.txt"), true}, {"tmp itself", tmpDir, false}, // only files inside, not the dir itself - {"outside tmp", "/etc/passwd", false}, + {"outside tmp", outsidePath(tmpDir, "etc/passwd"), false}, {"relative path", "relative/path.txt", false}, {"traversal", filepath.Join(tmpDir, "..", "etc", "passwd"), false}, }