diff --git a/cmd/okf/mcp.go b/cmd/okf/mcp.go index 85de080..d5e81fe 100644 --- a/cmd/okf/mcp.go +++ b/cmd/okf/mcp.go @@ -295,7 +295,8 @@ func (s *mcpServer) resolveBundleDir(callParams mcpToolCallParams) (string, erro } var absTarget string - if filepath.IsAbs(normTarget) { + isAbs := filepath.IsAbs(target) || strings.HasPrefix(normTarget, "/") || (len(normTarget) >= 2 && (normTarget[0] >= 'a' && normTarget[0] <= 'z' || normTarget[0] >= 'A' && normTarget[0] <= 'Z') && normTarget[1] == ':') + if isAbs { absTarget = normTarget } else { absTarget = filepath.Join(s.rootDir, filepath.FromSlash(normTarget)) diff --git a/cmd/okf/mcp_test.go b/cmd/okf/mcp_test.go index 9f81d73..9f49c5d 100644 --- a/cmd/okf/mcp_test.go +++ b/cmd/okf/mcp_test.go @@ -115,6 +115,35 @@ func TestMCPToolsListOutputSchemas(t *testing.T) { } } +func TestMCPBundle_CrossPlatformAbsoluteEscape(t *testing.T) { + tmpDir := t.TempDir() + serverRoot := filepath.Join(tmpDir, "workspace") + _ = os.MkdirAll(serverRoot, 0o755) + + inputs := []string{ + // Windows-style absolute paths (escaping workspace root) + `{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"okf_search","arguments":{"bundle":"C:\\Windows\\System32","query":"test"}}}`, + `{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"okf_search","arguments":{"bundle":"D:\\secret","query":"test"}}}`, + `{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"okf_search","arguments":{"bundle":"\\etc\\passwd","query":"test"}}}`, + `{"jsonrpc":"2.0","id":4,"method":"tools/call","params":{"name":"okf_search","arguments":{"bundle":"/etc/passwd","query":"test"}}}`, + } + + responses := runMCPConversation(t, serverRoot, inputs) + if len(responses) != 4 { + t.Fatalf("Expected 4 responses, got %d", len(responses)) + } + + for i, r := range responses { + rMap, ok := r.Result.(map[string]any) + if !ok { + t.Fatalf("Response %d result type invalid: %T", i+1, r.Result) + } + if isErr, _ := rMap["isError"].(bool); !isErr { + t.Errorf("Expected response %d to return isError: true (path traversal denied), got: %+v", i+1, rMap) + } + } +} + func TestMCPOutputSchemasProperties(t *testing.T) { byName := map[string]map[string]any{} for _, tool := range getMCPTools() { diff --git a/pkg/okf/bundle.go b/pkg/okf/bundle.go index c2f2a14..bb575b8 100644 --- a/pkg/okf/bundle.go +++ b/pkg/okf/bundle.go @@ -53,7 +53,10 @@ func ensureWithinRoot(rootDir, targetPath string) (string, error) { } cleanTarget := strings.ReplaceAll(targetPath, "\\", "/") - if filepath.IsAbs(targetPath) { + + isAbs := filepath.IsAbs(targetPath) || strings.HasPrefix(cleanTarget, "/") || (len(cleanTarget) >= 2 && (cleanTarget[0] >= 'a' && cleanTarget[0] <= 'z' || cleanTarget[0] >= 'A' && cleanTarget[0] <= 'Z') && cleanTarget[1] == ':') + + if isAbs { cleanTarget = filepath.Clean(cleanTarget) } else { cleanTarget = path.Join(filepath.ToSlash(realRoot), cleanTarget) diff --git a/pkg/okf/mutate_security_test.go b/pkg/okf/mutate_security_test.go index 1f7358c..20a0936 100644 --- a/pkg/okf/mutate_security_test.go +++ b/pkg/okf/mutate_security_test.go @@ -55,6 +55,29 @@ func TestSaveConceptRejectsTraversal(t *testing.T) { } } +// TestEnsureWithinRootCrossPlatformAbsolute checks that Windows-style absolute paths (e.g. C:\) +// are correctly identified and rejected on POSIX systems instead of being treated as relative paths. +func TestEnsureWithinRootCrossPlatformAbsolute(t *testing.T) { + root := t.TempDir() + bundleDir := filepath.Join(root, "knowledge") + if err := InitBundle(bundleDir); err != nil { + t.Fatalf("InitBundle: %v", err) + } + + absolutePaths := []string{ + `C:\Windows\System32\evil.md`, + `D:\secret.md`, + `/etc/passwd`, + `\Windows\System32`, + } + + for _, p := range absolutePaths { + if _, err := ensureWithinRoot(bundleDir, p); err == nil { + t.Errorf("ensureWithinRoot(%q): expected path traversal error for cross-platform absolute path, got nil", p) + } + } +} + // TestSaveConceptAllowsNestedConcept verifies the containment check does not // reject legitimate nested concept paths, and that bookkeeping stays inside // the bundle.