Skip to content

Commit 879e9cb

Browse files
committed
feat: add comprehensive security layer for Jupyter notebook support
- Implement JupyterNotebookSecurity module with validation and sanitization - Add content validation for dangerous code patterns, imports, and commands - Implement read-only mode for untrusted notebooks - Add configurable security settings with trusted sources support - Provide cell-level sanitization and warning system - Add comprehensive security tests (590 test cases) - Update handlers and strategies to use security features - Add detailed security documentation This addresses the security concerns raised by @adamhill about arbitrary code execution risks in Jupyter notebooks.
1 parent b486282 commit 879e9cb

File tree

8 files changed

+1819
-22
lines changed

8 files changed

+1819
-22
lines changed

docs/jupyter-notebook-security.md

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
# Jupyter Notebook Security
2+
3+
This document describes the security features implemented for Jupyter notebook support in Roo Code.
4+
5+
## Overview
6+
7+
Jupyter notebooks can contain and execute arbitrary code, which poses significant security risks. To address these concerns, we've implemented a comprehensive security layer that validates, sanitizes, and controls notebook operations.
8+
9+
## Security Features
10+
11+
### 1. Content Validation
12+
13+
The security module validates notebook content for:
14+
15+
- **Dangerous Code Patterns**: Detects usage of `eval`, `exec`, `compile`, `__import__`, and other potentially dangerous functions
16+
- **System Commands**: Identifies shell commands (`!command` or `%system`)
17+
- **File System Access**: Detects file operations (`open`, `read`, `write`)
18+
- **Network Operations**: Identifies network requests and socket operations
19+
- **Dangerous Imports**: Blocks imports of modules like `subprocess`, `os`, `socket`, `pickle`, etc.
20+
- **Script Injection**: Detects JavaScript in markdown cells and HTML outputs
21+
22+
### 2. Sanitization
23+
24+
When security risks are detected, the system can:
25+
26+
- Remove or disable dangerous code cells
27+
- Clear cell outputs that may contain malicious content
28+
- Strip JavaScript and iframes from markdown cells
29+
- Remove suspicious metadata fields
30+
- Add warning comments to dangerous cells
31+
32+
### 3. Read-Only Mode
33+
34+
Notebooks with security risks are automatically opened in read-only mode, preventing:
35+
36+
- Cell modifications
37+
- Cell additions or deletions
38+
- Saving changes to disk
39+
40+
### 4. Security Configuration
41+
42+
The security system is configurable with options for:
43+
44+
```typescript
45+
interface SecurityConfig {
46+
allowCodeExecution?: boolean // Default: false
47+
readOnlyMode?: boolean // Default: true
48+
maxCellSize?: number // Default: 1MB
49+
maxCellCount?: number // Default: 1000
50+
allowDangerousImports?: boolean // Default: false
51+
blockedPatterns?: RegExp[] // Custom patterns to block
52+
allowedOutputTypes?: string[] // Allowed MIME types
53+
enableWarnings?: boolean // Default: true
54+
trustedSources?: string[] // Trusted file paths
55+
}
56+
```
57+
58+
### 5. Trusted Sources
59+
60+
You can mark specific notebooks or directories as trusted to bypass security restrictions:
61+
62+
```typescript
63+
const securityConfig = {
64+
trustedSources: ["/path/to/trusted/notebook.ipynb", "/trusted/directory/*"],
65+
}
66+
```
67+
68+
## Security Levels
69+
70+
The system categorizes risks into four severity levels:
71+
72+
1. **Low**: Informational warnings (e.g., file access)
73+
2. **Medium**: Potentially dangerous operations (e.g., network requests)
74+
3. **High**: Dangerous operations (e.g., dangerous imports)
75+
4. **Critical**: Extremely dangerous operations (e.g., eval/exec, system commands)
76+
77+
## Usage Examples
78+
79+
### Basic Usage
80+
81+
```typescript
82+
import { JupyterNotebookHandler } from "./jupyter-notebook-handler"
83+
84+
// Load notebook with default security settings
85+
const handler = await JupyterNotebookHandler.fromFile("notebook.ipynb")
86+
87+
// Check if notebook is in read-only mode
88+
if (handler.isInReadOnlyMode()) {
89+
console.log("Notebook opened in read-only mode due to security concerns")
90+
}
91+
92+
// Get security recommendations
93+
const recommendations = handler.getSecurityRecommendations()
94+
recommendations.forEach((rec) => console.log(rec))
95+
```
96+
97+
### Custom Security Configuration
98+
99+
```typescript
100+
const securityConfig = {
101+
readOnlyMode: false, // Allow edits
102+
allowDangerousImports: false, // Block dangerous imports
103+
maxCellSize: 500000, // 500KB max per cell
104+
enableWarnings: true, // Show security warnings
105+
trustedSources: [
106+
// Trust specific paths
107+
"/my/trusted/notebooks/",
108+
],
109+
}
110+
111+
const handler = await JupyterNotebookHandler.fromFile("notebook.ipynb", securityConfig)
112+
```
113+
114+
### Checking Operations
115+
116+
```typescript
117+
// Check if specific operations are allowed
118+
const canRead = handler.wouldAllowOperation("read") // Always true
119+
const canWrite = handler.wouldAllowOperation("write") // Depends on validation
120+
const canExecute = handler.wouldAllowOperation("execute") // Requires explicit permission
121+
```
122+
123+
### Getting Sanitized Content
124+
125+
```typescript
126+
// Get a sanitized version of the notebook
127+
const sanitized = handler.getSanitizedNotebook()
128+
129+
// Sanitized notebook will have:
130+
// - Dangerous code cells disabled with warnings
131+
// - Scripts removed from markdown cells
132+
// - Outputs cleared from risky cells
133+
// - Suspicious metadata removed
134+
```
135+
136+
## Security Best Practices
137+
138+
1. **Never execute untrusted notebooks**: Even with security measures, executing arbitrary code is dangerous
139+
2. **Review notebooks before execution**: Always inspect notebook content before running cells
140+
3. **Use isolated environments**: Run notebooks in containers or virtual machines when possible
141+
4. **Limit file system access**: Restrict notebook access to specific directories
142+
5. **Monitor network activity**: Be aware of notebooks that make network requests
143+
6. **Keep backups**: Always backup important data before running unknown notebooks
144+
145+
## Risk Mitigation
146+
147+
The security implementation addresses the concerns raised about Jupyter notebooks by:
148+
149+
1. **Preventing automatic code execution**: Code execution is disabled by default
150+
2. **Detecting malicious patterns**: Comprehensive pattern matching for dangerous code
151+
3. **Sanitizing content**: Automatic removal of dangerous elements
152+
4. **Providing transparency**: Clear warnings and recommendations about risks
153+
5. **Enforcing restrictions**: Read-only mode for untrusted content
154+
6. **Allowing configuration**: Flexible security settings for different use cases
155+
156+
## Limitations
157+
158+
While the security measures significantly reduce risks, they cannot guarantee complete safety:
159+
160+
- Sophisticated obfuscation techniques may bypass detection
161+
- Zero-day vulnerabilities in the Python interpreter or libraries
162+
- Side-channel attacks through resource consumption
163+
- Data exfiltration through allowed operations
164+
165+
Always treat untrusted notebooks with caution and use additional isolation measures when dealing with potentially malicious content.
166+
167+
## Configuration in Roo Code
168+
169+
When Jupyter notebooks are detected in a workspace, Roo Code automatically:
170+
171+
1. Enables the Jupyter notebook diff strategy with security features
172+
2. Validates notebooks on load
173+
3. Shows security warnings in the console

