OPC Journal

• Outcome: Review
• Latest audit: Sep 16, 2026 · 5:55 AM UTC
• Version: 2.5.2

Overview

This local journaling skill is mostly disclosed and purpose-aligned, but its export command can write outside the promised customer folder and overwrite arbitrary user-writable files.

Review before installing. The skill is local-only and I found no network calls or credential handling, but do not allow untrusted prompts to choose export paths. Keep exports inside the customer folder, avoid using partial entry IDs for deletion, and expect journal/task data to persist under ~/.openclaw/customers/{customer_id}/ until archived or cleared.

A.I.G

Vulnerability Patterns

• 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
• Embedded Malicious Code: Ships malicious scripts inside the skill and executes them locally

Findings (4)

T09 · Insecure Skill Coding Practices

Error

• Location: scripts/commands/export.py:63
• Finding: Arbitrary Local-File Overwrite Through the Export Command## Vulnerability Details **File Location**: `scripts/commands/export.py:63-72` **Vulnerability Type**: Unrestricted file write and unsafe overwrite **Risk Level**: High ### Vulnerable Code ```python # Write to file if output_path not provided, use default if not output_path: output_path = os.path.join(base, f"export_{customer_id}{ext}") else: output_path = os.path.expanduser(output_path) try: Path(output_path).write_text(exported_content, encoding="utf-8") except Exception as e: ``` ### Technical Analysis The user-controlled `output_path` is expanded and passed directly to `Path.write_text()`. The implementation does not: - Restrict the destination to the customer's journal or export directory. - Canonicalize the path and verify its parent directory. - Reject symbolic links. - Check whether the destination already exists. - Require confirmation before truncating an existing file. `Path.write_text()` opens an existing destination for truncating writes. It also follows symbolic links. Consequently, an invocation can replace any file writable by the account running the Skill. This behavior contradicts the documented security model in `SKILL.md`, which states that all I/O is constrained to `~/.openclaw/customers/{customer_id}/`. ### Attack Path 1. Ensure the selected customer has at least one journal entry so that export reaches the file-writing branch. 2. Invoke the export command with `--output-path` set to an existing file writable by the Skill process. 3. Alternatively, select a path that is a symbolic link to another writable file. 4. The command calls `Path(output_path).write_text(...)`. 5. The destination file is truncated and replaced with the generated journal export. ### Impact Assessment The attacker can overwrite files with the permissions of the account running the Skill. The scope includes user documents, application configuration, shell configuration, local state fil ...[truncated 197 chars]
• Remediation: ## Remediation Suggestions - Store all exports in a dedicated directory beneath the sanitized customer directory. - Resolve both the allowed export directory and requested destination with `Path.resolve()`, then verify that the destination is a descendant of the allowed directory. - Reject destinations whose path or existing components are symbolic links. - Use exclusive creation by default, such as mode `"x"`, to prevent silent replacement. - Require a separate explicit overwrite flag when replacing an existing export. - Write through a same-directory temporary file, flush and `fsync()` it, and use `os.replace()` only after all validation succeeds. - Do not disclose unrestricted host filesystem paths as a supported export feature unless such access is explicitly required and authorized.

T09 · Insecure Skill Coding Practices

Warning

• Location: scripts/commands/delete.py:12
• Finding: Partial Entry IDs Can Cause Unintended Bulk Deletion## Vulnerability Details **File Location**: `scripts/commands/delete.py:12-30, 47-65` **Vulnerability Type**: Improper identifier validation and substring-based deletion **Risk Level**: Medium ### Vulnerable Code ```python def _find_entry_file(base: str, entry_id: str): memory_dir = os.path.join(base, "memory") if not os.path.exists(memory_dir): return None, None for f in sorted(glob.glob(os.path.join(memory_dir, "*.md"))): content = Path(f).read_text(encoding="utf-8") if f"entry_id: {entry_id}" in content: return f, content return None, None def _remove_entry(content: str, entry_id: str) -> str: """Remove an entry block by entry_id, preserving separators for remaining blocks.""" blocks = split_entries(content) kept = [] for block in blocks: if f"entry_id: {entry_id}" in block: continue kept.append(block) return join_entries(kept) ``` ```python base = os.path.expanduser(build_customer_dir(customer_id)) file_path, content = _find_entry_file(base, entry_id) if file_path is None: return {"status": "error", "result": None, "message": f"Entry {entry_id} not found"} new_content = _remove_entry(content, entry_id) # Backup before mutation if os.path.exists(file_path) and os.path.getsize(file_path) > 0: shutil.copy2(file_path, file_path + ".bak") if not new_content.strip(): os.remove(file_path) else: Path(file_path).write_text(new_content, encoding="utf-8") meta = read_meta(customer_id) if meta: meta["total_entries"] = max(0, meta.get("total_entries", 0) - 1) ``` ### Technical Analysis The deletion command does not validate that the supplied identifier follows the complete generated ID format. It searches for the text `entry_id: {entry_id}` as a substring anywhere in a file and then removes every block containing the same substring. Generated identifiers ...[truncated 1473 chars]
• Remediation: ## Remediation Suggestions - Validate identifiers against the complete expected format, for example `^JE-[0-9]{8}-[A-F0-9]{6}$`. - Parse each block with the shared entry parser and compare the parsed field using exact equality: `parsed["entry_id"] == entry_id`. - Stop after exactly one matching entry has been removed. - Treat duplicate exact identifiers as a data-integrity error rather than deleting all duplicates. - Calculate the actual number of removed entries and update metadata accordingly. - Perform the journal mutation and metadata update as one coordinated transaction where practical. - Add regression tests for empty IDs, prefixes, suffixes, embedded newlines, duplicate IDs, and multiple entries in one file.

T09 · Insecure Skill Coding Practices

Warning

• Location: scripts/commands/_meta.py:43
• Finding: Metadata File Is Truncated Before the Exclusive Lock Is Acquired## Vulnerability Details **File Location**: `scripts/commands/_meta.py:43-62` **Vulnerability Type**: Incorrect file-lock ordering and non-atomic persistence **Risk Level**: Medium ### Vulnerable Code ```python def write_meta(customer_id: str, data: dict) -> bool: try: path = os.path.expanduser(Path(build_customer_dir(customer_id)) / "journal_meta.json") os.makedirs(os.path.dirname(path), exist_ok=True) # Ensure meta has timezone-aware timestamps if "last_updated" not in data: data["last_updated"] = now_tz().isoformat() with open(path, "w", encoding="utf-8") as f: fcntl.flock(f.fileno(), fcntl.LOCK_EX) try: json.dump(data, f, indent=2, ensure_ascii=False) f.flush() os.fsync(f.fileno()) finally: fcntl.flock(f.fileno(), fcntl.LOCK_UN) return True except Exception: return False ``` ### Technical Analysis Opening a file with mode `"w"` truncates it immediately. The exclusive `flock()` is only requested after this truncation has occurred. As a result, the lock does not protect the file from the most destructive part of opening it. Concurrent metadata operations also perform their reads and writes under separate locks. A command may read an earlier state, release the read lock, modify its local copy, and later overwrite changes made by another command. The broad exception handling compounds the problem by converting malformed or empty metadata into an empty dictionary without exposing the underlying corruption to callers. ### Attack Path 1. Run two commands that update metadata for the same customer concurrently, such as simultaneous record operations. 2. Both commands read the same original metadata state. 3. One process opens `journal_meta.json` with `"w"` and truncates it before obtaining the exclusive lock. 4. A conc ...[truncated 683 chars]
• Remediation: ## Remediation Suggestions - Use a separate, stable lock file and acquire its exclusive lock before opening or replacing the metadata file. - Hold the exclusive lock across the complete read-modify-write transaction. - Serialize metadata to a temporary file in the same directory. - Flush and `fsync()` the temporary file, then atomically replace the destination using `os.replace()`. - Where durability is required, `fsync()` the parent directory after replacement. - Apply restrictive file permissions appropriate for private journal data. - Return explicit corruption and persistence errors instead of silently treating all exceptions as missing metadata. - Add concurrency tests that run simultaneous record, archive, initialization, and metadata-update commands.

T09 · Insecure Skill Coding Practices

Warning

• Location: utils/task_storage.py:42
• Finding: Task Persistence Allows Lost Updates and Pre-Lock File Truncation## Vulnerability Details **File Location**: `utils/task_storage.py:42-69` **Vulnerability Type**: Non-atomic read-modify-write and incorrect locking **Risk Level**: Medium ### Vulnerable Code ```python def write_tasks(customer_id: str, tasks: List[Dict[str, Any]]) -> bool: """Write tasks with exclusive file locking.""" path = _get_tasks_path(customer_id) try: os.makedirs(os.path.dirname(path), exist_ok=True) with open(path, "w", encoding="utf-8") as f: # Acquire exclusive lock for writing fcntl.flock(f.fileno(), fcntl.LOCK_EX) try: json.dump(tasks, f, indent=2, ensure_ascii=False) f.flush() os.fsync(f.fileno()) finally: fcntl.flock(f.fileno(), fcntl.LOCK_UN) return True except Exception: return False def add_task(customer_id: str, task: Dict[str, Any]) -> bool: """Add a single task to storage.""" tasks = read_tasks(customer_id) tasks.append(task) return write_tasks(customer_id, tasks) def add_tasks(customer_id: str, new_tasks: List[Dict[str, Any]]) -> bool: """Add multiple tasks to storage.""" tasks = read_tasks(customer_id) tasks.extend(new_tasks) return write_tasks(customer_id, tasks) ``` ### Technical Analysis The task writer opens `tasks.json` in truncating mode before acquiring its exclusive lock. Therefore, the lock does not prevent another operation from observing or contributing to a truncated file. In addition, task mutations release the shared read lock before acquiring the write lock. The read-modify-write sequence is not atomic. Two concurrent calls can read the same task list, independently append different records, and then write their versions in sequence. The last writer wins, silently discarding the other update. The same pattern affects functions that update or delete tas ...[truncated 1031 chars]
• Remediation: ## Remediation Suggestions - Acquire an exclusive lock on a separate lock file before reading the current task list. - Keep that lock for the entire read-modify-write operation. - Perform all add, update, and delete operations through one transactional helper. - Write the new JSON document to a same-directory temporary file, flush and `fsync()` it, and atomically replace `tasks.json`. - Validate the existing JSON structure and report corruption rather than converting all read errors into an empty list. - Consider SQLite with transactions if concurrent access is expected. - Add stress tests for simultaneous single-task creation, batch creation, updates, and deletions.

SkillSpector

Vulnerability Patterns

• Prompt Injection: Instruction Override, Hidden Instructions, Exfiltration Commands
• Excessive Agency: Unrestricted Tool Access, Autonomous Decision Making, Scope Creep
• Memory Poisoning: Persistent Context Injection, Context Window Stuffing, Memory Manipulation
• Trigger Abuse: Overly Broad Trigger, Shadow Command Trigger, Keyword Baiting Trigger
• Behavioral AST: exec() Call, eval() Call, Dynamic Import

Findings (34)

Tp4

High

• Category: MCP Tool Poisoning
• Confidence: 99% confidence
• Finding: The declared description is for a journal and insight-generation skill, but the supplied code is a task-management utility. It accepts task descriptions, assigns task IDs, sets timeouts and estimated completion times, and persists the tasks through storage. There is no journaling, dream/memory analysis, milestone detection, or insight generation in this code chunk. While the code does not show network access, its primary purpose is materially different from the declared purpose, so this is a clear mismatch.

Tp4

High

• Category: MCP Tool Poisoning
• Confidence: 94% confidence
• Finding: The declared description presents a richer journal-analysis skill that can detect milestones and generate insights locally. The supplied code chunk is much narrower: it only accepts content and day, gets a language value, and returns a milestone candidate payload with an explicit note that classification is left to the caller/LLM. That is a material description-to-behavior mismatch for this chunk because the code does not perform milestone detection or analysis itself. There is no evidence of network access, so the local-only claim is not contradicted.

Tp4

High

• Category: MCP Tool Poisoning
• Confidence: 98% confidence
• Finding: The code does not implement journaling, pattern analysis, milestone detection, or insight generation. Its sole behavior is creating and persisting a task record with status, timestamps, and timeout metadata. While the description says the skill is local-only and this snippet does not show network access, the primary functionality is materially different from the declared journal-oriented purpose. Therefore this is a clear description-behavior mismatch.

Tp4

High

• Category: MCP Tool Poisoning
• Confidence: 92% confidence
• Finding: The code does not match the declared primary functionality. Instead of recording journal entries or analyzing memories/dreams for patterns and milestones, it reads and writes local metadata and may modify existing markdown files under a memory directory to translate template text after a language change. This is a materially different purpose: configuration/maintenance versus journaling analytics. The LOCAL-ONLY claim is consistent because the code uses only local filesystem operations and no network calls. There are no evident extra permissions beyond local file access, but the main behavior shown is not accurately represented by the declared description.

Tp4

High

• Category: MCP Tool Poisoning
• Confidence: 98% confidence
• Finding: The code is local-only and does not make network calls, which is consistent with the description's locality constraint. However, its actual function is a task storage layer for persisting task objects in JSON files with file locking. This is materially different from the declared journal-oriented purpose involving recording entries and analyzing patterns, milestones, and insights. The code neither processes journal content nor performs any analysis; instead it exposes CRUD operations over tasks. That constitutes a clear description-behavior mismatch.

Description-Behavior Mismatch

High

• Confidence: 96% confidence
• Finding: This file implements batch async task creation and persistence, which is materially different from the stated journal-only purpose of the skill. Capability drift like this is dangerous because it creates a hidden task orchestration surface that could be abused to store arbitrary queued work, bypass user expectations, and expand the effective privileges and attack surface of an otherwise low-risk local journaling tool.

Memory Manipulation

High

• Category: Memory Poisoning

Content

meta = read_meta(customer_id)
    if meta is not None and meta.get("total_entries", 0) > 0:
        return False
    # Fallback: scan files if meta is missing or corrupt
    memory_dir = os.path.expanduser(Path(build_customer_dir(customer_id)) / "memory")
    if not os.path.exists(memory_dir):
        return True

• Confidence: 90% confidence
• Finding: Skill manipulates agent memory, state, or stored context. Memory corruption can alter personality, override safety rules, or cause unpredictable behavior.

Memory Manipulation

High

• Category: Memory Poisoning

Content

# archive
    p = subparsers.add_parser("archive", help="Archive journal data")
    p.add_argument("--customer-id", default="OPC-001")
    p.add_argument("--clear", action="store_true", help="Clear memory files after archiving")
    p.add_argument("--force", action="store_true", help="Confirm destructive clear")

    # update-meta

• Confidence: 80% confidence
• Finding: Skill manipulates agent memory, state, or stored context. Memory corruption can alter personality, override safety rules, or cause unpredictable behavior.

Lp3

Medium

• Category: MCP Least Privilege
• Confidence: 88% confidence
• Finding: The skill declares command-dispatch to a tool and documents local file read/write behavior, but it does not explicitly declare a restrictive tool scope such as permissions or allowed-tools. This creates an authorization ambiguity: a host may expose broader tool capabilities than intended, and reviewers cannot verify from metadata that file access is intentionally constrained.

Natural-Language Policy Violations

Medium

• Confidence: 90% confidence
• Finding: The phrase 'templates are now English-only by design' is a natural-language locale policy constraint. Although the skill has a language update command, this line indicates a forced English-only design choice without documenting user opt-in or a clear region/compliance justification.

Natural-Language Policy Violations

Medium

• Confidence: 92% confidence
• Finding: The code collapses language selection to "zh" when stored metadata starts with zh and otherwise defaults to "en", and detection likewise returns only zh or en based on presence of Chinese characters. This hard-coded locale behavior can override user preference or exclude other languages without any documented opt-in, which matches the language-policy violation criteria.

Intent-Code Divergence

Medium

• Confidence: 97% confidence
• Finding: The comment at L070 says the analysis is limited to language-agnostic structural signals and explicitly says there are no hardcoded semantic labels or interpretations. However, L082-L084 extract fragments using specific English keyword lists grouped into semantic categories like action, achievement, and obstacle, which are hardcoded interpretations.

Natural-Language Policy Violations

Medium

• Confidence: 93% confidence
• Finding: This code includes natural-language guidance stating the fragment extraction "Works best with English text," which signals a language-specific limitation while the skill also returns a customer language value elsewhere. Because the file does not show any user opt-in, fallback, or explicit limitation handling for non-English users, this is a locale/language policy concern.

Context-Inappropriate Capability

Medium

• Confidence: 91% confidence
• Finding: The run function exposes generalized asynchronous task orchestration under a journal-branded skill, allowing arbitrary descriptions, task typing, and persisted records. In context, this mismatch increases risk because users and reviewers may trust the package as a simple local journal utility while it quietly provides a reusable job queue primitive that can support unintended automation or data accumulation.

Missing User Warnings

Medium

• Confidence: 89% confidence
• Finding: The export command writes full journal contents to a caller-supplied path after only applying expanduser, with no restriction that the destination remain inside the customer's local workspace and no explicit disclosure at this code point that sensitive data may be written anywhere on the filesystem. In a journal skill handling highly personal memory and dream data, this increases the risk of accidental or unauthorized placement of sensitive exports into shared, synced, or security-relevant locations if an upstream caller or prompt controls output_path.

Natural-Language Policy Violations

Medium

• Confidence: 93% confidence
• Finding: The code assigns a default timezone of "Asia/Shanghai" when no user preferences are provided. This imposes a specific locale choice by default rather than offering neutrality or explicit user opt-in, which matches the language/locale policy concern for all file types.

Missing User Warnings

Medium

• Confidence: 93% confidence
• Finding: The command returns full `raw_text` from dreams and memory entries directly to the caller, which can expose highly sensitive personal journal content without any minimization, redaction, or explicit consent gate in this code path. In a journaling skill, this is especially risky because the data is inherently intimate and may include mental health, financial, or relationship details that downstream components or logs could inadvertently retain or disclose.

Natural-Language Policy Violations

Medium

• Confidence: 81% confidence
• Finding: The code reads a language value via get_language(customer_id) and persists it into the journal entry metadata as `language: {lang}`. If that setting forces a specific language/locale for entries without an explicit user-facing choice in this file, it may violate the policy against imposing a language or locale without opt-in.

Dynamic import via __import__()

Medium

• Category: Dangerous Code Execution

Content

def _parse_md_date(basename: str) -> str:
    m = __import__("re").search(r"(\d{2}-\d{2}-\d{2})\.md$", basename)
    if m:
        return m.group(1)
    return "00-00-00"

• Confidence: 75% confidence
• Finding: Dynamic __import__() can load arbitrary modules at runtime, bypassing static analysis and potentially importing malicious code.

Description-Behavior Mismatch

Medium

• Confidence: 91% confidence
• Finding: The manifest describes a local journal skill for recording entries, analyzing patterns, detecting milestones, and generating insights. This file instead implements creation and persistence of asynchronous task records with task IDs, timeouts, and estimated completion tracking, which is a separate task-management workflow not reflected in the manifest description.

Natural-Language Policy Violations

Medium

• Confidence: 92% confidence
• Finding: The comment states that the skill now generates English-only templates, and the translation rule tables are empty, indicating a forced language policy rather than a user-selectable locale. This conflicts with the policy against forcing a specific language without explicit user opt-in or documented justification.

Missing User Warnings

Medium

• Confidence: 85% confidence
• Finding: The command writes updated metadata and may retroactively rewrite all matching markdown files in the customer's memory directory when the language changes. Although the return message reports changes after the fact, there is no confirmation prompt, pre-action warning, or inline disclosure before these file modifications occur.

Description-Behavior Mismatch

Medium

• Confidence: 90% confidence
• Finding: The manifest describes a local journal skill for recording entries, analyzing patterns, detecting milestones, and generating insights. However, the documented command set also includes `task` and `batch-task`, which create async tasks and are not an obvious or declared part of journaling or growth tracking functionality.

Description-Behavior Mismatch

Medium

• Confidence: 92% confidence
• Finding: The code defines CLI arguments for `task` and `batch-task`, including task type, descriptions, and timeouts, indicating operational support for creating standalone async tasks. This behavior goes beyond the manifest's stated scope of local journal recording, analysis, milestone detection, and insight generation.

Description-Behavior Mismatch

Medium

• Confidence: 95% confidence
• Finding: The manifest describes a local-only journal skill for recording entries, analyzing dreams/memory, detecting milestones, and generating insights. This module implements persistent CRUD operations for generic customer tasks in a separate tasks.json store, which is a materially different capability than journal entry handling and is not suggested by the stated skill purpose.

Static analysis

No suspicious patterns detected.