fix: add secret survey variable on opentofu task issue #2322 #3083
Conversation
bdoubrovine
changed the title
There was a problem hiding this comment.
Pull Request Overview
This pull request enhances secret variable handling in the getEnvironmentExtraVars method by adding support for parsing and merging secret variables from a separate JSON field while ensuring secrets are cleared from memory after processing.
- Added parsing of secret variables from
t.SecretJSON field with error handling - Implemented automatic clearing of the
t.Secretfield after processing to prevent secret retention - Merged secret variables with existing environment variables using
maps.Copy
| if err != nil { | ||
| return | ||
| } | ||
| } |
There was a problem hiding this comment.
Setting t.Secret = "{}" may not be sufficient to clear sensitive data from memory. The original string containing secrets may still exist in memory. Consider using a more secure approach like overwriting the memory or ensuring the original secret data is properly cleared.
| } | |
| } | |
| // Securely clear the sensitive data in t.Secret | |
| if len(t.Secret) > 0 { | |
| secretBytes := []byte(t.Secret) | |
| for i := range secretBytes { | |
| secretBytes[i] = 0 | |
| } | |
| } |
Copilot uses AI. Check for mistakes.
| } | ||
| t.Secret = "{}" | ||
|
|
||
| maps.Copy(extraVars, extraSecretVars) |
There was a problem hiding this comment.
Secret variables are being copied into extraVars which may be used elsewhere in the application. Consider whether these secret values should be masked or handled with additional security measures when stored in the shared extraVars map.
| maps.Copy(extraVars, extraSecretVars) | |
| for key, value := range extraSecretVars { | |
| extraVars[key] = maskSensitiveValue(value) | |
| } |
Copilot uses AI. Check for mistakes.
| err = json.Unmarshal([]byte(t.Secret), &extraSecretVars) | ||
| if err != nil { | ||
| return | ||
| } |
There was a problem hiding this comment.
The condition t.Secret != "" may not handle the case where t.Secret is "{}" (empty JSON object). Consider checking for both empty string and empty JSON object, or use a more robust validation method.
| err = json.Unmarshal([]byte(t.Secret), &extraSecretVars) | |
| if err != nil { | |
| return | |
| } | |
| var tempSecretVars map[string]any | |
| err = json.Unmarshal([]byte(t.Secret), &tempSecretVars) | |
| if err != nil { | |
| return | |
| } | |
| if len(tempSecretVars) > 0 { | |
| extraSecretVars = tempSecretVars | |
| } |
Copilot uses AI. Check for mistakes.
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no bugs!
Bugbot free trial expires on August 10, 2025
Learn more in the Cursor dashboard.
This pull request introduces enhancements to the
getEnvironmentExtraVarsmethod inservices/tasks/LocalJob.goto improve the handling of secret variables. The most important change involves merging secret variables into the existing environment variables while ensuring the secret is reset after processing.Enhancements to secret variable handling:
extraSecretVars, to handle secret variables separately and parse them from thet.SecretJSON string. If parsing fails, the method returns an error.t.Secretfield to an empty JSON object ("{}") after processing to ensure that secrets are not retained in memory.extraSecretVarsinto theextraVarsmap usingmaps.Copy, consolidating all variables into a single map for further use.#2322 and #2287