Back to skill

Security audit

Code Review Gate

Security checks for vulnerabilities and agentic risk

Overview

This local code-review skill is read-only, but its CI gate can incorrectly allow unreviewed changes when diff collection fails or the diff is too large.

Install only if you are comfortable treating this as an advisory reviewer, or if you first fix the fail-open cases. Do not rely on its exit code as a merge-blocking security gate until git diff errors and oversized diffs fail closed, max-lines is validated as positive, and CI treats skipped analysis as blocking unless explicitly approved.

Vulnerability Patterns
  • 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
  • Embedded Malicious CodeShips malicious scripts inside the skill and executes them locally
Findings (2)

T09 · Insecure Skill Coding Practices

Error
Location
src/gate.py:77
Finding

Git Diff Failures Are Misclassified as Safe Review Skips

Content
View full analysis
bool: """判断是否应该跳过审查。""" diff = self.get_diff() if not diff.strip(): print("No changes detected. Skipping review.") return True ``` ```python def run(self) -> int: """执行门禁检查,返回退出码。""" if self.should_skip(): return 3 # 跳过 ``` ### Technical Analysis The `get_diff()` method does not inspect `subprocess.run()`'s return code and does not use `check=True`. If `git diff` fails, Git normally writes an error to stderr, returns a nonzero status, and may leave stdout empty. The method discards that failure state and returns only stdout. The empty result is subsequently interpreted by `should_skip()` as evidence that there are no changes. `run()` then returns exit code 3 without executing any of the seven review checkers. The documented gate behavior identifies exit code 3 as a non-blocking skip, so a Git error can become an approval-equivalent outcome. The subprocess call uses an argument array and does not enable a shell, so this is not a command-injection vulnerability. The issue is specifically the failure to distinguish a valid empty diff from an unsuccessful Git operation. ### Attack Path 1. An attacker or misconfigured CI invocation supplies an invalid, missing, or ambiguous revision through `--base` or `--head`. 2. The application constructs and runs `git diff ..`. 3. Git returns a nonzero exit status and writes diagnostic information to stderr. 4. `get_diff()` ignores both the nonzero status and stderr and returns emp ...[truncated 749 chars]
Remediation
View remediation

T09 · Insecure Skill Coding Practices

Error
Location
src/gate.py:129
Finding

Oversized or Invalidly Configured Diffs Bypass All Security Checks

Content
View full analysis
self.max_lines: print( f"Diff too large ({line_count} lines > {self.max_lines} max). " "Please split into smaller changes or increase --max-lines.", file=sys.stderr, ) return True return False ``` ```python def run(self) -> int: """执行门禁检查,返回退出码。""" if self.should_skip(): return 3 # 跳过 ``` The command-line value is accepted without positive-range validation: ```python parser.add_argument( "--max-lines", type=int, default=1000, help="单次审查最大行数 (默认: 1000)", ) ``` ### Technical Analysis When the diff exceeds `max_lines`, `should_skip()` returns `True`. The caller then exits with code 3 before instantiating or running any checker. The documented default is 1,000 lines, and the documented meaning of exit code 3 permits merging. A size limit can be appropriate for availability protection, but treating the limit as a successful, non-blocking skip makes the security control fail open. An attacker can deliberately enlarge a change or pad it with irrelevant lines until it exceeds the threshold. Additionally, `--max-lines` accepts zero or negative values. With a negative value, virtually every nonempty diff exceeds the configured limit and is skipped. This can create a deterministic bypass where users or CI callers can influence command-line arguments. ### Attack Path 1. The attacker prepares a change containing vulnerable or malicious code. 2. The attacker adds enough changed or padding lines to make the textual diff exceed the default 1,000-line limit. Alternatively, a controllable invocation supplies a non-positive `--max-lines` value. 3. `should_skip()` determines that `line_count > sel ...[truncated 821 chars]
Remediation
View remediation
self.max_lines: raise DiffTooLargeError( f"Diff contains {line_count} lines; maximum is {self.max_lines}" ) ``` 2. Prefer bounded chunking so large changes are still analyzed. Split the diff by file or hunk, run each checker on bounded chunks, and aggregate the findings. 3. Validate the command-line argument as a strictly positive integer: ```python def positive_int(value: str) -> int: parsed = int(value) if parsed <= 0: raise argparse.ArgumentTypeError("value must be greater than zero") return parsed ``` 4. Use `type=positive_int` for `--max-lines`. 5. Reserve exit code 3 for explicitly approved low-risk skip conditions, such as documentation-only changes verified by policy. Oversized changes should never be considered low risk merely because they cannot be scanned in one pass. 6. Configure downstream CI to block any result indicating that source-code analysis did not complete. 7. Add regression tests for a diff just below the limit, exactly at the limit, above the limit, and values of zero and negative one. ]]>
Vulnerability Patterns
  • Data ExfiltrationExternal Transmission, Env Variable Harvesting, File System Enumeration
  • Trigger AbuseOverly Broad Trigger, Shadow Command Trigger, Keyword Baiting Trigger
  • Behavioral ASTexec() Call, eval() Call, Dynamic Import
  • MCP Tool PoisoningHidden Instructions, Unicode Deception, Parameter Description Injection
  • Prompt InjectionInstruction Override, Hidden Instructions, Exfiltration Commands
Findings (36)

os.system() or os exec-family call

High
Category
Dangerous Code Execution
Confidence
99% confidence
Finding

This is a real command injection sink: untrusted input is concatenated into a shell command and executed with os.system(). Even in a mock/test file, this pattern is dangerous because copied code or accidental execution could allow arbitrary command execution on the host.

Content

Scanner excerpt · tests/mock_bad_code.py (reported line 65)May include surrounding context.

python
def run_user_command(user_input):
    """命令注入 - os.system 拼接 - 应被 SecurityChecker 检测。"""
    os.system("echo " + user_input)


def run_dynamic_script(script_name):

eval() call detected

High
Category
Dangerous Code Execution
Confidence
99% confidence
Finding

This is a real dynamic code execution vulnerability: eval() executes attacker-controlled Python code when script_name is influenced by external input. That can lead to arbitrary code execution, data theft, or system compromise.

Content

Scanner excerpt · tests/mock_bad_code.py (reported line 70)May include surrounding context.

python
def run_dynamic_script(script_name):
    """命令注入 - eval - 应被 SecurityChecker 检测。"""
    eval(script_name + "()")


# ============================================================

Vague Triggers

Medium
Category
Not specified by scanner
Confidence
91% confidence
Finding

The 'When to Use' section includes broad conditions such as '任何需要确保代码质量、安全性和设计一致性的场景', which could overlap with many ordinary development contexts without clearly defining when this skill should or should not be invoked. The file does not provide exclusion conditions or negative examples to narrow activation boundaries.

Content

No source excerpt is available for this finding.

Natural-Language Policy Violations

Medium
Category
Not specified by scanner
Confidence
81% confidence
Finding

The file's top-level docstring and user-facing labels/questions are written in Chinese, indicating the skill is designed around a fixed language/locale. There is no natural-language indication that users can choose another language or that the Chinese-only behavior is justified as region-specific.

Content

No source excerpt is available for this finding.

Natural-Language Policy Violations

Medium
Category
Not specified by scanner
Confidence
93% confidence
Finding

This code file contains user-facing natural-language strings exclusively in Chinese, including the module docstring and checker label. The provided policy says to flag language or locale constraints when a skill forces a specific language without user opt-in, and there is no indication here of language choice or a documented region-specific justification.

Content

No source excerpt is available for this finding.

Natural-Language Policy Violations

Medium
Category
Not specified by scanner
Confidence
93% confidence
Finding

The module-level docstring is written entirely in Chinese, and the checker’s user-facing labels, questions, details, risks, and fixes throughout the file are also hard-coded in Chinese. This indicates a locale/language constraint in natural-language output without any visible user choice or opt-in, which matches the language policy violation criteria.

Content

No source excerpt is available for this finding.

Intent-Code Divergence

Medium
Category
Not specified by scanner
Confidence
91% confidence
Finding

The module documentation says the checker covers "XSS / 命令注入", implying meaningful command-injection detection coverage. In code, COMMAND_INJECTION_PATTERNS only matches a few specific forms such as os.system with concatenation, subprocess.call(..., shell=True), and eval/exec with +, which is substantially narrower than the stated coverage and can miss many command-injection cases.

Content

No source excerpt is available for this finding.

Intent-Code Divergence

Medium
Category
Not specified by scanner
Confidence
96% confidence
Finding

The docstring states coverage of "权限控制缺失" as a general security category. The actual _check_auth logic only performs a simplified heuristic for route decorators without following real authorization logic, role checks, or non-decorator-based access control, so the documentation overstates what the code verifies.

Content

No source excerpt is available for this finding.

Natural-Language Policy Violations

Medium
Category
Not specified by scanner
Confidence
90% confidence
Finding

The module docstring and user-facing usage/help text are written in Chinese, including the skill description at L001-L007, which presents the tool in a single language with no opt-in or alternative locale. This is a natural-language locale policy concern because the file imposes one language by default rather than offering a choice or documenting that it is intentionally region-specific.

Content

No source excerpt is available for this finding.

Intent-Code Divergence

Medium
Category
Not specified by scanner
Confidence
98% confidence
Finding

The module usage examples and argparse help describe a selectable output format via --format, implying the tool can vary report rendering. However, after parsing the argument, the value is never passed into CodeReviewGate or Reporter, and the program always prints reporter.generate() with no format control, so the documentation overstates actual behavior.

Content

No source excerpt is available for this finding.

subprocess module call

Medium
Category
Dangerous Code Execution
Confidence
70% confidence
Finding

subprocess module calls execute external commands. Without careful input validation, this enables command injection.

Content

Scanner excerpt · src/gate.py (reported line 77)May include surrounding context.

python
cmd = ["git", "diff"]

        try:
            result = subprocess.run(
                cmd,
                capture_output=True,
                text=True,

subprocess module call

Medium
Category
Dangerous Code Execution
Confidence
70% confidence
Finding

subprocess module calls execute external commands. Without careful input validation, this enables command injection.

Content

Scanner excerpt · src/gate.py (reported line 102)May include surrounding context.

python
cmd = ["git", "diff", "--stat"]

        try:
            result = subprocess.run(
                cmd,
                capture_output=True,
                text=True,

Natural-Language Policy Violations

Medium
Category
Not specified by scanner
Confidence
87% confidence
Finding

The argparse description, epilog examples, and argument help strings are presented in Chinese, and this locale choice is not optional or justified as region-specific. Because these are user-facing strings that determine how the skill is used, they create a language policy issue under the locale-choice rule.

Content

No source excerpt is available for this finding.

Intent-Code Divergence

Medium
Category
Not specified by scanner
Confidence
87% confidence
Finding

The argument help says --severity controls the minimum reporting level, which implies it affects what findings are included or how the gate behaves. In this file, severity is only stored and passed to Reporter, while the actual pass/block decision is hard-coded to count only critical failures, so the documented operational meaning is misleading relative to enforcement behavior.

Content

No source excerpt is available for this finding.

Natural-Language Policy Violations

Medium
Category
Not specified by scanner
Confidence
96% confidence
Finding

This code file contains natural-language strings such as the module docstring and class docstrings exclusively in Chinese. Under the policy, forcing a specific language without offering user choice or documenting a justified locale constraint is a natural-language policy violation.

Content

No source excerpt is available for this finding.

Natural-Language Policy Violations

Medium
Category
Not specified by scanner
Confidence
89% confidence
Finding

The file contains user-facing natural-language strings and documentation in Chinese, including the module docstring and class docstring, which suggests a fixed language choice. Under the policy, forcing a specific language without user opt-in is a locale/language policy violation unless clearly justified or optional.

Content

No source excerpt is available for this finding.

Intent-Code Divergence

Medium
Category
Not specified by scanner
Confidence
89% confidence
Finding

The top-level docstring frames the file as a demonstration artifact for validating a code-review skill, implying intentionally synthetic examples. However, later code includes concrete-looking credentials and a production database URL at L44-L46, which contradicts the stated demo-only intent and could be mistaken for or reused as real secrets.

Content

No source excerpt is available for this finding.

Missing User Warnings

Medium
Category
Not specified by scanner
Confidence
86% confidence
Finding

The function executes a shell command via os.system using user-controlled input, which is a safety-critical operation under the code-file warning criteria. Although the docstring labels it as command injection for testing, there is no user-facing confirmation prompt, logging disclosure, or operational warning about executing shell commands.

Content

No source excerpt is available for this finding.

Missing User Warnings

Medium
Category
Not specified by scanner
Confidence
79% confidence
Finding

The function uses eval on dynamically constructed input, which is a safety-critical execution behavior for a code skill. The code contains no confirmation, visible disclosure, or explanatory warning to the user that arbitrary code-like execution may occur.

Content

No source excerpt is available for this finding.

External Transmission

Medium
Category
Data Exfiltration
Confidence
60% confidence
Finding

Data is being sent to an external URL. This could be legitimate telemetry or data exfiltration. Manual review is recommended.

Content

Scanner excerpt · tests/mock_bad_code.py (reported line 155)May include surrounding context.

python
print(f"Processing: {data}")
    timeout = 30
    max_retry = 5
    endpoint = "https://api.example.com/v2/tasks"
    print(f"Done. Timeout was {timeout}, retries: {max_retry}")
    return {"status": "ok", "endpoint": endpoint}

Natural-Language Policy Violations

Medium
Category
Not specified by scanner
Confidence
93% confidence
Finding

This Python test file includes user-facing natural-language strings almost exclusively in Chinese, such as docstrings and expected finding text. Under the policy, forcing a specific language without offering user choice or documenting a justified locale constraint is a natural-language policy violation.

Content

No source excerpt is available for this finding.

Intent-Code Divergence

Medium
Category
Not specified by scanner
Confidence
88% confidence
Finding

The docstring describes a straightforward verification flow, but the code later replaces get_diff, get_diff_stats, and should_skip on the gate object and explicitly comments that it is bypassing git and forcing execution. That is an intent-level contradiction because the script is not merely running the gate on a converted diff; it is altering control flow to avoid the gate's normal input and skip mechanisms.

Content

No source excerpt is available for this finding.

Natural-Language Policy Violations

Medium
Category
Not specified by scanner
Confidence
92% confidence
Finding

The module docstring and function docstring are written only in Chinese, which imposes a specific language on users/readers without any opt-in or documented locale constraint. The policy explicitly flags language or locale restrictions unless the skill offers choice or clearly justifies the locale limitation.

Content

No source excerpt is available for this finding.

Natural-Language Policy Violations

Low
Category
Not specified by scanner
Confidence
97% confidence
Finding

The summary and description are presented in Chinese only, and the document consistently assumes that locale without stating that language is configurable or user-selected. This can violate language/locale policy when a skill forces a specific language experience without user opt-in.

Content

No source excerpt is available for this finding.

Natural-Language Policy Violations

Low
Category
Not specified by scanner
Confidence
93% confidence
Finding

The manifest description is written in Chinese, and the config descriptions later in the file are also Chinese-only. For a general-purpose developer tool, this imposes a specific language/locale without any opt-in or documented justification, which matches the policy's language/locale violation criterion.

Content

No source excerpt is available for this finding.