From 5ef4ab68cac4b41b2f8e62cf071803471da4d26e Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 26 May 2026 10:53:19 +0800 Subject: [PATCH 01/35] test: add reproducer for H-1 username/snapshot_id path injection (RED) --- tests/tools/storage-boxes.test.ts | 90 +++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/tests/tools/storage-boxes.test.ts b/tests/tools/storage-boxes.test.ts index 3ce1532..f4dd253 100644 --- a/tests/tools/storage-boxes.test.ts +++ b/tests/tools/storage-boxes.test.ts @@ -1392,3 +1392,93 @@ describe("hetzner_disable_storage_box_snapshot_plan", () => { expect(result.isError).toBe(true); }); }); + +// H-1 security: path injection prevention in URL path parameters +describe("H-1 security: username / snapshot_id path injection prevention", () => { + type SchemaLike = { safeParse: (v: unknown) => { success: boolean } }; + type ToolWithSchema = { name: string; opts: { inputSchema: SchemaLike } }; + + function captureWithSchema(): ToolWithSchema[] { + return captureRegisteredTools() as unknown as ToolWithSchema[]; + } + + // hetzner_update_storage_box_subaccount — username + it("update_subaccount: rejects username with path traversal (../evil)", () => { + const tool = captureWithSchema().find((t) => t.name === "hetzner_update_storage_box_subaccount")!; + expect( + tool.opts.inputSchema.safeParse({ id: 1, username: "../evil", response_format: "markdown" }).success + ).toBe(false); + }); + + it("update_subaccount: rejects username with forward slash (a/b)", () => { + const tool = captureWithSchema().find((t) => t.name === "hetzner_update_storage_box_subaccount")!; + expect( + tool.opts.inputSchema.safeParse({ id: 1, username: "a/b", response_format: "markdown" }).success + ).toBe(false); + }); + + it("update_subaccount: accepts valid username (u123-sub1)", () => { + const tool = captureWithSchema().find((t) => t.name === "hetzner_update_storage_box_subaccount")!; + expect( + tool.opts.inputSchema.safeParse({ id: 1, username: "u123-sub1", response_format: "markdown" }).success + ).toBe(true); + }); + + it("update_subaccount: accepts valid username with dot (u123.sub1)", () => { + const tool = captureWithSchema().find((t) => t.name === "hetzner_update_storage_box_subaccount")!; + expect( + tool.opts.inputSchema.safeParse({ id: 1, username: "u123.sub1", response_format: "markdown" }).success + ).toBe(true); + }); + + // hetzner_delete_storage_box_subaccount — username + it("delete_subaccount: rejects username with path traversal (../actions/reset)", () => { + const tool = captureWithSchema().find((t) => t.name === "hetzner_delete_storage_box_subaccount")!; + expect( + tool.opts.inputSchema.safeParse({ id: 1, username: "../actions/reset" }).success + ).toBe(false); + }); + + it("delete_subaccount: rejects username with percent-encoded slash (u123%2fevil)", () => { + const tool = captureWithSchema().find((t) => t.name === "hetzner_delete_storage_box_subaccount")!; + expect( + tool.opts.inputSchema.safeParse({ id: 1, username: "u123%2fevil" }).success + ).toBe(false); + }); + + it("delete_subaccount: accepts valid username (u123-sub1)", () => { + const tool = captureWithSchema().find((t) => t.name === "hetzner_delete_storage_box_subaccount")!; + expect( + tool.opts.inputSchema.safeParse({ id: 1, username: "u123-sub1" }).success + ).toBe(true); + }); + + // hetzner_delete_storage_box_snapshot — snapshot_id + it("delete_snapshot: rejects snapshot_id with path traversal (../evil)", () => { + const tool = captureWithSchema().find((t) => t.name === "hetzner_delete_storage_box_snapshot")!; + expect( + tool.opts.inputSchema.safeParse({ id: 1, snapshot_id: "../evil" }).success + ).toBe(false); + }); + + it("delete_snapshot: rejects snapshot_id with forward slash (a/b)", () => { + const tool = captureWithSchema().find((t) => t.name === "hetzner_delete_storage_box_snapshot")!; + expect( + tool.opts.inputSchema.safeParse({ id: 1, snapshot_id: "a/b" }).success + ).toBe(false); + }); + + it("delete_snapshot: accepts valid snapshot_id with hyphen (snapshot-2024-01-01)", () => { + const tool = captureWithSchema().find((t) => t.name === "hetzner_delete_storage_box_snapshot")!; + expect( + tool.opts.inputSchema.safeParse({ id: 1, snapshot_id: "snapshot-2024-01-01" }).success + ).toBe(true); + }); + + it("delete_snapshot: accepts numeric snapshot_id (12345)", () => { + const tool = captureWithSchema().find((t) => t.name === "hetzner_delete_storage_box_snapshot")!; + expect( + tool.opts.inputSchema.safeParse({ id: 1, snapshot_id: "12345" }).success + ).toBe(true); + }); +}); From 61d675622b4c41bfdede32980d97f6e7d5150673 Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 26 May 2026 10:54:10 +0800 Subject: [PATCH 02/35] fix(security): add regex allowlist to username and snapshot_id URL path params (H-1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prevent path traversal / SSRF by guarding three URL path parameters that were only validated with z.string().min(1): - username in hetzner_update_storage_box_subaccount - username in hetzner_delete_storage_box_subaccount - snapshot_id in hetzner_delete_storage_box_snapshot Restricts each to /^[a-zA-Z0-9._-]+$/ — same pattern used for ssh_user. --- src/tools/storage-boxes.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/tools/storage-boxes.ts b/src/tools/storage-boxes.ts index 815934d..5c9b2e3 100644 --- a/src/tools/storage-boxes.ts +++ b/src/tools/storage-boxes.ts @@ -818,7 +818,10 @@ Use access settings to configure which protocols the subaccount can use.`, description: `Update access settings or comment for a Storage Box subaccount.`, inputSchema: z.object({ id: z.number().int().positive().describe("The Storage Box ID"), - username: z.string().min(1).describe("The subaccount username to update"), + username: z + .string() + .regex(/^[a-zA-Z0-9._-]+$/, "username must contain only alphanumeric characters, dots, underscores, or hyphens") + .describe("The subaccount username to update"), comment: z.string().optional().describe("New comment"), labels: z.record(z.string(), z.string()).optional().describe("Labels (replaces existing)"), ssh_enabled: z.boolean().optional().describe("Allow SSH access"), @@ -881,7 +884,10 @@ Use access settings to configure which protocols the subaccount can use.`, description: `Delete a subaccount from a Storage Box.`, inputSchema: z.object({ id: z.number().int().positive().describe("The Storage Box ID"), - username: z.string().min(1).describe("The subaccount username to delete") + username: z + .string() + .regex(/^[a-zA-Z0-9._-]+$/, "username must contain only alphanumeric characters, dots, underscores, or hyphens") + .describe("The subaccount username to delete") }).strict(), annotations: { readOnlyHint: false, @@ -919,7 +925,10 @@ Use access settings to configure which protocols the subaccount can use.`, ⚠️ DESTRUCTIVE: The snapshot will be permanently deleted and cannot be recovered.`, inputSchema: z.object({ id: z.number().int().positive().describe("The Storage Box ID"), - snapshot_id: z.string().min(1).describe("Snapshot name or numeric ID (as string)") + snapshot_id: z + .string() + .regex(/^[a-zA-Z0-9._-]+$/, "snapshot_id must contain only alphanumeric characters, dots, underscores, or hyphens") + .describe("Snapshot name or numeric ID (as string)") }).strict(), annotations: { readOnlyHint: false, From c6dcfaa86be767381e36678990694973d295097b Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 26 May 2026 10:55:14 +0800 Subject: [PATCH 03/35] fix(security): force fast-uri@3.1.2 via overrides to resolve HIGH CVEs (H-2) GHSA-v39h-62p7-jpjc: host confusion via percent-encoded authority delimiters GHSA-q3j6-qgpj-74h6: path traversal via percent-encoded dot segments bun update alone could not upgrade the transitive dep (locked by ajv range); using package.json overrides forces fast-uri to the patched 3.1.2 release. Remaining 8 vulnerabilities are all moderate/low via stdio-only transport paths. --- bun.lock | 13 ++++++++----- package.json | 7 +++++-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/bun.lock b/bun.lock index c948483..799967b 100644 --- a/bun.lock +++ b/bun.lock @@ -5,7 +5,7 @@ "": { "name": "hetzner-mcp", "dependencies": { - "@modelcontextprotocol/sdk": "^1.6.1", + "@modelcontextprotocol/sdk": "^1.29.0", "axios": "^1.7.9", "zod": "^4.3.0", }, @@ -13,7 +13,7 @@ "@eslint/js": "^10.0.1", "@types/node": "^25.0.0", "@vitest/coverage-v8": "^4.1.5", - "eslint": "^10.2.0", + "eslint": "^10.4.0", "eslint-config-prettier": "^10.1.8", "globals": "^17.4.0", "typescript": "^6.0.0", @@ -22,6 +22,9 @@ }, }, }, + "overrides": { + "fast-uri": "3.1.2", + }, "packages": { "@babel/helper-string-parser": ["@babel/helper-string-parser@7.27.1", "", {}, "sha512-qMlSxKbpRlAridDExk92nSobyDdpPijUq2DW6oDnUqd0iOGxmQjyqhMIihI9+zv4LPyZdRje2cavWPbCbWm3eA=="], @@ -97,7 +100,7 @@ "@eslint/config-array": ["@eslint/config-array@0.23.5", "", { "dependencies": { "@eslint/object-schema": "^3.0.5", "debug": "^4.3.1", "minimatch": "^10.2.4" } }, "sha512-Y3kKLvC1dvTOT+oGlqNQ1XLqK6D1HU2YXPc52NmAlJZbMMWDzGYXMiPRJ8TYD39muD/OTjlZmNJ4ib7dvSrMBA=="], - "@eslint/config-helpers": ["@eslint/config-helpers@0.5.5", "", { "dependencies": { "@eslint/core": "^1.2.1" } }, "sha512-eIJYKTCECbP/nsKaaruF6LW967mtbQbsw4JTtSVkUQc9MneSkbrgPJAbKl9nWr0ZeowV8BfsarBmPpBzGelA2w=="], + "@eslint/config-helpers": ["@eslint/config-helpers@0.6.0", "", { "dependencies": { "@eslint/core": "^1.2.1" } }, "sha512-ii6Bw9jJ2zi2cWA2Z+9/QZ/+3DX6kwaV5Q986D/CdP3Lap3w/pgQZ373FV7byY/i7L4IRH/G43I5dz1ClsCbpA=="], "@eslint/core": ["@eslint/core@1.2.1", "", { "dependencies": { "@types/json-schema": "^7.0.15" } }, "sha512-MwcE1P+AZ4C6DWlpin/OmOA54mmIZ/+xZuJiQd4SyB29oAJjN30UW9wkKNptW2ctp4cEsvhlLY/CsQ1uoHDloQ=="], @@ -295,7 +298,7 @@ "escape-string-regexp": ["escape-string-regexp@4.0.0", "", {}, "sha512-TtpcNJ3XAzx3Gq8sWRzJaVajRs0uVxA2YAkdb1jm2YkPz4G6egUFAyA3n5vtEIZefPk5Wa4UXbKuS5fKkJWdgA=="], - "eslint": ["eslint@10.2.1", "", { "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.2", "@eslint/config-array": "^0.23.5", "@eslint/config-helpers": "^0.5.5", "@eslint/core": "^1.2.1", "@eslint/plugin-kit": "^0.7.1", "@humanfs/node": "^0.16.6", "@humanwhocodes/module-importer": "^1.0.1", "@humanwhocodes/retry": "^0.4.2", "@types/estree": "^1.0.6", "ajv": "^6.14.0", "cross-spawn": "^7.0.6", "debug": "^4.3.2", "escape-string-regexp": "^4.0.0", "eslint-scope": "^9.1.2", "eslint-visitor-keys": "^5.0.1", "espree": "^11.2.0", "esquery": "^1.7.0", "esutils": "^2.0.2", "fast-deep-equal": "^3.1.3", "file-entry-cache": "^8.0.0", "find-up": "^5.0.0", "glob-parent": "^6.0.2", "ignore": "^5.2.0", "imurmurhash": "^0.1.4", "is-glob": "^4.0.0", "json-stable-stringify-without-jsonify": "^1.0.1", "minimatch": "^10.2.4", "natural-compare": "^1.4.0", "optionator": "^0.9.3" }, "peerDependencies": { "jiti": "*" }, "optionalPeers": ["jiti"], "bin": "bin/eslint.js" }, "sha512-wiyGaKsDgqXvF40P8mDwiUp/KQjE1FdrIEJsM8PZ3XCiniTMXS3OHWWUe5FI5agoCnr8x4xPrTDZuxsBlNHl+Q=="], + "eslint": ["eslint@10.4.0", "", { "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.2", "@eslint/config-array": "^0.23.5", "@eslint/config-helpers": "^0.6.0", "@eslint/core": "^1.2.1", "@eslint/plugin-kit": "^0.7.1", "@humanfs/node": "^0.16.6", "@humanwhocodes/module-importer": "^1.0.1", "@humanwhocodes/retry": "^0.4.2", "@types/estree": "^1.0.6", "ajv": "^6.14.0", "cross-spawn": "^7.0.6", "debug": "^4.3.2", "escape-string-regexp": "^4.0.0", "eslint-scope": "^9.1.2", "eslint-visitor-keys": "^5.0.1", "espree": "^11.2.0", "esquery": "^1.7.0", "esutils": "^2.0.2", "fast-deep-equal": "^3.1.3", "file-entry-cache": "^8.0.0", "find-up": "^5.0.0", "glob-parent": "^6.0.2", "ignore": "^5.2.0", "imurmurhash": "^0.1.4", "is-glob": "^4.0.0", "json-stable-stringify-without-jsonify": "^1.0.1", "minimatch": "^10.2.4", "natural-compare": "^1.4.0", "optionator": "^0.9.3" }, "peerDependencies": { "jiti": "*" }, "optionalPeers": ["jiti"], "bin": { "eslint": "bin/eslint.js" } }, "sha512-loXy6bWOoP3EP6JA7jo6p5jMpBJmHmsNZM5SFRHLdh1MGOPurMnNBj4ZlAbaqUAaQWbCr7jHV4P7gzAyryZWkQ=="], "eslint-config-prettier": ["eslint-config-prettier@10.1.8", "", { "peerDependencies": { "eslint": ">=7.0.0" }, "bin": "bin/cli.js" }, "sha512-82GZUjRS0p/jganf6q1rEO25VSoHH0hKPCTrgillPjdI/3bgBhAE1QzHrHTizjpRvy6pGAvKjDJtk2pF9NDq8w=="], @@ -333,7 +336,7 @@ "fast-levenshtein": ["fast-levenshtein@2.0.6", "", {}, "sha512-DCXu6Ifhqcks7TZKY3Hxp3y6qphY5SJZmrWMDrKcERSOXWQdMhU9Ig/PYrzyw/ul9jOIyh0N4M0tbC5hodg8dw=="], - "fast-uri": ["fast-uri@3.1.0", "", {}, "sha512-iPeeDKJSWf4IEOasVVrknXpaBV0IApz/gp7S2bb7Z4Lljbl2MGJRqInZiUrQwV16cpzw/D3S5j5Julj/gT52AA=="], + "fast-uri": ["fast-uri@3.1.2", "", {}, "sha512-rVjf7ArG3LTk+FS6Yw81V1DLuZl1bRbNrev6Tmd/9RaroeeRRJhAt7jg/6YFxbvAQXUCavSoZhPPj6oOx+5KjQ=="], "fdir": ["fdir@6.5.0", "", { "peerDependencies": { "picomatch": "^3 || ^4" } }, "sha512-tIbYtZbucOs0BRGqPJkshJUYdL+SDH7dVM8gjy+ERp3WAUjLEFJE+02kanyHtwjWOnwrKYBiwAmM0p4kLJAnXg=="], diff --git a/package.json b/package.json index b51fa7f..42cbe55 100644 --- a/package.json +++ b/package.json @@ -44,15 +44,18 @@ "README.md" ], "dependencies": { - "@modelcontextprotocol/sdk": "^1.6.1", + "@modelcontextprotocol/sdk": "^1.29.0", "axios": "^1.7.9", "zod": "^4.3.0" }, + "overrides": { + "fast-uri": "3.1.2" + }, "devDependencies": { "@eslint/js": "^10.0.1", "@types/node": "^25.0.0", "@vitest/coverage-v8": "^4.1.5", - "eslint": "^10.2.0", + "eslint": "^10.4.0", "eslint-config-prettier": "^10.1.8", "globals": "^17.4.0", "typescript": "^6.0.0", From e9f5cd7851087b7a60df10cd7989b55bae42d5cc Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 26 May 2026 10:56:44 +0800 Subject: [PATCH 04/35] test: add reproducer for M-2 password complexity not enforced in schema (RED) --- tests/tools/storage-boxes.test.ts | 68 +++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/tests/tools/storage-boxes.test.ts b/tests/tools/storage-boxes.test.ts index f4dd253..21471f6 100644 --- a/tests/tools/storage-boxes.test.ts +++ b/tests/tools/storage-boxes.test.ts @@ -1482,3 +1482,71 @@ describe("H-1 security: username / snapshot_id path injection prevention", () => ).toBe(true); }); }); + +// M-2 security: password complexity policy enforcement +describe("M-2 security: password complexity policy enforcement", () => { + type SchemaLike = { safeParse: (v: unknown) => { success: boolean } }; + type ToolWithSchema = { name: string; opts: { inputSchema: SchemaLike } }; + + function captureWithSchema(): ToolWithSchema[] { + return captureRegisteredTools() as unknown as ToolWithSchema[]; + } + + const validCreateBase = { + name: "test-box", + storage_box_type: "bx11", + location: "fsn1", + response_format: "markdown" + }; + + // hetzner_create_storage_box — password + it("create_storage_box: rejects password with only lowercase (aaaaaaaaaaaa)", () => { + const tool = captureWithSchema().find((t) => t.name === "hetzner_create_storage_box")!; + expect( + tool.opts.inputSchema.safeParse({ ...validCreateBase, password: "aaaaaaaaaaaa" }).success + ).toBe(false); + }); + + it("create_storage_box: rejects password missing special char (Abcdef123456)", () => { + const tool = captureWithSchema().find((t) => t.name === "hetzner_create_storage_box")!; + expect( + tool.opts.inputSchema.safeParse({ ...validCreateBase, password: "Abcdef123456" }).success + ).toBe(false); + }); + + it("create_storage_box: rejects password shorter than 12 chars (Ab1!)", () => { + const tool = captureWithSchema().find((t) => t.name === "hetzner_create_storage_box")!; + expect( + tool.opts.inputSchema.safeParse({ ...validCreateBase, password: "Ab1!" }).success + ).toBe(false); + }); + + it("create_storage_box: accepts compliant password (Correct$Horse7)", () => { + const tool = captureWithSchema().find((t) => t.name === "hetzner_create_storage_box")!; + expect( + tool.opts.inputSchema.safeParse({ ...validCreateBase, password: "Correct$Horse7" }).success + ).toBe(true); + }); + + // hetzner_reset_storage_box_password — password + it("reset_password: rejects password with only lowercase (aaaaaaaaaaaa)", () => { + const tool = captureWithSchema().find((t) => t.name === "hetzner_reset_storage_box_password")!; + expect( + tool.opts.inputSchema.safeParse({ id: 1, password: "aaaaaaaaaaaa" }).success + ).toBe(false); + }); + + it("reset_password: rejects password missing uppercase (correct$horse7)", () => { + const tool = captureWithSchema().find((t) => t.name === "hetzner_reset_storage_box_password")!; + expect( + tool.opts.inputSchema.safeParse({ id: 1, password: "correct$horse7" }).success + ).toBe(false); + }); + + it("reset_password: accepts compliant password (NewP@ss123Word)", () => { + const tool = captureWithSchema().find((t) => t.name === "hetzner_reset_storage_box_password")!; + expect( + tool.opts.inputSchema.safeParse({ id: 1, password: "NewP@ss123Word" }).success + ).toBe(true); + }); +}); From a9bc3e11f1e4526a1f35b21bb18054fc86b97368 Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 26 May 2026 10:57:03 +0800 Subject: [PATCH 05/35] fix(security): enforce password complexity policy in schema validation (M-2) Both hetzner_create_storage_box and hetzner_reset_storage_box_password described the policy (uppercase+lowercase+digit+special) but only enforced min(12). Weak passwords like 'aaaaaaaaaaaa' were accepted. Added regex lookahead to enforce all four character class requirements. --- src/tools/storage-boxes.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/tools/storage-boxes.ts b/src/tools/storage-boxes.ts index 5c9b2e3..d7a2935 100644 --- a/src/tools/storage-boxes.ts +++ b/src/tools/storage-boxes.ts @@ -563,7 +563,14 @@ Returns the new Storage Box and an action tracking provisioning.`, storage_box_type: z.string().min(1).describe("Storage box type name (e.g., 'bx11', 'bx20')"), location: z.string().min(1).describe("Location name (e.g., 'fsn1', 'nbg1', 'hel1')"), name: z.string().min(1).describe("Name for the storage box"), - password: z.string().min(12).describe("Initial password (min 12 chars, must include uppercase, lowercase, number, special char)"), + password: z + .string() + .min(12) + .regex( + /^(?=.*[A-Z])(?=.*[a-z])(?=.*\d)(?=.*[^A-Za-z0-9])/, + "Password must include uppercase, lowercase, number, and special character" + ) + .describe("Initial password (min 12 chars, must include uppercase, lowercase, number, special char)"), labels: z.record(z.string(), z.string()).optional().describe("Optional key-value labels"), ssh_enabled: z.boolean().optional().describe("Enable SSH access"), samba_enabled: z.boolean().optional().describe("Enable Samba access"), @@ -1045,7 +1052,14 @@ When delete protection is enabled, the Storage Box cannot be deleted until prote Password policy: minimum 12 characters, must include uppercase, lowercase, number, and special character.`, inputSchema: z.object({ id: z.number().int().positive().describe("The Storage Box ID"), - password: z.string().min(12).describe("New password (min 12 chars, must include uppercase, lowercase, number, special char)") + password: z + .string() + .min(12) + .regex( + /^(?=.*[A-Z])(?=.*[a-z])(?=.*\d)(?=.*[^A-Za-z0-9])/, + "Password must include uppercase, lowercase, number, and special character" + ) + .describe("New password (min 12 chars, must include uppercase, lowercase, number, special char)") }).strict(), annotations: { readOnlyHint: false, From da1f717bb118c0b3d564141059e8b9362cc9cc45 Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 26 May 2026 10:58:30 +0800 Subject: [PATCH 06/35] fix(security): add overrides to eliminate all remaining CVEs (M-3) Forced patched versions via package.json overrides: - brace-expansion: 5.0.6 (GHSA-jxxr-4gwj-5jf2, DoS) - qs: 6.15.2 (GHSA-q8mj-m7cp-5q26, DoS) - hono: 4.12.23 (GHSA-qp7p-654g-cw7p and others) - ip-address: 10.2.0 (GHSA-v2v4-37r5-5v8g, XSS) bun audit now reports: No vulnerabilities found --- bun.lock | 12 ++++++++---- package.json | 6 +++++- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/bun.lock b/bun.lock index 799967b..504f588 100644 --- a/bun.lock +++ b/bun.lock @@ -23,7 +23,11 @@ }, }, "overrides": { + "brace-expansion": "5.0.6", "fast-uri": "3.1.2", + "hono": "4.12.23", + "ip-address": "10.2.0", + "qs": "6.15.2", }, "packages": { "@babel/helper-string-parser": ["@babel/helper-string-parser@7.27.1", "", {}, "sha512-qMlSxKbpRlAridDExk92nSobyDdpPijUq2DW6oDnUqd0iOGxmQjyqhMIihI9+zv4LPyZdRje2cavWPbCbWm3eA=="], @@ -240,7 +244,7 @@ "body-parser": ["body-parser@2.2.2", "", { "dependencies": { "bytes": "^3.1.2", "content-type": "^1.0.5", "debug": "^4.4.3", "http-errors": "^2.0.0", "iconv-lite": "^0.7.0", "on-finished": "^2.4.1", "qs": "^6.14.1", "raw-body": "^3.0.1", "type-is": "^2.0.1" } }, "sha512-oP5VkATKlNwcgvxi0vM0p/D3n2C3EReYVX+DNYs5TjZFn/oQt2j+4sVJtSMr18pdRr8wjTcBl6LoV+FUwzPmNA=="], - "brace-expansion": ["brace-expansion@5.0.5", "", { "dependencies": { "balanced-match": "^4.0.2" } }, "sha512-VZznLgtwhn+Mact9tfiwx64fA9erHH/MCXEUfB/0bX/6Fz6ny5EGTXYltMocqg4xFAQZtnO3DHWWXi8RiuN7cQ=="], + "brace-expansion": ["brace-expansion@5.0.6", "", { "dependencies": { "balanced-match": "^4.0.2" } }, "sha512-kLpxurY4Z4r9sgMsyG0Z9uzsBlgiU/EFKhj/h91/8yHu0edo7XuixOIH3VcJ8kkxs6/jPzoI6U9Vj3WqbMQ94g=="], "bytes": ["bytes@3.1.2", "", {}, "sha512-/Nf7TyzTx6S3yRJObOAV7956r8cr2+Oj8AC5dt8wSP3BQAoeX58NoHyCU8P8zGkNXStjTSi6fzO6F0pBdcYbEg=="], @@ -382,7 +386,7 @@ "hasown": ["hasown@2.0.3", "", { "dependencies": { "function-bind": "^1.1.2" } }, "sha512-ej4AhfhfL2Q2zpMmLo7U1Uv9+PyhIZpgQLGT1F9miIGmiCJIoCgSmczFdrc97mWT4kVY72KA+WnnhJ5pghSvSg=="], - "hono": ["hono@4.12.15", "", {}, "sha512-qM0jDhFEaCBb4TxoW7f53Qrpv9RBiayUHo0S52JudprkhvpjIrGoU1mnnr29Fvd1U335ZFPZQY1wlkqgfGXyLg=="], + "hono": ["hono@4.12.23", "", {}, "sha512-eIaZ9qDgu7XV0pxOCrg7/WhnQ6Ivm22UcxhXx/A3dcbqbbYgBEkc6e/J/s7j2tS96zoB0S9VBdLwQNCWwUo4LA=="], "html-escaper": ["html-escaper@2.0.2", "", {}, "sha512-H2iMtd0I4Mt5eYiapRdIDjp+XzelXQ0tFE4JS7YFwFevXXMmOp9myNrUvCg0D6ws8iqkRPBfKHgbwig1SmlLfg=="], @@ -396,7 +400,7 @@ "inherits": ["inherits@2.0.4", "", {}, "sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ=="], - "ip-address": ["ip-address@10.1.0", "", {}, "sha512-XXADHxXmvT9+CRxhXg56LJovE+bmWnEWB78LB83VZTprKTmaC5QfruXocxzTZ2Kl0DNwKuBdlIhjL8LeY8Sf8Q=="], + "ip-address": ["ip-address@10.2.0", "", {}, "sha512-/+S6j4E9AHvW9SWMSEY9Xfy66O5PWvVEJ08O0y5JGyEKQpojb0K0GKpz/v5HJ/G0vi3D2sjGK78119oXZeE0qA=="], "ipaddr.js": ["ipaddr.js@1.9.1", "", {}, "sha512-0KI/607xoxSToH7GjN1FfSbLoU0+btTicjsQSWQlh/hZykN8KpmMf7uYwPW3R+akZ6R/w18ZlXSHBYXiYUPO3g=="], @@ -524,7 +528,7 @@ "punycode": ["punycode@2.3.1", "", {}, "sha512-vYt7UD1U9Wg6138shLtLOvdAu+8DsC/ilFtEVHcH+wydcSpNE20AfSOduf6MkRFahL5FY7X1oU7nKVZFtfq8Fg=="], - "qs": ["qs@6.15.1", "", { "dependencies": { "side-channel": "^1.1.0" } }, "sha512-6YHEFRL9mfgcAvql/XhwTvf5jKcOiiupt2FiJxHkiX1z4j7WL8J/jRHYLluORvc1XxB5rV20KoeK00gVJamspg=="], + "qs": ["qs@6.15.2", "", { "dependencies": { "side-channel": "^1.1.0" } }, "sha512-Rzq0KEyX/w/tEybncDgdkZrJgVUsUMk3xjh3t5bv3S1HTAtg+uOYt72+ZfwiQwKdysThkTBdL/rTi6HDmX9Ddw=="], "range-parser": ["range-parser@1.2.1", "", {}, "sha512-Hrgsx+orqoygnmhFbKaHE6c296J+HTAQXoxEF6gNupROmmGJRoyzfG3ccAveqCBrwr/2yxQ5BVd/GTl5agOwSg=="], diff --git a/package.json b/package.json index 42cbe55..9a75580 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,11 @@ "zod": "^4.3.0" }, "overrides": { - "fast-uri": "3.1.2" + "fast-uri": "3.1.2", + "brace-expansion": "5.0.6", + "qs": "6.15.2", + "hono": "4.12.23", + "ip-address": "10.2.0" }, "devDependencies": { "@eslint/js": "^10.0.1", From bcf02bd7de97c2ea724633d833ba73c10c8db40a Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 26 May 2026 10:59:04 +0800 Subject: [PATCH 07/35] docs(security): document StrictHostKeyChecking=accept-new trust model in tool description (M-1) The tool uses accept-new which silently trusts new SSH host keys. If a server's IP is reused after deletion, the tool connects to the new machine without warning. Added a clear warning in the tool description so operators understand the trust model and can pre-register fingerprints in known_hosts if needed. --- src/tools/server-ssh.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/tools/server-ssh.ts b/src/tools/server-ssh.ts index 0016d02..77233fd 100644 --- a/src/tools/server-ssh.ts +++ b/src/tools/server-ssh.ts @@ -124,6 +124,12 @@ 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, which automatically +trusts and records new host keys. If a server is deleted and its IP is later +reassigned to a different machine, the tool will connect to the new machine +without warning. For higher security, pre-register expected host fingerprints +in ~/.ssh/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({ id: z.number().int().positive() From 19b5dd18048ffea476edfe2802b284d8aa250b86 Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 26 May 2026 12:13:57 +0800 Subject: [PATCH 08/35] fix: address Copilot and Claude bot review feedback on PR #32 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review items resolved: 1. server-ssh.ts: Correct StrictHostKeyChecking=accept-new description — it still rejects mismatched keys for known hosts; the auto-trust only applies to hosts not yet in known_hosts. (Copilot review) 2. tests: Remove 'as unknown as' double cast by extending CapturedTool['opts'] with inputSchema?: z.ZodTypeAny, making schema access type-safe without runtime assertions. (Copilot review) 3. storage-boxes.ts: Replace snapshot_id allowlist regex with a denylist refine that only blocks '/', '\\', and '..' — allowlist was too strict and would reject Hetzner auto-snapshot names with ISO 8601 timestamps (e.g. 2024-01-15T12:00:00+01:00). (Claude bot review) --- src/tools/server-ssh.ts | 10 +-- src/tools/storage-boxes.ts | 6 +- tests/tools/storage-boxes.test.ts | 101 +++++++++++++++--------------- 3 files changed, 59 insertions(+), 58 deletions(-) diff --git a/src/tools/server-ssh.ts b/src/tools/server-ssh.ts index 77233fd..583d61a 100644 --- a/src/tools/server-ssh.ts +++ b/src/tools/server-ssh.ts @@ -124,11 +124,11 @@ 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, which automatically -trusts and records new host keys. If a server is deleted and its IP is later -reassigned to a different machine, the tool will connect to the new machine -without warning. For higher security, pre-register expected host fingerprints -in ~/.ssh/known_hosts and set StrictHostKeyChecking=yes in your SSH config. +⚠️ 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. Returns used / total / available in MiB and overall usage %, plus swap state.`, inputSchema: z.object({ diff --git a/src/tools/storage-boxes.ts b/src/tools/storage-boxes.ts index d7a2935..0a20530 100644 --- a/src/tools/storage-boxes.ts +++ b/src/tools/storage-boxes.ts @@ -934,7 +934,11 @@ Use access settings to configure which protocols the subaccount can use.`, id: z.number().int().positive().describe("The Storage Box ID"), snapshot_id: z .string() - .regex(/^[a-zA-Z0-9._-]+$/, "snapshot_id must contain only alphanumeric characters, dots, underscores, or hyphens") + .min(1) + .refine( + (s) => !s.includes("/") && !s.includes("\\") && !s.includes(".."), + "snapshot_id must not contain path separators or traversal sequences" + ) .describe("Snapshot name or numeric ID (as string)") }).strict(), annotations: { diff --git a/tests/tools/storage-boxes.test.ts b/tests/tools/storage-boxes.test.ts index 21471f6..f2f28fc 100644 --- a/tests/tools/storage-boxes.test.ts +++ b/tests/tools/storage-boxes.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import { ZodError } from "zod"; +import { z, ZodError } from "zod"; vi.mock("../../src/api.js", async (importOriginal) => { const actual = await importOriginal(); @@ -403,7 +403,11 @@ type ToolHandler = (params: unknown) => Promise<{ content: { type: string; text: interface CapturedTool { name: string; handler: ToolHandler; - opts: { annotations?: Record; description?: string }; + opts: { + annotations?: Record; + description?: string; + inputSchema?: z.ZodTypeAny; + }; } function captureRegisteredTools(): CapturedTool[] { @@ -1395,103 +1399,96 @@ describe("hetzner_disable_storage_box_snapshot_plan", () => { // H-1 security: path injection prevention in URL path parameters describe("H-1 security: username / snapshot_id path injection prevention", () => { - type SchemaLike = { safeParse: (v: unknown) => { success: boolean } }; - type ToolWithSchema = { name: string; opts: { inputSchema: SchemaLike } }; - - function captureWithSchema(): ToolWithSchema[] { - return captureRegisteredTools() as unknown as ToolWithSchema[]; - } - // hetzner_update_storage_box_subaccount — username it("update_subaccount: rejects username with path traversal (../evil)", () => { - const tool = captureWithSchema().find((t) => t.name === "hetzner_update_storage_box_subaccount")!; + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_update_storage_box_subaccount")!; expect( - tool.opts.inputSchema.safeParse({ id: 1, username: "../evil", response_format: "markdown" }).success + tool.opts.inputSchema?.safeParse({ id: 1, username: "../evil", response_format: "markdown" }).success ).toBe(false); }); it("update_subaccount: rejects username with forward slash (a/b)", () => { - const tool = captureWithSchema().find((t) => t.name === "hetzner_update_storage_box_subaccount")!; + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_update_storage_box_subaccount")!; expect( - tool.opts.inputSchema.safeParse({ id: 1, username: "a/b", response_format: "markdown" }).success + tool.opts.inputSchema?.safeParse({ id: 1, username: "a/b", response_format: "markdown" }).success ).toBe(false); }); it("update_subaccount: accepts valid username (u123-sub1)", () => { - const tool = captureWithSchema().find((t) => t.name === "hetzner_update_storage_box_subaccount")!; + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_update_storage_box_subaccount")!; expect( - tool.opts.inputSchema.safeParse({ id: 1, username: "u123-sub1", response_format: "markdown" }).success + tool.opts.inputSchema?.safeParse({ id: 1, username: "u123-sub1", response_format: "markdown" }).success ).toBe(true); }); it("update_subaccount: accepts valid username with dot (u123.sub1)", () => { - const tool = captureWithSchema().find((t) => t.name === "hetzner_update_storage_box_subaccount")!; + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_update_storage_box_subaccount")!; expect( - tool.opts.inputSchema.safeParse({ id: 1, username: "u123.sub1", response_format: "markdown" }).success + tool.opts.inputSchema?.safeParse({ id: 1, username: "u123.sub1", response_format: "markdown" }).success ).toBe(true); }); // hetzner_delete_storage_box_subaccount — username it("delete_subaccount: rejects username with path traversal (../actions/reset)", () => { - const tool = captureWithSchema().find((t) => t.name === "hetzner_delete_storage_box_subaccount")!; + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_delete_storage_box_subaccount")!; expect( - tool.opts.inputSchema.safeParse({ id: 1, username: "../actions/reset" }).success + tool.opts.inputSchema?.safeParse({ id: 1, username: "../actions/reset" }).success ).toBe(false); }); it("delete_subaccount: rejects username with percent-encoded slash (u123%2fevil)", () => { - const tool = captureWithSchema().find((t) => t.name === "hetzner_delete_storage_box_subaccount")!; + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_delete_storage_box_subaccount")!; expect( - tool.opts.inputSchema.safeParse({ id: 1, username: "u123%2fevil" }).success + tool.opts.inputSchema?.safeParse({ id: 1, username: "u123%2fevil" }).success ).toBe(false); }); it("delete_subaccount: accepts valid username (u123-sub1)", () => { - const tool = captureWithSchema().find((t) => t.name === "hetzner_delete_storage_box_subaccount")!; + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_delete_storage_box_subaccount")!; expect( - tool.opts.inputSchema.safeParse({ id: 1, username: "u123-sub1" }).success + tool.opts.inputSchema?.safeParse({ id: 1, username: "u123-sub1" }).success ).toBe(true); }); // hetzner_delete_storage_box_snapshot — snapshot_id it("delete_snapshot: rejects snapshot_id with path traversal (../evil)", () => { - const tool = captureWithSchema().find((t) => t.name === "hetzner_delete_storage_box_snapshot")!; + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_delete_storage_box_snapshot")!; expect( - tool.opts.inputSchema.safeParse({ id: 1, snapshot_id: "../evil" }).success + tool.opts.inputSchema?.safeParse({ id: 1, snapshot_id: "../evil" }).success ).toBe(false); }); it("delete_snapshot: rejects snapshot_id with forward slash (a/b)", () => { - const tool = captureWithSchema().find((t) => t.name === "hetzner_delete_storage_box_snapshot")!; + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_delete_storage_box_snapshot")!; expect( - tool.opts.inputSchema.safeParse({ id: 1, snapshot_id: "a/b" }).success + tool.opts.inputSchema?.safeParse({ id: 1, snapshot_id: "a/b" }).success ).toBe(false); }); it("delete_snapshot: accepts valid snapshot_id with hyphen (snapshot-2024-01-01)", () => { - const tool = captureWithSchema().find((t) => t.name === "hetzner_delete_storage_box_snapshot")!; + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_delete_storage_box_snapshot")!; expect( - tool.opts.inputSchema.safeParse({ id: 1, snapshot_id: "snapshot-2024-01-01" }).success + tool.opts.inputSchema?.safeParse({ id: 1, snapshot_id: "snapshot-2024-01-01" }).success ).toBe(true); }); it("delete_snapshot: accepts numeric snapshot_id (12345)", () => { - const tool = captureWithSchema().find((t) => t.name === "hetzner_delete_storage_box_snapshot")!; + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_delete_storage_box_snapshot")!; expect( - tool.opts.inputSchema.safeParse({ id: 1, snapshot_id: "12345" }).success + tool.opts.inputSchema?.safeParse({ id: 1, snapshot_id: "12345" }).success + ).toBe(true); + }); + + it("delete_snapshot: accepts ISO-style snapshot name with colon (2024-01-15T12:00:00+01:00)", () => { + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_delete_storage_box_snapshot")!; + expect( + tool.opts.inputSchema?.safeParse({ id: 1, snapshot_id: "2024-01-15T12:00:00+01:00" }).success ).toBe(true); }); }); // M-2 security: password complexity policy enforcement describe("M-2 security: password complexity policy enforcement", () => { - type SchemaLike = { safeParse: (v: unknown) => { success: boolean } }; - type ToolWithSchema = { name: string; opts: { inputSchema: SchemaLike } }; - - function captureWithSchema(): ToolWithSchema[] { - return captureRegisteredTools() as unknown as ToolWithSchema[]; - } - const validCreateBase = { name: "test-box", storage_box_type: "bx11", @@ -1501,52 +1498,52 @@ describe("M-2 security: password complexity policy enforcement", () => { // hetzner_create_storage_box — password it("create_storage_box: rejects password with only lowercase (aaaaaaaaaaaa)", () => { - const tool = captureWithSchema().find((t) => t.name === "hetzner_create_storage_box")!; + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_create_storage_box")!; expect( - tool.opts.inputSchema.safeParse({ ...validCreateBase, password: "aaaaaaaaaaaa" }).success + tool.opts.inputSchema?.safeParse({ ...validCreateBase, password: "aaaaaaaaaaaa" }).success ).toBe(false); }); it("create_storage_box: rejects password missing special char (Abcdef123456)", () => { - const tool = captureWithSchema().find((t) => t.name === "hetzner_create_storage_box")!; + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_create_storage_box")!; expect( - tool.opts.inputSchema.safeParse({ ...validCreateBase, password: "Abcdef123456" }).success + tool.opts.inputSchema?.safeParse({ ...validCreateBase, password: "Abcdef123456" }).success ).toBe(false); }); it("create_storage_box: rejects password shorter than 12 chars (Ab1!)", () => { - const tool = captureWithSchema().find((t) => t.name === "hetzner_create_storage_box")!; + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_create_storage_box")!; expect( - tool.opts.inputSchema.safeParse({ ...validCreateBase, password: "Ab1!" }).success + tool.opts.inputSchema?.safeParse({ ...validCreateBase, password: "Ab1!" }).success ).toBe(false); }); it("create_storage_box: accepts compliant password (Correct$Horse7)", () => { - const tool = captureWithSchema().find((t) => t.name === "hetzner_create_storage_box")!; + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_create_storage_box")!; expect( - tool.opts.inputSchema.safeParse({ ...validCreateBase, password: "Correct$Horse7" }).success + tool.opts.inputSchema?.safeParse({ ...validCreateBase, password: "Correct$Horse7" }).success ).toBe(true); }); // hetzner_reset_storage_box_password — password it("reset_password: rejects password with only lowercase (aaaaaaaaaaaa)", () => { - const tool = captureWithSchema().find((t) => t.name === "hetzner_reset_storage_box_password")!; + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_reset_storage_box_password")!; expect( - tool.opts.inputSchema.safeParse({ id: 1, password: "aaaaaaaaaaaa" }).success + tool.opts.inputSchema?.safeParse({ id: 1, password: "aaaaaaaaaaaa" }).success ).toBe(false); }); it("reset_password: rejects password missing uppercase (correct$horse7)", () => { - const tool = captureWithSchema().find((t) => t.name === "hetzner_reset_storage_box_password")!; + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_reset_storage_box_password")!; expect( - tool.opts.inputSchema.safeParse({ id: 1, password: "correct$horse7" }).success + tool.opts.inputSchema?.safeParse({ id: 1, password: "correct$horse7" }).success ).toBe(false); }); it("reset_password: accepts compliant password (NewP@ss123Word)", () => { - const tool = captureWithSchema().find((t) => t.name === "hetzner_reset_storage_box_password")!; + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_reset_storage_box_password")!; expect( - tool.opts.inputSchema.safeParse({ id: 1, password: "NewP@ss123Word" }).success + tool.opts.inputSchema?.safeParse({ id: 1, password: "NewP@ss123Word" }).success ).toBe(true); }); }); From 23aad438b898bdf0232a119192abb2b17b624ef8 Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 26 May 2026 13:39:49 +0800 Subject: [PATCH 09/35] fix(security): replace snapshot_id denylist with allowlist regex to block percent-encoded traversal The previous .refine() denylist blocked literal "/", "\", ".." but allowed percent-encoded variants (%2f, %5c, %2e%2e) which downstream URL parsers can decode into path separators. Switch to an allowlist regex /^[A-Za-z0-9._:+@-]+$/ that: - Inherently blocks all % sequences since % is not whitelisted - Covers all known Hetzner snapshot formats: numeric IDs, named snapshots, and ISO 8601 timestamps (e.g. 2024-01-15T12:00:00+01:00) Also: change inputSchema type from z.ZodTypeAny to z.ZodType in tests to comply with the no-any ESLint rule (ZodTypeAny is ZodType). Adds two regression tests for %2f and %5c encoded traversal payloads. --- src/tools/storage-boxes.ts | 6 +++--- tests/tools/storage-boxes.test.ts | 16 +++++++++++++++- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/tools/storage-boxes.ts b/src/tools/storage-boxes.ts index 0a20530..f0d27f9 100644 --- a/src/tools/storage-boxes.ts +++ b/src/tools/storage-boxes.ts @@ -935,9 +935,9 @@ Use access settings to configure which protocols the subaccount can use.`, snapshot_id: z .string() .min(1) - .refine( - (s) => !s.includes("/") && !s.includes("\\") && !s.includes(".."), - "snapshot_id must not contain path separators or traversal sequences" + .regex( + /^[A-Za-z0-9._:+@-]+$/, + "snapshot_id must contain only safe characters (alphanumeric, . _ : + @ -)" ) .describe("Snapshot name or numeric ID (as string)") }).strict(), diff --git a/tests/tools/storage-boxes.test.ts b/tests/tools/storage-boxes.test.ts index f2f28fc..2c2ef91 100644 --- a/tests/tools/storage-boxes.test.ts +++ b/tests/tools/storage-boxes.test.ts @@ -406,7 +406,7 @@ interface CapturedTool { opts: { annotations?: Record; description?: string; - inputSchema?: z.ZodTypeAny; + inputSchema?: z.ZodType; }; } @@ -1465,6 +1465,20 @@ describe("H-1 security: username / snapshot_id path injection prevention", () => ).toBe(false); }); + it("delete_snapshot: rejects percent-encoded slash (%2f traversal)", () => { + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_delete_storage_box_snapshot")!; + expect( + tool.opts.inputSchema?.safeParse({ id: 1, snapshot_id: "evil%2fsubpath" }).success + ).toBe(false); + }); + + it("delete_snapshot: rejects percent-encoded backslash (%5c traversal)", () => { + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_delete_storage_box_snapshot")!; + expect( + tool.opts.inputSchema?.safeParse({ id: 1, snapshot_id: "..%5c..%5cadmin" }).success + ).toBe(false); + }); + it("delete_snapshot: accepts valid snapshot_id with hyphen (snapshot-2024-01-01)", () => { const tool = captureRegisteredTools().find((t) => t.name === "hetzner_delete_storage_box_snapshot")!; expect( From 1cbae6020e5628bf6ccf7d8df81a1607b6f91cab Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 26 May 2026 14:06:38 +0800 Subject: [PATCH 10/35] fix(security): add allowlist regex to rollback snapshot parameter (H-1b) hetzner_rollback_storage_box_snapshot's `snapshot` field only had .min(1) and a blank-check refine, allowing path traversal and percent-encoded variants (%2f, %5c) that the delete_snapshot's snapshot_id already guards against. Apply the same /^[A-Za-z0-9._:+@-]+$/ allowlist regex used by snapshot_id, covering all known Hetzner snapshot formats (numeric IDs, named snapshots, ISO 8601 timestamps) while blocking all path separators including encoded. Adds 6 regression tests (3 reject, 3 accept). --- src/tools/storage-boxes.ts | 5 +++- tests/tools/storage-boxes.test.ts | 45 +++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/tools/storage-boxes.ts b/src/tools/storage-boxes.ts index f0d27f9..338b6e8 100644 --- a/src/tools/storage-boxes.ts +++ b/src/tools/storage-boxes.ts @@ -1244,7 +1244,10 @@ this tool uses the replacement \`snapshot\` field.)`, snapshot: z .string() .min(1) - .refine((s) => s.trim().length > 0, { message: "snapshot must not be blank" }) + .regex( + /^[A-Za-z0-9._:+@-]+$/, + "snapshot must contain only safe characters (alphanumeric, . _ : + @ -)" + ) .describe("Snapshot name or numeric ID (as string) to roll back to"), response_format: ResponseFormatSchema.describe("Output format: 'markdown' or 'json'") }).strict(), diff --git a/tests/tools/storage-boxes.test.ts b/tests/tools/storage-boxes.test.ts index 2c2ef91..104f688 100644 --- a/tests/tools/storage-boxes.test.ts +++ b/tests/tools/storage-boxes.test.ts @@ -1501,6 +1501,51 @@ describe("H-1 security: username / snapshot_id path injection prevention", () => }); }); +// H-1b security: rollback snapshot injection prevention +describe("H-1b security: rollback snapshot injection prevention", () => { + it("rollback_snapshot: rejects path traversal (../evil)", () => { + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_rollback_storage_box_snapshot")!; + expect( + tool.opts.inputSchema?.safeParse({ id: 1, snapshot: "../evil", response_format: "markdown" }).success + ).toBe(false); + }); + + it("rollback_snapshot: rejects forward slash (a/b)", () => { + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_rollback_storage_box_snapshot")!; + expect( + tool.opts.inputSchema?.safeParse({ id: 1, snapshot: "a/b", response_format: "markdown" }).success + ).toBe(false); + }); + + it("rollback_snapshot: rejects percent-encoded slash (%2f)", () => { + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_rollback_storage_box_snapshot")!; + expect( + tool.opts.inputSchema?.safeParse({ id: 1, snapshot: "evil%2fsubpath", response_format: "markdown" }).success + ).toBe(false); + }); + + it("rollback_snapshot: accepts numeric snapshot (12345)", () => { + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_rollback_storage_box_snapshot")!; + expect( + tool.opts.inputSchema?.safeParse({ id: 1, snapshot: "12345", response_format: "markdown" }).success + ).toBe(true); + }); + + it("rollback_snapshot: accepts named snapshot (snapshot-2024-01-01)", () => { + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_rollback_storage_box_snapshot")!; + expect( + tool.opts.inputSchema?.safeParse({ id: 1, snapshot: "snapshot-2024-01-01", response_format: "markdown" }).success + ).toBe(true); + }); + + it("rollback_snapshot: accepts ISO-style snapshot (2024-01-15T12:00:00+01:00)", () => { + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_rollback_storage_box_snapshot")!; + expect( + tool.opts.inputSchema?.safeParse({ id: 1, snapshot: "2024-01-15T12:00:00+01:00", response_format: "markdown" }).success + ).toBe(true); + }); +}); + // M-2 security: password complexity policy enforcement describe("M-2 security: password complexity policy enforcement", () => { const validCreateBase = { From faf92e334fbe88e51df4a2de67af8731c408242e Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 26 May 2026 14:18:29 +0800 Subject: [PATCH 11/35] fix(security): validate IPv4 format before passing to SSH execFile (M-1) Add regex guard /^(?:\d{1,3}\.){3}\d{1,3}$/ after resolving the server's public IP from the Hetzner API response. If the API returns an unexpected string (e.g. due to a compromised response or schema bypass), the handler returns isError before the value reaches ssh execFile arguments. Adds one regression test covering the invalid-format error path. --- src/tools/server-ssh.ts | 6 ++++++ tests/tools/server-ssh.test.ts | 15 +++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/src/tools/server-ssh.ts b/src/tools/server-ssh.ts index 583d61a..38cac0e 100644 --- a/src/tools/server-ssh.ts +++ b/src/tools/server-ssh.ts @@ -168,6 +168,12 @@ Returns used / total / available in MiB and overall usage %, plus swap state.`, isError: true }; } + if (!/^(?:\d{1,3}\.){3}\d{1,3}$/.test(ipv4)) { + return { + content: [{ type: "text", text: `Error: Resolved IPv4 address has unexpected format: ${ipv4}` }], + isError: true + }; + } // Step 2: SSH and run free -m const stdout = await sshRunner(ipv4, sshPort, sshUser, "free -m"); diff --git a/tests/tools/server-ssh.test.ts b/tests/tools/server-ssh.test.ts index 23b1e72..425da86 100644 --- a/tests/tools/server-ssh.test.ts +++ b/tests/tools/server-ssh.test.ts @@ -272,4 +272,19 @@ describe("hetzner_get_server_ram — error handling", () => { expect(result.isError).toBe(true); expect(result.content[0].text).toContain("network error"); }); + + it("returns isError when API returns non-IPv4 string for ip field", async () => { + const badIpServer = { + server: { + ...serverResponse.server, + public_net: { ipv4: { ip: "not-an-ip" }, ipv6: { ip: "2a01:4f8::1" } } + } + }; + mockedRequest.mockResolvedValueOnce(badIpServer); + + const result = await captureHandler()({ id: 1, response_format: "markdown" }); + + expect(result.isError).toBe(true); + expect(result.content[0].text).toMatch(/IPv4|invalid/i); + }); }); From d2bdb1b2b060ce7b724343fb240273326c188969 Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 26 May 2026 14:20:29 +0800 Subject: [PATCH 12/35] fix(security): explicitly serialize only storage_box+action in create_storage_box JSON (M-3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace JSON.stringify(data) with JSON.stringify({ storage_box, action }) to prevent unexpected fields (e.g. an echoed password from a future API change) from appearing in the JSON output. The Zod schema already strips unknown top-level fields in production, but the mock bypasses Zod — the test documents and enforces the intended output contract regardless of how data arrives. --- src/tools/storage-boxes.ts | 2 +- tests/tools/storage-boxes.test.ts | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/tools/storage-boxes.ts b/src/tools/storage-boxes.ts index 338b6e8..b062e16 100644 --- a/src/tools/storage-boxes.ts +++ b/src/tools/storage-boxes.ts @@ -606,7 +606,7 @@ Returns the new Storage Box and an action tracking provisioning.`, const data = await makeStorageBoxApiRequest("/storage_boxes", CreateStorageBoxResponseSchema, "POST", body); if (params.response_format === ResponseFormat.JSON) { - return { content: [{ type: "text", text: JSON.stringify(data, null, 2) }] }; + return { content: [{ type: "text", text: JSON.stringify({ storage_box: data.storage_box, action: data.action }, null, 2) }] }; } const lines = [ diff --git a/tests/tools/storage-boxes.test.ts b/tests/tools/storage-boxes.test.ts index 104f688..ae3351c 100644 --- a/tests/tools/storage-boxes.test.ts +++ b/tests/tools/storage-boxes.test.ts @@ -1032,6 +1032,30 @@ describe("hetzner_create_storage_box", () => { const tool = tools.find((t) => t.name === "hetzner_create_storage_box")!; expect(tool.opts.description).toMatch(/costs?/i); }); + + it("JSON response does not include unexpected top-level fields (e.g. echoed password)", async () => { + const tools = captureRegisteredTools(); + const handler = tools.find((t) => t.name === "hetzner_create_storage_box")!.handler; + // Simulate an API response that unexpectedly echoes back extra fields. + mockedRequest.mockResolvedValueOnce({ + storage_box: baseBox, + action: baseAction, + password: "ShouldNotAppear1!" + }); + + const result = await handler({ + storage_box_type: "bx11", + location: "fsn1", + name: "new-box", + password: "TestP@ss123!", + response_format: "json" + }); + + const parsed = JSON.parse(result.content[0].text); + expect(parsed.password).toBeUndefined(); + expect(parsed.storage_box).toBeDefined(); + expect(parsed.action).toBeDefined(); + }); }); describe("hetzner_update_storage_box", () => { From fa284889a08b620b2f4d01e55a89b36f3e5045a7 Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 26 May 2026 14:22:57 +0800 Subject: [PATCH 13/35] fix(security): escape HTML special chars in label key/value Markdown output (L-2) Add escapeHtml() helper to storage-boxes.ts, servers.ts, and ssh-keys.ts, and apply it to label rendering in formatSnapshot, formatServer, and formatSSHKey. Prevents '; + const SAFE = '<script>alert(1)</script>'; + + it("hetzner_get_server escapes server.name in markdown output", async () => { + const tools = captureRegisteredTools(); + const handler = tools.find((t) => t.name === "hetzner_get_server")!.handler; + mockedRequest.mockResolvedValueOnce({ server: { ...baseServer, 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); + }); + + it("hetzner_get_server escapes server.image.name in markdown output", async () => { + const tools = captureRegisteredTools(); + const handler = tools.find((t) => t.name === "hetzner_get_server")!.handler; + mockedRequest.mockResolvedValueOnce({ + server: { ...baseServer, image: { ...baseServer.image, name: '' } } + }); + const result = await handler({ id: 1, response_format: "markdown" }); + expect(result.content[0].text).not.toContain(''); + expect(result.content[0].text).toContain('<evil-image>'); + }); +}); diff --git a/tests/tools/ssh-keys.test.ts b/tests/tools/ssh-keys.test.ts index 49500d0..5f98d1b 100644 --- a/tests/tools/ssh-keys.test.ts +++ b/tests/tools/ssh-keys.test.ts @@ -170,3 +170,18 @@ describe("hetzner_list_ssh_keys — edge cases", () => { expect(result.success).toBe(true); }); }); + +// L-2b security: HTML escaping in non-label fields of formatSSHKey +describe("L-2b security: HTML escaping in formatSSHKey non-label fields", () => { + const XSS = ''; + const SAFE = '<script>alert(1)</script>'; + + it("hetzner_get_ssh_key escapes key.name in markdown output", async () => { + const tools = captureRegisteredTools(); + const handler = tools.find((t) => t.name === "hetzner_get_ssh_key")!.handler; + mockedRequest.mockResolvedValueOnce({ ssh_key: { ...baseKey, 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); + }); +}); diff --git a/tests/tools/storage-boxes.test.ts b/tests/tools/storage-boxes.test.ts index 9e4c155..de069fd 100644 --- a/tests/tools/storage-boxes.test.ts +++ b/tests/tools/storage-boxes.test.ts @@ -1679,3 +1679,51 @@ describe("M-2 security: password complexity policy enforcement", () => { ).toBe(true); }); }); + +// L-2b security: HTML escaping in non-label fields of format functions +describe("L-2b security: HTML escaping in non-label fields", () => { + const XSS = ''; + const SAFE = '<script>alert(1)</script>'; + + it("formatStorageBox escapes box.name in heading", () => { + const out = formatStorageBox({ ...baseBox, name: XSS }); + expect(out).not.toContain(XSS); + expect(out).toContain(SAFE); + }); + + it("formatStorageBox escapes box.username", () => { + const out = formatStorageBox({ ...baseBox, username: 'user' }); + expect(out).not.toContain('user'); + expect(out).toContain('<b>user</b>'); + }); + + it("formatSubaccount escapes sub.username in heading", () => { + const out = formatSubaccount({ ...baseSubaccount, username: XSS }); + expect(out).not.toContain(XSS); + expect(out).toContain(SAFE); + }); + + it("formatSubaccount escapes sub.home_directory", () => { + const out = formatSubaccount({ ...baseSubaccount, home_directory: '/home/' }); + expect(out).not.toContain(''); + expect(out).toContain('<evil>'); + }); + + it("formatSubaccount escapes sub.comment", () => { + const out = formatSubaccount({ ...baseSubaccount, comment: '' }); + expect(out).not.toContain(' { + const out = formatSnapshot({ ...baseSnapshot, name: XSS }); + expect(out).not.toContain(XSS); + expect(out).toContain(SAFE); + }); + + it("formatSnapshot escapes snap.description", () => { + const out = formatSnapshot({ ...baseSnapshot, description: 'desc' }); + expect(out).not.toContain('desc'); + expect(out).toContain('<b>desc</b>'); + }); +}); From fd5baeeee868320603e724df66ec4f8332b5edec Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 26 May 2026 14:56:41 +0800 Subject: [PATCH 17/35] fix(security): apply escapeHtml to non-label fields in all format functions (L-2b) Applied escapeHtml() to user-controlled API-returned fields that were previously interpolated raw into Markdown output: - storage-boxes.ts: box.name, box.username, box.server, sub.username, sub.home_directory, sub.comment, snap.name, snap.description - servers.ts: server.name, server.image.name, datacenter city/country/name - ssh-keys.ts: key.name Label key/value pairs were already escaped in a prior commit; this covers all remaining heading-level and attribute fields. 289 tests pass. --- src/tools/servers.ts | 6 +++--- src/tools/ssh-keys.ts | 2 +- src/tools/storage-boxes.ts | 16 ++++++++-------- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/tools/servers.ts b/src/tools/servers.ts index e23ea63..6d2d6ae 100644 --- a/src/tools/servers.ts +++ b/src/tools/servers.ts @@ -37,16 +37,16 @@ function formatServer(server: HetznerServer): string { const ipv6 = server.public_net.ipv6?.ip || "N/A"; const lines = [ - `## ${server.name} (ID: ${server.id})`, + `## ${escapeHtml(server.name)} (ID: ${server.id})`, `- **Status**: ${server.status}`, `- **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**: ${server.datacenter.location.city}, ${server.datacenter.location.country} (${server.datacenter.name})` + `- **Location**: ${escapeHtml(server.datacenter.location.city)}, ${escapeHtml(server.datacenter.location.country)} (${escapeHtml(server.datacenter.name)})` ]; if (server.image) { - lines.push(`- **Image**: ${server.image.name} (${server.image.os_flavor} ${server.image.os_version})`); + lines.push(`- **Image**: ${escapeHtml(server.image.name)} (${server.image.os_flavor} ${server.image.os_version})`); } lines.push(`- **Created**: ${new Date(server.created).toLocaleString()}`); diff --git a/src/tools/ssh-keys.ts b/src/tools/ssh-keys.ts index 45d2551..9eb612c 100644 --- a/src/tools/ssh-keys.ts +++ b/src/tools/ssh-keys.ts @@ -33,7 +33,7 @@ function escapeHtml(s: string): string { function formatSSHKey(key: HetznerSSHKey): string { const lines = [ - `## ${key.name} (ID: ${key.id})`, + `## ${escapeHtml(key.name)} (ID: ${key.id})`, `- **Fingerprint**: ${key.fingerprint}`, `- **Created**: ${new Date(key.created).toLocaleString()}` ]; diff --git a/src/tools/storage-boxes.ts b/src/tools/storage-boxes.ts index ac8db6f..7b51faf 100644 --- a/src/tools/storage-boxes.ts +++ b/src/tools/storage-boxes.ts @@ -78,12 +78,12 @@ export function formatStorageBox(box: HetznerStorageBox): string { ? Math.round((box.stats.size_data / totalSize) * 100) : 0; return [ - `## ${box.name} (ID: ${box.id})`, - `- **Username**: ${box.username}`, + `## ${escapeHtml(box.name)} (ID: ${box.id})`, + `- **Username**: ${escapeHtml(box.username)}`, `- **Status**: ${box.status}`, `- **Type**: ${box.storage_box_type.name}`, `- **Location**: ${box.location.name}`, - `- **Server**: ${box.server ?? "—"}`, + `- **Server**: ${box.server ? escapeHtml(box.server) : "—"}`, `- **Storage**: ${formatBytes(box.stats.size_data)} used / ${formatBytes(totalSize)} total (~${usagePercent}% used)`, `- **Snapshots**: ${formatBytes(box.stats.size_snapshots)}`, `- **Protocols**: ${protocols}`, @@ -99,15 +99,15 @@ export function formatSubaccount(sub: HetznerStorageBoxSubaccount): string { .join(", ") || "none"; const lines: string[] = [ - `## ${sub.username}`, - `- **Home directory**: ${sub.home_directory}`, + `## ${escapeHtml(sub.username)}`, + `- **Home directory**: ${escapeHtml(sub.home_directory)}`, `- **Protocols**: ${protocols}`, `- **External reachability**: ${sub.external_reachability ? "yes" : "no"}`, `- **Read-only**: ${sub.readonly ? "yes" : "no"}` ]; if (sub.comment) { - lines.push(`- **Comment**: ${sub.comment}`); + lines.push(`- **Comment**: ${escapeHtml(sub.comment)}`); } return lines.join("\n"); @@ -116,11 +116,11 @@ export function formatSubaccount(sub: HetznerStorageBoxSubaccount): string { // Exported for unit testing. export function formatSnapshot(snap: HetznerStorageBoxSnapshot): string { const lines: string[] = [ - `## ${snap.name} (ID: ${snap.id})`, + `## ${escapeHtml(snap.name)} (ID: ${snap.id})`, `- **Created**: ${snap.created.slice(0, 10)}` ]; if (snap.description) { - lines.push(`- **Description**: ${snap.description}`); + lines.push(`- **Description**: ${escapeHtml(snap.description)}`); } if (snap.stats?.size !== undefined) { lines.push(`- **Size**: ${formatBytes(snap.stats.size)}`); From 9d44b285176472c6cfc0d28557668e416c7d7b46 Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 26 May 2026 14:57:09 +0800 Subject: [PATCH 18/35] test: add reproducer for IPv4 octet range validation gap (999.0.0.1 passes old regex) (RED) --- tests/tools/server-ssh.test.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/tools/server-ssh.test.ts b/tests/tools/server-ssh.test.ts index 425da86..1728b80 100644 --- a/tests/tools/server-ssh.test.ts +++ b/tests/tools/server-ssh.test.ts @@ -287,4 +287,19 @@ describe("hetzner_get_server_ram — error handling", () => { expect(result.isError).toBe(true); expect(result.content[0].text).toMatch(/IPv4|invalid/i); }); + + it("returns isError when API returns IP with out-of-range octet (999.0.0.1)", async () => { + const badIpServer = { + server: { + ...serverResponse.server, + public_net: { ipv4: { ip: "999.0.0.1" }, ipv6: { ip: "2a01:4f8::1" } } + } + }; + mockedRequest.mockResolvedValueOnce(badIpServer); + + const result = await captureHandler()({ id: 1, response_format: "markdown" }); + + expect(result.isError).toBe(true); + expect(result.content[0].text).toMatch(/IPv4|unexpected format/i); + }); }); From fe16c7e96202bdfe634aa7ac0ec4d130289e39fb Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 26 May 2026 14:57:37 +0800 Subject: [PATCH 19/35] fix(security): tighten IPv4 validation regex to enforce octet range 0-255 (L-5) Old regex /^(?:\d{1,3}\.){3}\d{1,3}$/ accepted invalid addresses like '999.0.0.1' since \d{1,3} only checks digit count, not value range. New per-octet alternation enforces 0-255: 25[0-5] | 2[0-4]\d | [01]?\d\d? A numerically invalid IP returned by the API now triggers the early 'unexpected format' error instead of silently failing at SSH connect time. --- src/tools/server-ssh.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/server-ssh.ts b/src/tools/server-ssh.ts index 38cac0e..c881875 100644 --- a/src/tools/server-ssh.ts +++ b/src/tools/server-ssh.ts @@ -168,7 +168,7 @@ Returns used / total / available in MiB and overall usage %, plus swap state.`, isError: true }; } - if (!/^(?:\d{1,3}\.){3}\d{1,3}$/.test(ipv4)) { + if (!/^(?:(?:25[0-5]|2[0-4]\d|[01]?\d\d?)\.){3}(?:25[0-5]|2[0-4]\d|[01]?\d\d?)$/.test(ipv4)) { return { content: [{ type: "text", text: `Error: Resolved IPv4 address has unexpected format: ${ipv4}` }], isError: true From c0e85527879da66e97c8d8791436aa081b682f93 Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 26 May 2026 14:58:11 +0800 Subject: [PATCH 20/35] test: add reproducer for image regex rejecting uppercase custom images (RED) --- tests/tools/servers.test.ts | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/tools/servers.test.ts b/tests/tools/servers.test.ts index 9a0e8fe..89eb741 100644 --- a/tests/tools/servers.test.ts +++ b/tests/tools/servers.test.ts @@ -212,6 +212,27 @@ describe("L-3 security: create_server body field character validation", () => { tool.opts.inputSchema?.safeParse({ name: "test", server_type: "cx22", image: "ubuntu-24.04" }).success ).toBe(true); }); + + it("accepts uppercase custom image name (Ubuntu-Hardened-2024)", () => { + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_create_server")!; + expect( + tool.opts.inputSchema?.safeParse({ name: "test", server_type: "cx22", image: "Ubuntu-Hardened-2024" }).success + ).toBe(true); + }); + + it("accepts numeric image ID string (12345)", () => { + const tool = captureRegisteredTools().find((t) => t.name === "hetzner_create_server")!; + expect( + tool.opts.inputSchema?.safeParse({ name: "test", server_type: "cx22", image: "12345" }).success + ).toBe(true); + }); + + it("still rejects image with injection characters (ubuntu + + + From 365833335180d8310c89e3816d73e01e598bf7b0 Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 26 May 2026 16:11:37 +0800 Subject: [PATCH 26/35] =?UTF-8?q?feat:=20add=20i18n=20support=20(=E7=B9=81?= =?UTF-8?q?=E9=AB=94=E4=B8=AD=E6=96=87,=20=E6=97=A5=E6=9C=AC=E8=AA=9E,=20?= =?UTF-8?q?=ED=95=9C=EA=B5=AD=EC=96=B4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/index.html | 362 +++++++++++++++++++++++++++++------------------- 1 file changed, 222 insertions(+), 140 deletions(-) diff --git a/docs/index.html b/docs/index.html index 7a9412d..a46a1fb 100644 --- a/docs/index.html +++ b/docs/index.html @@ -30,13 +30,35 @@ a { color: var(--blue); text-decoration: none; } a:hover { text-decoration: underline; } - /* ── Layout ── */ .container { max-width: 860px; margin: 0 auto; padding: 0 24px; } + /* ── Lang switcher ── */ + .lang-bar { + display: flex; + justify-content: flex-end; + gap: 4px; + padding: 10px 24px; + border-bottom: 1px solid var(--border); + max-width: 860px; + margin: 0 auto; + } + .lang-btn { + background: none; + border: 1px solid transparent; + border-radius: 5px; + color: var(--muted); + cursor: pointer; + font-size: 12px; + padding: 3px 9px; + transition: color 0.15s, border-color 0.15s; + } + .lang-btn:hover { color: var(--text); border-color: var(--border); } + .lang-btn.active { color: var(--text); border-color: var(--blue); } + /* ── Hero ── */ header { border-bottom: 1px solid var(--border); - padding: 64px 0 48px; + padding: 56px 0 44px; text-align: center; } @@ -52,27 +74,12 @@ color: var(--muted); margin-bottom: 24px; } - .badge .dot { - width: 8px; height: 8px; - border-radius: 50%; - background: var(--green); - display: inline-block; - } + .badge .dot { width: 8px; height: 8px; border-radius: 50%; background: var(--green); display: inline-block; } - h1 { - font-size: clamp(28px, 5vw, 42px); - font-weight: 700; - letter-spacing: -0.5px; - margin-bottom: 16px; - } + h1 { font-size: clamp(28px, 5vw, 42px); font-weight: 700; letter-spacing: -0.5px; margin-bottom: 16px; } h1 span { color: var(--accent); } - .tagline { - font-size: 18px; - color: var(--muted); - max-width: 560px; - margin: 0 auto 32px; - } + .tagline { font-size: 18px; color: var(--muted); max-width: 560px; margin: 0 auto 32px; } .install-box { display: inline-flex; @@ -92,13 +99,7 @@ .install-box .copy-icon { color: var(--muted); font-size: 14px; user-select: none; } .install-box.copied .copy-icon { color: var(--green); } - .hero-links { - margin-top: 24px; - display: flex; - justify-content: center; - gap: 16px; - flex-wrap: wrap; - } + .hero-links { margin-top: 24px; display: flex; justify-content: center; gap: 16px; flex-wrap: wrap; } .btn { display: inline-flex; align-items: center; @@ -127,29 +128,15 @@ overflow: hidden; margin: 48px 0; } - .stat { - background: var(--surface); - padding: 20px; - text-align: center; - } + .stat { background: var(--surface); padding: 20px; text-align: center; } .stat .num { font-size: 28px; font-weight: 700; color: var(--text); } .stat .label { font-size: 13px; color: var(--muted); margin-top: 2px; } /* ── Tool categories ── */ .section { margin: 48px 0; } - h2 { - font-size: 20px; - font-weight: 600; - margin-bottom: 20px; - padding-bottom: 8px; - border-bottom: 1px solid var(--border); - } + h2 { font-size: 20px; font-weight: 600; margin-bottom: 20px; padding-bottom: 8px; border-bottom: 1px solid var(--border); } - .categories { - display: grid; - grid-template-columns: repeat(auto-fill, minmax(240px, 1fr)); - gap: 12px; - } + .categories { display: grid; grid-template-columns: repeat(auto-fill, minmax(240px, 1fr)); gap: 12px; } .category { background: var(--surface); border: 1px solid var(--border); @@ -158,28 +145,11 @@ transition: border-color 0.15s; } .category:hover { border-color: var(--muted); } - .category-header { - display: flex; - align-items: center; - justify-content: space-between; - margin-bottom: 8px; - } + .category-header { display: flex; align-items: center; justify-content: space-between; margin-bottom: 8px; } .category-name { font-weight: 600; font-size: 14px; } - .category-count { - background: #21262d; - border: 1px solid var(--border); - border-radius: 20px; - padding: 1px 8px; - font-size: 12px; - color: var(--muted); - } + .category-count { background: #21262d; border: 1px solid var(--border); border-radius: 20px; padding: 1px 8px; font-size: 12px; color: var(--muted); } .category-desc { font-size: 13px; color: var(--muted); line-height: 1.5; } - .tool-list { - margin-top: 10px; - display: flex; - flex-wrap: wrap; - gap: 4px; - } + .tool-list { margin-top: 10px; display: flex; flex-wrap: wrap; gap: 4px; } .tool-tag { font-family: "SFMono-Regular", Consolas, monospace; font-size: 11px; @@ -192,24 +162,12 @@ .destructive { color: var(--accent); } /* ── Config snippet ── */ - .code-block { - background: var(--surface); - border: 1px solid var(--border); - border-radius: 8px; - overflow: auto; - } - .code-block pre { - padding: 20px; - font-family: "SFMono-Regular", Consolas, monospace; - font-size: 13px; - line-height: 1.6; - } + .code-block { background: var(--surface); border: 1px solid var(--border); border-radius: 8px; overflow: auto; } + .code-block pre { padding: 20px; font-family: "SFMono-Regular", Consolas, monospace; font-size: 13px; line-height: 1.6; } .code-block .key { color: #79c0ff; } - .code-block .val { color: #a5d6ff; } .code-block .str { color: #a5d6ff; } .code-block .comment { color: var(--muted); } - /* ── Token note ── */ .callout { background: #161b22; border: 1px solid #30363d; @@ -223,44 +181,38 @@ .callout strong { color: var(--text); } /* ── Footer ── */ - footer { - border-top: 1px solid var(--border); - padding: 32px 0; - text-align: center; - font-size: 13px; - color: var(--muted); - margin-top: 64px; - } + footer { border-top: 1px solid var(--border); padding: 32px 0; text-align: center; font-size: 13px; color: var(--muted); margin-top: 64px; } footer a { color: var(--muted); } footer a:hover { color: var(--text); } + +
+
+ + + + +
+
+
-
- - v1.3.3 on npm -
+
v1.3.3 on npm

Hetzner MCP Server

-

Give Claude Code 40 tools to manage your Hetzner Cloud infrastructure — from your chat window.

+

Give Claude Code 40 tools to manage your Hetzner Cloud infrastructure — from your chat window.

npm install -g @jurislm/hetzner-mcp - ⎘ Copy + ⎘ Copy
@@ -268,25 +220,23 @@

Hetzner MCP Server

-
-
40
Tools
-
7
Categories
-
2
API Endpoints
-
stdio
Transport
+
40
Tools
+
7
Categories
+
2
API Endpoints
+
stdio
Transport
-
-

Tools

+

Tools

- Servers + Servers 7
-

Create, power on/off/reboot, and delete cloud servers.

+

Create, power on/off/reboot, and delete cloud servers.

list_servers get_server @@ -300,10 +250,10 @@

Tools

- SSH Keys + SSH Keys 4
-

Manage SSH public keys for server authentication.

+

Manage SSH public keys for server authentication.

list_ssh_keys get_ssh_key @@ -314,10 +264,10 @@

Tools

- Cloud Volumes + Cloud Volumes 4
-

Attach and detach persistent block storage volumes.

+

Attach and detach persistent block storage volumes.

list_volumes get_volume @@ -328,10 +278,10 @@

Tools

- Metrics & RAM + Metrics & RAM 2
-

CPU, disk I/O, network metrics — and RAM via SSH (the Metrics API doesn't expose memory).

+

CPU, disk I/O, network metrics — and RAM via SSH (the Metrics API doesn't expose memory).

get_server_metrics get_server_ram @@ -340,10 +290,10 @@

Tools

- Reference + Reference 3
-

Browse available server types, OS images, and datacenter locations.

+

Browse available server types, OS images, and datacenter locations.

list_server_types list_images @@ -353,10 +303,10 @@

Tools

- Storage Boxes + Storage Boxes 20
-

Full CRUD for Hetzner Storage Boxes — subaccounts, snapshots, access settings, and scheduled backup plans. Requires a unified API token.

+

Full CRUD for Hetzner Storage Boxes — subaccounts, snapshots, access settings, and scheduled backup plans. Requires a unified API token.

list / get / create update / delete @@ -369,10 +319,9 @@

Tools

-
-

Quick Setup

-

Add to ~/.claude.json (run /mcp in Claude Code to confirm the path):

+

Quick Setup

+

Add to ~/.claude.json (run /mcp in Claude Code to confirm the path):

{
   "mcpServers": {
@@ -382,7 +331,7 @@ 

Quick Setup

"args": ["@jurislm/hetzner-mcp"], "env": { "HETZNER_API_TOKEN": "your-api-token", - // Storage Boxes only — unified token from console.hetzner.com + // Storage Boxes only — unified token from console.hetzner.com "HETZNER_API_TOKEN_UNIFIED": "your-unified-token" } } @@ -390,11 +339,12 @@

Quick Setup

}
- Two token types: All tools except Storage Boxes use a Cloud project token from - console.hetzner.cloud. + Two token types: + All tools except Storage Boxes use a Cloud project token from + console.hetzner.cloud. Storage Box tools use a unified token from - console.hetzner.com. - A Cloud token on the unified API returns 401. + console.hetzner.com. + A Cloud token on the unified API returns 401.
@@ -405,25 +355,157 @@

Quick Setup

jurislm/hetzner-mcp -  ·  - MIT License -  ·  +  ·  MIT License  ·  @jurislm/hetzner-mcp on npm

From bbfd8853125080535e8668a0f4378c29c4dce7ed Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 26 May 2026 16:18:46 +0800 Subject: [PATCH 27/35] =?UTF-8?q?docs:=20explain=20=E2=9A=A0=20legend=20in?= =?UTF-8?q?=20README=20and=20landing=20page=20(all=204=20languages)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 2 ++ docs/index.html | 5 +++++ 2 files changed, 7 insertions(+) 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 키", From 90f1bb843adb8502d563e000a3fe4005ed6f9d60 Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 26 May 2026 17:02:22 +0800 Subject: [PATCH 28/35] fix(security): address 9 security findings from automated audit (HIGH/MEDIUM/LOW) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit H-1: extract formatStartupError() in index.ts — prevent raw AxiosError (with Authorization header) from being logged via console.error on startup crash H-2: add expected_fingerprint parameter + runSshKeyScan() DI to server-ssh tool — gives callers a way to verify host key fingerprint before connecting and prevent TOFU MITM attacks on first connection M-1/M-4: add .max(256)/.max(255) and character-allowlist regex to all filter params (label_selector, name, username) in list tools across servers.ts, volumes.ts, storage-boxes.ts — consistent with mutation endpoints; caps URL length and blocks anomalous query strings M-2: apply escapeHtml() to formatVolume() fields (name, location, labels) in volumes.ts and to folder names in storage-boxes folder listing — matches the existing convention in servers.ts / ssh-keys.ts M-3: add MCP-plaintext password warning to hetzner_create_storage_box and hetzner_reset_storage_box_password tool descriptions L-1: note in hetzner_create_server description that JSON mode returns root_password in plaintext; advise against logging full JSON output L-2: add @internal JSDoc to __resetClientsForTesting in api.ts L-3: add inline comment on SSH output footer explaining that interpolation is safe because all three values are validated by strict Zod schemas Tests: 322 tests passing; 25 new tests added (TDD red-green for each fix) --- src/api.ts | 8 ++- src/index.ts | 12 +++- src/tools/server-ssh.ts | 84 ++++++++++++++++++++--- src/tools/servers.ts | 5 +- src/tools/storage-boxes.ts | 14 ++-- src/tools/volumes.ts | 25 ++++--- tests/index.test.ts | 35 ++++++++++ tests/tools/server-ssh.test.ts | 108 ++++++++++++++++++++++++++++-- tests/tools/servers.test.ts | 34 ++++++++++ tests/tools/storage-boxes.test.ts | 66 ++++++++++++++++++ tests/tools/volumes.test.ts | 78 +++++++++++++++++++++ 11 files changed, 436 insertions(+), 33 deletions(-) create mode 100644 tests/index.test.ts 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..b027bf7 100644 --- a/src/index.ts +++ b/src/index.ts @@ -32,6 +32,14 @@ registerVolumeTools(server); registerMetricsTools(server); registerServerSshTools(server); +/** 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); +} + // Main function async function main(): Promise { // Check for API token @@ -54,7 +62,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..3ae9334 100644 --- a/src/tools/server-ssh.ts +++ b/src/tools/server-ssh.ts @@ -4,6 +4,45 @@ import { z } from "zod"; import { makeApiRequest, handleApiError } from "../api.js"; import { ResponseFormat, ResponseFormatSchema, GetServerResponseSchema } from "../types.js"; +/** + * Resolves the SHA256 fingerprint of a host's SSH key via ssh-keyscan + ssh-keygen. + * 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 + 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; + } + // Step 2: compute fingerprint from the raw key 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; } + const match = keygenOut.match(/SHA256:[A-Za-z0-9+/]+/); + if (!match) { + reject(new Error(`Could not parse fingerprint from: ${keygenOut.trim()}`)); + return; + } + resolve(match[0]); + } + ); + proc.stdin?.write(rawKey + "\n"); + proc.stdin?.end(); + } + ); + }); +} + export interface RamStats { total: number; used: number; @@ -104,11 +143,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 +163,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 +182,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 +221,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 actualFp: string; + try { + actualFp = await keyScanRunner(ipv4, sshPort); + } catch (scanErr) { + return { + content: [{ type: "text", text: `Error: fingerprint check failed — could not run ssh-keyscan: ${scanErr instanceof Error ? scanErr.message : String(scanErr)}` }], + isError: true + }; + } + if (actualFp !== params.expected_fingerprint) { + return { + content: [{ type: "text", text: `Error: fingerprint mismatch for ${ipv4}. Expected: ${params.expected_fingerprint} — Got: ${actualFp}` }], + 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 +279,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..1c083f0 100644 --- a/src/tools/servers.ts +++ b/src/tools/servers.ts @@ -79,7 +79,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 +215,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/storage-boxes.ts b/src/tools/storage-boxes.ts index 7b51faf..caad6c6 100644 --- a/src/tools/storage-boxes.ts +++ b/src/tools/storage-boxes.ts @@ -175,8 +175,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 +313,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 +567,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 +754,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 +1064,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 diff --git a/src/tools/volumes.ts b/src/tools/volumes.ts index 9663e32..f3b8690 100644 --- a/src/tools/volumes.ts +++ b/src/tools/volumes.ts @@ -17,25 +17,34 @@ import { HetznerVolume } from "../types.js"; const CLOUD_DEFAULT_PER_PAGE = 25; + +function escapeHtml(s: string): string { + return s + .replace(/&/g, "&") + .replace(//g, ">") + .replace(/"/g, """) + .replace(/'/g, "'"); +} const TRUNCATION_NOTE = `> ⚠️ Truncated at ${PAGINATION_HARD_CAP_PAGES} pages — supply explicit \`page\` to fetch more.`; 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 +71,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/tests/index.test.ts b/tests/index.test.ts new file mode 100644 index 0000000..ba52c91 --- /dev/null +++ b/tests/index.test.ts @@ -0,0 +1,35 @@ +import { describe, it, expect } from "vitest"; +import { formatStartupError } from "../src/index.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/server-ssh.test.ts b/tests/tools/server-ssh.test.ts index 1728b80..9f87309 100644 --- a/tests/tools/server-ssh.test.ts +++ b/tests/tools/server-ssh.test.ts @@ -5,17 +5,19 @@ vi.mock("../../src/api.js", async (importOriginal) => { return { ...actual, makeApiRequest: vi.fn() }; }); -import { parseFreeOutput, registerServerSshTools, runSsh } from "../../src/tools/server-ssh.js"; +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"; 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(); }); // ── Fixtures ────────────────────────────────────────────────────────────────── @@ -61,21 +63,31 @@ 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; +} + // ── parseFreeOutput — pure unit tests ───────────────────────────────────────── describe("parseFreeOutput", () => { @@ -303,3 +315,91 @@ 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(); + }); +}); diff --git a/tests/tools/servers.test.ts b/tests/tools/servers.test.ts index 89eb741..4bd2227 100644 --- a/tests/tools/servers.test.ts +++ b/tests/tools/servers.test.ts @@ -260,3 +260,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..8aa727a 100644 --- a/tests/tools/storage-boxes.test.ts +++ b/tests/tools/storage-boxes.test.ts @@ -1727,3 +1727,69 @@ 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); + }); +}); diff --git a/tests/tools/volumes.test.ts b/tests/tools/volumes.test.ts index b695092..bfc351e 100644 --- a/tests/tools/volumes.test.ts +++ b/tests/tools/volumes.test.ts @@ -330,3 +330,81 @@ 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); + }); +}); From 86e1512d5ed57e8a12f0ad5509d46e6d6c19249b Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 26 May 2026 17:20:45 +0800 Subject: [PATCH 29/35] fix: address 5 code-review findings in PR #42 Findings from code-review of the security-review PR: - Finding #1 (HIGH): runSshKeyScan now returns string[] of ALL key-type fingerprints; handler uses .includes() so any matching key type passes - Finding #2 (HIGH): extraction regex updated to /SHA256:[A-Za-z0-9+/]+=*/g preserving base64 padding '=' chars; Zod schema already allowed '=' - Finding #3 (MEDIUM): ssh-keyscan non-zero exit now rejected even when partial stdout is present, preventing corrupt key material reaching ssh-keygen - Finding #4 (MEDIUM): formatStartupError moved to src/utils.ts so tests/index.test.ts no longer imports src/index.ts and avoids the top-level main().catch(process.exit) side-effect in CI - Finding #6 (LOW): escapeHtml in volumes.ts now uses ' (matches servers.ts and storage-boxes.ts) TDD: 8 new tests added (330 total), all passing. --- src/index.ts | 9 +-- src/tools/server-ssh.ts | 29 +++++---- src/tools/volumes.ts | 2 +- src/utils.ts | 7 +++ tests/index.test.ts | 2 +- tests/tools/server-ssh.test.ts | 107 ++++++++++++++++++++++++++++++++- tests/tools/volumes.test.ts | 16 +++++ 7 files changed, 148 insertions(+), 24 deletions(-) create mode 100644 src/utils.ts diff --git a/src/index.ts b/src/index.ts index b027bf7..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({ @@ -32,14 +33,6 @@ registerVolumeTools(server); registerMetricsTools(server); registerServerSshTools(server); -/** 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); -} - // Main function async function main(): Promise { // Check for API token diff --git a/src/tools/server-ssh.ts b/src/tools/server-ssh.ts index 3ae9334..e71d6d4 100644 --- a/src/tools/server-ssh.ts +++ b/src/tools/server-ssh.ts @@ -5,12 +5,13 @@ import { makeApiRequest, handleApiError } from "../api.js"; import { ResponseFormat, ResponseFormatSchema, GetServerResponseSchema } from "../types.js"; /** - * Resolves the SHA256 fingerprint of a host's SSH key via ssh-keyscan + ssh-keygen. + * 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 { +export function runSshKeyScan(host: string, port: number): Promise { return new Promise((resolve, reject) => { - // Step 1: fetch raw host key entries + // Step 1: fetch raw host key entries (all key types) execFile( "ssh-keyscan", ["-p", String(port), "-T", "10", host], @@ -21,19 +22,25 @@ export function runSshKeyScan(host: string, port: number): Promise { reject(new Error(`ssh-keyscan failed: ${scanStderr.trim() || "no output"}`)); return; } - // Step 2: compute fingerprint from the raw key via ssh-keygen -l + // 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; } - const match = keygenOut.match(/SHA256:[A-Za-z0-9+/]+/); - if (!match) { + // 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(match[0]); + resolve(matches); } ); proc.stdin?.write(rawKey + "\n"); @@ -223,18 +230,18 @@ Returns used / total / available in MiB and overall usage %, plus swap state.`, // Step 2: verify host fingerprint if caller supplied one if (params.expected_fingerprint) { - let actualFp: string; + let actualFps: string[]; try { - actualFp = await keyScanRunner(ipv4, sshPort); + actualFps = await keyScanRunner(ipv4, sshPort); } catch (scanErr) { return { content: [{ type: "text", text: `Error: fingerprint check failed — could not run ssh-keyscan: ${scanErr instanceof Error ? scanErr.message : String(scanErr)}` }], isError: true }; } - if (actualFp !== params.expected_fingerprint) { + if (!actualFps.includes(params.expected_fingerprint)) { return { - content: [{ type: "text", text: `Error: fingerprint mismatch for ${ipv4}. Expected: ${params.expected_fingerprint} — Got: ${actualFp}` }], + content: [{ type: "text", text: `Error: fingerprint mismatch for ${ipv4}. Expected: ${params.expected_fingerprint} — Got: ${actualFps.join(", ")}` }], isError: true }; } diff --git a/src/tools/volumes.ts b/src/tools/volumes.ts index f3b8690..c3e6e03 100644 --- a/src/tools/volumes.ts +++ b/src/tools/volumes.ts @@ -24,7 +24,7 @@ function escapeHtml(s: string): string { .replace(//g, ">") .replace(/"/g, """) - .replace(/'/g, "'"); + .replace(/'/g, "'"); } const TRUNCATION_NOTE = `> ⚠️ Truncated at ${PAGINATION_HARD_CAP_PAGES} pages — supply explicit \`page\` to fetch more.`; diff --git a/src/utils.ts b/src/utils.ts new file mode 100644 index 0000000..0784f24 --- /dev/null +++ b/src/utils.ts @@ -0,0 +1,7 @@ +/** 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); +} diff --git a/tests/index.test.ts b/tests/index.test.ts index ba52c91..81c6d73 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from "vitest"; -import { formatStartupError } from "../src/index.js"; +import { formatStartupError } from "../src/utils.js"; import { AxiosError, AxiosHeaders } from "axios"; describe("formatStartupError", () => { diff --git a/tests/tools/server-ssh.test.ts b/tests/tools/server-ssh.test.ts index 9f87309..8b48ddb 100644 --- a/tests/tools/server-ssh.test.ts +++ b/tests/tools/server-ssh.test.ts @@ -5,9 +5,14 @@ vi.mock("../../src/api.js", async (importOriginal) => { return { ...actual, makeApiRequest: vi.fn() }; }); +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. @@ -18,6 +23,7 @@ beforeEach(() => { mockedRequest.mockReset(); mockSsh.mockReset(); mockKeyScan.mockReset(); + mockExecFile.mockReset(); }); // ── Fixtures ────────────────────────────────────────────────────────────────── @@ -88,6 +94,68 @@ function captureToolOpts(): { description: string; inputSchema: { shape: Record< 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", () => { @@ -345,7 +413,7 @@ describe("hetzner_get_server_ram — expected_fingerprint", () => { it("calls keyScanRunner with resolved IP and port when expected_fingerprint is provided", async () => { mockedRequest.mockResolvedValueOnce(serverResponse); - mockKeyScan.mockResolvedValueOnce(FAKE_FP); + mockKeyScan.mockResolvedValueOnce([FAKE_FP]); mockSsh.mockResolvedValueOnce(FREE_OUTPUT_NORMAL); await captureHandler()({ @@ -360,7 +428,7 @@ describe("hetzner_get_server_ram — expected_fingerprint", () => { it("proceeds normally when fingerprint matches", async () => { mockedRequest.mockResolvedValueOnce(serverResponse); - mockKeyScan.mockResolvedValueOnce(FAKE_FP); + mockKeyScan.mockResolvedValueOnce([FAKE_FP]); mockSsh.mockResolvedValueOnce(FREE_OUTPUT_NORMAL); const result = await captureHandler()({ @@ -375,7 +443,7 @@ describe("hetzner_get_server_ram — expected_fingerprint", () => { it("returns isError and does NOT call sshRunner when fingerprint mismatches", async () => { mockedRequest.mockResolvedValueOnce(serverResponse); - mockKeyScan.mockResolvedValueOnce(FAKE_FP); + mockKeyScan.mockResolvedValueOnce([FAKE_FP]); const result = await captureHandler()({ id: 1, @@ -402,4 +470,37 @@ describe("hetzner_get_server_ram — expected_fingerprint", () => { 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/volumes.test.ts b/tests/tools/volumes.test.ts index bfc351e..7a53278 100644 --- a/tests/tools/volumes.test.ts +++ b/tests/tools/volumes.test.ts @@ -408,3 +408,19 @@ describe("hetzner_get_volume — escapeHtml in output", () => { 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("'"); + }); +}); From b8334f6e0e455bc62f8f37b372b526c7aaf843ee Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 26 May 2026 17:24:26 +0800 Subject: [PATCH 30/35] refactor: centralise escapeHtml in src/utils.ts; improve error message - Move escapeHtml() from 4 tool files (servers, ssh-keys, volumes, storage-boxes) to src/utils.ts, eliminating 4 duplicates. Addresses Copilot review comment on volumes.ts escapeHtml duplication. - Improve fingerprint verification error message from "could not run ssh-keyscan" to "fingerprint verification failed" so it accurately covers ssh-keyscan, ssh-keygen, and parse failures. Addresses Copilot review comment on server-ssh.ts line 104. No behaviour change; all 330 tests pass. --- src/tools/server-ssh.ts | 2 +- src/tools/servers.ts | 10 +--------- src/tools/ssh-keys.ts | 10 +--------- src/tools/storage-boxes.ts | 10 +--------- src/tools/volumes.ts | 10 +--------- src/utils.ts | 10 ++++++++++ 6 files changed, 15 insertions(+), 37 deletions(-) diff --git a/src/tools/server-ssh.ts b/src/tools/server-ssh.ts index e71d6d4..5320a1e 100644 --- a/src/tools/server-ssh.ts +++ b/src/tools/server-ssh.ts @@ -235,7 +235,7 @@ Returns used / total / available in MiB and overall usage %, plus swap state.`, actualFps = await keyScanRunner(ipv4, sshPort); } catch (scanErr) { return { - content: [{ type: "text", text: `Error: fingerprint check failed — could not run ssh-keyscan: ${scanErr instanceof Error ? scanErr.message : String(scanErr)}` }], + content: [{ type: "text", text: `Error: fingerprint verification failed: ${scanErr instanceof Error ? scanErr.message : String(scanErr)}` }], isError: true }; } diff --git a/src/tools/servers.ts b/src/tools/servers.ts index 1c083f0..f361fb6 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"; 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 caad6c6..93036bc 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,15 +49,6 @@ 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, "'"); -} - // Exported for unit testing. export function formatBytes(bytes: number): string { const gib = bytes / (1024 ** 3); diff --git a/src/tools/volumes.ts b/src/tools/volumes.ts index c3e6e03..5595ad3 100644 --- a/src/tools/volumes.ts +++ b/src/tools/volumes.ts @@ -16,16 +16,8 @@ import { VolumeActionResponseSchema, HetznerVolume } from "../types.js"; +import { escapeHtml } from "../utils.js"; const CLOUD_DEFAULT_PER_PAGE = 25; - -function escapeHtml(s: string): string { - return s - .replace(/&/g, "&") - .replace(//g, ">") - .replace(/"/g, """) - .replace(/'/g, "'"); -} const TRUNCATION_NOTE = `> ⚠️ Truncated at ${PAGINATION_HARD_CAP_PAGES} pages — supply explicit \`page\` to fetch more.`; const paginatedFetch = createPaginatedFetch(makeApiRequest); diff --git a/src/utils.ts b/src/utils.ts index 0784f24..eb08267 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -5,3 +5,13 @@ export function formatStartupError(error: unknown): string { } 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, "'"); +} From efada080c71da5537b6d0a4bf46e49e686481169 Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 2 Jun 2026 14:16:05 +0800 Subject: [PATCH 31/35] =?UTF-8?q?ci:=20=E7=A7=BB=E9=99=A4=20claude-code-re?= =?UTF-8?q?view.yml?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/claude-code-review.yml | 200 ----------------------- 1 file changed, 200 deletions(-) delete mode 100644 .github/workflows/claude-code-review.yml 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 - ``` From 62ed41a9b14e0ea11756abcc34e5138b267940be Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Tue, 2 Jun 2026 14:16:06 +0800 Subject: [PATCH 32/35] =?UTF-8?q?ci:=20=E7=A7=BB=E9=99=A4=20claude.yml?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/claude.yml | 39 ------------------------------------ 1 file changed, 39 deletions(-) delete mode 100644 .github/workflows/claude.yml 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 From 4c23c35f77cd67087d711b67eb96630774e0ce41 Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Thu, 25 Jun 2026 09:56:02 +0800 Subject: [PATCH 33/35] =?UTF-8?q?feat(storage-boxes):=20=E6=96=B0=E5=A2=9E?= =?UTF-8?q?=20hetzner=5Fget=5Fstorage=5Fbox=5Fstats=20=E8=88=87=20hetzner?= =?UTF-8?q?=5Fassert=5Fstorage=5Fbox=5Fspace=20=E5=B7=A5=E5=85=B7?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 新增兩個 Storage Box 空間管理工具(issue #115): - hetzner_get_storage_box_stats:回傳 used_bytes/used_gib、total_bytes/total_gib、 available_gib、usage_percent(2 位小數),stats.size(data+snapshots 合計)為已用空間 - hetzner_assert_storage_box_space:接受 required_gib 參數,空間不足時回傳 isError:true, 供備份 pipeline 在執行前做 pre-flight check - 抽出 computeStorageBoxStats() 共用輔助函式(exported for testing) 344/344 tests pass;lint 0 error;tsc clean --- src/tools/storage-boxes.ts | 129 ++++++++++++++++++++++++ tests/tools/storage-boxes.test.ts | 159 +++++++++++++++++++++++++++++- 2 files changed, 286 insertions(+), 2 deletions(-) diff --git a/src/tools/storage-boxes.ts b/src/tools/storage-boxes.ts index 93036bc..f4e1798 100644 --- a/src/tools/storage-boxes.ts +++ b/src/tools/storage-boxes.ts @@ -49,6 +49,29 @@ 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[]; +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. export function formatBytes(bytes: number): string { const gib = bytes / (1024 ** 3); @@ -1231,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/tests/tools/storage-boxes.test.ts b/tests/tools/storage-boxes.test.ts index 8aa727a..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" ]); }); @@ -1793,3 +1796,155 @@ describe("password tools — MCP plaintext security warning", () => { 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); + }); +}); From 20ee2ee195362a9587fda0b0394f73cab037cafb Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Wed, 8 Jul 2026 13:30:09 +0800 Subject: [PATCH 34/35] =?UTF-8?q?fix(servers):=20=E6=94=B9=E7=94=A8=20loca?= =?UTF-8?q?tion=20=E5=8F=96=E4=BB=A3=E5=B7=B2=E8=A2=AB=20Hetzner=20?= =?UTF-8?q?=E7=A7=BB=E9=99=A4=E7=9A=84=20datacenter=20=E6=AC=84=E4=BD=8D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hetzner Cloud API 於 2026-06-30 正式從 Servers 與 Primary IPs 資源移除 `datacenter` 屬性(2025-12-16 公告 "Phasing out Datacenters in favor of Locations",https://docs.hetzner.cloud/changelog#2025-12-16-phasing-out-datacenters)。 原本 HetznerServerSchema 將 `datacenter` 列為 required,且 formatServer 讀取 `server.datacenter.location.*`,導致移除後所有 hetzner_list_servers / hetzner_get_server / metrics / ssh 相關工具一律拋出: Invalid input: expected object, received undefined (servers.0.datacenter) 改用 API 早已提供的頂層 `location` 物件,且**只宣告 formatServer 實際會渲染的 三個欄位**(name / city / country)。zod 的 z.object 預設就會剝除未宣告的多餘 欄位,因此宣告得越少對上游變更越寬容;反之,一個「宣告為必要卻從未讀取」的欄位 就是一顆定時炸彈——這次的 datacenter 正是如此。 why 不用 .passthrough():實測 zod 4.3.6,預設 z.object 對「多出來的欄位」本來 就 PASS(自動剝除),passthrough 只是改為保留;而兩者對「缺少必要欄位」一律 THROW。也就是說 passthrough 對這次的失效模式毫無防護作用,加了只會誤導。 驗證: - 346 unit tests / typecheck / lint 全綠 - 新增測試以真實 API 形狀(無 datacenter、含 latitude/longitude/network_zone 等未宣告欄位)驗證 schema 可正常解析 - runtime 實打 Hetzner API:hetzner_list_servers 與 hetzner_get_server 均 回傳 "**Location**: Nuremberg, DE (nbg1)" --- src/tools/servers.ts | 2 +- src/types.ts | 20 +++++++++------- tests/tools/metrics.test.ts | 7 +----- tests/tools/server-ssh.test.ts | 7 +----- tests/tools/servers.test.ts | 43 ++++++++++++++++++++++++++++++---- 5 files changed, 53 insertions(+), 26 deletions(-) diff --git a/src/tools/servers.ts b/src/tools/servers.ts index f361fb6..9f444c7 100644 --- a/src/tools/servers.ts +++ b/src/tools/servers.ts @@ -34,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) { 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/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 8b48ddb..d3cd812 100644 --- a/tests/tools/server-ssh.test.ts +++ b/tests/tools/server-ssh.test.ts @@ -55,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" diff --git a/tests/tools/servers.test.ts b/tests/tools/servers.test.ts index 4bd2227..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(); From 0ca4736585276367371641cd42c5bedec5ad7400 Mon Sep 17 00:00:00 2001 From: Terry Chen Date: Wed, 8 Jul 2026 16:21:38 +0800 Subject: [PATCH 35/35] chore: skip CodeRabbit review on release-please PRs --- .coderabbit.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 .coderabbit.yaml 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"