T09 · Insecure Skill Coding Practices
Error
- Location
task_planner.py:132- Finding
Plan Integrity and Approval State Can Be Bypassed
- Content
View full analysis
bool: """Verify plan hasn't been tampered with""" if not self.checksum: return True return self.checksum == self.calculate_checksum() ``` ```python # Check if plan is approved if plan.status != TaskStatus.APPROVED and not self.auto_approve: self.logger.error("Plan must be approved before execution") return False, [] # Auto-approve if enabled if self.auto_approve and plan.status == TaskStatus.PENDING: if not self.approve_plan(plan): return False, [] ``` ```python plan = TaskPlan( task_id=data['task_id'], title=data['title'], description=data['description'], steps=steps, created_at=data['created_at'], status=TaskStatus(data['status']), approved_by=data.get('approved_by'), approved_at=data.get('approved_at'), completed_at=data.get('completed_at'), metadata=data.get('metadata', {}), checksum=data.get('checksum') ) ``` ### Technical Analysis Integrity verification fails open when a plan has no checksum. A persisted plan can therefore omit the `checksum` field or set it to `null`, after which `verify_integrity()` reports success. More importantly, `execute_plan()` does not verify the plan checksum immediately before execution. It trusts the mutable `status` property and accepts any plan marked `APPROVED`. Approval is not cryptographically or logically bound to the checksum that was validated at approval time. Consequently, executable fields such as step actions and parameters can be changed after approval without invalidating the approval. Loaded JSON can also directly supply an approved status even though the plan was never approved by the current process. Execution-time per-step validation does not eli ...[truncated 1860 chars]- Remediation
View remediation
bool: if not self.checksum: return False return hashlib.compare_digest( self.checksum, self.calculate_checksum() ) ``` 2. Revalidate the complete plan immediately before changing its status to `RUNNING`: ```python is_valid, warnings = self.validate_plan(plan) if not is_valid: return False, [] ``` 3. Store the exact checksum approved by the approver, such as `approved_checksum`, and require it to equal the current calculated checksum before execution. 4. Do not trust persisted approval state by default. Loaded plans should be reset to `PENDING` unless approval is authenticated with a trusted digital signature or verified external authorization record. 5. Ensure the checksum covers every security-relevant field, including: - Step actions and parameters. - Safety and rollback flags. - Retry configuration. - Plan metadata that influences execution. - Step order and expected outputs where relevant. 6. Prevent silent post-approval mutation by using immutable plan objects or by automatically clearing approval whenever an executable field changes. 7. Add tests for missing checksums, forged approved status, post-approval mutation, and execution-time integrity failure. ]]>