Potential fix for code scanning alert no. 5: Cleartext logging of sensitive information

[vsilent]

Potential fix for https://github.com/trydirect/stacker/security/code-scanning/5

In general, to fix cleartext logging issues, remove logs that contain sensitive information, or at minimum redact those parts (for example, hashing or truncating user identifiers) so that secrets and sensitive identifiers are not exposed in logs. Prefer structured, leveled logging via your logging framework, and avoid ad‑hoc println!/eprintln! debugging that might accidentally include secrets.

For this specific case, the best fix without changing existing behavior of the encryption/decryption logic is to stop printing the full Redis key (which embeds self.user_id and the specific secret field). Since this is purely diagnostic output, the safest and simplest approach is to remove or comment out the eprintln! call. If you still want a log line for troubleshooting, you can log only non‑sensitive metadata, such as the provider and which field type is being decrypted, without including user_id or the full key string.

Concretely:

• In src/helpers/cloud/security.rs, within impl Secret → pub fn decrypt, change the line eprintln!("decrypt: Key str {rkey:?}"); to either a redacted log (e.g., tracing::debug!("decrypt: provider={}, field={}", self.provider, self.field);) or remove it entirely. This eliminates logging of self.user_id and the full storage key.
• Optionally, consider also removing/toning down the other eprintln! calls in decrypt (nonce and encrypted_data) if they are not strictly needed, but CodeQL’s specific alert is on the rkey line, so that is sufficient to address the reported issue.

No new imports are needed if you simply remove the eprintln! line. If you switch to tracing::debug!, tracing is already in use in this file (via #[tracing::instrument]), so no additional imports are required.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

[gitguardian]

⚠️ GitGuardian has uncovered 6 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request

GitGuardian id	GitGuardian status	Secret	Commit	Filename
24149610	Triggered	Bearer Token	bd423f5	src/connectors/admin_service/jwt.rs	View secret
25120004	Triggered	Hashicorp Vault Token	1f483ee	config-to-validate.yaml	View secret
9658888	Triggered	Generic Password	0a5afab	config-to-validate.yaml	View secret
26598182	Triggered	influxdb_token	16b5a2a	src/project_app/tests.rs	View secret
25120005	Triggered	Generic High Entropy Secret	1f483ee	config-to-validate.yaml	View secret
25120006	Triggered	Generic High Entropy Secret	0a5afab	test_agent_report.sh	View secret

🛠 Guidelines to remediate hardcoded secrets

1. Understand the implications of revoking this secret by investigating where it is used in your code.
2. Replace and store your secrets safely. Learn here the best practices.
3. Revoke and rotate these secrets.
4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider

• following these best practices for managing and storing secrets including API keys and other credentials
• install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.

---

^{🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.}

Generally, to fix cleartext transmission issues, you must ensure that any network request carrying sensitive data is sent only over an encrypted channel (e.g., HTTPS/TLS). In code, this usually means enforcing the scheme of the base URL, refusing to operate if an insecure scheme is configured, or upgrading to a secure URL before sending the request.

For this specific code, the best minimal fix is to validate that self.address uses HTTPS when constructing the path used in .post(&path). We can add a small helper method (within the shown impl) that checks the scheme of the Vault address and returns an error if it is not https. We then call this helper at the beginning of store_ssh_key, so that if someone accidentally configures VaultSettings.address with http://..., the method will fail fast instead of sending the SSH keypair over cleartext HTTP. This keeps existing functionality for valid HTTPS configurations while preventing insecure configurations from being used.

Concretely:

• Add a private method in impl VaultClient (within the snippet) to parse self.address and ensure it starts with https://. We can do this with simple string checking to avoid adding dependencies or imports.
• At the start of store_ssh_key, call this validation method and return an error if the address is insecure.
• Leave the existing .post(&path) call intact; we just ensure it is only ever called with a secure base address.

---

In general, there are two broad strategies to address “cleartext transmission of sensitive information in URLs”:

1. Avoid putting sensitive data in the URL at all (path or query string), and instead send it in the request body over HTTPS, or map it to a non-sensitive surrogate identifier.
2. If the API requires the identifier in the path, ensure that:
   • The communication is strictly over HTTPS.
   • Application-level logging/tracing does not inadvertently log the full URL or sensitive parameters in cleartext.

For a Vault client, we usually cannot change Vault’s HTTP API to move user_id out of the path. Within this snippet, we also cannot see or modify how Client is constructed (for example to enforce HTTPS). Therefore, the most effective and least invasive changes we can make here are:

• Prevent sensitive data (user_id, and implicitly the derived path) from being logged in cleartext by the application’s tracing span.
• Explicitly annotate the tracing to skip both user_id and server_id, and to skip logging of the computed path.

Concretely, in src/helpers/vault.rs:

1. Update the #[tracing::instrument] attribute on fetch_ssh_key so that it skips recording user_id and server_id (and already skips self). This ensures that structured tracing does not record those values, which often end up in log files and log aggregation systems.
2. Since the path is derived directly from user_id/server_id, making sure it is not captured by the tracing span parameters (it is a local variable) is already handled; tracing only records function arguments and explicit fields by default, so no further code change inside the function is needed.

These changes do not alter the functional behavior of the Vault client at all; they only reduce the amount of sensitive information that may be exposed in logs, which is the primary channel where URL paths can become problematic in cleartext.