[EXPERIMENTAL] : Add Rust CPEX secrets detection experiment - #148
gandhipratik203 wants to merge 7 commits into
Conversation
36eb387 to
7e1f5a8
Compare
Signed-off-by: Pratik Gandhi <gandhipratik203@gmail.com>
7e1f5a8 to
59ca82a
Compare
Signed-off-by: Pratik Gandhi <gandhipratik203@gmail.com>
Signed-off-by: Pratik Gandhi <gandhipratik203@gmail.com>
Signed-off-by: Pratik Gandhi <gandhipratik203@gmail.com>
Signed-off-by: Pratik Gandhi <gandhipratik203@gmail.com>
lucarlig
left a comment
There was a problem hiding this comment.
Experimental scope looks good. Leaving only non-blocking nitpicks for the dataplane trial.
| impl StageHandler { | ||
| fn scan_payload(&self, payload: &MessagePayload) -> PayloadScan { | ||
| let config = self.core.scanner_config(); | ||
| let mut modified_payload = payload.clone(); |
There was a problem hiding this comment.
nit (non-blocking for the spike): this deep-clones the complete CMF payload before we know whether redaction is enabled or any finding exists. Even clean/default (redact: false) invocations therefore allocate proportional to payload size, which could skew the dataplane experiment for large tool/resource results. Could we scan the borrowed payload first and clone lazily only when returning a modification?
| "cmf.resource_post_fetch" => Stage::ResourcePostFetch, | ||
| _ => return None, | ||
| }; | ||
| let hook_name: &'static str = Box::leak(hook.to_owned().into_boxed_str()); |
There was a problem hiding this comment.
nit: these are four fixed hook names, so leaking a fresh String on every factory creation seems unnecessary. CPEX may invoke the factory again for route-level config overrides, so this can accumulate beyond startup. Could the match return the corresponding static literal or CMF hook constant alongside Stage?
| publish = false | ||
|
|
||
| [dependencies] | ||
| cpex-core = "=0.2.2" |
There was a problem hiding this comment.
nit/versioning: if 0.2.2 is the release carrying the breaking plugin API/boundary changes this spike is adapting to, this should really be CPEX 0.3.0. A 0.2.x patch communicates compatibility, while this is breaking for plugin implementers/consumers.
Signed-off-by: Pratik Gandhi <gandhipratik203@gmail.com>
Lang-Akshay
left a comment
There was a problem hiding this comment.
Thanks for the PR @gandhipratik203
Please address following blocking Changes
| # | Area | File | Line | Blocking reason | Required change |
|---|---|---|---|---|---|
| 1 | Security | experiments/secrets_detection_rust/src/patterns.rs; src/scanner.rs |
48; 129-137 | High / CWE-200: private_key_block matches only a PEM BEGIN marker. With redact: true and non-blocking operation, the key body and END marker remain in the forwarded payload. |
Detect and replace the complete bounded PEM block (header, body, footer), or fail closed whenever a private-key marker is found. Add coverage for multiline PEM redaction and blocking. |
| 2 | Security | experiments/secrets_detection_rust/src/lib.rs |
90-94; 333-349 | High / CWE-693: the factory accepts non-blocking CPEX modes even when block_on_detection is enabled. CPEX suppresses continue_processing=false for Transform/Audit/FireAndForget; the deny fallback can attach the original payload, allowing detected secrets to continue. |
Reject enforcement configurations paired with non-enforcing modes; require a mode that honors denial (or fail closed), and never attach the original unredacted payload as a deny fallback. |
Signed-off-by: Pratik Gandhi <gandhipratik203@gmail.com>
|
Thanks Akshay. This PR is scoped to validating the Rust CPEX integration path, with production-level secrets detection behavior deferred to follow-up hardening work. Agreed on both points. The private key matcher should be tightened before production use, and I’ve created #162 to track these production hardening items. |
Lang-Akshay
left a comment
There was a problem hiding this comment.
Blocking Changes
| # | Area | File | Line | Blocking reason | Required change |
|---|---|---|---|---|---|
| 1 | Security | src/patterns.rs |
48 | CWE-522: private_key_block regex matches only the -----BEGIN … PRIVATE KEY----- header line. detect_and_redact redacts only that single matched span; the entire key body and -----END … PRIVATE KEY----- remain in output when redact=true. A payload containing a PEM private key is silently under-redacted. |
Extend the pattern to cover the full PEM block (header through footer, including the base64 body), or post-process to strip everything between the matched header and the corresponding footer line. |
| 2 | Security | src/lib.rs |
93 | CWE-200: On deny (should_block = true), result.modified_payload is set to `scan.modified_payload.or_else( |
|
closing as the realated work is got moved to the pr here: contextforge-org/contextforge-data-plane#74 |
Summary
experiments/secrets_detection_rustPluginFactory+ shared core + stage-bound CMF handlerscmf.prompt_pre_fetch,cmf.tool_pre_invoke,cmf.tool_post_invoke, andcmf.resource_post_fetchexperiments/secrets_detection_rust/examples/manual_probe.rsfor manual PluginManager probingexperiments/secrets_detection_rust/smoke/crate_consumerScope
This is intentionally a draft spike and is not intended to merge as-is. The goal is to understand how a Rust-native secrets detection plugin integrates with the new CPEX framework and what parity looks like against the current plugin behavior.
Related to #117. This spike is not intended to close the issue as-is.
No production plugin code under
plugins/rust/python-package/secrets_detectionis changed by this PR.Coverage Added
validator/secrets-detectionManual Testing
From
experiments/secrets_detection_rust:The probe registers the plugin factory, loads CPEX YAML config, invokes the selected CMF hook through
PluginManager, and prints the resultingPipelineResultfields plus the returned CMF content.Manual Crate-Level Smoke Test
This is the Rust equivalent of the wheel-level smoke test from #134. Instead of installing a Python wheel into a clean venv, it uses a standalone Cargo consumer committed under the spike directory and depends on the spike crate by relative path.
Tested from:
cd experiments/secrets_detection_rust/smoke/crate_consumer cargo runThe consumer uses only the public crate exports:
What it verifies:
cmf.tool_pre_invokeredacts tool argumentscmf.tool_pre_invokeblocks tool argumentscmf.prompt_pre_fetchapplies field filters to prompt argumentsfield_allowlist=["accounts"]scans/redactsaccounts.keepfield_denylist=["accounts.skip"]excludesaccounts.skipcmf.tool_post_invokeapplies field filters to tool result JSONcmf.resource_post_fetchblocks direct resource textignored, remain unchangedResult:
Known CPEX 0.2.2 Gaps
The spike pins two framework-boundary gaps with explicit tests:
PluginResult.metadatais produced by the plugin at the handler layer, but is dropped by the manager/executor path when returned as aPipelineResult.modified_payload, butPluginManagerdoes not currently surface that payload on denied results.Validation
cargo testinexperiments/secrets_detection_rust: 47 passed (34inline/unit tests +13PluginManager integration tests)tool-redact,tool-block,prompt-filter,tool-allow-deny,tool-result-filter,resource-blockexperiments/secrets_detection_rust/smoke/crate_consumer: passed, including the manual probe hook scenarioscargo test -p secrets_detection: 96 passedmake install: passedmake test-integration: 114 passedmake test-unit: not available becausecargo-nextestis not installed