Harden the 1.12 release changes - #614
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens several 1.12 release-critical behaviors across shutdown lifecycle handling, client retry cancellation semantics, WebSocket pre-auth read bounding (without rejecting valid SSH traffic), log safety for untrusted usernames, and release-archive validation before publication.
Changes:
- Ensure
HTTPServer.Wait()observes graceful drain completion (including grace-timeout force-close) and extend unit coverage for both drain and timeout paths. - Raise and formalize the default WebSocket read limit to accommodate
x/crypto/ssh’s 256 KiB transport packet ceiling while still bounding unauthenticated reads; add unit + e2e coverage and update docs. - Add a non-publishing release-archive preflight gate in CI and wire verified archives into the publishing run (checksums + GitHub release extra files).
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/ws_limit_test.go | Adds e2e coverage for pre-auth oversized messages and a large real SSH config request traversing the default WS envelope. |
| TASKS.md | Updates the WS read-limit rationale and recommended default to match the SSH packet ceiling reality. |
| share/cnet/http_server.go | Moves shutdown/drain into the errgroup lifecycle so Wait() observes completion; preserves grace-based shutdown behavior. |
| share/cnet/http_server_test.go | Strengthens graceful-drain assertions and adds grace-timeout coverage. |
| share/cnet/conn_ws.go | Introduces a default WS read limit sized for SSH transport max + headroom; treats negative limits as invalid (fallback to default). |
| share/cnet/conn_ws_test.go | Adds unit tests for default envelope acceptance, default oversize rejection, custom limits, negative fallback, and explicit disable. |
| share/cio/logger.go | Adds SetOutput(io.Writer) to make logger output testable/configurable. |
| server/server.go | Quotes failed-login usernames in info logs to prevent log forging via control characters/newlines. |
| server/server_handler.go | Centralizes handshake-failure logging and quotes/escapes aggregated auth errors for safe parsing. |
| server/server_handler_test.go | Adds regression coverage ensuring handshake-failure logs remain single-line and safely escaped. |
| server/server_auth_test.go | Extends auth tests to validate quoted failed-login logging while keeping auth error semantics unchanged. |
| README.md | Updates WS read-limit documentation and env-var table to reflect the new default and negative/zero semantics. |
| client/client_retry_test.go | Adds regression tests ensuring cancellation wins over retry exhaustion (both pre-connection and post-connection). |
| client/client_connect.go | Checks for cancellation immediately after connectionOnce() returns to avoid reporting retry exhaustion on normal shutdown. |
| 1.12-review-tasks.md | Adds a structured checklist documenting reviewed tasks, acceptance criteria, and validation for 1.12 hardening items. |
| 1.12-changes.md | Updates the 1.12 change log to reflect quoted usernames and corrected WS read-cap rationale/default. |
| .github/workflows/ci.yml | Adds a credential-free GoReleaser preflight to build + verify archives before any publishing; preserves verified archives in publish run. |
| .github/goreleaser.yml | Wires CI-staged verified archives into checksums and draft-release uploads via extra_files glob. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
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.
Summary
Validation
go test ./...go build ./...go vet ./...gofmt -d share/cnet/conn_ws.go share/cnet/http_server.go