feat(secrets): bump dd-sds to b2dca51 and revert #921 stack-overflow workaround#925
Open
isabella-garza-datadog wants to merge 3 commits into
Open
feat(secrets): bump dd-sds to b2dca51 and revert #921 stack-overflow workaround#925isabella-garza-datadog wants to merge 3 commits into
isabella-garza-datadog wants to merge 3 commits into
Conversation
|
🎯 Code Coverage (details) 🔗 Commit SHA: 13f502a | Docs | Datadog PR Page | Give us feedback! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the secrets scanning integration to use the upstream-fixed dd-sds implementation again, restoring scan_with_options(validate_matches=true) behavior (including supporting-rule match-pairing) and removing the temporary workaround introduced for the stack overflow incident.
Changes:
- Bump the
dd-sdsgit dependency to commitb2dca51…(now viapackage = "dd-sensitive-data-scanner"alias). - Revert the incident-56036 workaround by switching
find_secretsback toscan_with_options(...)instead ofscan() + validate_matches(). - Re-enable the previously ignored supporting-rule match-pairing test.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/secrets/src/scanner.rs | Restore scan_with_options call path and re-enable supporting-rule match-pairing test. |
| crates/secrets/Cargo.toml | Bump dd-sds dependency to upstream-fixed commit and alias renamed package. |
| Cargo.lock | Lockfile updates from the dd-sds dependency bump (transitive dep changes). |
Comments suppressed due to low confidence (1)
crates/secrets/src/scanner.rs:16
- The doc comment above
build_sds_scannerstill says to usescanner.scan()to find secrets, butfind_secretsnow usesScanner::scan_with_options(and this file no longer usesscan()directly). Updating the comment will prevent confusion for future readers.
use dd_sds::{RootRuleConfig, RuleConfig, ScanOptionBuilder, Scanner};
use itertools::Itertools;
use std::sync::Arc;
/// Build the SDS scanner used to scan all code using the rules fetched from
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
804fd17 to
a813941
Compare
Restore main's Cargo.lock and reconcile only the dd-sds change instead of fully regenerating, which avoided unrelated downgrades (socket2 0.6->0.5, windows-sys 0.61/0.60->0.59) that churned the dependency graph. This keeps v8's dependency subtree identical to main so the prebuilt librusty_v8.a stays cache-valid in the check-regressions job, fixing the "could not find native static library rusty_v8" failure on the cold rebuild. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem are you trying to solve?
dd-sdscommitb2dca51contains an upstream fix for the stack overflow inScanner::scan_with_options()that was the root cause of #incident-56036. PR #921 worked around that crash by switching toscan() + validate_matches()as a temporary measure, but this workaround regressed theis_supporting_ruleHTTP match-pairing feature (cross-rule template-variable resolution only fires insidescan_with_options), and the corresponding test was marked #[ignore].What is your solution?
scan_with_options(validate_matches=true)as the single call path infind_secretsand re-enable the previously ignoredtest_supporting_rule_excluded_from_output_but_used_for_match_pairingtest.Alternatives considered
Keeping the PR #921 workaround in place, but that leaves the supporting-rule match-pairing feature permanently broken for validators that rely on cross-rule template variables.
What the reviewer should know
dd-sdstodd-sensitive-data-scanner, so Cargo.toml uses apackage = "dd-sensitive-data-scanner"alias to keep the dd_sds:: import path unchanged.Testing
Ran secret scanning on
dd-sourceandweb-ui, both of which previously caused stack overflows.Results are identical before and after this PR (same files scanned, secrets found, and rules matched), confirming no regression in detection — with a small improvement in scan duration.
dd-source
web-ui