Closed dbarrosop closed 2 months ago
β±οΈ Estimated effort to review: 2 π΅π΅βͺβͺβͺ |
π§ͺ PR contains tests |
π Security concerns Sensitive information exposure: The Turnstile secret key is hardcoded in the example configuration file (cmd/config/example.go) and in the test configuration (dockercompose/main_test.go). Although these are likely not production files, it's generally a bad practice to include even example secrets in code, as they might accidentally be used in real environments. |
β‘ Key issues to review Hardcoded Secret The Turnstile secret key is hardcoded in the example configuration. This might lead to accidental exposure of the secret in version control. |
Category | Suggestion | Score |
Security |
Use an environment variable for sensitive configuration data___ **Consider using a constant or environment variable for the Turnstile secret keyinstead of hardcoding it directly in the configuration.** [cmd/config/example.go [162-168]](https://github.com/nhost/cli/pull/916/files#diff-49de1d039dc29237e62d701aea07feeeb61ca2f6b2fd3c8ca5ac36c68e1396ecR162-R168) ```diff SignUp: &model.ConfigAuthSignUp{ Enabled: ptr(true), DisableNewUsers: ptr(false), Turnstile: &model.ConfigAuthSignUpTurnstile{ - SecretKey: "turnstileSecretKey", + SecretKey: os.Getenv("TURNSTILE_SECRET_KEY"), }, }, ``` Suggestion importance[1-10]: 9Why: The suggestion addresses a significant security concern by recommending the use of environment variables for sensitive data instead of hardcoding it. | 9 |
Best practice |
Use a mock value for sensitive data in test configurations___ **Consider using a placeholder or mock value for the Turnstile secret key in testconfigurations to avoid exposing sensitive information in test files.** [dockercompose/auth_test.go [31]](https://github.com/nhost/cli/pull/916/files#diff-70746ae54c1a33a5ec884f6aed960e89daef7db608a5bb35e3cfedbf64b0a66eR31-R31) ```diff -"AUTH_TURNSTILE_SECRET": "turnstileSecretKey", +"AUTH_TURNSTILE_SECRET": "test_turnstile_secret", ``` Suggestion importance[1-10]: 8Why: This suggestion improves security and test practices by recommending the use of mock values for sensitive data in test configurations. | 8 |
Maintainability |
Use a constant for sensitive test data to improve maintainability___ **Similar to the previous suggestions, consider using a mock value or a constant forthe Turnstile secret key in test configurations to maintain consistency and avoid potential security risks.** [dockercompose/main_test.go [203-209]](https://github.com/nhost/cli/pull/916/files#diff-cbd7bf3ae89752b4e39860c598bf584d6fecb846d098ad2fb9bafa5b205774faR203-R209) ```diff SignUp: &model.ConfigAuthSignUp{ Enabled: ptr(true), DisableNewUsers: ptr(false), Turnstile: &model.ConfigAuthSignUpTurnstile{ - SecretKey: "turnstileSecretKey", + SecretKey: testTurnstileSecretKey, }, }, ``` Suggestion importance[1-10]: 7Why: The suggestion enhances maintainability and consistency in test configurations, though it's slightly less critical than the previous two suggestions. | 7 |
PR Type
Enhancement
Description
Turnstile
configuration in theSignUp
struct with aSecretKey
fieldgithub.com/nhost/be
dependency to the latest versionChanges walkthrough π
example.go
Add Turnstile configuration to SignUp
cmd/config/example.go
Turnstile
configuration to theSignUp
struct with aSecretKey
field
auth_test.go
Add Turnstile secret to auth test configuration
dockercompose/auth_test.go
AUTH_TURNSTILE_SECRET
environment variable with value"turnstileSecretKey"
main_test.go
Update test config with Turnstile settings
dockercompose/main_test.go
Turnstile
configuration to theSignUp
struct in the test configgo.mod
Update nhost/be dependency version
go.mod
github.com/nhost/be
dependency to versionv0.0.0-20240925125635-9b2298f21170
go.sum
Update checksum for nhost/be dependency
go.sum
github.com/nhost/be
to match the new version