security: fix env race, signing key, upload traversal, password hash (#106)
* security: fix four audit findings -- env race, signing key, upload traversal, password hash 1. Race condition in os.environ (HIGH): Per-session _agent_lock didn't prevent cross-session env writes from interleaving. Added global _ENV_LOCK in streaming.py that serializes the entire env save/restore block across all sessions. 2. Predictable signing key (MEDIUM): sha256(STATE_DIR) was deterministic. Now generates a random 32-byte key on first startup and persists it to STATE_DIR/.signing_key (chmod 600). Existing sessions invalidated on first restart (acceptable for a security fix). 3. Upload path traversal (MEDIUM): Filename '..' survived the regex sanitization (dots are allowed chars). Added explicit rejection of dot-only names and safe_resolve_ws() check to verify the resolved path stays within the workspace. 4. Weak password hashing (MEDIUM): Replaced bare SHA-256 with PBKDF2- SHA256 (600k iterations per OWASP). Uses stdlib hashlib.pbkdf2_hmac, no new dependencies. Note: existing passwords must be re-set after this change (hash format changed). Closes #106 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use random signing key as PBKDF2 salt (replaces predictable STATE_DIR salt) --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
31
api/auth.py
31
api/auth.py
@@ -26,14 +26,35 @@ _sessions = {}
|
|||||||
|
|
||||||
|
|
||||||
def _signing_key():
|
def _signing_key():
|
||||||
"""Derive a stable signing key from STATE_DIR."""
|
"""Return a random signing key, generating and persisting one on first call."""
|
||||||
return hashlib.sha256(str(STATE_DIR).encode()).digest()
|
key_file = STATE_DIR / '.signing_key'
|
||||||
|
if key_file.exists():
|
||||||
|
try:
|
||||||
|
raw = key_file.read_bytes()
|
||||||
|
if len(raw) >= 32:
|
||||||
|
return raw[:32]
|
||||||
|
except Exception:
|
||||||
|
pass
|
||||||
|
# Generate a new random key
|
||||||
|
key = secrets.token_bytes(32)
|
||||||
|
try:
|
||||||
|
STATE_DIR.mkdir(parents=True, exist_ok=True)
|
||||||
|
key_file.write_bytes(key)
|
||||||
|
key_file.chmod(0o600)
|
||||||
|
except Exception:
|
||||||
|
pass # key works for this process even if persist fails
|
||||||
|
return key
|
||||||
|
|
||||||
|
|
||||||
def _hash_password(password):
|
def _hash_password(password):
|
||||||
"""SHA-256 hash with a salt derived from STATE_DIR."""
|
"""PBKDF2-SHA256 with 600k iterations (OWASP recommendation).
|
||||||
salt = str(STATE_DIR).encode()
|
Salt is the persisted random signing key, which is secret and unique per
|
||||||
return hashlib.sha256(salt + password.encode()).hexdigest()
|
installation. This keeps the stored hash format a plain hex string
|
||||||
|
(no format change to settings.json) while replacing the predictable
|
||||||
|
STATE_DIR-derived salt from the original implementation."""
|
||||||
|
salt = _signing_key()
|
||||||
|
dk = hashlib.pbkdf2_hmac('sha256', password.encode(), salt, 600_000)
|
||||||
|
return dk.hex()
|
||||||
|
|
||||||
|
|
||||||
def get_password_hash():
|
def get_password_hash():
|
||||||
|
|||||||
@@ -17,6 +17,12 @@ from api.config import (
|
|||||||
resolve_model_provider,
|
resolve_model_provider,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Global lock for os.environ writes. Per-session locks (_agent_lock) prevent
|
||||||
|
# concurrent runs of the SAME session, but two DIFFERENT sessions can still
|
||||||
|
# interleave their os.environ writes. This global lock serializes the env
|
||||||
|
# save/restore around the entire agent run.
|
||||||
|
_ENV_LOCK = threading.Lock()
|
||||||
|
|
||||||
# Lazy import to avoid circular deps -- hermes-agent is on sys.path via api/config.py
|
# Lazy import to avoid circular deps -- hermes-agent is on sys.path via api/config.py
|
||||||
try:
|
try:
|
||||||
from run_agent import AIAgent
|
from run_agent import AIAgent
|
||||||
@@ -101,7 +107,7 @@ def _run_agent_streaming(session_id, msg_text, model, workspace, stream_id, atta
|
|||||||
HERMES_HOME=_profile_home,
|
HERMES_HOME=_profile_home,
|
||||||
)
|
)
|
||||||
# Still set process-level env as fallback for tools that bypass thread-local
|
# Still set process-level env as fallback for tools that bypass thread-local
|
||||||
with _agent_lock:
|
with _ENV_LOCK:
|
||||||
old_cwd = os.environ.get('TERMINAL_CWD')
|
old_cwd = os.environ.get('TERMINAL_CWD')
|
||||||
old_exec_ask = os.environ.get('HERMES_EXEC_ASK')
|
old_exec_ask = os.environ.get('HERMES_EXEC_ASK')
|
||||||
old_session_key = os.environ.get('HERMES_SESSION_KEY')
|
old_session_key = os.environ.get('HERMES_SESSION_KEY')
|
||||||
|
|||||||
@@ -70,7 +70,11 @@ def handle_upload(handler):
|
|||||||
return j(handler, {'error': 'Session not found'}, status=404)
|
return j(handler, {'error': 'Session not found'}, status=404)
|
||||||
workspace = Path(s.workspace)
|
workspace = Path(s.workspace)
|
||||||
safe_name = _re.sub(r'[^\w.\-]', '_', Path(filename).name)[:200]
|
safe_name = _re.sub(r'[^\w.\-]', '_', Path(filename).name)[:200]
|
||||||
dest = workspace / safe_name
|
# Reject names that are purely dots (path traversal: ".." survives regex)
|
||||||
|
if not safe_name or safe_name.strip('.') == '':
|
||||||
|
return j(handler, {'error': 'Invalid filename'}, status=400)
|
||||||
|
# Verify the resolved path stays within the workspace
|
||||||
|
dest = safe_resolve_ws(workspace, safe_name)
|
||||||
dest.write_bytes(file_bytes)
|
dest.write_bytes(file_bytes)
|
||||||
return j(handler, {'filename': safe_name, 'path': str(dest), 'size': dest.stat().st_size})
|
return j(handler, {'filename': safe_name, 'path': str(dest), 'size': dest.stat().st_size})
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
|
|||||||
Reference in New Issue
Block a user