feat(harness): default-off risk evaluator in the permission engine

[rifkir23]

Description

Implements the shape agreed in #245: one strictness-only risk evaluator wired into the permission engine, default off, rules only.

The evaluator runs only on calls the mode default (step 4) would allow, and its only move is allow → ask. That ask goes through _apply_approval_policy like every other one, so a never approval policy or a headless run ends in a deny that carries the evaluator's reason. Sensitive-path protection, the read-only upper bound, and explicit rules all decide first and stay final in either direction. The runner's fail-closed path is untouched.

Related Issues

Closes #245. (#189's heuristics can join later as extra rules on top of this, each with its own test, as suggested in the thread.)

Changes Made

• core/harness/risk_evaluator.py (new): a RiskEvaluator protocol ((tool_name, arguments) -> reason | None, so it returns an opinion and the engine decides) and CommandGuardRiskEvaluator, which reuses command_guard.screen_all. That means the destructive-command screen always applies, and the egress/install screens apply under DEEPCODE_COMMAND_SCREEN=strict, the same as at the shell front door. There is no new keyword scorer and no model call.
• core/harness/permissions.py: the three step-4 allow returns (plan read-only, full_auto, default read-only) go through a single _mode_default_allow hook. Opinions are cached per call signature. An evaluator that raises or returns an unusable value fails closed to ask and is not cached, so a transient error doesn't stick to a signature for the rest of the session. make_engine gains an optional risk_evaluator argument.
• core/harness/policy.py: build_permission_engine wires the built-in evaluator in when DEEPCODE_RISK_EVALUATOR=1.
• docs/P1_SECURITY_BASE.md: one bullet documenting the flag.

Tests (tests/test_harness_risk_evaluator.py)

These map to the five tests proposed in the issue, plus the parity test that was asked for:

1. Cannot loosen anything: sensitive-path DENY and plan-mode DENY survive an allow opinion, and explicit allow/deny rules stay final (the evaluator is never consulted).
2. Intended path: in full_auto with the flag on, rm -rf → ask with the screen's reason. With never, it's a deny that carries both reasons.
3. Failure contained: raising, non-string, and empty opinions → ask (fail-closed), never a crash. Failures are not cached.
4. No waste: zero calls when steps 1–3 (or the default-mode mutating ask) already decided, and one call per signature (cache hit asserted).
5. Evasion fixtures: whitespace, split flags, long flags, chained commands, and 0777 are flagged. A base64-encoded payload is shown passing, as a pinned, documented limit: this is advisory, not a security boundary. The sandbox is.
6. Feature off: across default/plan/full_auto × on_request/never, make_engine gives decisions and reasons identical to a plain engine. build_permission_engine follows the flag too.

Mutation check: removing the hook fails 7 tests, and treating evaluator failure as allow fails the fail-closed test.

Checklist

• Changes tested locally: the new file plus every test that touches the permission engine / policy / command guard (302 passed). ruff check and ruff format --check are clean with the pre-commit-pinned ruff v0.15.21.
• Code reviewed (self)
• Documentation updated
• Unit tests added

Additional Notes

One behaviour worth a decision from you: with the Full Access preset and DEEPCODE_RISK_EVALUATOR=1, the evaluator still asks, because I read the flag as explicit operator intent. If you'd rather Full Access ignore it (the way it bypasses MCP origin approval), that's a one-line change in build_permission_engine and I'm happy to make it.

[rifkir23]

FYI on the red Windows job: it fails in "Install package and test tools" at pip check, before any test runs. The cause is the runner's preinstalled pipx 1.17.11, which wants newer filelock/packaging/platformdirs than scripts/ci/requirements.lock pins:

pipx 1.17.11 requires filelock>=4.0.9, but you have filelock 3.32.6
pipx 1.17.11 requires packaging>=26.3, but you have packaging 26.2
pipx 1.17.11 requires platformdirs>=4.12.2, but you have platformdirs 4.11.8

The same step fails the same way on run 37660317969 (an unrelated PR from 2026-10-07), and the last main run (2026-09-28) passed it, so this looks like a runner-image change. This PR does not touch CI or dependencies, and all test (3.12/3.13/3.14) jobs pass. Happy to open a separate small PR (a venv for the Windows job, or a lock bump) if you would like.

[Zongwei9888]

Thanks for this, @rifkir23, and for following the #245 shape so closely: one hook at the three step-4 allow returns, allow → ask only, explicit rules and both boundaries untouched, rules-only on top of command_guard, and the six tests. I checked the off path: with DEEPCODE_RISK_EVALUATOR unset, build_permission_engine passes None and _mode_default_allow returns the same tuple as today, so decisions and reasons are unchanged. I also could not find a path where turning it on loosens a decision.

Thank you as well for diagnosing the red Windows job. As you found, it fails at pip check because the runner image's preinstalled pipx 1.17.11 wants newer filelock/packaging/platformdirs than scripts/ci/requirements.lock pins. It is unrelated to this PR and is fixed on main in f6ab0e7, so a rebase onto main will turn the job green; no separate PR is needed.

A few changes before merge:

1. Opinion cache. The key is _argument_string() (permissions.py:456 on this branch), but the evaluator screens what _command_argument() finds (risk_evaluator.py:77–88), and the two differ. For a non-shell tool _argument_string prefers a path-like key, so {"command": "ls", "path": "/w"} and a later {"command": "rm -rf /", "path": "/w"} share the key "/w" and the second call reuses the first None. The same happens for a shell call with an empty command and a non-empty cmd. For tools without a path key, the key is the full argument string, so every apply_patch body and web_fetch URL+prompt stays in memory for the session. Since v1 is rules-only and screen_all takes about 15 µs per command, we'd prefer to drop the engine-level cache for now and bring it back with a model-based evaluator, which is where [Design] Risk triage for tool calls: what does it classify, and where does it sit in the permission engine? #245 motivated it. If you'd rather keep it, please key it on the exact string the evaluator inspects (or a canonical dump of all arguments) and bound it (a small LRU is fine).
2. Full Access. Full Access resolves to full_auto with approval_policy=never (core/domain/execution_security.py:175–180), so with the flag on, a flagged call there becomes a deny that cannot be approved, not an ask. We'd like Full Access to skip the evaluator, the same way it skips origin approval: please pass None in build_permission_engine when profile.access_preset is ExecutionAccessPreset.FULL_ACCESS (policy.py:214), with a test through build_permission_engine(..., execution_security_profile=ExecutionSecurityProfile.for_preset("full_access")).
3. Read the flag only in policy.py. make_engine has no production caller, and permissions.py describes itself as "mechanism, not policy". Reading the env there (permissions.py:536–537) also lets an exported flag change the existing make_engine tests in tests/test_harness_permissions.py. Keeping just the explicit risk_evaluator argument means permissions.py only needs the RiskEvaluator type.

Smaller items:

4. Please add the step-4 second look to the precedence list in the permissions.py module docstring (lines 26–30). In the doc bullet, please note that under Read only (approval policy never) a flag is a deny, and that a session-wide approval of a tool also covers that tool's later flagged calls (grants are per thread and tool). It may also help to say that for the built-in bash tool the front door already refuses everything this evaluator flags (core/harness/tools/shell.py:190), so the gain is mainly for MCP tools that take a command-style argument.
5. One test through evaluate_tool with an MCP approval_mode would cover the origin path. A test through the runner with approval_callback=None is welcome too, if it isn't much work.

Happy to re-review once these are in. cc @raymondginger2018-sudo, since #189's rules were planned to build on this seam.