diff --git a/.coderabbit.yaml b/.coderabbit.yaml new file mode 100644 index 0000000..fced5e8 --- /dev/null +++ b/.coderabbit.yaml @@ -0,0 +1,10 @@ +# yaml-language-server: $schema=https://storage.googleapis.com/coderabbit_public_assets/schema.v2.json +language: "zh" +tone_instructions: "一律使用繁體中文(台灣正體用語)撰寫審查意見與回覆。" +reviews: + auto_review: + base_branches: + - "^main$" + # release-please 自動開的 release PR(標題固定 "chore(main): release X.Y.Z")純版號/CHANGELOG,跳過審查 + ignore_title_keywords: + - "chore(main): release" diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml deleted file mode 100644 index caf631a..0000000 --- a/.github/workflows/claude-code-review.yml +++ /dev/null @@ -1,200 +0,0 @@ -name: Claude Code Review - -on: - pull_request: - types: [opened, synchronize, ready_for_review, reopened] - workflow_dispatch: - inputs: - profile: - description: Review profile (chill = HIGH/CRITICAL only; assertive = also MEDIUM/LOW/INFO) - default: chill - type: choice - options: [chill, assertive] - pr_number: - description: PR number to review (required when triggered manually) - type: number - required: true - -jobs: - claude-review: - runs-on: ubuntu-latest - permissions: - contents: read - pull-requests: write - id-token: write - - steps: - - name: Checkout repository - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - - name: Determine review profile - id: profile - env: - GH_TOKEN: ${{ github.token }} - run: | - if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then - echo "profile=${{ inputs.profile }}" >> "$GITHUB_OUTPUT" - exit 0 - fi - if gh pr view ${{ github.event.pull_request.number }} \ - --json labels -q '.labels[].name' \ - | grep -qx "review:assertive"; then - echo "profile=assertive" >> "$GITHUB_OUTPUT" - else - echo "profile=chill" >> "$GITHUB_OUTPUT" - fi - - - name: Resolve PR context - id: pr_ctx - env: - GH_TOKEN: ${{ github.token }} - run: | - PR="${{ inputs.pr_number || github.event.pull_request.number }}" - SHA=$(gh pr view "$PR" --repo "${{ github.repository }}" --json headRefOid -q '.headRefOid') - echo "pr_number=$PR" >> "$GITHUB_OUTPUT" - echo "head_sha=$SHA" >> "$GITHUB_OUTPUT" - - - name: Run Claude Code Review - uses: anthropics/claude-code-action@v1 - with: - claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} - claude_args: '--allowedTools "Bash(gh:*),Write"' - prompt: | - REVIEW_PROFILE: ${{ steps.profile.outputs.profile }} - - You are a code reviewer for PR #${{ steps.pr_ctx.outputs.pr_number }}. - Goal: catch real defects this PR introduces. Match findings to severity - honestly. Do NOT generate volume to look thorough. - - ## Operating Principles - - 1. **Diff-bounded scope** — review only lines this PR adds/modifies. Do - not flag pre-existing content unless this PR worsens it. - 2. **Evidence required** — every finding cites file:line + concrete - trigger (input/scenario that breaks). No "Consider..." vagueness. - 3. **Severity honesty** — when between two levels, pick the lower. - 4. **Actionability cap** — fix size ≤ this PR's diff size, otherwise - mark as INFO or open follow-up issue, do NOT block. - 5. **No architectural redesign suggestions** — surface as INFO if at all. - 6. **Profile-aware**: - - chill: report only ⚠️ Potential Issue at HIGH/CRITICAL. - Drop everything else (or surface as INFO if cross-cutting). - - assertive: also report 🛠️ Refactor (MEDIUM) and 🧹 Nitpick (LOW/INFO). - - ## Phase 1 — FETCH - - ``` - gh pr view ${{ steps.pr_ctx.outputs.pr_number }} --json number,title,body,author,baseRefName,headRefName,changedFiles,additions,deletions,labels - gh pr diff ${{ steps.pr_ctx.outputs.pr_number }} --name-only - gh pr diff ${{ steps.pr_ctx.outputs.pr_number }} - ``` - - Note total_diff_size = additions + deletions. - - ## Phase 2 — FILTER & CONTEXT - - ### Path filter (drop findings on these files entirely) - - Skip any file matching: - - Build/deps: `**/dist/**`, `**/build/**`, `**/node_modules/**`, `**/coverage/**` - - Lock files: `**/*.lock`, `**/package-lock.json`, `**/bun.lockb` - - Generated: `**/generated/**`, `**/*.generated.*`, `**/*.gen.*`, `**/*.pb.ts` - - Binary/media: `**/*.{png,jpg,jpeg,gif,svg,ico,webp,pdf,zip,tar,gz}` - - Snapshots: `**/__snapshots__/**`, `**/*.snap` - - ### CLAUDE.md as primary rulebook - - ``` - gh api "repos/${{ github.repository }}/contents/CLAUDE.md?ref=${{ steps.pr_ctx.outputs.head_sha }}" --jq '.content' | base64 -d - ``` - - Extract ❌ / 禁止 / MUST / NEVER rules. Any violation is HIGH minimum. - - ### File-type → applicable categories - - | File type | Categories that apply | - |-----------|----------------------| - | Code (`.ts`, `.tsx`, `.py`, `.go`, ...) | Correctness, Type Safety, Security, Performance, Completeness, Pattern Compliance, Maintainability | - | Docs (`.md`) | Factual accuracy, Internal consistency, Pattern Compliance | - | CI/config (`.yml`, `.yaml`) | Correctness, Security, Pattern Compliance | - | Data (`.json`) | Schema correctness, Pattern Compliance | - - For each non-filtered file, fetch full content at PR head for context: - - ``` - gh api "repos/${{ github.repository }}/contents/{file}?ref=${{ steps.pr_ctx.outputs.head_sha }}" --jq '.content' | base64 -d - ``` - - ## Phase 3 — INTERNAL TRIAGE - - **Step A** — Generate candidate findings internally. - - **Step B** — Filter pipeline. Each candidate must pass ALL: - 1. On lines this PR added/modified? (else drop) - 2. Concrete trigger describable? (else drop — speculation) - 3. Fix size ≤ total_diff_size? (else demote to INFO) - 4. File-type ↔ category applicable? (else drop) - - **Step C** — Type × Severity matrix. - - Types: ⚠️ Potential Issue | 🛠️ Refactor Suggestion | 🧹 Nitpick - - Severity: - - 🔴 CRITICAL — security vulnerability, data loss, crash on common input. Blocks merge. - - 🟠 HIGH — logic bug, CLAUDE.md ❌ violation, silently wrong output. Blocks merge. - - 🟡 MEDIUM — quality issue, rule contradiction. Does NOT block; follow-up acceptable. - - 🔵 LOW — style/wording polish. Never blocks. - - ⚪ INFO — pure FYI, no action expected. - - **Step D** — Profile gate: - - chill: keep only ⚠️ HIGH/CRITICAL. Drop rest (or single ⚪ INFO if cross-cutting). - - assertive: keep all types up to LOW; INFO for cross-cutting only. - - **Step E** — Cap by PR size: - - total_diff_size < 100: max 5 findings - - 100–500: max 7 findings - - > 500: max 10 findings - - ## Phase 4 — WRITE REVIEW - - Write to "review.md" in Traditional Chinese: - - ## Code Review - - ### 變更摘要 - 1–3 bullets capturing intent. - - ### 優點 (略過此節 if nothing concrete to praise) - - ### 問題與建議 - - For each finding: - ``` - [TYPE icon] [SEVERITY] file:line — Issue - Trigger: - Fix: - ``` - - If no findings: `無 — 此 PR 通過 Phase 3 全部過濾。` - - ### 結論 - - - Any 🔴 CRITICAL or 🟠 HIGH → `**需修改**` - - Otherwise → `**可合併**(含 N 條 🟡 MEDIUM / M 條 🔵 LOW / K 條 ⚪ INFO)` - - ## Phase 5 — Self-check (do NOT include in review.md) - - - [ ] Every finding cites file:line + concrete trigger - - [ ] Every finding is on lines this PR changed - - [ ] No findings on path-filtered files - - [ ] Profile gate respected - - [ ] Findings count ≤ cap - - [ ] Conclusion follows mechanical rule - - ## Phase 6 — POST - - ``` - gh pr review ${{ steps.pr_ctx.outputs.pr_number }} --comment --body-file review.md - ``` diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml deleted file mode 100644 index 5c980e1..0000000 --- a/.github/workflows/claude.yml +++ /dev/null @@ -1,39 +0,0 @@ -name: Claude Code - -on: - issue_comment: - types: [created] - pull_request_review_comment: - types: [created] - issues: - types: [opened, assigned] - pull_request_review: - types: [submitted] - -jobs: - claude: - if: | - (github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude')) || - (github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@claude')) || - (github.event_name == 'pull_request_review' && contains(github.event.review.body, '@claude')) || - (github.event_name == 'issues' && (contains(github.event.issue.body, '@claude') || contains(github.event.issue.title, '@claude'))) - runs-on: ubuntu-latest - permissions: - contents: read - pull-requests: write - issues: write - id-token: write - actions: read - steps: - - name: Checkout repository - uses: actions/checkout@v4 - with: - fetch-depth: 1 - - - name: Run Claude Code - uses: anthropics/claude-code-action@v1 - with: - claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} - system_prompt: "請使用繁體中文回覆所有問題與建議。" - additional_permissions: | - actions: read diff --git a/README.md b/README.md index 5cf5932..8237a77 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,8 @@ Storage Box tools call `api.hetzner.com/v1` (unified API), while all other tools ## Available Tools (40 total) +⚠️ marks destructive or hard-to-reverse operations. + ### Servers (7) | Tool | Description | ⚠️ | diff --git a/docs/index.html b/docs/index.html index a46a1fb..f5de597 100644 --- a/docs/index.html +++ b/docs/index.html @@ -229,6 +229,7 @@

