Skip to content

deps: security bumps (x/crypto, otel/sdk, chi) - #3071

Open
momosh-ssv wants to merge 5 commits into
stagefrom
deps/security-bumps
Open

momosh-ssv wants to merge 5 commits into
stagefrom
deps/security-bumps

Conversation

@momosh-ssv

@momosh-ssv momosh-ssv commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

One commit per dependency so each can be reverted alone. Each lands in a different part of the node, so a problem on stage points at one commit.

Supersedes #2942, #2708, #2322.

Dependency Version Code it touches How to check on stage
golang.org/x/crypto 0.48.0 -> 0.52.0 (root + ssvsigner) ssvsigner/keys (hkdf), ssvsigner/tls (pkcs12) ssvsigner e2e, TLS handshake
go.opentelemetry.io/otel/sdk, sdk/metric 1.41.0 -> 1.45.0 observability/metrics, observability/traces metrics scrape
github.com/go-chi/chi/v5 5.0.8 -> 5.2.2 api/server exporter API smoke

Why #2942 failed CI: it bumped only ssvsigner/go.mod, but the root module replaces ssvsigner with ./ssvsigner, so root go.mod needs tidying too.

Verified locally: build, vet, lint, unit tests (both modules, network packages excluded, CI covers those).

Note: these are the Renovate-proposed versions, not the advisory-free ones. Still open on the bumped modules (govulncheck v1.1.4 at 11ded7f):

  • x/crypto 0.52.0: GO-2026-6354/6355/6303, all in x/crypto/ssh. The only trace is ssvsigner's e2e testenv via testcontainers, not shipped code.
  • chi 5.2.2: 4 advisories vs 2 on 5.0.8 (adds GO-2026-4316 RedirectSlashes and GO-2026-5774 RealIP). None reachable, api/server only uses Throttle, Compress and NewWrapResponseWriter.
  • otel OTLP trace exporters stay at v1.41.0: GO-2026-6505, reached only when OTLP trace export is enabled (opt-in).

Not touched here, but reachable in both modules: x/text 0.37.0 GO-2026-5970 via keystore encrypt/decrypt.

The stacked follow-ups clear all of the above: #3093 (grpc v1.83.2, root x/net, x/crypto, x/text) and #3094 (chi 5.3.2, x/crypto 0.56.0 and x/net/x/text in ssvsigner, otel exporters aligned to v1.45.0).

@momosh-ssv
momosh-ssv requested review from a team as code owners September 29, 2026 13:40
@codecov

codecov Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.6%. Comparing base (41069c1) to head (a3ccb4c).

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@greptile-apps

greptile-apps Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

[High risk] Updates core dependencies including HTTP router and observability SDK.

The PR should not merge until the new metrics-scrape test uses a metric name consistent with its test environment.

Findings

  1. P1 Scrape test expects wrong name ▶

Summary

The PR updates cryptography, OpenTelemetry, and chi dependencies, aligns semantic-convention imports, and adds resource and metrics-scrape tests. The new scrape test appears to expect a metric name produced only under a naming scheme its test package does not initialize.

Reviews (2) · Last reviewed commit: "observability/metrics: assert recorded m..."

Comment thread ssvsigner/go.mod
Comment thread go.mod
iurii-ssv
iurii-ssv previously approved these changes Sep 30, 2026

@iurii-ssv iurii-ssv left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LTGM, but we should probably look into those Greptile findings before merging.

y0sher
y0sher previously approved these changes Sep 30, 2026
@momosh-ssv

Copy link
Copy Markdown
Contributor Author

Both Greptile findings are addressed: metrics scrape assertion added in 11ded7f, PKCS#12 covered by the signer E2E run (details in thread). The pushes dismissed the earlier approvals, could you take another look?

@iurii-ssv

Copy link
Copy Markdown
Contributor

@greptile pls re-review

Comment thread observability/metrics/provider_test.go
@momosh-ssv

Copy link
Copy Markdown
Contributor Author

@iurii-ssv @y0sher the re-review raised one finding on the scrape test, which is a false positive (details in the thread, the test passes at 11ded7f). All threads are resolved and CI is green, could you re-approve when you get a chance?

@iurii-ssv
iurii-ssv changed the base branch from stage to main October 7, 2026 11:11
@iurii-ssv
iurii-ssv added this pull request to stack #3095 October 7, 2026 11:11
@iurii-ssv
iurii-ssv removed this pull request from stack #3095 October 7, 2026 11:11
@iurii-ssv
iurii-ssv changed the base branch from main to stage October 7, 2026 11:11
@iurii-ssv
iurii-ssv added this pull request to stack #3096 October 7, 2026 11:12

@iurii-ssv iurii-ssv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cross-checked against govulncheck. Notes are inline and none of them block this PR.

grpc is still at v1.79.1 here (5 advisories). #3093 bumps it to v1.83.2 and is stacked on this PR because it needs this PR's otel v1.45.0 + semconv v1.43.0 move. Bumping grpc on stage directly pulls otel/sdk to 1.44 while semconv stays on 1.39, and buildResources then fails at startup with a schema URL conflict. #3094 on top of that handles the remaining cleanup mentioned below.

Comment thread go.mod
Comment thread go.mod
Comment thread go.mod
Comment thread go.mod
iurii-ssv
iurii-ssv previously approved these changes Oct 8, 2026

@iurii-ssv iurii-ssv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@y0sher
y0sher dismissed iurii-ssv’s stale review October 13, 2026 13:07

The merge-base changed after approval.

Base automatically changed from stage to main October 13, 2026 13:18
Supersedes renovate #2942, which only touched ssvsigner/go.mod; the root
module replaces ssvsigner with ./ssvsigner so root go.mod needs tidying too.
Supersedes renovate #2322 (abandoned).
otel/sdk v1.45.0's resource.Default() reports the semconv 1.43.0 schema.
With resources.go still on v1.39.0, resource.Merge returns
ErrSchemaURLConflict and start_node exits via log.Fatalf (same failure
as #3020). Moves the root module's semconv imports to v1.43.0; the
attribute keys used are unchanged between the two versions.

ssvsigner keeps v1.39.0: its own go.mod is on otel v1.41.0, which does
not ship v1.43.0, and it only uses attribute keys (no resource merge).

Adds TestBuildResources so the next otel/sdk bump fails CI instead of
node startup.
…crape

The Prometheus exporter stays at v0.58.0 while otel/sdk/metric moves to
v1.45.0. TestHandler_Start only checks for HTTP 200, so an exporter/sdk
incompatibility that drops measurements would pass. Records a counter
through InitializeProvider and asserts its value in the parsed scrape.
@momosh-ssv
momosh-ssv removed this pull request from stack #3096 October 15, 2026 07:36
@momosh-ssv
momosh-ssv changed the base branch from main to stage October 15, 2026 07:37
@momosh-ssv
momosh-ssv added this pull request to stack #3123 October 15, 2026 07:40

@iurii-ssv iurii-ssv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, just 1 nit to adjust PR description.

// (it advances when otel/sdk is bumped); a mismatch makes the resource.Merge
// below fail with ErrSchemaURLConflict and aborts node startup (issue #3020).
semconv "go.opentelemetry.io/otel/semconv/v1.39.0"
semconv "go.opentelemetry.io/otel/semconv/v1.43.0"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Process note: this import move and the otel/sdk bump only work as a pair, so "each can be reverted alone" from the description doesn't hold for those two commits. I tried both on this branch:

The otel/sdk commit on its own has the same startup failure, so a bisect could land on it if the commits go in individually. I'd note in the description that the two revert together.

This branch has not been deployed

No deployments
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