-
Notifications
You must be signed in to change notification settings - Fork 612
feat: add session state management vercel-labs/agent-browser #184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Implement session name validation to ensure only safe characters are used. - Add support for session state persistence with auto-save/load functionality. - Introduce encryption for saved state files using AES-256-GCM. - Forward encryption key and debug flag from environment variables to the daemon. - Update command-line flags parsing to handle session names and validation errors. - Improve output handling to display warnings for issues like decryption failures. - Refactor state management utilities for better organization and reusability. - Enhance documentation to reflect new session management features and usage examples.
… errors - Fixed duplicate function signatures in connection.rs and main.rs - Created validation.rs module for session name validation - Fixed syntax errors in daemon.ts, flags.rs, and output.rs - Removed duplicate imports in actions.ts and browser.ts - All tests passing: 182 TypeScript + 114 Rust - Session persistence with encryption working correctly
fix(merge-conflicts):
…gement features, including session name functionality, state encryption, management commands, and performance optimizations.
Remove PR_DESCRIPTION.md
|
@amannhq is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
…ssion ID validation
feat: enhance ensure_daemon function to support extensions and add session ID validation
feat: add support for opening links in a new tab with --new-tab option
|
Appreciate the work on session persistence! The encryption implementation looks solid (AES-256-GCM, proper auth tags, random IVs), but there are critical issues: Security: Path Traversal Vulnerability src/state-utils.ts:54-62 Attack vector:
None validate before passing to getAutoStateFilePath(). An attacker can: Required fix: Missing Tests for Cryptographic Code encryption.ts (111 lines of crypto code) has zero unit tests. This is a blocker. Need tests for:
Please fix the path traversal vulnerability and add crypto tests before this can be merged. |
- Combined upstream features (extensions, profile, proxy, proxy_bypass, args, user_agent, provider) with session features (session_name, encryption, state management commands) - Added state list, show, clear, clean, rename commands - Added session name validation - Combined persistent profile and session persistence documentation - Fixed TypeScript type definitions to include all launch options
…or duplicated code
## Security Fixes
### 1. Path Traversal Prevention (ALREADY FIXED - Verified)
The original security concern about path traversal in `getAutoStateFilePath` was
already addressed. Verification confirms:
- `state-utils.ts:70-74`: `isValidSessionName()` validation added
- `daemon.ts:373-377, 406-408, 426-428`: All 3 daemon entry points now validate
session names from environment variables before use
- Attack vector `AGENT_BROWSER_SESSION_NAME="../../../etc/passwd"` is now blocked
### 2. Cryptographic Tests (ALREADY FIXED - Verified)
The `encryption.test.ts` file contains 37 comprehensive tests covering:
- Round-trip encryption/decryption
- IV uniqueness verification (same data encrypted twice produces different IVs)
- Tampered auth tag detection
- Tampered ciphertext detection
- Tampered IV detection
- Wrong key handling
- Malformed payload detection
- Key format validation
### 3. StreamServer Network Exposure (NEW FIX)
- Location: `stream-server.ts:90`
- Issue: WebSocket server was binding to `0.0.0.0` (all interfaces)
- Risk: Network exposure of browser viewport and input injection (mouse/keyboard)
- Fix: Now explicitly binds to `127.0.0.1` (localhost only)
### 4. Prototype Pollution Prevention (NEW FIX)
- Location: `browser.ts:544-548`
- Issue: Object spread `{...requestHeaders, ...headers}` vulnerable to prototype
pollution if headers contain `__proto__`, `constructor`, or `prototype` keys
- Fix: Added `safeHeaderMerge()` utility that filters dangerous keys and returns
a null-prototype object
## Code Quality: Duplicated Code Refactored
### Consolidated into `state-utils.ts`:
1. **`listStateFiles()`** - Replaces repeated `fs.readdirSync().filter('.json')`
pattern in daemon.ts and actions.ts
2. **`cleanupExpiredStates(days)`** - Consolidates identical cleanup logic from:
- `daemon.ts` (startup cleanup)
- `actions.ts` (state clean command)
3. **`safeHeaderMerge(base, override)`** - Security utility for header merging
### Removed Dead Code:
- `daemon.ts:loadStateFromFile()` - Was unused, identical to `readStateFile()`
## Test Coverage
- Added 11 new tests for security utilities (268 total, up from 257)
- Tests cover prototype pollution filtering, null-prototype objects, edge cases
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
fix: security review addressed
|
Hey Chris, thanks for the thorough security review! I've addressed all the concerns. Here's a summary: Issues You Raised1. Path Traversal Vulnerability - FIXEDIdentified that Fix applied:
Test coverage: 22 tests covering path traversal attempts, special characters, Unicode homograph attacks, etc. 2. Missing Crypto Tests - FIXEDThe
Additional Issues Found & FixedDuring the security audit, I discovered two more issues: 3. StreamServer Network Exposure - FIXEDThe WebSocket server was binding to - this.wss = new WebSocketServer({ port: this.port });
+ this.wss = new WebSocketServer({ port: this.port, host: '127.0.0.1' });4. Prototype Pollution Risk - FIXEDHeader merging used object spread which was vulnerable to - headers: { ...requestHeaders, ...headers }
+ headers: safeHeaderMerge(requestHeaders, headers)The new Code Quality ImprovementsAlso refactored duplicated code while I was in there:
Let me know if you'd like any changes! |
This PR implements session persistence with automatic state save/restore, encryption support, and comprehensive state management commands.
Resolves #42
Resolves #86
Resolves #115
Resolves #158
Features
1. Session Persistence (
--session-name)Automatically save and restore cookies/localStorage across browser restarts:
3. State Management Commands
state liststate show <file>state rename <old> <new>state clear <session>state clear --allstate clean --older-than <days>state save <path>state load <path>4. Auto-Expiration
Automatically clean up old state files:
Environment Variables
AGENT_BROWSER_SESSION_NAMEAGENT_BROWSER_ENCRYPTION_KEYAGENT_BROWSER_STATE_EXPIRE_DAYSCLI Click Command with
--new-tabFlagHow It Works
1. User Input
agent-browser click #button --new-tab