fix(#984): harden the opt-in HTTP API — POST-only tool dispatch, optional token, loopback bind

CLAUDE.md

@@ -31,7 +31,8 @@ cmake --build build_local --target UnitTests -j4
 ```bash
 ./build_local/bin/QtMeshEditor.app/Contents/MacOS/QtMeshEditor --with-mcp          # GUI + MCP
 ./build_local/bin/QtMeshEditor.app/Contents/MacOS/QtMeshEditor --mcp               # headless MCP only
-./build_local/bin/QtMeshEditor.app/Contents/MacOS/QtMeshEditor --with-mcp --http-port 8080  # with HTTP API
+./build_local/bin/QtMeshEditor.app/Contents/MacOS/QtMeshEditor --with-mcp --http-port 8080  # with HTTP API (loopback-only; POST /api/tools/<name> executes, GET lists)
+./build_local/bin/QtMeshEditor.app/Contents/MacOS/QtMeshEditor --with-mcp --http-port 8080 --http-token-file ~/.qtmesh_http_token --http-bind 0.0.0.0  # #984: require `Authorization: Bearer <file content>` (or X-Api-Key), expose beyond localhost. NB `--http-token <secret>` is REFUSED (argv is readable by every local user via ps); use the file, QTMESH_HTTP_TOKEN, or QSettings mcp/httpToken
 ```
 
 **CLI pipeline (`qtmesh`):**
@@ -368,6 +369,7 @@ The animation pipeline started skeleton-only; the #517 epic broadens it. Slices
 - Launch modes: `--mcp` (headless), `--with-mcp` (GUI + MCP).
 - stdout is redirected to stderr to isolate MCP JSON-RPC from Ogre/Qt debug output; original stdout fd saved for MCP responses.
 - HTTP API uses QTcpServer with deferred tool execution (QTimer::singleShot) to avoid re-entrant crashes from Ogre event processing.
+- **HTTP API hardening (#984)**: the REST surface can run EVERY tool, mutating ones included, so (1) it binds **loopback** by default — `--http-bind <addr>` / `QTMESH_HTTP_BIND` opt into `0.0.0.0` (containers with a mapped port), and a non-loopback bind without a token logs a warning; (2) tools execute **only via `POST /api/tools/<name>`** — `GET /api/tools/<name>` answers `405` + `Allow: POST` and dispatches nothing (it used to run the tool with no arguments, so any link/prefetch could decimate a mesh); `GET /api/tools` still lists; (3) an **optional shared secret** — `--http-token-file <path>` (`MCPServer::readHttpTokenFile`, trimmed; warns when group/other-readable; an unreadable/blank file means the HTTP server is NOT started — down beats up-and-unprotected), else `QTMESH_HTTP_TOKEN`, else QSettings `mcp/httpToken` (`MCPServer::resolveHttpToken`). **`--http-token <secret>` on argv is refused at startup (exit 2)** — review caught that a command-line secret sits in `ps`/`/proc/<pid>/cmdline` for the whole session, readable by every local user, i.e. useless on exactly the shared machines it targets — when set, every request except the CORS preflight (which cannot carry credentials by spec) must send `Authorization: Bearer <t>` or `X-Api-Key: <t>`, else `401` + `WWW-Authenticate`; the compare is constant-time (`httpRequestAuthorized`, pure + unit-tested). Default (no token) stays open to local processes, so the `scripts/anim-*.sh` harnesses (all POST, plus a `GET /api/tools` liveness probe) run unchanged. `AppLaunchHandler::isGuiModeValueFlag` skips the `--http-port`/`--http-token(-file)`/`--http-bind` VALUES in BOTH `collectGuiLaunchPaths` (a token file never becomes a launch path) AND `isCliInvocation` (a value equal to a subcommand name or `--cli` — `--http-token-file scan` — must not reroute the whole launch into `CLIPipeline::run`; review finding) — and that skip is now checked BEFORE the generic leading-`-` `continue` in `collectGuiLaunchPaths`; it used to sit after it, i.e. was dead code, and `--http-port 8080` only appeared to work because "8080" is not an importable file (the test uses a real `.obj` as the value so the skip is observable).
 - **`take_screenshot` captures via an Ogre RTT, NOT `QWidget::grab()`** — Ogre renders straight to the native window surface (`WA_PaintOnScreen`) so `grab()` returns a black buffer. The tool renders the active viewport's `SpaceCamera` camera into an offscreen `PF_BYTE_RGBA` render target (RTSS `MSN_SHADERGEN` scheme + a temporary ambient boost & directional key light so imported materials aren't black, restored after) and reads it back to PNG. `load_mesh` calls `frameSceneInActiveViewport()` (select every user node → `frameSelection()`) so a headless `load_mesh`→`take_screenshot` actually frames + shows the mesh. This makes autonomous visual QA (load → optionally explode via `transform_submesh` → screenshot) work without a GUI operator.
 
 ### CLI Pipeline

README.md

@@ -320,7 +320,7 @@ Split View|Skeleton Animation Controls
 - **Performance capture** — video/webcam → facial morph animation (ARKit blendshapes), head pose, and full-body skeletal capture onto humanoid rigs; live preview + record in the editor, `qtmesh mocap` on the CLI (`-DENABLE_MOCAP` builds)
 - **AI chat** — natural language scene editing via local LLMs
 - **MCP server** — 57+ tools for AI agents (Claude, Cursor, etc.), including HDR/IBL (`set_hdr_environment`, `set_tonemap`, …) and QtMesh Cloud (`cloud_*`)
-- **REST API** — HTTP interface for external automation
+- **REST API** — opt-in HTTP interface for external automation (`--with-mcp --http-port 8080`). Bound to **localhost** by default; tools execute only via `POST /api/tools/<name>` (`GET /api/tools` lists them); set `QTMESH_HTTP_TOKEN` (or `--http-token-file <path>`; a secret on the command line is refused, since `ps` shows it to every local user) to require `Authorization: Bearer <token>` on every request, and `--http-bind 0.0.0.0` only when you mean to expose it
 
 ---
 

src/AppLaunchHandler.cpp

@@ -38,10 +38,20 @@
     return kSubcommands.contains(arg);
 }
 
