Back to skill

Security audit

Improvement Gate

Security checks for vulnerabilities and agentic risk

Overview

The skill is a disclosed review gate, but its local scripts can make or undo approval-related file changes with weak path and gate controls.

Install only after review or fixes if this will run against real repositories or shared approval state. In particular, require trusted signed artifacts, validate and contain rollback/review paths, make required gates non-bypassable, and use narrower triggers or explicit confirmation before approve/reject actions.

Vulnerability Patterns
  • Unauthorized Access and Privilege EscalationObtains permissions beyond the task's legitimate needs
  • Insecure Skill Coding PracticesFinds exploitable flaws such as hardcoded secrets or command injection
  • Skill Instruction HijackingAlters the agent's session goals or safety constraints when the skill loads
  • Agent Memory PoisoningWrites attacker-controlled rules into memory that affect later sessions
  • Remote Payload Retrieval and ExecutionFetches external code whose behavior can change after review
Findings (5)

T05 · Unauthorized Access and Privilege Escalation

Error
Location
scripts/gate.py:437
Finding

Protected-target validation can be bypassed through inconsistent artifact paths

Content
View full analysis

Vulnerability Details

File Location: scripts/gate.py:74-76 and scripts/gate.py:437-446
Vulnerability Type: Inconsistent authorization target validation
Risk Level: High

Vulnerable Code:

python
target = execution.get("result", {}).get("rollback_pointer", {}).get("target_path", "")
if target.endswith(".py"):
    import py_compile
python
category = candidate.get("category")
target_path = candidate.get("target_path")
keep_eligible = (
    recommendation == "accept_for_execution"
    and category in KEEP_CATEGORIES
    and candidate.get("risk_level") == "low"
    and not protected_target(target_path)
    and execution_result.get("status") in {"success", "no_change"}
)

Technical Analysis

The gate consumes two independently controlled representations of the modified target:

  • candidate["target_path"], used by protected_target() to make the authorization decision.
  • execution["result"]["rollback_pointer"]["target_path"], used by the compile and rollback logic.

The implementation does not canonicalize these paths and require them to identify the same file. Consequently, the protection check can be applied to a harmless declared path while the execution artifact references a different, protected target.

This is a confused-deputy and artifact-binding failure. A security decision must be made against the actual modified resource rather than an unverified claim in a separate artifact.

Attack Path

  1. Construct a ranking artifact containing a candidate categorized as a low-risk documentation, reference, or guardrail change.
  2. Set the candidate's target_path to a harmless unprotected file.
  3. Construct the corresponding execution artifact with status set to success or no_change.
  4. Set the execution artifact's rollback target to a different protected file that was actually modified.
  5. Ensure the candidate receives accept_for_execution and pass ...[truncated 638 chars]
Remediation
View remediation

Remediation Suggestions

  • Canonicalize both candidate and execution target paths before validation.
  • Require the canonical candidate target to exactly equal the canonical execution target.
  • Reject missing, mismatched, relative, or ambiguous target paths.
  • Verify that the target resides under an explicit repository or workspace allowlist.
  • Apply protected_target() to the canonical target obtained from trusted execution metadata.
  • Bind ranking and execution artifacts using the run ID, candidate ID, target path, and a trusted integrity mechanism.
  • Add tests for mismatched paths, symlink targets, traversal components, and protected execution targets paired with harmless candidate targets.

T05 · Unauthorized Access and Privilege Escalation

Error
Location
scripts/gate.py:370
Finding

Untrusted rollback pointers permit arbitrary writable-file replacement

Content
View full analysis

Vulnerability Details

File Location: scripts/gate.py:370-383
Vulnerability Type: Arbitrary file write through unvalidated rollback paths
Risk Level: High

Vulnerable Code:

python
def maybe_restore(execution_artifact: dict) -> dict:
    result = execution_artifact.get("result", {})
    rollback = result.get("rollback_pointer")
    if not rollback or not result.get("modified"):
        return {"attempted": False, "success": False, "reason": "no modified file to restore"}
    backup_path = Path(rollback["backup_path"]).expanduser().resolve()
    target_path = Path(rollback["target_path"]).expanduser().resolve()
    restore_backup(backup_path, target_path)
    return {
        "attempted": True,
        "success": True,
        "backup_path": str(backup_path),
        "target_path": str(target_path),
    }

Technical Analysis

The rollback source and destination are accepted directly from the execution artifact. Although the paths are resolved, resolution alone does not enforce authorization or containment.

The implementation does not verify that:

  • The target belongs to the current candidate.
  • The target is inside an authorized workspace.
  • The backup is inside a dedicated trusted backup directory.
  • The backup was created for the specified run and target.
  • Either path is free of symlink-based redirection.
  • The rollback pointer was issued by a trusted executor.

Because restore_backup() receives both attacker-selected paths, a rollback-triggering decision can act as an arbitrary file-copy or replacement primitive.

Attack Path

  1. Place attacker-controlled content in a readable file that will be supplied as backup_path.
  2. Create an execution artifact with result.modified set to true.
  3. Set rollback_pointer.backup_path to the attacker-controlled source.
  4. Set rollback_pointer.target_path to an arbitrary file writable by the gate process. 5 ...[truncated 811 chars]
Remediation
View remediation

Remediation Suggestions

  • Do not accept arbitrary rollback source and destination paths from an unsigned artifact.
  • Register backups in trusted state when they are created, including the run ID, candidate ID, canonical target, backup path, and content hash.
  • Require the target to be inside an explicit authorized workspace and the backup to be inside a dedicated backup root.
  • Verify that the rollback target exactly matches the canonical candidate target.
  • Reject symlinks and re-check containment immediately before opening or replacing files.
  • Use descriptor-based or otherwise race-resistant filesystem operations where possible.
  • Verify backup ownership, permissions, type, and hash before restoration.
  • Perform restoration atomically and report actual failures instead of unconditionally returning success: true.
  • Add negative tests for absolute external paths, traversal, symlinks, mismatched targets, and unregistered backups.

T09 · Insecure Skill Coding Practices

Error
Location
scripts/review.py:47
Finding

Review request identifier allows path traversal and unauthorized JSON modification

Content
View full analysis

Vulnerability Details

File Location: scripts/review.py:47-81
Vulnerability Type: Path traversal and arbitrary state-file overwrite
Risk Level: High

Vulnerable Code:

python
def complete_review(
    state_root: Path,
    request_id: str,
    decision: str,
    reviewer: str = "cli-user",
    reason: str = "",
) -> dict:
    """Mark a pending review as completed.
    ...
    """
    if decision not in ("approve", "reject"):
        raise ValueError(f"Decision must be 'approve' or 'reject', got: {decision!r}")

    reviews_dir = _reviews_dir(state_root)
    review_path = reviews_dir / f"{request_id}.json"
    if not review_path.exists():
        raise FileNotFoundError(f"Review request not found: {review_path}")

    data = read_json(review_path)
    if data.get("status") == "completed":
        raise ValueError(f"Review {request_id} is already completed (decision: {data.get('decision')})")

    data["status"] = "completed"
    data["decision"] = decision
    data["reviewer"] = reviewer
    data["comments"] = reason
    data["completed_at"] = utc_now_iso()
    write_json(review_path, data)
    return data

Technical Analysis

request_id is supplied by the CLI and interpolated directly into a filesystem path. The code does not reject path separators or traversal components, does not resolve the final path for a containment check, and does not require the identifier to match the expected review-ID format.

Values containing ../ can therefore escape state/reviews. If the resulting .json file exists and contains readable JSON, the function loads it and writes review-controlled fields back to the same location.

Checking review_path.exists() does not mitigate traversal; it only requires the selected target to exist.

Attack Path

  1. Identify an existing JSON file reachable relative to the reviews directory.
  2. Calculate a traversal-style request I ...[truncated 868 chars]
Remediation
View remediation

Remediation Suggestions

  • Validate request_id against a strict allowlist pattern, such as ^review-[A-Za-z0-9_-]+$.
  • Explicitly reject /, backslashes, traversal components, null characters, and platform-specific separators.
  • Resolve the reviews directory and final path, then require the final path's parent to equal the canonical reviews directory.
  • Obtain review files through a trusted registry rather than deriving arbitrary paths solely from user input.
  • Verify that the document's stored request_id exactly matches the requested identifier.
  • Verify the expected schema and require status == "pending" before modification.
  • Use safe atomic writes and reject symlinked review files.
  • Add tests for ../, absolute paths, nested paths, symlinks, malformed identifiers, and mismatched embedded request IDs.

T09 · Insecure Skill Coding Practices

Error
Location
scripts/gate.py:323
Finding

Callers can bypass all mandatory gate layers through the layer-selection option

Content
View full analysis

Vulnerability Details

File Location: scripts/gate.py:323-357 and scripts/gate.py:412-417
Vulnerability Type: Security-control bypass through unsafe configuration
Risk Level: High

Vulnerable Code:

python
parser.add_argument(
    "--layers",
    default=None,
    help="Comma-separated list of layer names to run (default: all). "
    "Available: schema,compile,lint,regression,review,human_review",
)
python
def select_layers(layer_names: str | None) -> list[GateLayer]:
    """Filter DEFAULT_GATE_LAYERS to only those whose name is in the comma-separated list."""
    if layer_names is None:
        return list(DEFAULT_GATE_LAYERS)
    requested = [n.strip() for n in layer_names.split(",") if n.strip()]
    available = {layer.name: layer for layer in DEFAULT_GATE_LAYERS}
    selected = []
    for name in requested:
        if name not in available:
            raise ValueError(f"Unknown gate layer: {name!r}. Available: {sorted(available)}")
        selected.append(available[name])
    return selected
python
selected_layers = select_layers(args.layers)
layer_verdict = run_gate_layers(candidate, execution_artifact, layers=selected_layers)

The success behavior for an empty or reduced layer list is established by:

python
results = []
all_passed = True
failed_at = None

for layer in layers:
    result = layer.validate(candidate, execution)
    results.append(result)
    if not result["passed"]:
        if layer.required:
            all_passed = False
            failed_at = layer.name
            break

Technical Analysis

The Skill describes schema, compile, regression, review, and doubt checks as required gates. However, the authoritative CLI permits callers to select any subset of layers.

A caller can select only an advisory layer such as lint, omit individual required controls, or provide a value that produces an ...[truncated 1351 chars]

Remediation
View remediation

Remediation Suggestions

  • Always include every required layer in authoritative gate executions.
  • If layer selection is retained, permit it to add or remove only advisory diagnostics.
  • Reject empty layer selections.
  • Separate reduced checks into a clearly named diagnostic command that cannot emit an authoritative keep decision.
  • Record the required-layer policy in code and verify that every required layer executed before decision processing.
  • Fail closed if a required layer is unavailable, throws an exception, or has no result.
  • Update the help text to include the doubt layer and clearly distinguish advisory diagnostics from mandatory controls.
  • Add integration tests proving that every required layer runs regardless of caller-supplied advisory options.

T09 · Insecure Skill Coding Practices

Warning
Location
scripts/gate.py:103
Finding

Required regression gate fails open when evaluator evidence is absent or disabled

Content
View full analysis

Vulnerability Details

File Location: scripts/gate.py:103-116 and scripts/gate.py:323-342; documentation mismatch at SKILL.md:60-66
Vulnerability Type: Fail-open validation and missing evaluator-artifact integration
Risk Level: Medium

Vulnerable Code:

python
class RegressionGate(GateLayer):
    """Layer 3: Check that the change doesn't regress any dimension."""

    def __init__(self):
        super().__init__("regression", required=True)

    def validate(self, candidate, execution=None):
        # Check evaluator evidence for regression signals
        evidence = candidate.get("evaluator_evidence", {})
        if not evidence or not evidence.get("enabled"):
            return {"passed": True, "details": "No evaluator evidence available", "layer": self.name}
        verdict = evidence.get("verdict", "")
        if verdict == "reject":
            return {"passed": False, "details": "Evaluator verdict: reject", "layer": self.name}
        return {"passed": True, "details": f"Evaluator verdict: {verdict}", "layer": self.name}

The CLI parser defines no --evaluation argument:

python
def parse_args() -> argparse.Namespace:
    parser = argparse.ArgumentParser(description="Apply gate decision for generic-skill lane")
    parser.add_argument("--ranking", required=True, help="Ranking artifact JSON")
    parser.add_argument("--execution", required=True, help="Execution artifact JSON")
    parser.add_argument("--state-root", default=str(DEFAULT_STATE_ROOT))
    parser.add_argument("--output", default=None)
    parser.add_argument(
        "--layers",
        default=None,
        help="Comma-separated list of layer names to run (default: all). "
        "Available: schema,compile,lint,regression,review,human_review",
    )
    return parser.parse_args()

Technical Analysis

Although RegressionGate is marked as required, it reports success when evalu ...[truncated 1591 chars]

Remediation
View remediation

Remediation Suggestions

  • Make required regression validation fail closed when evidence is missing, disabled, malformed, or contains an unknown verdict.
  • Implement the documented --evaluation argument and load the evaluator artifact from that explicit source.
  • Define an allowlist of valid verdicts and reject all unknown values.
  • Validate the evaluator artifact schema.
  • Require matching run ID, candidate ID, target path, and relevant execution identity across ranking, evaluation, and execution artifacts.
  • Protect artifact integrity through trusted storage, signatures, or authenticated hashes where artifacts cross trust boundaries.
  • If evaluation is legitimately optional for a defined change class, encode that as an explicit policy exemption rather than reporting absent evidence as a passed required gate.
  • Add tests for absent, disabled, empty, malformed, mismatched, and unknown evaluator evidence.
Vulnerability Patterns
  • Trigger AbuseOverly Broad Trigger, Shadow Command Trigger, Keyword Baiting Trigger
  • MCP Least PrivilegeUnderdeclared Capability, Wildcard Permission, Missing Permission Declaration
  • MCP Tool PoisoningHidden Instructions, Unicode Deception, Parameter Description Injection
  • Prompt InjectionInstruction Override, Hidden Instructions, Exfiltration Commands
  • Data ExfiltrationExternal Transmission, Env Variable Harvesting, File System Enumeration
Findings (9)

Tp4

High
Category
MCP Tool Poisoning
Confidence
97% confidence
Finding

A second description-behavior mismatch exists around human-review validation versus operational queue management: the skill emphasizes holistic gate enforcement but also supports completing approve/reject actions. In security-sensitive orchestration, this kind of overstated assurance can cause downstream systems or users to rely on nonexistent checks, resulting in unauthorized promotion or insufficient review of risky changes.

Content

No source excerpt is available for this finding.

Tp4

High
Category
MCP Tool Poisoning
Confidence
95% confidence
Finding

A second description-behavior mismatch exists around human-review validation versus operational queue management: the skill emphasizes holistic gate enforcement but also supports completing approve/reject actions. In security-sensitive orchestration, this kind of overstated assurance can cause downstream systems or users to rely on nonexistent checks, resulting in unauthorized promotion or insufficient review of risky changes.

Content

No source excerpt is available for this finding.

Undeclared Tool Scope

Medium
Category
MCP Least Privilege
Confidence
82% confidence
Finding

The skill advertises CLI operations that invoke local scripts and references external state/artifacts, but it does not declare any tool scope or permissions boundaries. In an agent system, missing scope declarations can let the runtime over-grant capabilities or make reviewers unaware that the skill may access networked or external resources, increasing the risk of unintended data access or side effects.

Content

No source excerpt is available for this finding.

Vague Triggers

Medium
Category
Not specified by scanner
Confidence
92% confidence
Finding

The trigger phrase human review is broad and likely to match many unrelated requests. Because this skill can list and complete review items, over-broad invocation raises the chance of accidental activation in contexts where the user only intended a generic discussion of review, potentially exposing workflow state or causing unintended approval actions.

Content

No source excerpt is available for this finding.

Vague Triggers

Medium
Category
Not specified by scanner
Confidence
86% confidence
Finding

The trigger 待审批 is very generic in Chinese and can apply to many unrelated approval scenarios. In a system where trigger matching auto-selects skills, this increases the risk that the skill is invoked outside its intended scope and performs queue-management or approval-related actions on the wrong workflow.

Content

No source excerpt is available for this finding.

Skill Enumeration

Medium
Category
Agent Snooping
Confidence
80% confidence
Finding

Skill enumerates or reads other installed skills. Access to other skills' SKILL.md files or the skills directory reveals prompt instructions, capabilities, and secrets that should be invisible to peer skills.

Content

Scanner excerpt · tests/test_gate.py (reported line 47)May include surrounding context.

python
"execution_plan": {"steps": []},
        "recommendation": "accept_for_execution",
        "panel_result": {"cognitive_label": "CONSENSUS"},
        "target_path": "skills/example/SKILL.md",
    }
    base.update(overrides)
    return base

Intent-Code Divergence

Low
Category
Not specified by scanner
Confidence
99% confidence
Finding

The module docstring states 'Includes a 6-layer mechanical validation system', but the implementation defines Schema, Compile, Lint, Regression, Review, Doubt, and HumanReview, and DEFAULT_GATE_LAYERS runs all seven. This is an active contradiction in the code documentation about the gate's behavior and structure.

Content

No source excerpt is available for this finding.

Intent-Code Divergence

Low
Category
Not specified by scanner
Confidence
95% confidence
Finding

The help text says available layers are 'schema,compile,lint,regression,review,human_review', but select_layers() can also accept 'doubt' because it is present in DEFAULT_GATE_LAYERS. This documentation misstates what the command actually supports.

Content

No source excerpt is available for this finding.

Intent-Code Divergence

Low
Category
Not specified by scanner
Confidence
97% confidence
Finding

The manifest describes a 7-layer mechanical gate: Schema→Compile→Lint→Regression→Review→Doubt→HumanReview. This test file’s module docstring states it covers a '6-layer' system while also saying 'including human review', which conflicts with the documented skill intent and can mislead maintainers about the gate model actually being exercised.

Content

No source excerpt is available for this finding.