fix!: address security and code review findings

.env.example

@@ -52,13 +52,19 @@ BOOKSTACK_BASE_URL=http://localhost:8080/api
 BOOKSTACK_API_TOKEN=your-api-token-here
 BOOKSTACK_TIMEOUT=30000
 
+# Per-request BookStack URL (HTTP transport, token mode)
+# Comma-separated BookStack API base URLs a caller may name in the x-bookstack-url header.
+# Unset refuses that header with a 400. An allowed URL also requires x-bookstack-token: the
+# BOOKSTACK_API_TOKEN above is only ever sent to BOOKSTACK_BASE_URL.
+# BOOKSTACK_ALLOWED_BASE_URLS=https://books.example.com/api,https://staging-books.example.com/api
+
 # Uploads (optional)
 # The image/attachment tools accept a `file_path` to a file on the server instead of
-# inlined base64. Under the stdio transport the client shares this process's trust
-# domain, so any readable path is allowed. Under HTTP the caller may be remote, so
-# `file_path` is refused outright unless you opt in by setting BOOKSTACK_UPLOAD_ROOT:
-# the path is then resolved with realpath() and must land inside this directory,
-# which blocks `../` traversal and symlink escapes.
+# inlined base64. Under every transport, stdio included, `file_path` is refused unless you
+# opt in by setting BOOKSTACK_UPLOAD_ROOT: whoever steers the tool arguments - a remote
+# caller, or text planted in a page an agent reads - could otherwise upload any file this
+# process can read. The path is resolved with realpath() and must land inside this
+# directory, which blocks `../` traversal and symlink escapes.
 # BOOKSTACK_UPLOAD_ROOT=/var/lib/bookstack-mcp/uploads
 
 # Server Configuration
@@ -145,7 +151,3 @@ VALIDATION_STRICT_MODE=true
 # tests/transport/stdio.test.ts, which assert them against the real output streams.
 LOG_LEVEL=info
 LOG_FORMAT=pretty
-
-# Development
-NODE_ENV=development
-DEBUG=false

.github/workflows/ci.yml

@@ -5,6 +5,9 @@ on:
     branches: [main]
   pull_request:
 
+permissions:
+  contents: read
+
 env:
   # Single source of truth for the Bun this repo is validated on. The Dockerfile's
   # BUN_IMAGE pin must name this same version: the docker job's smoke suite compares
@@ -17,8 +20,8 @@ jobs:
     name: Typecheck, test, lint
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v5
-      - uses: oven-sh/setup-bun@v2
+      - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
+      - uses: oven-sh/setup-bun@735343b667d3e6f658f44d0eca948eb6282f2b76 # v2.0.2
         with:
           bun-version: ${{ env.BUN_VERSION }}
       - name: Install dependencies
@@ -47,8 +50,8 @@ jobs:
     # Independent of `quality` on purpose: the two report in parallel, so a lint failure
     # and a broken image surface on the same run instead of one hiding the other.
     steps:
-      - uses: actions/checkout@v5
-      - uses: oven-sh/setup-bun@v2
+      - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
+      - uses: oven-sh/setup-bun@735343b667d3e6f658f44d0eca948eb6282f2b76 # v2.0.2
         with:
           bun-version: ${{ env.BUN_VERSION }}
       - name: Build image

.github/workflows/image.yml

@@ -28,7 +28,17 @@ jobs:
       contents: read
       packages: write
     steps:
+      # Full history, so the tagged commit can be checked against origin/main.
       - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
+        with:
+          fetch-depth: 0
+
+      - name: Require the tagged commit to be on main
+        run: |
+          if ! git merge-base --is-ancestor HEAD refs/remotes/origin/main; then
+            echo "::error::The tagged commit $(git rev-parse HEAD) is not on main; only merged commits are released." >&2
+            exit 1
+          fi
 
       - name: Read the version from the tag
         id: version
@@ -83,6 +93,21 @@ jobs:
           username: ${{ github.actor }}
           password: ${{ secrets.GITHUB_TOKEN }}
 
