Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
156 changes: 156 additions & 0 deletions cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,20 @@ const {
dispatchAutomationNotifiers,
formatTaskRunNotificationPayload
} = require('./lib/automation');
const {
ALLOWED_EVENTS: WEBHOOK_ALLOWED_EVENTS,
defaultConfigPath: defaultWebhookConfigPath,
loadWebhookConfig,
saveWebhookConfig,
notifyWebhook
} = require('./lib/cli-webhook');
const {
performHandshake: performWsHandshake,
sendText: wsSendText,
sendJson: wsSendJson,
sendClose: wsSendClose,
makeFrameReader: makeWsFrameReader
} = require('./lib/cli-ws-server');
const { buildConfigHealthReport: buildConfigHealthReportCore } = require('./cli/config-health');
const { buildDoctorReport, buildDoctorLegacyPayload, renderDoctorMarkdown } = require('./cli/doctor-core');
const {
Expand Down Expand Up @@ -9764,6 +9778,89 @@ const PUBLIC_WEB_UI_STATIC_ASSETS = new Set([
'session-helpers.mjs'
]);

const TERMINAL_WS_ALLOWED_COMMAND_BASENAMES = new Set([
'codexmate', 'codexmate.cmd', 'codexmate.exe',
'node', 'node.exe',
'echo', 'echo.exe'
]);

function isTerminalWsCommandAllowed(cmd) {
if (!cmd || typeof cmd !== 'string') return false;
const trimmed = cmd.trim();
if (!trimmed) return false;
const base = path.basename(trimmed).toLowerCase();
return TERMINAL_WS_ALLOWED_COMMAND_BASENAMES.has(base);
}
Comment on lines +9787 to +9793
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Command allowlist can be bypassed via path-prefixed binaries

Using path.basename(cmd) means /tmp/node or ./codexmate passes the allowlist. That defeats the whitelist intent.

Suggested fix
 function isTerminalWsCommandAllowed(cmd) {
     if (!cmd || typeof cmd !== 'string') return false;
     const trimmed = cmd.trim();
     if (!trimmed) return false;
-    const base = path.basename(trimmed).toLowerCase();
-    return TERMINAL_WS_ALLOWED_COMMAND_BASENAMES.has(base);
+    if (trimmed.includes('/') || trimmed.includes('\\')) return false;
+    const base = trimmed.toLowerCase();
+    return TERMINAL_WS_ALLOWED_COMMAND_BASENAMES.has(base);
 }

Also applies to: 10096-10101

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cli.js` around lines 10055 - 10061, The allowlist check in
isTerminalWsCommandAllowed (and the duplicate at the other occurrence) uses
path.basename so path-prefixed binaries like "/tmp/node" bypass the whitelist;
fix by extracting the command's first token (split trimmed on whitespace) and
immediately rejecting if that token contains any path separators or is
path-prefixed (e.g., starts with '.' or has a dirname other than '.'), then
compare the plain basename against TERMINAL_WS_ALLOWED_COMMAND_BASENAMES; apply
the same change to the other function at 10096-10101 to ensure only bare command
names (no paths) are allowed.


function attachTerminalSession(socket) {
let child = null;
let closed = false;
const close = () => {
if (closed) return;
closed = true;
if (child && !child.killed) {
try { child.kill(); } catch (_) {}
}
try { wsSendClose(socket, 1000); } catch (_) {}
};
socket.on('error', close);
socket.on('end', close);
socket.on('close', close);

const reader = makeWsFrameReader((message) => {
if (closed) return;
let parsed;
try { parsed = JSON.parse(message); } catch (_) { return; }
if (!parsed || typeof parsed !== 'object') return;
if (parsed.type === 'run') {
if (child) {
wsSendJson(socket, { type: 'error', message: 'busy' });
return;
}
const cmd = String(parsed.cmd || '').trim();
const argv = Array.isArray(parsed.args) ? parsed.args.map(String) : [];
if (!isTerminalWsCommandAllowed(cmd)) {
wsSendJson(socket, { type: 'error', message: 'command not allowed' });
wsSendClose(socket, 1008);
return;
}
try {
child = spawn(cmd, argv, {
cwd: process.cwd(),
env: process.env,
windowsHide: true,
shell: false
});
} catch (e) {
wsSendJson(socket, { type: 'error', message: e && e.message ? e.message : String(e) });
wsSendClose(socket, 1011);
return;
}
wsSendJson(socket, { type: 'started', pid: child.pid });
child.stdout.on('data', (chunk) => {
wsSendJson(socket, { type: 'data', stream: 'stdout', text: chunk.toString('utf-8') });
});
child.stderr.on('data', (chunk) => {
wsSendJson(socket, { type: 'data', stream: 'stderr', text: chunk.toString('utf-8') });
});
child.on('error', (err) => {
wsSendJson(socket, { type: 'error', message: err && err.message ? err.message : String(err) });
});
child.on('exit', (code, signal) => {
wsSendJson(socket, { type: 'exit', code: code, signal: signal || '' });
wsSendClose(socket, 1000);
});
} else if (parsed.type === 'kill') {
if (child && !child.killed) {
try { child.kill(); } catch (_) {}
}
} else if (parsed.type === 'ping') {
wsSendJson(socket, { type: 'pong' });
}
}, close);
socket.on('data', reader);
}

function createWebServer({ htmlPath, assetsDir, webDir, host, port, openBrowser }) {
const connections = new Set();
const probeWebUiReadiness = (callback) => {
Expand Down Expand Up @@ -10133,6 +10230,10 @@ function createWebServer({ htmlPath, assetsDir, webDir, host, port, openBrowser
break;
case 'apply-claude-md-file':
result = applyClaudeMdFile(params || {});
if (result && !result.error) {
const mdTarget = (params && params.targetPath) ? String(params.targetPath) : 'CLAUDE.md';
notifyWebhook('claude-md-edit', 'CLAUDE.md modified: ' + mdTarget, { targetPath: mdTarget }).catch(function () {});
}
break;
case 'preview-agents-diff':
result = buildAgentsDiff(params || {});
Expand Down Expand Up @@ -10208,7 +10309,32 @@ function createWebServer({ htmlPath, assetsDir, webDir, host, port, openBrowser
break;
case 'apply-claude-config':
result = applyToClaudeSettings(params.config);
if (result && !result.error) {
const cfgName = (params && params.config && typeof params.config.name === 'string') ? params.config.name : '';
const cfgFrom = (params && typeof params.previousName === 'string') ? params.previousName : '';
const summary = cfgFrom
? ('Provider switched: ' + cfgFrom + ' -> ' + cfgName)
: ('Provider applied: ' + cfgName);
notifyWebhook('provider-switch', summary, { name: cfgName, previousName: cfgFrom }).catch(function () {});
}
break;
case 'get-webhook-config':
result = loadWebhookConfig();
break;
case 'set-webhook-config':
result = saveWebhookConfig(params && params.config ? params.config : {});
break;
case 'test-webhook': {
const overrideCfg = params && params.config ? params.config : null;
const probe = await notifyWebhook(
'provider-switch',
'codexmate webhook test ping',
{ test: true },
overrideCfg ? { config: overrideCfg } : {}
);
result = probe;
break;
}
case 'export-claude-share':
result = buildClaudeSharePayload(params && params.config ? params.config : {});
break;
Expand Down Expand Up @@ -10723,6 +10849,36 @@ function createWebServer({ htmlPath, assetsDir, webDir, host, port, openBrowser
socket.on('close', () => connections.delete(socket));
});

server.on('upgrade', (req, socket, head) => {
const requestPath = (req.url || '/').split('?')[0];
if (requestPath !== '/ws/terminal') {
try { socket.destroy(); } catch (_) {}
return;
}
const remoteAddr = socket && socket.remoteAddress ? socket.remoteAddress : '';
const isLoopback = !remoteAddr
|| remoteAddr === '127.0.0.1'
|| remoteAddr === '::1'
|| remoteAddr === '::ffff:127.0.0.1';
if (!isLoopback) {
const expected = typeof process.env.CODEXMATE_HTTP_TOKEN === 'string'
? process.env.CODEXMATE_HTTP_TOKEN.trim()
: '';
const headers = req && req.headers ? req.headers : {};
const auth = typeof headers.authorization === 'string' ? headers.authorization.trim() : '';
const match = auth ? auth.match(/^bearer\s+(.+)$/i) : null;
const token = match && match[1]
? match[1].trim()
: (typeof headers['x-codexmate-token'] === 'string' ? String(headers['x-codexmate-token']).trim() : '');
if (!expected || token !== expected) {
try { socket.destroy(); } catch (_) {}
return;
}
}
Comment on lines +10858 to +10877
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

Loopback trust on /ws/terminal enables cross-site websocket command execution

The endpoint skips token checks for loopback clients. Any website opened in the user’s browser can initiate a WebSocket to 127.0.0.1 and send terminal commands (including node ...).

Require explicit auth for terminal WS even on loopback (and/or strict Origin validation plus CSRF-style session token tied to the Web UI page load).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cli.js` around lines 11129 - 11148, Currently the code uses isLoopback to
skip auth for loopback sockets; change this so the token check always runs for
the `/ws/terminal` connection: always read process.env.CODEXMATE_HTTP_TOKEN,
extract token from req.headers.authorization or req.headers['x-codexmate-token']
(same logic already in the diff) and compare it to expected, and only destroy
the socket and return on mismatch; remove or disable the branch that bypasses
validation when isLoopback is true. Optionally, if you want defense-in-depth,
also validate req.headers.origin against an allowlist and require a CSRF/session
token tied to the Web UI page load, but the immediate fix is to stop bypassing
auth for loopback by making the token validation unconditional.

if (!performWsHandshake(req, socket)) return;
attachTerminalSession(socket);
Comment on lines +10878 to +10879
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Upgrade head bytes are dropped, which can lose the first WS frame

server.on('upgrade') ignores head; if the client’s first frame arrives with upgrade, the command can be lost.

Suggested fix
-function attachTerminalSession(socket) {
+function attachTerminalSession(socket, initialHead = null) {
     let child = null;
@@
     const reader = makeWsFrameReader((message) => {
@@
     }, close);
     socket.on('data', reader);
+    if (initialHead && initialHead.length > 0) {
+        reader(initialHead);
+    }
 }
@@
-        if (!performWsHandshake(req, socket)) return;
-        attachTerminalSession(socket);
+        if (!performWsHandshake(req, socket)) return;
+        attachTerminalSession(socket, head);
     });

