Turning messages on and off using config flag

[jcrichlake]

Description

We are adding a check of the config files to turn off SFTP for a given partner.

Issue

devex item

Checklist

• I have added tests to cover my changes
• I have added logging where useful (with appropriate log level)

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Hardcoded Secrets The code constructs secret keys using a pattern that includes the environment name and partner ID. This could potentially expose the pattern of secret keys, making it easier for an attacker to guess other secret keys. hostPublicKeyName := partnerId + "-sftp-host-public-key-" + utils.EnvironmentName() // pragma: allowlist secret Error Handling The function checkIsActive logs an error and returns false if the partner ID is not found in the configuration, which could lead to silent failures in processing messages if the configuration is not set up correctly. func checkIsActive(partnerId string) bool { isActive := false if val, ok := config.Configs[partnerId]; ok { isActive = val.PartnerSettings.IsActive } else { slog.Error("Partner not found in config", slog.String("partnerId", partnerId)) return false } if !isActive { slog.Warn("Partner not active, skipping", slog.String("partnerId", partnerId)) return false } return true }

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Add nil check for message.MessageText to prevent potential runtime panics Ensure that the checkIsActive function handles potential panics or errors when dereferencing message.MessageText which could be nil. src/orchestration/polling_message_handler.go [17] +if message.MessageText == nil { + return errors.New("message text is nil") +} partnerId := *message.MessageText ... Suggestion importance[1-10]: 8 Why: This suggestion correctly identifies a potential runtime panic due to dereferencing a possible nil pointer. Adding a nil check enhances the robustness and reliability of the code.	8
	Add checks to handle nil values in configuration retrieval to prevent panics Handle the case where config.Configs[partnerId] might not exist to avoid potential runtime panics from nil dereferencing. src/orchestration/polling_message_handler.go [50-56] -if val, ok := config.Configs[partnerId]; ok { - isActive = val.PartnerSettings.IsActive -} else { +val, ok := config.Configs[partnerId] +if !ok { slog.Error("Partner not found in config", slog.String("partnerId", partnerId)) return false } +if val == nil || val.PartnerSettings == nil { + slog.Error("Invalid config for partner", slog.String("partnerId", partnerId)) + return false +} +isActive = val.PartnerSettings.IsActive Suggestion importance[1-10]: 8 Why: This suggestion is highly relevant as it addresses the risk of runtime panics due to nil dereferencing in configuration retrieval, enhancing the code's safety and stability.	8
Security	Validate partnerId to ensure it meets expected format and content before usage Validate partnerId for correctness before using it to construct key names to prevent potential security risks or errors in key retrieval. src/sftp/handler.go [36] +if !isValidPartnerId(partnerId) { + return nil, errors.New("invalid partner ID") +} hostPublicKeyName := partnerId + "-sftp-host-public-key-" + utils.EnvironmentName() ... Suggestion importance[1-10]: 7 Why: Validating partnerId before using it in key construction is crucial for security and correctness, preventing misuse and potential errors in key retrieval.	7
General	Improve error handling by wrapping errors for better traceability and management Ensure error handling is in place for all secret retrieval operations to manage failures gracefully. src/sftp/handler.go [38-39] hostPublicKey, err := credentialGetter.GetSecret(hostPublicKeyName) if err != nil { slog.Error("Unable to get host public key", slog.String("KeyName", hostPublicKeyName), slog.Any(utils.ErrorKey, err)) - return nil, err + return nil, fmt.Errorf("failed to retrieve host public key: %w", err) } Suggestion importance[1-10]: 6 Why: Wrapping errors provides better error traceability and management, which is a good practice, especially when dealing with external resources like secrets retrieval.	6

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
25.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[saquino0827]

Nice work, overall it looks good but I made some minor comments