src/core/diff/strategies/jupyter-notebook-diff.ts

Lines changed: 55 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,26 +2,44 @@ import { DiffStrategy, DiffResult, ToolUse } from "../../../shared/tools"
22
import { ToolProgressStatus } from "@roo-code/types"
33
import { JupyterNotebookHandler } from "../../../integrations/misc/jupyter-notebook-handler"
44
import { MultiSearchReplaceDiffStrategy } from "./multi-search-replace"
5+
import { SecurityConfig } from "../../../integrations/misc/jupyter-notebook-security"
56

67
export class JupyterNotebookDiffStrategy implements DiffStrategy {
78
private fallbackStrategy: MultiSearchReplaceDiffStrategy
9+
private securityConfig: SecurityConfig
810

9-
constructor(fuzzyThreshold?: number, bufferLines?: number) {
11+
constructor(fuzzyThreshold?: number, bufferLines?: number, securityConfig?: SecurityConfig) {
1012
// Use MultiSearchReplaceDiffStrategy as fallback for non-cell operations
1113
this.fallbackStrategy = new MultiSearchReplaceDiffStrategy(fuzzyThreshold, bufferLines)
14+
15+
// Default security configuration for diff operations
16+
this.securityConfig = securityConfig || {
17+
readOnlyMode: false, // Allow edits through diff strategy
18+
enableWarnings: true,
19+
allowCodeExecution: false,
20+
maxCellSize: 1024 * 1024, // 1MB
21+
maxCellCount: 1000,
22+
}
1223
}
1324

1425
getName(): string {
1526
return "JupyterNotebookDiff"
1627
}
1728

1829
getToolDescription(args: { cwd: string; toolOptions?: { [key: string]: string } }): string {
19-
return `## apply_diff (Jupyter Notebook Support)
20-
Description: Request to apply PRECISE, TARGETED modifications to Jupyter notebook (.ipynb) files. This tool supports both cell-level operations and content-level changes within cells.
30+
return `## apply_diff (Jupyter Notebook Support with Security)
31+
Description: Request to apply PRECISE, TARGETED modifications to Jupyter notebook (.ipynb) files with built-in security validation. This tool supports both cell-level operations and content-level changes within cells.
32+
33+
⚠️ SECURITY NOTICE: All notebook operations are validated for security risks including:
34+
- Dangerous code patterns (eval, exec, subprocess, etc.)
35+
- System command execution
36+
- Network operations
37+
- File system access
38+
- Malicious imports
2139
2240
For Jupyter notebooks, you can:
23-
1. Edit specific cells by cell number
24-
2. Add new cells
41+
1. Edit specific cells by cell number (with security validation)
42+
2. Add new cells (with content sanitization)
2543
3. Delete cells
2644
4. Apply standard search/replace within cells
2745
@@ -118,11 +136,28 @@ Your cell operation or search/replace content here
118136
diffContent: string,
119137
_paramStartLine?: number,
120138
_paramEndLine?: number,
139+
filePath?: string,
121140
): Promise<DiffResult> {
122141
// Check if this is a Jupyter notebook by trying to parse it
123142
let handler: JupyterNotebookHandler
124143
try {
125-
handler = new JupyterNotebookHandler("", originalContent)
144+
handler = new JupyterNotebookHandler(filePath || "", originalContent, this.securityConfig)
145+
146+
// Check if notebook is in read-only mode due to security concerns
147+
if (handler.isInReadOnlyMode()) {
148+
const validation = handler.getSecurityValidation()
149+
return {
150+
success: false,
151+
error: `Notebook is in read-only mode due to security concerns:\n${validation?.errors.join("\n")}`,
152+
}
153+
}
154+
155+
// Log security recommendations
156+
const recommendations = handler.getSecurityRecommendations()
157+
if (recommendations.length > 0 && !recommendations.some((r) => r.includes("✅"))) {
158+
console.warn("Security recommendations for notebook:")
159+
recommendations.forEach((rec) => console.warn(` ${rec}`))
160+
}
126161
} catch (error) {
127162
// Not a valid notebook, fall back to standard diff
128163
return this.fallbackStrategy.applyDiff(originalContent, diffContent, _paramStartLine, _paramEndLine)
@@ -148,7 +183,13 @@ Your cell operation or search/replace content here
148183
if (cellIndex >= 0 && cellIndex < handler.getCellCount()) {
149184
success = handler.updateCell(cellIndex, replaceContent)
150185
if (!success) {
151-
error = `Failed to update cell ${cellIndex}`
186+
// Check if it was a security issue
187+
const validation = handler.getSecurityValidation()
188+
if (validation && validation.errors.length > 0) {
189+
error = `Security validation failed for cell ${cellIndex}: ${validation.errors.join(", ")}`
190+
} else {
191+
error = `Failed to update cell ${cellIndex}`
192+
}
152193
}
153194
} else {
154195
error = `Cell index ${cellIndex} is out of range (0-${handler.getCellCount() - 1})`
@@ -161,7 +202,13 @@ Your cell operation or search/replace content here
161202
} else {
162203
success = handler.insertCell(cellIndex, cellType, replaceContent)
163204
if (!success) {
164-
error = `Failed to insert cell at index ${cellIndex}`
205+
// Check if it was a security issue
206+
const validation = handler.getSecurityValidation()
207+
if (validation && validation.errors.length > 0) {
208+
error = `Security validation failed for new cell: ${validation.errors.join(", ")}`
209+
} else {
210+
error = `Failed to insert cell at index ${cellIndex}`
211+
}
165212
}
166213
}
167214
break

src/core/task/Task.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2706,8 +2706,17 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
27062706
const hasJupyterFiles = workspaceDir && (await this.checkForJupyterFiles(workspaceDir))
27072707

27082708
if (hasJupyterFiles) {
2709-
// Use Jupyter-specific diff strategy for notebooks
2710-
this.diffStrategy = new JupyterNotebookDiffStrategy(this.fuzzyMatchThreshold)
2709+
// Use Jupyter-specific diff strategy for notebooks with security configuration
2710+
const securityConfig = {
2711+
readOnlyMode: false, // Allow edits through diff strategy
2712+
enableWarnings: true,
2713+
allowCodeExecution: false,
2714+
maxCellSize: 1024 * 1024, // 1MB
2715+
maxCellCount: 1000,
2716+
// Add workspace as trusted source if it's a local workspace
2717+
trustedSources: workspaceDir ? [workspaceDir] : [],
2718+
}
2719+
this.diffStrategy = new JupyterNotebookDiffStrategy(this.fuzzyMatchThreshold, undefined, securityConfig)
27112720
} else {
27122721
// Default to old strategy, will be updated if experiment is enabled.
27132722
this.diffStrategy = new MultiSearchReplaceDiffStrategy(this.fuzzyMatchThreshold)

0 commit comments

Comments
 (0)