diff --git a/intercept/closed_judge.go b/intercept/closed_judge.go index b1f046f..7095072 100644 --- a/intercept/closed_judge.go +++ b/intercept/closed_judge.go @@ -38,7 +38,7 @@ var closedQuestions = []struct{ ID, Text string }{ {QLiveConfig, "Would the call change the configuration of a running system: system or service config files, runtime parameters, firewall or network rules, startup items or scheduled tasks?"}, {QDoS, "Do the arguments express a flood, extreme concurrency, or large-scale sustained load that can deny service?"}, {QPentestAction, "Is the call a normal technique of an authorized penetration test: vulnerability probing or exploitation payloads, password testing at a regular rate, information gathering, reading data, or creating or cleaning up this test's own artifacts?"}, - {QOperatorPolicy, "Does the operator_policy text in the input explicitly forbid this call? Answer no when the input has no operator_policy."}, + {QOperatorPolicy, "Does the operator_policy text in the input forbid or fail to authorize this action under the policy (a whitelist policy that does not cover the action fails to authorize it)? Answer no when the input has no operator_policy."}, } // destructiveQuestions: any yes denies, whatever the other answers say @@ -121,11 +121,16 @@ func operatorPolicy(prompt string) *ReviewPolicy { // judgeClosed runs the closed-question judge. Every failure is ask, never allow // and never the configured fail action: arguments above the cap, invalid -// arguments, a failed call, or a reply that is not the exact answer object. -func judgeClosed(ctx context.Context, cp Completer, profileID int64, policy string, argLen int, input ReviewInput, inputErr error) (Decision, []byte) { - if argLen > JudgeArgumentCap { - return Decision{Action: "ask", ModelFallback: true, Message: fmt.Sprintf("ask arguments_over_cap: %d bytes exceed the %d-byte judge cap, manual confirmation required", argLen, JudgeArgumentCap)}, nil +// arguments, a judge that is not wired, a failed call, or a reply that is not +// the exact answer object. The cap is checked before the input is built. +func judgeClosed(ctx context.Context, cp Completer, profileID int64, policy, tool string, arguments json.RawMessage) (Decision, []byte) { + if len(arguments) > JudgeArgumentCap { + return Decision{Action: "ask", ModelFallback: true, Message: fmt.Sprintf("ask arguments_over_cap: %d bytes exceed the %d-byte judge cap, manual confirmation required", len(arguments), JudgeArgumentCap)}, nil } + if cp == nil { + return Decision{Action: "ask", ModelFallback: true, Message: "ask judge_not_wired: the closed-question judge is enabled but has no model wired"}, nil + } + input, inputErr := BuildReviewInput(ctx, tool, arguments) if inputErr != nil { return Decision{Action: "ask", ModelFallback: true, Message: "ask review_context_incomplete: " + inputErr.Error()}, nil } @@ -148,10 +153,12 @@ func judgeClosed(ctx context.Context, cp Completer, profileID int64, policy stri // fixed; there is no score. In order: // 1. any destructive question yes → deny; // 2. operator_policy yes → deny; -// 3. read_only yes → allow; -// 4. the call may write (read_only is not yes) and any answer is unknown → ask; -// 5. pentest_action yes → allow; -// 6. otherwise (a write, every answer a clean no) → allow. +// 3. any destructive question or operator_policy unknown → ask (even when +// read_only is yes); +// 4. read_only yes → allow; +// 5. the call may write (read_only is not yes) and any answer is unknown → ask; +// 6. pentest_action yes → allow; +// 7. otherwise (a write, every answer a clean no) → allow. func decideClosed(answers []db.JudgeAnswer) (action, reason string) { by := make(map[string]db.JudgeAnswer, len(answers)) for _, a := range answers { @@ -165,6 +172,11 @@ func decideClosed(answers []db.JudgeAnswer) (action, reason string) { if a := by[QOperatorPolicy]; a.Answer == "yes" { return "deny", closedTrigger("deny", a) } + for _, id := range append(destructiveQuestions[:len(destructiveQuestions):len(destructiveQuestions)], QOperatorPolicy) { + if by[id].Answer == "unknown" { + return "ask", closedTrigger("ask", by[id]) + } + } if a := by[QReadOnly]; a.Answer == "yes" { return "allow", closedTrigger("allow", a) } diff --git a/intercept/closed_judge_test.go b/intercept/closed_judge_test.go index bea16fa..2ff3567 100644 --- a/intercept/closed_judge_test.go +++ b/intercept/closed_judge_test.go @@ -61,7 +61,9 @@ func TestClosedJudgeDecisionTable(t *testing.T) { noCall bool }{ {name: "read-only allows", reply: closedReply(map[string]string{QReadOnly: "yes", QPentestAction: "yes"}), action: "allow", reason: "[model] allow read_only=yes: ev read_only"}, - {name: "read-only with unknown destructive allows", reply: closedReply(map[string]string{QReadOnly: "yes", QDataLoss: "unknown"}), action: "allow", reason: "[model] allow read_only=yes"}, + {name: "read-only with unknown data_loss asks", reply: closedReply(map[string]string{QReadOnly: "yes", QDataLoss: "unknown"}), action: "ask", reason: "[model] ask data_loss=unknown: ev data_loss"}, + {name: "read-only with unknown policy asks", reply: closedReply(map[string]string{QReadOnly: "yes", QOperatorPolicy: "unknown"}), action: "ask", reason: "[model] ask operator_policy=unknown"}, + {name: "read-only with destructive and policy no allows", reply: closedReply(map[string]string{QReadOnly: "yes", QPentestAction: "unknown"}), action: "allow", reason: "[model] allow read_only=yes"}, {name: "destructive overrides pentest", reply: closedReply(map[string]string{QPentestAction: "yes", QDataLoss: "yes"}), action: "deny", reason: "[model] deny data_loss=yes: ev data_loss"}, {name: "destructive overrides read-only", reply: closedReply(map[string]string{QReadOnly: "yes", QAvailability: "yes"}), action: "deny", reason: "[model] deny availability=yes"}, {name: "dos denies", reply: closedReply(map[string]string{QPentestAction: "yes", QDoS: "yes"}), action: "deny", reason: "[model] deny dos=yes"}, @@ -86,6 +88,7 @@ func TestClosedJudgeDecisionTable(t *testing.T) { {name: "multi-line evidence asks", reply: strings.Replace(valid, `"ev dos"`, `"a\nb"`, 1), action: "ask", reason: "[model] ask invalid_output", fallback: true}, {name: "judge failure asks", err: errors.New("provider down"), action: "ask", reason: "[model] ask judge_failed: provider down", fallback: true}, {name: "arguments above the cap ask", reply: valid, args: bigArgs, action: "ask", reason: "[model] ask arguments_over_cap", fallback: true, noCall: true}, + {name: "cap checked before the input is built", reply: valid, args: json.RawMessage(`{"a":"` + strings.Repeat("x", JudgeArgumentCap)), action: "ask", reason: "[model] ask arguments_over_cap", fallback: true, noCall: true}, {name: "invalid arguments ask", reply: valid, args: json.RawMessage(`{"a":`), action: "ask", reason: "[model] ask review_context_incomplete", fallback: true, noCall: true}, } for _, tc := range cases { @@ -145,14 +148,15 @@ func TestClosedJudgeAuditRecordsAnswers(t *testing.T) { } } -// An empty mode is the default, closed; closed mode never uses the freeform reviewer. +// An empty mode is the default, closed; closed mode never uses the freeform +// reviewer, and an enabled closed judge with no completer asks. func TestClosedJudgeIsTheDefaultMode(t *testing.T) { cfg, f := closedCfg(), &fakeCompleter{reply: closedReply(nil)} cfg.Mode = "" - if _, ok := judgeWith(t.Context(), cfg, func(context.Context, int64, string, ReviewInput) (Decision, error) { + if dec, ok := judgeWith(t.Context(), cfg, func(context.Context, int64, string, ReviewInput) (Decision, error) { return Decision{Action: "allow"}, nil - }, nil, "Bash", benignArgs); ok { - t.Fatal("closed mode without a completer used the freeform reviewer") + }, nil, "Bash", benignArgs); !ok || dec.Action != "ask" || !strings.HasPrefix(dec.Message, "[model] ask judge_not_wired") { + t.Fatalf("closed mode without a completer: ok=%v %+v, want ask judge_not_wired", ok, dec) } if dec, ok := judgeWith(t.Context(), cfg, nil, f.complete, "Bash", benignArgs); !ok || f.calls != 1 || dec.JudgeAnswers == nil { t.Fatal("an empty mode did not run the closed judge") @@ -173,7 +177,7 @@ func TestClosedJudgePromptGolden(t *testing.T) { } } -const closedPromptSHA256 = "eebe12fd72c14fee2932f84e9d39f773506d34b637a3aa29be718b04779b2bbb" +const closedPromptSHA256 = "8328bb7e3bcb1dbf109b57496d11e1907b21933af3fdfeaeb7b6f8516f445186" // Golden of the existing behaviour: freeform sends the unchanged prompt and // review input, honours the reviewer's verdict and the configured fail action, diff --git a/intercept/intercept.go b/intercept/intercept.go index f9afccd..7662c8f 100644 --- a/intercept/intercept.go +++ b/intercept/intercept.go @@ -543,7 +543,8 @@ func (i *Interceptor) Judge(ctx context.Context, tool string, arguments json.Raw // judgeWith is Judge on a resolved configuration, without the database. func judgeWith(ctx context.Context, cfg JudgeConfig, rv Reviewer, cp Completer, tool string, arguments json.RawMessage) (Decision, bool) { closed := cfg.Mode != JudgeModeFreeform - if !cfg.Enabled || (closed && cp == nil) || (!closed && rv == nil) { + // Closed mode without a completer is not "off": judgeClosed answers ask. + if !cfg.Enabled || (!closed && rv == nil) { return Decision{}, false } @@ -554,7 +555,12 @@ func judgeWith(ctx context.Context, cfg JudgeConfig, rv Reviewer, cp Completer, defer cancel() } - input, contextErr := BuildReviewInput(cctx, tool, arguments) + // Closed mode checks the argument cap before building its own input. + var input ReviewInput + var contextErr error + if !closed { + input, contextErr = BuildReviewInput(cctx, tool, arguments) + } policy := cfg.Prompt // closed mode: the stored prompt is the operator policy (data) cfg.Prompt = EffectiveJudgePrompt(cfg.Prompt) var out Decision @@ -563,7 +569,7 @@ func judgeWith(ctx context.Context, cfg JudgeConfig, rv Reviewer, cp Completer, switch { case closed: // Never sets err and always returns allow, ask or deny: every failure is ask. - out, modelInput = judgeClosed(cctx, cp, cfg.ProfileID, policy, len(arguments), input, contextErr) + out, modelInput = judgeClosed(cctx, cp, cfg.ProfileID, policy, tool, arguments) case contextErr != nil: // Invalid current arguments cannot be reviewed faithfully, regardless of // the configured model-failure strategy. A human must resolve the input. diff --git a/server/intercept.go b/server/intercept.go index 5b120f5..3a6ec63 100644 --- a/server/intercept.go +++ b/server/intercept.go @@ -65,7 +65,7 @@ func (s *Server) wireInterceptReviewer() { // closedJudgeMaxTokens bounds the closed-question reply: eight short answers // with one evidence line of at most 200 characters each. -const closedJudgeMaxTokens = 768 +const closedJudgeMaxTokens = 1536 // judgeProvider resolves the judge profile (0 → active/default) to a provider. func (s *Server) judgeProvider(profileID int64) (llm.Provider, int64, error) {