Fix masking logic for file output#12
Conversation
WalkthroughThe update enhances the secret handling process in Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as Main Process
participant Secrets as keyValueSecrets
participant Core as core.setSecret
participant Logger as DebugLogger
Runner->>Secrets: Retrieve secrets
loop For each secret
Runner->>Core: Mask secret value
end
Runner->>Logger: Log environment variables
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
index.js (2)
61-65: Consider adding explicit error handling.While the current implementation is effective, there's no error handling for the masking operation. Consider wrapping this in a try-catch block to gracefully handle any potential errors during the masking process.
// Mask secrets in GitHub logs -Object.entries(keyValueSecrets).forEach(([, value]) => { - core.setSecret(value); -}); +try { + Object.entries(keyValueSecrets).forEach(([, value]) => { + core.setSecret(value); + }); + core.debug("Successfully masked all secrets"); +} catch (error) { + core.warning(`Error masking secrets: ${error.message}`); + // Continue execution as this is not a critical failure +}
62-64: Check for null/undefined values.The current implementation doesn't check if the secret value is null or undefined before calling
core.setSecret(). Although it's unlikely to happen based on howkeyValueSecretsis constructed ininfisical.js, adding a safety check would make the code more robust.-Object.entries(keyValueSecrets).forEach(([, value]) => { - core.setSecret(value); -}); +Object.entries(keyValueSecrets).forEach(([, value]) => { + if (value !== null && value !== undefined) { + core.setSecret(value); + } +});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
index.js(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
index.js (1)
infisical.js (1)
keyValueSecrets(78-83)
🔇 Additional comments (1)
index.js (1)
61-64: Great security improvement!This change correctly masks all secret values before they're exported, preventing them from appearing in GitHub logs even when using commands like
caton exported files. Previously, masking was likely only applied for environment variables but not for file exports.
2 participants
When using
export-type: file, the retrieved secret values can be seen in the GitHub logs by usingcat .env. This PR moves the masking logic above the export logic which will fix the issue and show masked values instead.Summary by CodeRabbit