security: add workspace path validation and session regeneration on login#63
Open
DeryFerd wants to merge 2 commits into
Open
security: add workspace path validation and session regeneration on login#63DeryFerd wants to merge 2 commits into
DeryFerd wants to merge 2 commits into
Conversation
…ogin - Add workspace path validator to prevent directory traversal attacks - Blocks dangerous system paths (/etc, /root, C:\Windows, etc.) - Rejects path traversal sequences (..) - Validates paths on agent creation and update - Regenerate session on successful login to prevent session fixation - Call session.clear() before setting authenticated flag - Prevents attackers from hijacking pre-set sessions - Add comprehensive test coverage for both fixes - test_agent_workspace_validation.py: 5 tests, 16 subtests - test_session_fixation.py: 2 tests Fixes two medium-high security vulnerabilities identified during audit.
The validator now correctly rejects subdirectories under dangerous paths (e.g., /var/log) by checking for both forward and backward slashes in cross-platform path validation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Problem
While reviewing the codebase, I found two security vulnerabilities that could compromise system integrity:
1. Workspace Path Injection (CWE-22)
Agents could be created with arbitrary filesystem paths as workspaces. An attacker who gained access to the agent creation endpoint could set a workspace to
/etc,/root, orC:\Windows\System32, potentially allowing the agent to read or modify sensitive system files.2. Session Fixation (CWE-384)
The login flow didn't regenerate the session ID after authentication. This meant an attacker who could set a victim's session cookie before login could hijack the authenticated session afterward—a classic session fixation attack.
The Fix
Workspace Path Validation
backend/workspace_validator.pywith functions to detect dangerous paths and path traversal attemptsmodels/mixins/agents.pyto validate workspace paths in bothcreate_agent()andupdate_agent()/etc,/root,C:\Windows, etc.)..patterns)ValueErrorwith descriptive error messages when validation failsSession Regeneration
routes/auth.pyto callsession.clear()before setting the authenticated flagnextURL from form data (not from pre-login session)Testing
Created comprehensive test coverage:
unit_tests/test_agent_workspace_validation.py(5 tests, 16 subtests)/etc,/root,C:\Windows, etc.)../../../etc/passwd)my_workspace,workspace/subdir)unit_tests/test_session_fixation.py(2 tests)Validation
All tests pass:
Compatibility
These changes are backward compatible:
Security Impact
Before:
After: