From b7c0d3d22a3d8834a83adfb22930ac14123c27c2 Mon Sep 17 00:00:00 2001 From: Molecule AI DevOps Engineer Date: Fri, 17 Apr 2026 07:14:12 +0000 Subject: [PATCH 1/5] infra: add rebuild-runtime-images.sh for post-PR#640 image fix (#658) Standalone adapter images (langgraph, claude-code, etc.) use ENTRYPOINT ["molecule-runtime"] which bypasses entrypoint.sh. PR #640's entrypoint.sh fix therefore never runs in adapter images. The correct fix is to bake git config --system into the image at build time. This script: 1. Rebuilds workspace-template:base from the monorepo Dockerfile (which has the fixed entrypoint.sh and molecule-git-token-helper.sh) 2. For each of the 6 runtime adapters: clones the standalone repo, patches its Dockerfile to COPY the credential helper and run git config --system, then builds the final image tagged as workspace-template: Usage (run on the host machine, not inside a workspace container): bash workspace-template/rebuild-runtime-images.sh # all 6 bash workspace-template/rebuild-runtime-images.sh claude-code # one See issue #658 for the architectural explanation. Co-Authored-By: Claude Sonnet 4.6 --- workspace-template/rebuild-runtime-images.sh | 175 +++++++++++++++++++ 1 file changed, 175 insertions(+) create mode 100755 workspace-template/rebuild-runtime-images.sh diff --git a/workspace-template/rebuild-runtime-images.sh b/workspace-template/rebuild-runtime-images.sh new file mode 100755 index 00000000..c9786d67 --- /dev/null +++ b/workspace-template/rebuild-runtime-images.sh @@ -0,0 +1,175 @@ +#!/usr/bin/env bash +# rebuild-runtime-images.sh — Rebuild all 6 workspace runtime Docker images. +# +# Run this script from the repo root (or from workspace-template/) after any +# change to workspace-template/Dockerfile, entrypoint.sh, or the git credential +# helper scripts. Also run after PR #640 merged. +# +# What this does: +# 1. Builds workspace-template:base from the monorepo Dockerfile (includes +# the fixed entrypoint.sh + molecule-git-token-helper.sh) +# 2. For each runtime adapter, clones its standalone repo to a temp dir, +# patches its Dockerfile to: +# a. COPY the git credential helper into the image +# b. Set git config --system to register the helper globally +# Then builds and tags workspace-template:. +# +# Why the patch is needed: +# Standalone adapter images (molecule-ai-workspace-template-*) use +# ENTRYPOINT ["molecule-runtime"] — they do not run entrypoint.sh, so the +# git config registration from entrypoint.sh never fires for them. Baking +# it into the image via git config --system at Docker build time is the +# correct permanent fix (issue #613 / PR #640). +# +# Prerequisites: docker, git, gh (authenticated) +# +# Usage (from repo root): +# bash workspace-template/rebuild-runtime-images.sh +# +# To rebuild a single runtime: +# bash workspace-template/rebuild-runtime-images.sh claude-code +# +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +REPO_ROOT="$(cd "${SCRIPT_DIR}/.." && pwd)" +HELPER_SCRIPT="${SCRIPT_DIR}/scripts/molecule-git-token-helper.sh" +RUNTIMES=(langgraph claude-code openclaw crewai autogen deepagents) + +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +RED='\033[0;31m' +NC='\033[0m' +log() { echo -e "${GREEN}[rebuild]${NC} $1"; } +warn() { echo -e "${YELLOW}[rebuild]${NC} $1"; } +err() { echo -e "${RED}[rebuild]${NC} $1"; } + +# ───────────────────────────────────────────────────── +# Argument: optional single runtime to rebuild +# ───────────────────────────────────────────────────── +if [ "${1:-}" != "" ]; then + RUNTIMES=("$1") +fi + +# ───────────────────────────────────────────────────── +# Preflight checks +# ───────────────────────────────────────────────────── +if ! command -v docker >/dev/null 2>&1; then + err "docker not found — run this on the host machine, not inside a workspace container" + exit 1 +fi + +if [ ! -f "${HELPER_SCRIPT}" ]; then + err "molecule-git-token-helper.sh not found at ${HELPER_SCRIPT}" + err "Run: git pull origin main (PR #640 adds this file)" + exit 1 +fi + +log "Building workspace-template:base from monorepo Dockerfile..." +docker build \ + --no-cache \ + -t workspace-template:base \ + -f "${SCRIPT_DIR}/Dockerfile" \ + "${SCRIPT_DIR}" +log "✓ workspace-template:base built" + +# ───────────────────────────────────────────────────── +# Build each runtime adapter image +# ───────────────────────────────────────────────────── +TMPBASE=$(mktemp -d) +trap "rm -rf ${TMPBASE}" EXIT + +SUCCESS=() +FAILED=() + +for runtime in "${RUNTIMES[@]}"; do + log "──────────────────────────────────────────" + log "Building workspace-template:${runtime} ..." + + TMPDIR="${TMPBASE}/${runtime}" + mkdir -p "${TMPDIR}" + + # Clone the standalone template repo + REPO="Molecule-AI/molecule-ai-workspace-template-${runtime}" + log " Cloning ${REPO} ..." + if ! git clone --depth 1 "https://github.com/${REPO}.git" "${TMPDIR}" 2>&1; then + err " Failed to clone ${REPO} — skipping ${runtime}" + FAILED+=("${runtime}") + continue + fi + + # Verify a Dockerfile exists + if [ ! -f "${TMPDIR}/Dockerfile" ]; then + err " No Dockerfile in ${REPO} — skipping ${runtime}" + FAILED+=("${runtime}") + continue + fi + + # Copy the credential helper into the build context so the Dockerfile can COPY it. + cp "${HELPER_SCRIPT}" "${TMPDIR}/molecule-git-token-helper.sh" + + # Patch the Dockerfile: + # 1. COPY the helper script into the image at a predictable path + # 2. git config --system registers it globally (applies to all users in the + # container, survives the root→agent gosu handoff) + # 3. Re-declare ENTRYPOINT last (safe — molecule-runtime entrypoint is + # unchanged, just ensuring it's after our additions) + # + # We do NOT replace the ENTRYPOINT or CMD — molecule-runtime remains the + # entry point. The git config --system baked into the image layer means + # git will call the helper on every push/fetch without any startup script. + cat >> "${TMPDIR}/Dockerfile" << 'PATCH' + +# ─── git credential helper (issue #613 / PR #640) ─────────────────────────── +# Bake the credential helper into the image so git always has a fresh +# GitHub App token. git config --system writes to /etc/gitconfig which is +# inherited by all users (root → agent gosu handoff). No startup script change +# needed — git invokes this helper automatically on push/fetch. +COPY molecule-git-token-helper.sh /usr/local/bin/molecule-git-credential-helper +RUN chmod +x /usr/local/bin/molecule-git-credential-helper && \ + git config --system credential.https://github.com.helper \ + '!molecule-git-credential-helper' && \ + echo "git credential helper registered (molecule-git-credential-helper)" +# ───────────────────────────────────────────────────────────────────────────── +PATCH + + # Build and tag + log " Running docker build ..." + if docker build \ + --no-cache \ + -t "workspace-template:${runtime}" \ + "${TMPDIR}" 2>&1 | grep -E "^(Step|#|---|\[|✓|ERROR|error)" ; then + log " ✓ workspace-template:${runtime} built" + SUCCESS+=("${runtime}") + else + err " Build failed for ${runtime}" + FAILED+=("${runtime}") + fi +done + +# ───────────────────────────────────────────────────── +# Summary +# ───────────────────────────────────────────────────── +echo "" +log "══════════════════════════════════════════" +log "Rebuild complete" +log "══════════════════════════════════════════" +if [ "${#SUCCESS[@]}" -gt 0 ]; then + log "✓ Succeeded: ${SUCCESS[*]}" +fi +if [ "${#FAILED[@]}" -gt 0 ]; then + err "✗ Failed: ${FAILED[*]}" +fi + +echo "" +log "Verify images:" +docker images | grep "workspace-template" | sort + +echo "" +log "To restart all running workspaces and pick up new images:" +log " docker ps --filter name=molecule --format '{{.Names}}' | xargs -r docker rm -f" +log " # Then restart workspaces via Canvas or API" + +if [ "${#FAILED[@]}" -gt 0 ]; then + exit 1 +fi From 7066fce6f47957bf6e266d098cf7512a483b2ca5 Mon Sep 17 00:00:00 2001 From: Molecule AI DevOps Engineer Date: Fri, 17 Apr 2026 10:25:43 +0000 Subject: [PATCH 2/5] =?UTF-8?q?fix(infra):=20rename=20TMPDIR=E2=86=92RUNTI?= =?UTF-8?q?ME=5FDIR,=20fix=20PIPESTATUS=20docker=20exit=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug 1: TMPDIR is a POSIX-reserved variable used by mktemp, Docker BuildKit, and git subprocesses as their system temp directory. Overwriting it redirected those tools to the build context, causing unpredictable failures. Renamed all 6 occurrences to RUNTIME_DIR. Bug 2: `docker build ... | grep` made grep's exit code (0=match, 1=no match) determine if the build succeeded, not docker's. Fixed by reading PIPESTATUS[0] immediately after the pipeline so docker's real exit code drives the SUCCESS/FAILED tracking. Also fixed two pre-existing shellcheck warnings: - SC2034: removed unused REPO_ROOT variable - SC2064: trap now uses single quotes so TMPBASE expands at signal time shellcheck clean with no warnings. Co-Authored-By: Claude Sonnet 4.6 --- workspace-template/rebuild-runtime-images.sh | 26 +++++++++++--------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/workspace-template/rebuild-runtime-images.sh b/workspace-template/rebuild-runtime-images.sh index c9786d67..61d7358d 100755 --- a/workspace-template/rebuild-runtime-images.sh +++ b/workspace-template/rebuild-runtime-images.sh @@ -32,7 +32,6 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" -REPO_ROOT="$(cd "${SCRIPT_DIR}/.." && pwd)" HELPER_SCRIPT="${SCRIPT_DIR}/scripts/molecule-git-token-helper.sh" RUNTIMES=(langgraph claude-code openclaw crewai autogen deepagents) @@ -77,7 +76,7 @@ log "✓ workspace-template:base built" # Build each runtime adapter image # ───────────────────────────────────────────────────── TMPBASE=$(mktemp -d) -trap "rm -rf ${TMPBASE}" EXIT +trap 'rm -rf "${TMPBASE}"' EXIT SUCCESS=() FAILED=() @@ -86,27 +85,27 @@ for runtime in "${RUNTIMES[@]}"; do log "──────────────────────────────────────────" log "Building workspace-template:${runtime} ..." - TMPDIR="${TMPBASE}/${runtime}" - mkdir -p "${TMPDIR}" + RUNTIME_DIR="${TMPBASE}/${runtime}" + mkdir -p "${RUNTIME_DIR}" # Clone the standalone template repo REPO="Molecule-AI/molecule-ai-workspace-template-${runtime}" log " Cloning ${REPO} ..." - if ! git clone --depth 1 "https://github.com/${REPO}.git" "${TMPDIR}" 2>&1; then + if ! git clone --depth 1 "https://github.com/${REPO}.git" "${RUNTIME_DIR}" 2>&1; then err " Failed to clone ${REPO} — skipping ${runtime}" FAILED+=("${runtime}") continue fi # Verify a Dockerfile exists - if [ ! -f "${TMPDIR}/Dockerfile" ]; then + if [ ! -f "${RUNTIME_DIR}/Dockerfile" ]; then err " No Dockerfile in ${REPO} — skipping ${runtime}" FAILED+=("${runtime}") continue fi # Copy the credential helper into the build context so the Dockerfile can COPY it. - cp "${HELPER_SCRIPT}" "${TMPDIR}/molecule-git-token-helper.sh" + cp "${HELPER_SCRIPT}" "${RUNTIME_DIR}/molecule-git-token-helper.sh" # Patch the Dockerfile: # 1. COPY the helper script into the image at a predictable path @@ -118,7 +117,7 @@ for runtime in "${RUNTIMES[@]}"; do # We do NOT replace the ENTRYPOINT or CMD — molecule-runtime remains the # entry point. The git config --system baked into the image layer means # git will call the helper on every push/fetch without any startup script. - cat >> "${TMPDIR}/Dockerfile" << 'PATCH' + cat >> "${RUNTIME_DIR}/Dockerfile" << 'PATCH' # ─── git credential helper (issue #613 / PR #640) ─────────────────────────── # Bake the credential helper into the image so git always has a fresh @@ -134,15 +133,20 @@ RUN chmod +x /usr/local/bin/molecule-git-credential-helper && \ PATCH # Build and tag + # Capture docker's exit code via PIPESTATUS[0] before grep's exit code + # overwrites $?. Without this, set -o pipefail causes grep's exit (0 = match + # found, 1 = no match) to determine success — not docker's exit code. log " Running docker build ..." - if docker build \ + docker build \ --no-cache \ -t "workspace-template:${runtime}" \ - "${TMPDIR}" 2>&1 | grep -E "^(Step|#|---|\[|✓|ERROR|error)" ; then + "${RUNTIME_DIR}" 2>&1 | grep -E "^(Step|#|---|\[|✓|ERROR|error)" + docker_exit=${PIPESTATUS[0]} + if [ "${docker_exit}" -eq 0 ]; then log " ✓ workspace-template:${runtime} built" SUCCESS+=("${runtime}") else - err " Build failed for ${runtime}" + err " Build failed for ${runtime} (docker exit ${docker_exit})" FAILED+=("${runtime}") fi done From bbfe2e92d4f9593e81ab43bac3ddac4007870a48 Mon Sep 17 00:00:00 2001 From: Molecule AI DevOps Engineer Date: Fri, 17 Apr 2026 10:27:11 +0000 Subject: [PATCH 3/5] fix(security): allowlist-validate runtime arg in rebuild-runtime-images.sh The optional $1 argument flowed directly into Docker image tag names (workspace-template:) and filesystem paths (RUNTIME_DIR) with no validation, enabling path traversal or unexpected tag injection via e.g. `bash rebuild-runtime-images.sh '../evil'`. Fix: introduce VALID_RUNTIMES allowlist and validate $1 against it before setting RUNTIMES. Any unlisted value now exits with a clear error message. The RUNTIMES array is populated from VALID_RUNTIMES when no argument is given, keeping the all-runtimes default path. shellcheck clean; $1 only appears inside the validated block. Co-Authored-By: Claude Sonnet 4.6 --- workspace-template/rebuild-runtime-images.sh | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/workspace-template/rebuild-runtime-images.sh b/workspace-template/rebuild-runtime-images.sh index 61d7358d..c98950d8 100755 --- a/workspace-template/rebuild-runtime-images.sh +++ b/workspace-template/rebuild-runtime-images.sh @@ -33,7 +33,7 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" HELPER_SCRIPT="${SCRIPT_DIR}/scripts/molecule-git-token-helper.sh" -RUNTIMES=(langgraph claude-code openclaw crewai autogen deepagents) +VALID_RUNTIMES=(langgraph claude-code openclaw crewai autogen deepagents) GREEN='\033[0;32m' YELLOW='\033[1;33m' @@ -45,9 +45,21 @@ err() { echo -e "${RED}[rebuild]${NC} $1"; } # ───────────────────────────────────────────────────── # Argument: optional single runtime to rebuild +# Allowlist-validated: $1 must be one of VALID_RUNTIMES. +# Prevents path traversal and unexpected Docker tag injection. # ───────────────────────────────────────────────────── -if [ "${1:-}" != "" ]; then +if [ -n "${1:-}" ]; then + valid=0 + for v in "${VALID_RUNTIMES[@]}"; do + [ "$1" = "$v" ] && valid=1 && break + done + if [ "${valid}" -eq 0 ]; then + err "Unknown runtime '${1}'. Valid: ${VALID_RUNTIMES[*]}" + exit 1 + fi RUNTIMES=("$1") +else + RUNTIMES=("${VALID_RUNTIMES[@]}") fi # ───────────────────────────────────────────────────── From 15d4b25c7860896cc9a4d289cebf931f4797bada Mon Sep 17 00:00:00 2001 From: "molecule-ai[bot]" <276602405+molecule-ai[bot]@users.noreply.github.com> Date: Fri, 17 Apr 2026 10:36:51 +0000 Subject: [PATCH 4/5] fix(security): Ed25519 signature verification for Discord webhooks + strip token from error chain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HIGH (#659-1): POST /webhooks/discord had no signature verification, allowing any attacker to POST forged Discord slash-command payloads. Add Ed25519 verification via verifyDiscordSignature() before adapter.ParseWebhook() is called. The function reads r.Body, verifies Ed25519(pubKey, timestamp+body, X-Signature-Ed25519), then restores r.Body with io.NopCloser so ParseWebhook can still read the payload. The public key is resolved from the first enabled Discord channel's app_public_key config (plaintext — it is a public key and not in sensitiveFields) with a fallback to DISCORD_APP_PUBLIC_KEY env var; no key configured -> 401 (fail-closed). discordPublicKey() is the DB helper. MEDIUM (#659-2): discord.go SendMessage() wrapped http.Client.Do errors with %w, propagating the *url.Error which includes the full webhook URL (https://discord.com/api/webhooks/{id}/{token}) into logs and error responses. Replace with a static "discord: HTTP request failed" string. Tests added (11 new): - TestVerifyDiscordSignature_Valid / _WrongKey / _TamperedBody / _MissingTimestamp / _MissingSignature / _InvalidHexSignature / _InvalidHexPubKey / _WrongLengthPubKey (real Ed25519 key pairs) - TestChannelHandler_Webhook_Discord_NoKey_Returns401 - TestChannelHandler_Webhook_Discord_InvalidSig_Returns401 - TestChannelHandler_Webhook_Discord_ValidSig_PingAccepted - TestDiscordAdapter_SendMessage_ErrorDoesNotLeakToken go test ./... green. Co-Authored-By: Claude Sonnet 4.6 --- platform/internal/channels/discord.go | 6 +- platform/internal/channels/discord_test.go | 28 +++ platform/internal/handlers/channels.go | 87 +++++++ platform/internal/handlers/channels_test.go | 242 +++++++++++++++++++- 4 files changed, 361 insertions(+), 2 deletions(-) diff --git a/platform/internal/channels/discord.go b/platform/internal/channels/discord.go index b7807724..44957e39 100644 --- a/platform/internal/channels/discord.go +++ b/platform/internal/channels/discord.go @@ -84,7 +84,11 @@ func (d *DiscordAdapter) SendMessage(ctx context.Context, config map[string]inte resp, err := client.Do(req) if err != nil { - return fmt.Errorf("discord: send: %w", err) + // Do NOT wrap err — the *url.Error from http.Client.Do includes the + // full request URL, which contains the Discord webhook token + // (https://discord.com/api/webhooks/{id}/{token}). Wrapping with %w + // would propagate that token into logs and error responses (#659). + return fmt.Errorf("discord: HTTP request failed") } body, _ := io.ReadAll(resp.Body) resp.Body.Close() diff --git a/platform/internal/channels/discord_test.go b/platform/internal/channels/discord_test.go index cd184d17..61b71a4c 100644 --- a/platform/internal/channels/discord_test.go +++ b/platform/internal/channels/discord_test.go @@ -287,6 +287,34 @@ func TestSplitMessage_LongMessage(t *testing.T) { } } +// TestDiscordAdapter_SendMessage_ErrorDoesNotLeakToken verifies that when the +// HTTP call to the Discord webhook fails (e.g. DNS error), the returned error +// message does NOT contain the webhook URL — which embeds the Discord token. +// Regression test for the MEDIUM security finding in PR #659. +func TestDiscordAdapter_SendMessage_ErrorDoesNotLeakToken(t *testing.T) { + a := &DiscordAdapter{} + // Use a valid-looking webhook URL with a fake token so we can check it + // doesn't appear in the error string. + fakeToken := "SUPER_SECRET_DISCORD_TOKEN_12345" + webhookURL := discordWebhookPrefix + "123456789/" + fakeToken + + // Point at an unroutable address to force a dial error. + err := a.SendMessage( + context.Background(), + map[string]interface{}{"webhook_url": webhookURL}, + "ignored", + "hello", + ) + + if err == nil { + // In some environments the request might actually succeed; that's fine. + t.Skip("request unexpectedly succeeded — skipping token-leak check") + } + if strings.Contains(err.Error(), fakeToken) { + t.Errorf("error message leaks Discord webhook token: %q", err.Error()) + } +} + func TestSplitMessage_SplitsAtNewline(t *testing.T) { // Build a message where a newline falls within the split window. line1 := strings.Repeat("a", 1500) + "\n" diff --git a/platform/internal/handlers/channels.go b/platform/internal/handlers/channels.go index c2bb0890..0c7df94c 100644 --- a/platform/internal/handlers/channels.go +++ b/platform/internal/handlers/channels.go @@ -1,12 +1,17 @@ package handlers import ( + "bytes" "context" + "crypto/ed25519" "crypto/subtle" "database/sql" + "encoding/hex" "encoding/json" + "io" "log" "net/http" + "os" "strings" "github.com/gin-gonic/gin" @@ -410,6 +415,22 @@ func (h *ChannelHandler) Webhook(c *gin.Context) { return } + // Discord: verify Ed25519 signature BEFORE the body is consumed by ParseWebhook. + // The app_public_key is the Discord application's public key (not a secret — + // it's a PUBLIC key and therefore stored in plaintext in channel_config). + // We look it up from the DB (first enabled Discord channel with the field set) + // and fall back to the DISCORD_APP_PUBLIC_KEY env var for self-hosted setups + // that prefer global configuration. Fail closed: no key configured → 401. + // verifyDiscordSignature restores r.Body after reading so ParseWebhook below + // can still read the payload. + if channelType == "discord" { + pubKey := discordPublicKey(ctx) + if pubKey == "" || !verifyDiscordSignature(c.Request, pubKey) { + c.JSON(http.StatusUnauthorized, gin.H{"error": "invalid signature"}) + return + } + } + // For webhooks, we need to find the channel by type and match by chat_id in the message // Parse the webhook first to get the chat_id msg, err := adapter.ParseWebhook(c, nil) @@ -489,3 +510,69 @@ func (h *ChannelHandler) Webhook(c *gin.Context) { c.JSON(http.StatusOK, gin.H{"status": "accepted"}) } + +// discordPublicKey returns the Ed25519 public key to use for Discord request +// signature verification. It queries the DB for the first enabled Discord +// channel whose config contains a non-empty app_public_key (stored in +// plaintext — it is a PUBLIC key and is not in the sensitiveFields list), +// then falls back to the DISCORD_APP_PUBLIC_KEY environment variable. +// +// Returns "" when no key is configured, which causes the caller to reject +// the incoming request with 401 (fail-closed behaviour). +func discordPublicKey(ctx context.Context) string { + var pubKey string + row := db.DB.QueryRowContext(ctx, ` + SELECT COALESCE(channel_config->>'app_public_key', '') + FROM workspace_channels + WHERE channel_type = 'discord' AND enabled = true + AND channel_config->>'app_public_key' IS NOT NULL + AND channel_config->>'app_public_key' != '' + LIMIT 1 + `) + _ = row.Scan(&pubKey) + if pubKey != "" { + return pubKey + } + return os.Getenv("DISCORD_APP_PUBLIC_KEY") +} + +// verifyDiscordSignature verifies a Discord Interactions request using the +// Ed25519 signature scheme described in Discord's Interactions documentation. +// Discord signs the concatenation of the X-Signature-Timestamp header and the +// raw request body with the application's private key; we verify with the +// public key stored in channel_config or DISCORD_APP_PUBLIC_KEY. +// +// The function reads r.Body in full and then replaces it with a bytes.Reader +// over the same bytes so that subsequent callers (adapter.ParseWebhook) can +// still read the body. +// +// Returns false when any required header is missing, when pubKeyHex cannot +// be hex-decoded to a 32-byte Ed25519 public key, when the signature header +// cannot be decoded, or when the Ed25519 verification itself fails. +func verifyDiscordSignature(r *http.Request, pubKeyHex string) bool { + sig := r.Header.Get("X-Signature-Ed25519") + ts := r.Header.Get("X-Signature-Timestamp") + if sig == "" || ts == "" || pubKeyHex == "" { + return false + } + + pubKeyBytes, err := hex.DecodeString(pubKeyHex) + if err != nil || len(pubKeyBytes) != ed25519.PublicKeySize { + return false + } + + body, err := io.ReadAll(r.Body) + if err != nil { + return false + } + // Restore body so adapter.ParseWebhook can read it. + r.Body = io.NopCloser(bytes.NewReader(body)) + + sigBytes, err := hex.DecodeString(sig) + if err != nil { + return false + } + + msg := append([]byte(ts), body...) + return ed25519.Verify(pubKeyBytes, msg, sigBytes) +} diff --git a/platform/internal/handlers/channels_test.go b/platform/internal/handlers/channels_test.go index 88f0a504..d05909ea 100644 --- a/platform/internal/handlers/channels_test.go +++ b/platform/internal/handlers/channels_test.go @@ -3,12 +3,17 @@ package handlers import ( "bytes" "context" + "crypto/ed25519" + "crypto/rand" + "encoding/hex" "encoding/json" + "io" "net/http" "net/http/httptest" + "strings" "testing" - "github.com/DATA-DOG/go-sqlmock" + sqlmock "github.com/DATA-DOG/go-sqlmock" "github.com/Molecule-AI/molecule-monorepo/platform/internal/channels" "github.com/gin-gonic/gin" ) @@ -579,3 +584,238 @@ func TestChannelHandler_Send_BudgetNotYetReached_PassesThrough(t *testing.T) { t.Errorf("expected budget check to pass (under limit), but got 429") } } + +// ==================== Discord Ed25519 signature verification ==================== +// +// These tests cover verifyDiscordSignature and the Discord signature gate in +// the Webhook handler. They use real Ed25519 key pairs generated in-process so +// the cryptographic assertions are load-bearing (not hand-crafted hex strings). + +// genDiscordKey generates a fresh Ed25519 key pair for tests. +// Returns (pubKeyHex, privKey). +func genDiscordKey(t *testing.T) (string, ed25519.PrivateKey) { + t.Helper() + pub, priv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatalf("ed25519.GenerateKey: %v", err) + } + return hex.EncodeToString(pub), priv +} + +// discordSignedRequest builds an *http.Request with the correct Discord +// Ed25519 headers signed by privKey. +func discordSignedRequest(t *testing.T, body string, ts string, privKey ed25519.PrivateKey) *http.Request { + t.Helper() + msg := append([]byte(ts), []byte(body)...) + sig := ed25519.Sign(privKey, msg) + req := httptest.NewRequest(http.MethodPost, "/webhooks/discord", strings.NewReader(body)) + req.Header.Set("X-Signature-Ed25519", hex.EncodeToString(sig)) + req.Header.Set("X-Signature-Timestamp", ts) + return req +} + +// TestVerifyDiscordSignature_Valid asserts that a correctly signed request +// passes verification. +func TestVerifyDiscordSignature_Valid(t *testing.T) { + pubHex, priv := genDiscordKey(t) + body := `{"type":1}` + req := discordSignedRequest(t, body, "1700000000", priv) + + if !verifyDiscordSignature(req, pubHex) { + t.Error("expected true for valid Discord signature, got false") + } + // Body must be restored so subsequent reads still work. + restored, _ := io.ReadAll(req.Body) + if string(restored) != body { + t.Errorf("body not restored: got %q, want %q", restored, body) + } +} + +// TestVerifyDiscordSignature_WrongKey asserts that a signature verified with +// a different public key returns false. +func TestVerifyDiscordSignature_WrongKey(t *testing.T) { + _, priv := genDiscordKey(t) + wrongPubHex, _ := genDiscordKey(t) // different key pair + req := discordSignedRequest(t, `{"type":1}`, "1700000000", priv) + + if verifyDiscordSignature(req, wrongPubHex) { + t.Error("expected false for signature verified with wrong public key") + } +} + +// TestVerifyDiscordSignature_TamperedBody asserts that modifying the body +// after signing invalidates the signature. +func TestVerifyDiscordSignature_TamperedBody(t *testing.T) { + pubHex, priv := genDiscordKey(t) + req := discordSignedRequest(t, `{"type":1}`, "1700000000", priv) + // Replace the body with different content after signing. + req.Body = io.NopCloser(strings.NewReader(`{"type":2,"tampered":true}`)) + + if verifyDiscordSignature(req, pubHex) { + t.Error("expected false for tampered body, got true") + } +} + +// TestVerifyDiscordSignature_MissingTimestamp asserts that a missing +// X-Signature-Timestamp header returns false. +func TestVerifyDiscordSignature_MissingTimestamp(t *testing.T) { + pubHex, priv := genDiscordKey(t) + req := discordSignedRequest(t, `{"type":1}`, "1700000000", priv) + req.Header.Del("X-Signature-Timestamp") + + if verifyDiscordSignature(req, pubHex) { + t.Error("expected false for missing X-Signature-Timestamp") + } +} + +// TestVerifyDiscordSignature_MissingSignature asserts that a missing +// X-Signature-Ed25519 header returns false. +func TestVerifyDiscordSignature_MissingSignature(t *testing.T) { + pubHex, priv := genDiscordKey(t) + req := discordSignedRequest(t, `{"type":1}`, "1700000000", priv) + req.Header.Del("X-Signature-Ed25519") + + if verifyDiscordSignature(req, pubHex) { + t.Error("expected false for missing X-Signature-Ed25519") + } +} + +// TestVerifyDiscordSignature_InvalidHexSignature asserts that a non-hex +// signature returns false. +func TestVerifyDiscordSignature_InvalidHexSignature(t *testing.T) { + pubHex, _ := genDiscordKey(t) + req := httptest.NewRequest(http.MethodPost, "/webhooks/discord", strings.NewReader(`{}`)) + req.Header.Set("X-Signature-Ed25519", "not-valid-hex!!!") + req.Header.Set("X-Signature-Timestamp", "1700000000") + + if verifyDiscordSignature(req, pubHex) { + t.Error("expected false for invalid hex signature") + } +} + +// TestVerifyDiscordSignature_InvalidHexPubKey asserts that a non-hex public +// key returns false. +func TestVerifyDiscordSignature_InvalidHexPubKey(t *testing.T) { + _, priv := genDiscordKey(t) + req := discordSignedRequest(t, `{}`, "1700000000", priv) + + if verifyDiscordSignature(req, "not-hex-at-all!!!") { + t.Error("expected false for non-hex public key") + } +} + +// TestVerifyDiscordSignature_WrongLengthPubKey asserts that a hex-encoded +// byte slice that is not 32 bytes returns false. +func TestVerifyDiscordSignature_WrongLengthPubKey(t *testing.T) { + _, priv := genDiscordKey(t) + req := discordSignedRequest(t, `{}`, "1700000000", priv) + // 16 bytes — too short for Ed25519. + shortKey := hex.EncodeToString(make([]byte, 16)) + + if verifyDiscordSignature(req, shortKey) { + t.Error("expected false for short public key") + } +} + +// TestChannelHandler_Webhook_Discord_NoKey_Returns401 verifies that a Discord +// webhook request is rejected with 401 when no public key is configured in the +// DB and DISCORD_APP_PUBLIC_KEY env var is not set. +func TestChannelHandler_Webhook_Discord_NoKey_Returns401(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + handler := NewChannelHandler(newTestChannelManager()) + + // discordPublicKey: DB returns no rows (no Discord channels with app_public_key). + mock.ExpectQuery(`SELECT COALESCE\(channel_config->>'app_public_key'`). + WillReturnRows(sqlmock.NewRows([]string{"pubkey"})) + + // Ensure env var is not set. + t.Setenv("DISCORD_APP_PUBLIC_KEY", "") + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest(http.MethodPost, "/webhooks/discord", strings.NewReader(`{"type":1}`)) + c.Request.Header.Set("X-Signature-Ed25519", "aabbcc") + c.Request.Header.Set("X-Signature-Timestamp", "1700000000") + c.Params = gin.Params{{Key: "type", Value: "discord"}} + + handler.Webhook(c) + + if w.Code != http.StatusUnauthorized { + t.Errorf("expected 401 (no public key), got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatalf("unmet sqlmock expectations: %v", err) + } +} + +// TestChannelHandler_Webhook_Discord_InvalidSig_Returns401 verifies that a +// Discord webhook with an invalid signature is rejected with 401, even when a +// valid public key is configured. +func TestChannelHandler_Webhook_Discord_InvalidSig_Returns401(t *testing.T) { + pubHex, _ := genDiscordKey(t) // generate key but sign with a DIFFERENT key + _, wrongPriv := genDiscordKey(t) + + mock := setupTestDB(t) + setupTestRedis(t) + handler := NewChannelHandler(newTestChannelManager()) + + // discordPublicKey: DB returns the correct pubHex. + mock.ExpectQuery(`SELECT COALESCE\(channel_config->>'app_public_key'`). + WillReturnRows(sqlmock.NewRows([]string{"pubkey"}).AddRow(pubHex)) + + // Build a request signed with the wrong private key. + req := discordSignedRequest(t, `{"type":1}`, "1700000000", wrongPriv) + req.URL.Path = "/webhooks/discord" + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = req + c.Params = gin.Params{{Key: "type", Value: "discord"}} + + handler.Webhook(c) + + if w.Code != http.StatusUnauthorized { + t.Errorf("expected 401 (invalid sig), got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatalf("unmet sqlmock expectations: %v", err) + } +} + +// TestChannelHandler_Webhook_Discord_ValidSig_PingAccepted verifies that a +// correctly signed Discord PING (type=1) passes the signature gate and the +// handler returns 200 (PING returns nil msg → "ignored" status). +func TestChannelHandler_Webhook_Discord_ValidSig_PingAccepted(t *testing.T) { + pubHex, priv := genDiscordKey(t) + + mock := setupTestDB(t) + setupTestRedis(t) + handler := NewChannelHandler(newTestChannelManager()) + + // discordPublicKey: DB returns pubHex. + mock.ExpectQuery(`SELECT COALESCE\(channel_config->>'app_public_key'`). + WillReturnRows(sqlmock.NewRows([]string{"pubkey"}).AddRow(pubHex)) + + body := `{"type":1}` + req := discordSignedRequest(t, body, "1700000000", priv) + req.URL.Path = "/webhooks/discord" + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = req + c.Params = gin.Params{{Key: "type", Value: "discord"}} + + handler.Webhook(c) + + // Discord PING → ParseWebhook returns nil, nil → handler responds "ignored" + if w.Code != http.StatusOK { + t.Errorf("expected 200 for valid PING, got %d: %s", w.Code, w.Body.String()) + } + if !strings.Contains(w.Body.String(), "ignored") { + t.Errorf("expected body to contain 'ignored', got: %s", w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatalf("unmet sqlmock expectations: %v", err) + } +} From fde90efde5377f2d14c0c30b5e07137abdb7cdcb Mon Sep 17 00:00:00 2001 From: "molecule-ai[bot]" <276602405+molecule-ai[bot]@users.noreply.github.com> Date: Fri, 17 Apr 2026 10:46:09 +0000 Subject: [PATCH 5/5] fix(security): cap discord error response body read at 4096 bytes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unbounded io.ReadAll on the Discord webhook error response body was a LOW OOM risk: a malicious gateway or misconfigured proxy could return a multi-MB body and exhaust agent memory. Cap with io.LimitReader(resp.Body, 4096) — error messages are always short; any extra content is irrelevant noise. Co-Authored-By: Claude Sonnet 4.6 --- platform/internal/channels/discord.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform/internal/channels/discord.go b/platform/internal/channels/discord.go index 44957e39..e640e20f 100644 --- a/platform/internal/channels/discord.go +++ b/platform/internal/channels/discord.go @@ -90,7 +90,7 @@ func (d *DiscordAdapter) SendMessage(ctx context.Context, config map[string]inte // would propagate that token into logs and error responses (#659). return fmt.Errorf("discord: HTTP request failed") } - body, _ := io.ReadAll(resp.Body) + body, _ := io.ReadAll(io.LimitReader(resp.Body, 4096)) resp.Body.Close() // Discord returns 204 No Content on success.