+      # A published tag is immutable: re-releasing a version takes a new -r<revision>.
+      - name: Refuse to overwrite a published image
+        env:
+          VERSION: ${{ steps.version.outputs.version }}
+        run: |
+          REF="${IMAGE,,}:${VERSION}"
+          if OUT="$(docker buildx imagetools inspect "$REF" 2>&1)"; then
+            echo "::error::${REF} is already published; tag a new -r<revision> instead." >&2
+            exit 1
+          elif ! grep -q ': not found' <<<"$OUT"; then
+            echo "::error::Could not check whether ${REF} is published; refusing to push blind." >&2
+            echo "$OUT" >&2
+            exit 1
+          fi
+
       # Rebuilds from the builder cache above, so the pushed layers are the ones just smoked.
       - name: Push image
         uses: docker/build-push-action@c3c9e263c25d99ce0380d002d59b67737d91b0dc # v7.4.0

Dockerfile

@@ -50,6 +50,6 @@ USER bun
 # GET /health returns 200 when healthy, 503 otherwise. The image ships `bun`
 # but NOT `node`, so the probe is implemented with `bun -e`.
 HEALTHCHECK --interval=30s --timeout=10s --start-period=15s --retries=3 \
-  CMD bun -e "const r=await fetch('http://localhost:3000/health').catch(()=>null); process.exit(r&&r.status===200?0:1)"
+  CMD bun -e "const r=await fetch('http://localhost:'+(process.env.SERVER_PORT||3000)+'/health').catch(()=>null); process.exit(r&&r.status===200?0:1)"
 
 CMD ["bun", "run", "src/server.ts"]

README.md

@@ -1,10 +1,10 @@
 # BookStack MCP Server
 
-Connect BookStack to Claude and other AI assistants through the Model Context Protocol (MCP). This server exposes 59 tools and 11 resources covering the supported subset of the BookStack API — books, pages, chapters, shelves, search, users, roles, permissions, attachments, images, the recycle bin, the audit log and system info.
+Connect BookStack to Claude and other AI assistants through the Model Context Protocol (MCP). This server exposes 59 tools, 5 resources and 6 resource templates covering the supported subset of the BookStack API — books, pages, chapters, shelves, search, users, roles, permissions, attachments, images, the recycle bin, the audit log and system info.
 
 This server supports two transport modes: **Streamable HTTP** (default) and **Stdio**.
 
-- **Streamable HTTP (default)**: A stateless HTTP transport. Authentication parameters can be overridden per-request using HTTP headers (`x-bookstack-url` and `x-bookstack-token`).
+- **Streamable HTTP (default)**: A stateless HTTP transport. The BookStack token can be overridden per request with `x-bookstack-token`, and the BookStack URL with `x-bookstack-url` when the operator allows it (see [per-request overrides](#per-request-overrides)).
 - **Stdio Mode**: Standard input/output for local integration (e.g., with Claude Desktop). Set `MCP_TRANSPORT=stdio` to enable.
 
 > ⚠️ **Looking for the HTTP endpoint?** The MCP endpoint is `POST /message` — not `/`.
@@ -13,7 +13,7 @@ This server supports two transport modes: **Streamable HTTP** (default) and **St
 ## ✨ What You Get
 
 - **BookStack Integration** - Access your books, pages, chapters, and content
-- **59 MCP Tools & 11 Resources** - CRUD, search and export across the supported endpoint families
+- **59 MCP Tools, 5 Resources & 6 Resource Templates** - CRUD, search and export across the supported endpoint families
 - **Search & Export** - Find content and export in multiple formats
 - **User Management** - Handle users, roles, and permissions
 - **Production Ready** - Rate limiting, validation, error handling, and logging
@@ -98,7 +98,9 @@ export MCP_TRANSPORT="http"
 | `BOOKSTACK_BASE_URL` | `http://localhost:8080/api` | Full URL to the BookStack API. Must be a valid URL and include the `/api` suffix. |
 | `BOOKSTACK_API_TOKEN` | _(none — required, unless `MCP_AUTH_MODE=oauth`)_ | **Outbound** BookStack API token as `token_id:token_secret`. This is the credential the server spends on every tool call. Startup fails if unset. |
 | `BOOKSTACK_TIMEOUT` | `30000` | BookStack request timeout in milliseconds. |
-| `SERVER_PORT` | `3000` | Port the HTTP transport listens on. Ignored in stdio mode. |
+| `BOOKSTACK_ALLOWED_BASE_URLS` | _(none)_ | Comma-separated BookStack API base URLs a caller may name in the `x-bookstack-url` header, compared canonically. Unset refuses that header. See [per-request overrides](#per-request-overrides). |
+| `BOOKSTACK_UPLOAD_ROOT` | _(none)_ | Directory the image/attachment tools' `file_path` may read from. Unset refuses `file_path` under every transport, stdio included; the path must resolve inside this directory. Must be unset with `MCP_AUTH_MODE=oauth`. |
+| `SERVER_PORT` | `3000` | Port the HTTP transport listens on; if it is taken, startup fails with exit code `1`. Ignored in stdio mode. |
 | `HTTP_BODY_LIMIT` | `73400320` (70 MiB) | Maximum accepted `POST /message` body, in bytes. Sized for the largest inline base64 upload the image/attachment tools advertise (50,000 KB). Express's own default is ~100 KB, which would reject real uploads with a `413`. Lower it if untrusted callers can reach the port. |
 | `SERVER_NAME` | `bookstack-mcp-server` | Server name reported over MCP and by `GET /`. |
 | `SERVER_VERSION` | the package's own version | Version reported over MCP `initialize`, by `GET /`, and by `bookstack_server_info`. Defaults to `package.json#version`; leave it unset unless you deliberately want a different value. |
@@ -108,8 +110,6 @@ export MCP_TRANSPORT="http"
 | `VALIDATION_STRICT_MODE` | `true` | Reject invalid tool params at the boundary. Set to `false` to log a warning and forward them to BookStack instead. |
 | `LOG_LEVEL` | `info` | One of `error`, `warn`, `info`, `debug`. |
 | `LOG_FORMAT` | `pretty` | One of `json`, `pretty`. |
-| `NODE_ENV` | `development` | One of `development`, `production`, `test`. |
-| `DEBUG` | `false` | Set to `true` for debug output. |
 
 > 💡 Need detailed setup? See the complete [Setup Guide](docs/setup-guide.md)
 
@@ -128,14 +128,15 @@ The transport is chosen at startup from `MCP_TRANSPORT`:
 ### HTTP endpoints
 
 When running in HTTP mode the server exposes three endpoints, plus the OAuth metadata
-document in [OAuth mode](#per-user-oauth). Any other path returns a JSON `404` listing
-the valid ones.
+document in [OAuth mode](#per-user-oauth). `GET` and `DELETE` on `/message` answer `405`
+with `Allow: POST`, before authentication: the server offers no SSE stream and no sessions.
+Any other path returns a JSON `404` listing the valid ones.
 
 | Method & path | Purpose | Status codes |
 | --- | --- | --- |
 | `GET /` | Server info JSON (name, version, `status: "running"`, endpoint list) | `200` |
 | `GET /health` | Health check — verifies live connectivity to BookStack | `200` healthy, `503` unhealthy |
-| `POST /message` | **The MCP endpoint.** Send JSON-RPC MCP messages here | `200`, `401` without a valid bearer token, `500` on error |
+| `POST /message` | **The MCP endpoint.** Send JSON-RPC MCP messages here | `200`, `400` on a malformed body or refused override, `401` without a valid bearer token, `500` on error |
 
 `GET /` and `GET /health` are unauthenticated. **`POST /message` requires an inbound
 `Authorization: Bearer <secret>` header** — it dispatches every tool, including
@@ -178,17 +179,19 @@ with the failing check named:
   "checks": [
     { "name": "bookstack_connection", "healthy": false, "message": "BookStack API connection" },
     { "name": "tools_loaded", "healthy": true, "message": "59 tools loaded" },
-    { "name": "resources_loaded", "healthy": true, "message": "11 resources loaded" }
+    { "name": "resources_loaded", "healthy": true, "message": "5 resources and 6 resource templates loaded" }
   ]
 }
 ```
 
-> ⚠️ A **missing** `BOOKSTACK_API_TOKEN` behaves differently: config validation rejects
-> an empty token at startup, so the process exits with
-> `Configuration validation failed: bookstack.apiToken: BookStack API token is required`
-> before Express ever listens. There is no `/health` to call — `curl` gets a connection
-> refused, and under Docker the container restart-loops. A `503` therefore always means
-> the token is **present but not working**; a dead port means it is **absent**.
+> ⚠️ A **missing** `BOOKSTACK_API_TOKEN` behaves differently: the transport refuses to
+> start. In HTTP token mode the process prints
+> `Failed to start HTTP transport: Configuration validation failed: bookstack.apiToken: BookStack API token is required - set BOOKSTACK_API_TOKEN environment variable`
+> to stderr and exits `1` before Express ever listens. There is no `/health` to call —
+> `curl` gets a connection refused, and under Docker the container restart-loops. A `503`
+> therefore always means the token is **present but not working**; a dead port means it
+> is **absent**. Under stdio the same reason is printed as
+> `Failed to start stdio transport: …`, stdout stays empty and the process exits `1`.
 
 **Call the MCP endpoint** — an `initialize` handshake. Both the `Content-Type`
 and `Accept` headers are required by the Streamable HTTP transport, and
@@ -214,9 +217,14 @@ curl -X POST http://localhost:3000/message \
   }'
 ```
 
-Per-request credential overrides are supported on `POST /message` via the
-`x-bookstack-url` and `x-bookstack-token` headers; both fall back to the
-`BOOKSTACK_BASE_URL` / `BOOKSTACK_API_TOKEN` environment variables. OAuth mode refuses them.
+#### Per-request overrides
+
+`POST /message` accepts two optional headers in token mode. OAuth mode refuses both.
+
+- `x-bookstack-token` alone spends the caller's own BookStack token at `BOOKSTACK_BASE_URL`.
+- `x-bookstack-url` is refused with a `400` unless it names one of
+  `BOOKSTACK_ALLOWED_BASE_URLS`, and then `x-bookstack-token` is required too. A
+  caller-chosen URL is never sent the configured `BOOKSTACK_API_TOKEN`.
 
 ### Per-user OAuth
 
@@ -328,9 +336,9 @@ only the fragment it wants changed:
 | Tool | What it does |
 |---|---|
 | `bookstack_pages_outline` | Heading structure with offsets and section sizes. No content transferred. |
-| `bookstack_pages_read` with `grep` | Matching excerpts from the **stored** source — paste one straight into `old_string`. |
+| `bookstack_pages_read` with `grep` | Matches from the **stored** source, each with an exact `context` slice (no ellipses) — paste it straight into `old_string`. |
 | `bookstack_pages_edit` | Literal find-and-replace. `old_string` must match exactly and be unique unless `replace_all` is set. |
-| `bookstack_pages_append` | Insert at the end of the page, the end of a named section, or right after a section heading. |
+| `bookstack_pages_append` | Insert at the end of the page, the end of a named section (after its subsections), or right after a section heading. Reports the heading used as `section_matched`. |
 
 ```jsonc
 // 1. What sections exist, and how big are they?
@@ -357,8 +365,10 @@ only the fragment it wants changed:
 error carries the first few matches with context. A missing anchor reports the same text found
 with different whitespace, which is the usual near-miss. A result smaller than half the
 original is refused unless `allow_shrink` is set. After a write the page is re-read and the
-change is looked for in normalised text — BookStack rewrites stored HTML on save (heading
-anchors, injected `id` attributes), so a byte comparison would call every success a failure.
+change is looked for in the raw stored source, then in normalised text — BookStack rewrites
+stored HTML on save (heading anchors, injected `id` attributes), so a byte comparison alone
+would call every success a failure. If that re-read fails, the result is `verified: null`:
+the write landed, so do not retry it.
 Every write creates a BookStack revision, so an applied edit can be rolled back in the UI.
 No response from these tools contains page content.
 `expected_updated_at` detects a page changed before this server reads it; BookStack's page

docker-compose.yml

@@ -88,7 +88,7 @@ services:
     volumes:
       - bookstack_config:/config
     ports:
-      - "6875:80"
+      - "127.0.0.1:6875:80"
 
   mcp:
     build: .
@@ -126,7 +126,7 @@ services:
       - "127.0.0.1:3000:3000"
     healthcheck:
       # Same bun-based probe as the Dockerfile (the image has bun, not node).
-      test: ["CMD", "bun", "-e", "const r=await fetch('http://localhost:3000/health').catch(()=>null); process.exit(r&&r.status===200?0:1)"]
+      test: ["CMD", "bun", "-e", "const r=await fetch('http://localhost:'+(process.env.SERVER_PORT||3000)+'/health').catch(()=>null); process.exit(r&&r.status===200?0:1)"]
       interval: 30s
       timeout: 10s
       retries: 3