T09 · Insecure Skill Coding Practices
Warning
- Location
review.py:20- Finding
Incorrect Default Path Resolution Can Overwrite an Unrelated Review Log
- Content
View full analysis
Vulnerability Details
File Location:
review.py, lines 20–22, with the resulting write at lines 237–254
Vulnerability Type: Incorrect path fallback and unintended file overwrite
Risk Level: MediumVulnerable Code
python SKILL_DIR = Path(__file__).parent WORKSPACE_DIR = SKILL_DIR.parent.parent # .../skills/vocabulary-anti-forgetting/ -> .../workspace-root/ MEMORY_DIR = Path(os.environ.get("REVIEW_MEMORY_DIR", "")) or WORKSPACE_DIR / "memory" LOG_PATH = MEMORY_DIR / "review_log.md"The resolved path is later created and written without validating that it is the intended memory directory:
python def ensure_log_exists(): MEMORY_DIR.mkdir(parents=True, exist_ok=True) if not LOG_PATH.exists(): LOG_PATH.write_text(INITIAL_LOG, encoding="utf-8")python def write_review_log(log: ReviewLog): lines = [ "# Vocabulary Review Log\n", "\n", f"> **Total sessions:** {log.total_sessions}\n", f"> **Last session date:** {log.last_session_date}\n", "\n", "| id | vocabulary | level | review_count | last_reviewed | next_review_date |\n", "|---|---|---|---|---|---|\n", ] for entry in sorted(log.entries.values(), key=lambda e: e.id): last = entry.last_reviewed.isoformat() if entry.last_reviewed else "—" nxt = entry.next_review_date.isoformat() if entry.next_review_date else "—" lines.append( f"| {entry.id} | {entry.vocabulary} | {entry.level} " f"| {entry.review_count} | {last} | {nxt} |\n" ) LOG_PATH.write_text("".join(lines), encoding="utf-8")Technical Analysis
The expression intended to select a default memory directory is incorrect:
python Path(os.environ.get("REVIEW_MEMORY_DIR", "")) or WORKSPACE_DIR / "memory"When
REVIEW_MEMORY_DIRis absent,os.environ.get()returns an empty string. Howe ...[truncated 2186 chars]- Remediation
View remediation
Remediation Suggestions
Test the environment-variable string before constructing a
Path:python memory_dir_value = os.environ.get("REVIEW_MEMORY_DIR") MEMORY_DIR = ( Path(memory_dir_value).expanduser() if memory_dir_value else WORKSPACE_DIR / "memory" ) LOG_PATH = MEMORY_DIR / "review_log.md"Apply the following additional hardening measures:
- Resolve and validate the selected directory with
Path.resolve()before writing. - Confirm that the default path remains beneath the expected workspace root.
- If an override is supported, document that it must refer to a trusted directory and reject paths that resolve to a regular file.
- Validate any pre-existing log before replacing it; refuse to overwrite malformed or unrelated content unless the user explicitly confirms.
- Write updates atomically by creating a temporary file in the same directory, flushing it, and replacing the destination with
os.replace(). - Preserve a backup before replacing an existing log where recovery is important.
- Add regression tests verifying that an unset or empty
REVIEW_MEMORY_DIRresolves to<workspace>/memory, while a non-empty override resolves to the explicitly supplied directory.
- Resolve and validate the selected directory with