Conversation
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>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
1 participant
Defensive Category: Safe Path Resolution (CWE-22 / CWE-59)
Defensive Rationale & Root Cause Analysis
The path boundary containment logic in
ensureWithinRootandresolveBundleDirpreviously relied solely on the nativefilepath.IsAbs()function. Because this function is platform-dependent, it fails to correctly identify a Windows absolute path (e.g.,C:\Windows\System32or\etc\passwd) when the Go binary is executing on a POSIX operating system. Consequently, the logic would treat the malicious absolute path as a relative path, joining it directly to the bundle root, and potentially enabling an attacker to escape the designated boundary after subsequent normalization.Implemented Hardening & Tests Added
pkg/okf/bundle.goandcmd/okf/mcp.goto explicitly test for drive letters and root slashes on the normalized path structure instead of relying solely onfilepath.IsAbs().TestEnsureWithinRootCrossPlatformAbsoluteinpkg/okf/mutate_security_test.goto verify deterministic rejection of payload vectors on the bundle API level.TestMCPBundle_CrossPlatformAbsoluteEscapeincmd/okf/mcp_test.goto verify the same defensive invariants on the MCP interface level.PR created automatically by Jules for task 15706751150931214226 started by @sknr