Enable remote redis and pgsql#115
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces conditional logic to switch between local container resources and cloud connection strings based on a 'UseCloud' configuration setting. While this adds flexibility for different environments, the review identified significant security risks where SSL certificate validation for Redis is unconditionally bypassed, potentially exposing the system to man-in-the-middle attacks. Feedback also includes a recommendation to use the more robust GetValue method for reading configuration flags instead of manual string comparison.
| { | ||
| options.ConfigurationOptions = ConfigurationOptions.Parse( | ||
| builder.Configuration.GetConnectionString("redis") ?? throw new InvalidOperationException()); | ||
| options.ConfigurationOptions.CertificateValidation += (sender, certificate, chain, sslPolicyErrors) => true; |
There was a problem hiding this comment.
Unconditionally returning true for certificate validation is a significant security risk. This configuration allows any certificate (including self-signed or malicious ones) to be accepted, making the connection susceptible to man-in-the-middle attacks. This should be avoided in production environments. Consider using proper certificate validation or restricting this bypass to development environments using conditional logic (e.g., if (builder.Environment.IsDevelopment())).
| { | ||
| options.ConfigurationOptions = ConfigurationOptions.Parse( | ||
| siloBuilder.Configuration.GetConnectionString("redis") ?? throw new InvalidOperationException()); | ||
| options.ConfigurationOptions.CertificateValidation += (sender, certificate, chain, sslPolicyErrors) => true; |
There was a problem hiding this comment.
Bypassing SSL certificate validation for Redis is insecure. This practice exposes the application to man-in-the-middle attacks, especially when connecting to remote cloud resources as implied by the PR title. It is recommended to properly validate certificates or ensure this bypass is only active in a local development context.
| .WaitFor(rdbms) | ||
| .WithReference(redis) | ||
| .WithReference(rdbms) | ||
| if (String.Compare(builder.Configuration.GetSection("UseCloud").Value, "true") == 0) |
There was a problem hiding this comment.
Using String.Compare on GetSection("UseCloud").Value is fragile. It depends on exact string matching and does not handle null values or different boolean representations (like True or actual JSON boolean types) gracefully. Using the GetValue<bool> extension method is more robust and idiomatic for retrieving boolean configuration flags.
if (builder.Configuration.GetValue<bool>("UseCloud"))1 participant
No description provided.