Back to skill

Security audit

Claude Code Framework

Security checks for vulnerabilities and agentic risk

Overview

This skill needs review because it presents itself as a permission-checking execution framework, but its code allows approval-required actions to continue without real approval.

Review this carefully before installing or using it as a real security boundary. It is reasonable as a prototype or reference framework, but do not rely on it to control writes, shell commands, network calls, Git changes, browser actions, or messages until approval enforcement, hook-denial handling, logging minimization, and mode-change controls are fixed.

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 (3)

T09 · Insecure Skill Coding Practices

Error
Location
handler.ts:128
Finding
Approval-required tools execute without explicit user approval<![CDATA[ ## Vulnerability Details **File Location**: `handler.ts:128-145` **Vulnerability Type**: Fail-open authorization control **Risk Level**: High ### Vulnerable Code ```typescript if (assessment.risk === RiskLevel.APPROVE) { if (this.mode === ExecutionMode.READ_ONLY) { throw new Error(`Write operation not allowed in READ_ONLY mode: ${toolCall.tool}`); } if (this.mode === ExecutionMode.DEFAULT) { // TODO: 请求用户审批 console.warn(`⚠️ Approval required: ${toolCall.tool} - ${assessment.reason}`); } } // 执行 pre_tool_call await this.hooks.execute('pre_tool_call', { tool: toolCall.tool, args: toolCall.args }); result.hooksExecuted!.push(`pre_tool_call:${toolCall.tool}`); // 执行工具(在子类中实现) await this.executeTool(toolCall.tool, toolCall.args); ``` ### Technical Analysis The framework classifies sensitive tools as `APPROVE`, but DEFAULT mode does not obtain, validate, or record approval. It only emits a warning and then continues directly to `executeTool()`. This is a fail-open authorization design: absence of an approval provider or affirmative decision is treated as permission to proceed. The affected built-in categories include command execution, file writes and deletion, package installation, network requests, Git modification, messaging, and browser operations. Although the base implementation of `executeTool()` throws an unimplemented-operation error, the method is explicitly designed to be implemented by the runtime or a subclass. Once connected to a real tool executor, this control-flow flaw permits sensitive operations without the consent promised by the framework. ### Attack Path 1. An attacker supplies or influences a task that is translated into a sensitive tool call. 2. The risk classifier assigns the call the `APPROVE` level. 3. The framework runs in its normal DEFAULT mode. 4. The code logs an approval warning but does not pause, request approval, or require an authorization token. 5. Execution continues to `executeTool()`. 6 ...[truncated 766 chars]
Remediation
<![CDATA[ ## Remediation Suggestions - Introduce a mandatory approval provider that returns an explicit, authenticated decision for each `APPROVE` assessment. - Deny execution when the approval provider is absent, fails, times out, or returns anything other than an affirmative decision. - Bind approval to the exact tool name, normalized arguments, task identifier, and expiration time to prevent approval reuse or time-of-check/time-of-use substitution. - Do not treat logging as authorization. - Define mode behavior explicitly: - `DEFAULT`: require affirmative approval. - `READ_ONLY`: reject every operation outside a strict read-only allowlist. - `AUTO`: permit only policy-approved operations, rather than silently treating `APPROVE` as allowed. - `BYPASS`: disable or remove in production, or protect it with privileged configuration. - Add tests proving that `executeTool()` is never called when approval is missing, denied, expired, malformed, or associated with different arguments. ]]>

T09 · Insecure Skill Coding Practices

Error
Location
handler.ts:138
Finding
Pre-execution hook denial results are ignored<![CDATA[ ## Vulnerability Details **File Location**: `handler.ts:101-104` and `handler.ts:138-145`; hook contract at `hook-manager.ts:125-127` **Vulnerability Type**: Security-hook enforcement bypass **Risk Level**: High ### Vulnerable Code ```typescript // 1. pre_task Hook await this.hooks.execute('pre_task', { task, mode: this.mode }); result.hooksExecuted!.push('pre_task'); ``` ```typescript // 执行 pre_tool_call await this.hooks.execute('pre_tool_call', { tool: toolCall.tool, args: toolCall.args }); result.hooksExecuted!.push(`pre_tool_call:${toolCall.tool}`); // 执行工具(在子类中实现) await this.executeTool(toolCall.tool, toolCall.args); ``` The hook manager explicitly defines `proceed: false` as a stop decision: ```typescript // 如果 proceed 为 false,停止执行 if (result.proceed === false) { break; } ``` ### Technical Analysis `HookManager.execute()` returns a `HookResult` whose `proceed` field communicates whether processing should continue. The framework invokes both `pre_task` and `pre_tool_call`, but discards their returned values. Consequently, `proceed: false` only stops execution of additional handlers inside that particular hook chain. It does not stop the task or prevent the tool invocation. This breaks the expected security semantics of permission, policy, validation, and context-budget hooks. The built-in `pre_task` hook demonstrates the problem by returning `{ proceed: false, error: 'Context blocked' }` when the budget status is blocked. A custom permission hook can behave correctly and still be unable to enforce its decision. ### Attack Path 1. A defender registers a `pre_task` or `pre_tool_call` security hook. 2. An attacker causes a task or tool call that violates the hook's policy. 3. The hook detects the violation and returns `proceed: false`. 4. `HookManager.execute()` returns that denial to the framework. 5. `executeTask()` discards the result and continues. 6. For a tool-level denial, `executeTool()` is invoked despite the security hook's ...[truncated 697 chars]
Remediation
<![CDATA[ ## Remediation Suggestions - Capture and enforce every pre-execution hook result: ```typescript const taskDecision = await this.hooks.execute('pre_task', { task, mode: this.mode }); if (taskDecision.proceed !== true) { throw new Error(taskDecision.error ?? 'Task rejected by pre_task hook'); } const toolDecision = await this.hooks.execute('pre_tool_call', { tool: toolCall.tool, args: toolCall.args }); if (toolDecision.proceed !== true) { throw new Error(toolDecision.reason ?? 'Tool call rejected by pre_tool_call hook'); } ``` - Fail closed if a hook returns `undefined`, malformed output, or a non-boolean `proceed` value. - Distinguish enforcement hooks from observational hooks and require all enforcement hooks to approve before execution. - Prevent later hooks from overriding an earlier denial. - Ensure hook timeouts and errors deny sensitive operations unless a documented, narrowly scoped fail-open policy is explicitly configured. - Add unit and integration tests that assert `executeTool()` is not reached after task-level or tool-level denial. ]]>

T09 · Insecure Skill Coding Practices

Warning
Location
risk-classifier.ts:101
Finding
Destructive network block patterns are compiled with incorrect semantics<![CDATA[ ## Vulnerability Details **File Location**: `risk-classifier.ts:101-114`; affected rules at `handler.ts:68-69` **Vulnerability Type**: Incorrect security-pattern compilation **Risk Level**: Medium ### Vulnerable Code The intended blocking rules are: ```typescript block: [ 'format', 'diskpart', 'net user', 'net localgroup', 'reg delete', 'reg add', 'curl.*--delete', 'wget.*--delete', 'shutdown', 'restart' ] ``` They are compiled as follows: ```typescript private patternToRegex(pattern: string): RegExp { // 精确匹配 if (!pattern.includes('*') && !pattern.includes('.')) { return new RegExp(`^${pattern}$`, 'i'); } // 简单 glob const escaped = pattern .replace(/[.+^${}()|[\]\\]/g, '\\$&') .replace(/\*/g, '.*') .replace(/\?/g, '.'); return new RegExp(escaped, 'i'); } ``` ### Technical Analysis The rules appear to intend regular-expression semantics in which `.*` means any sequence of characters. However, `patternToRegex()` treats input as a glob-like pattern and escapes the dot before expanding the asterisk. For example, `curl.*--delete` is transformed into a regular expression equivalent to: ```text curl\..*--delete ``` This requires a literal period immediately after `curl`. A normal command or argument string such as `curl --delete https://example.test/item` therefore does not match the BLOCK rule. The generic `curl` or `wget` approval rule can still classify the operation as `APPROVE`, but the separate fail-open approval flaw means that this downgrade can lead directly to execution without consent. The classifier also scans a JSON-serialized argument string rather than parsing command structure, flags, shell quoting, aliases, or equivalent HTTP methods. This makes the policy fragile against syntactic variations. ### Attack Path 1. An attacker causes a tool call containing a destructive network request, such as a curl invocation using a DELETE method. 2. The classifier tests the serialized arguments agains ...[truncated 1072 chars]
Remediation
<![CDATA[ ## Remediation Suggestions - Do not mix regular-expression syntax with glob syntax. Define one documented rule format and validate it at configuration load time. - Prefer structured policies based on tool identity and parsed arguments rather than substring matching over `JSON.stringify(args)`. - For HTTP tools, normalize and inspect the actual HTTP method, destination, credential use, and body rather than searching for command text. - If regular expressions are required, accept explicitly delimited and reviewed expressions or compile fixed internal expressions directly. - Anchor patterns where appropriate and reject invalid or ambiguous custom rules. - Add tests covering: - `curl --delete URL` - `curl -X DELETE URL` - `curl --request DELETE URL` - Equivalent wget or HTTP-client forms. - Case, whitespace, quoting, and argument-order variations. - Benign strings that must not trigger false positives. - Combine the classifier fix with fail-closed approval enforcement so that a classification downgrade cannot lead to automatic execution. ]]>
Vulnerability Patterns
  • Data ExfiltrationExternal Transmission, Env Variable Harvesting, File System Enumeration
  • Excessive AgencyUnrestricted Tool Access, Autonomous Decision Making, Scope Creep
  • Trigger AbuseOverly Broad Trigger, Shadow Command Trigger, Keyword Baiting Trigger
  • MCP Tool PoisoningHidden Instructions, Unicode Deception, Parameter Description Injection
  • Prompt InjectionInstruction Override, Hidden Instructions, Exfiltration Commands
Findings (13)

Intent-Code Divergence

High
Confidence
99% confidence
Finding
The code claims every task is subject to permission checks, but in DEFAULT mode an APPROVE-rated tool only triggers a warning and execution continues. This creates a fail-open authorization path where writes, network access, package installs, browser/message actions, or shell execution can run without explicit user consent.

Missing User Warnings

High
Confidence
99% confidence
Finding
The APPROVE branch only logs 'Approval required' in DEFAULT mode and then proceeds into pre_tool_call and executeTool. Because many sensitive tools are classified as approve (exec, write, delete, curl/wget, git push, npm/pip install, browser, message), an attacker or unsafe prompt can trigger side effects without any real authorization barrier.

Natural-Language Policy Violations

Medium
Confidence
95% confidence
Finding
The entire skill description is written in Chinese and does not indicate that language selection is optional or that the skill is intended only for a Chinese-language context. Under the stated policy, forcing a specific language without user opt-in is a natural-language locale violation.

Missing User Warnings

Medium
Confidence
92% confidence
Finding
The skill describes post-task logging and memory updates but does not disclose what data is retained, where it is stored, how long it persists, or whether sensitive task content may be captured. In an agent execution framework, silent logging and memory persistence can expose prompts, file contents, credentials, or operational history beyond user expectations.

Vague Triggers

Medium
Confidence
91% confidence
Finding
The skill presents multiple invocation styles and mode-switching commands without clearly defining the authoritative activation path or safety boundaries. In an agent framework that governs tool permissions, ambiguity can cause operators or downstream agents to invoke the wrong command set, bypass expected review steps, or assume protections are active when they are not.

Natural-Language Policy Violations

Medium
Confidence
87% confidence
Finding
The manifest uses Chinese-only natural-language fields for the skill description and author-facing metadata, and later command descriptions are also Chinese-only. This indicates a language constraint without any visible opt-in, alternative locale, or justification that the skill is intended only for a Chinese-language environment.

Natural-Language Policy Violations

Medium
Confidence
90% confidence
Finding
All user-facing command descriptions in this section are Chinese-only, which can violate language/locale policy when no alternative language option or opt-in is provided. There is also no note indicating the skill is region-specific or restricted to Chinese-speaking users.

Missing User Warnings

Medium
Confidence
93% confidence
Finding
The hook manager records full hook inputs and outputs into an internal execution log, which can include tool arguments, messages, errors, or other sensitive runtime data. In an agent framework, hooks often sit around privileged operations, so retaining raw payloads increases the chance of accidental disclosure through debugging, log export, or later compromise of process memory/application state.

Missing User Warnings

Medium
Confidence
95% confidence
Finding
The decorator forwards the full method argument list to hook execution, exposing all runtime parameters to any registered hook for that lifecycle event. Because hooks are extensibility points that may be third-party or loosely trusted, this creates a broad data-exposure surface and can leak secrets, user content, or privileged internal objects beyond the original method boundary.

Natural-Language Policy Violations

Low
Confidence
95% confidence
Finding
The file's natural-language comments and descriptions are written in Chinese, including the primary class description and operational notes, without indicating that the skill supports other languages or that Chinese is required for a documented regional purpose. This can violate language/locale policy when a skill imposes a specific language without user opt-in.

Natural-Language Policy Violations

Low
Confidence
81% confidence
Finding
The top-level natural-language comments are written in Chinese and present the framework description in that locale without offering any user language choice or documenting a region-specific constraint. Under the policy, language constraints should be optional or explicitly justified.

Natural-Language Policy Violations

Low
Confidence
78% confidence
Finding
The file's descriptive comments are written in Chinese, and the implementation does not indicate any user-selectable language or a region-specific justification. Under the stated policy, forcing a specific language without opt-in can be a locale-policy issue when embedded in skill-facing instructions or messaging.

Natural-Language Policy Violations

Low
Confidence
82% confidence
Finding
The natural-language comments and title present the skill description entirely in Chinese alongside a bilingual title, with no indication that language is configurable or intentionally region-specific. This can violate language/locale policy when a skill implicitly requires a specific language without user opt-in.

Static analysis

No suspicious patterns detected.