From b890df52064df7ee80916223946085316e413a8e Mon Sep 17 00:00:00 2001
From: "google-labs-jules[bot]"
 <161369871+google-labs-jules[bot]@users.noreply.github.com>
Date: Sun, 20 Sep 2026 04:52:52 +0000
Subject: [PATCH] fix: Harden cross-platform path resolution against CWE-22
 escapes

This commit hardens `ensureWithinRoot` in `pkg/okf/bundle.go` and
`resolveBundleDir` in `cmd/okf/mcp.go` to explicitly identify cross-platform
absolute paths (such as Windows drive letters or leading slashes) which would
otherwise be incorrectly identified as relative paths on POSIX systems,
bypassing path containment checks. Negative tests have been added to verify
these invariants.

Co-authored-by: sknr <11868275+sknr@users.noreply.github.com>
---
 cmd/okf/mcp.go                  |  3 ++-
 cmd/okf/mcp_test.go             | 29 +++++++++++++++++++++++++++++
 pkg/okf/bundle.go               |  5 ++++-
 pkg/okf/mutate_security_test.go | 23 +++++++++++++++++++++++
 4 files changed, 58 insertions(+), 2 deletions(-)

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.