feat(sdk): wire semantic scanner into the SDK with on/off flag - #60
Conversation
Adds optional in-SDK semantic scanning. Default off — base SDK stays zero-deps. Enable via enable_semantic_scan=True or ACF_SEMANTIC_SCAN env var (env wins). When on, the scanner runs against the hook payload and pre-populates signals in the RiskContext. Sidecar's scan stage appends its lexical signals on top — OPA sees both. semantic_signal_threshold (default 0.85) filters low-confidence hits before they reach the wire. Latency overhead: +0.26 ms per _build_payload (TF-IDF, 1000-call microbench). 29 new tests. Full SDK suite 109/109 passing.
|
Good work overall — the on/off flag implementation is solid, the signal wire format matches the sidecar's Signal struct exactly (no Go-side changes needed), and the test coverage is thorough. Three things to fix before merge:
Firewall.init always calls SemanticScanner(backend="tfidf"). The docstring mentions "when users upgrade to the sentence-transformer backend" but there's no way to do that through the Firewall interface. Please add a backend: str = "tfidf" constructor argument (and optionally an ACF_SEMANTIC_SCAN_BACKEND env var) so users with sentence-transformers installed can opt in to the higher-accuracy backend.
on_tool_call fires before the tool executes — it's checking the call parameters, not the output. Currently harmless because SemanticScanner.scan() never reads inp.input_type, but the field is described as "determines which policy rules apply" so this will cause problems if the scanner is ever updated to filter by type. Please add TOOL_CALL = "tool_call" to the InputType enum in scanners/models.py and update the hook_to_input_type mapping accordingly. |
Two fixes from @tharindupr's review on PR c2siorg#60: 1. Added semantic_backend constructor arg + ACF_SEMANTIC_SCAN_BACKEND env var so users can switch to sentence-transformer without editing code. Env wins, default tfidf. 2. Added TOOL_CALL enum to InputType and updated the hook mapping. on_tool_call checks call parameters, not tool output. 53/53 tests passing.
|
thanks for the catches @tharindupr, fixed them all
53/53 tests are passing |
|
Perfect. Looks good now |
First step on the scanner integration scope. Wires the semantic scanner from PR #58 into the SDK with an on/off config flag and a confidence threshold, so semantic signals flow into the existing
signalsfield on the RiskContext alongside the sidecar's lexical signals.What changed
sdk/python/acf/firewall.pyenable_semantic_scanconstructor arg +ACF_SEMANTIC_SCANenv var (env wins; default off so base SDK stays zero-deps)semantic_signal_threshold(default 0.85) — only hits at or above this similarity are forwarded as wire-format signals_run_semantic_scannerand_extract_texthelpers that handle each hook's payload shape (string foron_promptandon_context, dict foron_tool_callandon_memory)[scanners]extra surfaces a clearFirewallErrorwith the install command, not a crypticImportErrorat request timesdk/python/tests/test_firewall_semantic_scan.py(new)_extract_textedge cases, scanner-failure handling, missing-extra error path[scanners]extra is not installed (CI without the extra still passes)Why a threshold
TF-IDF scores both paraphrased attacks and benign surface-similar text in the 0.75–0.85 band — they don't separate cleanly with that backend. A higher SDK-side forwarding threshold keeps weak hits out of OPA's view until we move to sentence-transformers in the upgrade PR (cleaner separation, threshold can drop).
Latency
1000-call microbenchmark on
_build_payload:Well inside the 4–8 ms end-to-end budget. Sentence-transformer overhead will be measured separately in the upgrade PR.
Wire format
Sidecar's
RiskContext.Signalsalready hasjson:"signals"and the scan stage usesappendeverywhere (sidecar/internal/pipeline/scan.go). No Go-side changes needed — the aggregate stage iteratesrc.Signalsregardless of source, so SDK signals and sidecar lexical signals both flow into OPA together.Tests
29/29 in
test_firewall_semantic_scan.py. Full SDK suite 109/109.What's next
The detection rate measurement report — run the adversarial corpus + benign set twice (flag off vs on), report true positive rate, false positive rate, latency at p50/p95/p99, per-hook breakdown.