Skip to content

Commit 5a63a87

Browse files
author
jovanSAPFIONEER
committed
security: fix path traversal vulnerability in blackboard.py change_id handling
- Added _sanitize_change_id() with regex validation (alphanumeric, hyphens, underscores, dots only) - Added _safe_pending_path() with resolved path boundary check - Applied to propose_change, validate_change, commit_change, abort_change - Sanitized archive file paths in commit and abort operations - Blocks both Unix (../../) and Windows (..\\) path traversal attacks - Found by VirusTotal during ClawHub security scan
1 parent 94830fa commit 5a63a87

File tree

1 file changed

+40
-7
lines changed

1 file changed

+40
-7
lines changed

scripts/blackboard.py

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,38 @@ def delete(self, key: str) -> bool:
307307
# ========================================================================
308308
# ATOMIC COMMIT WORKFLOW: propose → validate → commit
309309
# ========================================================================
310-
310+
311+
@staticmethod
312+
def _sanitize_change_id(change_id: str) -> str:
313+
"""
314+
Sanitize change_id to prevent path traversal attacks.
315+
Only allows alphanumeric characters, hyphens, underscores, and dots.
316+
Rejects any path separators or parent directory references.
317+
"""
318+
if not change_id or not isinstance(change_id, str):
319+
raise ValueError("change_id must be a non-empty string")
320+
# Strip whitespace
321+
sanitized = change_id.strip()
322+
# Reject path separators and parent directory traversal
323+
if any(c in sanitized for c in ('/', '\\', '..')):
324+
raise ValueError(
325+
f"Invalid change_id '{change_id}': must not contain path separators or '..'"
326+
)
327+
# Only allow safe characters: alphanumeric, hyphen, underscore, dot
328+
if not re.match(r'^[a-zA-Z0-9_\-\.]+$', sanitized):
329+
raise ValueError(
330+
f"Invalid change_id '{change_id}': only alphanumeric, hyphen, underscore, and dot allowed"
331+
)
332+
return sanitized
333+
334+
def _safe_pending_path(self, change_id: str, suffix: str = ".pending.json") -> Path:
335+
"""Build a pending-file path and verify it stays inside pending_dir."""
336+
safe_id = self._sanitize_change_id(change_id)
337+
target = (self.pending_dir / f"{safe_id}{suffix}").resolve()
338+
if not str(target).startswith(str(self.pending_dir.resolve())):
339+
raise ValueError(f"Path traversal blocked for change_id '{change_id}'")
340+
return target
341+
311342
def propose_change(self, change_id: str, key: str, value: Any,
312343
source_agent: str = "unknown", ttl: Optional[int] = None,
313344
operation: str = "write") -> dict[str, Any]:
@@ -317,7 +348,7 @@ def propose_change(self, change_id: str, key: str, value: Any,
317348
The change is written to a .pending file and must be validated
318349
and committed by the orchestrator before it takes effect.
319350
"""
320-
pending_file = self.pending_dir / f"{change_id}.pending.json"
351+
pending_file = self._safe_pending_path(change_id)
321352

322353
# Check for duplicate change_id
323354
if pending_file.exists():
@@ -365,7 +396,7 @@ def validate_change(self, change_id: str) -> dict[str, Any]:
365396
- No conflicting changes to the same key
366397
- Base hash matches (data hasn't changed since proposal)
367398
"""
368-
pending_file = self.pending_dir / f"{change_id}.pending.json"
399+
pending_file = self._safe_pending_path(change_id)
369400

370401
if not pending_file.exists():
371402
return {
@@ -439,7 +470,7 @@ def commit_change(self, change_id: str) -> dict[str, Any]:
439470
"""
440471
Apply a validated change atomically (Step 3 of atomic commit).
441472
"""
442-
pending_file = self.pending_dir / f"{change_id}.pending.json"
473+
pending_file = self._safe_pending_path(change_id)
443474

444475
if not pending_file.exists():
445476
return {
@@ -479,9 +510,10 @@ def commit_change(self, change_id: str) -> dict[str, Any]:
479510
change_set["status"] = "committed"
480511
change_set["committed_at"] = datetime.now(timezone.utc).isoformat()
481512

513+
safe_id = self._sanitize_change_id(change_id)
482514
archive_dir = self.pending_dir / "archive"
483515
archive_dir.mkdir(exist_ok=True)
484-
archive_file = archive_dir / f"{change_id}.committed.json"
516+
archive_file = archive_dir / f"{safe_id}.committed.json"
485517
archive_file.write_text(json.dumps(change_set, indent=2))
486518

487519
# Remove pending file
@@ -497,7 +529,7 @@ def commit_change(self, change_id: str) -> dict[str, Any]:
497529

498530
def abort_change(self, change_id: str) -> dict[str, Any]:
499531
"""Abort a pending change without applying it."""
500-
pending_file = self.pending_dir / f"{change_id}.pending.json"
532+
pending_file = self._safe_pending_path(change_id)
501533

502534
if not pending_file.exists():
503535
return {
@@ -510,9 +542,10 @@ def abort_change(self, change_id: str) -> dict[str, Any]:
510542
change_set["aborted_at"] = datetime.now(timezone.utc).isoformat()
511543

512544
# Archive the aborted change
545+
safe_id = self._sanitize_change_id(change_id)
513546
archive_dir = self.pending_dir / "archive"
514547
archive_dir.mkdir(exist_ok=True)
515-
archive_file = archive_dir / f"{change_id}.aborted.json"
548+
archive_file = archive_dir / f"{safe_id}.aborted.json"
516549
archive_file.write_text(json.dumps(change_set, indent=2))
517550

518551
pending_file.unlink()

0 commit comments

Comments
 (0)