+// Flags that take a value — the value must not be mistaken for a launch path.
+bool isGuiModeValueFlag(const QString& arg)
+{
+    // --http-token is listed so its VALUE is still consumed (never read as a
+    // path or a subcommand) even though main() refuses it (#984 review: a
+    // secret on argv is visible to every local user via ps).
+    return arg == QStringLiteral("--http-port") || arg == QStringLiteral("--http-token")
+        || arg == QStringLiteral("--http-token-file") || arg == QStringLiteral("--http-bind");
+}
+
 bool isGuiModeFlag(const QString& arg)
 {
     return arg == QStringLiteral("--mcp") || arg == QStringLiteral("-mcp")
-        || arg == QStringLiteral("--with-mcp") || arg == QStringLiteral("--http-port");
+        || arg == QStringLiteral("--with-mcp") || isGuiModeValueFlag(arg);
 }
 
 } // namespace
@@ -67,8 +77,12 @@
     if (execName.startsWith(QStringLiteral("qtmesh")) && !execName.contains(QStringLiteral("editor")))
         return true;
 
+    // #984 review: the values of --http-port/--http-token-file/--http-bind are
+    // opaque — a token file named "scan" or a value "--cli" must not route the
+    // launch into the CLI. Consume each value in BOTH scans.
     for (int i = 1; i < argc; ++i) {
         const QString arg = QString::fromLocal8Bit(argv[i]);
+        if (isGuiModeValueFlag(arg)) { ++i; continue; }
         if (arg == QStringLiteral("--cli") || arg == QStringLiteral("--help")
             || arg == QStringLiteral("-h") || arg == QStringLiteral("--version")
             || arg == QStringLiteral("-v")) {
@@ -76,8 +90,9 @@
         }
     }
 
     for (int i = 1; i < argc; ++i) {
         const QString arg = QString::fromLocal8Bit(argv[i]);
+        if (isGuiModeValueFlag(arg)) { ++i; continue; }
         if (arg.startsWith(QLatin1Char('-')))
             continue;
         if (isCliSubcommand(arg))
@@ -109,13 +124,17 @@
     QStringList paths;
     for (int i = 1; i < arguments.size(); ++i) {
         const QString& arg = arguments.at(i);
-        if (arg.startsWith(QLatin1Char('-')))
-            continue;
+        // Value-taking flags FIRST: the generic '-' skip below used to run
+        // before this branch, so the ++i never executed and the VALUE was
+        // examined as a launch path (#984 — a --http-token naming an existing
+        // mesh would have been opened in the editor).
         if (isGuiModeFlag(arg)) {
-            if (arg == QStringLiteral("--http-port") && i + 1 < arguments.size())
+            if (isGuiModeValueFlag(arg) && i + 1 < arguments.size())
                 ++i;
             continue;
         }
+        if (arg.startsWith(QLatin1Char('-')))
+            continue;
         if (isCliSubcommand(arg))
             break;
 

src/AppLaunchHandler_coverage_test.cpp

@@ -223,8 +223,10 @@ TEST(AppLaunchHandlerCoverageTest, DefaultImportExtensions_NotEmpty)
 TEST(AppLaunchHandlerCoverageTest, Collect_HttpPortSkipsPortNumber)
 {
     // "--http-port" is a GUI-mode flag; the numeric arg right after must be
-    // consumed by the ++i advance and never treated as a path. The port number
-    // also is not importable, but the ++i guarantees it's skipped entirely.
+    // consumed by the ++i advance and never treated as a path. NB a port number
+    // is not importable anyway, so this case alone cannot tell the skip from a
+    // no-op — Collect_HttpTokenAndBindValuesAreSkipped uses a REAL file as the
+    // value for that (it caught the skip being dead code, #984).
     const QStringList args = {
         QStringLiteral("QtMeshEditor"),
         QStringLiteral("--http-port"),
@@ -243,6 +245,66 @@ TEST(AppLaunchHandlerCoverageTest, Collect_HttpPortAtEndNoFollowingArg)
     EXPECT_TRUE(AppLaunchHandler::collectGuiLaunchPaths(args).isEmpty());
 }
 
+TEST(AppLaunchHandlerCoverageTest, Cli_HttpOptionValuesNeverRouteToCli)
+{
+    // #984 review: a token (file) that happens to equal a subcommand, or a value
+    // of "--cli", must not turn a GUI/MCP launch into a CLI run.
+    {
+        char a0[] = "QtMeshEditor", a1[] = "--with-mcp", a2[] = "--http-token-file", a3[] = "scan";
+        char* argv[] = {a0, a1, a2, a3};
+        EXPECT_FALSE(AppLaunchHandler::isCliInvocation(4, argv));
+    }
+    {
+        char a0[] = "QtMeshEditor", a1[] = "--http-token", a2[] = "scan";   // refused by main(), still consumed here
+        char* argv[] = {a0, a1, a2};
+        EXPECT_FALSE(AppLaunchHandler::isCliInvocation(3, argv));
+    }
+    {
+        char a0[] = "QtMeshEditor", a1[] = "--http-token-file", a2[] = "--cli";
+        char* argv[] = {a0, a1, a2};
+        EXPECT_FALSE(AppLaunchHandler::isCliInvocation(3, argv));
+    }
+    {
+        char a0[] = "QtMeshEditor", a1[] = "--http-bind", a2[] = "0.0.0.0", a3[] = "--http-port", a4[] = "9000";
+        char* argv[] = {a0, a1, a2, a3, a4};
+        EXPECT_FALSE(AppLaunchHandler::isCliInvocation(5, argv));
+    }
+    {
+        // …while a REAL subcommand after the value pairs is still detected.
+        char a0[] = "QtMeshEditor", a1[] = "--http-port", a2[] = "9000", a3[] = "info";
+        char* argv[] = {a0, a1, a2, a3};
+        EXPECT_TRUE(AppLaunchHandler::isCliInvocation(4, argv));
+    }
+}
+
+TEST(AppLaunchHandlerCoverageTest, Collect_HttpTokenAndBindValuesAreSkipped)
+{
+    // #984: --http-token-file / --http-bind take a value like --http-port does. The
+    // value is skipped even when it names a REAL importable file — otherwise a
+    // token that happens to look like a path would be opened in the editor.
+    QTemporaryDir dir;
+    ASSERT_TRUE(dir.isValid());
+    const QString decoy = dir.filePath(QStringLiteral("token.obj"));
+    QFile obj(decoy);
+    ASSERT_TRUE(obj.open(QIODevice::WriteOnly));
+    obj.write("v 0 0 0\n");
+    obj.close();
+
+    const QStringList args = {
+        QStringLiteral("QtMeshEditor"),
+        QStringLiteral("--with-mcp"),
+        QStringLiteral("--http-token-file"), decoy,
+        QStringLiteral("--http-bind"),       QStringLiteral("0.0.0.0"),
+    };
+    EXPECT_TRUE(AppLaunchHandler::collectGuiLaunchPaths(args).isEmpty());
+
+    // …while a real file AFTER the value pairs is still collected.
+    const QStringList withFile = args + QStringList{decoy};
+    const QStringList paths = AppLaunchHandler::collectGuiLaunchPaths(withFile);
+    ASSERT_EQ(paths.size(), 1);
+    EXPECT_EQ(paths.front(), QFileInfo(decoy).absoluteFilePath());
+}
+
 TEST(AppLaunchHandlerCoverageTest, Collect_HttpPortThenRealFileStillCollected)
 {
     QTemporaryDir dir;