fix: explicit cookie parsing, colon guard on the token payload, and gate scope comment for the form password

src/webserver/api/webhook_controller.cpp

@@ -224,6 +224,23 @@ void WebhookController::handleWebhook(const httplib::Request& req, httplib::Resp
 
         const std::string form_password = form_config.value("password", "");
         if (!form_password.empty() && !formCookieValid(req, workflow_id)) {
+            // The prompt body itself is generic - it never names the form or
+            // says whether the password was merely wrong versus nothing being
+            // here at all. That is the only layer this gate controls. One
+            // layer up, the response still distinguishes: no such workflow is
+            // a 404 "Workflow not found" from earlier in this function (that
+            // 404 is pre-existing and applies to every webhook on the
+            // platform, not something this task introduced or could remove
+            // without changing behaviour every existing integration already
+            // depends on); a workflow that exists but has no form node falls
+            // through to the trigger-node path below, a structurally
+            // different response; and a form that exists behind a wrong
+            // password gets this 200 prompt. So existence and node-type are
+            // still an oracle to someone who can tell those apart, even
+            // though the password itself never leaks. Accepted as-is: closing
+            // that gap would mean reshaping the 404 behaviour of the whole
+            // webhook surface, and workflow ids are 122-bit random UUIDs, so
+            // there is nothing practical to enumerate against it.
             const std::string action = "/webhook/" + workflow_id + path;
             std::string attempt;
             if (req.method == "POST" && req.has_param("__form_password")) {
@@ -675,22 +692,70 @@ void WebhookController::handleFormGet(const httplib::Request& req, httplib::Resp
     res.set_content(form_renderer::renderForm(config, action, ""), "text/html; charset=utf-8");
 }
 
+namespace {
+
+// The Cookie header is a ';'-separated list of name=value pairs, and RFC 6265
+// allows '=' inside a value - so a plain substring search for "name=" can
+// match partway into an unrelated cookie's value instead of the cookie it
+// was looking for. Splitting on ';' and matching the name exactly (after
+// trimming the leading space every pair after the first carries) is the only
+// way to be sure which cookie was actually found; whatever comes back still
+// has to pass verifyDetached; this only fixes what gets handed to it in the
+// first place.
+std::optional<std::string> findCookieValue(const std::string& cookie_header, const std::string& name) {
+    std::size_t pos = 0;
+    while (pos <= cookie_header.size()) {
+        auto semi = cookie_header.find(';', pos);
+        std::string pair = cookie_header.substr(pos, semi == std::string::npos ? std::string::npos : semi - pos);
+
+        std::size_t start = 0;
+        while (start < pair.size() && pair[start] == ' ') ++start;
+        std::size_t stop = pair.size();
+        while (stop > start && pair[stop - 1] == ' ') --stop;
+        pair = pair.substr(start, stop - start);
+
+        const auto eq = pair.find('=');
+        if (eq != std::string::npos && pair.compare(0, eq, name) == 0) {
+            return pair.substr(eq + 1);
+        }
+
+        if (semi == std::string::npos) break;
+        pos = semi + 1;
+    }
+    return std::nullopt;
+}
+
+}  // namespace
+
 std::string WebhookController::formCookieToken(const std::string& workflow_id, int64_t expires_at) {
     // A signed token rather than the password: the password never reaches the
     // browser, and a stolen cookie opens one form until it expires.
+    //
+    // The payload packs workflow_id and expires_at together with ':' and
+    // parses them back apart the same way, which only works if neither piece
+    // can itself contain ':'. expires_at is a decimal integer, so that part
+    // is safe unconditionally; workflow_id is refused here if it contains
+    // ':' so the split stays unambiguous rather than silently moving and
+    // signing/verifying different bytes than the caller intended. Today's
+    // workflow ids (wf_<uuid>) never hit this, but nothing enforced that
+    // before - this makes the assumption load-bearing instead of implicit.
+    if (workflow_id.find(':') != std::string::npos) {
+        return "";
+    }
     const std::string payload = workflow_id + ":" + std::to_string(expires_at);
     return payload + ":" + jwt_utils_.signDetached(payload);
 }
 
 bool WebhookController::formCookieValid(const httplib::Request& req, const std::string& workflow_id) {
-    const std::string cookie_header = req.get_header_value("Cookie");
-    const std::string key = "sb_form_" + workflow_id + "=";
-    const auto at = cookie_header.find(key);
-    if (at == std::string::npos) return false;
+    // See the comment in formCookieToken: the token format only parses back
+    // correctly if workflow_id has no ':' in it, so a workflow id that
+    // violates that can never have a valid cookie either.
+    if (workflow_id.find(':') != std::string::npos) return false;
 
-    std::string value = cookie_header.substr(at + key.size());
-    const auto end = value.find(';');
-    if (end != std::string::npos) value = value.substr(0, end);
+    const std::string cookie_header = req.get_header_value("Cookie");
+    const auto found = findCookieValue(cookie_header, "sb_form_" + workflow_id);
+    if (!found.has_value()) return false;
+    const std::string& value = found.value();
 
     const auto first = value.find(':');
     const auto second = value.rfind(':');

src/webserver/api/webhook_controller.hpp

@@ -53,6 +53,14 @@ public:
     // use-after-free. Safe to call more than once.
     void stop();
 
+    // A signed token rather than the password itself: the password never
+    // reaches the browser, and a stolen cookie opens one form until it
+    // expires rather than the account behind it. workflow_id must not
+    // contain ':' - see the comment on formCookieToken's definition for why
+    // the token format depends on that.
+    std::string formCookieToken(const std::string& workflow_id, int64_t expires_at);
+    bool formCookieValid(const httplib::Request& req, const std::string& workflow_id);
+
 private:
     void handleWebhook(const httplib::Request& req, httplib::Response& res);
 
@@ -68,15 +76,6 @@ private:
     bool buildFormTriggerData(const httplib::Request& req, const nlohmann::json& config,
                               nlohmann::json& out, std::string& error);
 
-public:
-    // A signed token rather than the password itself: the password never
-    // reaches the browser, and a stolen cookie opens one form until it
-    // expires rather than the account behind it.
-    std::string formCookieToken(const std::string& workflow_id, int64_t expires_at);
-    bool formCookieValid(const httplib::Request& req, const std::string& workflow_id);
-
-private:
-
     void sendJson(httplib::Response& res, const nlohmann::json& data, int status = 200);
     void sendError(httplib::Response& res, const std::string& message, int status);