Hetzner MCP Server

Tools

+

⚠ marks destructive or hard-to-reverse operations.

@@ -373,6 +374,7 @@

Quick Setup

"stat-endpoints": "API Endpoints", "stat-transport": "Transport", "section-tools": "Tools", + "tools-note": "⚠ marks destructive or hard-to-reverse operations.", "cat-servers": "Servers", "cat-servers-desc": "Create, power on/off/reboot, and delete cloud servers.", "cat-ssh": "SSH Keys", @@ -401,6 +403,7 @@

Quick Setup

"stat-endpoints": "API 端點", "stat-transport": "傳輸協定", "section-tools": "工具", + "tools-note": "⚠ 標示破壞性或難以復原的操作。", "cat-servers": "伺服器", "cat-servers-desc": "建立、開關機、重新開機、刪除雲端伺服器。", "cat-ssh": "SSH 金鑰", @@ -429,6 +432,7 @@

Quick Setup

"stat-endpoints": "API エンドポイント", "stat-transport": "トランスポート", "section-tools": "ツール", + "tools-note": "⚠ は破壊的または元に戻しにくい操作を示します。", "cat-servers": "サーバー", "cat-servers-desc": "クラウドサーバーの作成、電源操作、再起動、削除。", "cat-ssh": "SSH キー", @@ -457,6 +461,7 @@

Quick Setup

