Create a new IDE specific secret scanning route by abrooksv · Pull Request #900 · DataDog/datadog-static-analyzer

abrooksv · 2026-04-24T15:40:56Z

What problem are you trying to solve?

IDE-5981

Differences between the current route and new one:

New route passes the JSON:API based format for the rule so the IDE is not in the job of translation. This allows the K9 team to evolve the rule inputs without involving the IDE team
New route disables the secret validation inside of the binary instead of having the IDE drop the fields on the floor before passing the rules to the binary
Some logic was removed like # of rules and file name validation. Both seemed out of the realm for the IDE route to enforce

Secret scanner cache changed to work with both routes by taking the JSON->SecretReule logic as a lambda.

Differences between the current route and new one: * New route passes the JSON:API based format for the rule so the IDE is not in the job of translation. This allows the K9 team to evolve the rule inputs without involving the IDE team * New route disables the secret validation inside of the binary instead of having the IDE drop the fields on the floor before passing the rules to the binary * Some logic was removed like # of rules and file name validation. Both seemed out of the realm for the IDE route to enforce Secret scaner cache changed to work with both routes by taking the JSON->SecretReule logic as a lambda.

datadog-official · 2026-04-24T15:44:56Z

🎯 Code Coverage (details)
• Patch Coverage: 88.29%
• Overall Coverage: 85.38% (+0.35%)

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: 3b75bf1 | Docs | Datadog PR Page | Give us feedback!}

Copilot

Pull request overview

Adds a new IDE-specific secrets scanning endpoint that accepts JSON:API rule payloads, disables online secret validation for IDE usage, and adapts the scanner cache to support multiple rule-parsing formats.

Changes:

Introduces disable_validation in AnalysisOptions and gates SDS match validation behind it.
Refactors SecretScannerCache to accept a caller-provided rule parser on cache miss; updates existing secret-scan path to use it.
Adds /ide/v1/secrets/scan endpoint (mounted under /ide) with end-to-end tests and updates IDE routing + request samples.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
crates/static-analysis-server/src/request.rs	Updates `AnalysisOptions` construction to use `..Default::default()` for new fields.
crates/static-analysis-kernel/src/analysis/analyze.rs	Updates test `AnalysisOptions` literals for new fields via `..Default::default()`.
crates/secrets/src/scanner.rs	Adds `disable_validation` gate around `scanner.validate_matches`.
crates/common/src/analysis_options.rs	Adds `disable_validation: bool` with default `false`.
crates/bins/src/bin/datadog_static_analyzer_server/secret_scanner_cache.rs	Refactors cache API to accept a parsing lambda on miss; updates tests.
crates/bins/src/bin/datadog_static_analyzer_server/ide/secrets/models.rs	Defines IDE request/response models for secret scanning.
crates/bins/src/bin/datadog_static_analyzer_server/ide/secrets/mod.rs	Adds IDE secrets module + end-to-end endpoint tests.
crates/bins/src/bin/datadog_static_analyzer_server/ide/secrets/endpoints.rs	Implements IDE secrets scan route using JSON:API rule parsing + disables validation.
crates/bins/src/bin/datadog_static_analyzer_server/ide/requests.http	Updates sample requests (host + config parse examples).
crates/bins/src/bin/datadog_static_analyzer_server/ide/mod.rs	Registers the new IDE secrets route.
crates/bins/src/bin/datadog_static_analyzer_server/endpoints.rs	Updates legacy secret scanning route to use the new cache API with a parser closure.
crates/bins/src/bin/datadog-static-analyzer.rs	Updates `AnalysisOptions` literal to include new field via default update.
crates/bins/src/bin/datadog-static-analyzer-test-ruleset.rs	Updates `AnalysisOptions` literal to include new field via default update.
crates/bins/src/bin/datadog-static-analyzer-git-hook.rs	Updates `AnalysisOptions` literal to include new field via default update.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-24T15:46:55Z

+    /// Callers supply the parser used on cache miss. The hash key is the raw JSON bytes,
+    /// so callers that feed the same raw rules share the same cache entry regardless of
+    /// which parser they use.


The cache key is derived only from the raw JSON bytes, but the docstring claims callers “share the same cache entry regardless of which parser they use”. That isn’t guaranteed: two different parsers could produce different Vec<SecretRule> from identical raw JSON, and a later call would silently reuse the first parsed rules/scanner. Consider either (a) including a parser/version identifier in the cache key, or (b) tightening the contract/doc to state that the parser must be deterministic and semantically equivalent for a given raw_rules payload (and ideally enforce this by only exposing route-specific helpers).

Copilot · 2026-04-24T15:46:55Z

+                SecretRule::try_from(api_rule)
+                    .map_err(|e| format!("Failed to convert secret rule: {e}"))


SecretRule::try_from(api_rule) can panic for certain malformed JSON: the TryFrom<SecretRuleApiMatchValidation> implementation uses expect(...) for required fields when match_validation.type == "CustomHttp" (e.g., missing endpoint/http_method). Because this endpoint accepts rule JSON from the IDE, a crafted request could trigger a panic in the blocking task and turn a client error into an internal error/DoS. Add explicit validation before calling try_from, or wrap the conversion in std::panic::catch_unwind and return a structured “Failed to convert secret rule” error instead of panicking.

Suggested change

SecretRule::try_from(api_rule)

.map_err(|e| format!("Failed to convert secret rule: {e}"))

match std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {

SecretRule::try_from(api_rule)

})) {

Ok(rule) => rule.map_err(|e| format!("Failed to convert secret rule: {e}")),

Err(_) => Err(

"Failed to convert secret rule: malformed secret rule payload"

.to_string(),

),

}

How does k9 want to handle this? Should try_from not panic, this affects the CLI so unsure on what route to take

Yeah, try_from should never panic. The only problematic thing I see is:

datadog-static-analyzer/crates/secrets/src/model/secret_rule.rs

Line 147 in b118e6e

timeout_seconds: custom_http.timeout_seconds.unwrap() as u32,

(Unless I am missing something). However, it's not clear to me if that data is controlled by our backend or potentially the user. If it's our backend, I'm less worried about an unwrap here, but we would want to convert it to an expect specifically so we can explain that assumption.

Otherwise, if it's user-provided data, we should probably use unwrap_or to set a reasonable default.

This should be coming from the backend since its triggered by the JSON:API format -> SecretRule conversion

That code already will panic on Err:

datadog-static-analyzer/crates/cli/src/datadog_utils.rs

Line 75 in 3a3091b

.map(|v| v.try_into().expect("cannot convert rule"))

So having this return Err seems best to me? Also the panics at

datadog-static-analyzer/crates/cli/src/model/datadog_api.rs

Line 325 in 2c6bee8

SecretRuleMatchValidation::CustomHttp(SecretRuleMatchValidationHttp {

492a026 should remove all the panics from the code path

jasonforal

LGTM, small nits

jasonforal · 2026-04-27T19:26:31Z

+                SecretRule::try_from(api_rule)
+                    .map_err(|e| format!("Failed to convert secret rule: {e}"))


Yeah, try_from should never panic. The only problematic thing I see is:

datadog-static-analyzer/crates/secrets/src/model/secret_rule.rs

Line 147 in b118e6e

timeout_seconds: custom_http.timeout_seconds.unwrap() as u32,

(Unless I am missing something). However, it's not clear to me if that data is controlled by our backend or potentially the user. If it's our backend, I'm less worried about an unwrap here, but we would want to convert it to an expect specifically so we can explain that assumption.

Otherwise, if it's user-provided data, we should probably use unwrap_or to set a reasonable default.

… path

robertohuertasm-datadog

LGTM! 🚀

Copilot AI review requested due to automatic review settings April 24, 2026 15:40

abrooksv requested review from a team as code owners April 24, 2026 15:40

abrooksv requested a review from gillarramendi April 24, 2026 15:40

Copilot started reviewing on behalf of abrooksv April 24, 2026 15:41 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

Add test for options.disable_validation

57269ca

jasonforal previously approved these changes Apr 28, 2026

View reviewed changes

abrooksv added 2 commits April 28, 2026 12:41

Address PR feedback

81d87ea

Remove the panics from the SecretRuleApiMatchValidation.try_from code…

492a026

… path

abrooksv dismissed jasonforal’s stale review via 492a026 April 28, 2026 19:45

robertohuertasm-datadog reviewed Apr 29, 2026

View reviewed changes

Comment thread crates/bins/src/bin/datadog_static_analyzer_server/ide/requests.http Outdated

robertohuertasm-datadog previously approved these changes Apr 29, 2026

View reviewed changes

Revert port change in ide HTTP request file

3b75bf1

abrooksv dismissed robertohuertasm-datadog’s stale review via 3b75bf1 April 29, 2026 14:19

robertohuertasm-datadog approved these changes Apr 29, 2026

View reviewed changes

jasonforal approved these changes Apr 29, 2026

View reviewed changes

abrooksv merged commit 5d6ef86 into main Apr 29, 2026
101 of 102 checks passed

abrooksv deleted the austin.brooks/scanSecretsIdeEndpoint branch April 29, 2026 16:04

		SecretRule::try_from(api_rule)
		.map_err(\|e\| format!("Failed to convert secret rule: {e}"))

-                SecretRule::try_from(api_rule)
-                    .map_err(|e| format!("Failed to convert secret rule: {e}"))
+                match std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+                    SecretRule::try_from(api_rule)
+                })) {
+                    Ok(rule) => rule.map_err(|e| format!("Failed to convert secret rule: {e}")),
+                    Err(_) => Err(
+                        "Failed to convert secret rule: malformed secret rule payload"
+                            .to_string(),
+                    ),
+                }

Conversation

abrooksv commented Apr 24, 2026

What problem are you trying to solve?

Uh oh!

datadog-official Bot commented Apr 24, 2026 • edited by datadog-prod-us1-4 Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

abrooksv Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

jasonforal Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

abrooksv Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

abrooksv Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jasonforal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jasonforal Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

robertohuertasm-datadog left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

datadog-official Bot commented Apr 24, 2026 •

edited by datadog-prod-us1-4 Bot

Loading