Skip to content

fix: protect MCP config credentials - #143

Open
vycdev2 wants to merge 2 commits into
vycdev:mainfrom
vycdev2:fix/protect-mcp-config-credentials
Open

fix: protect MCP config credentials#143
vycdev2 wants to merge 2 commits into
vycdev:mainfrom
vycdev2:fix/protect-mcp-config-credentials

Conversation

@vycdev2

@vycdev2 vycdev2 commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • create the generated MCP config with owner-only permissions on POSIX systems
  • tighten existing POSIX config-file permissions before writing authentication credentials
  • preserve config generation on Windows without claiming Unix-mode ACL guarantees
  • cover permissive pre-existing config files with a regression test

Verification

  • npm run build — passed
  • node --test tests/mcpHttp.test.mjs — passed (3 tests)
  • npm test — passed (198 tests)
  • git diff --check — passed

Risk

  • Low: the JSON write path is unchanged; POSIX file mode is tightened to 0600, while Windows retains its platform-native permission behavior.

This was generated by an AI agent (vycdev2). Please verify any changes before merging or applying.

@vycdev

vycdev commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Found a Windows regression in the new permission test (tests/mcpHttp.test.mjs:142). I built this PR and ran node --test tests/mcpHttp.test.mjs on Windows / Node 24.19.0: 2 passed, 1 failed, with 438 !== 384 (reported mode 0666 rather than 0600). Windows does not implement the Unix owner/group/other permission model through fchmodSync, so this assertion fails even when config generation succeeds.

Please make the mode assertion platform-aware, retain the 0600 check on POSIX, and avoid describing fchmodSync as providing owner-only ACLs on Windows. The JSON and credential transport assertions should still run on every platform. Leaving this PR open until the newly introduced failure is addressed.

@vycdev2

vycdev2 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the Windows test regression. The 0600 mode assertion now runs only on non-Windows platforms, while the JSON and credential-transport assertions still run everywhere. The changelog now scopes the owner-only permission claim to POSIX systems.

Verification: node --test tests/mcpHttp.test.mjs (3 passed) and npm test (198 passed).

This was generated by an AI agent (vycdev2). Please verify any changes before merging or applying.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants