Skip to content

Follow-up improvements from mTLS PR review#103

Merged
pchuri merged 2 commits into
pchuri:mainfrom
anandghegde:fix/mtls-review-followups
Apr 14, 2026
Merged

Follow-up improvements from mTLS PR review#103
pchuri merged 2 commits into
pchuri:mainfrom
anandghegde:fix/mtls-review-followups

Conversation

@anandghegde
Copy link
Copy Markdown
Contributor

Summary

Follow-up to #102, addressing the post-merge review suggestions from @pchuri.

  • Warn when client key file permissions are more permissive than 0600 on Unix systems (private key security risk)
  • Extract mtlsCertQuestion() helper to eliminate duplicated prompt definitions across initConfig interactive mode and promptForMissingValues
  • Extract validateMtlsProtocol() helper to share the http+mTLS incompatibility check across CLI validation, env config, and profile config
  • Update README mTLS examples to use platform-neutral paths (~/.certs/) instead of macOS-specific /Users/you/.certs/

Validation

  • npm test -- --runInBand — 178 tests passing
  • npm run lint — clean

- Warn when client key file permissions are more permissive than 0600
  on Unix systems (security risk for private keys)
- Extract mtlsCertQuestion() helper to eliminate duplicated prompt
  definitions across initConfig interactive mode and promptForMissingValues
- Extract validateMtlsProtocol() helper to share the http+mtls
  incompatibility check across CLI validation, env config, and profiles
- Update README mTLS examples to use platform-neutral paths (~/.certs/)
  instead of macOS-specific /Users/you/.certs/
Update follow-redirects 1.15.11 -> 1.16.0 (via axios 1.13.5 -> 1.15.0)
to fix moderate severity vulnerability where custom authentication
headers leak to cross-domain redirect targets.
Copy link
Copy Markdown
Owner

@pchuri pchuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — clean follow-up that addresses all feedback from #102.

All four items landed correctly: private key permission check with a helpful chmod 600 hint, mtlsCertQuestion() helper eliminating the triple duplication, validateMtlsProtocol() shared across all three config paths, and platform-neutral README paths. Net -39 lines is a nice bonus.

The bundled follow-redirects bump (GHSA-r4q5-vmmm-2653) is a welcome addition — good to get the auth header leak fix in alongside the mTLS work.

@pchuri pchuri merged commit df9e871 into pchuri:main Apr 14, 2026
6 checks passed
@github-actions
Copy link
Copy Markdown

🎉 This PR is included in version 1.30.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants