Skip to content
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

fix: avoid problematic serde release #15481

Closed
wants to merge 2 commits into from

Conversation

matklad
Copy link
Member

@matklad matklad commented Aug 19, 2023

serde 1.0.172 and up rely on opaque non-reproducible binary blobs to
function, explicitly not providing a library-level opt-out.

This is problematic for two reasons:

  • directly, unauditable binary blobs are a security issue.
  • indirectly, it becomes much harder to predict future behaviors of the
    crate.

As such, I am willing to go on a limb here and forbid building
rust-analyzer with those versions of serde. Normally, my philosophy is
to defer the choice to the end user, but it's also a design constraint
of rust-analyzer that we don't run random binaries downloaded from the
internet without explicit user's concent.

Concretely, this upper-bounds serde for both rust-analyzer workspace, as
well as the lsp-server lib.

See serde-rs/serde#2538 for wider context.

serde 1.0.172 and up rely on opaque non-reproducible binary blobs to
function, explicitly not providing a library-level opt-out.

This is problematic for two reasons:

- directly, unauditable binary blobs are a security issue.
- indirectly, it becomes much harder to predict future behaviors of the
  crate.

As such, I am willing to go on a limb here and forbid building
rust-analyzer with those versions of serde. Normally, my philosophy is
to defer the choice to the end user, but it's also a design constraint
of rust-analyzer that we don't run random binaries downloaded from the
internet without explicit user's concent.

Concretely, this upper-bounds serde for both rust-analyzer workspace, as
well as the lsp-server lib.

See serde-rs/serde#2538 for wider context.
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 19, 2023
@@ -9,7 +9,8 @@ edition = "2021"
[dependencies]
log = "0.4.17"
serde_json = "1.0.96"
serde = { version = "1.0.156", features = ["derive"] }
# See https://github.com/serde-rs/serde/issues/2538#issuecomment-1684517372 for why we pin serde
serde = { version = "1.0.156, < 1.0.171", features = ["derive"] }
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@matklad ☝️

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, looks like 1.0.171 is the latest version without it.

@matklad matklad closed this Aug 19, 2023
@matklad
Copy link
Member Author

matklad commented Aug 19, 2023

reopened as #15482 --- github's having trouble with updateing the PR after a --force-with-lease?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants