Skip to content

fix(ocp-admin): replace anti-inspection directives#70

Merged
ikrispin merged 1 commit into
mainfrom
ocp-admin-security/remove-anti-inspection
May 5, 2026
Merged

fix(ocp-admin): replace anti-inspection directives#70
ikrispin merged 1 commit into
mainfrom
ocp-admin-security/remove-anti-inspection

Conversation

@ikrispin

@ikrispin ikrispin commented May 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Replace anti-inspection directives with auditable script usage rules

Addresses issues OCP-ADMIN-001, OCP-ADMIN-002.

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)

@github-actions

github-actions Bot commented May 5, 2026

Copy link
Copy Markdown

Gemini Code Review

1. Summary

This PR introduces a security enhancement to the cluster-report skill's assemble.py helper script by restricting file resolution to the /tmp/ directory, effectively preventing path traversal and Local File Inclusion (LFI) vulnerabilities. It also updates the corresponding unit tests and modifies SKILL.md to remove the "black box" restriction, allowing the AI agent to read the helper scripts for debugging purposes.

2. Project Rules Compliance

  • Document Consultation Transparency (Principle Add Red Hat Developer collection with Containerize & Deploy workflow #1): The update to SKILL.md perfectly aligns with the repository's design principles. By allowing the AI to read the scripts for debugging rather than treating them as black boxes, it encourages actual document consultation over hallucination.
  • Build Reminder: Since skills/cluster-report/SKILL.md was modified, please ensure you run make validate locally to verify that the skill linter and catalog compliance checks still pass.

3. Code Quality Issues

No blocking code quality issues or bugs were found.

  • The use of os.path.realpath(file_path) correctly resolves symlinks and normalizes paths like /tmp/../etc/passwd to /etc/passwd.
  • The .startswith("/tmp/") check correctly includes the trailing slash, which prevents bypasses using similarly named directories (e.g., /tmp-fake/).
  • The tests are appropriately updated to reflect the new /tmp/ requirement and correctly assert the new security boundary.

4. Suggestions

  • Hardcoded /tmp/ vs $TMPDIR: Hardcoding /tmp/ works perfectly fine for standard Linux/container environments. However, if there's any chance this script runs in environments where the temporary directory is overridden (e.g., via the TMPDIR environment variable) or on macOS (where /tmp is often a symlink to /private/tmp), this strict check might fail.
    Consideration for future-proofing (Optional):
    import tempfile
    import os
    
    # Get the system's temp directory and ensure it ends with a separator
    allowed_dir = os.path.realpath(tempfile.gettempdir()) + os.sep
    
    def resolve_file_ref(file_path):
        abs_path = os.path.realpath(file_path)
        if not abs_path.startswith(allowed_dir):
            return None, f"Path outside allowed directory: {file_path}"
        # ...
    (Note: If you know this will only ever run in standard OpenShift/Linux pods where /tmp/ is guaranteed, the current implementation is perfectly fine).

5. Verdict

APPROVE — The PR implements a solid security fix and improves the AI's debugging capabilities by relaxing an overly restrictive prompt rule. The code is well-tested and clean.


Automated review by Gemini 3.1 Pro | Rules: CLAUDE.md, SKILL_DESIGN_PRINCIPLES.md

@ikrispin ikrispin requested a review from dmartinol May 5, 2026 08:42
@dmartinol dmartinol changed the title fix(ocp-admin): replace anti-inspection directives with auditable scr… fix(ocp-admin): replace anti-inspection directives May 5, 2026
@ikrispin ikrispin merged commit 1fc015a into main May 5, 2026
3 checks passed
@ikrispin ikrispin deleted the ocp-admin-security/remove-anti-inspection branch May 5, 2026 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants