Skip to content

feat(classifier): harness risk classifier (split from #181) - #189

Closed
raymondginger2018-sudo wants to merge 2 commits into
HKUDS:mainfrom
raymondginger2018-sudo:pr181/classifier
Closed

feat(classifier): harness risk classifier (split from #181)#189
raymondginger2018-sudo wants to merge 2 commits into
HKUDS:mainfrom
raymondginger2018-sudo:pr181/classifier

Conversation

@raymondginger2018-sudo

Copy link
Copy Markdown
Contributor

Summary

Splits the risk-classifier module out of #181 (maintainer suggestion). New files only — no existing behavior modified while DEEPCODE_RISK_CLASSIFIER is unset.

core/harness/classifier.py is a prompt-injection defense: a fast rule-based scorer (keyword + boundary heuristics) that flags suspicious user/harness content before it reaches the loop, with a classify(text) -> RiskDecision API and an allowlist for expected patterns. Gated behind DEEPCODE_RISK_CLASSIFIER so behavior is opt-in.

Tests

tests/test_harness_classifier.py — 14 passed locally.

@raymondginger2018-sudo

Copy link
Copy Markdown
Contributor Author

@Zongwei9888 Per your 08-17 suggestion on #181, this is the risk classifier module split out as an independent PR: new files only (core/harness/classifier.py + tests/test_harness_classifier.py, 14 tests), opt-in behind DEEPCODE_RISK_CLASSIFIER, merge-order independent of the sibling splits (#188 keyring, #190 wiring).

CI status: Python CI / Desktop CI / Linting green; the Security CI failure is the repo-wide pip advisory (PYSEC-2026-3721 on the locked pip==26.1.2), fixed in #191 — unrelated to this change.

Ready for review.

Prompt-injection defense: DEEPCODE_RISK_CLASSIFIER-gated classifier
that scores incoming user/harness content before it reaches the loop.
All new files; existing behavior untouched when the knob is off.
@raymondginger2018-sudo

Copy link
Copy Markdown
Contributor Author

Hi Zongwei, this is the classifier module split from #181. Pure new files, 14 tests, no behavior changes to existing code. Rebased onto latest main (e0767d0), all 14 CI checks green. Merge-order independent. Ready for review when you have a moment. Thanks!

@raymondginger2018-sudo

Copy link
Copy Markdown
Contributor Author

Closing as cleanup after #200 — thanks for your patience. For the record: this is a clean new-module PR (no duplicate on upstream/main), but as it stands it has no runtime call site wired in (it is not yet plugged into the permission/approval path), which per the #200 feedback is the shape that will not be accepted on its own. Parking it rather than leaving it stale: the branch stays on my fork, and I can reopen with the call-site wiring once the current narrow PR (#204) lands. Happy to re-scope if you prefer.

@Zongwei9888

Copy link
Copy Markdown
Collaborator

Apologies for the late reply. We did not take this for three reasons: the description (a rule-based keyword scorer with an allowlist and RiskDecision) does not match the code (an LLM-backed LLMRiskClassifier.classify(tool_name, arguments, reason)); nothing in the approval path calls it; and its intended effect — auto-allowing "low risk" calls — would replace today's fail-closed behaviour in headless runs with a model judgement that receives attacker-controlled tool arguments unframed.

Auto-mode-style risk triage is a real product question, but it needs a design discussion first (framing of the classifier prompt, where it sits in the permission engine, integration tests). Happy to have that conversation in an issue before any code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants