T09 · Insecure Skill Coding Practices
- Location
scripts/state_manager.py:71- Finding
Workflow Identifier Path Traversal Enables Arbitrary JSON File Access
- Content
View full analysis
Vulnerability Details
File Location:
scripts/state_manager.py, lines 71, 91, and 201
Vulnerability Type: Unvalidated path construction and directory traversal
Risk Level: HighThe application constructs workflow file paths directly from the caller-controlled
workflow_idwithout validating its format or ensuring that the resolved path remains inside the configured state directory.python def load_workflow(self, workflow_id: str) -> Optional[Dict]: """ Load a workflow's state. Args: workflow_id: Workflow ID Returns: Workflow state dictionary, or None if not found """ workflow_file = self.state_dir / f"{workflow_id}.json" if not workflow_file.exists(): return None with open(workflow_file, "r") as f: return json.load(f)python def save_workflow(self, workflow_id: str, state: Dict) -> bool: """ Save a workflow's state. Args: workflow_id: Workflow ID state: State dictionary Returns: True if saved successfully, False otherwise """ workflow_file = self.state_dir / f"{workflow_id}.json" state["updated_at"] = datetime.now().isoformat() try: with open(workflow_file, "w") as f: json.dump(state, f, indent=2) return True except Exception as e: print(f"Error saving workflow: {e}") return Falsepython def delete_workflow(self, workflow_id: str) -> bool: """ Delete a workflow. Args: workflow_id: Workflow ID Returns: True if deleted successfully, False otherwise """ workflow_file = self.state_dir / f"{workflow_id}.json" if workflow_file.exists(): workflow_file.unlink() return True return FalseTechnical Analysis
workflow_idis accepted from com ...[truncated 2251 chars]- Remediation
View remediation
Remediation Suggestions
-
Enforce the identifier format used by
create_workflow()before any filesystem operation:python import re WORKFLOW_ID_PATTERN = re.compile(r"^[0-9a-f]{8}$") def validate_workflow_id(workflow_id: str) -> None: if not WORKFLOW_ID_PATTERN.fullmatch(workflow_id): raise ValueError("Invalid workflow ID") -
Centralize safe path generation and verify containment after canonicalization:
python def get_workflow_path(self, workflow_id: str) -> Path: validate_workflow_id(workflow_id) base = self.state_dir.resolve() candidate = (base / f"{workflow_id}.json").resolve() if candidate.parent != base: raise ValueError("Workflow path escapes the state directory") return candidate -
Replace every direct construction of
self.state_dir / f"{workflow_id}.json"with the centralized safe helper. -
Reject symlink-based file targets, especially before writes and deletions. Where supported, use operating-system APIs that avoid following symlinks.
-
Add tests for identifiers containing
.., absolute paths, path separators, encoded traversal sequences, unexpected Unicode, and symlink targets. -
Run the state manager under a minimally privileged account so that any remaining filesystem flaw has limited impact.
-