MCP: Gate stdio on DisableMCP and add scope regression test #5024

internal/auth/acl/scope_test.go

@@ -81,6 +81,14 @@ func TestScopePermits(t *testing.T) {
 		assert.True(t, ScopePermits("read mcp", ResourceMCP, Permissions{ActionView}))
 		assert.False(t, ScopePermits("read mcp", ResourceMCP, Permissions{ActionUpdate}))
 		assert.False(t, ScopePermits("metrics", ResourceMCP, Permissions{ActionView}))
+		// "mcp:read" is NOT an alias for "mcp read" - KeyValue.Parse treats
+		// it as {key=mcp, value=read}, which Attr.Find("mcp") never matches
+		// because the resource lookup expects a {mcp,true} entry. This
+		// regression assertion locks in the documented behavior in
+		// specs/platform/acl-scopes.md so the parser does not silently
+		// start accepting the colon form.
+		assert.False(t, ScopePermits("mcp:read", ResourceMCP, Permissions{ActionView}))
+		assert.False(t, ScopePermits("mcp:read", ResourceMCP, Permissions{ActionUpdate}))
 	})
 }
 

internal/commands/mcp.go

@@ -2,12 +2,14 @@ package commands
 
 import (
 	"context"
+	"errors"
 	"log/slog"
 	"os"
 
 	sdkmcp "github.com/modelcontextprotocol/go-sdk/mcp"
 	"github.com/urfave/cli/v2"
 
+	"github.com/photoprism/photoprism/internal/config"
 	"github.com/photoprism/photoprism/internal/mcp"
 )
 
@@ -29,8 +31,22 @@ var MCPServeCommand = &cli.Command{
 
 // mcpServeAction starts the MCP server over the stdio transport,
 // writing a startup line to stderr so the JSON-RPC stream on stdout
-// stays clean for the MCP client.
+// stays clean for the MCP client. The action loads the active config
+// and refuses to start when DisableMCP is set, mirroring how the
+// HTTP transport at /api/v1/mcp is gated; pass --disable-mcp=false on
+// the command line to override the operator setting for ad-hoc
+// development runs.
 func mcpServeAction(ctx *cli.Context) error {
+	conf, err := InitConfig(ctx)
+
+	if err != nil {
+		return err
+	}
+
+	if err = mcpDisabledExit(conf); err != nil {
+		return err
+	}
+
 	logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelInfo}))
 	logger.Info("starting mcp server",
 		"transport", "stdio",
@@ -41,6 +57,17 @@ func mcpServeAction(ctx *cli.Context) error {
 	return runMCPServer(context.Background(), ctx, &sdkmcp.StdioTransport{})
 }
 
+// mcpDisabledExit returns a cli.ExitCoder error when MCP is disabled
+// by config, pointing the caller at --disable-mcp=false for ad-hoc
+// development runs. It returns nil when MCP is enabled.
+func mcpDisabledExit(conf *config.Config) error {
+	if conf == nil || !conf.DisableMCP() {
+		return nil
+	}
+
+	return cli.Exit(errors.New("mcp serve disabled by config; pass --disable-mcp=false to override"), 1)
+}
+
 // runMCPServer builds an MCP server from the CLI app metadata (Version,
 // Edition) and runs it over the given transport until ctx is canceled or
 // the transport closes. The transport is a parameter so tests can

internal/commands/mcp_test.go

@@ -10,6 +10,8 @@ import (
 	sdkmcp "github.com/modelcontextprotocol/go-sdk/mcp"
 	"github.com/stretchr/testify/require"
 	"github.com/urfave/cli/v2"
+
+	"github.com/photoprism/photoprism/internal/photoprism/get"
 )
 
 // TestMCPCommandRegistered ensures the MCP command is present in the CLI catalog.
@@ -85,6 +87,65 @@ func TestRunMCPServerOverInMemoryTransport(t *testing.T) {
 	}
 }
 
