Fix CVE-2026-29181: bump OpenTelemetry-Go to v1.41.0#728
Conversation
Resolves HIGH severity vulnerability in OpenTelemetry-Go where multi-value baggage header extraction causes excessive allocations, enabling remote DoS amplification. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@sheltoncyril: No Jira issue with key CVE-2026 exists in the tracker at https://redhat.atlassian.net. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
📝 WalkthroughWalkthroughUpdates: OpenTelemetry Go modules bumped v1.39.0→v1.41.0; GitHub Actions smoke workflow adds step-level GITHUB_TOKEN; two driver tests loosen exact progress-message assertions to check count and prefix. ChangesRepository small updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
|
@sheltoncyril: No Jira issue with key CVE-2026 exists in the tracker at https://redhat.atlassian.net. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@sheltoncyril: No Jira issue with key CVE-2026 exists in the tracker at https://redhat.atlassian.net. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Tests asserted exact error strings from python exec failures, but the message differs depending on whether python is missing vs exits non-zero. Use Contains instead of Equal to match the error prefix only. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The install script queries the GitHub API to find the latest release. Without authentication, CI runners hit rate limits causing silent download failures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@sheltoncyril: No Jira issue with key CVE-2026 exists in the tracker at https://redhat.atlassian.net. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.github/workflows/smoke.yaml:
- Around line 41-45: The workflow step currently sets env: GITHUB_TOKEN for the
step that executes a remote "curl ... | bash" install (the block with env:
GITHUB_TOKEN and run piping raw.githubusercontent.com into bash), which exposes
the token to an untrusted script; remove GITHUB_TOKEN from that step (delete or
move the env entry) or replace the inline curl|bash install with a safer
approach such as using a pinned GitHub Action (e.g., actions that install
kustomize) or downloading a pinned release artifact and verifying its checksum
before moving it to /usr/local/bin to eliminate the token exposure risk.
In `@controllers/lmes/driver/driver_test.go`:
- Around line 235-236: The test uses testify assertions but must follow the
controller-test rule (Ginkgo v2 + Gomega); replace assert.Len(t, msgs, 1) and
assert.Contains(t, msgs[0], "...") with Gomega expectations (e.g., g :=
NewWithT(t); g.Expect(msgs).To(HaveLen(1)) and
g.Expect(msgs[0]).To(ContainSubstring("failed to detect available
device(s):"))), remove the testify/assert import, and apply the same
replacements for the other occurrences around the second spot (lines referenced
as 262-263).
- Around line 235-236: The test currently uses assert.Len(t, msgs, 1) before
indexing msgs[0], which is non-fatal and can lead to a panic; replace
assert.Len(t, msgs, 1) with require.Len(t, msgs, 1) (and similarly replace the
second assert.Len usage that precedes msgs[0] at the later check) so the test
fails immediately on length mismatch — ensure the require package is imported
(testing/require) if not already and update the assertions that reference msgs
to use require.Len before any indexed access.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c7dc02ea-b49d-4b80-a5db-1c185183fc8a
📒 Files selected for processing (2)
.github/workflows/smoke.yamlcontrollers/lmes/driver/driver_test.go
| assert.Len(t, msgs, 1) | ||
| assert.Contains(t, msgs[0], "failed to detect available device(s):") |
There was a problem hiding this comment.
Controller test assertions in changed lines should follow project test framework rules
These updated assertions still use testify/assert, but this path is governed by the controller-test rule requiring Ginkgo v2 + Gomega + envtest.
As per coding guidelines, "Use Ginkgo v2 with Gomega assertions and controller-runtime envtest for all controller tests".
Also applies to: 262-263
🤖 Prompt for 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.
In `@controllers/lmes/driver/driver_test.go` around lines 235 - 236, The test uses
testify assertions but must follow the controller-test rule (Ginkgo v2 +
Gomega); replace assert.Len(t, msgs, 1) and assert.Contains(t, msgs[0], "...")
with Gomega expectations (e.g., g := NewWithT(t); g.Expect(msgs).To(HaveLen(1))
and g.Expect(msgs[0]).To(ContainSubstring("failed to detect available
device(s):"))), remove the testify/assert import, and apply the same
replacements for the other occurrences around the second spot (lines referenced
as 262-263).
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify risky pattern: non-fatal assert.Len followed by indexed access.
rg -nP -C2 'assert\.Len\(t,\s*msgs,\s*1\)\s*\n\s*assert\.Contains\(t,\s*msgs\[0\],' controllers/lmes/driver/driver_test.goRepository: trustyai-explainability/trustyai-service-operator
Length of output: 75
🏁 Script executed:
cat -n controllers/lmes/driver/driver_test.go | sed -n '230,270p'Repository: trustyai-explainability/trustyai-service-operator
Length of output: 1653
🏁 Script executed:
head -20 controllers/lmes/driver/driver_test.goRepository: trustyai-explainability/trustyai-service-operator
Length of output: 677
🏁 Script executed:
sed -n '18,50p' controllers/lmes/driver/driver_test.goRepository: trustyai-explainability/trustyai-service-operator
Length of output: 678
🏁 Script executed:
# Final verification: confirm non-fatal vs fatal assertion behavior in testify
rg -A3 'assert.Len.*msgs.*1' controllers/lmes/driver/driver_test.go | head -20Repository: trustyai-explainability/trustyai-service-operator
Length of output: 351
Replace non-fatal assert.Len with require.Len before indexed access
assert.Len(...) does not halt execution. If the length check fails, indexing msgs[0] on line 236 and line 263 can panic, obscuring the actual failure. Use require.Len(...) instead to fail fast and maintain test clarity.
Suggested fix
import (
"context"
"flag"
"fmt"
"math/rand"
"os"
"path/filepath"
"testing"
"time"
"github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
"github.com/trustyai-explainability/trustyai-service-operator/api/lmes/v1alpha1"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/log/zap"
)
@@
- assert.Len(t, msgs, 1)
+ require.Len(t, msgs, 1)
assert.Contains(t, msgs[0], "failed to detect available device(s):")
@@
- assert.Len(t, msgs, 1)
+ require.Len(t, msgs, 1)
assert.Contains(t, msgs[0], "failed to download assets from S3:")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| assert.Len(t, msgs, 1) | |
| assert.Contains(t, msgs[0], "failed to detect available device(s):") | |
| require.Len(t, msgs, 1) | |
| assert.Contains(t, msgs[0], "failed to detect available device(s):") |
🤖 Prompt for 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.
In `@controllers/lmes/driver/driver_test.go` around lines 235 - 236, The test
currently uses assert.Len(t, msgs, 1) before indexing msgs[0], which is
non-fatal and can lead to a panic; replace assert.Len(t, msgs, 1) with
require.Len(t, msgs, 1) (and similarly replace the second assert.Len usage that
precedes msgs[0] at the later check) so the test fails immediately on length
mismatch — ensure the require package is imported (testing/require) if not
already and update the assertions that reference msgs to use require.Len before
any indexed access.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: RobGeada The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@sheltoncyril: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
3 participants
Summary
go.opentelemetry.io/otel,go.opentelemetry.io/otel/metric, andgo.opentelemetry.io/otel/tracefrom v1.39.0 → v1.41.0baggageheader extraction causes excessive allocations, enabling remote DoS amplificationTest plan
go build ./...passes🤖 Generated with Claude Code
Summary by CodeRabbit
Chores
Tests