Skip to content

fix(NATSRS-010-2): CU-86akbhh82 async-nats/Cargo.toml enables both ring and aws-lc-rs unconditionally by default - #136

Draft
flamingo[bot] wants to merge 1 commit into
mainfrom
ai-fix/natsrs-010-2-f9364fee-4fa2224e
Draft

flamingo[bot] wants to merge 1 commit into
mainfrom
ai-fix/natsrs-010-2-f9364fee-4fa2224e

Conversation

@flamingo

@flamingo flamingo Bot commented Sep 14, 2026

Copy link
Copy Markdown

Closes findings from rule NATSRS-010-2 — async-nats/Cargo.toml enables both ring and aws-lc-rs unconditionally by default.

Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.

Warning

This PR edits CI-executable files (workflows, build/manifest definitions). A same-repo PR can run a modified workflow with a write-scoped token as soon as it opens — review those hunks FIRST, before anything else in this PR.

# Fix confidence Finding Location
1 🔴 55 low — review closely async-nats/Cargo.toml enables both ring and aws-lc-rs unconditionally by default async-nats/Cargo.toml:63

What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.


Run: https://product-hub.flamingo.so/admin/code-review
Run id: 4fa2224e-8528-443d-a51a-823c78308977

Merging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.

ClickUp task: CU-86akbhh82 NATS async-nats and CI review findings (9 PRs)

@flamingo flamingo Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 What this fix changed, finding by finding

1 finding(s) fixed in this draft — 1 explained inline on the diff; 1 low-confidence hunk(s) need close review before merging.

Comment thread async-nats/Cargo.toml
num = "0.4.1"

[features]
default = ["server_2_10", "server_2_11", "server_2_12", "ring"]

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🔴 async-nats/Cargo.toml enables both ring and aws-lc-rs unconditionally by default

In async-nats/Cargo.toml, removed ring from the default feature list (line default = ["server_2_10", "server_2_11", "server_2_12", "ring"] changed to default = ["server_2_10", "server_2_11", "server_2_12"]) so that no crypto backend (ring or aws-lc-rs) is unconditionally compiled in via defaults, matching the --at-least-one-of aws-lc-rs,ring cargo-hack policy described in the finding. Risk: this is a behavior-affecting change — consumers relying on default-features = true to get TLS/crypto working via ring will now need to explicitly enable ring or aws-lc-rs themselves, and the crate's own code paths that assume a backend is present (e.g. TLS connector construction) may fail to compile or behave incorrectly with default-features alone; a complete fix would also need to verify all non-dev code compiles with zero backend features enabled (likely gating TLS code behind cfg(any(feature = "ring", feature = "aws-lc-rs")) or similar), which is outside what can be safely done from Cargo.toml alone in this file-scoped change. The dev-dependency async-nats line already disables default features and explicitly lists service, server_2_10, so it is unaffected; the [dev-dependencies] ring = "0.17" line is untouched since it's a separate direct test dependency, not the feature default.

🤖 Prompt for AI agents
In async-nats/Cargo.toml around line 63, review and complete this code-review fix: async-nats/Cargo.toml enables both ring and aws-lc-rs unconditionally by default.
What the draft fix changed: In `async-nats/Cargo.toml`, removed `ring` from the `default` feature list (line `default = ["server_2_10", "server_2_11", "server_2_12", "ring"]` changed to `default = ["server_2_10", "server_2_11", "server_2_12"]`) so that no crypto backend (`ring` or `aws-lc-rs`) is unconditionally compiled in via defaults, matching the `--at-least-one-of aws-lc-rs,ring` cargo-hack policy described in the finding. Risk: this is a behavior-affecting change — consumers relying on `default-features = true` to get TLS/crypto working via `ring` will now need to explicitly enable `ring` or `aws-lc-rs` themselves, and the crate's own code paths that assume a backend is present (e.g. TLS connector construction) may fail to compile or behave incorrectly with default-features alone; a complete fix would also need to verify all non-dev code compiles with zero backend features enabled (likely gating TLS code behind `cfg(any(feature = "ring", feature = "aws-lc-rs"))` or similar), which is outside what can be safely done from Cargo.toml alone in this file-scoped change. The dev-dependency `async-nats` line already disables default features and explicitly lists `service, server_2_10`, so it is unaffected; the `[dev-dependencies] ring = "0.17"` line is untouched since it's a separate direct test dependency, not the feature default.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 55 low — review closely — react 👍/👎 to teach the reviewer

@flamingo flamingo Bot changed the title fix(NATSRS-010-2): async-nats/Cargo.toml enables both ring and aws-lc-rs unconditionally by default fix(NATSRS-010-2): CU-86akbhh82 async-nats/Cargo.toml enables both ring and aws-lc-rs unconditionally by default Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants