fix(lint): resolve staticcheck SA1019 findings - #890
Merged
Conversation
Migrated k8s.io/apimachinery/pkg/util/httpstream to its k8s.io/streaming replacement (drop-in, already an indirect dependency) and httputil.ReverseProxy.Director to .Rewrite in pkg/ts/workspace_server.go's three reverse proxies, preserving exact prior behavior including the implicit X-Forwarded-For header Director set automatically. Suppressed the remaining findings where migration is out of scope for a lint-cleanup PR: the MCP SDK's session.Log (SEP-2577 protocol-level deprecation, no replacement API in go-sdk v1.7.0) and the three inject.ExecFunc/inject.Inject call sites in pkg/agent/inject.go, which are part of the legacy shell injection path retained until callers migrate to AgentDelivery.
✅ Deploy Preview for images-devsy-sh canceled.
|
✅ Deploy Preview for devsydev canceled.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe change updates credential and port-forward proxy rewriting, moves the Kubernetes streaming package to a direct dependency, updates its import, and documents intentional use of deprecated MCP and shell injection APIs. ChangesProxy and compatibility updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant WorkspaceServer
participant ReverseProxy as httputil.ReverseProxy
participant CredentialEndpoint
Client->>WorkspaceServer: Send credential or port-forward request
WorkspaceServer->>ReverseProxy: Rewrite destination and headers
ReverseProxy->>CredentialEndpoint: Forward request with X-Forwarded-For
CredentialEndpoint-->>ReverseProxy: Return response
ReverseProxy-->>Client: Return proxied response
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@pkg/ts/workspace_server.go`:
- Around line 438-452: Update addForwardedFor to preserve the validated
X-Forwarded-For chain from pr.In.Header before appending the client address,
since pr.Out.Header no longer contains inbound values after
ReverseProxy.Rewrite. Copy the inbound header into the outbound request, then
retain the existing omission and RemoteAddr handling.
🪄 Autofix
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: df56afb3-05dd-4ada-931b-6ec8899f7afc
📒 Files selected for processing (5)
cmd/mcp/notify.gogo.modpkg/agent/inject.gopkg/driver/kubernetes/client.gopkg/ts/workspace_server.go
Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
skevetter
marked this pull request as draft
August 7, 2026 01:18
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves all 8
staticcheckSA1019 deprecation findings, handled per-case rather than blanket-suppressed:Migrated (real fix):
pkg/driver/kubernetes/client.go: swappedk8s.io/apimachinery/pkg/util/httpstreamfor itsk8s.io/streaming/pkg/httpstreamreplacement — identicalIsUpgradeFailure(error) boolsignature, already an indirect module dependency, now promoted to direct viago mod tidy.pkg/ts/workspace_server.go: migrated all threehttputil.ReverseProxy.Directorusages (git/docker credentials proxies, HTTP port-forward proxy) to.Rewrite. Added a sharedaddForwardedForhelper that replicates theX-Forwarded-ForheaderReverseProxysets automatically forDirectorbut not forRewrite, so proxy behavior is unchanged. Verified via a temporary httptest-based smoke test (routing, header stripping, andX-Forwarded-Forparity) before removing it.Suppressed (out of scope for a lint-cleanup PR):
cmd/mcp/notify.go:session.Log— MCP SDK v1.7.0 has no replacement API; deprecation is protocol-level (SEP-2577) with a 12-month functional window.pkg/agent/inject.go(3 sites):inject.ExecFunc/inject.Inject— explicitly named legacy shell injection path with a real replacement (AgentDeliveryimplementations), but migrating callers is a behavior-changing change outside this PR's scope.golangci-lint run --enable-only=staticcheck --max-same-issues=0 ./...now reports 0 issues.Summary by CodeRabbit
Bug Fixes
Maintenance