gravity: trust system CA pool and make SNI fallback configurable

[robindiddams]

Summary

• Use x509.SystemCertPool() instead of x509.NewCertPool() in both grpc_client.go and identify.go so servers presenting certs signed by public root CAs are trusted alongside the Agentuity CA
• Make the hardcoded "gravity.agentuity.com" SNI fallback (used when connecting via IP address) configurable via DefaultServerName on GravityConfig and IdentifyConfig
• Refactor Identify() to take an IdentifyConfig struct — CACert and DefaultServerName are optional and default to system roots / "gravity.agentuity.com" respectively

Breaking Changes

• Identify() signature changed from positional args to IdentifyConfig struct — callers get a compile error and must migrate
• extractHostnameFromGravityURL() now takes a second fallbackServerName parameter (internal, not exported)

Summary by CodeRabbit

• New Features

  • Added configurable default server name for TLS connections to improve hostname resolution when connecting via IP addresses

• Bug Fixes

  • Enhanced TLS certificate loading with fallback mechanism for more robust connection establishment

• Refactor

  • Simplified Identify operation by consolidating parameters into a configuration struct for cleaner API

[coderabbitai]

📝 Walkthrough

Walkthrough

These changes introduce a new DefaultServerName configuration field to support TLS server name fallback when connecting via IP addresses. The field is wired through the GravityConfig struct, into GravityClient initialization, and the Identify function is refactored to accept a structured IdentifyConfig parameter instead of multiple arguments. Hostname extraction logic is updated to apply the fallback when needed, and tests are expanded to validate the new behavior.

Changes

Cohort / File(s)	Summary
Configuration & Types gravity/types.go	Added DefaultServerName field to GravityConfig struct to specify TLS server name fallback for IP-based connections.
gRPC Client Implementation gravity/grpc_client.go	Added defaultServerName field to GravityClient, updated hostname extraction function signature to accept fallbackServerName parameter, wired config value during initialization, and enhanced TLS setup with system cert pool fallback handling.
Client API Refactoring gravity/identify.go	Introduced new IdentifyConfig struct encapsulating parameters (GravityURL, InstanceID, ECDSAPrivateKey, CACert, DefaultServerName), refactored Identify function to accept single config parameter instead of multiple arguments, updated hostname extraction and TLS configuration logic accordingly.
Test Updates gravity/grpc_interface_test.go	Updated TestExtractHostnameFromURL to test new function signature with fallback parameter, added test cases for default fallback behavior and custom fallback hostname handling for IP-based URLs.

🚥 Pre-merge checks | ✅ 1

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

gravity/identify.go (1)

53-55: Consider validating the CA certificate when provided.

Unlike grpc_client.go (lines 364-367), this code silently ignores invalid PEM data. If a user provides CACert but it contains invalid/malformed PEM, TLS connections may fail with confusing certificate errors rather than a clear message about the bad CA cert.

🔧 Proposed fix to validate CACert

 	if config.CACert != "" {
-		pool.AppendCertsFromPEM([]byte(config.CACert))
+		if ok := pool.AppendCertsFromPEM([]byte(config.CACert)); !ok {
+			return nil, fmt.Errorf("failed to parse CACert PEM")
+		}
 	}

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8c2aaff and 04dfdcd.

📒 Files selected for processing (4)

• gravity/grpc_client.go
• gravity/grpc_interface_test.go
• gravity/identify.go
• gravity/types.go

🧰 Additional context used

🧬 Code graph analysis (1)

gravity/identify.go (1)

gravity/proto/gravity_session.pb.go (3)

• IdentifyResponse (72-77)
• IdentifyResponse (90-90)
• IdentifyResponse (105-107)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Analyze (go)

🔇 Additional comments (7)

gravity/types.go (1)

53-53: LGTM!

Clean addition of the configuration field with clear documentation of its purpose and default behavior.

gravity/grpc_interface_test.go (1)

176-206: LGTM!

Good test coverage for the new fallback behavior:

• Default fallback for IP-based URLs with empty config
• Custom fallback for IP-based URLs
• Hostname-based URLs ignoring the fallback

The mix of standalone function calls and method calls provides coverage for both code paths.

gravity/grpc_client.go (3)

171-172: LGTM!

Fields appropriately added to store configuration for TLS handling.

---

359-367: LGTM!

Good approach to TLS certificate handling:

1. Attempt to load system cert pool (enables trust for public CA-signed certs)
2. Graceful fallback to empty pool with warning if system pool unavailable
3. Agentuity CA cert is still validated as required (error on failure)

---

2160-2191: LGTM!

The function correctly handles the three scenarios:

• Hostname URLs return the hostname as-is
• IP-based URLs use the provided fallback if non-empty
• IP-based URLs use the default constant when no fallback provided

gravity/identify.go (2)

17-24: LGTM!

Clean configuration struct with clear documentation of optional fields. This improves API ergonomics and allows for future extensibility.

---

28-37: LGTM!

Input validation is clear and follows the same patterns used elsewhere in the codebase. The breaking change from positional arguments to a config struct is documented in the PR objectives.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}