feat: implement optional SMTP password support#91
Merged
utkarsh232005 merged 2 commits intoMay 25, 2026
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds optional SMTP password storage to KDM's configuration system with environment-variable precedence. The interactive ChangesSMTP Password Configuration Support
Sequence DiagramsequenceDiagram
participant User
participant ConfigSetup as ConfigSetup
participant ConfigStore as ConfigStore
participant getSMTPSettings as getSMTPSettings()
participant SMTPAuth as SMTPAuth
User->>ConfigSetup: provide SMTP password (or leave empty)
ConfigSetup->>ConfigStore: conditionally setConfig('email_password', pass)
Note over ConfigStore: persists only if non-empty
User->>getSMTPSettings: request SMTP credentials
getSMTPSettings->>getSMTPSettings: check process.env.KDM_SMTP_PASSWORD
alt env var defined
getSMTPSettings->>SMTPAuth: use KDM_SMTP_PASSWORD
else env var undefined
getSMTPSettings->>ConfigStore: config.get('email_password')
ConfigStore->>SMTPAuth: return stored password
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Implements optional SMTP password support in the CLI by extending the config schema, updating the interactive config setup flow, and adding tests to cover password precedence and persistence behavior.
Changes:
- Add
email_passwordto config, ensure it’s cleared with other notification credentials, and allowgetSMTPSettings()to fall back to it whenKDM_SMTP_PASSWORDis not set. - Prompt for an optional SMTP password during
kdm config setupand update user-facing guidance text. - Add/extend Vitest coverage for setup prompting and SMTP password precedence.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/utils/config.ts | Adds email_password to config and uses it as a fallback for SMTP auth. |
| src/commands/config.ts | Prompts for optional SMTP password and updates CLI guide/list messaging. |
| src/tests/config.test.ts | Extends setup flow tests to cover password being skipped or saved. |
| src/tests/config-utils.test.ts | Adds tests for credential clearing and env-var vs config precedence. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
32
to
40
| @@ -34,7 +36,7 @@ export const getSMTPSettings = () => { | |||
| port: config.get('email_port') || 587, | |||
| auth: { | |||
| user: config.get('email_user'), | |||
| pass: process.env.KDM_SMTP_PASSWORD, | |||
| pass: process.env.KDM_SMTP_PASSWORD || config.get('email_password'), | |||
| }, | |||
Comment on lines
126
to
+130
| }); | ||
| } | ||
|
|
||
| console.log(chalk.gray('──────────────────────────────────────────────────')); | ||
| console.log(chalk.dim('\n Note: SMTP passwords must be set via KDM_SMTP_PASSWORD env var.\n')); | ||
| console.log(chalk.dim('\n Note: SMTP password can be set either in config or via the KDM_SMTP_PASSWORD environment variable, which takes precedence if both are set.\n')); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/__tests__/config-utils.test.ts`:
- Around line 70-86: Add a third case to the getSMTPSettings() test to assert
strict environment-variable precedence when KDM_SMTP_PASSWORD is an empty
string: set process.env.KDM_SMTP_PASSWORD = '' (after setting mockConfigStore
email_password) then call getSMTPSettings() and expect settings.auth.pass to
equal '' (not the config value). Ensure you restore or delete the env var
between cases so the other two assertions still run; reference the test around
getSMTPSettings and mockConfigStore.set('email_password', ...) when adding this
assertion.
In `@src/utils/config.ts`:
- Line 10: The config field email_password should not be persisted in
plain-text; remove or stop writing email_password to the persistent conf storage
and instead read it from an environment variable or secure OS keychain at
runtime (reference the email_password property in src/utils/config.ts). If
persistence is absolutely required add an explicit opt-in flag (e.g.,
persistEmailPassword) and store only an encrypted blob or a protected reference,
and update documentation and any README to warn about the risk; ensure code
paths that previously read/write email_password now pull from process.env or the
keychain API and that any new opt-in is clearly gated and logged.
- Line 39: Replace the falsy-OR fallback for the SMTP password so an
intentionally set empty KDM_SMTP_PASSWORD is honored; instead of using
"process.env.KDM_SMTP_PASSWORD || config.get('email_password')" update the
assignment for the pass property to use an explicit undefined check (e.g.,
process.env.KDM_SMTP_PASSWORD !== undefined ? process.env.KDM_SMTP_PASSWORD :
config.get('email_password')) so env takes strict precedence even when empty.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ef3ef831-f6bb-40d3-8abd-137c078791fe
📒 Files selected for processing (4)
src/__tests__/config-utils.test.tssrc/__tests__/config.test.tssrc/commands/config.tssrc/utils/config.ts
| email_port?: number; | ||
| email_user?: string; | ||
| email_to?: string; | ||
| email_password?: string; |
Contributor
There was a problem hiding this comment.
Avoid storing SMTP password in plain-text config storage.
Persisting email_password in conf creates a secret-at-rest risk. Prefer env-only or secure OS keychain storage; if persistence is required, gate it behind explicit opt-in and document the risk clearly.
As per coding guidelines "Verify that sensitive information is not stored in plain text if possible."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/config.ts` at line 10, The config field email_password should not
be persisted in plain-text; remove or stop writing email_password to the
persistent conf storage and instead read it from an environment variable or
secure OS keychain at runtime (reference the email_password property in
src/utils/config.ts). If persistence is absolutely required add an explicit
opt-in flag (e.g., persistEmailPassword) and store only an encrypted blob or a
protected reference, and update documentation and any README to warn about the
risk; ensure code paths that previously read/write email_password now pull from
process.env or the keychain API and that any new opt-in is clearly gated and
logged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #74
Description
This PR implements optional SMTP password support across the configuration schema, interactive setup flow, and user-facing text in
src/utils/config.tsandsrc/commands/config.ts.Changes
email_passwordproperty toKDMConfig.clearNotificationCredentials()to deleteemail_passwordwhen resetting/switching provider credentials.getSMTPSettings()to prioritizeprocess.env.KDM_SMTP_PASSWORDand fall back toemail_password.config.test.ts.config-utils.test.tsto test clean retrieval logic.Summary by CodeRabbit
New Features
Bug Fixes
Tests