Repository navigation
fix(crypto): gate libdd-common TLS features in remaining internal crates + add CI guard #1943
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
gh-worker-dd-mergequeue-cf854d
merged 6 commits into
main
from
jordan.gonzalez/fix-trace-stats-data-pipeline-fips
May 5, 2026
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
e7be98f
fix(crypto): gate libdd-common TLS features in remaining internal cra…
duncanista 4cf2292
fix(scripts): treat unexpected cargo tree errors as hard failures
duncanista 9a37d22
fix(scripts): match cargo tree heading line-anchored, not stdout-anch…
duncanista 0637cf8
feat(scripts): broaden crypto check to all workspace crates
duncanista 5e5fb07
refactor(scripts): drive crypto check off cargo metadata jq output
duncanista 225c04a
Merge branch 'main' into jordan.gonzalez/fix-trace-stats-data-pipelin…
duncanista File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| name: 'Check crypto providers' | ||
| on: | ||
| pull_request: | ||
| push: | ||
| branches: | ||
| - main | ||
| - mq-working-branch-* | ||
| env: | ||
| rust_version: "1.84.1" | ||
| CARGO_TERM_COLOR: always | ||
| CARGO_INCREMENTAL: 0 | ||
| jobs: | ||
| check-crypto-providers: | ||
| name: "ring + aws-lc-rs are not both pulled into any workspace crate" | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout sources | ||
| uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # 4.2.2 | ||
| - name: Install ${{ env.rust_version }} toolchain | ||
| run: rustup set profile minimal && rustup install ${{ env.rust_version }} && rustup default ${{ env.rust_version }} | ||
| - name: Cache [rust] | ||
| uses: Swatinem/rust-cache@f13886b937689c021905a6b90929199931d60db1 # 2.8.1 | ||
| with: | ||
| cache-targets: true # cache build artifacts | ||
| cache-bin: true # cache the ~/.cargo/bin directory | ||
| - name: Run crypto provider check | ||
| run: ./scripts/check_crypto_providers.sh | ||
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,132 @@ | ||
| #!/bin/bash | ||
| # Copyright 2026-Present Datadog, Inc. https://www.datadoghq.com/ | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| # | ||
| # Asserts that no workspace crate ends up with both `ring` and `aws-lc-rs` in | ||
| # its runtime dependency graph at the same time. | ||
| # | ||
| # Mixing both backends bloats release binaries (e.g. datadog-lambda-extension | ||
| # pulls a few hundred KiB of unused crypto) and breaks downstream FIPS | ||
| # compliance checks. See #1816 and #1872 for the original gating work. | ||
| # | ||
| # Iterates every workspace member (libdd-* and the datadog-* / sidecar / | ||
| # remote-config / live-debugger / etc. crates that depend transitively on | ||
| # libdd-common) and runs: | ||
| # * default feature set (whatever `cargo` picks) | ||
| # * `--no-default-features --features fips` if the crate exposes `fips` | ||
| # * `--no-default-features --features https` if the crate exposes `https` | ||
| # | ||
| # Each crate is resolved against its own Cargo.toml so workspace-level feature | ||
| # unification from other members does not skew the result, and dev-deps are | ||
| # excluded so test-only graphs do not produce false positives. | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
| ROOT_DIR="$(dirname "$SCRIPT_DIR")" | ||
|
|
||
| cd "$ROOT_DIR" | ||
|
|
||
| # crate_has_feature <pkg_json> <feature_name> | ||
| # 0 if the cargo-metadata package blob declares the named feature, 1 otherwise. | ||
| crate_has_feature() { | ||
| printf '%s' "$1" | jq -e --arg feat "$2" '.features | has($feat)' > /dev/null 2>&1 | ||
| } | ||
|
|
||
| # pulls <manifest> <package> [extra-cargo-flags...] | ||
| # 0 if `package` is in the runtime dep graph of `manifest`. Resolved against | ||
| # the crate's manifest in isolation, with dev-deps excluded. | ||
| # | ||
| # `cargo tree -i` has three observed outcomes: | ||
| # - exit 0, output starts with "<pkg> v..." → pulled | ||
| # - exit 0, output contains "nothing to print" → not pulled (in workspace | ||
| # but not in this graph) | ||
| # - exit non-zero, "did not match any packages" → not pulled (not in | ||
| # workspace at all) | ||
| # Anything else (transient registry / git failures, manifest errors, etc.) | ||
| # is surfaced as a hard error so the guard cannot silently pass when | ||
| # dependency resolution itself broke. | ||
| pulls() { | ||
| local manifest="$1" pkg="$2" | ||
| shift 2 | ||
| local output rc | ||
| output=$(cargo tree --manifest-path "$manifest" --edges no-dev "$@" -i "$pkg" 2>&1) && rc=0 || rc=$? | ||
|
|
||
| # The tree heading line is "<pkg> v<version>" — match it line-anchored so | ||
| # `Downloading crates...` progress noise printed first does not throw us off. | ||
| if grep -qE "^${pkg} v[0-9]" <<<"$output"; then | ||
| return 0 | ||
| fi | ||
| if [[ "$output" == *"nothing to print"* ]] || \ | ||
| [[ "$output" == *"did not match any packages"* ]]; then | ||
| return 1 | ||
| fi | ||
|
|
||
| echo "ERROR: cargo tree failed for $(basename "$(dirname "$manifest")") -i $pkg (exit $rc):" >&2 | ||
| echo "$output" | sed 's/^/ /' >&2 | ||
| exit 2 | ||
| } | ||
|
|
||
| tree_for() { | ||
| local manifest="$1" pkg="$2" | ||
| shift 2 | ||
| cargo tree --manifest-path "$manifest" --edges no-dev "$@" -i "$pkg" 2>&1 | sed 's/^/ /' | ||
| } | ||
|
|
||
| # check <manifest> <label> [cargo flags...] | ||
| # Fails if the dep graph contains both ring and aws-lc-rs. | ||
| check() { | ||
| local manifest="$1" label="$2" | ||
| shift 2 | ||
| local crate | ||
| crate="$(basename "$(dirname "$manifest")")" | ||
|
|
||
| if pulls "$manifest" "ring" "$@" && pulls "$manifest" "aws-lc-rs" "$@"; then | ||
| echo "FAIL: $crate ($label) pulls both ring and aws-lc-rs" | ||
| tree_for "$manifest" "ring" "$@" | ||
| tree_for "$manifest" "aws-lc-rs" "$@" | ||
| return 1 | ||
| fi | ||
| return 0 | ||
| } | ||
|
|
||
| errors=0 | ||
| checked=0 | ||
|
|
||
| # Enumerate all workspace members (libdd-* and the datadog-* / sidecar / | ||
| # live-debugger / remote-config / ffi crates) via `cargo metadata` so we | ||
| # don't have to hardcode prefixes or maintain an allow-list. | ||
| METADATA=$(cargo metadata --no-deps --format-version 1) | ||
|
|
||
| mapfile -t PACKAGES < <(printf '%s' "$METADATA" | jq -c \ | ||
| '.packages[] | {manifest: .manifest_path, features: .features}') | ||
|
|
||
| for pkg in "${PACKAGES[@]}"; do | ||
| manifest=$(printf '%s' "$pkg" | jq -r '.manifest') | ||
| checked=$((checked + 1)) | ||
|
|
||
| check "$manifest" "default" || errors=$((errors + 1)) | ||
|
|
||
| if crate_has_feature "$pkg" "https"; then | ||
| check "$manifest" "--features https" --no-default-features --features https \ | ||
| || errors=$((errors + 1)) | ||
| fi | ||
|
|
||
| if crate_has_feature "$pkg" "fips"; then | ||
| check "$manifest" "--features fips" --no-default-features --features fips \ | ||
| || errors=$((errors + 1)) | ||
| fi | ||
| done | ||
|
duncanista marked this conversation as resolved.
|
||
|
|
||
| if [ "$checked" -eq 0 ]; then | ||
| echo "no workspace crates found" >&2 | ||
| exit 2 | ||
| fi | ||
|
|
||
| if [ "$errors" -gt 0 ]; then | ||
| echo | ||
| echo "crypto provider check failed: $errors violation(s) across $checked crate(s)" | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "crypto provider check passed for $checked crate(s)" | ||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're running this on PRs and MQ branches, do we need to run it on main too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep —
mainis already in thepush: branches:list on line 6 itself. Kept it consistent with howtest.yml,lint.yml,fuzz.yml,coverage.yml, etc. all trigger:pull_request+push: branches: [main, mq-working-branch-*]. Running on the post-mergemainpush gives a baseline status on the default branch and catches anything that slips through (rebase races, MQ skips, etc.) — small CI cost vs. the convention break of opting out only here.