Skip to content

refactor: resolve vulns and upgrade#1788

Merged
taddes merged 9 commits intomasterfrom
refactor/resolve-vulns-STOR-352
Mar 11, 2026
Merged

refactor: resolve vulns and upgrade#1788
taddes merged 9 commits intomasterfrom
refactor/resolve-vulns-STOR-352

Conversation

@taddes
Copy link
Copy Markdown
Collaborator

@taddes taddes commented Sep 3, 2025

Description

Resolves a number of security vulnerabilities raised by cargo audit and several outdated packages that introduced breaking changes, therefore we didn't update.

Will need some context from @pjenvey on deadpool and protobuf, as the effects of updating those may be larger. grpcio warnings remain.

Regarding the historical issue holding us back with config:

The historical comment next to the dep said: "pin to 11, 12+ introduces a breaking change for env vars".

Turns out in config 0.12, the prefix is separated from the key using the same separator we configure. So with separator __, the env var SYNC_TOKENSERVER__ENABLED would need to become SYNC__TOKENSERVER__ENABLED. The old SYNC_ prefix format no longer worked.

This means in CI, SYNC_TOKENSERVER__DATABASE_URL and SYNC_SYNCSTORAGE__DATABASE_URL were never parsed and the settings fell back to defaults (empty), causing the test failures.

The fix is to restore the _ prefix delimiter explicitly using the newerprefix_separator method.

Testing

compilation

Issue(s)

Closes STOR-352.

@taddes taddes self-assigned this Sep 3, 2025
Comment thread syncserver-settings/Cargo.toml Outdated
slog-scope.workspace = true

config = "0.11" # pin to 11, 12+ introduces a breaking change for env vars.
config = "0.15" # pin to 11, 12+ introduces a breaking change for env vars.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
config = "0.15" # pin to 11, 12+ introduces a breaking change for env vars.
config = "0.15"

@taddes taddes force-pushed the refactor/resolve-vulns-STOR-352 branch from b26bd8b to a392c75 Compare February 9, 2026 15:59
@taddes taddes force-pushed the refactor/resolve-vulns-STOR-352 branch 2 times, most recently from 68f3a1e to 5c07731 Compare March 3, 2026 20:30
@taddes taddes changed the title WIP: refactor: resolve vulns and upgrade refactor: resolve vulns and upgrade Mar 5, 2026
@taddes taddes force-pushed the refactor/resolve-vulns-STOR-352 branch from 9fe7a88 to fda2fc6 Compare March 5, 2026 02:18
@taddes taddes marked this pull request as ready for review March 5, 2026 13:59
@taddes taddes requested a review from pjenvey March 5, 2026 14:01
@taddes taddes force-pushed the refactor/resolve-vulns-STOR-352 branch 2 times, most recently from d6e07a0 to 4fb5324 Compare March 5, 2026 15:56
@pjenvey
Copy link
Copy Markdown
Member

pjenvey commented Mar 5, 2026

Will need some context from @pjenvey on deadpool and protobuf, as the effects of updating those may be larger. grpcio warnings remain.

I don't recall any issues with moving to a new deadpool.

Are there grpcio warnings on this branch currently? Or when upgrading protobuf?

We should be able to go to max protobuf 2.28 per the google-cloud-rust-raw package we depend on, which locks it to that version. It looks like we have a protobuf req in the root Cargo.toml but I'm not sure why, it's only used by syncstorage-spanner which specifies its own dep there. Can we kill it from the root?

@taddes
Copy link
Copy Markdown
Collaborator Author

taddes commented Mar 5, 2026

Will need some context from @pjenvey on deadpool and protobuf, as the effects of updating those may be larger. grpcio warnings remain.

I don't recall any issues with moving to a new deadpool.

Are there grpcio warnings on this branch currently? Or when upgrading protobuf?

We should be able to go to max protobuf 2.28 per the google-cloud-rust-raw package we depend on, which locks it to that version. It looks like we have a protobuf req in the root Cargo.toml but I'm not sure why, it's only used by syncstorage-spanner which specifies its own dep there. Can we kill it from the root?

Removing protobuf from the root, though funny cargo machete never saw it? Perhaps a re-declaration in a child crate doesn't show it, since it's not using the workspace default.

I’ve looked at https://lib.rs/crates/google-cloud-rust-raw to check the max protobuf version, which appears to be 3.4.0. Looks like we can bump that and as per your comment earlier in that PR, but not higher than what is a dep in google-cloud-rust-raw, right?

Ah, nevermind about deadpool now... I remember. We can't upgrade yet since there are some compat issues between it and diesel-async, gotta wait a little longer before bumping.

@taddes taddes force-pushed the refactor/resolve-vulns-STOR-352 branch from 04ee009 to 32467a0 Compare March 5, 2026 22:03
Comment thread syncstorage-spanner/Cargo.toml Outdated
# `cargo build --features grpcio/openssl ...`
grpcio = { version = "0.13.0", features = ["openssl"] }
protobuf = { version = "2.28.0" } # must match what's used by googleapis-raw
protobuf = { version = "2.28.0" } # must match what's used by google-cloud-rust-raw
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

"=2.28.0"?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I don't think we were getting issues with this, but makes sense that we pin the exact version. Thx

@taddes taddes force-pushed the refactor/resolve-vulns-STOR-352 branch from 32467a0 to d1c2f32 Compare March 11, 2026 15:41
Comment thread Cargo.toml
regex = "1.12"
reqwest = { version = "0.12", default-features = false, features = [
"rustls-tls",
reqwest = { version = "0.13.2", default-features = false, features = [
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

FYI I usually use cargo upgrade from the cargo edit suite to update deps in Cargo.toml (it would have upgraded to "0.13" here, leaving off the patch version because the original value lacked a patch version)

@taddes taddes merged commit 88e7eb9 into master Mar 11, 2026
30 checks passed
@taddes taddes deleted the refactor/resolve-vulns-STOR-352 branch March 11, 2026 19:46
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.

3 participants