MCP Server Critique: Architectural Friction and Security Vulnerabilities
This report outlines an engineering and security critique of the `wikantik-admin` and `wikantik-knowledge` MCP servers, derived from a session involving large-scale refactoring and aggressive robustness testing ("face-punching").
1. Security Vulnerabilities: Input Validation
**Observation:** The server allowed renaming a page to `../../../../etc/passwd`. While it performed a "normalization" (resulting in `........EtcPasswd`), it did not block the path traversal attempt outright.
**Critique:** The normalization logic appears to be a naive replacement of forbidden characters rather than a strict allow-list for CamelCase names. Furthermore, the server accepted a 5,000-character page name and a name containing a null byte (`\u0000`).
**Recommendation:**
* Implement a strict **regex allow-list** for page names (e.g., `^[A-Z][a-zA-Z0-9]+$`).
* Enforce a **maximum length** for page names (e.g., 128 characters) to prevent UI breakage and potential file-system issues.
* Reject any input containing control characters or null bytes.
2. Resource Exhaustion and DoS Risks
**Observation:** The server successfully processed a 5,000-character page name and a complex YAML structure with recursive anchors (YAML Bomb).
**Critique:** While the server didn't crash, the lack of resource limits on string lengths and object depth makes it vulnerable to memory exhaustion attacks.
**Recommendation:**
* Implement **global limits** on the size of a single `write_pages` or `update_page` request (e.g., 2MB).
* Limit the **maximum depth** of JSON/YAML objects in `metadata` and content blocks.
* The YAML parser should be configured to disable or strictly limit anchor/alias expansion.
3. Optimistic Concurrency vs. Background Mutations
**Observation:** A `rename_page` call with `updateLinks=true` mutates other pages, changing their hashes.
**Critique:** If an agent attempts an edit based on a stale hash, the server returns a generic "hash mismatch" error, forcing an extra round-trip to re-read the content.
**Recommendation:** Return the `latestContent` and `newVersion` directly in the 409-equivalent error response to allow the agent to rebase immediately.
4. Discovery Friction and "Broken Intent"
**Observation:** Identifying high-leverage broken links is turn-intensive.
**Critique:** The Admin server knows a link is broken but lacks "semantic awareness" of why.
**Recommendation:** Integrate `get_broken_links` with the Knowledge server's embedding space to provide "Fix Suggestions" (e.g., "Page `SystemsThinking` is missing, but `SystemsTheory` exists with 92% similarity").
5. Architectural Silos
**Observation:** Conceptual split between Admin and Knowledge servers leads to redundant context gathering.
**Critique:** An agent often needs "is it a page?" (Admin) and "what is it about?" (Knowledge) simultaneously.
**Recommendation:** Consolidate into a single "Agentic API" surface where a single tool call can return both metadata and semantic context.
---
Final Audit Summary of "Face-Punching" Session
| Vector | Input | Result | Status |
| :--- | :--- | :--- | :--- |
| **Path Traversal** | `../../etc/passwd` | Sanitized to `........EtcPasswd` | **FAIL** (Should block) |
| **SQL Injection** | `' OR 1=1 --` | Handled gracefully (no matches) | **PASS** |
| **Shell Injection** | `test; ls -la` | Handled gracefully (no matches) | **PASS** |
| **Billion Laughs** | YAML anchors/aliases | Processed and re-emitted | **WARN** (Potential DoS) |
| **Buffer Overflow** | 5,000 char name | Created successfully | **FAIL** (Should limit length) |
| **Null Byte** | `Test\u0000Page` | Created successfully | **FAIL** (Should block) |
| **System Protect** | `delete_pages(["Main"])` | Refused | **PASS** |
---
**See Also:**
- [Wikantik Development](WikantikDevelopment) — Platform architecture overview.
- [Systems Thinking](SystemsThinking) — Theoretical framework for these system dynamics.