fix(hawk): enforce path sandbox guard in patch/structured-edit/smart-create + MCP updates#98
Merged
Conversation
configuredStartupMCPServers only ever read cfg.Command/cfg.Args and skipped any entry without a Command — configuring "type": "http"/"sse"/"websocket" in settings.json had zero effect, since nothing dispatched to mcp.ConnectHTTP/ConnectSSE/ConnectWS from config at all. Even the existing static Headers-based auth mechanism was unreachable as a result. Adds LoadRemoteMCPTools and dispatches on cfg.Type; unifies tool-wrapping across all three transports via a minimal mcpClient interface (CallTool + Close, which already have identical signatures on mcp.Server/HTTPServer/ WSServer) so MCPTool can wrap any of them without changing any of those concrete types. Resource listing (ListResources/ReadResource) stays stdio-only via a type assertion, matching what those transports actually implement today.
…remote servers Hand-rolled to match internal/mcp's existing zero-SDK-dependency style (no mark3labs/mcp-go added as a dependency). internal/mcp/oauth.go: RFC 9728 protected-resource discovery falling back to RFC 8414 authorization-server discovery falling back to the previous guessed defaults; RFC 7591 dynamic client registration when no client_id is configured; PKCE (S256); a from-scratch loopback callback listener (hawk had no redirect-listener pattern before — only the existing device-code/poll flow used for Hawk Cloud login, which doesn't fit MCP's authorization-code+PKCE requirement); token exchange/refresh. internal/mcp/oauth_store.go persists tokens via internal/auth.SecureStorage (the real keychain-backed store — internal/auth.TokenStore looks similar but its Load/Save are literal no-ops). internal/tool/mcp_auth.go completes the previously half-built McpAuthTool: Execute now runs discovery through PKCE and starts the loopback listener, with a background goroutine that awaits the callback, exchanges the code, and saves the token — matching hawk's agent-tool-call UX (returns the authorization URL immediately, picks up asynchronously once the user authorizes). AuthHeaderForMCPServer wires the stored token into the Authorization header the transport-dispatch fix already reads, refreshing first if it's close to expiring.
Add the hawk-mcpkit replace directive so the shared MCP scaffolding resolves as a sibling module, and bump the external/yaad submodule to the mcpkit-migrated commit.
…create PatchTool, StructuredEditTool, and SmartCreateTool wrote or deleted files via LLM-authored paths without calling validatePathAllowed, the guard every other read/write tool uses. An LLM could direct them at paths outside the workspace. Add validatePathAllowed(ctx, path) to each tool's Execute method. Patch validates each parsed patch target in Execute (the only method that has a ctx) before ApplyAll, so the package-level Apply/ApplyAll signatures stay stable. Added regression tests asserting out-of-workspace paths are rejected and in-sandbox paths still succeed.
Advance the three submodule pointers to commits that include the fixes
made in the sibling repos:
eyrie -> f6abb1e delete dead errors package + stop falling back to
OPENAI base URL for non-OpenAI providers (credential
hijack fix)
trace -> 164d86b serialize V2GitStore storer access under StorerMu
(git object corruption race)
yaad -> 7a796a0 migrate MCP server onto hawk-mcpkit (yaad MCPkit
replace path corrected for the external/ layout)
The sibling repos (../eyrie, ../trace, ../yaad) hold the matching
source commits; these pointers make hawk build against the fixed code.
Patel230
force-pushed
the
fix/hawk-sandbox-mcp
branch
from
July 15, 2026 19:20
851a4a5 to
6b5152a
Compare
Patel230
enabled auto-merge (squash)
July 15, 2026 20:58
Patel230
added a commit
that referenced
this pull request
Jul 16, 2026
…create + MCP updates (#98) * fix(mcp): dispatch remote MCP server configs to the correct transport configuredStartupMCPServers only ever read cfg.Command/cfg.Args and skipped any entry without a Command — configuring "type": "http"/"sse"/"websocket" in settings.json had zero effect, since nothing dispatched to mcp.ConnectHTTP/ConnectSSE/ConnectWS from config at all. Even the existing static Headers-based auth mechanism was unreachable as a result. Adds LoadRemoteMCPTools and dispatches on cfg.Type; unifies tool-wrapping across all three transports via a minimal mcpClient interface (CallTool + Close, which already have identical signatures on mcp.Server/HTTPServer/ WSServer) so MCPTool can wrap any of them without changing any of those concrete types. Resource listing (ListResources/ReadResource) stays stdio-only via a type assertion, matching what those transports actually implement today. * feat(mcp): implement OAuth (discovery, DCR, PKCE, token storage) for remote servers Hand-rolled to match internal/mcp's existing zero-SDK-dependency style (no mark3labs/mcp-go added as a dependency). internal/mcp/oauth.go: RFC 9728 protected-resource discovery falling back to RFC 8414 authorization-server discovery falling back to the previous guessed defaults; RFC 7591 dynamic client registration when no client_id is configured; PKCE (S256); a from-scratch loopback callback listener (hawk had no redirect-listener pattern before — only the existing device-code/poll flow used for Hawk Cloud login, which doesn't fit MCP's authorization-code+PKCE requirement); token exchange/refresh. internal/mcp/oauth_store.go persists tokens via internal/auth.SecureStorage (the real keychain-backed store — internal/auth.TokenStore looks similar but its Load/Save are literal no-ops). internal/tool/mcp_auth.go completes the previously half-built McpAuthTool: Execute now runs discovery through PKCE and starts the loopback listener, with a background goroutine that awaits the callback, exchanges the code, and saves the token — matching hawk's agent-tool-call UX (returns the authorization URL immediately, picks up asynchronously once the user authorizes). AuthHeaderForMCPServer wires the stored token into the Authorization header the transport-dispatch fix already reads, refreshing first if it's close to expiring. * chore(hawk): wire hawk-mcpkit into go.work and refresh yaad pin Add the hawk-mcpkit replace directive so the shared MCP scaffolding resolves as a sibling module, and bump the external/yaad submodule to the mcpkit-migrated commit. * fix(hawk): enforce path sandbox guard in patch/structured-edit/smart-create PatchTool, StructuredEditTool, and SmartCreateTool wrote or deleted files via LLM-authored paths without calling validatePathAllowed, the guard every other read/write tool uses. An LLM could direct them at paths outside the workspace. Add validatePathAllowed(ctx, path) to each tool's Execute method. Patch validates each parsed patch target in Execute (the only method that has a ctx) before ApplyAll, so the package-level Apply/ApplyAll signatures stay stable. Added regression tests asserting out-of-workspace paths are rejected and in-sandbox paths still succeed. * chawk: sync external/eyrie, external/trace, external/yaad to fixes Advance the three submodule pointers to commits that include the fixes made in the sibling repos: eyrie -> f6abb1e delete dead errors package + stop falling back to OPENAI base URL for non-OpenAI providers (credential hijack fix) trace -> 164d86b serialize V2GitStore storer access under StorerMu (git object corruption race) yaad -> 7a796a0 migrate MCP server onto hawk-mcpkit (yaad MCPkit replace path corrected for the external/ layout) The sibling repos (../eyrie, ../trace, ../yaad) hold the matching source commits; these pointers make hawk build against the fixed code. * fix(hawk): gofumpt format + bump submodule parity versions * fix(hawk): bump yaad resolve to c6c5b04 (sibling mcpkit path) * fix(hawk): sync go.sum/work after yaad version bump * fix(hawk): repin eyrie+yaad submodules to squash-merge commits (reachability) * fix(hawk): drop stale go.sum entries for rewritten submodule history * fix(hawk): address lint findings in oauth + patch/mcp_auth
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why
Closes the highest-blast-radius audit finding (OS-level credential/config tampering via unguarded write tools) and brings remote MCP support + dependency wiring current.
Verification