"stat-endpoints": "API 엔드포인트", "stat-transport": "전송 방식", "section-tools": "도구", + "tools-note": "⚠ 는 파괴적이거나 되돌리기 어려운 작업을 나타냅니다.", "cat-servers": "서버", "cat-servers-desc": "클라우드 서버 생성, 전원 켜기/끄기, 재시작, 삭제.", "cat-ssh": "SSH 키", diff --git a/src/api.ts b/src/api.ts index 31b7254..5a99dce 100644 --- a/src/api.ts +++ b/src/api.ts @@ -283,9 +283,11 @@ export function createPaginatedFetch(requestFn: PaginatedRequestFn) { }; } -// I-5: Test-only reset hook for clearing cached clients between tests. -// Throws unless NODE_ENV === "test" — catches both explicit "production" and the -// common MCP production case where NODE_ENV is simply not set. +/** + * @internal + * Test-only reset hook for clearing cached singleton clients between test runs. + * Throws in any non-test environment — do NOT call from production code. + */ export function __resetClientsForTesting(): void { if (process.env.NODE_ENV !== "test") { throw new Error("__resetClientsForTesting must not be called in production"); diff --git a/src/index.ts b/src/index.ts index d5f1b45..6116a0b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -16,6 +16,7 @@ import { registerStorageBoxTools } from "./tools/storage-boxes.js"; import { registerVolumeTools } from "./tools/volumes.js"; import { registerMetricsTools } from "./tools/metrics.js"; import { registerServerSshTools } from "./tools/server-ssh.js"; +import { formatStartupError } from "./utils.js"; // Create MCP server instance const server = new McpServer({ @@ -54,7 +55,7 @@ async function main(): Promise { console.error("Hetzner MCP server running via stdio"); } -main().catch((error) => { - console.error("Server error:", error); +main().catch((error: unknown) => { + console.error("Server error:", formatStartupError(error)); process.exit(1); }); diff --git a/src/tools/server-ssh.ts b/src/tools/server-ssh.ts index c881875..5320a1e 100644 --- a/src/tools/server-ssh.ts +++ b/src/tools/server-ssh.ts @@ -4,6 +4,52 @@ import { z } from "zod"; import { makeApiRequest, handleApiError } from "../api.js"; import { ResponseFormat, ResponseFormatSchema, GetServerResponseSchema } from "../types.js"; +/** + * Resolves SHA256 fingerprints of all host SSH key types via ssh-keyscan + ssh-keygen. + * Returns an array because a host advertises multiple key types (RSA, ECDSA, ed25519). + * Exported so tests can inject a mock via the keyScanRunner DI parameter. + */ +export function runSshKeyScan(host: string, port: number): Promise { + return new Promise((resolve, reject) => { + // Step 1: fetch raw host key entries (all key types) + execFile( + "ssh-keyscan", + ["-p", String(port), "-T", "10", host], + { timeout: 15_000 }, + (scanErr, scanOut, scanStderr) => { + const rawKey = scanOut.trim(); + if (!rawKey) { + reject(new Error(`ssh-keyscan failed: ${scanStderr.trim() || "no output"}`)); + return; + } + // Reject if ssh-keyscan exited non-zero even with partial stdout — data may be corrupt. + if (scanErr) { + reject(scanErr); + return; + } + // Step 2: compute all fingerprints from the raw keys via ssh-keygen -l + const proc = execFile( + "ssh-keygen", + ["-l", "-E", "sha256", "-f", "/dev/stdin"], + { timeout: 10_000 }, + (keygenErr, keygenOut) => { + if (keygenErr) { reject(keygenErr); return; } + // Extract every SHA256:... token; include trailing '=' (base64 padding). + const matches = [...keygenOut.matchAll(/SHA256:[A-Za-z0-9+/]+=*/g)].map(m => m[0]); + if (matches.length === 0) { + reject(new Error(`Could not parse fingerprint from: ${keygenOut.trim()}`)); + return; + } + resolve(matches); + } + ); + proc.stdin?.write(rawKey + "\n"); + proc.stdin?.end(); + } + ); + }); +} + export interface RamStats { total: number; used: number; @@ -104,11 +150,11 @@ export function runSsh( }); } -// The sshRunner parameter lets tests inject a mock without fighting ESM binding. -// Production callers omit it — the real runSsh is used by default. +// sshRunner / keyScanRunner let tests inject mocks without fighting ESM binding. export function registerServerSshTools( server: McpServer, - sshRunner: typeof runSsh = runSsh + sshRunner: typeof runSsh = runSsh, + keyScanRunner: typeof runSshKeyScan = runSshKeyScan ): void { server.registerTool( "hetzner_get_server_ram", @@ -124,11 +170,14 @@ Prerequisites: - The SSH private key must be available in the system SSH agent or ~/.ssh (the tool calls the system \`ssh\` binary directly). -⚠️ Host key trust: uses StrictHostKeyChecking=accept-new. For hosts not yet -in ~/.ssh/known_hosts, the host key is automatically trusted and recorded. For -hosts already in known_hosts, a key mismatch is still rejected with an error. -For higher security, pre-register expected host fingerprints in known_hosts -and set StrictHostKeyChecking=yes in your SSH config. +⚠️ TOFU risk: uses StrictHostKeyChecking=accept-new. On the first connection to +a host, any key is automatically trusted (Trust-On-First-Use). An active MITM +attack on the first connection would go undetected. To prevent this, supply the +expected_fingerprint parameter (SHA256 format, e.g. "SHA256:abc123…"). When +provided, the host key fingerprint is verified via ssh-keyscan before connecting +and the connection is aborted if it does not match. For the highest security, +pre-register fingerprints in known_hosts and set StrictHostKeyChecking=yes in +your SSH config. Returns used / total / available in MiB and overall usage %, plus swap state.`, inputSchema: z.object({ @@ -140,6 +189,10 @@ Returns used / total / available in MiB and overall usage %, plus swap state.`, .describe("SSH username (default: 'root')"), ssh_port: z.number().int().positive().max(65535).default(22) .describe("SSH port (default: 22)"), + expected_fingerprint: z.string() + .regex(/^SHA256:[A-Za-z0-9+/]+=*$/, "expected_fingerprint must be in SHA256:base64 format") + .optional() + .describe("Expected SSH host key fingerprint (e.g. 'SHA256:abc123…'). When provided, the host key is verified via ssh-keyscan before connecting. Strongly recommended to prevent TOFU MITM attacks."), response_format: ResponseFormatSchema.describe("Output format: 'markdown' or 'json'") }).strict(), annotations: { @@ -175,7 +228,26 @@ Returns used / total / available in MiB and overall usage %, plus swap state.`, }; } - // Step 2: SSH and run free -m + // Step 2: verify host fingerprint if caller supplied one + if (params.expected_fingerprint) { + let actualFps: string[]; + try { + actualFps = await keyScanRunner(ipv4, sshPort); + } catch (scanErr) { + return { + content: [{ type: "text", text: `Error: fingerprint verification failed: ${scanErr instanceof Error ? scanErr.message : String(scanErr)}` }], + isError: true + }; + } + if (!actualFps.includes(params.expected_fingerprint)) { + return { + content: [{ type: "text", text: `Error: fingerprint mismatch for ${ipv4}. Expected: ${params.expected_fingerprint} — Got: ${actualFps.join(", ")}` }], + isError: true + }; + } + } + + // Step 3: SSH and run free -m const stdout = await sshRunner(ipv4, sshPort, sshUser, "free -m"); const { ram, swap } = parseFreeOutput(stdout); @@ -214,6 +286,7 @@ Returns used / total / available in MiB and overall usage %, plus swap state.`, } lines.push(""); + // sshUser matches /^[a-zA-Z0-9._-]+$/, ipv4 matches IPv4 regex, sshPort is a validated integer — interpolation is safe. lines.push(`*Source: \`free -m\` via ${sshUser}@${ipv4}:${sshPort}*`); return { diff --git a/src/tools/servers.ts b/src/tools/servers.ts index 38af81e..9f444c7 100644 --- a/src/tools/servers.ts +++ b/src/tools/servers.ts @@ -16,6 +16,7 @@ import { ServerActionResponseSchema, HetznerServer } from "../types.js"; +import { escapeHtml } from "../utils.js"; const ResponseFormatSchema = z.nativeEnum(ResponseFormat).default(ResponseFormat.MARKDOWN); const CLOUD_DEFAULT_PER_PAGE = 25; @@ -23,15 +24,6 @@ const TRUNCATION_NOTE = `> ⚠️ Truncated at ${PAGINATION_HARD_CAP_PAGES} page const paginatedFetch = createPaginatedFetch(makeApiRequest); -function escapeHtml(s: string): string { - return s - .replace(/&/g, "&") - .replace(//g, ">") - .replace(/"/g, """) - .replace(/'/g, "'"); -} - function formatServer(server: HetznerServer): string { const ipv4 = server.public_net.ipv4?.ip || "N/A"; const ipv6 = server.public_net.ipv6?.ip || "N/A"; @@ -42,7 +34,7 @@ function formatServer(server: HetznerServer): string { `- **IPv4**: ${ipv4}`, `- **IPv6**: ${ipv6}`, `- **Type**: ${server.server_type.name} (${server.server_type.cores} cores, ${server.server_type.memory}GB RAM, ${server.server_type.disk}GB disk)`, - `- **Location**: ${escapeHtml(server.datacenter.location.city)}, ${escapeHtml(server.datacenter.location.country)} (${escapeHtml(server.datacenter.name)})` + `- **Location**: ${escapeHtml(server.location.city)}, ${escapeHtml(server.location.country)} (${escapeHtml(server.location.name)})` ]; if (server.image) { @@ -79,7 +71,7 @@ Returns servers with their: inputSchema: z.object({ page: z.number().int().positive().optional().describe("Page number (1-based). When set, fetches a single page only."), per_page: z.number().int().positive().max(50).optional().describe("Items per page (max 50). Default 25."), - label_selector: z.string().optional() + label_selector: z.string().max(256).optional() .describe("Filter by label (e.g., 'env=production')"), response_format: ResponseFormatSchema.describe("Output format: 'markdown' or 'json'") }).strict(), @@ -215,7 +207,8 @@ Optional parameters: - ssh_keys: List of SSH key names or IDs for server access - labels: Key-value labels for organization -Returns the new server details including IP address and root password (if no SSH keys specified).`, +Returns the new server details including IP address and root password (if no SSH keys specified). +When using JSON output format, the response includes root_password in plaintext — avoid logging the full JSON output to unprotected storage.`, inputSchema: z.object({ name: z.string().min(1).max(255) .regex(/^[a-zA-Z0-9-]+$/, "Name can only contain letters, digits, and hyphens") diff --git a/src/tools/ssh-keys.ts b/src/tools/ssh-keys.ts index 9eb612c..8b0504a 100644 --- a/src/tools/ssh-keys.ts +++ b/src/tools/ssh-keys.ts @@ -15,6 +15,7 @@ import { CreateSSHKeyResponseSchema, HetznerSSHKey } from "../types.js"; +import { escapeHtml } from "../utils.js"; const ResponseFormatSchema = z.nativeEnum(ResponseFormat).default(ResponseFormat.MARKDOWN); const CLOUD_DEFAULT_PER_PAGE = 25; @@ -22,15 +23,6 @@ const TRUNCATION_NOTE = `> ⚠️ Truncated at ${PAGINATION_HARD_CAP_PAGES} page const paginatedFetch = createPaginatedFetch(makeApiRequest); -function escapeHtml(s: string): string { - return s - .replace(/&/g, "&") - .replace(//g, ">") - .replace(/"/g, """) - .replace(/'/g, "'"); -} - function formatSSHKey(key: HetznerSSHKey): string { const lines = [ `## ${escapeHtml(key.name)} (ID: ${key.id})`, diff --git a/src/tools/storage-boxes.ts b/src/tools/storage-boxes.ts index 7b51faf..f4e1798 100644 --- a/src/tools/storage-boxes.ts +++ b/src/tools/storage-boxes.ts @@ -30,6 +30,7 @@ import { HetznerAction, BooleanKeys } from "../types.js"; +import { escapeHtml } from "../utils.js"; const ResponseFormatSchema = z.nativeEnum(ResponseFormat).default(ResponseFormat.MARKDOWN); const DEFAULT_PER_PAGE = 50; @@ -48,13 +49,27 @@ const STORAGE_BOX_PROTOCOL_KEYS = [ // fails typecheck instead of silently filtering to false at runtime. const SUBACCOUNT_PROTOCOLS = ["ssh", "webdav", "samba"] as const satisfies readonly BooleanKeys[]; -function escapeHtml(s: string): string { - return s - .replace(/&/g, "&") - .replace(//g, ">") - .replace(/"/g, """) - .replace(/'/g, "'"); +export interface StorageBoxStats { + used_bytes: number; + used_gib: number; + total_bytes: number; + total_gib: number; + available_gib: number; + usage_percent: number; +} + +// Exported for unit testing. +export function computeStorageBoxStats(box: HetznerStorageBox): StorageBoxStats { + const used_bytes = box.stats.size; // size = size_data + size_snapshots (total consumed) + const total_bytes = box.storage_box_type.size; + const GiB = 1024 ** 3; + const used_gib = used_bytes / GiB; + const total_gib = total_bytes / GiB; + const available_gib = total_gib - used_gib; + const usage_percent = total_bytes > 0 + ? Math.round((used_bytes / total_bytes) * 10000) / 100 + : 0; + return { used_bytes, used_gib, total_bytes, total_gib, available_gib, usage_percent }; } // Exported for unit testing. @@ -175,8 +190,8 @@ Returns Storage Boxes with their: inputSchema: z.object({ page: z.number().int().positive().optional().describe("Page number (1-based). When set, fetches a single page only."), per_page: z.number().int().positive().max(50).optional().describe("Items per page (max 50). Default 50."), - label_selector: z.string().optional().describe("Filter by label selector (e.g. 'env=prod')"), - name: z.string().optional().describe("Filter by exact name"), + label_selector: z.string().max(256).optional().describe("Filter by label selector (e.g. 'env=prod')"), + name: z.string().max(255).optional().describe("Filter by exact name"), response_format: ResponseFormatSchema.describe("Output format: 'markdown' or 'json'") }).strict(), annotations: { @@ -313,7 +328,7 @@ Returns subaccounts with their: id: z.number().int().positive().describe("The Storage Box ID"), page: z.number().int().positive().optional().describe("Page number (1-based). When set, fetches a single page only."), per_page: z.number().int().positive().max(50).optional().describe("Items per page (max 50). Default 50."), - username: z.string().optional().describe("Filter by exact subaccount username"), + username: z.string().max(255).regex(/^[a-zA-Z0-9._-]+$/, "username must contain only alphanumeric characters, dots, hyphens, or underscores").optional().describe("Filter by exact subaccount username"), response_format: ResponseFormatSchema.describe("Output format: 'markdown' or 'json'") }).strict(), annotations: { @@ -567,6 +582,8 @@ Required parameters: - name: Name for the storage box. - password: Initial password (min 12 chars, must include uppercase, lowercase, number, and special character). +⚠️ Security: the password parameter is transmitted as plaintext in the MCP protocol. Ensure MCP session logs are access-controlled and do not persist to unprotected storage. + Returns the new Storage Box and an action tracking provisioning.`, inputSchema: z.object({ storage_box_type: z.string().min(1).regex(/^[a-z0-9-]+$/, "storage_box_type must be a valid slug (lowercase alphanumeric and hyphens)").describe("Storage box type name (e.g., 'bx11', 'bx20')"), @@ -752,7 +769,7 @@ This action cannot be undone.`, const lines = [ `# Folders in Storage Box ${params.id}`, "", - ...data.folders.map((f) => `- \`${f}\``) + ...data.folders.map((f) => `- \`${escapeHtml(f)}\``) ]; return { content: [{ type: "text", text: lines.join("\n") }] }; } catch (error) { @@ -1062,7 +1079,9 @@ When delete protection is enabled, the Storage Box cannot be deleted until prote title: "Reset Storage Box Password", description: `Reset the password for a Storage Box. -Password policy: minimum 12 characters, must include uppercase, lowercase, number, and special character.`, +Password policy: minimum 12 characters, must include uppercase, lowercase, number, and special character. + +⚠️ Security: the password parameter is transmitted as plaintext in the MCP protocol. Ensure MCP session logs are access-controlled and do not persist to unprotected storage.`, inputSchema: z.object({ id: z.number().int().positive().describe("The Storage Box ID"), password: z @@ -1235,6 +1254,112 @@ Schedule options: } ); + // Get Storage Box Stats + server.registerTool( + "hetzner_get_storage_box_stats", + { + title: "Get Storage Box Stats", + description: `Get storage usage statistics for a specific Storage Box. + +Returns: +- used_bytes / used_gib — current data usage +- total_bytes / total_gib — plan capacity +- available_gib — remaining free space +- usage_percent — utilisation as a percentage (2 decimal places) + +Useful for dashboards, cron jobs, and pre-flight capacity checks before backup operations.`, + inputSchema: z.object({ + id: z.number().int().positive().describe("The Storage Box ID"), + response_format: ResponseFormatSchema.describe("Output format: 'markdown' or 'json'") + }).strict(), + annotations: { + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: true + } + }, + async (params) => { + try { + const data = await makeStorageBoxApiRequest(`/storage_boxes/${params.id}`, GetStorageBoxResponseSchema); + const stats = computeStorageBoxStats(data.storage_box); + + if (params.response_format === ResponseFormat.JSON) { + return { + content: [{ type: "text", text: JSON.stringify(stats, null, 2) }] + }; + } + + const lines = [ + `# Storage Box ${params.id} — Usage Stats`, + "", + `- **Used**: ${formatBytes(stats.used_bytes)} (${stats.used_gib.toFixed(2)} GiB)`, + `- **Total**: ${formatBytes(stats.total_bytes)} (${stats.total_gib.toFixed(2)} GiB)`, + `- **Available**: ${stats.available_gib.toFixed(2)} GiB`, + `- **Usage**: ${stats.usage_percent.toFixed(2)}%` + ]; + return { + content: [{ type: "text", text: lines.join("\n") }] + }; + } catch (error) { + return { + content: [{ type: "text", text: handleApiError(error) }], + isError: true + }; + } + } + ); + + // Assert Storage Box Space + server.registerTool( + "hetzner_assert_storage_box_space", + { + title: "Assert Storage Box Space", + description: `Pre-flight space check: assert that a Storage Box has at least \`required_gib\` GiB of available space. + +Returns success if space is sufficient, or an error (isError: true) if space is insufficient. +Designed for use in cron jobs and backup pipelines before executing storage-intensive operations.`, + inputSchema: z.object({ + id: z.number().int().positive().describe("The Storage Box ID"), + required_gib: z.number().positive().describe("Minimum required free space in GiB") + }).strict(), + annotations: { + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: true + } + }, + async (params) => { + try { + const data = await makeStorageBoxApiRequest(`/storage_boxes/${params.id}`, GetStorageBoxResponseSchema); + const stats = computeStorageBoxStats(data.storage_box); + + if (stats.available_gib >= params.required_gib) { + return { + content: [{ + type: "text", + text: `✓ Storage Box ${params.id} has sufficient space: ${stats.available_gib.toFixed(2)} GiB available (required: ${params.required_gib} GiB, usage: ${stats.usage_percent.toFixed(2)}%).` + }] + }; + } + + return { + content: [{ + type: "text", + text: `✗ Storage Box ${params.id} has insufficient space: ${stats.available_gib.toFixed(2)} GiB available but ${params.required_gib} GiB required (usage: ${stats.usage_percent.toFixed(2)}%, total: ${stats.total_gib.toFixed(2)} GiB).` + }], + isError: true + }; + } catch (error) { + return { + content: [{ type: "text", text: handleApiError(error) }], + isError: true + }; + } + } + ); + // Rollback Storage Box Snapshot server.registerTool( "hetzner_rollback_storage_box_snapshot", diff --git a/src/tools/volumes.ts b/src/tools/volumes.ts index 9663e32..5595ad3 100644 --- a/src/tools/volumes.ts +++ b/src/tools/volumes.ts @@ -16,6 +16,7 @@ import { VolumeActionResponseSchema, HetznerVolume } from "../types.js"; +import { escapeHtml } from "../utils.js"; const CLOUD_DEFAULT_PER_PAGE = 25; const TRUNCATION_NOTE = `> ⚠️ Truncated at ${PAGINATION_HARD_CAP_PAGES} pages — supply explicit \`page\` to fetch more.`; @@ -23,19 +24,19 @@ const paginatedFetch = createPaginatedFetch(makeApiRequest); function formatVolume(vol: HetznerVolume): string { const lines = [ - `## ${vol.name} (ID: ${vol.id})`, - `- **Status**: ${vol.status}`, + `## ${escapeHtml(vol.name)} (ID: ${vol.id})`, + `- **Status**: ${escapeHtml(vol.status)}`, `- **Size**: ${vol.size} GB`, - `- **Location**: ${vol.location.city}, ${vol.location.country} (${vol.location.name})`, - `- **Mount path**: ${vol.linux_device ?? "N/A"}`, + `- **Location**: ${escapeHtml(vol.location.city)}, ${escapeHtml(vol.location.country)} (${escapeHtml(vol.location.name)})`, + `- **Mount path**: ${vol.linux_device ? escapeHtml(vol.linux_device) : "N/A"}`, `- **Attached server**: ${vol.server !== null ? `ID ${vol.server}` : "not attached"}`, - `- **Format**: ${vol.format ?? "unknown"}`, + `- **Format**: ${vol.format ? escapeHtml(vol.format) : "unknown"}`, `- **Delete protected**: ${vol.protection.delete ? "yes" : "no"}`, `- **Created**: ${new Date(vol.created).toLocaleString()}` ]; if (Object.keys(vol.labels).length > 0) { - lines.push(`- **Labels**: ${Object.entries(vol.labels).map(([k, v]) => `${k}=${v}`).join(", ")}`); + lines.push(`- **Labels**: ${Object.entries(vol.labels).map(([k, v]) => `${escapeHtml(k)}=${escapeHtml(v)}`).join(", ")}`); } return lines.join("\n"); @@ -62,8 +63,8 @@ Returns volumes with their: inputSchema: z.object({ page: z.number().int().positive().optional().describe("Page number (1-based). When set, fetches a single page only."), per_page: z.number().int().positive().max(50).optional().describe("Items per page (max 50). Default 25."), - label_selector: z.string().optional().describe("Filter by label (e.g., 'env=production')"), - status: z.string().optional().describe("Filter by volume status (known values: 'available', 'creating')"), + label_selector: z.string().max(256).optional().describe("Filter by label (e.g., 'env=production')"), + status: z.string().max(64).optional().describe("Filter by volume status (known values: 'available', 'creating')"), response_format: ResponseFormatSchema.describe("Output format: 'markdown' or 'json'") }).strict(), annotations: { diff --git a/src/types.ts b/src/types.ts index 7d64074..23be40a 100644 --- a/src/types.ts +++ b/src/types.ts @@ -68,16 +68,18 @@ export const HetznerServerSchema = z.object({ memory: z.number(), disk: z.number() }), - datacenter: z.object({ - id: z.number(), + // Hetzner removed the `datacenter` property from the Servers API on 2026-06-30 + // (announced 2025-12-16, "Phasing out Datacenters in favor of Locations"). + // https://docs.hetzner.cloud/changelog#2025-12-16-phasing-out-datacenters + // + // Only the fields formatServer() actually renders are declared. z.object already + // strips unknown keys, so listing fewer fields is strictly more tolerant of the + // next upstream change — a required field we never read is a crash waiting to + // happen (that is exactly how the datacenter removal broke every server tool). + location: z.object({ name: z.string(), - description: z.string(), - location: z.object({ - id: z.number(), - name: z.string(), - city: z.string(), - country: z.string() - }) + country: z.string(), + city: z.string() }), image: z.object({ id: z.number(), diff --git a/src/utils.ts b/src/utils.ts new file mode 100644 index 0000000..eb08267 --- /dev/null +++ b/src/utils.ts @@ -0,0 +1,17 @@ +/** Extracts a safe, credential-free string from an unknown thrown value. */ +export function formatStartupError(error: unknown): string { + if (error instanceof Error) { + return error.message; + } + return String(error); +} + +/** Escapes HTML special characters to prevent XSS in markdown tool output. */ +export function escapeHtml(s: string): string { + return s + .replace(/&/g, "&") + .replace(//g, ">") + .replace(/"/g, """) + .replace(/'/g, "'"); +} diff --git a/tests/index.test.ts b/tests/index.test.ts new file mode 100644 index 0000000..81c6d73 --- /dev/null +++ b/tests/index.test.ts @@ -0,0 +1,35 @@ +import { describe, it, expect } from "vitest"; +import { formatStartupError } from "../src/utils.js"; +import { AxiosError, AxiosHeaders } from "axios"; + +describe("formatStartupError", () => { + it("returns message string for Error instances", () => { + const err = new Error("something went wrong"); + expect(formatStartupError(err)).toBe("something went wrong"); + }); + + it("converts non-Error to string", () => { + expect(formatStartupError("plain string error")).toBe("plain string error"); + expect(formatStartupError(42)).toBe("42"); + }); + + it("does not expose Authorization header from AxiosError", () => { + const headers = new AxiosHeaders({ Authorization: "Bearer secret-token-xyz" }); + const axiosErr = new AxiosError( + "Request failed", + "ERR_BAD_RESPONSE", + { headers, url: "https://api.hetzner.cloud/v1/servers" } as never, + null, + undefined + ); + const result = formatStartupError(axiosErr); + expect(result).not.toContain("secret-token-xyz"); + expect(result).not.toContain("Authorization"); + expect(typeof result).toBe("string"); + }); + + it("handles null / undefined gracefully", () => { + expect(formatStartupError(null)).toBe("null"); + expect(formatStartupError(undefined)).toBe("undefined"); + }); +}); diff --git a/tests/tools/metrics.test.ts b/tests/tools/metrics.test.ts index d087126..9b295ba 100644 --- a/tests/tools/metrics.test.ts +++ b/tests/tools/metrics.test.ts @@ -60,12 +60,7 @@ const serverResponse = { status: "running", public_net: { ipv4: { ip: "91.99.173.93" }, ipv6: { ip: "2a01:4f8::1" } }, server_type: { id: 22, name: "cx53", description: "CX53", cores: 16, memory: 32, disk: 320 }, - datacenter: { - id: 2, - name: "nbg1-dc3", - description: "Nuremberg DC Park 1", - location: { id: 2, name: "nbg1", city: "Nuremberg", country: "DE" } - }, + location: { id: 2, name: "nbg1", description: "Nuremberg DC Park 1", country: "DE", city: "Nuremberg" }, image: { id: 1, name: "ubuntu-22.04", description: "Ubuntu 22.04", os_flavor: "ubuntu", os_version: "22.04" }, labels: {}, created: "2024-01-01T00:00:00+00:00" diff --git a/tests/tools/server-ssh.test.ts b/tests/tools/server-ssh.test.ts index 1728b80..d3cd812 100644 --- a/tests/tools/server-ssh.test.ts +++ b/tests/tools/server-ssh.test.ts @@ -5,17 +5,25 @@ vi.mock("../../src/api.js", async (importOriginal) => { return { ...actual, makeApiRequest: vi.fn() }; }); -import { parseFreeOutput, registerServerSshTools, runSsh } from "../../src/tools/server-ssh.js"; +vi.mock("child_process"); + +import { parseFreeOutput, registerServerSshTools, runSsh, runSshKeyScan } from "../../src/tools/server-ssh.js"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { makeApiRequest } from "../../src/api.js"; +import { execFile } from "child_process"; + +const mockExecFile = vi.mocked(execFile); const mockedRequest = vi.mocked(makeApiRequest); // Injected via dependency injection — no module mocking required. const mockSsh = vi.fn(); +const mockKeyScan = vi.fn(); beforeEach(() => { mockedRequest.mockReset(); mockSsh.mockReset(); + mockKeyScan.mockReset(); + mockExecFile.mockReset(); }); // ── Fixtures ────────────────────────────────────────────────────────────────── @@ -47,12 +55,7 @@ const serverResponse = { ipv6: { ip: "2a01:4f8::1" } }, server_type: { id: 22, name: "cx53", description: "CX53", cores: 16, memory: 32, disk: 320 }, - datacenter: { - id: 2, - name: "nbg1-dc3", - description: "Nuremberg DC Park 1", - location: { id: 2, name: "nbg1", city: "Nuremberg", country: "DE" } - }, + location: { id: 2, name: "nbg1", description: "Nuremberg DC Park 1", country: "DE", city: "Nuremberg" }, image: { id: 1, name: "ubuntu-22.04", description: "Ubuntu 22.04", os_flavor: "ubuntu", os_version: "22.04" }, labels: {}, created: "2024-01-01T00:00:00+00:00" @@ -61,21 +64,93 @@ const serverResponse = { type ToolHandler = (params: unknown) => Promise<{ content: { type: string; text: string }[]; isError?: boolean }>; -function captureHandler(): ToolHandler { +function captureHandler(keyScanRunner?: typeof runSshKeyScan): ToolHandler { let captured: ToolHandler | undefined; const fakeServer = { registerTool: vi.fn((_name: string, _opts: unknown, handler: ToolHandler) => { captured = handler; }) }; - // Inject mockSsh so the handler never opens a real SSH connection. - registerServerSshTools(fakeServer as unknown as McpServer, mockSsh); + // Inject mockSsh (and optional keyScanRunner) so the handler never touches real SSH. + registerServerSshTools(fakeServer as unknown as McpServer, mockSsh, keyScanRunner ?? mockKeyScan); if (!captured) { throw new Error("registerServerSshTools did not call registerTool — handler not captured"); } return captured; } +function captureToolOpts(): { description: string; inputSchema: { shape: Record } } { + let opts: { description: string; inputSchema: { shape: Record } } | undefined; + const fakeServer = { + registerTool: vi.fn((_name: string, o: typeof opts) => { opts = o; }) + }; + registerServerSshTools(fakeServer as unknown as McpServer, mockSsh, mockKeyScan); + if (!opts) throw new Error("opts not captured"); + return opts; +} + +// ── runSshKeyScan — direct unit tests (execFile mocked) ─────────────────────── + +type ExecCallback = (err: Error | null, stdout: string, stderr: string) => void; +type FakeChildProcess = { stdin: { write: ReturnType; end: ReturnType } | null }; + +function stubExecFileCalls(...calls: Array<{ err: Error | null; stdout: string; stderr: string }>): FakeChildProcess { + const mockStdin = { write: vi.fn(), end: vi.fn() }; + let callIndex = 0; + mockExecFile.mockImplementation((_file, _args, _opts, cb) => { + const call = calls[callIndex++] ?? { err: null, stdout: "", stderr: "" }; + (cb as ExecCallback)(call.err, call.stdout, call.stderr); + return { stdin: mockStdin } as ReturnType; + }); + return { stdin: mockStdin }; +} + +describe("runSshKeyScan — direct unit tests", () => { + it("rejects when ssh-keyscan returns empty stdout", async () => { + stubExecFileCalls({ err: null, stdout: "", stderr: "Connection refused" }); + await expect(runSshKeyScan("1.2.3.4", 22)).rejects.toThrow("ssh-keyscan failed"); + }); + + it("rejects when ssh-keyscan exits non-zero even with partial stdout (Finding #3)", async () => { + const scanError = new Error("ssh-keyscan: connection timeout"); + stubExecFileCalls({ err: scanError, stdout: "partial-key-data\n", stderr: "" }); + await expect(runSshKeyScan("1.2.3.4", 22)).rejects.toThrow("connection timeout"); + }); + + it("returns array of all fingerprints preserving base64 padding (Findings #1 and #2)", async () => { + const keyscanOut = "1.2.3.4 ecdsa-sha2-nistp256 ECDSA\n1.2.3.4 ssh-ed25519 ED25519"; + // Second fingerprint has trailing '=' (base64 padding) + const keygenOut = "256 SHA256:AbCdEf+abc root@host (ECDSA)\n256 SHA256:XyZ123/q8= user@host (ED25519)"; + stubExecFileCalls( + { err: null, stdout: keyscanOut, stderr: "" }, + { err: null, stdout: keygenOut, stderr: "" } + ); + + const result = await runSshKeyScan("1.2.3.4", 22); + expect(Array.isArray(result)).toBe(true); + expect(result).toContain("SHA256:AbCdEf+abc"); + expect(result).toContain("SHA256:XyZ123/q8="); // '=' must be preserved + expect(result).toHaveLength(2); + }); + + it("rejects when ssh-keygen produces no recognisable fingerprint", async () => { + stubExecFileCalls( + { err: null, stdout: "1.2.3.4 ssh-ed25519 KEY", stderr: "" }, + { err: null, stdout: "garbled output without SHA256", stderr: "" } + ); + await expect(runSshKeyScan("1.2.3.4", 22)).rejects.toThrow("Could not parse fingerprint"); + }); + + it("rejects when ssh-keygen exits non-zero", async () => { + const keygenError = new Error("permission denied"); + stubExecFileCalls( + { err: null, stdout: "1.2.3.4 ssh-ed25519 KEY", stderr: "" }, + { err: keygenError, stdout: "", stderr: "" } + ); + await expect(runSshKeyScan("1.2.3.4", 22)).rejects.toThrow("permission denied"); + }); +}); + // ── parseFreeOutput — pure unit tests ───────────────────────────────────────── describe("parseFreeOutput", () => { @@ -303,3 +378,124 @@ describe("hetzner_get_server_ram — error handling", () => { expect(result.content[0].text).toMatch(/IPv4|unexpected format/i); }); }); + +// ── [H-2] expected_fingerprint — TOFU MITM prevention ─────────────────────── + +const FAKE_FP = "SHA256:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; +const WRONG_FP = "SHA256:BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"; + +describe("hetzner_get_server_ram — expected_fingerprint", () => { + it("tool description warns about TOFU risk and mentions expected_fingerprint", () => { + const opts = captureToolOpts(); + expect(opts.description).toMatch(/TOFU|accept-new/i); + expect(opts.description).toContain("expected_fingerprint"); + }); + + it("input schema accepts expected_fingerprint as optional string", () => { + const opts = captureToolOpts(); + expect(opts.inputSchema.shape).toHaveProperty("expected_fingerprint"); + }); + + it("skips fingerprint check when expected_fingerprint is not provided", async () => { + mockedRequest.mockResolvedValueOnce(serverResponse); + mockSsh.mockResolvedValueOnce(FREE_OUTPUT_NORMAL); + + await captureHandler()({ id: 1, response_format: "markdown" }); + + expect(mockKeyScan).not.toHaveBeenCalled(); + expect(mockSsh).toHaveBeenCalled(); + }); + + it("calls keyScanRunner with resolved IP and port when expected_fingerprint is provided", async () => { + mockedRequest.mockResolvedValueOnce(serverResponse); + mockKeyScan.mockResolvedValueOnce([FAKE_FP]); + mockSsh.mockResolvedValueOnce(FREE_OUTPUT_NORMAL); + + await captureHandler()({ + id: 1, + expected_fingerprint: FAKE_FP, + ssh_port: 22, + response_format: "markdown" + }); + + expect(mockKeyScan).toHaveBeenCalledWith("91.99.173.93", 22); + }); + + it("proceeds normally when fingerprint matches", async () => { + mockedRequest.mockResolvedValueOnce(serverResponse); + mockKeyScan.mockResolvedValueOnce([FAKE_FP]); + mockSsh.mockResolvedValueOnce(FREE_OUTPUT_NORMAL); + + const result = await captureHandler()({ + id: 1, + expected_fingerprint: FAKE_FP, + response_format: "markdown" + }); + + expect(result.isError).toBeUndefined(); + expect(mockSsh).toHaveBeenCalled(); + }); + + it("returns isError and does NOT call sshRunner when fingerprint mismatches", async () => { + mockedRequest.mockResolvedValueOnce(serverResponse); + mockKeyScan.mockResolvedValueOnce([FAKE_FP]); + + const result = await captureHandler()({ + id: 1, + expected_fingerprint: WRONG_FP, + response_format: "markdown" + }); + + expect(result.isError).toBe(true); + expect(result.content[0].text).toMatch(/fingerprint mismatch/i); + expect(mockSsh).not.toHaveBeenCalled(); + }); + + it("returns isError when keyScan fails", async () => { + mockedRequest.mockResolvedValueOnce(serverResponse); + mockKeyScan.mockRejectedValueOnce(new Error("ssh-keyscan: connection refused")); + + const result = await captureHandler()({ + id: 1, + expected_fingerprint: FAKE_FP, + response_format: "markdown" + }); + + expect(result.isError).toBe(true); + expect(result.content[0].text).toMatch(/fingerprint|keyscan/i); + expect(mockSsh).not.toHaveBeenCalled(); + }); + + it("proceeds when expected_fingerprint matches second key in multi-key response (Finding #1)", async () => { + mockedRequest.mockResolvedValueOnce(serverResponse); + // keyScanRunner now returns string[] — expected is the SECOND fingerprint + const multiKeyMock = vi.fn().mockResolvedValueOnce([WRONG_FP, FAKE_FP]); + mockSsh.mockResolvedValueOnce(FREE_OUTPUT_NORMAL); + + const result = await captureHandler(multiKeyMock)({ + id: 1, + expected_fingerprint: FAKE_FP, + response_format: "markdown" + }); + + expect(result.isError).toBeUndefined(); + expect(mockSsh).toHaveBeenCalled(); + }); + + it("returns isError when padded fingerprint (SHA256:abc==) is expected but extraction strips padding (Finding #2)", async () => { + const PADDED_FP = "SHA256:AbCdEfGhIjKlMnOpQrStUvWxYzABCDEFGHIJK=="; + mockedRequest.mockResolvedValueOnce(serverResponse); + const paddedMock = vi.fn().mockResolvedValueOnce([PADDED_FP]); + mockSsh.mockResolvedValueOnce(FREE_OUTPUT_NORMAL); + + const result = await captureHandler(paddedMock)({ + id: 1, + expected_fingerprint: PADDED_FP, + response_format: "markdown" + }); + + // Should succeed — padded fingerprint in response must match padded expected + expect(result.isError).toBeUndefined(); + expect(mockSsh).toHaveBeenCalled(); + }); +}); diff --git a/tests/tools/servers.test.ts b/tests/tools/servers.test.ts index 89eb741..f7f3ba8 100644 --- a/tests/tools/servers.test.ts +++ b/tests/tools/servers.test.ts @@ -12,7 +12,7 @@ vi.mock("../../src/api.js", async (importOriginal) => { import { registerServerTools } from "../../src/tools/servers.js"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { makeApiRequest } from "../../src/api.js"; -import { HetznerServer, ListServersResponse, ListServersResponseSchema } from "../../src/types.js"; +import { HetznerServer, HetznerServerSchema, ListServersResponse, ListServersResponseSchema } from "../../src/types.js"; const mockedRequest = vi.mocked(makeApiRequest); @@ -29,11 +29,12 @@ const baseServer: HetznerServer = { ipv6: { ip: "2001:db8::1" } }, server_type: { id: 1, name: "cx22", description: "CX22", cores: 2, memory: 4, disk: 40 }, - datacenter: { + location: { id: 1, - name: "fsn1-dc14", + name: "fsn1", description: "Falkenstein DC Park 1", - location: { id: 1, name: "fsn1", city: "Falkenstein", country: "DE" } + country: "DE", + city: "Falkenstein" }, image: { id: 1, name: "ubuntu-24.04", description: "Ubuntu 24.04", os_flavor: "ubuntu", os_version: "24.04" }, labels: {}, @@ -73,6 +74,40 @@ function captureRegisteredTools(): CapturedTool[] { return captured; } +describe("hetzner_list_servers — location rendering", () => { + // Regression: Hetzner removed `datacenter` from the Servers API on 2026-06-30. + // The formatter must read the top-level `location` object instead. + it("renders Location from the top-level location object", async () => { + const tools = captureRegisteredTools(); + const handler = tools.find((t) => t.name === "hetzner_list_servers")!.handler; + mockedRequest.mockResolvedValueOnce(pageResponse([makeServer(1)], null)); + + const result = await handler({ response_format: "markdown" }); + + expect(result.isError).toBeUndefined(); + expect(result.content[0].text).toContain("**Location**: Falkenstein, DE (fsn1)"); + }); + + // The live payload carries no `datacenter` and extra location keys we never render. + // Declaring only the consumed fields must tolerate both. + it("parses a raw API payload with no datacenter and unknown extra keys", () => { + const rawApiServer = { + ...baseServer, + location: { + id: 1, + name: "fsn1", + description: "Falkenstein DC Park 1", + country: "DE", + city: "Falkenstein", + latitude: 50.47612, + longitude: 12.370071, + network_zone: "eu-central" + } + }; + expect(() => HetznerServerSchema.parse(rawApiServer)).not.toThrow(); + }); +}); + describe("hetzner_list_servers — auto-pagination", () => { it("fetches all pages and combines results", async () => { const tools = captureRegisteredTools(); @@ -260,3 +295,37 @@ describe("L-2b security: HTML escaping in formatServer non-label fields", () => expect(result.content[0].text).toContain('<evil-image>'); }); }); + +// ── [M-1/M-4] filter parameter validation ───────────────────────────────────── + +describe("hetzner_list_servers — filter parameter validation", () => { + it("rejects label_selector longer than 256 characters", () => { + const tools = captureRegisteredTools(); + const tool = tools.find((t) => t.name === "hetzner_list_servers")!; + const longStr = "a".repeat(257); + expect( + (tool.opts.inputSchema as { safeParse: (v: unknown) => { success: boolean } }) + .safeParse({ label_selector: longStr, response_format: "markdown" }).success + ).toBe(false); + }); + + it("accepts label_selector of exactly 256 characters", () => { + const tools = captureRegisteredTools(); + const tool = tools.find((t) => t.name === "hetzner_list_servers")!; + const okStr = "a".repeat(256); + expect( + (tool.opts.inputSchema as { safeParse: (v: unknown) => { success: boolean } }) + .safeParse({ label_selector: okStr, response_format: "markdown" }).success + ).toBe(true); + }); +}); + +// ── [L-1] create_server — root_password plaintext warning ───────────────────── + +describe("hetzner_create_server — root_password warning", () => { + it("description warns that JSON mode returns root_password in plaintext", () => { + const tools = captureRegisteredTools(); + const tool = tools.find((t) => t.name === "hetzner_create_server")!; + expect(tool.opts.description).toMatch(/root_password|log|plaintext/i); + }); +}); diff --git a/tests/tools/storage-boxes.test.ts b/tests/tools/storage-boxes.test.ts index de069fd..54ae7f1 100644 --- a/tests/tools/storage-boxes.test.ts +++ b/tests/tools/storage-boxes.test.ts @@ -16,7 +16,8 @@ import { formatSnapshot, formatAction, paginatedFetch, - registerStorageBoxTools + registerStorageBoxTools, + computeStorageBoxStats } from "../../src/tools/storage-boxes.js"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { makeStorageBoxApiRequest } from "../../src/api.js"; @@ -424,7 +425,7 @@ function captureRegisteredTools(): CapturedTool[] { } describe("registerStorageBoxTools — handler integration (I-7)", () => { - it("registers exactly 20 tools with the expected names", () => { + it("registers exactly 22 tools with the expected names", () => { const tools = captureRegisteredTools(); expect(tools.map((t) => t.name)).toEqual([ "hetzner_list_storage_boxes", @@ -446,6 +447,8 @@ describe("registerStorageBoxTools — handler integration (I-7)", () => { "hetzner_update_storage_box_access_settings", "hetzner_enable_storage_box_snapshot_plan", "hetzner_disable_storage_box_snapshot_plan", + "hetzner_get_storage_box_stats", + "hetzner_assert_storage_box_space", "hetzner_rollback_storage_box_snapshot" ]); }); @@ -1727,3 +1730,221 @@ describe("L-2b security: HTML escaping in non-label fields", () => { expect(out).toContain('<b>desc</b>'); }); }); + +// ── [M-1/M-4] filter parameter validation ───────────────────────────────────── + +describe("filter parameter validation — M-1/M-4", () => { + it("hetzner_list_storage_boxes rejects label_selector > 256 chars", () => { + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_list_storage_boxes")!; + expect( + tool.opts.inputSchema?.safeParse({ label_selector: "a".repeat(257), response_format: "markdown" }).success + ).toBe(false); + }); + + it("hetzner_list_storage_boxes rejects name > 255 chars", () => { + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_list_storage_boxes")!; + expect( + tool.opts.inputSchema?.safeParse({ name: "a".repeat(256), response_format: "markdown" }).success + ).toBe(false); + }); + + it("hetzner_list_storage_box_subaccounts rejects filter username > 255 chars", () => { + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_list_storage_box_subaccounts")!; + expect( + tool.opts.inputSchema?.safeParse({ id: 1, username: "a".repeat(256), response_format: "markdown" }).success + ).toBe(false); + }); + + it("hetzner_list_storage_box_subaccounts rejects filter username with special chars", () => { + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_list_storage_box_subaccounts")!; + expect( + tool.opts.inputSchema?.safeParse({ id: 1, username: "evil', 'normal-folder'] }); + const result = await tool.handler({ id: 1, response_format: "markdown" }); + expect(result.content[0].text).not.toContain(''); + expect(result.content[0].text).toContain('<script>evil</script>'); + expect(result.content[0].text).toContain('normal-folder'); + }); +}); + +// ── [M-3] password tools — plaintext MCP warning ───────────────────────────── + +describe("password tools — MCP plaintext security warning", () => { + it("hetzner_create_storage_box description warns about MCP plaintext password", () => { + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_create_storage_box")!; + expect(tool.opts.description).toMatch(/MCP|plaintext|log/i); + }); + + it("hetzner_reset_storage_box_password description warns about MCP plaintext password", () => { + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_reset_storage_box_password")!; + expect(tool.opts.description).toMatch(/MCP|plaintext|log/i); + }); +}); + +// ── computeStorageBoxStats ──────────────────────────────────────────────────── + +describe("computeStorageBoxStats", () => { + const GiB = 1024 ** 3; + + it("computes stats for a box with zero usage", () => { + const box = { ...baseBox, stats: { size: 0, size_data: 0, size_snapshots: 0 } }; + const stats = computeStorageBoxStats(box); + expect(stats.used_bytes).toBe(0); + expect(stats.used_gib).toBe(0); + expect(stats.total_bytes).toBe(GiB * 1024); + expect(stats.total_gib).toBeCloseTo(1024, 1); + expect(stats.available_gib).toBeCloseTo(1024, 1); + expect(stats.usage_percent).toBe(0); + }); + + it("computes stats when 69% used (707 GiB of 1024 GiB)", () => { + const used = Math.round(707 * GiB); + const box = { ...baseBox, stats: { size: used, size_data: used, size_snapshots: 0 } }; + const stats = computeStorageBoxStats(box); + expect(stats.used_gib).toBeCloseTo(707, 0); + expect(stats.total_gib).toBeCloseTo(1024, 0); + expect(stats.available_gib).toBeCloseTo(1024 - 707, 0); + expect(stats.usage_percent).toBeCloseTo(69.04, 1); + }); + + it("returns 0 usage_percent when total_bytes is 0 (guard against division by zero)", () => { + const box = { + ...baseBox, + storage_box_type: { ...baseBox.storage_box_type, size: 0 }, + stats: { size: 0, size_data: 0, size_snapshots: 0 } + }; + const stats = computeStorageBoxStats(box); + expect(stats.usage_percent).toBe(0); + }); + + it("rounds usage_percent to 2 decimal places", () => { + const used = 1; + const total = 3; + const box = { + ...baseBox, + storage_box_type: { ...baseBox.storage_box_type, size: total }, + stats: { size: used, size_data: used, size_snapshots: 0 } + }; + const stats = computeStorageBoxStats(box); + expect(stats.usage_percent).toBe(33.33); + }); +}); + +// ── hetzner_get_storage_box_stats ──────────────────────────────────────────── + +describe("hetzner_get_storage_box_stats", () => { + const GiB = 1024 ** 3; + const usedBox: HetznerStorageBox = { + ...baseBox, + storage_box_type: { ...baseBox.storage_box_type, size: 1024 * GiB }, + stats: { size: Math.round(707 * GiB), size_data: Math.round(707 * GiB), size_snapshots: 0 } + }; + + it("returns JSON stats when response_format=json", async () => { + mockedRequest.mockResolvedValueOnce({ storage_box: usedBox }); + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_get_storage_box_stats")!; + const result = await tool.handler({ id: 1, response_format: "json" }); + const parsed = JSON.parse(result.content[0].text); + expect(parsed.used_gib).toBeCloseTo(707, 0); + expect(parsed.total_gib).toBeCloseTo(1024, 0); + expect(parsed.available_gib).toBeCloseTo(317, 0); + expect(parsed.usage_percent).toBeGreaterThan(60); + expect(result.isError).toBeFalsy(); + }); + + it("returns markdown stats with GiB labels", async () => { + mockedRequest.mockResolvedValueOnce({ storage_box: usedBox }); + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_get_storage_box_stats")!; + const result = await tool.handler({ id: 1, response_format: "markdown" }); + expect(result.content[0].text).toMatch(/Usage Stats/); + expect(result.content[0].text).toMatch(/Used/); + expect(result.content[0].text).toMatch(/Available/); + expect(result.content[0].text).toMatch(/GiB/); + expect(result.isError).toBeFalsy(); + }); + + it("returns isError on API failure", async () => { + mockedRequest.mockRejectedValueOnce(new Error("network error")); + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_get_storage_box_stats")!; + const result = await tool.handler({ id: 1, response_format: "json" }); + expect(result.isError).toBe(true); + }); + + it("has readOnlyHint: true", () => { + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_get_storage_box_stats")!; + expect(tool.opts.annotations?.readOnlyHint).toBe(true); + expect(tool.opts.annotations?.destructiveHint).toBe(false); + }); +}); + +// ── hetzner_assert_storage_box_space ───────────────────────────────────────── + +describe("hetzner_assert_storage_box_space", () => { + const GiB = 1024 ** 3; + + const makeBox = (usedGib: number, totalGib: number): HetznerStorageBox => ({ + ...baseBox, + storage_box_type: { ...baseBox.storage_box_type, size: totalGib * GiB }, + stats: { size: Math.round(usedGib * GiB), size_data: Math.round(usedGib * GiB), size_snapshots: 0 } + }); + + it("returns success when available space exceeds required_gib", async () => { + mockedRequest.mockResolvedValueOnce({ storage_box: makeBox(707, 1024) }); + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_assert_storage_box_space")!; + const result = await tool.handler({ id: 1, required_gib: 15 }); + expect(result.isError).toBeFalsy(); + expect(result.content[0].text).toMatch(/sufficient/); + }); + + it("returns isError when available space is less than required_gib", async () => { + mockedRequest.mockResolvedValueOnce({ storage_box: makeBox(1010, 1024) }); + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_assert_storage_box_space")!; + const result = await tool.handler({ id: 1, required_gib: 15 }); + expect(result.isError).toBe(true); + expect(result.content[0].text).toMatch(/insufficient/); + }); + + it("returns success when available space exactly equals required_gib", async () => { + const totalGib = 1024; + const availableGib = 15; + mockedRequest.mockResolvedValueOnce({ storage_box: makeBox(totalGib - availableGib, totalGib) }); + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_assert_storage_box_space")!; + const result = await tool.handler({ id: 1, required_gib: 15 }); + expect(result.isError).toBeFalsy(); + }); + + it("returns isError on API failure", async () => { + mockedRequest.mockRejectedValueOnce(new Error("timeout")); + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_assert_storage_box_space")!; + const result = await tool.handler({ id: 1, required_gib: 10 }); + expect(result.isError).toBe(true); + }); + + it("has readOnlyHint: true", () => { + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_assert_storage_box_space")!; + expect(tool.opts.annotations?.readOnlyHint).toBe(true); + expect(tool.opts.annotations?.destructiveHint).toBe(false); + }); + + it("rejects required_gib <= 0 at schema level", () => { + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_assert_storage_box_space")!; + const result = tool.opts.inputSchema?.safeParse({ id: 1, required_gib: 0 }); + expect(result?.success).toBe(false); + }); +}); diff --git a/tests/tools/volumes.test.ts b/tests/tools/volumes.test.ts index b695092..7a53278 100644 --- a/tests/tools/volumes.test.ts +++ b/tests/tools/volumes.test.ts @@ -330,3 +330,97 @@ describe("hetzner_detach_volume", () => { expect(result.isError).toBe(true); }); }); + +// ── [M-1/M-4] filter parameter validation ───────────────────────────────────── + +describe("hetzner_list_volumes — filter parameter validation", () => { + it("rejects label_selector longer than 256 characters", () => { + const tools = captureRegisteredTools(); + const tool = tools.find((t) => t.name === "hetzner_list_volumes")!; + const longStr = "a".repeat(257); + expect( + (tool.opts as { inputSchema?: { safeParse: (v: unknown) => { success: boolean } } }) + .inputSchema?.safeParse({ label_selector: longStr, response_format: "markdown" }).success + ).toBe(false); + }); + + it("rejects status longer than 64 characters", () => { + const tools = captureRegisteredTools(); + const tool = tools.find((t) => t.name === "hetzner_list_volumes")!; + expect( + (tool.opts as { inputSchema?: { safeParse: (v: unknown) => { success: boolean } } }) + .inputSchema?.safeParse({ status: "a".repeat(65), response_format: "markdown" }).success + ).toBe(false); + }); + + it("accepts valid label_selector and status", () => { + const tools = captureRegisteredTools(); + const tool = tools.find((t) => t.name === "hetzner_list_volumes")!; + expect( + (tool.opts as { inputSchema?: { safeParse: (v: unknown) => { success: boolean } } }) + .inputSchema?.safeParse({ label_selector: "env=prod", status: "available", response_format: "markdown" }).success + ).toBe(true); + }); +}); + +// ── [M-2] escapeHtml in formatVolume ───────────────────────────────────────── + +const XSS = ''; +const SAFE = '<script>alert(1)</script>'; + +describe("hetzner_list_volumes — escapeHtml in output", () => { + it("escapes vol.name containing HTML in markdown output", async () => { + const tools = captureRegisteredTools(); + const handler = tools.find((t) => t.name === "hetzner_list_volumes")!.handler; + mockedRequest.mockResolvedValueOnce({ volumes: [{ ...baseVolume, name: XSS }], meta: { pagination: { next_page: null } } }); + const result = await handler({ response_format: "markdown" }); + expect(result.content[0].text).not.toContain(XSS); + expect(result.content[0].text).toContain(SAFE); + }); + + it("escapes vol.location.city/country/name containing HTML in markdown output", async () => { + const tools = captureRegisteredTools(); + const handler = tools.find((t) => t.name === "hetzner_list_volumes")!.handler; + const badLoc = { ...baseVolume.location, city: XSS, country: "DE", name: "nbg1" }; + mockedRequest.mockResolvedValueOnce({ volumes: [{ ...baseVolume, location: badLoc }], meta: { pagination: { next_page: null } } }); + const result = await handler({ response_format: "markdown" }); + expect(result.content[0].text).not.toContain(XSS); + expect(result.content[0].text).toContain(SAFE); + }); + + it("escapes label keys and values containing HTML in markdown output", async () => { + const tools = captureRegisteredTools(); + const handler = tools.find((t) => t.name === "hetzner_list_volumes")!.handler; + mockedRequest.mockResolvedValueOnce({ volumes: [{ ...baseVolume, labels: { [XSS]: "val" } }], meta: { pagination: { next_page: null } } }); + const result = await handler({ response_format: "markdown" }); + expect(result.content[0].text).not.toContain(XSS); + expect(result.content[0].text).toContain(SAFE); + }); +}); + +describe("hetzner_get_volume — escapeHtml in output", () => { + it("escapes vol.name in markdown output for single volume", async () => { + const tools = captureRegisteredTools(); + const handler = tools.find((t) => t.name === "hetzner_get_volume")!.handler; + mockedRequest.mockResolvedValueOnce({ volume: { ...baseVolume, name: XSS } }); + const result = await handler({ id: 1, response_format: "markdown" }); + expect(result.content[0].text).not.toContain(XSS); + expect(result.content[0].text).toContain(SAFE); + }); +}); + +// ── [Finding #6] escapeHtml apostrophe consistency ─────────────────────────── + +describe("hetzner_list_volumes — escapeHtml apostrophe uses ' (Finding #6)", () => { + it("encodes apostrophe in vol.name as ' not '", async () => { + const tools = captureRegisteredTools(); + const handler = tools.find((t) => t.name === "hetzner_list_volumes")!.handler; + mockedRequest.mockResolvedValueOnce({ + volumes: [{ ...baseVolume, name: "O'Brian's volume" }], + meta: { pagination: { next_page: null } } + }); + const result = await handler({ response_format: "markdown" }); + expect(result.content[0].text).not.toContain("'"); + expect(result.content[0].text).toContain("'"); + }); +});