2.2.0 #50
2.2.0 #50
Conversation
WalkthroughThis update modifies various files to upgrade the Go version and improve configuration management. It adjusts GitHub workflow files, changelog entries, and version identifiers, and adds outbound proxy configuration support. Deployment scripts are streamlined with function inlining, and core modules now pass configuration objects to initialize proxy settings. Changes
Sequence Diagram(s)sequenceDiagram
participant Main
participant Config
participant Proxy
Main->>Config: Load configuration
Main->>Proxy: Call InitReq(cfg)
Proxy->>Proxy: Check cfg.Outbound.Enabled
alt Outbound Enabled
Proxy->>Proxy: Invoke initTransport(cfg, transport)
else Not Enabled
Proxy->>Proxy: Use default HTTP transport
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 golangci-lint (1.62.2)Error: can't load config: the Go language version (go1.23) used to build golangci-lint is lower than the targeted Go version (1.24.0) ✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
proxy/chunkreq.go (1)
37-56: 🛠️ Refactor suggestionAdd timeout settings for proxy dialer.
When using a proxy, it's important to set appropriate timeouts for the proxy dialer to prevent hanging connections.
func initChunkedHTTPClient(cfg *config.Config) { ctr = &http.Transport{ MaxIdleConns: 100, MaxConnsPerHost: 60, IdleConnTimeout: 20 * time.Second, TLSHandshakeTimeout: 10 * time.Second, ExpectContinueTimeout: 1 * time.Second, ResponseHeaderTimeout: 10 * time.Second, DialContext: (&net.Dialer{ Timeout: 30 * time.Second, KeepAlive: 30 * time.Second, }).DialContext, + // Add proxy dialer timeout + ProxyConnectHeader: http.Header{}, + ProxyConnectTimeout: 30 * time.Second, } if cfg.Outbound.Enabled { initTransport(cfg, ctr) } cclient = &http.Client{ Transport: ctr, + Timeout: 60 * time.Second, // Add client timeout } }
🧹 Nitpick comments (11)
proxy/dial.go (2)
17-63: Validate credentials redaction in logs.While the proxy initialization is logically correct (fallback, scheme-based handling, etc.), consider carefully whether
cfg.Outbound.Urlmight contain credentials. If so, ensure they are consistently redacted or omitted from log messages to mitigate potential credential leaks, especially if log files are publicly accessible.
108-129: Ensure secure handling of authentication data.This function correctly extracts user credentials from the URL. However, remain mindful of how these credentials are stored, logged, or exposed in error messages to prevent leaks. Always favor explicit redaction or careful handling if credentials are logged.
config/config.toml (1)
40-42: Consider security implications of default proxy configuration.While the outbound proxy is disabled by default (good security practice), consider:
- Adding a comment warning about security risks of using SOCKS5 proxies
- Documenting the supported proxy URL formats
✅ Verification successful
🌐 Web query:
What are the security risks of using SOCKS5 proxies?💡 Result:
Using SOCKS5 proxies introduces several security risks primarily related to data exposure, implementation vulnerabilities, and potential misuse by attackers. While they offer flexibility for bypassing network restrictions and handling diverse traffic types (TCP/UDP), their lack of built-in encryption and dependency on proper configuration create significant vulnerabilities.
Data Exposure Risks
- No inherent encryption: SOCKS5 does not encrypt traffic by default, leaving sensitive data (e.g., login credentials, financial information) exposed to interception[1][3][9]. This contrasts with HTTPS proxies or VPNs that provide encryption layers.
- IP address leakage: Despite masking your IP via the proxy server, improper configurations or compromised proxies can still reveal your original IP address[1][3].
Proxy Implementation Vulnerabilities
CVE-2023-38545: Critical Heap Overflow Flaw
A high-severity vulnerability in curl/libcurl (versions 7.69.0–8.3.0) allows heap buffer overflow during SOCKS5 proxy handshakes when resolving overly long hostnames (>255 bytes). Attackers exploiting this could execute arbitrary code or crash systems via malicious redirects[5][7][8][10].
Risk Factor Impact Affected Systems Remote code execution Compromise of devices/applications using vulnerable curl versions Enterprise systems, IoT devices This flaw was patched in curl 8.4.0 but remains a threat in unupdated systems[5][10].
Exploitation by Malicious Actors
- Proxy hijacking: Attackers exploit weak proxy configurations (e.g., unpatched software or exposed ports) to intercept/modify traffic for data theft or malware distribution[2][4]. The 911 S5 vulnerability exemplifies this[2].
- Malware propagation: Ransomware like QNAPCrypt and fileless malware (e.g., Nodersok) abuse SOCKS proxies to route malicious traffic and evade detection[4].
- Cryptojacking: Attackers hijack proxies to redirect computational resources for cryptocurrency mining[4].
Proxy-Specific Risks
- Malicious proxy servers: Free/public SOCKS5 proxies may log or sell user data[1][6].
- Performance trade-offs: Encryption overhead (if added) can slow connections[1][3].
- Firewall detection: Strict networks may block SOCKS proxies entirely[9].
Mitigation Strategies
- Use encryption: Pair SOCKS5 with VPNs or TLS/SSL to secure traffic[1][3].
- Verify proxy providers: Opt for reputable services with authentication and no-logs policies[6].
- Patch software: Update curl/libcurl to ≥8.4.0 if using SOCKS5 proxy-resolver mode[5][7].
- Avoid risky configurations: Disable
CURLPROXY_SOCKS5_HOSTNAMEandsocks5h://environment variables[5][10].By addressing these risks through proactive measures, users can leverage SOCKS5 proxies safely while minimizing exposure to cyber threats.
Citations:
- 1: https://proxyempire.io/socks-proxy/
- 2: https://fischsolutions.com/what-is-911-s5-proxy-vulnerability/
- 3: https://www.ranktracker.com/blog/socks5-proxy-full-review-advantages-and-disadvantages/
- 4: https://securityintelligence.com/articles/what-is-socks-proxy-exploit/
- 5: https://www.tarlogic.com/blog/cve-2023-38545-curl-vulnerability/
- 6: https://www.upguard.com/blog/proxy-server
- 7: https://www.securityweek.com/critical-socks5-vulnerability-in-curl-puts-enterprise-systems-at-risk/
- 8: https://www.anvilogic.com/threat-reports/curl-socks5-vuln-cve-2023-38545
- 9: https://www.piaproxy.com/blog/Socks5/full-analysis-of-the-advantages-and-disadvantages-of-socks-proxy.html
- 10: https://curl.se/docs/CVE-2023-38545.html
Security Warning Recommendation: Document SOCKS5 Risks and Supported Proxy URL Formats
The review comment is valid. Although the outbound proxy is disabled by default, enabling it without clear warnings can expose users to inherent risks (e.g., lack of encryption, potential exposure to known vulnerabilities as detailed in recent analyses). It is advisable to:
- Add a comment in the config file warning that using a SOCKS5 proxy does not provide built-in encryption and may expose data or be vulnerable to exploitation if enabled.
- Document supported proxy URL formats (e.g.,
socks5://and alternatives) so users are aware of the configuration requirements and associated security implications.main.go (2)
103-105: Consider adding error handling for InitReq.The function might fail when initializing proxy settings, but errors aren't being handled.
-func InitReq(cfg *config.Config) { - proxy.InitReq(cfg) +func InitReq(cfg *config.Config) error { + return proxy.InitReq(cfg)
112-112: Handle potential errors from InitReq.If InitReq returns an error, it should be handled appropriately.
- InitReq(cfg) + if err := InitReq(cfg); err != nil { + logError("Failed to initialize proxy settings: %v", err) + }proxy/chunkreq.go (1)
25-35: Consider adding error handling for proxy initialization.The function should return any errors that occur during proxy initialization.
-func InitReq(cfg *config.Config) { +func InitReq(cfg *config.Config) error { - initChunkedHTTPClient(cfg) - initGitHTTPClient(cfg) + if err := initChunkedHTTPClient(cfg); err != nil { + return fmt.Errorf("failed to initialize chunked client: %w", err) + } + if err := initGitHTTPClient(cfg); err != nil { + return fmt.Errorf("failed to initialize git client: %w", err) + } // Initialize fixed-size buffer pool BufferPool = &sync.Pool{ New: func() interface{} { return make([]byte, BufferSize) }, } + return nil }.github/workflows/build.yml (1)
48-48: Ensure consistent Go version format.The Go version is specified as "1.24" here but as "1.24.0" in go.mod. While both formats work, it's better to maintain consistency across files.
- GO_VERSION: 1.24 + GO_VERSION: 1.24.0.github/workflows/build-dev.yml (1)
49-49: Ensure consistent Go version format.The Go version is specified as "1.24" here but as "1.24.0" in go.mod. While both formats work, it's better to maintain consistency across files.
- GO_VERSION: 1.24 + GO_VERSION: 1.24.0deploy/install.sh (1)
36-39: Consider dropping root privileges after necessary operations.While root privileges are required for installation, consider dropping them after the necessary operations are completed for better security.
if [ "$EUID" -ne 0 ]; then echo "请以root用户运行此脚本" exit 1 fi + +# Function to drop root privileges +drop_privileges() { + if [ -n "$SUDO_USER" ]; then + # Get the original user's home directory + USER_HOME=$(getent passwd "$SUDO_USER" | cut -d: -f6) + # Drop privileges for the specified command + su - "$SUDO_USER" -c "$*" + fi +}README.md (1)
180-180: Format the URL as a markdown link.Convert the bare URL to a proper markdown link for better readability and consistency.
-爱发电: https://afdian.com/a/wjqserver +爱发电: [https://afdian.com/a/wjqserver](https://afdian.com/a/wjqserver)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
180-180: Bare URL used
null(MD034, no-bare-urls)
CHANGELOG.md (1)
3-3: Use consistent heading styles throughout the changelog.Convert setext-style headings to ATX-style headings for consistency.
-2.2.0 ---- +## 2.2.0 -25w13b ---- +## 25w13b -25w13a ---- +## 25w13aAlso applies to: 10-10, 15-15
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
3-3: Heading style
Expected: atx; Actual: setext(MD003, heading-style)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (16)
.github/workflows/build-dev.yml(1 hunks).github/workflows/build.yml(1 hunks)CHANGELOG.md(1 hunks)DEV-VERSION(1 hunks)README.md(3 hunks)VERSION(1 hunks)config/config.go(2 hunks)config/config.toml(1 hunks)deploy/config.toml(2 hunks)deploy/install-dev.sh(3 hunks)deploy/install.sh(2 hunks)go.mod(1 hunks)main.go(1 hunks)proxy/chunkreq.go(3 hunks)proxy/dial.go(1 hunks)proxy/gitreq.go(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- VERSION
- DEV-VERSION
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
180-180: Bare URL used
null
(MD034, no-bare-urls)
CHANGELOG.md
3-3: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
10-10: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
15-15: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
🔇 Additional comments (15)
proxy/dial.go (3)
1-5: Good reference to the PR.Including the PR link and author name in the header is helpful for traceability. Ensure it remains up to date in case the upstream repository location changes in future.
66-106: Confirm fallback behavior for multi-proxy chaining.The multi-proxy chaining logic here is robust, but if all listed proxies fail or are non-SOCKS5, it reverts to a direct dialer without error. This may lead to unintended bypass of the proxy if configuration mistakes occur. Confirm that this silent fallback is desired; otherwise, consider making it explicit with an error or warning if no usable proxy is found.
131-135: SOCKS5 dialer creation is straightforward.The direct invocation of
proxy.SOCKS5with the previously chained dialer cleanly builds the dialer stack. No issues observed here.config/config.go (2)
16-16: Outbound config integration looks good.The new
Outboundfield is added cleanly to theConfigstruct. Ensure that consumers of this struct handle outbound-proxy logic gracefully if the config file omits this section.
66-74: Optional sample configuration is beneficial.Providing the commented-out TOML snippet clarifies typical usage. This helps users discover and enable outbound proxy easily. Good to keep references to both
socks5://andhttp://as examples.deploy/config.toml (1)
5-5: Verify impact of H2C setting type change.The
enableH2Csetting has changed from a boolean to a string type. This might affect existing deployments.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for any code that directly reads the enableH2C setting rg -A 2 "enableH2C.*=.*" --type tomlLength of output: 235
Impact of H2C Setting Type Change Verified
- Both
config/config.tomlanddeploy/config.tomlnow setenableH2Cusing the string"on"(with the inline comment indicating valid values as"on"or"off").- Our repository search did not reveal any other references that expect a boolean value for this setting.
- It appears the change to a string value is intentional and consistent; however, please ensure that any downstream consumers of this configuration properly handle the string type.
go.mod (1)
3-3: Verify Go version compatibility.The Go version has been upgraded to 1.24.0. This is a major version upgrade that may introduce breaking changes.
❌ Verification inconclusive
🌐 Web query:
What are the breaking changes in Go 1.24.0?💡 Result:
Go 1.24 introduces several breaking changes that may require adjustments to existing code:
Language and compiler changes
- Methods on cgo-generated types
The compiler now explicitly prohibits defining methods on cgo-generated types through aliases[4][8]. Code using aliases to bypass previous restrictions will now fail to compile:// Previously allowed via alias type MyCType = C.some_c_type func (m MyCType) Method() {} // Now causes compiler errorStandard library changes
crypto/tls
- Removed experimental
X25519Kyber768Draft00post-quantum key exchange support[4]. Existing configurations using this will break.crypto/cipher
- XORBytes overlap handling
Now panics if output buffer overlaps with input buffers in non-exact matches[4]:src := []byte{1,2,3} dst := make([]byte, 3) // Overlapping slice (previously undefined behavior) cipher.XORBytes(dst[1:], src, src[:2]) // Now panicsos/user (Windows-specific)
- Impersonation behavior
Returns process owner user during thread impersonation instead of error[4]. Code expecting errors in these scenarios may break.sync
- Map implementation
The new Swiss Tables-based implementation changes contention characteristics[1][4][5]. While semantically compatible, performance profiles may differ significantly:Revert withvar m sync.Map // Different internal locking behaviorGOEXPERIMENT=nosynchashtriemapif issues arise.encoding/json
- omitzero tag interaction
Combined use with omitempty may omit more fields than before when using zero time values[4]:type T struct { Timestamp time.Time `json:",omitzero,omitempty"` } // Zero time now omitted even without omitemptyDeprecations
- runtime.GOROOT()
Now deprecated in favor ofgo env GOROOT[4]. While not immediately breaking, future versions may remove it.Developers should review these areas and test thoroughly when upgrading. The Go team recommends using
go vetand the newtestanalyzer to detect potential issues[1][4].Citations:
- 1: https://go.dev/blog/go1.24
- 2: https://antonz.org/go-1-24/
- 3: https://blog.howardjohn.info/posts/go-tools-command/
- 4: https://tip.golang.org/doc/go1.24
- 5: https://devcenter.upsun.com/posts/go-124/
- 6: https://devblogs.microsoft.com/go/go-1-24-fips-update/
- 7: https://go.dev/doc/devel/release
- 8: doc: write release notes for Go 1.24 golang/go#68545
- 9: https://golangnugget.com/p/what-to-expect-from-go-1-24-part-1
- 10: https://github.com/golang/go/milestone/322
Go 1.24 Upgrade – Confirm Compatibility with Breaking Changes
The Go version upgrade to 1.24.0 introduces several breaking changes (e.g., restrictions on methods for cgo-generated types, changes in crypto libraries, altered behavior in os/user and sync.Map implementations, and modifications to encoding/json handling) that could impact your project if you rely on these features. Additionally, the minor upgrade of the
golang.org/x/netdependency appears consistent with semantic versioning and is less likely to cause issues.Please ensure you:
- Audit your code for dependencies on cgo-generated types or areas affected by the noted breaking changes.
- Run comprehensive tests (e.g., using
go vetand your test suite) to catch any compatibility issues..github/workflows/build-dev.yml (1)
52-52: LGTM! Good upgrade of checkout action.Upgrading actions/checkout from v3 to v4 is a good practice for security and features.
Also applies to: 110-110
deploy/install.sh (1)
119-135: LGTM! Good improvement in systemd service file creation.The inlined systemd service creation using heredoc is more maintainable and easier to read than the previous function-based approach.
deploy/install-dev.sh (2)
36-39: Consider dropping root privileges after necessary operations.While root privileges are required for installation, consider dropping them after the necessary operations are completed for better security.
119-135: LGTM! Good improvement in systemd service file creation.The inlined systemd service creation using heredoc is more maintainable and easier to read than the previous function-based approach.
README.md (2)
74-78: LGTM! Dev deployment script added.The new dev deployment script follows the same secure pattern as the existing deployment script.
125-127: LGTM! Well-documented outbound proxy configuration.The new outbound proxy configuration:
- Supports both Socks5 and HTTP(S) proxies
- Is disabled by default (good security practice)
- Includes clear examples
CHANGELOG.md (2)
3-8: LGTM! Clear and well-structured changelog entry.The changelog entry for v2.2.0:
- Follows the established format
- Clearly documents all major changes
- Properly categorizes changes
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
3-3: Heading style
Expected: atx; Actual: setext(MD003, heading-style)
10-19: LGTM! Well-documented pre-release entries.The pre-release entries (25w13b and 25w13a):
- Clearly mark the versions as pre-release
- Include appropriate production usage warnings
- Document the staged development of features
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
10-10: Heading style
Expected: atx; Actual: setext(MD003, heading-style)
15-15: Heading style
Expected: atx; Actual: setext(MD003, heading-style)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
proxy/gitreq.go (1)
38-39: Clean up commented out code blocks.There are two blocks of commented out code that should be removed to improve code maintainability:
- Line 39: Unused client creation
- Lines 104-109: Unused response size handling
- // 创建HTTP客户端 - //client := &http.Client{} - /* - if err := HandleResponseSize(resp, cfg, c); err != nil { - logWarning("%s %s %s %s %s Response-Size-Error: %v", c.ClientIP(), method, u, c.Request.Header.Get("User-Agent"), c.Request.Proto, err) - return - } - */Also, consider translating Chinese comments to English for consistency:
- // 创建HTTP客户端 + // Create HTTP clientAlso applies to: 104-109
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
proxy/gitreq.go(1 hunks)
🔇 Additional comments (1)
proxy/gitreq.go (1)
20-28: LGTM! Configuration-driven transport initialization looks good.The changes correctly implement configuration-based transport initialization with proper outbound proxy support. The previous variable reference issue has been fixed by using
gtrinstead ofctr.Let's verify the configuration structure:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Verify the Config struct has the expected Outbound field ast-grep --pattern 'type Config struct { $$$ Outbound OutboundConfig $$$ }'Length of output: 542
LGTM! The Config struct correctly includes the Outbound field, and the outbound proxy configuration is implemented as expected.
Summary by CodeRabbit
New Features
Documentation
Chores