From 8b32975eaf5cfa24661243460f9086eaacd833aa Mon Sep 17 00:00:00 2001 From: jmcqueen Date: Mon, 6 Jul 2026 15:48:54 -0400 Subject: [PATCH] Harden agent bridge + command inputs - Agent hello handshake: agent (v1.2.0) sends {type, version, capabilities} on connect; bot logs "Agent hello: v1.2.0 (capabilities: insecure, hello)" and exposes getAgentInfo(). Backward-compatible with older agents. - proxyRequest tracks method + url per request; all error paths (agent errors, timeouts, disconnect rejects, send failures) now include "(for METHOD URL)" so the failing endpoint is unambiguous - Add requireAgent(bot) preflight; buildStore/stageStore/migrateStore reject up front when the agent is disconnected instead of failing mid-flow after partial Webex mutations - Add parseStoreNumber (^\d{1,5}$) and wire into store-number commands with proper usage messages; add loose email-shape check to /userInfo - Fix pre-existing catch(_e) lint warning in remoteAgent.js (bare catch) Co-authored-by: Cursor --- docker/remote-agent/package.json | 2 +- docker/remote-agent/remoteAgent.js | 23 +++++++++++- src/commands/buildStore.js | 7 ++-- src/commands/helpers.js | 35 ++++++++++++++++++ src/commands/migrateStore.js | 7 ++-- src/commands/stageStore.js | 7 ++-- src/commands/storeInfo.js | 6 +-- src/commands/userInfo.js | 11 +++++- src/services/websocket.js | 59 ++++++++++++++++++++++++++---- 9 files changed, 134 insertions(+), 23 deletions(-) diff --git a/docker/remote-agent/package.json b/docker/remote-agent/package.json index f88a18e..b5a2ea9 100644 --- a/docker/remote-agent/package.json +++ b/docker/remote-agent/package.json @@ -1,6 +1,6 @@ { "name": "wbxprov-remote-agent", - "version": "1.1.0", + "version": "1.2.0", "private": true, "description": "Standalone container for the wbxStoreProvision remote agent (WebSocket proxy for Store Info Web from an internal network).", "main": "remoteAgent.js", diff --git a/docker/remote-agent/remoteAgent.js b/docker/remote-agent/remoteAgent.js index 2e8f7ef..871c1a3 100644 --- a/docker/remote-agent/remoteAgent.js +++ b/docker/remote-agent/remoteAgent.js @@ -1,6 +1,7 @@ const WebSocket = require('ws'); const axios = require('axios'); const https = require('https'); +const { version: AGENT_VERSION } = require('./package.json'); require('dotenv').config(); const WS_URL = process.env.WS_URL; @@ -15,6 +16,10 @@ const INITIAL_BACKOFF_MS = 2000; const MAX_BACKOFF_MS = 60000; const PROXY_TIMEOUT_MS = 30000; +// Capabilities advertised in the hello handshake so the bot can log what +// this agent understands. Keep additive; older bots ignore unknown caps. +const AGENT_CAPABILITIES = ['insecure', 'hello']; + // Shared https.Agent used only when the bot flags a proxied request with // `insecure: true` (e.g. reaching Store Info Web, which is served with an // internal-CA cert Node doesn't know about). All other requests use axios's @@ -42,8 +47,22 @@ function connect() { ws = new WebSocket(WS_URL, buildClientOptions()); ws.on('open', () => { - console.log('Remote Agent connected to wbxStoreProvision'); + console.log(`Remote Agent v${AGENT_VERSION} connected to wbxStoreProvision`); reconnectAttempts = 0; + // One-shot hello so the bot can log which agent version + capabilities + // are on the other end of the socket. Fire-and-forget — the bot + // tolerates its absence for backward compatibility with v1.1 agents. + try { + ws.send( + JSON.stringify({ + type: 'hello', + version: AGENT_VERSION, + capabilities: AGENT_CAPABILITIES, + }), + ); + } catch (err) { + console.error('Failed to send hello:', err.message); + } }); ws.on('message', async (data) => { @@ -118,7 +137,7 @@ function shutdownRemote(signal) { if (ws) { try { ws.close(); - } catch (_e) { + } catch { // ignore close errors during shutdown } } diff --git a/src/commands/buildStore.js b/src/commands/buildStore.js index 7f62d75..6ca27d8 100644 --- a/src/commands/buildStore.js +++ b/src/commands/buildStore.js @@ -2,16 +2,17 @@ import { logger } from '../logger.js'; import { getStoreInfo } from '../integrations/siw.js'; import { findWebexUser } from '../webex/users.js'; import { buildStoreInfoCard } from '../cards/storeInfoCard.js'; -import { parseStoreArg, storeEmail } from './helpers.js'; +import { parseStoreNumber, requireAgent, storeEmail } from './helpers.js'; export function register(framework) { framework.hears( /\/buildstore/i, async (bot, trigger) => { logger.info(`${trigger.person.displayName} ran the buildStore command.`); - const storeNumber = parseStoreArg(trigger); + if (!requireAgent(bot)) return; + const storeNumber = parseStoreNumber(trigger); if (!storeNumber) { - bot.say("You didn't enter a store number."); + bot.say('Usage: `/buildStore ` — e.g. `/buildStore 499`'); return; } try { diff --git a/src/commands/helpers.js b/src/commands/helpers.js index 86af743..3583e9e 100644 --- a/src/commands/helpers.js +++ b/src/commands/helpers.js @@ -1,7 +1,42 @@ +import { isAgentConnected } from '../services/websocket.js'; + export function storeEmail(storeNumber) { return `ae${String(storeNumber).padStart(5, '0')}@ae.com`; } +/** + * Preflight check for commands that require the on-prem remote agent + * (anything touching SIW or Google). Returns true when it's safe to + * proceed; returns false AND already sent the user a helpful message + * when the agent is not connected. Caller should `if (!requireAgent(bot)) + * return;` at the top of the handler. + * + * Failing fast at command entry prevents partial provisioning — otherwise + * a flow would run a few Webex mutations, then blow up mid-way on the + * first proxyRequest. + */ +export function requireAgent(bot) { + if (isAgentConnected()) return true; + bot.say( + 'markdown', + 'The on-prem remote agent is not connected — no SIW or Google calls can be made. ' + + 'Start the agent on the internal host and retry.', + ); + return false; +} + +/** + * Parse and validate a numeric store argument. Returns the digits as a + * string when valid (1-5 digits, matches AE store numbering), or `null` + * when missing/malformed. Callers should show a usage message on null. + */ +export function parseStoreNumber(trigger) { + const raw = parseStoreArg(trigger); + if (!raw) return null; + if (!/^\d{1,5}$/.test(raw)) return null; + return raw; +} + /** * Pull the first argument out of a webex-node-bot-framework trigger. * diff --git a/src/commands/migrateStore.js b/src/commands/migrateStore.js index e774066..5f84ed8 100644 --- a/src/commands/migrateStore.js +++ b/src/commands/migrateStore.js @@ -2,16 +2,17 @@ import { logger } from '../logger.js'; import { getStoreInfo } from '../integrations/siw.js'; import { findWebexUser } from '../webex/users.js'; import { buildStoreInfoCard } from '../cards/storeInfoCard.js'; -import { parseStoreArg, storeEmail } from './helpers.js'; +import { parseStoreNumber, requireAgent, storeEmail } from './helpers.js'; export function register(framework) { framework.hears( /\/migratestore/i, async (bot, trigger) => { logger.info(`${trigger.person.displayName} ran the migrateStore command.`); - const storeNumber = parseStoreArg(trigger); + if (!requireAgent(bot)) return; + const storeNumber = parseStoreNumber(trigger); if (!storeNumber) { - bot.say("You didn't enter a store number."); + bot.say('Usage: `/migrateStore ` — e.g. `/migrateStore 499`'); return; } try { diff --git a/src/commands/stageStore.js b/src/commands/stageStore.js index d059a71..a3901d2 100644 --- a/src/commands/stageStore.js +++ b/src/commands/stageStore.js @@ -2,16 +2,17 @@ import { logger } from '../logger.js'; import { getStoreInfo } from '../integrations/siw.js'; import { findWebexUser } from '../webex/users.js'; import { buildStoreInfoCard } from '../cards/storeInfoCard.js'; -import { parseStoreArg, storeEmail } from './helpers.js'; +import { parseStoreNumber, requireAgent, storeEmail } from './helpers.js'; export function register(framework) { framework.hears( /\/stagestore/i, async (bot, trigger) => { logger.info(`${trigger.person.displayName} ran the stageStore command.`); - const storeNumber = parseStoreArg(trigger); + if (!requireAgent(bot)) return; + const storeNumber = parseStoreNumber(trigger); if (!storeNumber) { - bot.say("You didn't enter a store number."); + bot.say('Usage: `/stageStore ` — e.g. `/stageStore 499`'); return; } try { diff --git a/src/commands/storeInfo.js b/src/commands/storeInfo.js index 2225637..c6f5192 100644 --- a/src/commands/storeInfo.js +++ b/src/commands/storeInfo.js @@ -1,15 +1,15 @@ import { logger } from '../logger.js'; import { buildUserInfoCard } from '../cards/userInfoCard.js'; -import { parseStoreArg, storeEmail } from './helpers.js'; +import { parseStoreNumber, storeEmail } from './helpers.js'; export function register(framework) { framework.hears( /\/storeinfo/i, async (bot, trigger) => { logger.info(`${trigger.person.displayName} ran the storeInfo command.`); - const storeNumber = parseStoreArg(trigger); + const storeNumber = parseStoreNumber(trigger); if (!storeNumber) { - bot.say("You didn't enter a store number."); + bot.say('Usage: `/storeInfo ` — e.g. `/storeInfo 792`'); return; } try { diff --git a/src/commands/userInfo.js b/src/commands/userInfo.js index 8e6c310..ca9203f 100644 --- a/src/commands/userInfo.js +++ b/src/commands/userInfo.js @@ -2,6 +2,11 @@ import { logger } from '../logger.js'; import { buildUserInfoCard } from '../cards/userInfoCard.js'; import { parseStoreArg } from './helpers.js'; +// Loose email format check — anything with `local@domain` shape. Intent +// is to catch obvious typos like `/userInfo joe` early rather than to +// enforce full RFC 5322 (Webex will reject bad addresses anyway). +const EMAIL_SHAPE = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + export function register(framework) { framework.hears( /\/userInfo/i, @@ -9,7 +14,11 @@ export function register(framework) { logger.info(`${trigger.person.displayName} ran the userInfo command.`); const email = parseStoreArg(trigger); if (!email) { - bot.say("You didn't enter a user email."); + bot.say('Usage: `/userInfo ` — e.g. `/userInfo mcqueenj@ae.com`'); + return; + } + if (!EMAIL_SHAPE.test(email)) { + bot.say(`\`${email}\` doesn't look like an email address.`); return; } try { diff --git a/src/services/websocket.js b/src/services/websocket.js index bcf8567..faf453c 100644 --- a/src/services/websocket.js +++ b/src/services/websocket.js @@ -21,6 +21,12 @@ let connectedAgent = null; let pingInterval = null; const pendingRequests = new Map(); +// Metadata received from the agent's hello handshake. Reset on +// (dis)connect. `hello: null` means either the agent hasn't sent hello yet +// or it's an older agent (<1.2.0) that doesn't know about hello — treat as +// unknown, not as an error. +let agentInfo = { hello: null }; + function extractToken(req) { const auth = req.headers['authorization']; if (auth && /^Bearer\s+/i.test(auth)) { @@ -35,9 +41,9 @@ function extractToken(req) { } function rejectPending(reason) { - for (const { reject, timeout } of pendingRequests.values()) { + for (const { reject, timeout, method, url } of pendingRequests.values()) { clearTimeout(timeout); - reject(new Error(reason)); + reject(new Error(`${reason} (in-flight: ${method} ${url})`)); } pendingRequests.clear(); } @@ -51,20 +57,40 @@ function handleAgentMessage(data) { return; } + // Hello handshake — one-shot metadata from the agent right after + // connect. Anything without a requestId that has type === 'hello' is + // treated as metadata; unknown message shapes are logged and dropped. + if (response.type === 'hello') { + agentInfo.hello = { + version: response.version ?? 'unknown', + capabilities: Array.isArray(response.capabilities) ? response.capabilities : [], + }; + logger.info( + `Agent hello: v${agentInfo.hello.version} (capabilities: ${agentInfo.hello.capabilities.join(', ') || 'none'})`, + ); + return; + } + const { requestId } = response; if (!requestId || !pendingRequests.has(requestId)) { logger.warn('Received response with unknown requestId:', requestId); return; } - const { resolve, reject, timeout } = pendingRequests.get(requestId); + const pending = pendingRequests.get(requestId); + const { resolve, reject, timeout, method, url } = pending; clearTimeout(timeout); pendingRequests.delete(requestId); if (response.error) { - const err = new Error(response.error); + // Prefix the agent's error message with the specific method+URL that + // failed so the caller doesn't have to reverse-engineer which + // proxied request in a Promise.all threw. + const err = new Error(`${response.error} (for ${method} ${url})`); err.status = response.status; err.data = response.data; + err.method = method; + err.url = url; reject(err); } else { resolve(response); @@ -111,6 +137,9 @@ export function startWebSocketServer() { logger.info('Remote agent connected', { remote }); connectedAgent = ws; + // Clear any stale hello metadata from a previous connection; the + // new agent will (re)send its own hello moments later. + agentInfo = { hello: null }; ws.isAlive = true; ws.on('message', handleAgentMessage); @@ -169,6 +198,15 @@ export function isAgentConnected() { return !!connectedAgent && connectedAgent.readyState === WebSocket.OPEN; } +/** + * Metadata reported by the connected agent via the hello handshake. Returns + * `{ hello: null }` if the agent hasn't sent hello yet or is an older + * version that doesn't implement it. Never throws. + */ +export function getAgentInfo() { + return { ...agentInfo }; +} + export class AgentNotConnectedError extends Error { constructor(message = 'No remote SIW agent connected') { super(message); @@ -193,13 +231,20 @@ export function proxyRequest(requestConfig) { return; } + const method = (requestConfig.method || 'GET').toUpperCase(); + const url = requestConfig.url || '(no url)'; + const requestId = `req_${Date.now()}_${Math.random().toString(36).slice(2)}`; const timeout = setTimeout(() => { pendingRequests.delete(requestId); - reject(new Error(`Proxy request timeout after ${PROXY_REQUEST_TIMEOUT_MS / 1000}s`)); + reject( + new Error( + `Proxy request timeout after ${PROXY_REQUEST_TIMEOUT_MS / 1000}s (for ${method} ${url})`, + ), + ); }, PROXY_REQUEST_TIMEOUT_MS); - pendingRequests.set(requestId, { resolve, reject, timeout }); + pendingRequests.set(requestId, { resolve, reject, timeout, method, url }); const payload = JSON.stringify({ action: 'proxyRequest', @@ -211,7 +256,7 @@ export function proxyRequest(requestConfig) { if (err) { clearTimeout(timeout); pendingRequests.delete(requestId); - reject(err); + reject(new Error(`Failed to send to agent: ${err.message} (for ${method} ${url})`)); } }); });