Also applies to: 10078-10130

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cli.js` around lines 11149 - 11150, The upgrade handler is dropping the
initial bytes because the listener ignores the upgrade "head"; modify the
upgrade flow so the raw head is preserved and passed into the WebSocket
handling: update performWsHandshake(req, socket) to accept the head
(performWsHandshake(req, socket, head)) or ensure the head is pushed back onto
the socket (e.g., socket.unshift(head)) before attaching the session, and then
pass the head (or use the pushed-back data) into attachTerminalSession(socket)
or the WebSocket server's handleUpgrade call so the client's first frame is not
lost; locate and update both performWsHandshake and attachTerminalSession call
sites (also apply similar changes in the other occurrences around the
10078-10130 range).

});

server.once('error', (err) => {
if (err && err.code === 'EADDRINUSE') {
console.error(`! 启动失败: 端口 ${port} 已被占用,可能有残留的 codexmate run 实例。`);
Expand Down
126 changes: 126 additions & 0 deletions lib/cli-webhook.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
const path = require('path');
const fs = require('fs');
const http = require('http');
const https = require('https');
const os = require('os');

const ALLOWED_EVENTS = ['provider-switch', 'claude-md-edit'];
const DEFAULT_TIMEOUT_MS = 5000;

function defaultConfigPath() {
return path.join(os.homedir(), '.codex', 'codexmate-webhook.json');
}

function normalizeConfig(cfg) {
const out = { enabled: false, url: '', events: ALLOWED_EVENTS.slice() };
if (!cfg || typeof cfg !== 'object') return out;
out.enabled = !!cfg.enabled;
out.url = typeof cfg.url === 'string' ? cfg.url.trim() : '';
if (Array.isArray(cfg.events)) {
const filtered = cfg.events.filter(function (e) { return ALLOWED_EVENTS.indexOf(e) !== -1; });
out.events = filtered.length ? filtered : ALLOWED_EVENTS.slice();
Comment on lines +19 to +21
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Preserve an explicitly empty event list.

normalizeConfig() currently rewrites events: [] to ALLOWED_EVENTS, so users cannot persist “no events selected”. That breaks the new event-filter feature and can resume deliveries after a save/reload.

Suggested fix
     if (Array.isArray(cfg.events)) {
         const filtered = cfg.events.filter(function (e) { return ALLOWED_EVENTS.indexOf(e) !== -1; });
-        out.events = filtered.length ? filtered : ALLOWED_EVENTS.slice();
+        out.events = filtered;
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (Array.isArray(cfg.events)) {
const filtered = cfg.events.filter(function (e) { return ALLOWED_EVENTS.indexOf(e) !== -1; });
out.events = filtered.length ? filtered : ALLOWED_EVENTS.slice();
if (Array.isArray(cfg.events)) {
const filtered = cfg.events.filter(function (e) { return ALLOWED_EVENTS.indexOf(e) !== -1; });
out.events = filtered;
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/cli-webhook.js` around lines 19 - 21, normalizeConfig currently replaces
an explicitly empty cfg.events ([]) with ALLOWED_EVENTS; change the branch that
handles Array.isArray(cfg.events) so it preserves an empty array: if
cfg.events.length === 0 set out.events = [] (preserve the explicit "no events"
choice), otherwise compute filtered = cfg.events.filter(e =>
ALLOWED_EVENTS.indexOf(e) !== -1) and set out.events = filtered.length ?
filtered : ALLOWED_EVENTS.slice(); reference normalizeConfig, cfg.events,
out.events, ALLOWED_EVENTS and the filtered variable to locate the change.

}
return out;
}

