T09 · Insecure Skill Coding Practices
Error
- Location
scripts/quotation-integration.js:355- Finding
Shell Command Injection Through Environment-Controlled Workflow Path
- Content
View full analysis
- Remediation
View remediation
Security audit
Pricing Engine
Security checks for vulnerabilities and agentic risk
Overview
This is a coherent pricing and quotation skill, but it needs Review because it can generate business quote files without enforcing its claimed approval hold and it uses an environment-controlled path in shell commands.
Install only in a controlled environment. Review and patch quotation-integration.js to use execFile/spawn with validated paths, and do not rely on pending_approval as a hold until the workflow actually blocks file generation and records approvals. Treat output/price-history.jsonl and generated quotation files as sensitive business records with clear retention and access controls.
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)
other
Warning
- Location
scripts/quotation-integration.js:498- Finding
Below-Floor Quotation Approval Gate Is Not Enforced
- Content
View full analysis
0 ? 'pending_approval' : 'ready'; // Step 4: 构建 quotation-workflow 数据 const quotationData = buildQuotationData( params, priceResult.pricedItems, quotationNo, priceResult.floorWarnings ); // Step 5: 生成报价单文件（PDF/HTML） const fileResult = generateQuotationFiles(quotationData, quotationNo); // Step 6: 记录报价历史 const historyRecords = recordAllQuotes( priceResult.pricedItems, quotationNo, params.customerId, params.currency || 'USD' ); ``` The configured policy states that below-floor quotations require notification and approval: ```json "on_trigger": { "action": "notify_and_hold", "discord_channel": "pricing-alerts", "require_approval": true, "approval_timeout_hours": 48 } ``` ### Technical Analysis When one or more items trigger the floor-price rule, the application changes the quotation status to `pending_approval`. It does not, however, stop processing or verify that an authorized approver has released the quotation. The function continues to: 1. Build the quotation data. 2. Generate HTML and PDF quotation artifacts. 3. Record the quotation in price history. 4. Return paths to the generated documents. The `require_approval` and `notify_and_hold` configuration values are therefore descriptive rather than enforced controls. No approval record, authorization check, signed approval token, approver identity, or approval-state transition is evaluated before document generation. The documented Discord notification is also not implemented in the reviewed code. Marking an object as pending does not provide a security boundary when the sensitive operation still o ...[truncated 1737 chars]- Remediation
View remediation
0) { const approvalRequest = createApprovalRequest({ customerId: params.customerId, items: priceResult.pricedItems, warnings: priceResult.floorWarnings }); return { quotationNo, status: 'pending_approval', approvalRequestId: approvalRequest.id, pdfPath: null, htmlPath: null, generatedFiles: [] }; } ``` 2. Implement a separate approval operation that: - Requires an authenticated and authorized approver. - Loads the exact pending quotation. - Verifies that its pricing inputs and calculated totals have not changed. - Records the approver identity, decision, timestamp, and comments. - Generates files only after approval succeeds. 3. Bind approvals cryptographically or transactionally to an immutable quotation digest. Include the customer, items, quantities, currency, prices, margins, and warning details in that digest so approval cannot be reused for a modified quotation. 4. Store explicit state transitions such as: ```text draft -> pending_approval -> approved -> generated -> rejected -> expired ``` Reject invalid transitions and prevent direct movement from `pending_approval` to `generated`. 5. Implement the configured notification behavior or remove the unsupported claim. Notification failure should not release the quotation. 6. Enforce the configured approval timeout. Expired requests should require recalculation and a new approval because copper prices and exchange rates may have changed. 7. Do not expose customer-distributable HTML or PDF paths while a quotation is pending. If an internal preview is necessary, visibly watermark it as unapproved and store it separately from final artifacts. 8. Record approval metadata in the audit history, including the approval request ID, approve ...[truncated 383 chars]
Vulnerability Patterns
- Data ExfiltrationExternal Transmission, Env Variable Harvesting, File System Enumeration
- Excessive AgencyUnrestricted Tool Access, Autonomous Decision Making, Scope Creep
- 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
Findings (26)
Tp4
High
- Category
- MCP Tool Poisoning
- Confidence
- 97% confidence
- Finding
This variant goes beyond mere feature mismatch by indicating undeclared persistent local storage of customer quote history. Hidden or under-declared data retention changes the security/privacy posture of the skill and can expose sensitive commercial data if users or the platform do not realize records are being stored on disk.
- Content
Tp4
High
- Category
- MCP Tool Poisoning
- Confidence
- 96% confidence
- Finding
This variant goes beyond mere feature mismatch by indicating undeclared persistent local storage of customer quote history. Hidden or under-declared data retention changes the security/privacy posture of the skill and can expose sensitive commercial data if users or the platform do not realize records are being stored on disk.
- Content
Tp4
High
- Category
- MCP Tool Poisoning
- Confidence
- 98% confidence
- Finding
This variant goes beyond mere feature mismatch by indicating undeclared persistent local storage of customer quote history. Hidden or under-declared data retention changes the security/privacy posture of the skill and can expose sensitive commercial data if users or the platform do not realize records are being stored on disk.
- Content
Tp4
High
- Category
- MCP Tool Poisoning
- Confidence
- 83% confidence
- Finding
This variant goes beyond mere feature mismatch by indicating undeclared persistent local storage of customer quote history. Hidden or under-declared data retention changes the security/privacy posture of the skill and can expose sensitive commercial data if users or the platform do not realize records are being stored on disk.
- Content
Ae1
High
- Category
- analysis-evasion
- Confidence
- 100% confidence
- Finding
Referenced artifact was not completely inspected
- Content
md ### scripts/pricing-engine.js — 核心定价引擎
Ae1
High
- Category
- analysis-evasion
- Confidence
- 100% confidence
- Finding
Referenced artifact was not completely inspected
- Content
md ### scripts/quotation-integration.js — 集成报价单工作流
Ae1
High
- Category
- analysis-evasion
- Confidence
- 100% confidence
- Finding
Referenced artifact was not completely inspected
- Content
md ### scripts/exchange-rate.js — 汇率模块
Ae1
High
- Category
- analysis-evasion
- Confidence
- 100% confidence
- Finding
Referenced artifact was not completely inspected
- Content
md ### scripts/copper-price-adapter.js — 铜价适配层
Ae1
High
- Category
- analysis-evasion
- Confidence
- 100% confidence
- Finding
Referenced artifact was not completely inspected
- Content
md ### scripts/price-history.js — 历史报价记录
Natural-Language Policy Violations
Medium
- Category
- Not specified by scanner
- Confidence
- 94% confidence
- Finding
The skill documentation is written almost entirely in Chinese, including overview, usage guidance, environment variable descriptions, and operational notes, with no indication that other languages are supported. This creates a language/locale policy concern because users are effectively required to use a specific language without explicit opt-in or justification.
- Content
Undeclared Tool Scope
Medium
- Category
- MCP Least Privilege
- Confidence
- 84% confidence
- Finding
The skill documents environment-variable-driven behavior, logging, caching, and persistent file paths but does not declare any explicit tool scope or permissions. In an agent environment, this can cause the skill to run with broader-than-expected access to environment data and local resources, weakening least-privilege controls and making misuse or accidental data exposure more likely.
- Content
Natural-Language Policy Violations
Medium
- Category
- Not specified by scanner
- Confidence
- 96% confidence
- Finding
The manifest description forces a specific language presentation for the skill metadata, which can violate language/locale policy when no user opt-in or alternative language is provided. There is no indication that the skill is intentionally restricted to a Chinese-only region or audience.
- Content
Natural-Language Policy Violations
Medium
- Category
- Not specified by scanner
- Confidence
- 91% confidence
- Finding
Nearly all user-facing instructions and examples are presented only in Chinese, with no indication that users can choose another language. Under the stated policy, a fixed locale/language is a concern unless it is optional or clearly justified as region-specific.
- Content
Missing User Warnings
Medium
- Category
- Not specified by scanner
- Confidence
- 93% confidence
- Finding
The skill states that quotation history is automatically recorded, but it does not prominently warn that customer pricing data will be persisted. Customer IDs, negotiated prices, quantities, and quote history are commercially sensitive; silent or poorly disclosed storage increases the risk of privacy violations, competitive exposure, and unintentional retention of sensitive business records.
- Content
Missing User Warnings
Medium
- Category
- Not specified by scanner
- Confidence
- 95% confidence
- Finding
Appending customer quote records to a persistent JSONL file without a clear warning creates a confidentiality and retention risk. JSONL files are easy to copy, search, and exfiltrate, so storing pricing history without prominent disclosure and safeguards can expose sensitive customer-specific commercial terms.
- Content
Natural-Language Policy Violations
Medium
- Category
- Not specified by scanner
- Confidence
- 89% confidence
- Finding
Natural-language descriptions and notification content in this JSON are entirely in Chinese, including the user-facing alert template. For a general skill/config file, this imposes a language choice without indicating user selection, opt-in, or a documented region-specific constraint.
- Content
Description-Behavior Mismatch
Medium
- Category
- Not specified by scanner
- Confidence
- 96% confidence
- Finding
Manifest 将该技能描述为“动态定价引擎”，应基于 LME 铜价、数量阶梯、客户等级、实时汇率自动计算报价，并集成 quotation-workflow 生成报价单。但本文件文档与实现都仅提供铜价读取、SKU 铜用量成本计算、缓存和 CLI 查询功能；代码中既没有数量/客户等级输入，也没有实时汇率对接或报价单生成调用。该实现明显窄于技能宣称的行为范围。
- Content
Natural-Language Policy Violations
Medium
- Category
- Not specified by scanner
- Confidence
- 88% confidence
- Finding
This file’s natural-language interface, comments describing usage, and CLI help/output are entirely in Chinese, and the CLI does not offer any language selection or indicate that the tool is intentionally region-specific. Under the language/locale policy, forcing a specific language without opt-in is a policy concern.
- Content
Natural-Language Policy Violations
Medium
- Category
- Not specified by scanner
- Confidence
- 89% confidence
- Finding
The comments and implementation explicitly force timestamps and date generation to use +08:00, which is a locale-specific behavior embedded in the skill. The file does not offer a user choice or explain why this region-specific timezone is required, so it can violate the language/locale policy for general-purpose use.
- Content
Natural-Language Policy Violations
Medium
- Category
- Not specified by scanner
- Confidence
- 93% confidence
- Finding
This code presents user-facing CLI output, usage text, and operational messages primarily in Chinese, while also exposing an English command interface, but it does not offer any user opt-in or locale selection. That creates a natural-language locale policy concern because the skill effectively enforces a specific language for user interaction.
- Content
Context-Inappropriate Capability
Medium
- Category
- Not specified by scanner
- Confidence
- 93% confidence
- Finding
The module invokes external programs via execSync to run Python scripts and a Chrome binary for HTML/PDF generation. Although some paths are quoted, this still creates an unnecessary command-execution surface for a pricing skill, especially because QUOTATION_WORKFLOW_ROOT is taken from the environment and shell commands are constructed as strings; if an attacker can influence environment, filesystem layout, or downstream scripts, they may trigger unintended code execution or execution of untrusted helper scripts.
- Content
Natural-Language Policy Violations
Medium
- Category
- Not specified by scanner
- Confidence
- 95% confidence
- Finding
This .js file contains extensive user-facing console messages in Chinese, such as the generation and preview summaries, while the file also includes English usage/help text and no opt-in or configuration for language selection. Under the policy, forcing a specific language without user choice is a natural-language policy violation unless clearly justified as region-specific.
- Content
Natural-Language Policy Violations
Low
- Category
- Not specified by scanner
- Confidence
- 80% confidence
- Finding
The natural-language description and notes are written in Chinese with no indication that language selection is optional or that this file is intentionally limited to a Chinese-speaking operational context. Per the policy, forcing a specific language without user opt-in can be a language/locale policy violation.
- Content
Natural-Language Policy Violations
Low
- Category
- Not specified by scanner
- Confidence
- 85% confidence
- Finding
This JSON contains user-facing descriptive text in both Chinese and English, such as the Chinese-only descriptions on L04, L49, L54, L59, L64, L69, L71, and L76 alongside English labels elsewhere. For an all-file-types language policy check, this can be a locale-policy concern because the file implicitly fixes multilingual output behavior without stating whether users may choose language or whether the mixed locale is intentional.
- Content
Natural-Language Policy Violations
Low
- Category
- Not specified by scanner
- Confidence
- 84% confidence
- Finding
The JSON metadata notes are written in Chinese and describe the data schema in a way that implies a fixed language choice for operators or downstream users. Because SQP-3 applies to all file types, this is a natural-language locale policy concern when no opt-in or region-specific justification is provided in the file.
- Content
Static analysis
Detected: suspicious.dangerous_exec
Shell command execution detected (child_process).
Critical
- Code
- suspicious.dangerous_exec
- Location
- scripts/quotation-integration.js:355