fix: critical security and concurrency fixes (C1-H16, Phase 1-3) - #165
Merged
Conversation
The Bash tool checked sandbox.Available() before calling WrapCommand. When no backend was available but a mode was configured, the command fell through to unsandboxed host execution — contradicting the documented 'fail closed' promise. C1: Foreground Bash — remove Available() guard, always call WrapCommand, propagate its error as fail-closed. C2: Background Bash — apply WrapCommand before startBackgroundBash, which now accepts the wrapped execName/execArgs. C3: PowerShell — add ModeFromContext + WrapCommand, fail-closed. Tests: bash_sandbox_test.go + powershell_test.go cover fail-closed for ModeWorkspace/ModeStrict, no-regression for ModeOff, and sandbox wrapping for background bash.
Three resource leaks in mission mode are fixed:
C4: Worktree leak on mission cancellation. The cleanup defer used the
mission context, which was already cancelled when the mission was
aborted — so 'git worktree remove' was killed before it could run.
Now uses a detached context (context.Background + 30s timeout) via
removeWorktreeDetached.
C5: Temp-dir leak when git worktree add fails. createWorktree calls
mktemp -d then git worktree add; if git failed, the temp dir was
never removed. Now calls os.RemoveAll before returning the error.
C6: Mission temp dirs never cleaned up. Every mission run created
/tmp/hawk-missions/{ID}/ with no cleanup. Added Mission.Cleanup()
and wired it into cmd/mission.go via defer.
Also: removeWorktree now does best-effort os.RemoveAll on the
directory itself if git worktree remove fails, so the mktemp dir
doesn't leak even when git metadata is already gone.
Tests: worker_cleanup_test.go covers all three fixes.
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
Implements the full audit remediation plan for the deep code review: 8 CRITICAL, 16 HIGH, and selected MEDIUM findings. Closes the sandbox fail-open gap, fixes concurrency/data-loss bugs in sessions and background agents, hardens permission bypass and Guardian injection, adds daemon rate limiting and the documented
/v1/cancelendpoint, and fixes a dead self-improvement loop.Changes
Stop()cancellation (C8);Finalizenow populatesToolsUsed/FilesChangedso the self-improvement loop actually triggers (H1); emergency-compact before overflow retry (H3); structural truncation of tool output (M)ValidateToolInputwired intoRegistry.Executewith explicit required fields (H5); sandbox fail-closed already landed incf16250/9efd879(C1-C6)<tool_data>structural isolation + 0.8 threshold instead of blocklist sanitizer (H7);Bash:git *auto-allow narrowed to safe git subcommands (M)POST /v1/cancelimplemented + documented in openapi/threat model (H9/H10)sk-ant-apikeys) (H15); telemetry now opt-in only (M)Testing
go test -race ./...— all packages passgo vet ./...,golangci-lint run ./...— cleangovulncheck ./...— no vulnerabilities in called code