fix(assets): a remote reference is inert and a declared size is bounded
Two MAJOR findings of the independent v0.10.0 review, both with the shipped defaults, both new in 0.10.0. Repros rebuilt as tests first. - A remote <img src>/xlink:href became a LIVE markdown image link in the persisted concept, with the address and query string chosen by whoever wrote the document. Extraction opens no socket; a consumer rendering the bundle does. Now inert text with the address in a code span, pinned by a property over the readers rather than by one string. The tier asymmetry (user-upload refuses, trusted-source persisted) went to the guard repo with the repro. - Nothing bounded a declared image size: 9.6 KB of PDF declaring 3000x3000 grayscale zeros took 83 MB peak RSS, linear in pixels. MAX_IMAGE_PIXELS (40 000 000) and MAX_IMAGE_BYTES (256 MiB) are read off the corpora (largest measured 18.6 MP on K2, 1.4 MP on R761) and checked on what the container declares, before any decompression; over them is asset_too_large, counted. The same bound closes the inline data: URI, which the review flagged and did not measure. Also fixed, added by PM to this order: an inline PDF image was named from id() of a Python object, so two concept files of the reference corpus differed between builds. It is now named from its position. R761 unchanged: 50 carried of 50 found, assets diff -rq clean. Report: docs/2026-09-17-bildestien-0-10-1.md Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
864570b320
commit
230d1cbccd
8 changed files with 580 additions and 3 deletions
44
CHANGELOG.md
44
CHANGELOG.md
|
|
@ -68,6 +68,50 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
what the SOURCES declare. A refused document's pictures therefore no longer
|
||||
read as "0 of 0 found".
|
||||
|
||||
### Security
|
||||
|
||||
- **A remote image reference is no longer a live markdown image link
|
||||
(0.10.1).** New in 0.10.0: before it, no reader read an `<img>` attribute at
|
||||
all. A document could put ``
|
||||
into a persisted concept, with the address and query string chosen by
|
||||
whoever wrote the document. This package opens no socket, but a consumer
|
||||
that renders the bundle — or an agent that fetches what it renders — does,
|
||||
which turns "this bundle was opened" into a beacon, and a server-side
|
||||
consumer into an SSRF. The guard refuses such a line at
|
||||
`guard-user-upload` and the build's default tier does not, so the same bytes
|
||||
were persisted under the default and refused one tier up. A remote reference
|
||||
is now inert text with the address in a code span, and a property test over
|
||||
the readers asserts that no reference produces a markdown image link outside
|
||||
`assets/`. Found by an independent review of 0.10.0 before it was pushed.
|
||||
- **A declared image size is bounded (0.10.1).** Nothing limited a PDF image's
|
||||
size: a 9.6 KB file declaring 3 000 x 3 000 grayscale zeros took 83 MB of
|
||||
peak RSS and a 63 KB one declaring 8 000 x 8 000 took 276 MB, linear in the
|
||||
pixel count, so one document could exhaust memory and take a whole batch
|
||||
build with it — before any gate, because the guard never sees image bytes.
|
||||
`MAX_IMAGE_PIXELS` (40 000 000) and `MAX_IMAGE_BYTES` (256 MiB) are now
|
||||
checked against what the container DECLARES, before anything is
|
||||
decompressed, and a larger image is refused with `asset_too_large` and
|
||||
counted. The bound is read off the corpora: the largest image in the
|
||||
43-document reference corpus is 18.6 MP and R761's largest is 1.4 MP. The
|
||||
same bound closes the inline `data:` URI, which the review flagged as the
|
||||
same class and did not measure.
|
||||
|
||||
### Fixed
|
||||
|
||||
- **An inline PDF image gets a stable name (0.10.1).** pdfminer names an
|
||||
inline image (`BI … EI`) from `id()` of a Python object, so a pointer line
|
||||
changed between two runs of one build and two concept files of the reference
|
||||
corpus differed — breaking the bit-exact rebuild invariant. Such an image is
|
||||
now named from its position on the page.
|
||||
|
||||
### Documented, not changed
|
||||
|
||||
- `images: N` in a concept counts POINTER BLOCKS, not unique pictures (12
|
||||
pointers to 2 files is `images: 12`). Now stated in the README.
|
||||
- A concept that is only a pointer block is persisted as substantive, because
|
||||
"degenerate" means zero characters after stripping whitespace and a pointer
|
||||
block is text.
|
||||
|
||||
## [0.10.0] — 2026-09-17
|
||||
|
||||
### Added
|
||||
|
|
|
|||
21
CLAUDE.md
21
CLAUDE.md
|
|
@ -890,6 +890,27 @@ and fixtures, never code.
|
|||
move (it is not delivered on the Arm B bundle either, by a different
|
||||
mechanism). hit@8 over the six published questions holds at **5 of 6 on both
|
||||
K2 bundles**, so the default move cost the retrieval side nothing.
|
||||
- **The image path has a size CEILING and no live links (0.10.1).** An
|
||||
independent review of `v0.10.0` found two MAJOR defects, both with the
|
||||
shipped defaults, both new in 0.10.0.
|
||||
- A remote `<img src>` / `xlink:href` became a LIVE markdown image link in
|
||||
the persisted concept. This package opens no socket; a consumer that
|
||||
renders the bundle does, and the guard refuses such a line at
|
||||
`user-upload` while the build's default tier persisted it. Now inert text
|
||||
with the address in a code span, pinned by a PROPERTY over the readers
|
||||
(`FOREIGN_IMAGE_LINK`), never one string.
|
||||
- Nothing bounded a declared image size: 9.6 KB of PDF declaring
|
||||
3 000 x 3 000 zeros took 83 MB peak RSS, linear in pixels.
|
||||
`MAX_IMAGE_PIXELS = 40 000 000` and `MAX_IMAGE_BYTES = 256 MiB` are
|
||||
checked on what the container DECLARES, before any decompression, and
|
||||
over them is `asset_too_large`, counted. Read off the corpora: the
|
||||
largest image measured is 18.6 MP (K2) and 1.4 MP (R761). The same bound
|
||||
closes the inline `data:` URI.
|
||||
- **An inline PDF image is named from its position, not from `id()`** —
|
||||
pdfminer's name broke the bit-exact rebuild on two K2 concepts.
|
||||
- Documented rather than changed: `images: N` counts POINTER BLOCKS, and a
|
||||
pointer-only concept is substantive.
|
||||
- Report: `docs/2026-09-17-bildestien-0-10-1.md`.
|
||||
- **Account for content: `okf build --accounting PATH` and its judge
|
||||
`python3 tools/okf_accounting_gate.py` (2026-09-17).**
|
||||
- The gate was written RED at `0b00de4` and turned GREEN on all six rows by
|
||||
|
|
|
|||
32
README.md
32
README.md
|
|
@ -299,6 +299,38 @@ in must not read like a bundle of documents that had none.
|
|||
text. The pointer block passes the gate like any other body line, and the file
|
||||
beside it passes nothing. `log.md` says so on every run that carries one.
|
||||
|
||||
**A size CEILING, read off the same corpora (0.10.1).** An image whose
|
||||
DECLARED size is over `MAX_IMAGE_PIXELS` (40 000 000 pixels) or
|
||||
`MAX_IMAGE_BYTES` (256 MiB of samples) is refused with `asset_too_large`,
|
||||
counted like every other refusal, and never decoded. The largest image in the
|
||||
43-document reference corpus is 4 515 x 4 128 (18.6 MP) and the largest of
|
||||
R761's 109 pictures is 2 072 x 656 (1.4 MP), so the bound is an order of
|
||||
magnitude above anything measured.
|
||||
|
||||
It exists because a few kilobytes can declare an enormous picture: a 9.6 KB
|
||||
PDF declaring 3 000 x 3 000 grayscale zeros took 83 MB of peak RSS, a 63 KB
|
||||
one declaring 8 000 x 8 000 took 276 MB, and the cost is linear in the pixel
|
||||
count, so one document could take a whole batch build with it — before any
|
||||
gate, because the guard never sees image bytes. The bound is checked on what
|
||||
the container declares, before anything is decompressed, on the two paths that
|
||||
decode: a PDF image's raw samples and an inline `data:` URI. An image file
|
||||
carried verbatim is not decoded at all, so its cost is its own file size.
|
||||
|
||||
<!-- asset-max-pixels: 40000000 -->
|
||||
|
||||
**A remote reference is INERT (0.10.1).** `<img src="https://...">` and an STS
|
||||
`xlink:href` to an address off this machine are written as text with the
|
||||
address in a code span, never as ``. Extraction opens no
|
||||
socket, but a markdown renderer or an agent that fetches what it renders does,
|
||||
which would turn "this bundle was opened" into a beacon to whoever wrote the
|
||||
document. The address is still stated, because a reader has to know what stood
|
||||
there.
|
||||
|
||||
**`images: N` in a concept counts POINTER BLOCKS, not unique pictures.** One
|
||||
image referenced twelve times in one concept is `images: 12` and one file in
|
||||
`assets/`. The key is a count of the places a picture stands, and dedup is on
|
||||
content.
|
||||
|
||||
**No size floor, and that is a measurement too.** The obvious filter is "ignore
|
||||
anything under N pixels", and the distribution offers no N: over the 4 828
|
||||
image objects in that corpus, 149 declare no size, 162 are under 32x32, 92
|
||||
|
|
|
|||
121
docs/2026-09-17-bildestien-0-10-1.md
Normal file
121
docs/2026-09-17-bildestien-0-10-1.md
Normal file
|
|
@ -0,0 +1,121 @@
|
|||
# Two findings in the image path, closed before push (0.10.1)
|
||||
|
||||
Order `20260917T135034Z-4840022792`, from an independent review of `v0.10.0`
|
||||
(`~/.claude/docs/2026-09-17-review-okf-0.10.0.local.md`, LOCAL-ONLY). The
|
||||
review's verdict was "safe to push, with reservations": no blocker, and
|
||||
byte-identity, determinism and `--no-assets` parity all hold. Both findings
|
||||
land with the shipped defaults (`--assets` on, `--gate guard-trusted-source`),
|
||||
and both are new in 0.10.0, because before it no reader read an `<img>`
|
||||
attribute or opened an image stream.
|
||||
|
||||
Both repros were rebuilt here as tests before anything was fixed
|
||||
(`tests/test_asset_limits.py`, 17 tests).
|
||||
|
||||
## MAJOR-1: a remote reference was a live markdown image link
|
||||
|
||||
Reproduced:
|
||||
|
||||
render_missing('https://collect.example.net/p.gif?u=S', href=…)
|
||||
-> '\nImage: … (not carried: …)'
|
||||
|
||||
The first line is a live image link to an address the document's author
|
||||
controls, query string included. This package opens no socket. A consumer that
|
||||
renders the bundle, or an agent that fetches what it renders, does — which
|
||||
turns "this bundle was opened" into a beacon (and a server-side consumer into
|
||||
an SSRF). Measured with guard 1.4.0: `PRESET_USER_UPLOAD` fails the document
|
||||
secure, `PRESET_TRUSTED_SOURCE` — the build's default — persists it.
|
||||
|
||||
**Fixed in `render_missing`:** a reference with an `href` is written as
|
||||
`Image: <name> (not carried: <reason>) address: `<address>`` — inert text,
|
||||
with the address still stated, because a reader has to know what stood there.
|
||||
|
||||
**Pinned as a property, not a string.** `FOREIGN_IMAGE_LINK` matches any
|
||||
markdown image whose target is not this bundle's own `assets/`. It is asserted
|
||||
over the two readers that resolve references (HTML, STS), over three shapes of
|
||||
remote address (`https:`, `//host`, upper-case scheme), and over a whole built
|
||||
bundle of the shipped fixture inbox. The known-positive beside it: a local
|
||||
image still produces a pointer block that `IMAGE_POINTER` matches, and a
|
||||
`data:` URI image is still carried.
|
||||
|
||||
The tier asymmetry is the guard's own question and was sent to
|
||||
`llm-ingestion-pipeline-security` with the repro
|
||||
(`20260917T221801Z-428505178`). Nothing was built there.
|
||||
|
||||
## MAJOR-2: nothing bounded a declared image size
|
||||
|
||||
Reproduced with the review's own generator, rebuilt in the test file: a PDF
|
||||
declaring one grayscale image of compressed zeros.
|
||||
|
||||
| declared | PDF file | peak RSS |
|
||||
|---|---:|---:|
|
||||
| 3 000 x 3 000 | 9.6 KB | 83 MB |
|
||||
| 8 000 x 8 000 | 63 KB | 276 MB (review's measurement) |
|
||||
|
||||
The cost is linear in the pixel count, so 50 000 x 50 000 is several GB. One
|
||||
document — malicious, defective, or a legitimately enormous scan — could take
|
||||
a whole batch build with it, before any gate, because the guard never sees
|
||||
image bytes.
|
||||
|
||||
**The bound is read off the corpora, not chosen.** Over the 4 828 image
|
||||
objects of the 43-document reference corpus the largest is 4 515 x 4 128
|
||||
(18.6 MP, a landscape drawing). Over R761's 109 delivered pictures the largest
|
||||
is 2 072 x 656 (1.4 MP).
|
||||
|
||||
- `MAX_IMAGE_PIXELS = 40 000 000` (2.1x the largest measured).
|
||||
- `MAX_IMAGE_BYTES = 256 MiB` of samples.
|
||||
- Over either: `asset_too_large`, counted like every other refusal, with the
|
||||
declared size in the reason. Never a silent skip and never a killed build.
|
||||
|
||||
**Checked on what the container DECLARES, before anything is decompressed.**
|
||||
`stream.get_data()` is what pays for the bomb, so the declared `Width` and
|
||||
`Height` are read first. The order is observable, not asserted: the test feeds
|
||||
a PDF whose image stream is corrupt AND whose declared size is over the bound.
|
||||
Decoding first gives `asset_pdf_unsupported`; reading the size first gives
|
||||
`asset_too_large`. `encode_png` refuses the same size on its own, so the
|
||||
encoder does not trust its caller.
|
||||
|
||||
**The `data:` URI, which the review flagged and did not measure,** is closed by
|
||||
the same bound, checked on the payload length before decoding (base64 expands
|
||||
by 3/4). Measured: refused with `asset_too_large` as a row.
|
||||
|
||||
Not bounded, and stated rather than hidden: an image FILE carried verbatim is
|
||||
never decoded, so its cost is its own file size.
|
||||
|
||||
## The determinism defect PM added to this order
|
||||
|
||||
pdfminer names an inline image (`BI … EI`) from `id()` of a Python object.
|
||||
Measured 2026-09-17: two concept files of the reference corpus differed
|
||||
between two builds of one commit, in the pointer line
|
||||
(`Image: page-3-6344095824` against `page-3-4555034320`). That breaks the
|
||||
bit-exact rebuild invariant.
|
||||
|
||||
**Fixed:** a name pdfminer derived from `id()` — all digits, no resource name —
|
||||
is replaced by the image's position on the page (`page-3-inline-2`). The test
|
||||
builds a PDF with an inline image and compares two extractions with the
|
||||
`_pdf_pages` cache cleared between them.
|
||||
|
||||
## The three MINOR findings
|
||||
|
||||
- **`images: N` counts pointer blocks, not unique pictures** (12 pointers to
|
||||
2 files is `images: 12`). Documented in the README rather than changed:
|
||||
the number is the count of places a picture stands, dedup is on content, and
|
||||
changing it would move bundle bytes and lose the placement count.
|
||||
- **A concept that is only a pointer block is persisted as substantive.**
|
||||
Documented: "degenerate" means zero characters after stripping whitespace,
|
||||
and a pointer block is text. Changing the definition is a spec question.
|
||||
- **`data:` URI:** closed, see above.
|
||||
|
||||
## Verification
|
||||
|
||||
- `tests/test_asset_limits.py`: 17 tests, both repros red first.
|
||||
- Full suite 2 045 passed, 1 skipped; `ruff`, `ruff format --check` and
|
||||
`mypy --strict` clean. Re-run after `git add` against a clean tree.
|
||||
- **R761 unchanged (PM's measurement, repeated):** `--gate none` gives
|
||||
`50 carried of 50 found`, 50 files in `assets/`, and `diff -rq` against the
|
||||
bundle built before this change reports no difference.
|
||||
- `tools/okf_accounting_gate.py`: green on all six rows.
|
||||
|
||||
## Proposal
|
||||
|
||||
Version `0.10.1`. No tag, no release, no push — the content accounting sits in
|
||||
the same tree and PM wants the order of the two settled first.
|
||||
|
|
@ -117,6 +117,58 @@ IMAGE_POINTER = re.compile(
|
|||
)
|
||||
|
||||
|
||||
#: THE SIZE A PICTURE MAY DECLARE. Read off the corpora rather than chosen:
|
||||
#: over the 4 828 image objects of the 43-document reference corpus the largest
|
||||
#: is 4 515 x 4 128 (18.6 MP, a landscape drawing), and over R761's 109
|
||||
#: delivered pictures it is 2 072 x 656 (1.4 MP). This sits an order of
|
||||
#: magnitude above both, so nothing anyone has measured is refused -- and a
|
||||
#: declared size beyond it is a COUNTED refusal instead of a killed build.
|
||||
#:
|
||||
#: The bound exists because a few kilobytes can declare an enormous picture: a
|
||||
#: 9.6 KB PDF declaring 3 000 x 3 000 grayscale zeros took 83 MB of peak RSS
|
||||
#: and a 63 KB one declaring 8 000 x 8 000 took 276 MB, linear in the pixel
|
||||
#: count, so 50 000 x 50 000 is several GB and takes the whole batch with it.
|
||||
#: Found by an independent review of 0.10.0 before it was pushed.
|
||||
MAX_IMAGE_PIXELS = 40_000_000
|
||||
|
||||
#: The same bound in bytes, for a payload whose pixels are not yet known (an
|
||||
#: inline `data:` URI) and for raw samples with several channels.
|
||||
MAX_IMAGE_BYTES = 256 * 1024 * 1024
|
||||
|
||||
|
||||
def check_size(
|
||||
width: int | None, height: int | None, *, name: str, channels: int = 1, bits: int = 8
|
||||
) -> None:
|
||||
"""Refuse a declared size beyond the bound, BEFORE anything is decoded.
|
||||
|
||||
Read off what the container DECLARES, which is the only number available
|
||||
before the cost is paid: a check after the decompression has already paid
|
||||
for the bomb it was meant to stop.
|
||||
"""
|
||||
if not width or not height:
|
||||
return
|
||||
pixels = width * height
|
||||
expected = pixels * channels * (bits // 8 or 1)
|
||||
if pixels > MAX_IMAGE_PIXELS or expected > MAX_IMAGE_BYTES:
|
||||
raise ExtractionError(
|
||||
f"the image {name!r} declares {width}x{height} = {pixels} pixels "
|
||||
f"({expected} bytes of samples), over this package's bound of "
|
||||
f"{MAX_IMAGE_PIXELS} pixels and {MAX_IMAGE_BYTES} bytes; refused "
|
||||
"unread so one picture cannot take the run with it",
|
||||
code="asset_too_large",
|
||||
)
|
||||
|
||||
|
||||
def check_payload(size: int, *, name: str) -> None:
|
||||
"""The same bound for an encoded payload of `size` bytes."""
|
||||
if size > MAX_IMAGE_BYTES:
|
||||
raise ExtractionError(
|
||||
f"the image {name!r} carries {size} encoded bytes, over this package's "
|
||||
f"bound of {MAX_IMAGE_BYTES}; refused unread",
|
||||
code="asset_too_large",
|
||||
)
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class ExtractedImage:
|
||||
"""One image a document carries, as this package will write it.
|
||||
|
|
@ -368,6 +420,10 @@ def encode_png(
|
|||
) -> bytes:
|
||||
"""8-bit samples as a PNG, using nothing but `zlib`.
|
||||
|
||||
Refuses a size over :data:`MAX_IMAGE_PIXELS` on its own rather than
|
||||
trusting the caller to have checked: this function is what allocates
|
||||
`width * height * channels` bytes twice over.
|
||||
|
||||
A PDF image is usually not a file: `FlateDecode` hands back raw samples
|
||||
with the colour model in the dictionary beside them, so carrying one at all
|
||||
means encoding it. Doing that with the stdlib rather than with a renderer
|
||||
|
|
@ -379,6 +435,7 @@ def encode_png(
|
|||
`channels` is 1 (grey, or indexed when `palette` is given) or 3 (RGB).
|
||||
`alpha` is one byte per pixel, from a PDF `SMask`; absent means opaque.
|
||||
"""
|
||||
check_size(width, height, name=f"{width}x{height}", channels=channels)
|
||||
if channels not in (1, 3):
|
||||
raise ExtractionError(
|
||||
f"PNG encoding supports 1 or 3 channels, not {channels}",
|
||||
|
|
@ -477,6 +534,15 @@ def render_missing(
|
|||
"""
|
||||
shown = _inline(name or "image")
|
||||
detail = f"Image: {shown} (not carried: {_inline(reason)})"
|
||||
if href and not re.search(r"[\s()\[\]]", href):
|
||||
return f"\n{detail}"
|
||||
if href:
|
||||
# INERT, never ``. A remote reference is an address the
|
||||
# document's author chose, and 0.10.0 wrote it as a live markdown
|
||||
# image: a consumer that renders the bundle, or an agent that fetches
|
||||
# what it renders, turns "this bundle was opened" into a beacon to
|
||||
# them -- and a server-side consumer into an SSRF. This package opens
|
||||
# no socket, which is not the same as the pointer being harmless.
|
||||
# The address is still STATED, in a code span, because a reader has to
|
||||
# know what stood there to judge whether the picture mattered.
|
||||
address = _inline(href).replace("`", "'")
|
||||
return f"{detail} address: `{address}`"
|
||||
return detail
|
||||
|
|
|
|||
|
|
@ -45,6 +45,8 @@ from xml.etree.ElementTree import Element
|
|||
from .assets import (
|
||||
AssetRejection,
|
||||
ExtractedImage,
|
||||
check_payload,
|
||||
check_size,
|
||||
encode_png,
|
||||
read_image,
|
||||
render_block,
|
||||
|
|
@ -473,6 +475,16 @@ class _AssetCollector:
|
|||
def _data_uri(self, match: re.Match[str], *, label: str | None) -> str:
|
||||
payload = match.group("payload")
|
||||
name = f"data-uri{Path(match.group('media').split('/')[-1] or 'bin').suffix or ''}"
|
||||
# Before decoding: base64 expands by 3/4, and a percent-encoded payload
|
||||
# by at most 1. An inline picture is small by nature, so a payload over
|
||||
# the bound is refused unread -- the same bomb class as a PDF image
|
||||
# declaring an enormous size. A refusal is a ROW, like every other.
|
||||
try:
|
||||
check_payload(
|
||||
len(payload) * 3 // 4 if match.group("base64") else len(payload), name=name
|
||||
)
|
||||
except ExtractionError as exc:
|
||||
return self.reject(name, code=exc.code, reason=str(exc), label=label)
|
||||
try:
|
||||
raw = (
|
||||
base64.b64decode(payload, validate=True)
|
||||
|
|
@ -1380,6 +1392,18 @@ def _pdf_image(stream: object, name: str) -> ExtractedImage:
|
|||
"""
|
||||
from pdfminer.pdftypes import resolve1
|
||||
|
||||
# THE DECLARED SIZE IS READ FIRST, and the stream is not touched until it
|
||||
# is within the bound: `get_data()` decompresses, so a check after it has
|
||||
# already paid for a picture of compressed zeros.
|
||||
declared = dict(getattr(stream, "attrs", {}))
|
||||
check_size(
|
||||
resolve1(declared.get("Width")) if isinstance(resolve1(declared.get("Width")), int) else 0,
|
||||
resolve1(declared.get("Height"))
|
||||
if isinstance(resolve1(declared.get("Height")), int)
|
||||
else 0,
|
||||
name=name,
|
||||
channels=1,
|
||||
)
|
||||
try:
|
||||
data = stream.get_data() # type: ignore[attr-defined]
|
||||
except Exception as exc:
|
||||
|
|
@ -1454,7 +1478,14 @@ def _pdf_images(page: object) -> tuple[tuple[ExtractedImage, ...], tuple[AssetRe
|
|||
# resource name local to one page's dictionary, so it is not an
|
||||
# identifier -- the page number in front of it is what makes the string
|
||||
# readable, and the content-addressed digest is what makes it unique.
|
||||
label = str(drawn.get("name") or index).lstrip("/")
|
||||
# A name pdfminer derived from `id()` is not a name. An inline image
|
||||
# (`BI ... EI`) has no resource name, and pdfminer fills the field with
|
||||
# the address of a Python object, so 0.10.0 wrote a pointer line that
|
||||
# changed between two runs of one build -- two concept files of the
|
||||
# reference corpus differed, which breaks the bit-exact rebuild.
|
||||
# Measured 2026-09-17.
|
||||
raw = str(drawn.get("name") or "").lstrip("/")
|
||||
label = raw if raw and not raw.isdigit() else f"inline-{index}"
|
||||
name = f"page-{number}-{label}"
|
||||
stream = drawn.get("stream")
|
||||
if stream is None:
|
||||
|
|
|
|||
249
tests/test_asset_limits.py
Normal file
249
tests/test_asset_limits.py
Normal file
|
|
@ -0,0 +1,249 @@
|
|||
"""Two findings of the independent 0.10.0 review, as red tests (0.10.1).
|
||||
|
||||
Both land with the SHIPPED defaults (`--assets` on, `--gate
|
||||
guard-trusted-source`), and both are new in 0.10.0 -- before it, no reader
|
||||
read an `<img>` attribute or opened an image stream at all.
|
||||
|
||||
1. **A remote image reference became a LIVE markdown image link** in the
|
||||
persisted concept, carrying an address the document's author controls,
|
||||
query string included. This package opens no socket, but a consumer that
|
||||
renders the markdown or lets an agent fetch images does, which turns "a
|
||||
bundle was opened" into a beacon. The guard's `user-upload` tier refuses
|
||||
such a line and the build's default tier does not, so the same bytes are
|
||||
persisted under the default and refused one tier up.
|
||||
2. **Nothing bounded a PDF image's size.** A 9.6 KB file declaring an
|
||||
8000x8000 grayscale image of compressed zeros made `encode_png` allocate
|
||||
width*height bytes twice over; measured peak RSS 83 MB at 3000x3000 and
|
||||
276 MB at 8000x8000, linear in the pixel count. One such document -- or
|
||||
one legitimately enormous scan -- takes the whole batch build with it,
|
||||
before any gate, because the guard never sees image bytes.
|
||||
|
||||
The limit is READ OFF the corpora rather than chosen: over the 4 828 image
|
||||
objects of the 43-document reference corpus the largest is 4 515 x 4 128
|
||||
(18.6 MP, a landscape drawing), and over R761's 109 delivered pictures the
|
||||
largest is 2 072 x 656 (1.4 MP). `MAX_IMAGE_PIXELS` sits above both with
|
||||
room to spare, and anything larger is a counted refusal rather than a
|
||||
killed build.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
import zlib
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from llm_ingestion_okf import assets, cli
|
||||
from llm_ingestion_okf.assets import IMAGE_POINTER, render_missing
|
||||
from llm_ingestion_okf.errors import ExtractionError
|
||||
from llm_ingestion_okf.extract import extract_document
|
||||
|
||||
FIXTURES = Path(__file__).parent / "fixtures" / "image-inbox"
|
||||
REMOTE = "https://collect.example.net/p.gif?doc=drift&u=SESSION"
|
||||
|
||||
#: Any markdown image whose target is not this bundle's own `assets/`.
|
||||
FOREIGN_IMAGE_LINK = re.compile(r"!\[[^\]\n]*\]\(\s*(?!/assets/)([^)\s]+)")
|
||||
|
||||
|
||||
# --- finding 1: a remote reference is inert ---------------------------------
|
||||
|
||||
|
||||
def test_a_remote_reference_is_not_a_markdown_image_link() -> None:
|
||||
line = render_missing(REMOTE, reason="the source is off this machine", label="fig", href=REMOTE)
|
||||
assert "](" not in line
|
||||
assert REMOTE in line, "the address is still stated -- a reader must see what was there"
|
||||
|
||||
|
||||
def test_the_pointer_block_of_a_carried_image_is_unchanged() -> None:
|
||||
"""The known-positive beside it: a local image keeps its image block, or
|
||||
the fix has merely disarmed the whole capability."""
|
||||
image = assets.read_image((FIXTURES / "graphics" / "figur-84-1.png").read_bytes(), name="f.png")
|
||||
assert IMAGE_POINTER.search(assets.render_block(image)) is not None
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"document",
|
||||
[
|
||||
'<html><body><p>A</p><img src="{ref}" alt="fig"></body></html>',
|
||||
'<standard xmlns:xlink="http://www.w3.org/1999/xlink"><body><sec><title>T</title>'
|
||||
'<p>A</p><graphic xlink:href="{ref}"/></sec></body></standard>',
|
||||
],
|
||||
ids=["html", "sts"],
|
||||
)
|
||||
@pytest.mark.parametrize("ref", [REMOTE, "//collect.example.net/p.gif", "HTTPS://EVIL/p.gif"])
|
||||
def test_no_reader_writes_a_live_link_for_a_remote_reference(document: str, ref: str) -> None:
|
||||
"""A property over the readers that resolve references, not one string."""
|
||||
suffix = ".html" if document.startswith("<html") else ".xml"
|
||||
# `&` is an entity opener in XML, so the reference is escaped for that
|
||||
# reader and not for the HTML one.
|
||||
escaped = ref if suffix == ".html" else ref.replace("&", "&")
|
||||
extracted = extract_document(
|
||||
f"doc{suffix}", document.format(ref=escaped).encode("utf-8"), assets=True
|
||||
)
|
||||
foreign = FOREIGN_IMAGE_LINK.findall(extracted.text)
|
||||
assert foreign == [], f"live image link(s) {foreign} for {ref}"
|
||||
assert ref.split("?")[0].lower() in extracted.text.lower()
|
||||
|
||||
|
||||
def test_a_data_uri_image_is_carried_and_leaves_no_foreign_link() -> None:
|
||||
png = (FIXTURES / "graphics" / "figur-84-1.png").read_bytes()
|
||||
import base64
|
||||
|
||||
uri = "data:image/png;base64," + base64.b64encode(png).decode("ascii")
|
||||
extracted = extract_document(
|
||||
"doc.html", f'<html><body><img src="{uri}"></body></html>'.encode(), assets=True
|
||||
)
|
||||
assert len(extracted.images) == 1
|
||||
assert FOREIGN_IMAGE_LINK.findall(extracted.text) == []
|
||||
|
||||
|
||||
def test_a_built_bundle_carries_no_foreign_image_link(tmp_path: Path) -> None:
|
||||
"""The shipped fixture inbox holds one remote `<img>`; the bundle must
|
||||
point at `assets/` and nowhere else."""
|
||||
pytest.importorskip("pdfplumber")
|
||||
pytest.importorskip("pypandoc")
|
||||
bundle = tmp_path / "bundle"
|
||||
code = cli.main(
|
||||
[
|
||||
"build",
|
||||
str(FIXTURES),
|
||||
"--bundle",
|
||||
str(bundle),
|
||||
"--bundle-id",
|
||||
"limits",
|
||||
"--okf-version",
|
||||
"0.2",
|
||||
]
|
||||
)
|
||||
assert code == 0
|
||||
foreign: list[str] = []
|
||||
for path in bundle.rglob("*.md"):
|
||||
foreign += FOREIGN_IMAGE_LINK.findall(path.read_text(encoding="utf-8"))
|
||||
assert foreign == []
|
||||
|
||||
|
||||
# --- finding 2: a declared size that is too large is refused, not decoded ---
|
||||
|
||||
|
||||
def _bomb(dimension: int, content: bytes | None = None) -> bytes:
|
||||
"""A tiny PDF declaring one `dimension` x `dimension` grayscale image of
|
||||
compressed zeros -- the review's own repro, built here. `content` replaces
|
||||
the page's content stream, for a page that draws an INLINE image instead."""
|
||||
payload = zlib.compress(b"\x00" * (dimension * dimension), 9)
|
||||
if content is None:
|
||||
content = b"BT /F1 12 Tf 20 100 Td (bomb) Tj ET\nq 100 0 0 100 20 20 cm /Im0 Do Q\n"
|
||||
objects = [
|
||||
b"<< /Type /Catalog /Pages 2 0 R >>",
|
||||
b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>",
|
||||
b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Resources << /XObject "
|
||||
b"<< /Im0 5 0 R >> /Font << /F1 6 0 R >> >> /Contents 4 0 R >>",
|
||||
b"<< /Length %d >>\nstream\n" % len(content) + content + b"\nendstream",
|
||||
(
|
||||
"<< /Type /XObject /Subtype /Image /Width %d /Height %d /ColorSpace /DeviceGray "
|
||||
"/BitsPerComponent 8 /Filter /FlateDecode /Length %d >>\nstream\n"
|
||||
% (dimension, dimension, len(payload))
|
||||
).encode("ascii")
|
||||
+ payload
|
||||
+ b"\nendstream",
|
||||
b"<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>",
|
||||
]
|
||||
out = bytearray(b"%PDF-1.4\n")
|
||||
offsets = []
|
||||
for number, body in enumerate(objects, start=1):
|
||||
offsets.append(len(out))
|
||||
out += b"%d 0 obj\n" % number + body + b"\nendobj\n"
|
||||
start = len(out)
|
||||
out += b"xref\n0 %d\n0000000000 65535 f \n" % (len(objects) + 1)
|
||||
for offset in offsets:
|
||||
out += b"%010d 00000 n \n" % offset
|
||||
out += b"trailer\n<< /Size %d /Root 1 0 R >>\nstartxref\n%d\n%%%%EOF\n" % (
|
||||
len(objects) + 1,
|
||||
start,
|
||||
)
|
||||
return bytes(out)
|
||||
|
||||
|
||||
def test_the_limit_is_above_every_image_measured_in_the_corpora() -> None:
|
||||
"""4 515 x 4 128 = 18.6 MP is the largest of the 4 828 objects measured in
|
||||
the reference corpus; R761's largest delivered picture is 1.4 MP."""
|
||||
assert assets.MAX_IMAGE_PIXELS > 4515 * 4128
|
||||
assert assets.MAX_IMAGE_BYTES >= assets.MAX_IMAGE_PIXELS
|
||||
|
||||
|
||||
def test_a_pdf_image_over_the_limit_is_refused_by_code() -> None:
|
||||
pytest.importorskip("pdfplumber")
|
||||
dimension = 1 + int(assets.MAX_IMAGE_PIXELS**0.5)
|
||||
extracted = extract_document("bomb.pdf", _bomb(dimension), assets=True)
|
||||
assert extracted.images == ()
|
||||
assert [rejection.code for rejection in extracted.rejected] == ["asset_too_large"]
|
||||
assert str(dimension) in extracted.rejected[0].reason
|
||||
|
||||
|
||||
def test_a_pdf_image_under_the_limit_is_still_carried() -> None:
|
||||
"""The boundary from the other side, on the same generator."""
|
||||
pytest.importorskip("pdfplumber")
|
||||
extracted = extract_document("small.pdf", _bomb(64), assets=True)
|
||||
assert [(image.width, image.height) for image in extracted.images] == [(64, 64)]
|
||||
|
||||
|
||||
def test_the_refusal_happens_before_the_stream_is_decompressed() -> None:
|
||||
"""The limit is read off the DECLARED size, and the order is observable.
|
||||
|
||||
The image stream here is CORRUPT (its bytes are not deflate data) while
|
||||
its declared size is over the bound. Decoding first gives
|
||||
`asset_pdf_unsupported` ("could not be decoded"); reading the declared
|
||||
size first gives `asset_too_large`. A check after `get_data()` has already
|
||||
paid for the bomb it was meant to stop.
|
||||
"""
|
||||
pytest.importorskip("pdfplumber")
|
||||
dimension = 1 + int(assets.MAX_IMAGE_PIXELS**0.5)
|
||||
document = _bomb(dimension)
|
||||
payload = zlib.compress(b"\x00" * (dimension * dimension), 9)
|
||||
corrupt = document.replace(payload, b"\xff" * len(payload))
|
||||
assert corrupt != document
|
||||
extracted = extract_document("corrupt.pdf", corrupt, assets=True)
|
||||
assert [rejection.code for rejection in extracted.rejected] == ["asset_too_large"]
|
||||
|
||||
|
||||
def test_a_data_uri_over_the_limit_is_refused_before_decoding(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.setattr(assets, "MAX_IMAGE_BYTES", 32)
|
||||
uri = "data:image/png;base64," + "A" * 4096
|
||||
extracted = extract_document(
|
||||
"doc.html", f'<html><body><img src="{uri}"></body></html>'.encode(), assets=True
|
||||
)
|
||||
assert [rejection.code for rejection in extracted.rejected] == ["asset_too_large"]
|
||||
|
||||
|
||||
def test_encode_png_refuses_the_same_size_on_its_own() -> None:
|
||||
"""Defence in depth: the encoder does not trust its caller to have checked."""
|
||||
dimension = 1 + int(assets.MAX_IMAGE_PIXELS**0.5)
|
||||
with pytest.raises(ExtractionError) as excinfo:
|
||||
assets.encode_png(dimension, dimension, b"", channels=1, palette=None, alpha=None)
|
||||
assert excinfo.value.code == "asset_too_large"
|
||||
|
||||
|
||||
# --- the determinism defect PM added to this order --------------------------
|
||||
|
||||
|
||||
def test_an_inline_pdf_image_gets_a_stable_name() -> None:
|
||||
"""pdfminer names an inline image (`BI ... EI`) from `id()` of a Python
|
||||
object, so the pointer line changed between two runs of one build -- two
|
||||
concept files of the reference corpus differed. Measured 2026-09-17."""
|
||||
pytest.importorskip("pdfplumber")
|
||||
from llm_ingestion_okf import extract as extract_module
|
||||
|
||||
inline = (
|
||||
b"BT /F1 12 Tf 20 100 Td (t) Tj ET\n"
|
||||
b"q 10 0 0 10 20 20 cm BI /W 2 /H 2 /CS /G /BPC 8 /F /AHx ID 00112233> EI Q\n"
|
||||
)
|
||||
document = _bomb(4, content=inline)
|
||||
first = extract_document("inline.pdf", document, assets=True)
|
||||
extract_module._pdf_pages.cache_clear()
|
||||
second = extract_document("inline.pdf", document, assets=True)
|
||||
names = [image.name for image in first.images] + [r.name for r in first.rejected]
|
||||
again = [image.name for image in second.images] + [r.name for r in second.rejected]
|
||||
assert names == again != []
|
||||
assert not any(part.isdigit() and len(part) > 6 for name in names for part in name.split("-"))
|
||||
|
|
@ -355,3 +355,16 @@ def test_the_readme_states_that_image_bytes_are_not_screened() -> None:
|
|||
"""
|
||||
text = README.read_text(encoding="utf-8")
|
||||
assert "image bytes are not screened" in text.lower()
|
||||
|
||||
|
||||
_MAX_PIXELS_LINE = re.compile(r"^<!-- asset-max-pixels: (\d+) -->$", re.MULTILINE)
|
||||
|
||||
|
||||
def test_the_readme_publishes_the_pixel_bound_the_code_applies() -> None:
|
||||
"""A published bound is a test obligation: the number in the README is the
|
||||
number that refuses an image."""
|
||||
from llm_ingestion_okf.assets import MAX_IMAGE_PIXELS
|
||||
|
||||
match = _MAX_PIXELS_LINE.search(README.read_text(encoding="utf-8"))
|
||||
assert match is not None, "README carries no `<!-- asset-max-pixels: N -->` marker"
|
||||
assert int(match.group(1)) == MAX_IMAGE_PIXELS
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue