fix(servers): 改用 location 取代已被 Hetzner 移除的 datacenter 欄位 - #52
Closed
terry90918 wants to merge 39 commits into
Closed
fix(servers): 改用 location 取代已被 Hetzner 移除的 datacenter 欄位#52terry90918 wants to merge 39 commits into
terry90918 wants to merge 39 commits into
Conversation
…th params (H-1) 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.
…s (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.
…n (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.
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
… 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.
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)
…lock 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<unknown> in tests to comply with the no-any ESLint rule (ZodTypeAny is ZodType<any>). Adds two regression tests for %2f and %5c encoded traversal payloads.
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).
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.
…_storage_box JSON (M-3)
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.
…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 <script> and similar payloads from appearing unescaped in Markdown output that a MCP host might render as HTML. Escapes: & < > " '
…torage_box_type/location (L-3) Restrict to /^[a-z0-9-]+$/ (or /^[a-z0-9._-]+$/ for image) to prevent unexpected characters from reaching the Hetzner API request body. Adds early validation failure with clear error messages. Adds 8 schema tests covering reject (special chars) and accept (valid slugs).
Previous condition (=== "production") failed to throw when NODE_ENV is undefined, which is the typical state in a real MCP production process where no NODE_ENV is set in the environment. Changed to (!== "test") so the function only executes in Vitest test runs, and throws for every other context including the production MCP case. RED: added test for NODE_ENV=undefined expecting throw. GREEN: guard updated, all 279 tests pass.
…ctions (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.
…asses old regex) (RED)
…-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.
…ames Old regex /^[a-z0-9._-]+$/ rejected valid Hetzner custom image names with uppercase letters (e.g., 'Ubuntu-Hardened-2024'). All standard system images (ubuntu-24.04, debian-12, etc.) are lowercase and continue to pass. Injection characters (<, >, ;, space, $, /) remain blocked. Updated description to reflect support for uppercase and numeric IDs.
- Update package name: hetzner-mcp-server → @jurislm/hetzner-mcp - Update tool count: 20 → 40 total - Add Cloud Volumes section (4 tools) - Add Server Metrics section (1 tool) - Add Server RAM via SSH section (1 tool) - Expand Storage Boxes: 6 → 20 tools with full table - Remove 'volumes not implemented' from cannot-do list - Fix GitHub clone URL (jurislm/hetzner-mcp) - Add HETZNER_API_TOKEN_UNIFIED to config examples - Add destructive-action column to all tool tables - Update development commands to bun
The Hetzner MCP is a standalone tool — Kamal integration is a user choice, not part of this project. The feature/cost comparison tables referenced third-party tools (Hatchbox, Vercel, Render) that have no bearing on what this MCP does, adding confusion instead of clarity.
- Remove "What is MCP?" and "What is Hetzner Cloud?" tutorial sections - Remove verbose example workflow - Consolidate env var docs into a single table with inline explanation - Shrink from 452 lines to 194 lines while keeping all 40 tools documented - Fix clone/build commands: npm → bun (matches actual toolchain) - Add Capabilities and Limitations section replacing the scattered CAN/CANNOT lists
# Conflicts: # README.md
# Conflicts: # docs/index.html
# Conflicts: # docs/index.html
…/MEDIUM/LOW)
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)
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.
- 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.
ci: 移除 Claude review workflows
…t_storage_box_space 工具 新增兩個 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
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)"
📝 WalkthroughWalkthrough本次變更新增了共用的 Changes安全強化與工具改進
CodeRabbit 與 CI 設定變更
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Comment |
4 tasks
There was a problem hiding this comment.
Pull request overview
此 PR 主要針對 Hetzner Cloud API 已移除的 datacenter 欄位進行相容性修復：將 server 相關 formatter 與 schema 改為使用頂層 location，避免所有 server 工具因 Zod required 欄位缺失而在 runtime 直接拋錯。同時也包含多處輸出 HTML escaping、filter 參數長度驗證、Storage Box 新工具（stats / space assert）、以及 server-ssh 增加 expected_fingerprint 驗證等變更。
Changes:
- 將
HetznerServerSchema/formatServer()從datacenter.*改為使用頂層location.*，並新增 regression test 覆蓋「無 datacenter + 具有未知欄位」的真實 payload 形狀 - 多個工具輸出改用共用
escapeHtml()，並補上對 filter 參數的長度/格式驗證測試 - Storage Box 新增 stats 與空間檢查工具、server-ssh 新增 TOFU MITM 緩解的 expected_fingerprint 機制，以及調整部分文件/CI 設定
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/tools/volumes.test.ts | 新增 list volumes 的 filter 驗證與 markdown escaping 測試 |
| tests/tools/storage-boxes.test.ts | 更新工具數量、補 filter/escaping 測試，新增 stats/space assert 相關測試 |
| tests/tools/servers.test.ts | 改用 location fixture；新增 location rendering 與 schema regression 測試；補 filter 驗證與 root_password 警告測試 |
| tests/tools/server-ssh.test.ts | mock child_process；新增 runSshKeyScan 與 expected_fingerprint 行為測試 |
| tests/tools/metrics.test.ts | server fixture 改用 location |
| tests/index.test.ts | 新增 formatStartupError 單元測試 |
| src/utils.ts | 新增 formatStartupError 與共用 escapeHtml |
| src/types.ts | HetznerServerSchema 改為 location（僅宣告 formatter 會用到的欄位） |
| src/tools/volumes.ts | markdown 輸出欄位改 escapeHtml；filter schema 加上 max 長度限制 |
| src/tools/storage-boxes.ts | 匯入共用 escapeHtml；新增 computeStorageBoxStats 與兩個新工具；加強 filter schema；folders 輸出 escape；password 工具加 plaintext 警告 |
| src/tools/ssh-keys.ts | 移除本地 escapeHtml，改用共用 utils.escapeHtml |
| src/tools/servers.ts | 移除本地 escapeHtml，改用共用；Location 改讀取 server.location；label_selector 加長度限制；create_server 加 plaintext root_password 警告 |
| src/tools/server-ssh.ts | 新增 runSshKeyScan；registerServerSshTools 增 DI；新增 expected_fingerprint 以降低 TOFU MITM 風險 |
| src/index.ts | 啟動錯誤輸出改用 formatStartupError（unknown） |
| src/api.ts | 將 __resetClientsForTesting 說明改為 JSDoc @internal |
| README.md | 工具清單區塊新增 ⚠ 標示說明 |
| docs/index.html | Tools 區塊新增 ⚠ 標示說明與多語系字串 |
| .github/workflows/claude.yml | 移除 Claude Code workflow |
| .github/workflows/claude-code-review.yml | 移除 Claude Code Review workflow |
| .coderabbit.yaml | 新增 CodeRabbit 設定（語言/語氣/忽略 release PR） |
Comment on lines
+1322
to
+1325
| 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(), |
Comment on lines
+1338
to
+1353
| 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 | ||
| }; |
Comment on lines
+1257
to
+1261
| // Get Storage Box Stats | ||
| server.registerTool( | ||
| "hetzner_get_storage_box_stats", | ||
| { | ||
| title: "Get Storage Box Stats", |
Collaborator
Author
|
改由 |
5 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/tools/server-ssh.ts`:
- Around line 231-250: The fingerprint check in the server SSH flow is only a
preflight scan and is not tied to the actual SSH session. Update the logic
around the `keyScanRunner` / `runSsh` path so the verified host key is written
to a temporary known_hosts file, then make `runSsh` use that file with
`UserKnownHostsFile` and `StrictHostKeyChecking=yes` instead of `accept-new`.
Keep the existing fingerprint mismatch/error handling, but ensure the live SSH
connection is validated against the same verified key.
In `@src/tools/storage-boxes.ts`:
- Around line 1322-1325: `hetzner_assert_storage_box_space` 的 `inputSchema`
目前只接受 `id` 和 `required_gib`，需補上共用的 `ResponseFormatSchema`，讓介面與
`hetzner_get_storage_box_stats` 一致並符合工具規範。請在該工具的 schema 中加入 `response_format`（預設
markdown），並在對應的 handler／執行邏輯中依 `ResponseFormat.JSON` 分支回傳結構化結果。
In `@tests/tools/server-ssh.test.ts`:
- Around line 469-500: Update the test case name in server-ssh.test.ts so it
matches the actual assertion in the padded fingerprint scenario: the current
title in the padded fingerprint test suggests it should return isError, but the
check in the captureHandler test expects success. Keep the assertion as-is and
rename the test to describe that expected_fingerprint with padding matches
correctly in runSshKeyScan/captureHandler, so the intent is clear to future
readers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4703ddf2-0caf-4f5d-b7dc-44d86f323f7d
📒 Files selected for processing (20)
.coderabbit.yaml.github/workflows/claude-code-review.yml.github/workflows/claude.ymlREADME.mddocs/index.htmlsrc/api.tssrc/index.tssrc/tools/server-ssh.tssrc/tools/servers.tssrc/tools/ssh-keys.tssrc/tools/storage-boxes.tssrc/tools/volumes.tssrc/types.tssrc/utils.tstests/index.test.tstests/tools/metrics.test.tstests/tools/server-ssh.test.tstests/tools/servers.test.tstests/tools/storage-boxes.test.tstests/tools/volumes.test.ts
💤 Files with no reviewable changes (2)
- .github/workflows/claude-code-review.yml
- .github/workflows/claude.yml
Comment on lines
+231
to
+250
| // Step 2: verify host fingerprint if caller supplied one | ||
| if (params.expected_fingerprint) { | ||
| let actualFps: string[]; | ||
| try { | ||
| actualFps = await keyScanRunner(ipv4, sshPort); | ||
| } catch (scanErr) { | ||
| return { | ||
| content: [{ type: "text", text: `Error: fingerprint verification failed: ${scanErr instanceof Error ? scanErr.message : String(scanErr)}` }], | ||
| isError: true | ||
| }; | ||
| } | ||
| if (!actualFps.includes(params.expected_fingerprint)) { | ||
| return { | ||
| content: [{ type: "text", text: `Error: fingerprint mismatch for ${ipv4}. Expected: ${params.expected_fingerprint} — Got: ${actualFps.join(", ")}` }], | ||
| isError: true | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| // Step 3: SSH and run free -m |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect relevant file and surrounding context
wc -l src/tools/server-ssh.ts
sed -n '1,260p' src/tools/server-ssh.ts
# Find definitions/usages of keyScanRunner and runSsh
rg -n "keyScanRunner|runSsh|StrictHostKeyChecking|UserKnownHostsFile|known_hosts|accept-new" src/tools/server-ssh.ts src -g '!dist' -g '!build'Repository: jurislm/hetzner-mcp
Length of output: 11293
將指紋驗證綁定到實際 SSH 連線
ssh-keyscan 這段比對目前只是前置檢查，runSsh 仍用 StrictHostKeyChecking=accept-new 開啟獨立連線；主動式 MITM 仍可能讓驗證與真正連線看到不同的 host key。建議把已驗證的 key 寫入暫存 known_hosts，並讓實際連線改用 -o UserKnownHostsFile=<tmpfile> -o StrictHostKeyChecking=yes。
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/tools/server-ssh.ts` around lines 231 - 250, The fingerprint check in the
server SSH flow is only a preflight scan and is not tied to the actual SSH
session. Update the logic around the `keyScanRunner` / `runSsh` path so the
verified host key is written to a temporary known_hosts file, then make `runSsh`
use that file with `UserKnownHostsFile` and `StrictHostKeyChecking=yes` instead
of `accept-new`. Keep the existing fingerprint mismatch/error handling, but
ensure the live SSH connection is validated against the same verified key.
Comment on lines
+1322
to
+1325
| 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(), |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
hetzner_assert_storage_box_space 缺少 response_format 參數。
其 inputSchema 只有 id 與 required_gib，未提供共用的 ResponseFormatSchema，與同批新增的 hetzner_get_storage_box_stats 行為不一致。
♻️ 建議補上 response_format
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")
+ required_gib: z.number().positive().describe("Minimum required free space in GiB"),
+ response_format: ResponseFormatSchema.describe("Output format: 'markdown' or 'json'")
}).strict(),如採用，記得在 handler 內加上 ResponseFormat.JSON 分支輸出結構化結果。
As per coding guidelines：「每個工具應接受 response_format: "markdown" | "json" 參數（預設為 "markdown"），使用共用的 ResponseFormatSchema」。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 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(), | |
| 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"), | |
| response_format: ResponseFormatSchema.describe("Output format: 'markdown' or 'json'") | |
| }).strict(), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/tools/storage-boxes.ts` around lines 1322 - 1325,
`hetzner_assert_storage_box_space` 的 `inputSchema` 目前只接受 `id` 和
`required_gib`，需補上共用的 `ResponseFormatSchema`，讓介面與
`hetzner_get_storage_box_stats` 一致並符合工具規範。請在該工具的 schema 中加入 `response_format`（預設
markdown），並在對應的 handler／執行邏輯中依 `ResponseFormat.JSON` 分支回傳結構化結果。
Source: Coding guidelines
Comment on lines
+469
to
+500
| 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<typeof runSshKeyScan>().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<typeof runSshKeyScan>().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(); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
測試標題與斷言內容不符，容易誤導維護者。
it("returns isError when padded fingerprint (SHA256:abc==) is expected but extraction strips padding (Finding #2)", ...)（Line 485）標題暗示「應回傳 isError」，但實際斷言卻是 expect(result.isError).toBeUndefined()（Line 498），驗證的其實是「padding 有正確保留、比對成功」的情境。建議修正標題以反映真實行為，避免日後誤解測試意圖。
📝 建議修正測試標題
- it("returns isError when padded fingerprint (SHA256:abc==) is expected but extraction strips padding (Finding `#2`)", async () => {
+ it("succeeds when padded fingerprint (SHA256:abc==) is preserved and matches expected (Finding `#2` regression)", async () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 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<typeof runSshKeyScan>().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<typeof runSshKeyScan>().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(); | |
| }); | |
| 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<typeof runSshKeyScan>().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("succeeds when padded fingerprint (SHA256:abc==) is preserved and matches expected (Finding `#2` regression)", async () => { | |
| const PADDED_FP = "SHA256:AbCdEfGhIjKlMnOpQrStUvWxYzABCDEFGHIJK=="; | |
| mockedRequest.mockResolvedValueOnce(serverResponse); | |
| const paddedMock = vi.fn<typeof runSshKeyScan>().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(); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/tools/server-ssh.test.ts` around lines 469 - 500, Update the test case
name in server-ssh.test.ts so it matches the actual assertion in the padded
fingerprint scenario: the current title in the padded fingerprint test suggests
it should return isError, but the check in the captureHandler test expects
success. Keep the assertion as-is and rename the test to describe that
expected_fingerprint with padding matches correctly in
runSshKeyScan/captureHandler, so the intent is clear to future readers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
問題
Hetzner Cloud API 於 2026-06-30 正式從 Servers 與 Primary IPs 資源移除
datacenter屬性（2025-12-16 公告：Phasing out Datacenters in favor of Locations）。HetznerServerSchema將datacenter列為 required，formatServer讀取server.datacenter.location.*，導致所有 server 相關工具一律拋出：影響
hetzner_list_servers/hetzner_get_server/ metrics / ssh 等工具。修法
改用 API 早已提供的頂層
location物件，且只宣告formatServer實際渲染的三個欄位（name/city/country）。zod 的
z.object預設就會剝除未宣告的多餘欄位，因此宣告得越少對上游變更越寬容；反之，一個「宣告為必要卻從未讀取」的欄位就是定時炸彈——這次的datacenter正是如此。為什麼不用
.passthrough()實測 zod 4.3.6：
z.object.passthrough()passthrough對這次的失效模式毫無防護作用，加了只會誤導。驗證
datacenter、含latitude/longitude/network_zone等未宣告欄位）驗證 schema 可正常解析hetzner_list_servers與hetzner_get_server均回傳**Location**: Nuremberg, DE (nbg1)Test plan
bun run testbun run typecheckbun run linthetzner_list_servers/hetzner_get_serverSummary by CodeRabbit