Skip to content

fix(audit): resolve HIGH+H5-M20+M2-M20 findings across hawk - #170

Merged
Patel230 merged 12 commits into
mainfrom
feat/code-audit-improvements
Aug 3, 2026
Merged

fix(audit): resolve HIGH+H5-M20+M2-M20 findings across hawk#170
Patel230 merged 12 commits into
mainfrom
feat/code-audit-improvements

Conversation

@Patel230

@Patel230 Patel230 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Comprehensive audit remediation on feat/code-audit-improvements (built green against eyrie fix #105).

Security:

  • H1-H7 (audit-critical): fail-open gaps in sandbox/ApprovalGate, secrets-in-config rejection, symlink/symlink-escape guards

HIGH:

  • H8-H10: sandbox root-fs hardening (read-only, no-new-privs, cap-drop ALL), per-call regex hoisting, compaction deep-clone → RawMessagesView

MEDIUM:

  • M2-M20: auth secrets, root fs, regex hoisting, transcript clone, prompt cache, startup latency (engine deferral off root preamble), TUI growth caps, hook WaitGroup drain, sandbox fail-closed, retry/overflow classification

LOW:

  • Atomic writes (safewrite with fsync + symlink guard), tolerant WAL/session loader (16MB cap), PID-namespaced temp, daemon session cap (1000, oldest-evict), real session IDs, countdown wiring, ctx-aware EndSession, dead-code deletion, image digest pinning, Retry-After honored on stream retry (capped 60s), tightened context-overflow heuristic (no spurious compact)

Phase-5/6:

  • internal/bench/bench_test.go: headless agent-loop smoke harness (HAWK_BENCH_HEADLESS=1)
  • internal/engine/jsonl_events.go: NDJSON event writer for headless/CI path (codex exec --json parity)
  • Note: MCP server mode (internal/mcp/server.go + cmd/mcp_serve.go) was already wired; audit §6 note corrected

Submodule: external/eyrie pinned to fix/deployment-retry-no-repeat (PR #105 pending merge — pointer bump to follow on merge).

All: go build, go vet, golangci-lint, staticcheck clean; engine + bench + cmd test suites pass.

Patel230 added 10 commits August 3, 2026 07:25
@Patel230
fix(security): close audit-critical fail-open and persistence gaps
314ea8b 
C1: wire panicRecovery + crash.Install into the binary entry point
H5: handle tea.InterruptMsg/tea.QuitMsg/SIGHUP with the shared quit-save path
H3: HTTP decision hooks deny (fail-closed) on error by default; explicit FailOpen opt-in
H4: streamSSE exits on write errors, per-frame write deadline, aborts orphaned sessions
H6: EvolvingMemory loads at construction, persists atomically after Learn
H7: LimitTracker accessors mutex-protected; MaxCostUSD falls back to MaxBudgetUSD; SetCostUSD synced from session cost accumulator
H12: agent-launched subprocesses get scrubbed env (no provider API keys)
H1: .agents/runtime.jsonc deps/env validated; malicious entries dropped with warnings
@Patel230
fix(audit): resolve H8-H10, M6, M9, M14 findings
06b1448 
- H10 engine/async: rewrite lifecycle (Stop joins loop via WaitGroup,
  Start-after-Stop spawns one fresh loop, non-recursive queue drain,
  direct turn cancel, terminal EventDone always emitted + ReplyTo
  forwarding, EventInfo mapping for unmapped stream events, Unsubscribe,
  full UUIDs). 8 tests, 88.2% coverage (was 0%), race-clean.
- H9 mission: retry loop rewrites feature.Branch per attempt
  (<base>/attempt-N), createWorktree falls back to checking out an
  existing branch, worktree cleanup deletes the branch; hawk mission
  now exits non-zero when any feature fails (CI honesty).
- M6: ReadOnlyValidationWorker uses detached-context cleanup.
- H8 codegraph: bounded embedding cache keyed by content hash,
  SemanticSearch memoized (3 new tests).
- M14: hoist per-call regexp.MustCompile to package vars in
  feature/eval, tool/spec_checklist, tool/ticket_compliance,
  feature/fingerprint.
- M9 oteltrace: StartSpan respects Disable() and caps buffer at
  10k spans (dropped spans stay functional). 2 new tests.
- Remove unused getKeys (staticcheck U1000).
@Patel230
refactor(audit): delete dead engine/docs package (H11)
e46d9b2 
2,213 lines with zero importers since f0aa8fd, plus six documented
bugs (truncated multi-line doc comments, OldDoc from new content,
false-positive description machine, nested-paren parse failures,
unwritten ExternalDocs.Cache, dropped generic methods). Wiring it
would ship those bugs as a new user-facing command; deleting keeps
the surface minimal and is fully recoverable from git history.
@Patel230
fix(audit): resolve M2-M5, M7-M8, M10-M13, M15, M17-M20 findings
d11c628 
M2: ParseAndApplyMemoryOps returns error (ErrNoMemoryOps,
wrapped parse/remember, nil-bridge) and call site logs slog.Warn
M3: SkillDistillerAdapter.Retrieve logs on Search error
M4: OnSessionEnd uses real session ID via Session.SessionID()
M5: MissionApprovalGate wired into worker approval path, mutex-safe
M7: MCP server marks dead + kills child on readLoop exit, fail-fast
M8: internal/composio/ deleted (fake-success stub, zero importers)
M10: diffsandbox.absPath component-wise Lstat walk, containment recheck
M11: PolicyManager defaults to deny-by-default, project-precedence load
M12: container --user fallback when userns remap unavailable
M13: file tools EvalSymlinks+SameFile TOCTOU guard, sensitive basenames
M15: hot reads use read-only RawMessagesView (no clone); RawMessages
     keeps its deep-copy contract
M16: no change — incremental render cache + stream tail already O(tail)
M17: MigrateProviderSecrets off root preamble into credential paths
M18: history (200) + messageQueue (100) caps, expansion map reindexed
M19: session end drains async hooks via WaitAsync (30s)
M20: Sandbox.Run fails closed unless tier=off explicit

Also correct stale report rows: batch-1 findings -> fixed, H10 test
count, M9 path. Full test -race + lint + staticcheck green.
@Patel230
fix(audit): pin sandbox image by digest; document LOW resolution stat…
7e93893 
…uses

LOW: sandbox image can now be pinned to an immutable digest via
HAWK_SANDBOX_IMAGE_DIGEST (repo@sha256:<digest>), replacing the mutable-tag
pull. Report LOW section updated with per-item resolution status: fixed,
deferred, or no-change-needed.
@Patel230
fix(audit): drop emoji from print countdown line
4758b10 
TestNoEmojiInCmd forbids emoji in cmd/; use a plain text label instead of
the timer emoji in the RemainingTime notice.
@Patel230
fix(audit): honor Retry-After on stream retry; surface rate-limited s…
0988505 
…treams

LOW: hawk's stream retry used a fixed 1–3s linear delay and ignored any
Retry-After hint from the provider. isRetryableStreamError already covered
rate-limit via retry.IsRetryable, but the delay was never informed by the
server's requested wait.

- streamRetryDelay(err, attempt): parses a "retry in|after N[ms]" hint from
  the error (best-effort regex matching eyrie's retryDelayRe), honors it
  capped at maxStreamRetryDelay (60s), else falls back to the linear backoff
- the stream retry timer now uses streamRetryDelay so a 429/503 with an
  explicit Retry-After waits the requested amount instead of the fixed delay
- added stream_retry_test.go (parse hint, cap, fallback, retryability)
@Patel230
fix(audit): tighten context-overflow classification to avoid spurious…
9b15bdb 
… retry

LOW: isContextOverflow matched a bare "too long" substring, so unrelated API
errors (e.g. "request timeout, too long") spuriously triggered an emergency
context-compact + retry. Match structured provider overflow signals
(context_length_exceeded / context_length_error / "exceeds the limit") and,
for the legacy "too long"/"too many tokens" phrasing, require a token/context
qualifier (tokens/token/limit/context) so plain "too long" text no longer
false-positives. Adds TestIsContextOverflow; existing emergency-compact test
unchanged.
@Patel230
docs(audit): update LOW statuses to reflect code changes
52959d0 
Retry-After honored (stream.go:448), deployment no-repeat fix live in eyrie
submodule branch, and isContextOverflow tightened — report's §3.4 statuses
now match the committed code instead of the original deferred/no-change notes.
@Patel230
feat(bench): add SWE-bench harness and JSONL headless writer
421ecdf 
- internal/bench/bench_test.go: headless agent-loop smoke harness
- internal/engine/jsonl_events.go: NDJSON event writer for headless path
- internal/engine/jsonl_events_test.go: tests
- MCP server mode already wired; audit §6 note was stale
@Patel230
Patel230 force-pushed the feat/code-audit-improvements branch from 92412d6 to 421ecdf Compare August 3, 2026 08:12
@Patel230
Patel230 merged commit 9741f2b into main Aug 3, 2026
23 checks passed
@Patel230
Patel230 deleted the feat/code-audit-improvements branch August 3, 2026 09:07
@Patel230 Patel230 mentioned this pull request Aug 3, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Patel230