diff --git a/cmd/kontext/main.go b/cmd/kontext/main.go index fa515abc..7667d7b6 100644 --- a/cmd/kontext/main.go +++ b/cmd/kontext/main.go @@ -30,6 +30,7 @@ import ( _ "github.com/kontext-security/kontext-cli/internal/agent/claude" _ "github.com/kontext-security/kontext-cli/internal/agent/codex" _ "github.com/kontext-security/kontext-cli/internal/agent/cowork" + _ "github.com/kontext-security/kontext-cli/internal/agent/devin" ) var version = "dev" diff --git a/internal/agent/devin/devin.go b/internal/agent/devin/devin.go new file mode 100644 index 00000000..6c21ea59 --- /dev/null +++ b/internal/agent/devin/devin.go @@ -0,0 +1,24 @@ +// Package devin registers the Devin agent adapter. +package devin + +import ( + "github.com/kontext-security/kontext-cli/internal/agent" + "github.com/kontext-security/kontext-cli/internal/hook" + "github.com/kontext-security/kontext-cli/internal/hookruntime" +) + +func init() { + agent.Register(&Devin{}) +} + +type Devin struct{} + +func (d *Devin) Name() string { return "devin" } + +func (d *Devin) DecodeHookInput(input []byte) (hook.Event, error) { + return hookruntime.DecodeDevinEvent(input, d.Name()) +} + +func (d *Devin) EncodeHookResult(event hook.Event, result hook.Result) ([]byte, error) { + return hookruntime.EncodeDevinResult(event.HookName.String(), result) +} diff --git a/internal/agent/devin/devin_test.go b/internal/agent/devin/devin_test.go new file mode 100644 index 00000000..7e758bdf --- /dev/null +++ b/internal/agent/devin/devin_test.go @@ -0,0 +1,59 @@ +package devin + +import ( + "errors" + "testing" + + "github.com/kontext-security/kontext-cli/internal/agent" + "github.com/kontext-security/kontext-cli/internal/hook" +) + +func TestDevinRegisteredUnderItsName(t *testing.T) { + t.Parallel() + + registered, ok := agent.Get("devin") + if !ok { + t.Fatal("agent.Get(\"devin\") not found, want the adapter registered") + } + if registered.Name() != "devin" { + t.Fatalf("Name() = %q, want devin", registered.Name()) + } +} + +func TestDecodeHookInputStampsAgentName(t *testing.T) { + t.Parallel() + + event, err := (&Devin{}).DecodeHookInput([]byte( + `{"hook_event_name":"PreToolUse","tool_name":"exec","tool_input":{"command":"ls"}}`, + )) + if err != nil { + t.Fatalf("DecodeHookInput() error = %v", err) + } + if event.Agent != "devin" { + t.Fatalf("Agent = %q, want devin", event.Agent) + } +} + +func TestDecodeHookInputReportsSkipForEventsWithoutDecisions(t *testing.T) { + t.Parallel() + + _, err := (&Devin{}).DecodeHookInput([]byte(`{"hook_event_name":"PostCompaction"}`)) + if !errors.Is(err, hook.ErrSkipEvent) { + t.Fatalf("DecodeHookInput() error = %v, want ErrSkipEvent", err) + } +} + +func TestEncodeHookResultUsesTheEventOnTheEvent(t *testing.T) { + t.Parallel() + + out, err := (&Devin{}).EncodeHookResult( + hook.Event{HookName: hook.HookPreToolUse}, + hook.Result{Decision: hook.DecisionDeny, Reason: "denied by policy"}, + ) + if err != nil { + t.Fatalf("EncodeHookResult() error = %v", err) + } + if want := `{"decision":"block","reason":"denied by policy"}`; string(out) != want { + t.Fatalf("EncodeHookResult() = %s, want %s", out, want) + } +} diff --git a/internal/guard/hookruntime/runtime.go b/internal/guard/hookruntime/runtime.go index b794abed..802c9c96 100644 --- a/internal/guard/hookruntime/runtime.go +++ b/internal/guard/hookruntime/runtime.go @@ -2,6 +2,7 @@ package hookruntime import ( "context" + "errors" "fmt" "io" "strings" @@ -34,6 +35,12 @@ type Processor interface { func Run(ctx context.Context, adapter Adapter, processor Processor, mode Mode, stdin io.Reader, stdout, stderr io.Writer) error { event, err := adapter.Decode(stdin) if err != nil { + // An event the adapter recognizes but does not translate carries no + // decision: emit nothing and succeed, rather than encoding a deny for + // what is only a lifecycle signal. + if errors.Is(err, hook.ErrSkipEvent) { + return nil + } fmt.Fprintf(stderr, "kontext: malformed hook input: %v\n", err) malformed := hook.Event{HookName: adapter.MalformedHookName()} return adapter.Encode(stdout, malformed, effectiveResult(malformed, hook.Result{ diff --git a/internal/guard/hookruntime/runtime_test.go b/internal/guard/hookruntime/runtime_test.go index 86771e5a..239d8fc0 100644 --- a/internal/guard/hookruntime/runtime_test.go +++ b/internal/guard/hookruntime/runtime_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "errors" + "fmt" "io" "testing" @@ -183,3 +184,41 @@ func TestEffectiveResultRemoteDowngradesUnstampedToObserve(t *testing.T) { t.Fatalf("reason = %q, want observe would-note", result.Reason) } } + +// An event the adapter recognizes but does not translate carries no decision. +// It must not be encoded as a deny the way malformed input is, and it must not +// reach the processor. +func TestRunSkippedEventEncodesNothing(t *testing.T) { + t.Parallel() + + adapter := &stubAdapter{ + decodeErr: fmt.Errorf("devin: event PostCompaction: %w", hook.ErrSkipEvent), + } + stdout := &bytes.Buffer{} + + err := Run(context.Background(), adapter, stubProcessor{err: errors.New("must not be called")}, ModeEnforce, bytes.NewReader(nil), stdout, io.Discard) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + if adapter.result.Decision != "" { + t.Fatalf("encoded decision = %q, want nothing encoded", adapter.result.Decision) + } + if stdout.String() != "" { + t.Fatalf("stdout = %q, want empty", stdout.String()) + } +} + +// Malformed input keeps its existing behavior: a policy-controlled result is +// still encoded, so this must not be swept into the skip path. +func TestRunMalformedInputStillEncodesDenyInEnforce(t *testing.T) { + t.Parallel() + + adapter := &stubAdapter{decodeErr: errors.New("malformed")} + err := Run(context.Background(), adapter, stubProcessor{}, ModeEnforce, bytes.NewReader(nil), io.Discard, io.Discard) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + if adapter.result.Decision != hook.DecisionDeny { + t.Fatalf("decision = %q, want deny", adapter.result.Decision) + } +} diff --git a/internal/guard/store/sqlite/store.go b/internal/guard/store/sqlite/store.go index 475c5bd0..11d5f799 100644 --- a/internal/guard/store/sqlite/store.go +++ b/internal/guard/store/sqlite/store.go @@ -1755,12 +1755,18 @@ func scanSession(scanner interface{ Scan(...any) error }) (SessionRecord, error) return record, nil } +// hostedAgentIdentity maps a local agent name to the (provider, agent) pair the +// hosted ledger records. An unmapped agent still exports its own name, but with +// an empty provider, which reads downstream as "provider unknown" rather than as +// the vendor it came from. func hostedAgentIdentity(agent string) (string, string) { switch strings.ToLower(strings.TrimSpace(agent)) { case "claude", "claude-code", "claude_code": return "anthropic", "claude_code" case "cowork", "claude-cowork", "claude_cowork": return "anthropic", "claude_cowork" + case "devin": + return "cognition", "devin" default: return "", strings.TrimSpace(agent) } diff --git a/internal/guard/store/sqlite/store_test.go b/internal/guard/store/sqlite/store_test.go index dac78e4c..3be90951 100644 --- a/internal/guard/store/sqlite/store_test.go +++ b/internal/guard/store/sqlite/store_test.go @@ -1862,3 +1862,28 @@ func assertRecordIDs(t *testing.T, records []LedgerRecord, want map[string]bool) } } } + +// The hosted ledger records a (provider, agent) pair per session. An unmapped +// agent exports an empty provider, which reads downstream as "provider unknown" +// rather than as the vendor it came from. +func TestHostedAgentIdentityMapsKnownAgents(t *testing.T) { + t.Parallel() + + for name, want := range map[string]struct{ provider, agent string }{ + "claude": {"anthropic", "claude_code"}, + "claude-code": {"anthropic", "claude_code"}, + "cowork": {"anthropic", "claude_cowork"}, + "devin": {"cognition", "devin"}, + " DeVin ": {"cognition", "devin"}, + "someoneelse": {"", "someoneelse"}, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + provider, agent := hostedAgentIdentity(name) + if provider != want.provider || agent != want.agent { + t.Fatalf("hostedAgentIdentity(%q) = (%q, %q), want (%q, %q)", name, provider, agent, want.provider, want.agent) + } + }) + } +} diff --git a/internal/hook/domain.go b/internal/hook/domain.go index 5798b1f4..28ecb34f 100644 --- a/internal/hook/domain.go +++ b/internal/hook/domain.go @@ -1,6 +1,20 @@ package hook -import "strings" +import ( + "errors" + "strings" +) + +// ErrSkipEvent reports a hook event an adapter recognizes but deliberately does +// not translate — a lifecycle signal that carries no authorization decision and +// must not be recorded as one. +// +// Adapters wrap it; hook runners test for it with errors.Is and exit +// successfully without writing a decision. It exists because some agent +// runtimes read a non-zero hook exit as "block the tool call", so reporting an +// unhandled event as a decode failure would stall the agent on an event we +// simply chose not to handle. +var ErrSkipEvent = errors.New("hook event skipped by adapter") type HookName string diff --git a/internal/hookruntime/devin.go b/internal/hookruntime/devin.go new file mode 100644 index 00000000..904159f5 --- /dev/null +++ b/internal/hookruntime/devin.go @@ -0,0 +1,157 @@ +package hookruntime + +import ( + "bytes" + "encoding/json" + "fmt" + + "github.com/kontext-security/kontext-cli/internal/hook" +) + +// devinDecisionBlock is the decision verb the Devin runtime understands on a +// pre-tool-use hook's stdout. Devin surfaces the accompanying reason to the +// agent verbatim, so reasons double as policy messaging. +const devinDecisionBlock = "block" + +// devinHookInput mirrors the payload the Devin runtime writes to a plugin +// hook's stdin. Devin emits snake_case only, so unlike the Claude Code adapter +// there are no camelCase aliases to tolerate. +// +// Fields Devin sends that we deliberately do not decode: +// +// - prompt_id: a per-turn correlator with no home on hook.Event yet. It is +// deliberately NOT folded into ToolInput, because ToolInput becomes the +// Cedar evaluation context; injecting a synthetic attribute there would +// make it policy-visible and would change the parameters hash recorded on +// every decision. +// - summary: only present on the compaction event, which is skipped outright. +// It carries verbatim conversation content and must not reach the ledger +// without going through payload capture. +type devinHookInput struct { + HookEventName string `json:"hook_event_name"` + SessionID string `json:"session_id"` + ToolName string `json:"tool_name"` + ToolInput json.RawMessage `json:"tool_input"` + ToolResponse json.RawMessage `json:"tool_response"` + ToolUseID string `json:"tool_use_id"` +} + +// devinHookOutput is the decision object Devin parses from hook stdout. An +// empty object is the "no directive, continue" form. +type devinHookOutput struct { + Decision string `json:"decision,omitempty"` + Reason string `json:"reason,omitempty"` +} + +// DecodeDevinEvent translates a Devin hook payload into a hook.Event. +// +// Events Devin emits that carry no authorization decision — and any event name +// this adapter does not recognize — are reported as hook.ErrSkipEvent rather +// than as a decode failure. That distinction matters: the Devin runtime treats +// a non-zero hook exit as "block the tool call", so failing on an event we +// simply do not handle would stall the agent on a legitimate lifecycle signal. +// +// A payload that is malformed or missing its event name is still an error. That +// indicates a bug or a contract change rather than an event we chose not to +// handle, and failing closed on it is the safer default. +func DecodeDevinEvent(input []byte, agentName string) (hook.Event, error) { + var h devinHookInput + if err := decodeUseNumber(input, &h); err != nil { + return hook.Event{}, fmt.Errorf("devin: decode hook input: %w", err) + } + if h.HookEventName == "" { + return hook.Event{}, fmt.Errorf("devin: hook event name missing") + } + + hookName := hook.HookName(h.HookEventName) + if !devinSupportedHook(hookName) { + return hook.Event{}, fmt.Errorf("devin: event %s: %w", h.HookEventName, hook.ErrSkipEvent) + } + + toolInput, err := normalizeDevinToolInput(h.ToolInput) + if err != nil { + return hook.Event{}, err + } + return hook.Event{ + // Devin session IDs are already namespaced (devin-), so unlike + // the Codex adapter there is no prefix to add and no synthetic ID to + // mint — session_id and tool_use_id map straight through. + SessionID: h.SessionID, + Agent: agentName, + HookName: hookName, + ToolName: h.ToolName, + ToolInput: toolInput, + ToolResponse: normalizeToolResponse(h.ToolResponse), + ToolUseID: h.ToolUseID, + }, nil +} + +// EncodeDevinResult renders a decision in the form Devin parses from stdout. +// +// Only pre-tool-use carries a decision; every other event is observation and +// must emit no directive. Anything other than an explicit allow blocks, so an +// unrecognized decision value fails closed rather than letting a tool call +// through. +func EncodeDevinResult(hookEventName string, result hook.Result) ([]byte, error) { + if hook.HookName(hookEventName) != hook.HookPreToolUse { + return json.Marshal(devinHookOutput{}) + } + + // Devin applies an updated tool input by REPLACING the original wholesale + // rather than merging it. Emitting a partial rewrite would silently drop + // every field the policy did not restate — turning a redaction into an + // arbitrary change to the agent's command — and the post-tool-use hook + // would then observe only the rewritten input, so the original would never + // reach the ledger. Until input rewriting is modelled end to end, refuse + // visibly instead of rewriting wrongly. + if result.UpdatedInput != nil { + return json.Marshal(devinHookOutput{ + Decision: devinDecisionBlock, + Reason: "Kontext: policy requested an input rewrite, which this adapter does not support; refusing instead of altering the request.", + }) + } + + if result.Decision != hook.DecisionAllow { + return json.Marshal(devinHookOutput{ + Decision: devinDecisionBlock, + Reason: result.ClaudeReason(), + }) + } + return json.Marshal(devinHookOutput{}) +} + +// devinSupportedHook reports the Devin events this adapter translates. Devin +// also emits compaction and permission-request events; both are skipped, as is +// any event name added in future, so an unhandled event never blocks a tool +// call. +func devinSupportedHook(hookName hook.HookName) bool { + switch hookName { + case hook.HookSessionStart, + hook.HookPreToolUse, + hook.HookPostToolUse, + hook.HookSessionEnd: + return true + default: + return false + } +} + +// normalizeDevinToolInput decodes tool_input into the map the rest of the +// pipeline expects. An absent or null input yields nil. A non-object input is +// not expected from Devin; it is wrapped rather than rejected so an unforeseen +// shape degrades to a recordable event instead of blocking the agent. +func normalizeDevinToolInput(raw json.RawMessage) (map[string]any, error) { + trimmed := bytes.TrimSpace(raw) + if len(trimmed) == 0 || string(trimmed) == "null" { + return nil, nil + } + var obj map[string]any + if err := decodeUseNumber(trimmed, &obj); err == nil { + return obj, nil + } + var value any + if err := decodeUseNumber(trimmed, &value); err != nil { + return nil, fmt.Errorf("devin: decode tool input: %w", err) + } + return map[string]any{"value": value}, nil +} diff --git a/internal/hookruntime/devin_test.go b/internal/hookruntime/devin_test.go new file mode 100644 index 00000000..b6bc65f9 --- /dev/null +++ b/internal/hookruntime/devin_test.go @@ -0,0 +1,298 @@ +package hookruntime + +import ( + "encoding/json" + "errors" + "strings" + "testing" + + "github.com/kontext-security/kontext-cli/internal/hook" +) + +func TestDecodeDevinEventMapsPreToolUsePayload(t *testing.T) { + t.Parallel() + + input := []byte(`{ + "hook_event_name": "PreToolUse", + "session_id": "devin-02714895131a4094893ce70889e3f2b6", + "prompt_id": "prompt-1", + "tool_name": "exec", + "tool_use_id": "toolu_9", + "tool_input": {"command": "curl https://example.com"} + }`) + + event, err := DecodeDevinEvent(input, "devin") + if err != nil { + t.Fatalf("DecodeDevinEvent() error = %v", err) + } + if event.HookName != hook.HookPreToolUse { + t.Fatalf("HookName = %q, want PreToolUse", event.HookName) + } + // Devin session IDs arrive already namespaced; the adapter must not add a + // prefix or mint a replacement. + if event.SessionID != "devin-02714895131a4094893ce70889e3f2b6" { + t.Fatalf("SessionID = %q, want the payload value verbatim", event.SessionID) + } + if event.ToolName != "exec" { + t.Fatalf("ToolName = %q, want exec", event.ToolName) + } + if event.ToolUseID != "toolu_9" { + t.Fatalf("ToolUseID = %q, want toolu_9", event.ToolUseID) + } + if event.Agent != "devin" { + t.Fatalf("Agent = %q, want devin", event.Agent) + } + if got := event.ToolInput["command"]; got != "curl https://example.com" { + t.Fatalf("ToolInput[command] = %v, want the command", got) + } +} + +// prompt_id must not leak into ToolInput: ToolInput becomes the Cedar +// evaluation context, so a synthetic attribute there would be policy-visible +// and would change the recorded parameters hash. +func TestDecodeDevinEventKeepsPromptIDOutOfToolInput(t *testing.T) { + t.Parallel() + + event, err := DecodeDevinEvent([]byte( + `{"hook_event_name":"PreToolUse","prompt_id":"prompt-1","tool_name":"exec","tool_input":{"command":"ls"}}`, + ), "devin") + if err != nil { + t.Fatalf("DecodeDevinEvent() error = %v", err) + } + if len(event.ToolInput) != 1 { + t.Fatalf("ToolInput = %v, want only the original tool input keys", event.ToolInput) + } + if _, present := event.ToolInput["prompt_id"]; present { + t.Fatal("ToolInput contains prompt_id, want it excluded from the Cedar context") + } +} + +func TestDecodeDevinEventCapturesToolResponse(t *testing.T) { + t.Parallel() + + event, err := DecodeDevinEvent([]byte( + `{"hook_event_name":"PostToolUse","tool_name":"exec","tool_input":{},"tool_response":{"success":false,"error":"boom"}}`, + ), "devin") + if err != nil { + t.Fatalf("DecodeDevinEvent() error = %v", err) + } + if event.ToolResponse["success"] != false { + t.Fatalf("ToolResponse[success] = %v, want false", event.ToolResponse["success"]) + } + if event.ToolResponse["error"] != "boom" { + t.Fatalf("ToolResponse[error] = %v, want boom", event.ToolResponse["error"]) + } +} + +func TestDecodeDevinEventPreservesLargeIntegerPrecision(t *testing.T) { + t.Parallel() + + event, err := DecodeDevinEvent([]byte( + `{"hook_event_name":"PreToolUse","tool_name":"exec","tool_input":{"id":12345678901234567890}}`, + ), "devin") + if err != nil { + t.Fatalf("DecodeDevinEvent() error = %v", err) + } + number, ok := event.ToolInput["id"].(json.Number) + if !ok { + t.Fatalf("ToolInput[id] = %T, want json.Number", event.ToolInput["id"]) + } + if number.String() != "12345678901234567890" { + t.Fatalf("ToolInput[id] = %s, want the exact literal", number) + } +} + +// Events Devin emits that carry no decision, and any event name added later, +// must report ErrSkipEvent. Reporting a decode failure would exit non-zero, +// which the Devin runtime reads as "block the tool call". +func TestDecodeDevinEventSkipsEventsWithoutDecisions(t *testing.T) { + t.Parallel() + + for _, name := range []string{"PostCompaction", "PermissionRequest", "SomeFutureEvent"} { + t.Run(name, func(t *testing.T) { + t.Parallel() + + _, err := DecodeDevinEvent([]byte(`{"hook_event_name":"`+name+`","session_id":"devin-1"}`), "devin") + if !errors.Is(err, hook.ErrSkipEvent) { + t.Fatalf("DecodeDevinEvent() error = %v, want ErrSkipEvent", err) + } + }) + } +} + +func TestDecodeDevinEventTranslatesSessionLifecycle(t *testing.T) { + t.Parallel() + + for name, want := range map[string]hook.HookName{ + "SessionStart": hook.HookSessionStart, + "SessionEnd": hook.HookSessionEnd, + "PostToolUse": hook.HookPostToolUse, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + event, err := DecodeDevinEvent([]byte(`{"hook_event_name":"`+name+`","session_id":"devin-1"}`), "devin") + if err != nil { + t.Fatalf("DecodeDevinEvent() error = %v", err) + } + if event.HookName != want { + t.Fatalf("HookName = %q, want %q", event.HookName, want) + } + }) + } +} + +// A malformed payload is a contract violation rather than an event we chose not +// to handle, so it must stay an error and must not be mistaken for a skip. +func TestDecodeDevinEventRejectsMalformedInput(t *testing.T) { + t.Parallel() + + for name, input := range map[string]string{ + "missing event name": `{"tool_name":"exec"}`, + "not json": `{`, + "trailing value": `{"hook_event_name":"PreToolUse"} {}`, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + _, err := DecodeDevinEvent([]byte(input), "devin") + if err == nil { + t.Fatal("DecodeDevinEvent() error = nil, want a decode error") + } + if errors.Is(err, hook.ErrSkipEvent) { + t.Fatalf("DecodeDevinEvent() error = %v, want a failure rather than a skip", err) + } + }) + } +} + +func TestDecodeDevinEventAllowsAbsentToolInput(t *testing.T) { + t.Parallel() + + event, err := DecodeDevinEvent([]byte(`{"hook_event_name":"SessionStart","tool_input":null}`), "devin") + if err != nil { + t.Fatalf("DecodeDevinEvent() error = %v", err) + } + if event.ToolInput != nil { + t.Fatalf("ToolInput = %v, want nil", event.ToolInput) + } +} + +func TestEncodeDevinResultBlocksPreToolUseDeny(t *testing.T) { + t.Parallel() + + out, err := EncodeDevinResult(hook.HookPreToolUse.String(), hook.Result{ + Decision: hook.DecisionDeny, + Reason: "network egress is not permitted", + }) + if err != nil { + t.Fatalf("EncodeDevinResult() error = %v", err) + } + var decoded devinHookOutput + if err := json.Unmarshal(out, &decoded); err != nil { + t.Fatalf("json.Unmarshal(%s) error = %v", out, err) + } + if decoded.Decision != "block" { + t.Fatalf("Decision = %q, want block", decoded.Decision) + } + // Devin surfaces this string to the agent verbatim, so it must survive. + if decoded.Reason != "network egress is not permitted" { + t.Fatalf("Reason = %q, want the policy reason verbatim", decoded.Reason) + } +} + +func TestEncodeDevinResultAlwaysCarriesABlockReason(t *testing.T) { + t.Parallel() + + out, err := EncodeDevinResult(hook.HookPreToolUse.String(), hook.Result{Decision: hook.DecisionDeny}) + if err != nil { + t.Fatalf("EncodeDevinResult() error = %v", err) + } + var decoded devinHookOutput + if err := json.Unmarshal(out, &decoded); err != nil { + t.Fatalf("json.Unmarshal(%s) error = %v", out, err) + } + if strings.TrimSpace(decoded.Reason) == "" { + t.Fatal("Reason is empty, want the fallback reason so the agent is told why") + } +} + +func TestEncodeDevinResultEmitsNoDirectiveOnAllow(t *testing.T) { + t.Parallel() + + out, err := EncodeDevinResult(hook.HookPreToolUse.String(), hook.Result{ + Decision: hook.DecisionAllow, + Reason: "allowed", + }) + if err != nil { + t.Fatalf("EncodeDevinResult() error = %v", err) + } + if string(out) != "{}" { + t.Fatalf("EncodeDevinResult() = %s, want {}", out) + } +} + +// Only pre-tool-use gates a call. A deny recorded against an observation event +// must not turn into a directive Devin would act on. +func TestEncodeDevinResultNeverBlocksObservationEvents(t *testing.T) { + t.Parallel() + + for _, name := range []hook.HookName{hook.HookPostToolUse, hook.HookSessionStart, hook.HookSessionEnd} { + t.Run(name.String(), func(t *testing.T) { + t.Parallel() + + out, err := EncodeDevinResult(name.String(), hook.Result{ + Decision: hook.DecisionDeny, + Reason: "would deny", + }) + if err != nil { + t.Fatalf("EncodeDevinResult() error = %v", err) + } + if string(out) != "{}" { + t.Fatalf("EncodeDevinResult() = %s, want {}", out) + } + }) + } +} + +// Devin replaces tool_input wholesale rather than merging, so a partial rewrite +// would silently drop whatever the policy did not restate. Refuse visibly. +func TestEncodeDevinResultRefusesInputRewrite(t *testing.T) { + t.Parallel() + + out, err := EncodeDevinResult(hook.HookPreToolUse.String(), hook.Result{ + Decision: hook.DecisionAllow, + UpdatedInput: map[string]any{"command": "echo redacted"}, + }) + if err != nil { + t.Fatalf("EncodeDevinResult() error = %v", err) + } + var decoded devinHookOutput + if err := json.Unmarshal(out, &decoded); err != nil { + t.Fatalf("json.Unmarshal(%s) error = %v", out, err) + } + if decoded.Decision != "block" { + t.Fatalf("Decision = %q, want block so the request is not silently altered", decoded.Decision) + } + if !strings.Contains(decoded.Reason, "rewrite") { + t.Fatalf("Reason = %q, want it to explain the refusal", decoded.Reason) + } +} + +// An unrecognized decision value must fail closed rather than letting the call +// through. +func TestEncodeDevinResultFailsClosedOnUnknownDecision(t *testing.T) { + t.Parallel() + + out, err := EncodeDevinResult(hook.HookPreToolUse.String(), hook.Result{Decision: hook.Decision("maybe")}) + if err != nil { + t.Fatalf("EncodeDevinResult() error = %v", err) + } + var decoded devinHookOutput + if err := json.Unmarshal(out, &decoded); err != nil { + t.Fatalf("json.Unmarshal(%s) error = %v", out, err) + } + if decoded.Decision != "block" { + t.Fatalf("Decision = %q, want block", decoded.Decision) + } +} diff --git a/internal/hookruntime/runner.go b/internal/hookruntime/runner.go index 7995ece4..466659ad 100644 --- a/internal/hookruntime/runner.go +++ b/internal/hookruntime/runner.go @@ -1,6 +1,7 @@ package hookruntime import ( + "errors" "fmt" "io" @@ -21,6 +22,12 @@ func Run(stdin io.Reader, stdout, stderr io.Writer, codec Codec, evaluate func(h event, err := codec.DecodeHookEvent(input) if err != nil { + // An adapter that recognizes the event but does not translate it is not + // a failure: succeed without emitting a decision, so runtimes that read + // a non-zero exit as "block" let the tool call proceed. + if errors.Is(err, hook.ErrSkipEvent) { + return 0 + } fmt.Fprintf(stderr, "kontext: failed to decode hook input: %v\n", err) return 2 } diff --git a/internal/hookruntime/runner_test.go b/internal/hookruntime/runner_test.go index 6746a8fe..db15b600 100644 --- a/internal/hookruntime/runner_test.go +++ b/internal/hookruntime/runner_test.go @@ -2,6 +2,8 @@ package hookruntime import ( "bytes" + "errors" + "fmt" "testing" "github.com/kontext-security/kontext-cli/internal/hook" @@ -70,14 +72,62 @@ func TestRunWritesStructuredUnsupportedDecisionResult(t *testing.T) { } type stubCodec struct { - event hook.Event - out []byte + event hook.Event + out []byte + decodeErr error } func (s stubCodec) DecodeHookEvent([]byte) (hook.Event, error) { + if s.decodeErr != nil { + return hook.Event{}, s.decodeErr + } return s.event, nil } func (s stubCodec) EncodeHookResult(hook.Event, hook.Result) ([]byte, error) { return s.out, nil } + +// A skipped event is not a failure. Some agent runtimes read a non-zero hook +// exit as "block the tool call", so an event the adapter recognizes but does not +// translate must exit 0 and emit no decision. +func TestRunSucceedsSilentlyOnSkippedEvent(t *testing.T) { + t.Parallel() + + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + + code := Run( + bytes.NewBufferString(`{"hook_event_name":"PostCompaction"}`), + stdout, + stderr, + stubCodec{decodeErr: fmt.Errorf("devin: event PostCompaction: %w", hook.ErrSkipEvent)}, + func(hook.Event) (hook.Result, error) { + t.Fatal("evaluate called for a skipped event") + return hook.Result{}, nil + }, + ) + + if code != 0 { + t.Fatalf("Run() exit code = %d, want 0", code) + } + if stdout.String() != "" { + t.Fatalf("stdout = %q, want no decision written", stdout.String()) + } +} + +func TestRunFailsOnDecodeErrorThatIsNotASkip(t *testing.T) { + t.Parallel() + + code := Run( + bytes.NewBufferString(`{`), + &bytes.Buffer{}, + &bytes.Buffer{}, + stubCodec{decodeErr: errors.New("malformed")}, + func(hook.Event) (hook.Result, error) { return hook.Result{}, nil }, + ) + + if code != 2 { + t.Fatalf("Run() exit code = %d, want 2", code) + } +}