Rh virt security review

[r2dedios]

Summary

Pack(s) affected

• rh-sre
• rh-developer
• ocp-admin
• rh-virt
• rh-ai-engineer
• Other / repo-wide

Change type

• New skill
• New agent
• New pack
• Update existing skill / agent
• MCP server config (mcps.json)
• Docs / README
• CI / tooling

CLAUDE.md compliance

• Agents orchestrate skills; no direct MCP/tool calls in agents
• Skills are single-purpose task executors
• Skills encapsulate all tool access (MCP tools invoked only inside skills)
• Document consultation: file is read with the Read tool, then declared to the user
• No credentials hardcoded; env vars used via ${VAR} references
• Human-in-the-loop confirmation added for any destructive or critical operations

Validation

• make validate passes locally
• New/changed skills have valid YAML frontmatter (name, description)
• New/changed agents have valid YAML frontmatter (name, description)

[r2dedios]

WIP, DO NOT merge

[r2dedios]

/skill-security-scan
/skill-code-review

[github-actions]

Gemini Code Review

📝 Review details

1. Summary

This PR introduces several excellent security and reliability hardening measures across the rh-virt pack and CI/CD pipelines. Key changes include:

• Linter Enforcement: Makes the allowed-tools frontmatter field mandatory and strictly space-delimited.
• CI/CD Improvements: Updates the Gemini Code Review and Security Scan workflows to append new runs to existing PR comments (tracking run numbers) rather than overwriting them, and adds PR reactions.
• Security Hardening: Removes dangerous CLI fallbacks (oc, kubectl) in favor of strict MCP tool usage, enforces strict YAML templating for snapshot creation, and adds input validation for Kubernetes resource names.
• Human-in-the-Loop (HitL): Upgrades batch operations and destructive actions (like vm-snapshot-delete) to require individual, typed confirmations.

2. Project Rules Compliance

Overall, the PR does a fantastic job aligning with the project's security and HitL principles. However, there is one violation of the core design principles:

• ❌ Violation of Principle Add Red Hat Developer collection with Containerize & Deploy workflow #1 (Document Consultation Transparency)
  In rh-virt/skills/vm-create/SKILL.md (lines 172-175), the explicit Document Consultation block was removed and replaced with a simple reference:

  -**CRITICAL: Document Consultation FIRST:**
  -1. Read [scheduling-errors.md](../../docs/troubleshooting/scheduling-errors.md) using Read tool
  -2. Output: "I detected ErrorUnschedulable. I consulted [scheduling-errors.md] to understand diagnosis strategies."
  +**Reference**: See [scheduling-errors.md](rh-virt/docs/troubleshooting/scheduling-errors.md) for diagnosis strategies.

  Rule: When consulting documentation, MUST actually read the file using Read tool, then declare consultation. By changing this to a simple reference, the LLM will likely hallucinate the troubleshooting steps or just provide a link without reading it. Please restore the explicit 1. **Action**: Read... 2. **Output to user**: ... pattern here.

• ⚠️ Build Reminder
  Since you have modified multiple skills, please remember to run make validate locally to ensure the catalog and structure remain compliant before merging.

3. Code Quality Issues

No functional bugs or broken links were found. The bash scripting for the linter and the GitHub Actions gh api comment patching are both robust and handle edge cases (like empty bodies or missing comments) gracefully.

4. Suggestions

• Praise for vm-inventory: Removing the document consultation from vm-inventory/SKILL.md and explicitly telling the LLM not to read external docs for status interpretation is a great application of Principle Rh developer/troubleshooting #8 (Single Responsibility). Delegating that to a dedicated troubleshooting skill keeps the inventory skill fast and focused.
• Praise for vm-snapshot-create: The addition of the fixed YAML template, strict regex input validation, and post-construction verification in vm-snapshot-create/SKILL.md is an outstanding example of defense-in-depth for LLM agents. This pattern should be considered the gold standard for any skill using resources_create_or_update.
• Linter Script Readability: In .claude/skills/skill-linter/scripts/validate-skill.sh, the nested if/else block for checking the YAML array format is perfectly functional, but could be slightly flattened in the future for readability. No change required for this PR.

5. Verdict

REQUEST_CHANGES

Please fix the Principle #1 (Document Consultation Transparency) violation in rh-virt/skills/vm-create/SKILL.md. Once that explicit Read tool instruction is restored, this PR is good to go!

---

Workflow run

[github-actions]

❌ Skill Security Scan

📋 rh-virt

Agent Skills Security Scan Report

Timestamp: 2026-05-07T09:32:38.739602+00:00

Summary

• Total Skills Scanned: 10
• Safe Skills: 8
• Total Findings: 17

Findings by Severity

• Critical: 0
• High: 3
• Medium: 5
• Low: 9
• Info: 0

Skill Results

---

[OK] vm-snapshot-delete

• Max Severity: SAFE
• Findings: 0
• Directory: /home/runner/work/agentic-collections/agentic-collections/rh-virt/skills/vm-snapshot-delete

---

[FAIL] vm-rebalance

• Max Severity: HIGH

• Findings: 2

• Directory: /home/runner/work/agentic-collections/agentic-collections/rh-virt/skills/vm-rebalance

  [HIGH] Indirect Prompt Injection via Unverified External Workflow Files

  Severity: HIGH
  Category: prompt_injection
  Rule ID: LLM_PROMPT_INJECTION
  Location: SKILL.md

  Description: The skill's core logic for both manual and automatic rebalancing is delegated to external markdown files (REBALANCE_MANUAL.md and REBALANCE_AUTOMATIC.md). The instructions explicitly state to read these files and then execute the workflow contained within them. This creates a transitive trust vulnerability, as any malicious instructions injected into these files would be executed by the agent. The files were not provided for analysis, making their content unknown and untrusted.

  Code Snippet:

  For Manual Mode:
  
  **Document Consultation** (REQUIRED - Execute FIRST):
  1. **Action**: Read [REBALANCE_MANUAL.md](rh-virt/skills/vm-rebalance/REBALANCE_MANUAL.md) using Read tool
  2. **Output to user**: "I consulted [REBALANCE_MANUAL.md]..."
  3. **Then execute**: Follow workflow in REBALANCE_MANUAL.md
  

  Remediation: The execution logic should be embedded directly within the SKILL.md instructions. Avoid delegating the primary execution flow to external files that can be modified independently of the skill's main logic. If external files must be used, they should be treated as data, not as executable instructions.

  [MEDIUM] Unauthorized Tool Use Violates 'allowed-tools' Policy

  Severity: MEDIUM
  Category: unauthorized_tool_use
  Rule ID: LLM_UNAUTHORIZED_TOOL_USE
  Location: SKILL.md

  Description: The skill's instructions require the use of the 'Read' tool to consult external markdown files. However, the 'Read' tool is not declared in the allowed-tools list in the YAML manifest. This violates the skill's own security policy, indicating a discrepancy between its declared capabilities and its actual behavior.

  Code Snippet:

  YAML Manifest:
  allowed-tools: mcp__openshift-virtualization__resources_list mcp__openshift-virtualization__resources_get ...
  
  Instruction Body:
  1. **Action**: Read [REBALANCE_MANUAL.md](rh-virt/skills/vm-rebalance/REBALANCE_MANUAL.md) using Read tool
  

  Remediation: Update the allowed-tools list in the SKILL.md manifest to include the 'Read' tool to accurately reflect the skill's required capabilities. Alternatively, if the use of the 'Read' tool is unintentional, remove the instruction to read external files.

---

[OK] vm-clone

• Max Severity: MEDIUM

• Findings: 1

• Directory: /home/runner/work/agentic-collections/agentic-collections/rh-virt/skills/vm-clone

  [MEDIUM] Potential for Resource Exhaustion via Batch Cloning

  Severity: MEDIUM
  Category: resource_abuse
  Rule ID: LLM_RESOURCE_ABUSE
  Location: SKILL.md

  Description: The skill allows for batch cloning of up to 5 virtual machines at a time. A malicious or careless user could leverage this feature to clone large, resource-intensive VMs, consuming significant CPU, memory, and storage. This could lead to a denial-of-service (DoS) condition, impacting the availability of the underlying cluster for other users and applications. While the skill includes a hardcoded limit as a mitigation, this may not be sufficient to prevent resource abuse.

  Code Snippet:

  Limit: Maximum 5 clones per batch request. If user requests more, refuse and explain the limit exists to prevent resource exhaustion.
  

  Remediation: Implement more robust and configurable quota management. The skill should check against cluster or namespace ResourceQuotas before initiating a clone and refuse if it would exceed a predefined threshold. For batch operations, the total resource impact should be calculated and confirmed against available quotas.

---

[OK] vm-delete

• Max Severity: SAFE
• Findings: 0
• Directory: /home/runner/work/agentic-collections/agentic-collections/rh-virt/skills/vm-delete

---

[OK] vm-snapshot-restore

• Max Severity: LOW

• Findings: 1

• Directory: /home/runner/work/agentic-collections/agentic-collections/rh-virt/skills/vm-snapshot-restore

  [LOW] Missing Optional 'compatibility' Field in Manifest

  Severity: LOW
  Category: skill_discovery_abuse
  Rule ID: LLM_SKILL_DISCOVERY_ABUSE
  Location: SKILL.md

  Description: The SKILL.md manifest (YAML frontmatter) does not specify the 'compatibility' field. While this field is optional, its absence reduces clarity on the intended environments and platforms where the skill is designed to function correctly.

  Code Snippet:

  compatibility: Not specified
  

  Remediation: Add the 'compatibility' field to the SKILL.md YAML frontmatter to declare the environments where the skill is expected to function correctly (e.g., 'Claude.ai, API').

---

[OK] vm-lifecycle-manager

• Max Severity: SAFE
• Findings: 0
• Directory: /home/runner/work/agentic-collections/agentic-collections/rh-virt/skills/vm-lifecycle-manager

---

[OK] vm-snapshot-list

• Max Severity: SAFE
• Findings: 0
• Directory: /home/runner/work/agentic-collections/agentic-collections/rh-virt/skills/vm-snapshot-list

---

[OK] vm-create

• Max Severity: LOW

• Findings: 1

• Directory: /home/runner/work/agentic-collections/agentic-collections/rh-virt/skills/vm-create

  [LOW] Missing Optional 'compatibility' Metadata

  Severity: LOW
  Category: skill_discovery_abuse
  Rule ID: LLM_SKILL_DISCOVERY_ABUSE
  Location: SKILL.md

  Description: The SKILL.md manifest does not specify the 'compatibility' field. While this is an optional field and does not pose a direct security risk, providing this information improves the skill's usability and helps users understand its intended environments and limitations.

  Code Snippet:

  compatibility: Not specified
  

  Remediation: Add the 'compatibility' key to the SKILL.md frontmatter and specify the environments where the skill is known to work (e.g., Claude.ai, API).

---

[FAIL] vm-snapshot-create

• Max Severity: HIGH

• Findings: 2

• Directory: /home/runner/work/agentic-collections/agentic-collections/rh-virt/skills/vm-snapshot-create

  [HIGH] Potential for Privilege Escalation via Generic Resource Creation Tool

  Severity: HIGH
  Category: command_injection
  Rule ID: LLM_COMMAND_INJECTION
  Location: SKILL.md

  Description: The skill uses the resources_create_or_update tool, which has broad permissions to create or modify any Kubernetes resource. The skill relies on prompt-based instructions and validation steps within the prompt to restrict the tool's usage to only creating VirtualMachineSnapshot resources. These prompt-level controls are not a substitute for technical enforcement (like RBAC) and could potentially be bypassed by a malicious user input, leading to the creation of arbitrary, potentially malicious Kubernetes resources (e.g., privileged pods, new role bindings).

  Code Snippet:

  Implementation Note: This skill uses generic Kubernetes resource tools (`resources_create_or_update`) to manage VirtualMachineSnapshot resources. ... `resources_create_or_update` MUST only be called with `apiVersion: snapshot.kubevirt.io/v1beta1` and `kind: VirtualMachineSnapshot` — using it to create or modify any other resource type is a security violation.
  

  Remediation: The ideal remediation is to replace the generic resources_create_or_update tool with a dedicated, purpose-built tool (e.g., create_vm_snapshot) that can only create the intended resource type. If this is not possible, the backend service account used by the MCP server tool should have its RBAC permissions strictly limited to only allow create and get operations on snapshot.kubevirt.io/v1beta1/VirtualMachineSnapshot resources, preventing it from creating other resource types.

  [LOW] Missing Optional 'compatibility' Field in Manifest

  Severity: LOW
  Category: skill_discovery_abuse
  Rule ID: LLM_SKILL_DISCOVERY_ABUSE
  Location: SKILL.md

  Description: The SKILL.md manifest frontmatter does not specify the 'compatibility' of the skill. While this is an optional field, providing it helps users understand the intended environments for the skill.

  Code Snippet:

  YAML Manifest Details:
  - ...
  - compatibility: Not specified
  - ...
  

  Remediation: Add the 'compatibility' field to the SKILL.md YAML frontmatter to declare the environments where the skill is expected to work (e.g., Claude.ai, API).

