hectorlee-momentum-position-advisor

• Outcome: Review
• Latest audit: Sep 16, 2026 · 10:46 AM UTC
• Version: 1.4.0

Overview

This looks like a legitimate stock-analysis skill, but it needs Review because it imports executable code from a user-writable sibling skill and can overwrite local portfolio-related files.

Install only if you trust both this skill and the local volume-price-screener skill it imports from. Use explicit .json portfolio filenames, keep backups of portfolio files, and treat all HOLD/REDUCE/SELL output as advisory rather than executable trading instruction. Review the Chinese-only output, broad triggers, and fundamentals/events overlay so you know when recommendations are based on more than price-volume momentum.

A.I.G

Vulnerability Patterns

• Insecure Dependencies: Introduces malicious components through unsafe dependency sources
• Insecure Skill Coding Practices: Finds exploitable flaws such as hardcoded secrets or command injection
• Skill Instruction Hijacking: Alters the agent's session goals or safety constraints when the skill loads
• Agent Memory Poisoning: Writes attacker-controlled rules into memory that affect later sessions
• Remote Payload Retrieval and Execution: Fetches external code whose behavior can change after review

Findings (2)

T08 · Insecure Dependencies

Warning

• Location: scripts/advisor.py:21
• Finding: Unverified Python Modules Loaded from a Mutable User Directory<![CDATA[ ## Vulnerability Details **File Location**: `scripts/advisor.py:21-28, 61-62`; `scripts/cross_ref.py:15-17, 42-46` **Vulnerability Type**: Unsafe dynamic module resolution from an unverified sibling Skill **Risk Level**: Medium ### Vulnerable Code ```python # scripts/advisor.py:21-28 _SCREENER_DIR = os.path.expanduser('~/.workbuddy/skills/volume-price-screener/scripts') _CURRENT_DIR = os.path.dirname(os.path.abspath(__file__)) if '__file__' in dir() and '__file__' in locals() else os.path.dirname(os.path.abspath(sys.argv[0])) if _CURRENT_DIR not in sys.path: sys.path.insert(0, _CURRENT_DIR) if os.path.exists(_SCREENER_DIR) and _SCREENER_DIR not in sys.path: sys.path.append(_SCREENER_DIR) ``` ```python # scripts/advisor.py:61-62 try: from data_provider import get_kline, get_realtime_quote ``` ```python # scripts/cross_ref.py:15-17 _SCREENER_DIR = os.path.expanduser('~/.workbuddy/skills/volume-price-screener/scripts') if _SCREENER_DIR not in sys.path: sys.path.insert(0, _SCREENER_DIR) ``` ```python # scripts/cross_ref.py:42-46 try: from pattern_detect import detect_pattern, Bar from scoring import score_pattern except ImportError: return None ``` ### Technical Analysis The application adds `~/.workbuddy/skills/volume-price-screener/scripts` to Python's module search path and subsequently imports `data_provider`, `pattern_detect`, and `scoring` by unqualified module name. No package version, cryptographic hash, canonical origin, file ownership, or directory permission verification is performed before these modules are imported. In `cross_ref.py`, the external directory is inserted at index zero, giving modules in that directory priority over other modules with the same names. Python executes module-level code during import. Therefore, any party able to create or replace files in the sibling Skill directory can execute arbitrary Python code when the affected import path is reached. This is a local supply-chain and depe ...[truncated 1481 chars]
• Remediation: <![CDATA[ ## Remediation Suggestions 1. Package the sibling functionality as a normal, versioned Python dependency rather than modifying `sys.path`. 2. Pin the dependency to an audited version and use a lockfile containing cryptographic hashes. 3. Install dependencies in an isolated virtual environment with restricted write permissions. 4. Use package-qualified imports to prevent module-name collisions. 5. If external-directory loading is unavoidable: - Resolve and validate the canonical directory path. - Confirm that the directory and module files are owned by the expected user. - Reject group-writable or world-writable directories and files. - Verify each imported file against a trusted SHA-256 allowlist. - Load modules by an explicit validated file path rather than by an ambiguous module name. 6. Avoid placing external dependency directories at the beginning of `sys.path`. 7. Document the sibling Skill as an executable trust dependency in the manifest, including its expected version and integrity requirements. ]]>

T09 · Insecure Skill Coding Practices

Warning

• Location: scripts/advisor.py:219
• Finding: User-Supplied Portfolio File Can Be Overwritten by Decision-State Persistence<![CDATA[ ## Vulnerability Details **File Location**: `scripts/advisor.py:219-225, 246-248` **Vulnerability Type**: Unsafe derivation of a writable state-file path from user input **Risk Level**: Medium ### Vulnerable Code ```python # scripts/advisor.py:219-225 prev_file = filepath.replace('.json', '_prev_decisions.json') if filepath else os.path.join( os.path.dirname(os.path.abspath(__file__)), '..', 'data', 'prev_decisions.json' ) try: if os.path.exists(prev_file): with open(prev_file, 'r') as f: prev_decisions = json.load(f) ``` ```python # scripts/advisor.py:246-248 os.makedirs(os.path.dirname(prev_file), exist_ok=True) with open(prev_file, 'w') as f: json.dump(current_decisions, f, indent=2) ``` ### Technical Analysis The `--file` command-line option controls `filepath`. The application derives the persistent decision-state path using: ```python filepath.replace('.json', '_prev_decisions.json') ``` If the supplied filename does not contain the exact lowercase substring `.json`, `str.replace()` returns the original path unchanged. The application then opens that same path in write mode after portfolio processing, truncating the original file and replacing its portfolio structure with a decision map. The path is not constrained to the project data directory, and no check confirms that `prev_file` differs from `filepath`. The code also does not reject symbolic links or use an atomic, no-follow write strategy. Consequently, an existing symbolic link may redirect the write to another file that the invoking user is permitted to modify. ### Attack Path 1. A user or calling automation supplies a valid portfolio JSON file whose name does not contain lowercase `.json`, for example: ```text python advisor.py --portfolio --file /home/user/portfolio ``` 2. `get_positions()` successfully reads `/home/user/portfolio` as JSON. 3. `filepath.replace('.json', '_prev_decisions.json')` produces `/home/user/portfolio`, because ...[truncated 1012 chars]
• Remediation: <![CDATA[ ## Remediation Suggestions 1. Derive the state filename structurally with `pathlib` instead of using string replacement: ```python from pathlib import Path portfolio_path = Path(filepath).expanduser().resolve() prev_file = portfolio_path.with_name( portfolio_path.stem + "_prev_decisions.json" ) ``` 2. Explicitly verify that the state path differs from the portfolio path before writing: ```python if prev_file.resolve() == portfolio_path.resolve(): raise ValueError("Decision-state path must differ from portfolio path") ``` 3. Require or normalize an approved portfolio filename suffix rather than relying on a case-sensitive substring. 4. Store decision state in a dedicated application-state directory instead of beside an arbitrary user-supplied portfolio file. 5. Reject symbolic-link destinations or open files using no-follow semantics where supported. 6. Write atomically: - Create a temporary file in the destination directory. - Set restrictive permissions. - Flush and synchronize the file. - Replace the destination using `os.replace()`. 7. Validate that the destination directory is expected and user-owned before creating directories or files. 8. Preserve a backup of existing state and report persistence failures rather than silently suppressing all write errors. ]]>

SkillSpector

Vulnerability Patterns

• Prompt Injection: Instruction Override, Hidden Instructions, Exfiltration Commands
• Excessive Agency: Unrestricted Tool Access, Autonomous Decision Making, Scope Creep
• Trigger Abuse: Overly Broad Trigger, Shadow Command Trigger, Keyword Baiting Trigger
• MCP Least Privilege: Underdeclared Capability, Wildcard Permission, Missing Permission Declaration
• MCP Tool Poisoning: Hidden Instructions, Unicode Deception, Parameter Description Injection

Findings (31)

Tp4

High

• Category: MCP Tool Poisoning
• Confidence: 95% confidence
• Finding: The declared description emphasizes a standalone 'pure volume-price momentum' holding advisor with specific scoring/pattern logic. The supplied code does not implement that scoring engine; instead it is an overlay module that revises an existing decision using four extra checks. Two of those checks rely on undeclared non-price-volume inputs: fundamentals.json for logic status and events.json for event windows. While some parts still relate to trading decision support and K-line pattern assessment, the primary behavior is materially different because it performs final decision overrides based on fundamentals and macro-event context, contradicting the 'pure量价' claim.

Tp4

High

• Category: MCP Tool Poisoning
• Confidence: 98% confidence
• Finding: 整体上，代码确实属于股票量价动量/形态分析与持仓决策工具，主方向与描述大体一致，不存在越权、外部资源访问或无关触发行为。但描述声称的若干关键能力在代码中缺失或不一致，已达到“描述与行为不完全匹配”的程度。尤其是“资金流向维度”完全未见实现，代码仅使用OHLCV和可选turnover；“减仓比例建议”未输出任何仓位比例；“108分制评分”也不存在，只有各形态的score_base、预警severity、反弹quality等分散评分；“M0横盘兜底”未实现；预警形态数量与描述不精确对应。虽然代码还实现了M12、M13、B2修复、M10豁免、今日放量反弹覆盖和有限市场自适应，这些与描述相符，但上述缺失属于描述中的重要产品能力，因此应判定为存在不匹配。

Tp4

High

• Category: MCP Tool Poisoning
• Confidence: 96% confidence
• Finding: 核心方向上，代码确实是在做持仓动量评分、输出持有/减仓/卖出/观望/加仓建议，也实现了108分制、资金流向、市场自适应、滞回和减仓比例建议，因此与声明有较强重合。但仍存在实质性不一致。最重要的是，声明明确写“纯量价动量评估”，而代码在最终决策阶段引入 fundamental 和 events，并通过 logic_overlay 对结果做覆盖，这已经超出纯量价范围，属于未声明能力。其次，声明的版本与具体功能列表是 V1.3.8，而代码主体多处标注 V1.3.5，且还混入 V1.4.0 逻辑覆盖层，说明描述与实际实现版本和功能边界并不一致。再次，声明强调了具体的形态数量与若干特定修复/覆盖项，但本代码片段并未完整实现这些检测，更像是接收外部 detect_result 后进行评分与决策。因此应判定为描述与代码行为存在不匹配。

Tp4

High

• Category: MCP Tool Poisoning
• Confidence: 99% confidence
• Finding: The declared description presents an analytical trading advisor focused on momentum assessment and decision recommendations. The supplied code does not implement any such analysis. Instead, it only manages a local holdings JSON file and provides CRUD-like position operations plus a simple profit/loss calculation. This is a materially different primary purpose. Additionally, the code performs local filesystem read/write operations that are not reflected in the declared permissions. Therefore the description does not accurately represent the code's actual behavior.

Tp4

High

• Category: MCP Tool Poisoning
• Confidence: 96% confidence
• Finding: 声明描述的是一个完整的动量持仓决策顾问，重点在量价动量分析、评分体系和交易动作建议；而实际代码只是一个独立的风险过滤脚本，用于标记特定证券风险状态。虽然风险过滤可能作为投资决策系统的辅助组成部分，但当前代码块没有体现声明中的核心能力，主功能明显不同，因此构成描述与行为不匹配。

Tp4

High

• Category: MCP Tool Poisoning
• Confidence: 98% confidence
• Finding: 声明描述的是单标的动量持仓顾问，重点在量价动量评分、持仓建议、反弹形态、滞回和资金流等维度；而代码并未实现这些动量评估逻辑，也没有108分制规则、M12/M13/M0/M10/B2等模式判断。相反，代码的主功能是对一组诊断结果做板块层面的共振风控，依据同板块多只股票的卖出信号，对其他股票追加-10分并可能改变决策。这属于 materially different primary purpose，而非单纯的辅助实现细节。因此描述与代码行为明显不一致。

Missing User Warnings

High

• Confidence: 99% confidence
• Finding: This code can convert a prior hold or reduce outcome into a sell purely by applying a sector-wide score penalty, including setting `sector_forced_sell = True`, with no built-in confirmation, audit gate, or user-facing warning at the decision point. In the context of an investment advisory skill, silently forcing sell recommendations based on correlated peer signals increases the risk of inappropriate liquidation, cascading losses, or misleading users about why a recommendation changed.

Lp3

Medium

• Category: MCP Least Privilege
• Confidence: 70% confidence
• Finding: Without declared permissions the skill's intent is opaque and cannot be validated.

Vague Triggers

Medium

• Confidence: 91% confidence
• Finding: The manifest defines trigger phrases that are broad and conversational, such as generic requests about holding, selling, reducing positions, and trend tracking. In an agent platform, overly broad triggers can cause unintended activation in adjacent finance conversations, which may lead users to receive unrequested trading guidance or expose portfolio context to the skill unnecessarily.

Natural-Language Policy Violations

Medium

• Confidence: 96% confidence
• Finding: The entire skill reference is written in Chinese, including headings, parameter descriptions, and rule explanations, with no indication that users may choose another language or that the locale restriction is intentional. Under the stated policy, forcing a specific language without opt-in is a natural-language policy violation.

Natural-Language Policy Violations

Medium

• Confidence: 93% confidence
• Finding: This is a natural-language policy concern because the file forces a specific language/locale for usage instructions and output-facing descriptions. The policy allows locale constraints only when documented and justified or when the user is given a choice, neither of which appears here.

Description-Behavior Mismatch

Medium

• Confidence: 95% confidence
• Finding: The manifest description focuses on evaluating whether a holding should be kept, reduced, sold, or added to based on momentum. However, `diagnose_scan` implements a broad market screener that fetches all stocks, batch-downloads market data, ranks candidates, and outputs Top N momentum stocks, which goes beyond advising on current holdings.

Natural-Language Policy Violations

Medium

• Confidence: 91% confidence
• Finding: This code presents its purpose and generated reasoning/messages entirely in Chinese, and later functions also return Chinese user-facing text. The file does not offer localization, user opt-in, or any documented reason that the skill must be Chinese-only, which can violate language/locale policy requirements.

Context-Inappropriate Capability

Medium

• Confidence: 95% confidence
• Finding: The code prepends a hard-coded path under the user's home directory to sys.path and then imports executable Python modules from another local skill. This creates a trust-boundary violation: any tampered or replaced files in ~/.workbuddy/skills/volume-price-screener/scripts will run in this skill's context, enabling unintended code execution and supply-chain style compromise.

Natural-Language Policy Violations

Medium

• Confidence: 98% confidence
• Finding: The module docstring and operational descriptions are entirely in Chinese, and there is no indication that the skill offers an alternate language, user opt-in, or a documented region-specific requirement. Under the policy rule, forcing a specific language/locale without user choice is a natural-language policy violation.

Description-Behavior Mismatch

Medium

• Confidence: 95% confidence
• Finding: The module docstring describes a '动量形态识别引擎' with '12种形态：6持有 + 5预警 + 4卖出' and says each pattern is independently detected. But the file also implements 4 buy/add-position signals (B1-B4), 3 rebound-speculation signals (R1-R3), a pre-break warning, and a top-level decision engine that outputs actionable recommendations like hold/reduce/sell in detect_all at L1407-L1545. That is broader than mere detection of 12 momentum patterns.

Intent-Code Divergence

Medium

• Confidence: 98% confidence
• Finding: The header explicitly documents '12种形态：6持有 + 5预警 + 4卖出', which is internally inconsistent even arithmetically and does not match the actual code. The implementation includes 8 hold detectors (M1-M6, M12, M13), multiple warnings including W1-W4, M7, and Pre-M11, 4 sell detectors, 4 buy detectors, and 3 rebound detectors. This is an active contradiction between documentation and behavior, not just missing detail.

Natural-Language Policy Violations

Medium

• Confidence: 93% confidence
• Finding: This Python file’s top-level docstring and extensive inline descriptions are written exclusively in Chinese, and there is no indication anywhere in the file that users can choose another language or locale. Under the stated policy, forcing a specific language without opt-in is a natural-language policy violation even when it appears in code comments or string literals.

Natural-Language Policy Violations

Medium

• Confidence: 94% confidence
• Finding: This Python file contains user-facing natural-language strings and docstrings predominantly in Chinese, and later functions format output using fixed Chinese labels and advice text. Under the policy, forcing a specific language without user opt-in is a natural-language locale violation unless the locale restriction is explicitly justified.

Description-Behavior Mismatch

Medium

• Confidence: 96% confidence
• Finding: The manifest describes the skill as doing ‘纯量价动量评估’ to decide hold/reduce/sell, but calculate_score also incorporates cost basis, fund-flow data, market regime, fundamental data, and event inputs into the final decision path. In particular, the logic overlay can override the pure momentum decision with non-price/volume considerations, which is broader than the claimed scope.

Natural-Language Policy Violations

Medium

• Confidence: 97% confidence
• Finding: The formatting functions generate end-user output with fixed labels and warnings such as Chinese advice text, Chinese risk notes, and mixed English headers. Because the skill does not offer a locale selection or state that it is region-specific, this constitutes a language-policy issue rather than a code-quality preference.

Description-Behavior Mismatch

Medium

• Confidence: 95% confidence
• Finding: This module does more than passive momentum analysis: it creates, updates, and deletes a local positions file. That expands the skill from advisory behavior into state-changing portfolio management, which can alter user data unexpectedly if the surrounding agent invokes these functions without explicit authorization. In the context of a read-only holding advisor, this capability is more dangerous because it exceeds the declared purpose and increases the chance of silent data tampering or loss.

Missing User Warnings

Medium

• Confidence: 88% confidence
• Finding: The module exposes save and delete operations with no built-in confirmation, warning, backup, or soft-delete behavior. If called by an agent workflow or UI layer without additional safeguards, user holdings can be overwritten or removed silently, causing integrity loss and operational harm. In a financial-assistance skill, silent mutation of portfolio records is particularly sensitive because users may rely on those records for decisions.

Natural-Language Policy Violations

Medium

• Confidence: 97% confidence
• Finding: This Python file contains all user-facing natural-language descriptions exclusively in Chinese, including the module header and multiple docstrings. Under the stated policy, forcing a specific language without user opt-in or a documented region-specific justification is a natural-language policy violation.

Missing User Warnings

Medium

• Confidence: 95% confidence
• Finding: The module’s stated purpose is to automatically downgrade or reduce positions for other holdings when multiple stocks in the same sector trigger sell signals. In a trading decision skill, hidden automatic portfolio-action logic without explicit user acknowledgment can cause users to act on materially changed recommendations without realizing that a sector-level heuristic overrode the original per-symbol analysis.

Static analysis

No suspicious patterns detected.