+// TestMCPDisabledExit covers the gate helper that mcpServeAction calls
+// before starting the stdio transport: DisableMCP=true must return a
+// cli.ExitCoder error whose message points at --disable-mcp=false as
+// the documented override; DisableMCP=false must return nil so the
+// server can start; nil config must be tolerated.
+func TestMCPDisabledExit(t *testing.T) {
+	conf := get.Config()
+	require.NotNil(t, conf, "test harness must provide a shared config")
+
+	original := conf.Options().DisableMCP
+	t.Cleanup(func() { conf.Options().DisableMCP = original })
+
+	t.Run("DisableMCPTrue_Refuses", func(t *testing.T) {
+		conf.Options().DisableMCP = true
+
+		err := mcpDisabledExit(conf)
+		require.Error(t, err)
+
+		coder, ok := err.(cli.ExitCoder)
+		require.True(t, ok, "expected cli.ExitCoder, got %T", err)
+		require.Equal(t, 1, coder.ExitCode())
+		require.Contains(t, err.Error(), "--disable-mcp=false")
+	})
+
+	t.Run("DisableMCPFalse_Allows", func(t *testing.T) {
+		conf.Options().DisableMCP = false
+
+		require.NoError(t, mcpDisabledExit(conf))
+	})
+
+	t.Run("NilConfig_Allows", func(t *testing.T) {
+		require.NoError(t, mcpDisabledExit(nil))
+	})
+}
+
+// TestMCPServeRefusesWhenDisabled drives the full mcp serve action through
+// RunWithTestContext to confirm the gate trips end-to-end before the stdio
+// transport is started. The DisableMCP=false branch is not exercised here
+// because mcpServeAction would block on stdin once it reaches the transport;
+// the gate-level coverage in TestMCPDisabledExit is sufficient to assert the
+// pre-transport branch.
+func TestMCPServeRefusesWhenDisabled(t *testing.T) {
+	conf := get.Config()
+	require.NotNil(t, conf, "test harness must provide a shared config")
+
+	original := conf.Options().DisableMCP
+	t.Cleanup(func() { conf.Options().DisableMCP = original })
+
+	conf.Options().DisableMCP = true
+
+	_, err := RunWithTestContext(MCPServeCommand, []string{"serve"})
+	require.Error(t, err)
+
+	coder, ok := err.(cli.ExitCoder)
+	require.True(t, ok, "expected cli.ExitCoder, got %T", err)
+	require.Equal(t, 1, coder.ExitCode())
+	require.Contains(t, err.Error(), "--disable-mcp=false")
+}
+
 // TestMCPAppMetadata covers the happy path and every fallback branch of
 // mcpAppMetadata: missing key, non-string value, empty-string value, and
 // nil metadata map.

internal/mcp/README.md

@@ -6,7 +6,7 @@
 
 - **Transports:**
   - CLI: `photoprism mcp serve` (stdio, no auth; development and testing). The static-data contract documented under *Authorization* and *Extending the Tool Surface* below applies — write-capable tools MUST require a session token even on stdio.
-  - HTTP: `POST/GET/DELETE /api/v1/mcp` (Streamable HTTP, authenticated). Can be disabled via `--disable-mcp` / `PHOTOPRISM_DISABLE_MCP` / `DisableMCP` so the route responds with the standard 404 when an operator does not want the endpoint exposed. The flag is also surfaced to the frontend through `ClientConfig.Disable.MCP` (`disable.mcp`), letting the UI hide MCP-related controls while the endpoint is off.
+  - HTTP: `POST/GET/DELETE /api/v1/mcp` (Streamable HTTP, authenticated). Disabled together with the stdio transport via `--disable-mcp` / `PHOTOPRISM_DISABLE_MCP` / `DisableMCP`: the HTTP route responds with the standard 404 and `photoprism mcp serve` refuses to start (pass `--disable-mcp=false` on the command line to override for ad-hoc development runs). The flag is also surfaced to the frontend through `ClientConfig.Disable.MCP` (`disable.mcp`), letting the UI hide MCP-related controls while the endpoint is off.
 - **Authorization:** HTTP endpoint enforces the `ResourceMCP` ACL (admin plus the API client roles in every edition, manager in Pro/Portal); anonymous access is permitted in public mode for the currently registered read-only tools.
 - **Request Body Cap:** HTTP POST bodies are bounded at `MaxMCPRequestBytes` (currently `MaxMutationRequestBytes`, 256 KiB). Oversized requests receive the standard `413 Request Entity Too Large` response before the upstream SDK reads the body. Early rejection via `Content-Length` protects against large known-size payloads; `http.MaxBytesReader` plus a response-writer wrapper handle chunked bodies. The wrapper translates the SDK's internal `400 "failed to read body"` into a consistent `413` and suppresses the SDK's error phrasing so it does not leak to clients.
 - **Session Timeout:** Streamable HTTP sessions idle out after `McpSessionTimeout` (5 minutes by default). Active clients renew the idle timer on every JSON-RPC request, so interactive IDE use is unaffected; sessions abandoned without the `DELETE` tear-down free up promptly instead of lingering.
@@ -62,6 +62,12 @@ Start the MCP server over stdio:
 
 The process waits for an MCP client on stdin/stdout. Logs are written to stderr so the MCP message stream stays valid.
 
+When the running config has `DisableMCP=true` (via `options.yml`, `PHOTOPRISM_DISABLE_MCP`, or `--disable-mcp`), the command refuses to start with `mcp serve disabled by config; pass --disable-mcp=false to override`. Pass the override on the command line for ad-hoc development runs without flipping the operator-facing setting:
+
+```bash
+./photoprism --disable-mcp=false mcp serve
+```
+
 ### Run via HTTP
 
 Start PhotoPrism: