Skip to content

Hardening report: MCP stdio cleanliness, Windows build, runtime validation, dependency audit, and prompt persistence #48

Description

@lcv-leo

Summary

I audited @mettamatt/code-reasoning after seeing MCP client failures consistent with stdio/JSON-RPC parsing problems. The package is useful and the current main branch already appears to include one important fix for non-JSON stdout logging in PromptManager, but several issues still remain across the npm-published package, current source, runtime validation, Windows development, dependency security, and prompt persistence.

This issue consolidates the findings so they can be tracked together or split into smaller issues/PRs.

Environment used for verification

  • OS: Windows 11 / PowerShell
  • Node.js: v25.9.0 locally; package declares >=22.0.0
  • npm package checked: @mettamatt/code-reasoning@0.8.1
  • GitHub source checked: main at 689ab67415ffc1eaa4d0fb6f453cb73f5c855d99 (Merge pull request #44 from GW-Kang/fix/redirect-stdout-logs-to-stderr)
  • npm latest at the time of audit: 0.8.1

Findings

1. npm 0.8.1 still emits non-JSON text to stdout

The installed npm package still contains console.info(...) calls in dist/src/prompts/manager.js:

console.info(`Using config directory: ${resolvedConfigDir}`);
console.info('PromptManager initialized with', Object.keys(this.prompts).length, 'prompts');
console.info(`Created ${description}: ${directoryPath}`);

For an MCP stdio server, stdout must be reserved for JSON-RPC. These lines can corrupt clients that parse stdout as MCP messages and can lead to errors such as Unexpected end of JSON input or similar JSON-RPC parsing failures.

Current main appears to have fixed this in source by changing these logs to console.error, but the fix does not appear to be published to npm yet.

Recommended action:

  • Publish a patch release containing the stdout redirection fix.
  • Add a regression test that spawns the built server and asserts stdout contains no non-JSON diagnostic text before/during initialization.

2. Server runtime metadata is stale

package.json reports 0.8.1, but src/server.ts still hardcodes runtime metadata as 0.7.0:

const serverMeta = { name: 'code-reasoning-server', version: '0.7.0' } as const;

This causes runtime inventory and MCP diagnostics to report the wrong version.

Recommended action:

  • Generate/read the version from package.json at build/runtime, or update it during release.
  • Add a regression test asserting tool._meta.server_version matches package.json.

3. Windows install/build fails because prepare runs chmod

On Windows, npm ci fails because prepare -> npm run build invokes Unix-only chmod:

> @mettamatt/code-reasoning@0.8.1 build
> tsc && chmod +x dist/*.js dist/**/*.js

'chmod' is not recognized as an internal or external command

Recommended action:

  • Replace direct chmod usage with a small cross-platform Node script that no-ops on Windows and chmods dist/**/*.js on POSIX.
  • Optionally add a Windows CI job.

4. Production dependency audit reports vulnerabilities with the locked MCP SDK

Using the current lockfile, npm audit --omit=dev --audit-level=moderate reported production vulnerabilities through @modelcontextprotocol/sdk@1.18.1 and transitive dependencies.

The latest MCP SDK available during the audit was 1.29.0, and updating to that version resolved production audit findings in my local verification.

Recommended action:

  • Upgrade @modelcontextprotocol/sdk to the latest compatible version.
  • Refresh package-lock.json.
  • Run npm audit --audit-level=moderate in CI.

5. Thought sequence validation accepts inconsistent or unbounded histories

The tool currently accepts semantically invalid sequences. Examples observed through an MCP client:

  • thought_number: 5, total_thoughts: 3 is accepted.
  • The first thought can be thought_number: 20.
  • A revision can reference a nonexistent/future thought, e.g. revises_thought: 99.
  • Repeating thought_number: 1 bypasses the intended MAX_THOUGHTS = 20 cap; after 25 calls the server returned thought_history_length: 25.

Recommended action:

  • Enforce thought_number <= total_thoughts.
  • Enforce sequential progression for a reasoning sequence, or introduce explicit session/reset semantics.
  • Validate revises_thought <= thoughtHistory.length.
  • Validate branch_from_thought <= thoughtHistory.length (already present for branch references).
  • Enforce the max cap against actual history length, not only against the submitted thought_number.
  • Add regression tests for these cases.

6. Thought history is process-global and not scoped by session/client

The server keeps thoughtHistory and branches in memory for the lifetime of the MCP process. In long-lived MCP hosts, separate tasks can inherit previous history unless the process restarts.

Recommended action:

  • Add an optional session_id field and scope histories by session, or define/reset a new sequence when thought_number === 1 with no branch/revision fields.
  • Consider exposing an explicit reset tool or reset semantics.

7. Full thought content is logged to stderr

logger.info(formatThought(data)) logs the full thought text. Thoughts may include source snippets, local paths, credentials pasted by mistake, API keys, or private business context.

Recommended action:

  • Log structured metadata at info level only: thought number, total, branch/revision fields, thought length, history length.
  • Log full thought text only in debug mode, with redaction and truncation.
  • Redact common secret patterns before logging.

8. Prompt value persistence stores unredacted user values and writes non-atomically

PromptManager stores prompt arguments in ~/.code-reasoning/prompt_values.json via fs.writeFileSync without atomic replace, locking, size limits, or redaction.

Potential impacts:

  • A crash or concurrent process can corrupt the JSON file.
  • Sensitive prompt arguments or token-like strings can be persisted in clear text.
  • If the file is corrupted, initialization behavior is fragile/surprising.

Recommended action:

  • Write to a temp file and rename atomically.
  • Redact or avoid storing sensitive argument names/values (api_key, token, secret, password, etc.).
  • Add per-argument length caps.
  • Preserve/rename corrupt files and recreate defaults instead of failing or silently falling back.
  • Validate prompt argument values are strings before calling .trim().

9. Built-in prompts assume a specific filesystem MCP and suggest modifications

Several prompt templates say:

Note: You can access and modify files using the filesystem tool mcp.

This is host-specific and can be unsafe for a reasoning prompt. The host may not expose a filesystem MCP, and a reasoning/template layer should not encourage file modification unless the caller explicitly requested implementation changes.

Recommended action:

  • Reword prompts to say: use available file-reading/search tools when exposed by the host, and do not modify files unless explicitly requested.

10. Test coverage gaps

Existing regression tests cover basic list/call/prompt behavior, but not the failure modes above.

Suggested tests:

  • stdio cleanliness test for built server startup.
  • server metadata version parity test.
  • Windows-safe build script test or CI matrix entry.
  • invalid thought_number > total_thoughts.
  • invalid future revises_thought.
  • repeated thought numbers cannot exceed actual history cap.
  • prompt persistence redaction and corrupt-file recovery.

Local validation of a patched build

I locally validated a patched build with the following outcomes:

  • npm run build: passed on Windows.
  • npm test: passed.
  • Additional prompt-manager tests: passed.
  • npm audit --audit-level=moderate: 0 vulnerabilities after dependency refresh.
  • Direct MCP stdio client probe: listed code-reasoning, returned server_version: 0.8.2-lcv.0, processed valid calls, and rejected invalid future revision references with structured JSON guidance.
  • Raw spawn probe: stdout remained empty during startup; diagnostics went to stderr.

I can provide a focused PR if helpful, but opened this issue first because the changes touch multiple concerns: release packaging, dependency updates, runtime validation, logging, persistence, and test coverage.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions