security: tighten Docker isolation and subprocess env
- Pin @playwright/mcp to 0.0.68 instead of @latest to prevent supply chain risk - Restrict MCP subprocess env to allowlist (PATH, HOME, NODE_PATH, DISPLAY, XDG_*) instead of spreading process.env - Add path traversal guard to @include() directive in prompt templates - Bind all Docker ports to 127.0.0.1 to prevent network exposure - Remove ipc: host — shm_size: 2gb already covers Chromium shared memory needs - Add prompt injection disclaimer for untrusted repositories to README
This commit is contained in:
@@ -82,7 +82,7 @@ function buildMcpServers(
|
||||
const isDocker = process.env.SHANNON_DOCKER === 'true';
|
||||
|
||||
const mcpArgs: string[] = [
|
||||
'@playwright/mcp@latest',
|
||||
'@playwright/mcp@0.0.68',
|
||||
'--isolated',
|
||||
'--user-data-dir', userDataDir,
|
||||
];
|
||||
@@ -92,13 +92,29 @@ function buildMcpServers(
|
||||
mcpArgs.push('--browser', 'chromium');
|
||||
}
|
||||
|
||||
const envVars: Record<string, string> = Object.fromEntries(
|
||||
Object.entries({
|
||||
...process.env,
|
||||
PLAYWRIGHT_HEADLESS: 'true',
|
||||
...(isDocker && { PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD: '1' }),
|
||||
}).filter((entry): entry is [string, string] => entry[1] !== undefined)
|
||||
);
|
||||
// NOTE: Explicit allowlist — the Playwright MCP subprocess must not inherit
|
||||
// secrets (API keys, AWS tokens) from the parent process.
|
||||
const MCP_ENV_ALLOWLIST = [
|
||||
'PATH', 'HOME', 'NODE_PATH', 'DISPLAY',
|
||||
'PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH',
|
||||
] as const;
|
||||
|
||||
const envVars: Record<string, string> = {
|
||||
PLAYWRIGHT_HEADLESS: 'true',
|
||||
...(isDocker && { PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD: '1' }),
|
||||
};
|
||||
|
||||
for (const key of MCP_ENV_ALLOWLIST) {
|
||||
if (process.env[key]) {
|
||||
envVars[key] = process.env[key]!;
|
||||
}
|
||||
}
|
||||
|
||||
for (const [key, value] of Object.entries(process.env)) {
|
||||
if (key.startsWith('XDG_') && value !== undefined) {
|
||||
envVars[key] = value;
|
||||
}
|
||||
}
|
||||
|
||||
mcpServers[playwrightMcpName] = {
|
||||
type: 'stdio' as const,
|
||||
|
||||
@@ -102,10 +102,19 @@ async function buildLoginInstructions(authentication: Authentication, logger: Ac
|
||||
// Pure function: Process @include() directives
|
||||
async function processIncludes(content: string, baseDir: string): Promise<string> {
|
||||
const includeRegex = /@include\(([^)]+)\)/g;
|
||||
// Use a Promise.all to handle all includes concurrently
|
||||
const resolvedBase = path.resolve(baseDir);
|
||||
|
||||
const replacements: IncludeReplacement[] = await Promise.all(
|
||||
Array.from(content.matchAll(includeRegex)).map(async (match) => {
|
||||
const includePath = path.join(baseDir, match[1]!);
|
||||
const includePath = path.resolve(baseDir, match[1]!);
|
||||
if (!includePath.startsWith(resolvedBase + path.sep) && includePath !== resolvedBase) {
|
||||
throw new PentestError(
|
||||
`Path traversal detected in @include(): ${match[1]}`,
|
||||
'prompt',
|
||||
false,
|
||||
{ includePath, baseDir: resolvedBase }
|
||||
);
|
||||
}
|
||||
const sharedContent = await fs.readFile(includePath, 'utf8');
|
||||
return {
|
||||
placeholder: match[0],
|
||||
|
||||
Reference in New Issue
Block a user