Repository navigation
fix(hitl): prevent tool execution when the TypeSafe audit response is incomplete - #394
Open
EddieMurphy89 wants to merge 1 commit into
Open
EddieMurphy89 wants to merge 1 commit into
EddieMurphy89 wants to merge 1 commit into
Conversation
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
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
When HITL is enabled with
default_reviewer: audit_agentandaudit_backend: typesafe, an HTTP 200 response with an empty or partialanswersobject is currently treated as a successful low-risk evaluation.SystemOneconverts nil answers into an empty map,Noulreturns 0 for every missing or non-numeric answer, andDecideJevapplies its "uncertain → approve" default, so the HITL middleware executes the tool. This PR validates every requested answer before accepting the result and makesDecideJevreject an empty or partial result, so an incomplete evaluation fails closed.Impact
What the bypass gives the attacker
exec(arbitrary shell commands as the server account), file writes, webshell/C2 tools, and any other tool outside the configured whitelist.hitl_interruptsrecord attributes the decision to the audit agent with "no destructive rule matched; default allow". It looks like a normal low-risk approval.default_reviewer: human, are not affected.Affected versions
Tested on the default branch at
b43bdb991c3bee4f433cd1a0a370a90c4e24241e(2026-11-08,v1.7.24-44-gb43bdb99) and on release tagv1.7.24at4ca8b7d33663cd4ba8213f4afa8194ae1928ef16(2026-10-16). The vulnerable code is present in both. Modulecyberstrike-ai,go 1.25.0.Reproduction
Control (valid destructive answer):
default_reviewer: audit_agent,audit_backend: typesafe, andhitl.audit_model.base_urlpointing at a local test server.exectool through/api/eino-agentwith a session HITL approval.destroys_availability.noul = 0.97,irreversible_data_loss.noul = 0.97, etc.). Expected and observed: the audit agent rejects, the tool does not run, and no marker file is created.Fault (empty or partial answer):
{"answers":{}}(or only one answer key, for example
{"answers":{"destroys_availability":{"noul":0.02}}}).2. Run the same
execcall.3. Baseline (unpatched): the audit agent approves, the tool runs, and the marker file is created. Patched: the audit agent rejects with
audit agent: TypeSafe 调用失败,保守拒绝, no marker file is created, andmcpExecutionIdsis empty.Expected difference: only the
answerspayload differs; all other inputs are identical.Fix
internal/typesafe/client.goaddsResult.Validate(questions). It requires a usable answer for every question that was sent: each key must be present,noulvalues must be numeric in[0,1], and choice answers must have a non-emptychoiceand a numericconfidence.SystemOnenow returns an error instead of aResultfor an empty, partial, or otherwise unusable 200 response, and the nil-answers normalization is removed.internal/hitl/jev.gocalls the same validation for the built-in decision questions before applying the policy, so an empty or partial result constructed outsideSystemOnealso rejects instead of falling into "uncertain → approve".internal/handler/hitl_audit_agent.goalready converts a TypeSafe error into a conservative reject and logs a warning, so no handler change is needed. Complete low-risk and destructive answer sets keep their previous behavior.Testing
go test ./internal/typesafe ./internal/hitl ./internal/handler— pass.go test -race ./internal/typesafe ./internal/hitl ./internal/handler— pass.go test ./internal/config ./internal/app— pass.go vet ./...— pass;go build ./...— pass;gofmtclean (the repository has no lint target).v1.7.24and pass with the patch: empty answers, partial answers, non-numericnoul, choice without confidence,DecideJevempty/partial/missing-value, and the audit handler's empty/partial cases.exec), a local fake TypeSafe endpoint and a local OpenAI-compatible stub:{"answers":{}}and partial answers → approve,execruns, marker created.mcpExecutionIdsempty.I may be missing context here — happy to adjust the approach if the project prefers a different fix. Thanks for taking a look.
Fixes #393