fix: add server ReadTimeouts, update security deps#1980
Conversation
✅ Deploy Preview for polite-licorice-3db33c canceled.
|
There was a problem hiding this comment.
Code Review
This pull request updates various dependencies, including upgrading Go modules in the integration tests and updating frontend packages in the playground application. However, critical issues were identified in the frontend dependency updates: the specified versions for @vitejs/plugin-react (^6.0.2) and vite (^8.0.16) do not exist on the public npm registry, which will cause build and installation failures.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
fe15340 to
cbc8680
Compare
cbc8680 to
bd2e47a
Compare
|
Warning Review limit reached
Next review available in: 5 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (9)
📝 WalkthroughWalkthroughChangesDependency Updates
h2c Server Hardening
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR resolves patchable Dependabot security alerts by upgrading Go module dependencies across core, flagd, flagd-proxy, and test modules, and by enforcing patched npm transitive versions in the playground app.
Changes:
- Bump
github.com/containerd/containerd/v2(and related container tooling deps) intest/integration. - Update
golang.org/x/*and related Go dependencies acrosscore,flagd, andflagd-proxy; add targetedstaticchecksuppressions for deprecatedh2c. - Add npm
overridesinplayground-appto force patchedjs-cookieand@babel/coreversions (and refresh lockfile accordingly).
Reviewed changes
Copilot reviewed 10 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
test/zero-downtime-flagd-proxy/go.mod |
Updates test-module Go dependencies (grpc + indirects) to newer patched versions. |
test/zero-downtime-flagd-proxy/go.sum |
Aligns checksums with the updated go.mod dependency graph. |
test/integration/go.mod |
Bumps testcontainers and container-related transitive dependencies (incl. containerd/containerd/v2). |
test/integration/go.sum |
Updates checksums for the upgraded integration-test dependency set. |
playground-app/package.json |
Adds npm overrides to force patched js-cookie and @babel/core versions. |
playground-app/package-lock.json |
Refreshes lockfile to reflect overrides and updated transitive dependency resolutions. |
flagd/pkg/service/middleware/h2c/h2c.go |
Documents and suppresses staticcheck deprecation warnings for h2c.NewHandler usage. |
flagd/pkg/service/flag-evaluation/connect_service.go |
Keeps plaintext HTTP/2 metrics support via h2c while suppressing deprecation lint warnings with rationale. |
flagd/go.mod |
Upgrades golang.org/x/net and related golang.org/x/* indirects to patched versions. |
flagd/go.sum |
Updates checksums for the bumped Go dependencies in flagd. |
flagd-proxy/pkg/service/server.go |
Same h2c deprecation suppression + rationale for the proxy metrics server. |
flagd-proxy/go.mod |
Upgrades golang.org/x/net and related golang.org/x/* indirects to patched versions. |
flagd-proxy/go.sum |
Updates checksums for the bumped Go dependencies in flagd-proxy. |
flagd-proxy/tests/loadtest/go.mod |
Updates loadtest module dependencies (grpc + indirects) to newer patched versions. |
flagd-proxy/tests/loadtest/go.sum |
Aligns checksums with the updated loadtest module dependency graph. |
core/go.mod |
Upgrades golang.org/x/* dependencies in core to patched versions. |
core/go.sum |
Updates checksums for the bumped Go dependencies in core. |
Files not reviewed (1)
- playground-app/package-lock.json: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@flagd-proxy/pkg/service/server.go`:
- Around line 149-157: The metrics server in server.go only sets
ReadHeaderTimeout on the http.Server, so slow request bodies can still hold the
h2c listener open. Update the s.metricsServer initialization to add a
ReadTimeout on the http.Server in addition to the existing ReadHeaderTimeout,
and consider setting WriteTimeout and IdleTimeout there as well. Use the
existing s.metricsServer construction and h2c.NewHandler setup as the place to
apply the timeout hardening.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d8ce310f-79bc-4832-a0e4-078e11115464
⛔ Files ignored due to path filters (6)
core/go.sumis excluded by!**/*.sumflagd-proxy/go.sumis excluded by!**/*.sumflagd-proxy/tests/loadtest/go.sumis excluded by!**/*.sumflagd/go.sumis excluded by!**/*.sumtest/integration/go.sumis excluded by!**/*.sumtest/zero-downtime-flagd-proxy/go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
core/go.modflagd-proxy/go.modflagd-proxy/pkg/service/server.goflagd-proxy/tests/loadtest/go.modflagd/go.modflagd/pkg/service/flag-evaluation/connect_service.goflagd/pkg/service/middleware/h2c/h2c.gotest/integration/go.modtest/zero-downtime-flagd-proxy/go.mod
✅ Files skipped from review due to trivial changes (1)
- flagd/pkg/service/middleware/h2c/h2c.go
5eeff7c to
20b4381
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@flagd/pkg/service/flag-evaluation/connect_service.go`:
- Around line 296-304: Update the management http.Server construction in the
metrics server setup to add a ReadTimeout (or equivalent request-body read
limit) alongside ReadHeaderTimeout, ensuring slow h2c request bodies cannot hold
the listener open while preserving the existing health, metrics, and gRPC health
handlers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0b50a7a5-c184-429f-843d-4d72d741a190
⛔ Files ignored due to path filters (4)
flagd-proxy/tests/loadtest/go.sumis excluded by!**/*.sumplayground-app/package-lock.jsonis excluded by!**/package-lock.jsontest/integration/go.sumis excluded by!**/*.sumtest/zero-downtime-flagd-proxy/go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
flagd-proxy/pkg/service/server.goflagd-proxy/tests/loadtest/go.modflagd/pkg/service/flag-evaluation/connect_service.goflagd/pkg/service/middleware/h2c/h2c.goplayground-app/package.jsontest/integration/go.modtest/zero-downtime-flagd-proxy/go.mod
🚧 Files skipped from review as they are similar to previous changes (3)
- test/zero-downtime-flagd-proxy/go.mod
- flagd-proxy/tests/loadtest/go.mod
- flagd/pkg/service/middleware/h2c/h2c.go
20b4381 to
1bc6b5e
Compare
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
- golang.org/x/net v0.52.0/v0.48.0 -> v0.55.0 in flagd, core, flagd-proxy, flagd-proxy/tests/loadtest, test/zero-downtime-flagd-proxy (medium, alerts #201-206) - github.com/testcontainers/testcontainers-go v0.41.0 -> v0.43.0 in test/integration, which migrates off github.com/containerd/containerd/v2 and picks up patched transitive deps (moby/buildkit, docker/buildx, docker/compose) used for alert #152's fix chain - suppress newly-introduced staticcheck deprecation warnings for golang.org/x/net/http2/h2c (deprecated as of x/net v0.55.0) with nolint directives; migrating to http.Server.Protocols is a larger refactor out of scope for this dependency bump Unresolvable: github.com/docker/docker (alerts #151, #152, #184, #185, unreachable transitive entry retained by Go's module graph pruning (no code path imports it); the advisory's patched version 29.3.1 has no corresponding tag in the Go module proxy for the legacy github.com/docker/docker import path (moby has moved new majors to github.com/moby/moby/v2), so it cannot be bumped without a replace/override, which is disallowed for this published project. Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
…edback - github.com/sigstore/sigstore-go v1.1.4 -> v1.2.2 in test/integration (medium, alert #260) - js-yaml ^4.1.1 -> ^4.2.0 and vite ^6.4.2 -> ^6.4.3 in playground-app (medium/high, alerts #207, #199, #200); lockfile already resolved these transitively, this pins the direct specifiers to match - Root-cause fix for a regression introduced by the prior testcontainers-go v0.41.0 -> v0.43.0 bump: v0.43.0's modules/compose changed DockerContainer.MappedPort's signature (nat.Port -> string), which is incompatible with go-sdk-contrib/tests/flagd/v2@v2.0.2's testframework (pinned, no newer version exists) and broke `go vet`/`go test` compilation in test/integration (go build alone didn't catch it since _test.go files aren't compiled by `go build`). Reverted testcontainers-go and testcontainers-go/modules/compose to v0.41.0 (API-compatible) while keeping the already-patched transitive containerd/containerd/v2 v2.2.5 and other bumps pinned via explicit go.mod requires, so alerts #189, #192-#196 remain resolved without reintroducing the vulnerable containerd version. - Add ReadTimeout to the flagd-proxy metrics server's http.Server per CodeRabbit review feedback on this PR: ReadHeaderTimeout alone only bounds header parsing, leaving the plain HTTP/h2c port open to slow-client (Slowloris-style) resource exhaustion. Still unresolvable: github.com/docker/docker (alerts #151, #152, #184, #185, #186) remains at v28.5.2+incompatible in test/integration. No version >= 28.5.2 (including the advisory's referenced 29.3.1 for alert #152) is published under the legacy github.com/docker/docker import path in the Go module proxy; moby now ships new majors under github.com/moby/moby/v2, which this module graph cannot switch to without deeper dependency surgery. Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
cd4b159 to
96a536d
Compare
|



Summary
Resolves all cleanly patchable open Dependabot security alerts (28 of 33). The remaining 5 have no available patched version for their legacy import path — see below.
Dependabot Alerts Resolved
github.com/sigstore/sigstore-gotest/integrationgolang.org/x/cryptomain(v0.52.0+); carried through the rebasejs-yaml^4.2.0inplayground-appgolang.org/x/netflagd,core,flagd-proxy,test/integration, and related test modulesvite^6.4.3inplayground-app@babel/core>=7.29.6via npm override inplayground-appgithub.com/containerd/containerd/v2test/integration(kept ontestcontainers-gov0.41.0 — see note below)js-cookie>=3.0.7via npm override inplayground-appNote on containerd fix: an earlier revision of this branch bumped
testcontainers-goto v0.43.0 to reach patchedcontainerd/v2, but that also changedDockerContainer.MappedPort's signature in a way that's incompatible withgo-sdk-contrib/tests/flagd/v2@v2.0.2(pinned; no newer version exists) — breakinggo vet/go testcompilation intest/integration(not caught bygo buildalone, since it doesn't compile_test.gofiles). Revertedtestcontainers-goto v0.41.0 (API-compatible) while keepingcontainerd/containerd/v2pinned at v2.2.5 via an explicitgo.modrequire — standard Go MVS, noreplacedirective needed.Unresolvable Alerts
github.com/docker/docker(#151, #152, #184, #185, #186) intest/integration: pinned atv28.5.2+incompatible. No version ≥ that (including the advisory's referencedv29.3.1for #152) exists under the legacygithub.com/docker/dockerimport path in the Go module proxy — moby now ships new majors undergithub.com/moby/moby/v2. Transitive dependency of testcontainers/buildx/compose; unresolvable until those upstreams migrate.github.com/docker/dockergithub.com/docker/dockergithub.com/docker/dockergithub.com/docker/dockergithub.com/docker/dockerOther changes
ReadTimeoutto the flagd-proxy metrics server'shttp.Server(flagd-proxy/pkg/service/server.go) per CodeRabbit review feedback, hardening the plain HTTP/h2c port against slow-client resource exhaustion.Verification
make test,make build, andgo vetpass across all Go modules (core,flagd,flagd-proxy,flagd-proxy/tests/loadtest,test/zero-downtime-flagd-proxy).test/integration:go build ./...,go vet ./..., andgo test(compile-only; actual container run needs a Docker daemon) all pass.playground-app:npm run buildsucceeds;npm install --package-lock-only --dry-runreports up to date.