Potential fix for code scanning alert no. 3: Clear-text logging of sensitive information#146
Conversation
…nsitive information Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
|
Warning
|
| Layer / File(s) | Summary |
|---|---|
Credential presence status display src/commands/auth.ts |
getMaskedSecret function is refactored to return credential presence indicators—(set) when credentials exist, (not set) otherwise—removing the previous masking logic that showed partial prefix/suffix patterns. |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
🐰 A masked secret hops away,
Status tags take center stage today,
(set)or(not set)we now shall see,
Simpler truths, no masquerade—just be!
The auth list hops more light and free! 🌟
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title directly addresses the primary change: modifying how sensitive credentials are displayed to prevent clear-text logging of sensitive information. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
alert-autofix-3
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.
Comment @coderabbitai help to get the list of available commands and usage tips.
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
There was a problem hiding this comment.
Gates Passed
See analysis details in CodeScene
Quality Gate Profile: The Bare Minimum
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
|
🤖 Hi @utkarsh232005, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/commands/auth.ts (1)
389-389:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate command description to reflect new behavior.
The description states "List configured AI providers with masked secrets," but the implementation now displays credential presence status (
(set)or(not set)) rather than masked secrets. Update the description for accuracy.📝 Suggested description update
- .description('List configured AI providers with masked secrets') + .description('List configured AI providers with credential status')🤖 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/commands/auth.ts` at line 389, Update the command description string used in the auth command builder (the .description(...) call in src/commands/auth.ts) to accurately reflect the current behavior: it no longer shows masked secrets but instead displays credential presence status like "(set)" or "(not set)". Replace the existing text "List configured AI providers with masked secrets" with a concise phrase such as "List configured AI providers and show credential presence (set/not set)" so the description matches the implementation.
🤖 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/commands/auth.ts`:
- Around line 69-76: Update the test in src/__tests__/auth.test.ts to match the
new output of getMaskedSecret: replace the expectation that checks for the
obfuscated key string (e.g., 'supe...ikey') with the literal '(set)' when
provider.password is present; locate the assertion referencing auth list output
and change its expected value to '(set)' so it aligns with the
getMaskedSecret(provider: AIProviderConfig) implementation.
---
Outside diff comments:
In `@src/commands/auth.ts`:
- Line 389: Update the command description string used in the auth command
builder (the .description(...) call in src/commands/auth.ts) to accurately
reflect the current behavior: it no longer shows masked secrets but instead
displays credential presence status like "(set)" or "(not set)". Replace the
existing text "List configured AI providers with masked secrets" with a concise
phrase such as "List configured AI providers and show credential presence
(set/not set)" so the description matches the implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| /** | ||
| * Masks secrets/API keys to avoid leaking them in plain text display. | ||
| * Returns a non-sensitive credential status for display. | ||
| * @param provider The AI provider configuration. | ||
| * @returns Masked representation. | ||
| * @returns '(set)' when a password exists, otherwise '(not set)'. | ||
| */ | ||
| const getMaskedSecret = (provider: AIProviderConfig): string => { | ||
| const secret = provider.password; | ||
| if (!secret) return '(not set)'; | ||
| if (secret.length <= 8) return '********'; | ||
| return `${secret.substring(0, 4)}...${secret.substring(secret.length - 4)}`; | ||
| return provider.password ? '(set)' : '(not set)'; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find test assertions that expect the old masked secret format
# Search for test files asserting on masked credential patterns
rg -n -C3 --type=ts --glob='**/*test*' -e 'supe.*ikey|stringContaining.*\.\.\.' -e 'getMaskedSecret' -e 'auth.*list'Repository: KDM-cli/kdm-cli
Length of output: 1057
Update auth list test expectations for credential status output.
src/__tests__/auth.test.ts currently asserts expect(...).toHaveBeenCalledWith(...'supe...ikey'), but getMaskedSecret now returns '(set)' when provider.password exists (and '(not set)' otherwise). Change the assertion to expect '(set)' to match the new output format.
🤖 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/commands/auth.ts` around lines 69 - 76, Update the test in
src/__tests__/auth.test.ts to match the new output of getMaskedSecret: replace
the expectation that checks for the obfuscated key string (e.g., 'supe...ikey')
with the literal '(set)' when provider.password is present; locate the assertion
referencing auth list output and change its expected value to '(set)' so it
aligns with the getMaskedSecret(provider: AIProviderConfig) implementation.
|
🤖 I'm sorry @utkarsh232005, but I was unable to process your request. Please see the logs for more details. |
1 participant
Potential fix for https://github.com/KDM-cli/kdm-cli/security/code-scanning/3
Best fix: stop emitting any value derived from
provider.passwordin logs.In
src/commands/auth.ts, replace the “Password” line to print only presence status, and update masking helper accordingly (or stop using it for output). This preserves functionality (users still know whether credentials are configured) without leaking secret fragments.Concrete changes:
getMaskedSecret, return only'(set)'or'(not set)', without reading or slicing secret contents beyond existence check.getMaskedSecret(p).No new imports or dependencies are required.
Suggested fixes powered by Copilot Autofix. Review carefully before merging.
Summary by CodeRabbit
auth listcommand now displays provider credential status as "(set)" or "(not set)" instead of masked values.