---

[OK] vm-inventory

• Max Severity: SAFE
• Findings: 0
• Directory: /home/runner/work/agentic-collections/agentic-collections/rh-virt/skills/vm-inventory

---

Cross-Skill Findings

• Findings: 10

  [HIGH] Potential data relay attack pattern detected

  Severity: HIGH
  Category: data_exfiltration
  Rule ID: CROSS_SKILL_DATA_RELAY
  Location: (cross-skill analysis)

  Description: Skills appear to form a data relay chain. Collectors (vm-snapshot-delete, vm-rebalance, vm-clone, vm-delete, vm-snapshot-restore, vm-lifecycle-manager, vm-snapshot-list, vm-create, vm-snapshot-create, vm-inventory) access sensitive data while exfiltrators (vm-snapshot-create) send data to external destinations. This pattern may indicate a coordinated attack.

  Remediation: Review these skills together to ensure they are not collaborating to exfiltrate sensitive data. Consider disabling one or both skills.

  [MEDIUM] Multiple skills reference the same external domain

  Severity: MEDIUM
  Category: data_exfiltration
  Rule ID: CROSS_SKILL_SHARED_URL
  Location: (cross-skill analysis)

  Description: Domain 'docs.redhat.com' is referenced by 10 skills: vm-snapshot-delete, vm-rebalance, vm-clone, vm-delete, vm-snapshot-restore, vm-lifecycle-manager, vm-snapshot-list, vm-create, vm-snapshot-create, vm-inventory. Multiple skills pointing to the same external resource may indicate coordinated C2 or exfiltration.

  Remediation: Review why multiple skills reference this domain and ensure it is a legitimate, trusted resource.

  [MEDIUM] Multiple skills reference the same external domain

  Severity: MEDIUM
  Category: data_exfiltration
  Rule ID: CROSS_SKILL_SHARED_URL
  Location: (cross-skill analysis)

  Description: Domain 'kubevirt.io' is referenced by 10 skills: vm-snapshot-delete, vm-rebalance, vm-clone, vm-delete, vm-snapshot-restore, vm-lifecycle-manager, vm-snapshot-list, vm-create, vm-snapshot-create, vm-inventory. Multiple skills pointing to the same external resource may indicate coordinated C2 or exfiltration.

  Remediation: Review why multiple skills reference this domain and ensure it is a legitimate, trusted resource.

  [MEDIUM] Multiple skills reference the same external domain

  Severity: MEDIUM
  Category: data_exfiltration
  Rule ID: CROSS_SKILL_SHARED_URL
  Location: (cross-skill analysis)

  Description: Domain 'kubernetes.io' is referenced by 3 skills: vm-clone, vm-delete, vm-snapshot-create. Multiple skills pointing to the same external resource may indicate coordinated C2 or exfiltration.

  Remediation: Review why multiple skills reference this domain and ensure it is a legitimate, trusted resource.

  [LOW] Skills have complementary descriptions

  Severity: LOW
  Category: social_engineering
  Rule ID: CROSS_SKILL_COMPLEMENTARY_TRIGGERS
  Location: (cross-skill analysis)

  Description: Skill 'vm-rebalance' (collector) and 'vm-snapshot-restore' (sender) have complementary descriptions with shared context: when, vm, create, not, use, creating. This may be intentional design or could indicate coordinated behavior.

  Remediation: Review these skills to ensure they are not designed to work together maliciously

  [LOW] Skills have complementary descriptions

  Severity: LOW
  Category: social_engineering
  Rule ID: CROSS_SKILL_COMPLEMENTARY_TRIGGERS
  Location: (cross-skill analysis)

  Description: Skill 'vm-rebalance' (collector) and 'vm-snapshot-create' (sender) have complementary descriptions with shared context: when, vm, create, not, use. This may be intentional design or could indicate coordinated behavior.

  Remediation: Review these skills to ensure they are not designed to work together maliciously

  [LOW] Skills have complementary descriptions

  Severity: LOW
  Category: social_engineering
  Rule ID: CROSS_SKILL_COMPLEMENTARY_TRIGGERS
  Location: (cross-skill analysis)

  Description: Skill 'vm-snapshot-list' (collector) and 'vm-snapshot-restore' (sender) have complementary descriptions with shared context: when, name, vm, snapshots, create, not, instead, virtual, confirmation, snapshot, use, creating. This may be intentional design or could indicate coordinated behavior.

  Remediation: Review these skills to ensure they are not designed to work together maliciously

  [LOW] Skills have complementary descriptions

  Severity: LOW
  Category: social_engineering
  Rule ID: CROSS_SKILL_COMPLEMENTARY_TRIGGERS
  Location: (cross-skill analysis)

  Description: Skill 'vm-snapshot-list' (collector) and 'vm-snapshot-create' (sender) have complementary descriptions with shared context: when, name, vm, snapshots, create, machine, not, status, recovery, virtual, snapshot, use. This may be intentional design or could indicate coordinated behavior.

  Remediation: Review these skills to ensure they are not designed to work together maliciously

  [LOW] Skills have complementary descriptions

  Severity: LOW
  Category: social_engineering
  Rule ID: CROSS_SKILL_COMPLEMENTARY_TRIGGERS
  Location: (cross-skill analysis)

  Description: Skill 'vm-inventory' (collector) and 'vm-snapshot-restore' (sender) have complementary descriptions with shared context: when, name, vm, create, not, instead, virtual, use, creating, machines. This may be intentional design or could indicate coordinated behavior.

  Remediation: Review these skills to ensure they are not designed to work together maliciously

  [LOW] Skills have complementary descriptions

  Severity: LOW
  Category: social_engineering
  Rule ID: CROSS_SKILL_COMPLEMENTARY_TRIGGERS
  Location: (cross-skill analysis)

  Description: Skill 'vm-inventory' (collector) and 'vm-snapshot-create' (sender) have complementary descriptions with shared context: when, name, vm, create, of, not, status, virtual, use. This may be intentional design or could indicate coordinated behavior.

  Remediation: Review these skills to ensure they are not designed to work together maliciously

Workflow run

[r2dedios]

/skill-security-scan
/skill-code-review

[r2dedios]

/skill-security-scan
/skill-code-review

[r2dedios]

/skill-security-scan
/skill-code-review

[r2dedios]

● ## rh-virt Security Review Summary

32 issues resolved across all 10 skills in the rh-virt pack.

By Severity

Severity	Count	Key Issues
HIGH	7	YAML injection, command injection via CLI fallback, indirect prompt injection via Read tool, unauthorized tool use
MEDIUM	17	Missing allowed-tools declarations (10), insufficient destructive-op confirmations (3), overly broad tool access, incomplete tool lists, missing Document Consultation pattern
LOW	8	Misleading documentation, unbounded polling, over-broad activation phrases, false security assurances, logic errors in thresholds

Resolved Issues

HIGH (7)

• RH-VIRT-001 — Fix YAML injection in vm-snapshot-create
• RH-VIRT-005 — Remove CLI fallback from vm-inventory to prevent command injection
• RH-VIRT-006 — Remove external doc reads to prevent indirect prompt injection
• RH-VIRT-012 — Replace ambiguous relative paths and remove Read tool injection in vm-rebalance
• RH-VIRT-020 — Remove shell commands from vm-create, use MCP tools exclusively
• RH-VIRT-043 — Add missing vm_lifecycle to vm-rebalance allowed-tools
• RH-VIRT-048 — Remove Read tool injection and fix relative paths in vm-create

MEDIUM (17)

• RH-VIRT-003 — Restrict resources_create_or_update to VirtualMachineSnapshot only
• RH-VIRT-013 — Add allowed-tools to vm-rebalance frontmatter
• RH-VIRT-015 — Require per-VM typed confirmation for batch deletion
• RH-VIRT-017 — Add allowed-tools to vm-delete frontmatter
• RH-VIRT-023 — Add allowed-tools to vm-create frontmatter
• RH-VIRT-026 — Enforce per-VM confirmation in vm-lifecycle-manager
• RH-VIRT-027 — Add allowed-tools to vm-lifecycle-manager frontmatter
• RH-VIRT-030 — Add allowed-tools to vm-snapshot-list frontmatter
• RH-VIRT-032 — Require typed snapshot name confirmation for deletion
• RH-VIRT-034 — Add allowed-tools to vm-snapshot-delete frontmatter
• RH-VIRT-038 — Limit batch cloning to 5 with per-VM confirmation
• RH-VIRT-040 — Add allowed-tools to vm-snapshot-restore frontmatter
• RH-VIRT-045 — Add missing vm_lifecycle to vm-snapshot-restore allowed-tools
• RH-VIRT-046 — Add missing vm_lifecycle to vm-lifecycle-manager allowed-tools
• RH-VIRT-047 — Add missing vm_create and vm_lifecycle to vm-create allowed-tools
• RH-VIRT-050 — Restore Document Consultation pattern in vm-rebalance
• RH-VIRT-052 — Add missing vm_lifecycle to vm-delete allowed-tools

LOW (8)

• RH-VIRT-007 — Clarify human-in-the-loop as not required for read-only vm-inventory
• RH-VIRT-008 — Skip VMI detail queries when listing more than 20 VMs
• RH-VIRT-021 — Add custom image provenance warning in vm-create
• RH-VIRT-022 — Bound provisioning polling to max 3 retries in vm-create
• RH-VIRT-035 — Remove unsupported batch deletion trigger phrases in vm-snapshot-delete
• RH-VIRT-041 — Narrow activation phrases to explicit snapshot restore only
• RH-VIRT-044 — Add ResourceQuota check before cloning to prevent resource exhaustion
• RH-VIRT-051 — Align VMI query threshold to 20 in vm-inventory

Infrastructure Improvements

• allowed-tools made a required field in the skill linter — prevents future omissions
• CI workflows: added iteration history, thumbs up/down reactions, and 3-run LLM consensus to reduce scanner false positives

Conclusion

The rh-virt pack had significant security gaps — primarily missing tool access controls (allowed-tools) and prompt injection vectors through Read tool instructions on external files. All 10 skills now declare
explicit tool boundaries, destructive operations enforce typed confirmations, and CLI fallback paths have been eliminated. The linter enforcement ensures new skills can't ship without allowed-tools.

[dmartinol]

Here we use resources_list but the allowed-tools list has mcp__openshift-virtualization__resources_list.
I'm not sure what is the right option, but we must be consistent.

[r2dedios]

the allowed-tools field was removed.

[dmartinol]

is the link correct? rh-virt folder is actually two parent levels above this SKILL.md file. Would the file be actually found?

[dmartinol]

did you evaluate this comment @r2dedios ?

[r2dedios]

From our perspective the links will not work, correct. But in terms of security, providing paths from the project's root folder enhances security and protecting our SKILL from injection attacks.

Example. If lola install the skills under .claude/skills/<skill-name> and our relative link to docs is ../../docs/troubleshooting/guide.md, it resolves to .claude/docs/troubleshooting/guide.md and that path doesn't exist. This scenario could cause an attacker to introduce fake doc in that path. However, specifying ./rh-virt/docs/troubleshooting/guide.md will resolve to the correct file regardless of where the SKILL.md is read from.

[dmartinol]

the point is that lola will not install such docs at all! it assumes docs are under skills// instead. please keep it as is as I'm preparing a bulk update of all docs to match the expected format

[dmartinol]

Thank you! /lgtm