mirror of
https://github.com/alirezarezvani/claude-skills.git
synced 2026-08-28 04:24:58 +00:00
fix(book-to-skill): guard EPUB XML, cap zip expansion, refuse symlinked trees
Addresses the second automated review on PR #941. All three code findings verified against the actual behaviour, not just patched. [Medium] shutil.copytree dereferenced symlinks the validator never saw. The validator checks SKILL.md, the three supporting files and chapters/*.md; the copy then followed a link anywhere else in the tree (assets/, any subdirectory) and baked the target's real content into a package that can go out as --distribution shareable. _assert_no_symlinks() now walks the whole tree and refuses, and runs BEFORE the validation branch so --skip-validation cannot bypass it. copytree also passes symlinks=True so loosening that check later cannot silently reintroduce dereferencing. Verified: a symlink in assets/ pointing at a secret file is refused both with and without --skip-validation, the secret never lands in a package, and a clean tree still emits. [Medium] The DOCX XXE/entity guard did not extend to EPUB's ebooklib path. Upstream hardened DOCX only. EPUB is the same zip-of-XML shape and ebooklib — one of the packages this skill recommends installing — parsed container.xml, the OPF and content docs with no equivalent pre-check. The guard moved to a new book_to_skill/zip_safety.py and now runs for both formats. Verified: an EPUB whose OPF declares an entity is refused; a clean EPUB still extracts and detects its chapter. [Low] No size cap before decompressing zip members. Every archive read now goes through safe_read(), which checks the declared uncompressed size and the compression ratio against the central directory before decompressing, and charges actual bytes against a per-archive budget so a lying directory cannot get past it either. Verified: a 200 MB / 1029x bomb is refused at ~14 MB peak RSS instead of being materialized. [Low] The PR body's "12 numbered items" was stale against README's list. Fixed in the PR description; the in-repo count is synced to 18 across plugin.json, CLAUDE.md and CHANGELOG. Recorded as deviations 17 and 18. Counters: tools 662 -> 663 (zip_safety.py); that module is allowlisted in smoke_exceptions.txt like its siblings. All gates green: compileall, check_paths --all, check_dual_publish, smoke_scripts (0 failed), derive_counters --check, check_plugin_json --all (0 FAIL). Security auditor unchanged at 0 critical / 4 high. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017zu9Gmm9S78c2t3kDLnpPX
This commit is contained in:
Claude
parent
b5031e976e
commit
9ed709aef3
11 changed files with 231 additions and 37 deletions
|
|
@ -8,7 +8,7 @@
|
|||
"homepage": "https://github.com/alirezarezvani/claude-skills",
|
||||
"repository": "https://github.com/alirezarezvani/claude-skills",
|
||||
"metadata": {
|
||||
"description": "363 production-ready skills across 18 domains (engineering, engineering-core, marketing, product, c-level, compliance-os, project management, RA/QM, business growth, finance, productivity, marketing top-level, research, research-ops, business-operations, commercial, markdown-html, loop-library, plus standards). 662 Python tools, 746 reference guides, 103 agents (cs-* + personas), 118 slash commands across 89 marketplace plugins. v2.11.2 vendors engineering/skillopt-sleep — a verbatim copy of microsoft/SkillOpt's stdlib-only skillopt_sleep engine + Claude Code plugin surface, giving a local agent a nightly gated self-improvement cycle (read-only session harvest -> mine -> offline replay -> held-out-gated CLAUDE.md/SKILL.md edits -> staged for explicit /skillopt-sleep adopt). productivity/fable-goal (unreleased, post-v2.11.1) converts a rambling description of a desired outcome into one polished /goal prompt for a fresh autonomous session. v2.11.1 turns product-team and project-management into agent-harness domains: fork-orchestrators with deterministic goal routers, a Jira MCP snapshot bridge (Kanban flow metrics + Monte Carlo forecasting), a delegation-governance loop gate, a continuous-discovery cadence tracker, and an Opportunity Solution Tree linter, with /cs:pm and /cs:product command families. v2.10.3 completes the markdown-html domain with md-slides — slide-deck converter (arrow-key / Space / PgDn / Home/End / P keyboard navigation + presenter mode with split-view clock + speaker notes + next-slide preview + URL-hash deep linking like #3 for direct slide jumps + @media print page-per-slide for browser-native PDF export). Reuses md-document's markdown parser; vanilla JS only (no framework runtime); Prism.js opt-in via --syntax. Joins md-review (v2.10.2 code-review converter), md-document (v2.10.1 long-form converter), and the v2.10.0 foundation (orchestrator + design-system). Compatible with Claude Code, Codex CLI, Gemini CLI, Cursor, OpenClaw, Hermes Agent, Mistral Vibe, and 5 more coding agents.",
|
||||
"description": "363 production-ready skills across 18 domains (engineering, engineering-core, marketing, product, c-level, compliance-os, project management, RA/QM, business growth, finance, productivity, marketing top-level, research, research-ops, business-operations, commercial, markdown-html, loop-library, plus standards). 663 Python tools, 746 reference guides, 103 agents (cs-* + personas), 118 slash commands across 89 marketplace plugins. v2.11.2 vendors engineering/skillopt-sleep — a verbatim copy of microsoft/SkillOpt's stdlib-only skillopt_sleep engine + Claude Code plugin surface, giving a local agent a nightly gated self-improvement cycle (read-only session harvest -> mine -> offline replay -> held-out-gated CLAUDE.md/SKILL.md edits -> staged for explicit /skillopt-sleep adopt). productivity/fable-goal (unreleased, post-v2.11.1) converts a rambling description of a desired outcome into one polished /goal prompt for a fresh autonomous session. v2.11.1 turns product-team and project-management into agent-harness domains: fork-orchestrators with deterministic goal routers, a Jira MCP snapshot bridge (Kanban flow metrics + Monte Carlo forecasting), a delegation-governance loop gate, a continuous-discovery cadence tracker, and an Opportunity Solution Tree linter, with /cs:pm and /cs:product command families. v2.10.3 completes the markdown-html domain with md-slides — slide-deck converter (arrow-key / Space / PgDn / Home/End / P keyboard navigation + presenter mode with split-view clock + speaker notes + next-slide preview + URL-hash deep linking like #3 for direct slide jumps + @media print page-per-slide for browser-native PDF export). Reuses md-document's markdown parser; vanilla JS only (no framework runtime); Prism.js opt-in via --syntax. Joins md-review (v2.10.2 code-review converter), md-document (v2.10.1 long-form converter), and the v2.10.0 foundation (orchestrator + design-system). Compatible with Claude Code, Codex CLI, Gemini CLI, Cursor, OpenClaw, Hermes Agent, Mistral Vibe, and 5 more coding agents.",
|
||||
"version": "2.11.2"
|
||||
},
|
||||
"plugins": [
|
||||
|
|
|
|||
14
CHANGELOG.md
14
CHANGELOG.md
|
|
@ -22,7 +22,7 @@ parsers) is vendored close to verbatim and keeps upstream's format chains, chapt
|
|||
detection across Latin/Roman/Chinese/Thai/Korean heading styles, invisible-Unicode
|
||||
(Trojan Source) sanitization, and the DOCX entity-expansion guard.
|
||||
|
||||
**16 numbered deviations** are recorded in `engineering/book-to-skill/README.md`,
|
||||
**18 numbered deviations** are recorded in `engineering/book-to-skill/README.md`,
|
||||
which is the authoritative list. Highlights:
|
||||
|
||||
- **(5) No implicit installs.** `--install-missing` defaults to `report` — it prints
|
||||
|
|
@ -48,6 +48,16 @@ which is the authoritative list. Highlights:
|
|||
`--workdir` is symlink-refused and mode-restricted. Also fixes a real bug: `parsers/calibre.py`
|
||||
read a module-level path constant and so ignored `--workdir` entirely.
|
||||
|
||||
- **(17) Zip-of-XML hardening generalized, plus decompression-bomb caps.** Upstream's
|
||||
DTD/entity guard covered DOCX only; EPUB's `ebooklib` path handed the archive straight to a
|
||||
third-party XML stack. The guard now lives in `book_to_skill/zip_safety.py` and runs for
|
||||
both, and every archive read checks declared size and compression ratio before
|
||||
decompressing — a 200 MB zip bomb is refused at ~14 MB peak RSS.
|
||||
- **(18) Packaging refuses a source tree containing symlinks.** `shutil.copytree` defaults to
|
||||
following links, which would bake a link target's real content into a package that may be
|
||||
emitted as `--distribution shareable`. `_assert_no_symlinks()` walks the whole tree and
|
||||
refuses, before the validation branch so `--skip-validation` cannot bypass it.
|
||||
|
||||
**Repo-native addition with no upstream counterpart — Step 11 / `/cs:book-to-plugin`.**
|
||||
Upstream stops at a bare folder in `~/.claude/skills/`, which this library cannot route
|
||||
to. `skill_plugin_emitter.py` wraps a compiled skill as a full plugin package (manifest
|
||||
|
|
@ -73,7 +83,7 @@ Regenerated `engineering/agent-harness/.../assets/harnesses/engineering.json`. P
|
|||
|
||||
### Changed — counters
|
||||
|
||||
skills 362 → 363, tools 644 → 662, refs 741 → 746, agents 102 → 103, commands
|
||||
skills 362 → 363, tools 644 → 663, refs 741 → 746, agents 102 → 103, commands
|
||||
116 → 118, plugins 88 → 89 (derived via `scripts/derive_counters.py --check`).
|
||||
|
||||
---
|
||||
|
|
|
|||
File diff suppressed because one or more lines are too long
|
|
@ -26,10 +26,10 @@ The most comprehensive open-source library of Claude Code skills and agent plugi
|
|||
Claude Code skills (also called agent skills or coding agent plugins) are modular instruction packages that give AI coding agents domain expertise they don't have out of the box. Each skill includes:
|
||||
|
||||
- **SKILL.md** — structured instructions, workflows, and decision frameworks
|
||||
- **Python tools** — 662 CLI scripts (all stdlib-only, zero pip installs)
|
||||
- **Python tools** — 663 CLI scripts (all stdlib-only, zero pip installs)
|
||||
- **Reference docs** — 746 templates, checklists, and domain-specific knowledge files
|
||||
|
||||
**One repo, thirteen platforms.** Works natively as Claude Code plugins, Codex agent skills, Gemini CLI skills, Hermes Agent skills, Mistral Vibe skills, and converts to more tools via `scripts/convert.sh`. All 662 Python tools run anywhere Python runs.
|
||||
**One repo, thirteen platforms.** Works natively as Claude Code plugins, Codex agent skills, Gemini CLI skills, Hermes Agent skills, Mistral Vibe skills, and converts to more tools via `scripts/convert.sh`. All 663 Python tools run anywhere Python runs.
|
||||
|
||||
### Skills vs Agents vs Personas
|
||||
|
||||
|
|
@ -354,7 +354,7 @@ Yes. Skills work natively with 13 tools: Claude Code, OpenAI Codex, Gemini CLI,
|
|||
No. We follow semantic versioning and maintain backward compatibility within patch releases. Existing script arguments, plugin source paths, and SKILL.md structures are never changed in patch versions. See the [CHANGELOG](CHANGELOG.md) for details on each release.
|
||||
|
||||
**Are the Python tools dependency-free?**
|
||||
Yes. All 662 Python tools use the standard library only — zero pip installs required. Every skill's CLI entry point is verified to run with `--help` (most skills ship one script per tool; a few, like the vendored `engineering/skillopt-sleep` engine, ship a multi-module package behind a single `python -m` entry point). A few tools — `engineering/book-to-skill`'s document extractors — can *optionally* use third-party parsers for higher-fidelity output, but every format falls back to a standard-library parser and nothing is installed implicitly.
|
||||
Yes. All 663 Python tools use the standard library only — zero pip installs required. Every skill's CLI entry point is verified to run with `--help` (most skills ship one script per tool; a few, like the vendored `engineering/skillopt-sleep` engine, ship a multi-module package behind a single `python -m` entry point). A few tools — `engineering/book-to-skill`'s document extractors — can *optionally* use third-party parsers for higher-fidelity output, but every format falls back to a standard-library parser and nothing is installed implicitly.
|
||||
|
||||
**How do I create my own Claude Code skill?**
|
||||
Each skill is a folder with a `SKILL.md` (frontmatter + instructions), optional `scripts/`, `references/`, and `assets/`. See the [Skills & Agents Factory](https://github.com/alirezarezvani/claude-code-skills-agents-factory) for a step-by-step guide.
|
||||
|
|
|
|||
|
|
@ -16,6 +16,6 @@
|
|||
"derived_from": "https://github.com/virgiliojr94/book-to-skill",
|
||||
"original_author": "virgiliojr94",
|
||||
"original_license": "MIT",
|
||||
"derivation_note": "The extraction library (book_to_skill/ — config, exceptions, sanitize, dependencies, utils, and the 7 per-format parsers) is vendored from upstream and carries upstream's multi-format chains, chapter detection across 5 script families, and Unicode/XXE hardening. 16 numbered deviations are recorded in README.md, which is the authoritative list: the workflow was rewritten Claude-Code-first for this repo's conventions, the 3 upstream tools were reworked into 4 argparse CLIs with --help/--sample, install-on-import behaviour was replaced with a report-only default, and Step 11 (plugin emission with a rights gate) plus the validator's budget and index families are additions with no upstream counterpart."
|
||||
"derivation_note": "The extraction library (book_to_skill/ — config, exceptions, sanitize, dependencies, utils, and the 7 per-format parsers) is vendored from upstream and carries upstream's multi-format chains, chapter detection across 5 script families, and Unicode/XXE hardening. 18 numbered deviations are recorded in README.md, which is the authoritative list: the workflow was rewritten Claude-Code-first for this repo's conventions, the 3 upstream tools were reworked into 4 argparse CLIs with --help/--sample, install-on-import behaviour was replaced with a report-only default, and Step 11 (plugin emission with a rights gate) plus the validator's budget and index families are additions with no upstream counterpart."
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -210,6 +210,26 @@ small; just read it."
|
|||
(`SKILL_FILE_BUDGETS`, `CHAPTER_TOKEN_CEILING`) rather than being restated in each tool,
|
||||
where they would drift the first time a cap changed.
|
||||
|
||||
17. **Zip-of-XML hardening generalized to EPUB, plus decompression-bomb caps.** Upstream
|
||||
hardened DOCX and only DOCX: `validate_docx_xml_safety()` screened that archive for
|
||||
DTD/entity declarations before any parser touched it. EPUB is the same shape — a zip whose
|
||||
members are XML — and its `ebooklib` path handed the file straight to a third-party XML
|
||||
stack with no equivalent check, despite `ebooklib` being one of the packages this skill
|
||||
recommends installing. The guard now lives in `book_to_skill/zip_safety.py` and runs for
|
||||
both formats. Every archive read also goes through `safe_read()`, which consults the
|
||||
declared uncompressed size and the compression ratio *before* decompressing: a 200 MB
|
||||
zip bomb is refused at ~14 MB peak RSS instead of being materialized. Neither parser ever
|
||||
writes archive members to disk, so zip-slip stays out of scope by construction.
|
||||
|
||||
18. **Packaging refuses a source tree containing symlinks.** The validator checks the files it
|
||||
knows about (`SKILL.md`, the three supporting files, `chapters/*.md`), but `shutil.copytree`
|
||||
defaults to `symlinks=False` and follows a link *anywhere else* in the tree — an `assets/`
|
||||
entry, any subdirectory — baking the target's real content into a package that may then be
|
||||
emitted as `--distribution shareable`. `_assert_no_symlinks()` now walks the whole tree and
|
||||
refuses, and it runs **before** the validation branch so `--skip-validation` cannot bypass
|
||||
it. `copytree` also passes `symlinks=True` so a future edit loosening that check cannot
|
||||
silently reintroduce dereferencing.
|
||||
|
||||
---
|
||||
|
||||
## Security audit
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ from __future__ import annotations
|
|||
import zipfile
|
||||
import sys
|
||||
from book_to_skill.exceptions import ExtractionError
|
||||
from book_to_skill.zip_safety import safe_read, validate_zip_xml_safety
|
||||
|
||||
|
||||
def extract_docx_with_python_docx(docx_path: str) -> str | None:
|
||||
|
|
@ -28,7 +29,7 @@ def extract_docx_with_zipfile(docx_path: str) -> str | None:
|
|||
import xml.etree.ElementTree as ET
|
||||
|
||||
with zipfile.ZipFile(docx_path) as zf:
|
||||
xml_bytes = zf.read("word/document.xml")
|
||||
xml_bytes = safe_read(zf, "word/document.xml")
|
||||
root = ET.fromstring(xml_bytes)
|
||||
ns = "{http://schemas.openxmlformats.org/wordprocessingml/2006/main}"
|
||||
parts: list[str] = []
|
||||
|
|
@ -69,27 +70,13 @@ def extract_docx_with_zipfile(docx_path: str) -> str | None:
|
|||
|
||||
|
||||
def validate_docx_xml_safety(docx_path: str) -> None:
|
||||
"""Scan all XML files in the DOCX zip archive to prevent XML Entity Expansion (Billion Laughs) and XXE injections."""
|
||||
try:
|
||||
with zipfile.ZipFile(docx_path) as zf:
|
||||
for name in zf.namelist():
|
||||
if name.endswith(".xml") or name.endswith(".rels"):
|
||||
xml_bytes = zf.read(name)
|
||||
for encoding in ("utf-8", "utf-16", "utf-16le", "utf-16be", "utf-32"):
|
||||
try:
|
||||
content = xml_bytes.decode(encoding, errors="ignore").upper()
|
||||
except LookupError:
|
||||
continue
|
||||
if "<!DOCTYPE" in content or "<!ENTITY" in content:
|
||||
raise ExtractionError(
|
||||
f"Security validation failed: XML file '{name}' in DOCX archive contains forbidden DTD or entity declarations."
|
||||
)
|
||||
except zipfile.BadZipFile as e:
|
||||
raise ExtractionError(f"Invalid DOCX file: {e}")
|
||||
except ExtractionError:
|
||||
raise
|
||||
except Exception as e:
|
||||
raise ExtractionError(f"Error during security validation of DOCX archive: {e}")
|
||||
"""Refuse a DOCX whose XML declares a DTD or entity (billion-laughs / XXE).
|
||||
|
||||
Now a thin wrapper over the shared zip guard so EPUB gets identical
|
||||
treatment; the scan also enforces per-member size and compression-ratio caps
|
||||
before decompressing anything.
|
||||
"""
|
||||
validate_zip_xml_safety(docx_path, label="DOCX")
|
||||
|
||||
|
||||
def extract_docx(docx_path: str) -> tuple[str, str]:
|
||||
|
|
|
|||
|
|
@ -5,9 +5,19 @@ import re
|
|||
import zipfile
|
||||
import sys
|
||||
from book_to_skill.parsers.html_text import _HTMLTextExtractor
|
||||
from book_to_skill.zip_safety import _Budget, safe_read, validate_zip_xml_safety
|
||||
|
||||
|
||||
def extract_with_ebooklib(epub_path: str) -> str | None:
|
||||
"""Extract via ebooklib, after the same XML guard DOCX gets.
|
||||
|
||||
ebooklib parses container.xml, the OPF package document and the content docs
|
||||
with a third-party XML stack whose entity handling this code does not
|
||||
control. The archive is an untrusted file the user was handed, so it is
|
||||
screened for DTD/entity declarations before ebooklib sees it — upstream ran
|
||||
this check on DOCX only.
|
||||
"""
|
||||
validate_zip_xml_safety(epub_path, label="EPUB")
|
||||
try:
|
||||
import ebooklib
|
||||
from ebooklib import epub
|
||||
|
|
@ -34,7 +44,7 @@ def _find_opf_path(zf: zipfile.ZipFile) -> str | None:
|
|||
"""
|
||||
# Spec-defined: read container.xml for the rootfile path
|
||||
try:
|
||||
container = zf.read("META-INF/container.xml").decode("utf-8", errors="replace")
|
||||
container = safe_read(zf, "META-INF/container.xml").decode("utf-8", errors="replace")
|
||||
match = re.search(r'full-path=["\']([^"\']+\.opf)["\']', container)
|
||||
if match:
|
||||
return match.group(1)
|
||||
|
|
@ -50,6 +60,7 @@ def extract_with_zipfile(epub_path: str) -> str | None:
|
|||
"""stdlib-only EPUB extractor: unzip → parse HTML files."""
|
||||
try:
|
||||
with zipfile.ZipFile(epub_path) as zf:
|
||||
budget = _Budget()
|
||||
names = zf.namelist()
|
||||
|
||||
# Locate OPF and determine its directory for resolving relative hrefs
|
||||
|
|
@ -61,7 +72,7 @@ def extract_with_zipfile(epub_path: str) -> str | None:
|
|||
spine_order: list[str] = []
|
||||
seen: set[str] = set()
|
||||
if opf_path:
|
||||
opf_text = zf.read(opf_path).decode("utf-8", errors="replace")
|
||||
opf_text = safe_read(zf, opf_path, budget).decode("utf-8", errors="replace")
|
||||
|
||||
# Manifest: item id -> resolved href. Parse each <item> opening
|
||||
# tag so attribute order (id before/after href) does not matter;
|
||||
|
|
@ -99,7 +110,7 @@ def extract_with_zipfile(epub_path: str) -> str | None:
|
|||
parts = []
|
||||
for name in html_files:
|
||||
try:
|
||||
raw = zf.read(name).decode("utf-8", errors="replace")
|
||||
raw = safe_read(zf, name, budget).decode("utf-8", errors="replace")
|
||||
parser = _HTMLTextExtractor()
|
||||
parser.feed(raw)
|
||||
parts.append(parser.get_text())
|
||||
|
|
@ -118,7 +129,7 @@ def count_epub_chapters(epub_path: str) -> int:
|
|||
opf_path = _find_opf_path(zf)
|
||||
if not opf_path:
|
||||
return 0
|
||||
opf_text = zf.read(opf_path).decode("utf-8", errors="replace")
|
||||
opf_text = safe_read(zf, opf_path).decode("utf-8", errors="replace")
|
||||
return len(re.findall(r'<itemref\b', opf_text))
|
||||
except Exception:
|
||||
return 0
|
||||
|
|
|
|||
|
|
@ -0,0 +1,130 @@
|
|||
"""Shared safety checks for zip-of-XML container formats (DOCX, EPUB).
|
||||
|
||||
Upstream hardened DOCX and only DOCX: `validate_docx_xml_safety()` scanned that
|
||||
archive for DTD/entity declarations before any parser touched it. EPUB is the
|
||||
same shape — a zip whose members are XML (`container.xml`, the OPF package
|
||||
document, NCX, content docs) — and its `ebooklib` path handed the file straight
|
||||
to an XML parser with no equivalent pre-check, even though `ebooklib` is one of
|
||||
the packages this skill recommends installing "for best results".
|
||||
|
||||
Two classes of attack, both from a file the user was handed rather than wrote:
|
||||
|
||||
1. **Entity expansion / XXE.** `<!ENTITY>` declarations drive billion-laughs
|
||||
memory exhaustion; external entities read local files into the parsed output.
|
||||
Python's `xml.etree.ElementTree` does not resolve *external* entities, but it
|
||||
does expand internal ones — and third-party parsers built on lxml vary. The
|
||||
check refuses the archive rather than trusting each parser's defaults.
|
||||
|
||||
2. **Decompression bombs.** A few KB of zip can expand to gigabytes. Every read
|
||||
goes through `safe_read()`, which consults the declared uncompressed size and
|
||||
the compression ratio *before* decompressing, so a bomb is refused rather
|
||||
than materialized.
|
||||
|
||||
Neither parser ever writes archive members to disk (no `extractall`/`extract`),
|
||||
so zip-slip is out of scope by construction.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import zipfile
|
||||
|
||||
from book_to_skill.exceptions import ExtractionError
|
||||
|
||||
# A single member of a book archive. Generous for a chapter's XHTML or a
|
||||
# document.xml, far below what a bomb needs to hurt.
|
||||
MAX_MEMBER_BYTES = 64 * 1024 * 1024
|
||||
# Everything read from one archive, across all members.
|
||||
MAX_TOTAL_BYTES = 512 * 1024 * 1024
|
||||
# Declared-uncompressed / stored ratio. Real prose and XML land well under 100x;
|
||||
# the classic zip bomb is ~1000x and up.
|
||||
MAX_COMPRESSION_RATIO = 200
|
||||
|
||||
# Members worth scanning for DTD/entity declarations: XML by extension, plus the
|
||||
# extensionless `mimetype` member and OPF/NCX which are XML without an .xml suffix.
|
||||
_XML_SUFFIXES = (".xml", ".rels", ".opf", ".ncx", ".xhtml", ".html", ".htm")
|
||||
|
||||
|
||||
class _Budget:
|
||||
"""Running total of bytes decompressed from one archive."""
|
||||
|
||||
def __init__(self, limit: int = MAX_TOTAL_BYTES) -> None:
|
||||
self.limit = limit
|
||||
self.used = 0
|
||||
|
||||
def charge(self, size: int, name: str) -> None:
|
||||
self.used += size
|
||||
if self.used > self.limit:
|
||||
raise ExtractionError(
|
||||
f"archive exceeds the {self.limit:,}-byte decompression budget "
|
||||
f"(reading '{name}') — refusing to continue"
|
||||
)
|
||||
|
||||
|
||||
def safe_read(zf: zipfile.ZipFile, name: str, budget: _Budget | None = None) -> bytes:
|
||||
"""Read one archive member after checking its declared size and ratio.
|
||||
|
||||
The checks run against the zip's central directory *before* decompressing,
|
||||
so a bomb never gets materialized. A liar in the directory still cannot get
|
||||
past the budget, because the actual read is charged against it too.
|
||||
"""
|
||||
try:
|
||||
info = zf.getinfo(name)
|
||||
except KeyError as exc:
|
||||
raise ExtractionError(f"archive member not found: {name}") from exc
|
||||
|
||||
if info.file_size > MAX_MEMBER_BYTES:
|
||||
raise ExtractionError(
|
||||
f"archive member '{name}' declares {info.file_size:,} bytes uncompressed; "
|
||||
f"the per-member limit is {MAX_MEMBER_BYTES:,} bytes"
|
||||
)
|
||||
if info.compress_size > 0:
|
||||
ratio = info.file_size / info.compress_size
|
||||
if ratio > MAX_COMPRESSION_RATIO:
|
||||
raise ExtractionError(
|
||||
f"archive member '{name}' expands {ratio:.0f}x "
|
||||
f"({info.compress_size:,} -> {info.file_size:,} bytes); the limit is "
|
||||
f"{MAX_COMPRESSION_RATIO}x — this looks like a decompression bomb"
|
||||
)
|
||||
|
||||
data = zf.read(name)
|
||||
if budget is not None:
|
||||
budget.charge(len(data), name)
|
||||
return data
|
||||
|
||||
|
||||
def is_xml_member(name: str) -> bool:
|
||||
return name.lower().endswith(_XML_SUFFIXES)
|
||||
|
||||
|
||||
def validate_zip_xml_safety(archive_path: str, label: str = "archive") -> None:
|
||||
"""Refuse a zip-of-XML archive that declares a DTD or any entity.
|
||||
|
||||
Scans every XML-ish member across the encodings a hostile file might use to
|
||||
hide the declaration from a naive UTF-8 substring search. Generalized from
|
||||
upstream's DOCX-only guard so EPUB gets the same treatment.
|
||||
"""
|
||||
try:
|
||||
with zipfile.ZipFile(archive_path) as zf:
|
||||
budget = _Budget()
|
||||
for name in zf.namelist():
|
||||
if not is_xml_member(name):
|
||||
continue
|
||||
xml_bytes = safe_read(zf, name, budget)
|
||||
for encoding in ("utf-8", "utf-16", "utf-16le", "utf-16be", "utf-32"):
|
||||
try:
|
||||
content = xml_bytes.decode(encoding, errors="ignore").upper()
|
||||
except LookupError:
|
||||
continue
|
||||
if "<!DOCTYPE" in content or "<!ENTITY" in content:
|
||||
raise ExtractionError(
|
||||
f"security validation failed: XML member '{name}' in the {label} "
|
||||
f"archive contains a forbidden DTD or entity declaration"
|
||||
)
|
||||
except zipfile.BadZipFile as exc:
|
||||
raise ExtractionError(f"invalid {label} file: {exc}") from exc
|
||||
except ExtractionError:
|
||||
raise
|
||||
except Exception as exc:
|
||||
raise ExtractionError(
|
||||
f"error during security validation of the {label} archive: {exc}"
|
||||
) from exc
|
||||
|
|
@ -43,6 +43,7 @@ from __future__ import annotations
|
|||
|
||||
import argparse
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
import shutil
|
||||
import sys
|
||||
|
|
@ -136,6 +137,34 @@ def _assert_replaceable(package_root: Path, dest_root: Path, skill_dir: Path) ->
|
|||
)
|
||||
|
||||
|
||||
def _assert_no_symlinks(skill_dir: Path) -> None:
|
||||
"""Refuse to package a source tree containing any symbolic link.
|
||||
|
||||
The validator checks the files it knows about — SKILL.md, the three
|
||||
supporting files, chapters/*.md — but `shutil.copytree` defaults to
|
||||
`symlinks=False`, which follows a link *anywhere else* in the tree (an
|
||||
`assets/` entry, an arbitrary subdirectory) and bakes the target's real
|
||||
content into the emitted package. That package can then go out as
|
||||
`--distribution shareable`, so a link pointing at something outside the
|
||||
skill becomes part of a published artifact.
|
||||
|
||||
Refusing beats `copytree(symlinks=True)`: preserving the link would ship a
|
||||
package whose contents depend on the emitting machine's filesystem. This
|
||||
check runs even under `--skip-validation`, which otherwise disables the
|
||||
file-level symlink checks entirely.
|
||||
"""
|
||||
offenders = [p for p in sorted(skill_dir.rglob("*")) if p.is_symlink()]
|
||||
if offenders:
|
||||
listed = "\n".join(f" {p.relative_to(skill_dir).as_posix()} -> {os.readlink(p)}"
|
||||
for p in offenders[:10])
|
||||
more = f"\n ... and {len(offenders) - 10} more" if len(offenders) > 10 else ""
|
||||
raise EmitError(
|
||||
f"source skill contains {len(offenders)} symbolic link(s); packaging would copy "
|
||||
f"the target's content into a distributable package:\n{listed}{more}\n"
|
||||
"Replace them with real files, or point --skill-dir at a tree without links."
|
||||
)
|
||||
|
||||
|
||||
def _plugin_manifest(identity: dict, *, domain: str, author: str, author_url: str,
|
||||
repository: str, distribution: str, rights: str | None,
|
||||
source_note: str) -> dict:
|
||||
|
|
@ -337,6 +366,10 @@ def emit(*, skill_dir: Path, dest_root: Path, domain: str, author: str, author_u
|
|||
"Or emit with --distribution local (the default) to keep it on this machine."
|
||||
)
|
||||
|
||||
# Unconditional: --skip-validation waives content findings, never the
|
||||
# guarantee that packaging copies only what is actually in the source tree.
|
||||
_assert_no_symlinks(skill_dir)
|
||||
|
||||
if not skip_validation:
|
||||
try:
|
||||
findings = validate(skill_dir)
|
||||
|
|
@ -389,7 +422,9 @@ def emit(*, skill_dir: Path, dest_root: Path, domain: str, author: str, author_u
|
|||
target = package_root / relative
|
||||
target.parent.mkdir(parents=True, exist_ok=True)
|
||||
target.write_text(text, encoding="utf-8")
|
||||
shutil.copytree(skill_dir, package_root / "skills" / identity["slug"])
|
||||
# symlinks=True is redundant after _assert_no_symlinks, and kept so a future
|
||||
# edit that loosens that check cannot silently reintroduce dereferencing.
|
||||
shutil.copytree(skill_dir, package_root / "skills" / identity["slug"], symlinks=True)
|
||||
return result
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -83,6 +83,7 @@ engineering/skillopt-sleep/skillopt_sleep/types.py # internal module; module it
|
|||
engineering/book-to-skill/skills/book-to-skill/scripts/book_to_skill/__init__.py # package init, not a CLI
|
||||
engineering/book-to-skill/skills/book-to-skill/scripts/book_to_skill/dependencies.py # internal module
|
||||
engineering/book-to-skill/skills/book-to-skill/scripts/book_to_skill/utils.py # internal module
|
||||
engineering/book-to-skill/skills/book-to-skill/scripts/book_to_skill/zip_safety.py # internal module
|
||||
engineering/book-to-skill/skills/book-to-skill/scripts/book_to_skill/parsers/calibre.py # internal module
|
||||
engineering/book-to-skill/skills/book-to-skill/scripts/book_to_skill/parsers/docx.py # internal module
|
||||
engineering/book-to-skill/skills/book-to-skill/scripts/book_to_skill/parsers/epub.py # internal module
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue