Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,31 @@ jobs:
with:
node-version: ${{ matrix.node-version }}

# `prepare` compiles src/ -> dist/ as part of the install.
- name: Install dependencies
run: npm ci

- name: Typecheck
run: npm run typecheck

- name: Lint
run: npm run lint

# Coverage thresholds are pinned at 100% in vitest.config.mjs, so this
# also fails the build if a change adds an untested line or branch.
- name: Run tests
run: npx vitest run --coverage

# The published entry point loads dist/; make sure the compiled server
# actually starts and answers on STDIO. This chain broke in 1.3.6 and
# 1.3.8, so it is verified rather than assumed.
- name: Smoke-test the compiled server
shell: bash
run: |
echo '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"ci","version":"1"}}}' \
| MCP_SILENT=true node bin/mcp-ssh.js \
| grep -q '"serverInfo"'

# The dependency tree is platform independent — auditing it once is enough.
- name: Security audit
if: matrix.os == 'ubuntu-latest' && matrix.node-version == 22
Expand Down
7 changes: 6 additions & 1 deletion .npmignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,17 @@ test-ssh/
vitest.config.mjs
coverage/

# TypeScript sources and build config - the package ships compiled dist/
tsconfig.json
tsconfig.build.json
eslint.config.mjs

# Generated files
*.tgz

# Keep these files for npm package
!package.json
!README.md
!server.mjs
!dist/
!LICENSE
!CHANGELOG.md
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Changed
- **Ported to TypeScript.** The single self-contained `server.mjs` is now seven typed modules under `src/`, compiled to `dist/` by `tsc`: `server.ts` (MCP wiring and `main()`), `tools.ts` (tool schemas and dispatch), `ssh-client.ts`, `ssh-config-parser.ts`, `config-values.ts`, `platform.ts` (everything with module-load side effects) and `types.ts`. `bin/mcp-ssh.js` and the DXT package load `dist/server.js`; `dist/` is generated, not tracked in git, and built by the `prepare` script on install.
- `tsconfig.json` runs `strict` plus `noUncheckedIndexedAccess`, `exactOptionalPropertyTypes`, `noImplicitOverride`, `noFallthroughCasesInSwitch`, `noPropertyAccessFromIndexSignature` and `verbatimModuleSyntax`. Test files are checked under a lighter config (`tsconfig.test.json`).
- Added ESLint with `typescript-eslint` type-aware rules (`strictTypeChecked` + `stylisticTypeChecked`). Relaxations are documented in place; notably `prefer-nullish-coalescing` exempts strings and numbers, because `??` is *not* equivalent to `||` for a stripped launcher environment (an empty `%ProgramData%` must fall through, see #10) or for `timeout || DEFAULT`.
- Removed dead devDependencies `ts-node` and `@types/ssh2` — the repo had no `.ts` files and never used `ssh2`.
- The test suite is split along the same module boundaries (four files plus shared `test-helpers.ts`) and grew from 148 to 152 tests, still at 100% coverage of statements, branches, functions and lines.
- CI now runs `typecheck`, `lint`, the suite, and a smoke test that starts the *compiled* server over STDIO — the delivery chain that broke in 1.3.6 and 1.3.8 is now verified on every push, on Linux and Windows across Node 20/22/24.
- No behavioural changes: every existing test passes unmodified except where a mock had to follow the module split.

### Security
- Resolved all 17 open `npm audit` advisories (12 high, 4 moderate, 1 low) that accumulated since the 1.3.7 cleanup. `npm audit` now reports zero vulnerabilities again. `npm audit fix` could not be used — it aborts with an internal npm error (`Cannot read properties of null (reading 'edgesOut')`) on this tree's `overrides` — so the fixes are pinned explicitly:
- Direct bumps, all within the existing semver range: `@modelcontextprotocol/sdk` 1.27.1 → 1.30.0, `vitest`/`@vitest/coverage-v8` 4.1.4 → 4.1.10, `@anthropic-ai/dxt` 0.2.5 → 0.2.6. These cleared the `vite`, `postcss` and `nanoid` advisories.
Expand Down
63 changes: 42 additions & 21 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,23 @@ This is MCP SSH Agent (@aiondadotcom/mcp-ssh) - a Model Context Protocol (MCP) s
### Basic Operations
- `npm start` - Start the MCP server (same as `npm run dev`)
- `npm run dev` - Start the MCP server with debug output
- `npm run build` - Currently a no-op (echo "Build skipped")
- `npm test` - Run the vitest suite (`server.test.mjs`) with coverage
- `npm run build` - Compile `src/` to `dist/` (`tsc -p tsconfig.build.json`)
- `npm run typecheck` - `tsc --noEmit` for `src/` (strict) and for the tests (relaxed)
- `npm run lint` / `npm run lint:fix` - ESLint with type-aware rules
- `npm test` - Run the vitest suite with coverage
- `npm run test:watch` - Vitest in watch mode

**The compiled output must exist before the server can run.** `npm install` builds it via the
`prepare` script; after editing `src/` run `npm run build` (or `npm test`, which reads `src/`
directly and needs no build).

### Development Scripts
- `./start.sh` - Start the server with debug output
- `./start-silent.sh` - Start the server in silent mode (no debug output)
- `node bin/mcp-ssh.js` - Direct server execution. **Not** `node server.mjs`: since 1.3.6 that file no longer auto-runs `main()`, so running it directly exits immediately (this caused the 1.3.8 regression)
- `node bin/mcp-ssh.js` - Direct server execution. It loads `dist/server.js` and fails with a
clear message if the build is missing. Do **not** add an `is this module run directly?` check
to the server module: that heuristic compared `process.argv[1]` against forward-slash suffixes,
never matched on Windows, and made the server exit silently (issue #8, regression in 1.3.8)

### Publishing
- `npm version patch|minor|major` - Bump version and create git tag
Expand All @@ -31,21 +40,33 @@ This is MCP SSH Agent (@aiondadotcom/mcp-ssh) - a Model Context Protocol (MCP) s

## Architecture

### Main Entry Point
- `server.mjs` - Self-contained MCP server implementation that includes all functionality inline to avoid module resolution issues
TypeScript under `src/`, compiled to `dist/` by `tsc`. `dist/` is generated and not in git;
`bin/mcp-ssh.js` and the DXT package both load `dist/server.js`.

### Modules
- `src/server.ts` - Entry point. Wires the MCP server, registers handlers, exports `main()`
- `src/tools.ts` - Tool schemas (`TOOL_DEFINITIONS`) and `callTool()` dispatch
- `src/ssh-client.ts` - `SSHClient`: every ssh/scp operation, plus the security assertions
- `src/ssh-config-parser.ts` - `SSHConfigParser`: host discovery, Includes, permission checks
- `src/config-values.ts` - ssh-config value normalization (see below) and `hostMatchesAlias()`
- `src/platform.ts` - `isWindows`, Windows env normalization, `resolveExecutable()`,
`SSH_BIN`/`SCP_BIN`, `debugLog()`. **Everything with module-load side effects lives here**
- `src/types.ts` - Shared types

### Other Files
- `bin/mcp-ssh.js` - Binary wrapper for npx compatibility; the real entry point (imports and calls `main()`)
- `server.test.mjs` - Test suite
- `vitest.config.mjs` - Test and coverage configuration, including the 100% coverage thresholds
- `bin/mcp-ssh.js` - Executable entry point; loads `dist/server.js`
- `tsconfig.json` (strict, `src/` only) / `tsconfig.build.json` (build) / `tsconfig.test.json`
(relaxed, tests). Test files are checked with a lighter rule set on purpose: mock objects and
index-signature access are normal there, and the tests themselves are the safety net
- `eslint.config.mjs` - typescript-eslint, type-aware. Relaxations are commented in place
- `vitest.config.mjs` - Test and coverage configuration, including the 100% thresholds
- `.gitattributes` - Forces LF checkout on every platform (see Testing)

### Key Design Decisions
1. **Native SSH Tools**: Uses system `ssh` and `scp` commands rather than JavaScript SSH libraries for reliability
2. **Self-contained**: `server.mjs` includes all code inline to avoid ESM import issues
3. **Silent Mode**: Controlled by `MCP_SILENT` environment variable to disable debug output when used as MCP server
4. **No shell on spawn**: All `spawn`/`execFile` calls use `shell: false`. On Windows, `ssh.exe`/`scp.exe` are resolved to absolute paths once at startup via `resolveExecutable()` (PATH + PATHEXT walk), so PATH lookup does not require `shell: true`. This is required to prevent local command injection through shell metacharacters in tool arguments.
5. **Strict `hostAlias` whitelist**: `_assertSafeHostAlias()` (`SSHClient`) rejects any `hostAlias` that does not match `^[A-Za-z0-9_.@:][A-Za-z0-9._@:-]*$`. Combined with the `--` argument terminator on every `ssh`/`scp` invocation, this blocks SSH option injection (e.g. `-oProxyCommand=…`) and shell-metacharacter injection. Validation is applied at the public entry points (`runRemoteCommand`, `uploadFile`, `downloadFile`) and transitively covers `checkConnectivity` and `runCommandBatch`. **Do not weaken or bypass this validator** without understanding the security implications — see CHANGELOG entry for 1.3.5.
2. **Silent Mode**: Controlled by `MCP_SILENT` environment variable to disable debug output when used as MCP server
3. **No shell on spawn**: All `spawn`/`execFile` calls use `shell: false`. On Windows, `ssh.exe`/`scp.exe` are resolved to absolute paths once at startup via `resolveExecutable()` (PATH + PATHEXT walk), so PATH lookup does not require `shell: true`. This is required to prevent local command injection through shell metacharacters in tool arguments.
4. **Strict `hostAlias` whitelist**: `_assertSafeHostAlias()` (`SSHClient`) rejects any `hostAlias` that does not match `^[A-Za-z0-9_.@:][A-Za-z0-9._@:-]*$`. Combined with the `--` argument terminator on every `ssh`/`scp` invocation, this blocks SSH option injection (e.g. `-oProxyCommand=…`) and shell-metacharacter injection. Validation is applied at the public entry points (`runRemoteCommand`, `uploadFile`, `downloadFile`) and transitively covers `checkConnectivity` and `runCommandBatch`. **Do not weaken or bypass this validator** without understanding the security implications — see CHANGELOG entry for 1.3.5.

## SSH Configuration Integration

Expand Down Expand Up @@ -113,19 +134,19 @@ Host myrouter

### Test Suite Invariants

`vitest.config.mjs` pins **100% of statements, branches, functions and lines** of `server.mjs`
and fails the build below that. `bin/mcp-ssh.js` is excluded (top-level await that starts a real
server). When adding code, add the test with it; when a branch turns out to be unreachable,
`vitest.config.mjs` pins **100% of statements, branches, functions and lines** of `src/` and
fails the build below that. `bin/mcp-ssh.js` is excluded (top-level await that starts a real
server), as is `src/types.ts` (compiles to nothing executable). When adding code, add the test with it; when a branch turns out to be unreachable,
prefer deleting it over working around the threshold — that is how the dead
`process.env.Path` fallback in `resolveExecutable()` was removed.

**Platform-specific code is tested from either OS, never skipped.** `loadServerAs(platform, env)`
in `server.test.mjs` re-imports `server.mjs` with `process.platform` (and optionally parts of the
in the test suite re-imports the module graph with `process.platform` (and optionally parts of the
environment) faked, so the Windows branches are covered when running on macOS/Linux and vice
versa. Things it has to handle, and that new tests must respect:
- `vi.resetModules()` re-runs the `vi.mock('fs/promises')` factory, so the returned `fs` spies are
**new objects** — the statically imported `readFile`/`stat`/… are a different module instance.
- `server.mjs` writes `ProgramData`/`ALLUSERSPROFILE` to `process.env` at import time. The helper
- `src/platform.ts` writes `ProgramData`/`ALLUSERSPROFILE` to `process.env` at import time. The helper
always saves and restores them (`ENV_MUTATED_AT_IMPORT`) and exposes `envAfterImport`, because
the restore happens before a test can assert. Without this, one Windows import leaks state into
later tests and makes those branches *look* covered while nothing asserts them.
Expand All @@ -135,8 +156,8 @@ versa. Things it has to handle, and that new tests must respect:
process-starting methods are stubbed so tests never spawn actual `ssh`/`scp`.

**CI** runs Linux and Windows across Node 20/22/24. `.gitattributes` forces an LF checkout on all
platforms: with CRLF, Vite's SSR transform cannot parse the `#!/usr/bin/env node` shebang in
`server.mjs` and the entire suite dies with a `SyntaxError` before any test runs. Do not remove it.
platforms. It was added because a CRLF checkout broke the suite outright, and it also keeps the
shell scripts executable. Do not remove it.

### Manual Testing
```bash
Expand Down Expand Up @@ -192,8 +213,8 @@ The LLM driving this MCP server is **not trusted** — its tool arguments can be

## Important Notes

- The project is ESM-only (`"type": "module"` in package.json). The `.mjs` extension on `server.mjs` is historical and redundant given `"type": "module"`; keep it for now to avoid touching `bin/`, `manifest.json`, `package.json` `main`, and the start scripts.
- Production code is in `server.mjs`, not compiled from TypeScript
- The project is ESM-only (`"type": "module"` in package.json), so `tsc` emits `.js` files that Node treats as ESM. Relative imports inside `src/` must carry the `.js` extension (NodeNext resolution), even though the source files are `.ts`.
- Production code is TypeScript in `src/`, compiled to `dist/`. Never edit `dist/` — it is regenerated on every build.
- SSH operations require properly configured SSH keys or `@password` annotations
- The agent runs over STDIO as an MCP server, not as a standalone application
- DXT packages provide one-click installation alternative to manual JSON configuration
37 changes: 30 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -660,16 +660,27 @@ The DXT file will be available as a release asset for users to download and inst
Contributions are welcome! Please feel free to submit a Pull Request.

```bash
npm install
npm install # also compiles src/ -> dist/ via the prepare script
npm run build # tsc
npm run typecheck # tsc --noEmit, strict for src/ and relaxed for tests
npm run lint # eslint with type-aware rules
npm test # vitest with coverage
npm run test:watch # watch mode
```

Two things to know before opening a PR:
The server is written in TypeScript under `src/` and compiled to `dist/`, which is what
`bin/mcp-ssh.js` loads and what ships to npm. `dist/` is not in git — a fresh checkout gets
it from `npm install`.

Three things to know before opening a PR:

- **Coverage is a build gate.** `vitest.config.mjs` pins statements, branches, functions and
lines of `server.mjs` at 100%, so a change that adds an untested line fails CI. If a branch
lines of `src/` at 100%, so a change that adds an untested line fails CI. If a branch
is genuinely unreachable, removing it is usually better than working around the threshold.
- **Lint rules are calibrated, not stock.** Where a rule is relaxed, the reason is in a comment
next to it. `prefer-nullish-coalescing` in particular exempts strings and numbers on purpose:
`||` and `??` are *not* interchangeable for environment variables (a stripped launcher env
reports an empty string, see issue #10) or for `timeout || DEFAULT`.
- **CI runs on Linux and Windows** across Node 20, 22 and 24. Platform-specific code paths are
tested from either OS by re-importing the module with `process.platform` faked — see
`loadServerAs()` in `server.test.mjs` — rather than by skipping tests on one platform.
Expand All @@ -682,8 +693,22 @@ MIT License - see LICENSE file for details.

```
mcp-ssh/
├── server.mjs # Main MCP server implementation (self-contained)
├── server.test.mjs # Test suite (vitest)
├── src/ # TypeScript sources
│ ├── server.ts # Entry point: MCP server wiring and main()
│ ├── tools.ts # Tool definitions and dispatch
│ ├── ssh-client.ts # All ssh/scp operations
│ ├── ssh-config-parser.ts # Host discovery from config and known_hosts
│ ├── config-values.ts # ssh-config value normalization
│ ├── platform.ts # Platform detection, binary resolution, logging
│ ├── types.ts # Shared types
│ └── server.test.ts # Test suite (vitest)
├── dist/ # Compiled output (generated, not in git)
├── bin/
│ └── mcp-ssh.js # Executable entry point (loads dist/server.js)
├── tsconfig.json # Strict compiler options for src/
├── tsconfig.build.json # Build config (excludes tests)
├── tsconfig.test.json # Relaxed options for test files
├── eslint.config.mjs # typescript-eslint, type-aware rules
├── vitest.config.mjs # Test and coverage configuration
├── manifest.json # DXT package manifest
├── package.json # Dependencies and scripts
Expand All @@ -694,8 +719,6 @@ mcp-ssh/
├── .gitattributes # Forces LF checkout on every platform
├── start.sh # Development startup script
├── start-silent.sh # Silent startup script
├── bin/
│ └── mcp-ssh.js # Executable entry point (imports and calls main())
├── scripts/
│ └── build-dxt.sh # DXT package build script
└── doc/ # Documentation assets
Expand Down
24 changes: 16 additions & 8 deletions bin/mcp-ssh.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,26 @@
#!/usr/bin/env node

// Simple wrapper to run the main server.mjs file.
// Wrapper that runs the compiled server (dist/server.js, built from src/ by tsc).
// We import main() explicitly and call it here instead of relying on a
// "is this module run directly?" check inside server.mjs. The latter is
// "is this module run directly?" check inside the server module. The latter is
// brittle on Windows because process.argv[1] uses backslashes while the
// check used forward-slash suffixes (fixes #8).
import path from 'path';
import { fileURLToPath, pathToFileURL } from 'url';
import path from 'node:path';
import { fileURLToPath, pathToFileURL } from 'node:url';
import { existsSync } from 'node:fs';

const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);
const __dirname = path.dirname(fileURLToPath(import.meta.url));
const serverPath = path.join(__dirname, '..', 'dist', 'server.js');

const serverUrl = pathToFileURL(path.join(__dirname, '..', 'server.mjs')).href;
const { main } = await import(serverUrl);
if (!existsSync(serverPath)) {
console.error(
`mcp-ssh: ${serverPath} is missing. Run "npm run build" first ` +
`(this happens automatically on npm install from a git checkout).`
);
process.exit(1);
}

const { main } = await import(pathToFileURL(serverPath).href);

main().catch((error) => {
console.error(`Unhandled error: ${error?.message ?? error}`);
Expand Down
Loading
Loading