function loadWebhookConfig(filePath) {
const target = filePath || defaultConfigPath();
try {
if (!fs.existsSync(target)) {
return normalizeConfig({});
}
const raw = fs.readFileSync(target, 'utf-8');
return normalizeConfig(JSON.parse(raw));
} catch (_) {
return normalizeConfig({});
}
}

function saveWebhookConfig(cfg, filePath) {
const target = filePath || defaultConfigPath();
const normalized = normalizeConfig(cfg);
try {
fs.mkdirSync(path.dirname(target), { recursive: true });
} catch (_) {}
fs.writeFileSync(target, JSON.stringify(normalized, null, 2), 'utf-8');
return normalized;
}

function postJson(targetUrl, payload, timeoutMs) {
return new Promise(function (resolve) {
let parsed;
try {
parsed = new URL(targetUrl);
} catch (_) {
resolve({ ok: false, error: 'invalid-url' });
return;
}
const transport = parsed.protocol === 'https:' ? https : http;
const body = JSON.stringify(payload || {});
let req;
try {
req = transport.request({
method: 'POST',
protocol: parsed.protocol,
hostname: parsed.hostname,
port: parsed.port || (parsed.protocol === 'https:' ? 443 : 80),
path: (parsed.pathname || '/') + (parsed.search || ''),
headers: {
'Content-Type': 'application/json; charset=utf-8',
'Content-Length': Buffer.byteLength(body, 'utf-8'),
'User-Agent': 'codexmate-webhook/1'
}
}, function (res) {
let raw = '';
res.on('data', function (chunk) {
if (raw.length < 1024) raw += chunk.toString('utf-8');
});
res.on('end', function () {
const status = res.statusCode || 0;
resolve({ ok: status >= 200 && status < 300, status: status, body: raw.slice(0, 200) });
});
});
} catch (e) {
resolve({ ok: false, error: e && e.message ? e.message : String(e) });
return;
}
const wait = Number.isFinite(timeoutMs) && timeoutMs > 0 ? timeoutMs : DEFAULT_TIMEOUT_MS;
req.setTimeout(wait, function () { req.destroy(new Error('timeout')); });
req.on('error', function (err) { resolve({ ok: false, error: err && err.message ? err.message : String(err) }); });
req.write(body);
req.end();
});
}

function buildPayload(event, summary, details) {
return {
event: String(event || ''),
summary: String(summary || ''),
operator: process.env.USER || process.env.USERNAME || (os.userInfo && os.userInfo().username) || '',
timestamp: new Date().toISOString(),
details: details && typeof details === 'object' ? details : {}
};
}

function notifyWebhook(event, summary, details, options) {
const opts = options || {};
const cfg = opts.config ? normalizeConfig(opts.config) : loadWebhookConfig(opts.filePath);
if (!cfg.enabled || !cfg.url) {
return Promise.resolve({ ok: false, skipped: true, reason: 'disabled' });
}
if (cfg.events.indexOf(event) === -1) {
return Promise.resolve({ ok: false, skipped: true, reason: 'event-filtered' });
}
return postJson(cfg.url, buildPayload(event, summary, details), opts.timeoutMs);
}

module.exports = {
ALLOWED_EVENTS,
defaultConfigPath,
normalizeConfig,
loadWebhookConfig,
saveWebhookConfig,
notifyWebhook,
buildPayload,
postJson
};
Loading
Loading