Skip to content

Commit ec25a28

Browse files
authored
Merge pull request #73 from pmoses-s1/fix/mcp-ssrf-origin-guard-and-monitor-pip
security: harden s1-secops MCP server and monitor plugins
2 parents b73ea3d + 4c4095f commit ec25a28

14 files changed

Lines changed: 416 additions & 46 deletions

README.md

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ ai-siem/ # AI SIEM core structure (260+ components)
3737
│ └── sentinelone/ # 17 official marketplace parsers (*.conf + metadata)
3838
├── workflows/ # Automated playbooks and responses (3 workflows with metadata)
3939
├── plugins/ # Claude plugins (skills bundled for Cowork / Claude Code)
40-
│ └── s1-secops-skills/ # 7 SentinelOne SecOps skills + built .plugin/.skill bundles
40+
│ └── s1-secops-skills/ # 8 SentinelOne SecOps skills + built .plugin/.skill bundles
4141
└── mcp/ # SentinelOne MCP server (Node.js) + container build
4242
```
4343

@@ -97,12 +97,22 @@ The monitors directory contains Python scripts for use with the Dataset Agent:
9797
- **powerquerymonitor.py** - PowerQuery monitoring capabilities
9898

9999
### Installation Steps
100-
1. Copy monitor files to Dataset Agent directory:
100+
1. Install the monitor Python dependencies once, before enabling any monitor.
101+
The monitors no longer install packages at runtime (that would run `pip
102+
install` as root on every agent restart), so the operator must install them
103+
out of band into the agent's Python environment:
104+
```bash
105+
pip install --require-hashes -r monitors/requirements.txt
106+
```
107+
See `monitors/requirements.txt` for the pinned versions and for how to
108+
regenerate a fully hash-locked file on a trusted build host.
109+
110+
2. Copy monitor files to Dataset Agent directory:
101111
```bash
102112
cp monitors/*.py /usr/share/scalyr-agent-2/py/scalyr_agent/builtin_monitors/
103113
```
104114

105-
2. Configure the agent by editing `/etc/scalyr-agent-2/agent.log`:
115+
3. Configure the agent by editing `/etc/scalyr-agent-2/agent.log`:
106116
```json
107117
monitors: [
108118
{
@@ -116,7 +126,7 @@ The monitors directory contains Python scripts for use with the Dataset Agent:
116126
]
117127
```
118128

119-
3. Start the Dataset Agent:
129+
4. Start the Dataset Agent:
120130
```bash
121131
scalyr-agent-2 start
122132
```
@@ -167,6 +177,21 @@ See [`workflows/community/README.md`](workflows/community/README.md) for the ful
167177

168178
---
169179

180+
## SentinelOne SecOps skills
181+
182+
Eight Claude skills for SentinelOne SecOps, bundled in `plugins/s1-secops-skills/` for Cowork and Claude Code:
183+
184+
- **powerquery**: author, debug, and run PowerQuery (Deep Visibility / SDL hunts, STAR rule bodies).
185+
- **mgmt-console-api**: query and act on the S1 Management Console API (threats, alerts, agents, IOCs, RemoteOps, UAM).
186+
- **sdl-api**: read data and manage SDL configuration files (parsers, dashboards, lookups) via the SDL API.
187+
- **sdl-dashboard**: create, edit, and deploy SDL dashboard JSON.
188+
- **sdl-log-parser**: author and validate SDL log parsers.
189+
- **sdl-solutions**: deploy packaged SDL solutions (source onboarding, UEBA, RBA, detection-as-code, alert-noise reduction).
190+
- **hyperautomation**: build and export Hyperautomation (SOAR) workflow JSON.
191+
- **soc-investigator**: autonomous DFIR investigation over S1 alerts and third-party sources.
192+
193+
See `plugins/s1-secops-skills/docs/skills.md` for details.
194+
170195
## Getting help
171196
Open an issue. Office hours TBD based on requests.
172197

mcp/s1-secops-mcp/lib/http-transport.js

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,32 @@ function summarizeParams(params) {
4242
return '';
4343
}
4444

45+
/**
46+
* DNS-rebinding guard for the no-auth HTTP mode.
47+
*
48+
* The Host header must match the address the server is bound to (or a loopback
49+
* literal when bound to loopback). A DNS-rebinding attack arrives with the
50+
* attacker's Host (e.g. "rebind.attacker.example:8765"), which will not match
51+
* the loopback bind. When bound to a wildcard address we cannot know the
52+
* intended FQDN, so we accept any Host and rely on the Origin check to block
53+
* browsers. HTTP/1.1 mandates a Host header, so a missing one is rejected.
54+
*/
55+
function isAllowedHost(hostHeader, bindHost, bindPort) {
56+
if (!hostHeader) return false;
57+
const wildcard = ['0.0.0.0', '::', ''].includes(String(bindHost));
58+
// Parse "hostname" or "hostname:port" (including IPv6 "[::1]:port").
59+
const m = /^(\[[^\]]+\]|[^:]+)(?::(\d+))?$/.exec(hostHeader.trim());
60+
if (!m) return false;
61+
const name = m[1].toLowerCase().replace(/^\[|\]$/g, '');
62+
const port = m[2];
63+
if (port !== undefined && Number(port) !== Number(bindPort)) return false;
64+
if (wildcard) return true;
65+
const loopback = ['127.0.0.1', '::1', 'localhost'];
66+
const bind = String(bindHost).toLowerCase();
67+
const allowed = new Set([bind, ...(loopback.includes(bind) ? loopback : [])]);
68+
return allowed.has(name);
69+
}
70+
4571
function readBody(req) {
4672
return new Promise((resolve, reject) => {
4773
let size = 0;
@@ -84,7 +110,7 @@ function sendText(res, status, text) {
84110
res.end(text);
85111
}
86112

87-
async function handleMcp(req, res, dispatch, clientIp) {
113+
async function handleMcp(req, res, dispatch, clientIp, bindHost, bindPort) {
88114
// Auth check (only if any tokens are configured)
89115
let clientName = '-';
90116
if (isAuthConfigured()) {
@@ -95,6 +121,26 @@ async function handleMcp(req, res, dispatch, clientIp) {
95121
return;
96122
}
97123
} else {
124+
// SECURITY (no-auth mode): a configured bearer token normally blocks
125+
// browser-driven requests, because a web page cannot forge an Authorization
126+
// header. The documented no-auth loopback mode has no such gate, so a
127+
// malicious page — or a DNS-rebinding attack — on the operator's own
128+
// workstation could otherwise drive this server via cross-origin fetch()
129+
// and invoke every state-changing tool with the tenant API token. Per the
130+
// MCP Streamable HTTP spec we reject any browser Origin and validate the
131+
// Host header to defeat DNS rebinding. Non-browser MCP clients (stdio
132+
// bridge, curl, Claude Desktop) omit Origin and send a matching Host.
133+
const origin = req.headers['origin'];
134+
if (origin) {
135+
audit(`${new Date().toISOString()} | ${clientIp} | - | - | 403 cross-origin (${origin})`);
136+
sendJson(res, 403, makeErr(null, -32001, 'Cross-origin requests are not allowed'));
137+
return;
138+
}
139+
if (!isAllowedHost(req.headers['host'], bindHost, bindPort)) {
140+
audit(`${new Date().toISOString()} | ${clientIp} | - | - | 403 bad-host (${req.headers['host'] || '-'})`);
141+
sendJson(res, 403, makeErr(null, -32001, 'Invalid Host header'));
142+
return;
143+
}
98144
clientName = `anon@${clientIp}`;
99145
}
100146

@@ -178,7 +224,7 @@ export async function startHttp(dispatch, { port, host, path }) {
178224
// MCP endpoint
179225
if (req.url === path) {
180226
if (req.method === 'POST') {
181-
await handleMcp(req, res, dispatch, clientIp);
227+
await handleMcp(req, res, dispatch, clientIp, host, port);
182228
return;
183229
}
184230
if (req.method === 'GET' || req.method === 'DELETE') {

mcp/s1-secops-mcp/lib/s1.js

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,31 @@ function jwt() {
2424
return tok;
2525
}
2626

27+
/**
28+
* Build a validated absolute URL for an S1 Mgmt API call.
29+
*
30+
* SECURITY: `path` frequently originates from an LLM tool call
31+
* (s1_api_get/post/put/delete/patch) and must never be able to change the
32+
* request authority. Bare string concatenation (`${base()}${path}`) let a
33+
* path like "@evil.example/x", ".evil.example/x", or "//evil.example/x"
34+
* rewrite the host and send the tenant ApiToken to an attacker-chosen origin.
35+
* We therefore (1) require a leading single "/" and (2) pin the resolved
36+
* origin to the configured console. Any deviation throws before doFetch runs.
37+
*/
38+
function safeUrl(path) {
39+
if (typeof path !== 'string' || !path.startsWith('/') || path.startsWith('//')) {
40+
throw new Error(
41+
`S1 API path must be a string starting with a single "/" (got: ${JSON.stringify(path)?.slice(0, 80)})`
42+
);
43+
}
44+
const origin = new URL(base()).origin;
45+
const u = new URL(path, origin);
46+
if (u.origin !== origin) {
47+
throw new Error(`S1 API path may not change the request origin (resolved to ${u.origin})`);
48+
}
49+
return u;
50+
}
51+
2752
async function doFetch(url, opts, retries = 3) {
2853
let delay = 500;
2954
for (let attempt = 0; attempt <= retries; attempt++) {
@@ -66,12 +91,11 @@ function sleep(ms) {
6691

6792
/** GET /web/api/v2.1/<path> */
6893
export async function apiGet(path, params = {}) {
69-
let url = `${base()}${path}`;
70-
const qs = new URLSearchParams(
71-
Object.fromEntries(Object.entries(params).filter(([, v]) => v !== undefined && v !== null))
72-
).toString();
73-
if (qs) url += '?' + qs;
74-
return doFetch(url, {
94+
const u = safeUrl(path);
95+
for (const [k, v] of Object.entries(params)) {
96+
if (v !== undefined && v !== null) u.searchParams.set(k, String(v));
97+
}
98+
return doFetch(u.toString(), {
7599
method: 'GET',
76100
headers: {
77101
Authorization: `ApiToken ${jwt()}`,
@@ -82,7 +106,7 @@ export async function apiGet(path, params = {}) {
82106

83107
/** POST /web/api/v2.1/<path> */
84108
export async function apiPost(path, body = {}) {
85-
return doFetch(`${base()}${path}`, {
109+
return doFetch(safeUrl(path).toString(), {
86110
method: 'POST',
87111
headers: {
88112
Authorization: `ApiToken ${jwt()}`,
@@ -94,7 +118,7 @@ export async function apiPost(path, body = {}) {
94118

95119
/** PUT /web/api/v2.1/<path> */
96120
export async function apiPut(path, body = {}) {
97-
return doFetch(`${base()}${path}`, {
121+
return doFetch(safeUrl(path).toString(), {
98122
method: 'PUT',
99123
headers: {
100124
Authorization: `ApiToken ${jwt()}`,
@@ -106,7 +130,7 @@ export async function apiPut(path, body = {}) {
106130

107131
/** DELETE /web/api/v2.1/<path> */
108132
export async function apiDelete(path, body = {}) {
109-
return doFetch(`${base()}${path}`, {
133+
return doFetch(safeUrl(path).toString(), {
110134
method: 'DELETE',
111135
headers: {
112136
Authorization: `ApiToken ${jwt()}`,
@@ -118,7 +142,7 @@ export async function apiDelete(path, body = {}) {
118142

119143
/** PATCH /web/api/v2.1/<path> */
120144
export async function apiPatch(path, body = {}) {
121-
return doFetch(`${base()}${path}`, {
145+
return doFetch(safeUrl(path).toString(), {
122146
method: 'PATCH',
123147
headers: {
124148
Authorization: `ApiToken ${jwt()}`,

mcp/s1-secops-mcp/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
"start": "node index.js",
2121
"start:http": "node index.js --transport http",
2222
"dev": "node --watch index.js",
23-
"test": "node --test tests/smoke.test.mjs tests/stdio-transport.test.mjs tests/http-transport.test.mjs",
23+
"test": "node --test tests/smoke.test.mjs tests/stdio-transport.test.mjs tests/http-transport.test.mjs tests/ssrf-path.test.mjs tests/http-origin-guard.test.mjs",
2424
"regen:readme": "node scripts/regen-readme-tools-table.mjs"
2525
},
2626
"engines": {
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
/**
2+
* HTTP transport CSRF / DNS-rebinding guard test (finding 019eff5a).
3+
*
4+
* The documented no-auth loopback mode (`s1-secops-mcp --transport http`, no
5+
* bearer tokens) used to dispatch any POST /mcp regardless of Origin or Host,
6+
* so a browser page — or a DNS-rebinding attack — on the operator's workstation
7+
* could invoke every state-changing tool. After the fix, in no-auth mode:
8+
* - a request carrying any Origin header is rejected 403
9+
* - a request whose Host header is not the loopback bind is rejected 403
10+
* - a normal non-browser request (no Origin, loopback Host) still works
11+
*
12+
* fetch() forbids setting Origin/Host, so we use the raw node:http client.
13+
*/
14+
15+
import { test } from 'node:test';
16+
import assert from 'node:assert/strict';
17+
import { spawn } from 'node:child_process';
18+
import http from 'node:http';
19+
import { mkdtempSync, writeFileSync, rmSync } from 'node:fs';
20+
import { tmpdir } from 'node:os';
21+
import { join, dirname, resolve } from 'node:path';
22+
import { fileURLToPath } from 'node:url';
23+
24+
const __dir = dirname(fileURLToPath(import.meta.url));
25+
const SERVER = resolve(__dir, '..', 'index.js');
26+
27+
async function waitForHealth(port, attempts = 50) {
28+
for (let i = 0; i < attempts; i++) {
29+
try {
30+
const r = await fetch(`http://127.0.0.1:${port}/healthz`);
31+
if (r.ok) return;
32+
} catch { /* retry */ }
33+
await new Promise(r => setTimeout(r, 100));
34+
}
35+
throw new Error(`Server did not become healthy on port ${port}`);
36+
}
37+
38+
function spawnServer(env = {}) {
39+
const port = 9000 + Math.floor(Math.random() * 1000);
40+
const child = spawn(process.execPath, [SERVER, '--transport', 'http', '--port', String(port), '--host', '127.0.0.1'], {
41+
stdio: ['ignore', 'ignore', 'pipe'],
42+
env: { ...process.env, ...env },
43+
});
44+
let stderrBuf = '';
45+
child.stderr.on('data', c => { stderrBuf += c.toString('utf-8'); });
46+
return { child, port, getStderr: () => stderrBuf };
47+
}
48+
49+
/** Raw HTTP POST so we can set the forbidden-in-fetch Origin / Host headers. */
50+
function rawPost(port, body, headers = {}) {
51+
return new Promise((resolve, reject) => {
52+
const data = Buffer.from(body, 'utf-8');
53+
const req = http.request({
54+
hostname: '127.0.0.1',
55+
port,
56+
path: '/mcp',
57+
method: 'POST',
58+
headers: { 'Content-Type': 'application/json', 'Content-Length': data.length, ...headers },
59+
}, (res) => {
60+
let buf = '';
61+
res.on('data', c => { buf += c; });
62+
res.on('end', () => resolve({ status: res.statusCode, body: buf }));
63+
});
64+
req.on('error', reject);
65+
req.write(data);
66+
req.end();
67+
});
68+
}
69+
70+
function rpc(id, method, params = {}) {
71+
return JSON.stringify({ jsonrpc: '2.0', id, method, params });
72+
}
73+
74+
async function withServer(env, fn) {
75+
const { child, port, getStderr } = spawnServer(env);
76+
try {
77+
await waitForHealth(port);
78+
await fn(port);
79+
} finally {
80+
child.kill('SIGTERM');
81+
await new Promise(r => setTimeout(r, 100));
82+
if (!child.killed) child.kill('SIGKILL');
83+
}
84+
}
85+
86+
test('guard(no-auth): request with an Origin header is rejected 403', async () => {
87+
await withServer({}, async (port) => {
88+
const r = await rawPost(port, rpc(1, 'tools/list'), { Origin: 'https://evil.example' });
89+
assert.equal(r.status, 403);
90+
const body = JSON.parse(r.body);
91+
assert.equal(body.error.code, -32001);
92+
});
93+
});
94+
95+
test('guard(no-auth): DNS-rebind Host header is rejected 403', async () => {
96+
await withServer({}, async (port) => {
97+
const r = await rawPost(port, rpc(1, 'tools/list'), { Host: `rebind.attacker.example:${port}` });
98+
assert.equal(r.status, 403);
99+
const body = JSON.parse(r.body);
100+
assert.equal(body.error.code, -32001);
101+
});
102+
});
103+
104+
test('guard(no-auth): simple text/plain cross-origin POST is rejected 403', async () => {
105+
// The pre-fix bypass: a "simple" cross-origin fetch with text/plain sends an
106+
// Origin header but no preflight. The Origin check must still catch it.
107+
await withServer({}, async (port) => {
108+
const r = await rawPost(port, rpc(1, 'tools/list'), {
109+
Origin: 'https://example.com',
110+
'Content-Type': 'text/plain',
111+
});
112+
assert.equal(r.status, 403);
113+
});
114+
});
115+
116+
test('guard(no-auth): normal non-browser request (no Origin, loopback Host) still works', async () => {
117+
await withServer({}, async (port) => {
118+
const r = await rawPost(port, rpc(1, 'tools/list'));
119+
assert.equal(r.status, 200);
120+
const body = JSON.parse(r.body);
121+
assert.equal(body.result.tools.length, 26);
122+
});
123+
});
124+
125+
test('guard(no-auth): localhost Host with matching port is allowed', async () => {
126+
await withServer({}, async (port) => {
127+
const r = await rawPost(port, rpc(1, 'tools/list'), { Host: `localhost:${port}` });
128+
assert.equal(r.status, 200);
129+
});
130+
});
131+
132+
test('guard(auth): authenticated path is unaffected by Origin (proxy compatibility)', async () => {
133+
// When bearer auth is configured (systemd / Caddy team path), the token
134+
// already blocks browsers, and a reverse proxy may legitimately forward a
135+
// public Host/Origin. The guard is scoped to no-auth mode, so a valid token
136+
// with an Origin header must still succeed.
137+
const goodToken = 'alice-token-' + 'x'.repeat(20);
138+
const dir = mkdtempSync(join(tmpdir(), 'mcp-auth-'));
139+
const file = join(dir, 'tokens.json');
140+
writeFileSync(file, JSON.stringify({ alice: goodToken }), { mode: 0o600 });
141+
try {
142+
await withServer({ MCP_BEARER_TOKENS_FILE: file }, async (port) => {
143+
const r = await rawPost(port, rpc(1, 'tools/list'), {
144+
Origin: 'https://proxy.s1.internal',
145+
Authorization: `Bearer ${goodToken}`,
146+
});
147+
assert.equal(r.status, 200);
148+
});
149+
} finally {
150+
rmSync(dir, { recursive: true, force: true });
151+
}
152+
});

0 commit comments

Comments
 (0)