fix: safely suppress verified Hugging Face model-card image examples #1791
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2338,24 +2338,216 @@ def _is_valid_official_readme_sample_image_example(example: bytes) -> bool: | |
| url = bindings[0][1] | ||
| else: | ||
| return False | ||
| if not _is_official_huggingface_documented_image_url(url): | ||
| return False | ||
| return True | ||
|
|
||
|
|
||
| def _is_official_huggingface_documented_image_url(url: str) -> bool: | ||
| """Return whether a URL is a bounded HTTPS fetch of a documented huggingface.co image.""" | ||
| try: | ||
| parsed = urlsplit(url) | ||
| port = parsed.port | ||
| except ValueError: | ||
| return False | ||
| segments = parsed.path.split("/") | ||
| return not ( | ||
| parsed.scheme != "https" | ||
| or parsed.hostname != "huggingface.co" | ||
| or parsed.username is not None | ||
| or parsed.password is not None | ||
| or port is not None | ||
| or parsed.netloc.lower() != parsed.hostname | ||
| or parsed.fragment | ||
| or parsed.query not in {"", "download=true"} | ||
| or "resolve" not in segments | ||
| or any(segment in {".", ".."} for segment in segments) | ||
| or not parsed.path.lower().endswith(_DOCUMENTED_IMAGE_SUFFIXES) | ||
| ) | ||
|
|
||
|
|
||
| @lru_cache(maxsize=1) | ||
| def official_readme_urlopen_image_example_spans(data: bytes) -> tuple[tuple[int, int], ...]: | ||
| """Return byte spans of Python fences whose only ``urlopen`` use fetches a documented image. | ||
|
|
||
| Model-card generators (notably ``timm``) emit a fixed ``Image.open(urlopen(<literal URL>))`` | ||
| snippet, so the documented shape is proven structurally instead of by pinning whole-file | ||
| digests. Callers use the returned spans to decide whether an individual ``urllib``/``urlopen`` | ||
| finding sits inside a proven-inert example. | ||
|
|
||
| Results are memoised for the payload currently being classified, because every candidate | ||
| finding in one file re-checks the same spans. | ||
| """ | ||
| if b"urlopen" not in data: | ||
| return () | ||
|
|
||
| spans: list[tuple[int, int]] = [] | ||
| cursor = 0 | ||
| while opening := _PYTHON_README_FENCE_PATTERN.search(data, cursor): | ||
| if len(spans) >= _MAX_README_IMAGE_EXAMPLE_FENCES: | ||
| return () | ||
| closing = _matching_readme_image_fence_end( | ||
| data, | ||
| opening, | ||
| opening.end(), | ||
| min(len(data), opening.end() + _MAX_README_IMAGE_EXAMPLE_BYTES), | ||
| ) | ||
| if closing is None: | ||
| next_cursor = min(len(data), opening.end() + _MAX_README_IMAGE_EXAMPLE_BYTES) | ||
| if next_cursor <= cursor: | ||
| break | ||
| cursor = next_cursor | ||
| continue | ||
| if _is_official_readme_urlopen_image_example(data[opening.end() : closing.start()]): | ||
| spans.append((opening.end(), closing.start())) | ||
| cursor = closing.end() | ||
|
|
||
| # A proven fence only speaks for itself. If any urllib reference in the file sits outside a | ||
| # proven fence - a second fence, prose, or trailing payload - the whole file stays actionable, | ||
| # mirroring how the `requests` example tracks unvalidated references. | ||
| # | ||
| # Guard on the bare substrings `urllib` and `urlopen` rather than on the specific tokens the | ||
| # findings report. The detector emits a single `network_library: urllib` finding per file and | ||
| # then retargets it to the earliest urllib token, so a narrower guard lets an unrelated | ||
| # `import urllib.request` + `urllib.request.build_opener()` fence inherit the benign example's | ||
| # position and be downgraded with it. | ||
| if not spans or _tokens_appear_outside_spans(data, (b"urllib", b"urlopen"), spans): | ||
| return () | ||
| return tuple(spans) | ||
|
|
||
|
|
||
| def _tokens_appear_outside_spans( | ||
| data: bytes, | ||
| tokens: tuple[bytes, ...], | ||
| spans: list[tuple[int, int]], | ||
| ) -> bool: | ||
| """Return whether any token occurrence falls outside every proven span.""" | ||
| for token in tokens: | ||
| position = data.find(token) | ||
| while position >= 0: | ||
| end = position + len(token) | ||
| if not any(start <= position and end <= stop for start, stop in spans): | ||
| return True | ||
| position = data.find(token, end) | ||
| return False | ||
|
|
||
|
|
||
| # Bare-name execution primitives. Attribute access is checked separately and far more narrowly: | ||
| # documented model cards legitimately call `model.eval()` (PyTorch eval mode), which has nothing to | ||
| # do with the `eval` builtin. | ||
| _DOCUMENTED_EXAMPLE_FORBIDDEN_NAMES = frozenset( | ||
| { | ||
| "__import__", | ||
| "compile", | ||
| "delattr", | ||
| "eval", | ||
| "exec", | ||
| "getattr", | ||
| "globals", | ||
| "locals", | ||
| "setattr", | ||
| "vars", | ||
| } | ||
| ) | ||
| _DOCUMENTED_EXAMPLE_FORBIDDEN_ATTRIBUTES = frozenset({"popen", "system"}) | ||
|
|
||
|
|
||
| def _is_official_readme_urlopen_image_example(example: bytes) -> bool: | ||
| """Return whether every ``urlopen`` use in one fence is a documented sample-image fetch.""" | ||
| if b"urlopen" not in example or len(example) > _MAX_README_IMAGE_EXAMPLE_BYTES: | ||
| return False | ||
| try: | ||
| tree = ast.parse(example.decode("utf-8")) | ||
| except (SyntaxError, UnicodeDecodeError, RecursionError, ValueError): | ||
| return False | ||
|
|
||
| nodes: list[ast.AST] = [] | ||
| for node in ast.walk(tree): | ||
| if len(nodes) >= _MAX_README_IMAGE_EXAMPLE_AST_NODES: | ||
| return False | ||
| nodes.append(node) | ||
| parents = {id(child): parent for parent in nodes for child in ast.iter_child_nodes(parent)} | ||
|
|
||
| imports = [ | ||
| node | ||
| for node in nodes | ||
| if isinstance(node, ast.ImportFrom) and node.level == 0 and node.module == "urllib.request" | ||
| ] | ||
| if len(imports) != 1 or len(imports[0].names) != 1: | ||
| return False | ||
| alias = imports[0].names[0] | ||
| if alias.name != "urlopen" or alias.asname is not None: | ||
| return False | ||
|
|
||
| # `Image` must provably be PIL's. Without this the response sink is only checked by name, so a | ||
| # fence can define its own `class Image` whose `open` executes the downloaded bytes and still | ||
| # be treated as the documented example. | ||
| pil_image_imports = [ | ||
| node | ||
| for node in nodes | ||
| if isinstance(node, ast.ImportFrom) | ||
| and node.level == 0 | ||
| and node.module == "PIL" | ||
| and any(entry.name == "Image" for entry in node.names) | ||
| ] | ||
| if len(pil_image_imports) != 1 or any( | ||
| entry.asname is not None for entry in pil_image_imports[0].names if entry.name == "Image" | ||
| ): | ||
| return False | ||
|
|
||
| urlopen_calls: list[ast.Call] = [] | ||
| for node in nodes: | ||
| # Neither `urlopen` nor `Image` may be rebound, shadowed, aliased, or reached via attribute. | ||
| if isinstance(node, ast.Attribute) and node.attr == "urlopen": | ||
| return False | ||
| if isinstance(node, ast.arg) and node.arg in {"urlopen", "Image"}: | ||
| return False | ||
| if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)) and node.name in { | ||
| "urlopen", | ||
| "Image", | ||
| }: | ||
| return False | ||
| if isinstance(node, ast.alias) and node.asname == "urlopen" and node is not alias: | ||
| return False | ||
| if isinstance(node, ast.alias) and node.asname == "Image": | ||
| return False | ||
| if isinstance(node, ast.Name) and node.id == "Image" and not isinstance(node.ctx, ast.Load): | ||
| return False | ||
|
Comment on lines
+2514
to
+2515
There was a problem hiding this comment.
When a model-card fence imports AGENTS.md reference: AGENTS.md:L25-L25 Useful? React with 👍 / 👎. |
||
| # Execution primitives inside a fence we are about to call inert. | ||
| if isinstance(node, ast.Name) and node.id in _DOCUMENTED_EXAMPLE_FORBIDDEN_NAMES: | ||
| return False | ||
| if isinstance(node, ast.Attribute) and ( | ||
| node.attr in _DOCUMENTED_EXAMPLE_FORBIDDEN_ATTRIBUTES or node.attr.startswith("__") | ||
| ): | ||
| return False | ||
| if not isinstance(node, ast.Name) or node.id != "urlopen": | ||
| continue | ||
| parent = parents.get(id(node)) | ||
| if not isinstance(node.ctx, ast.Load) or not isinstance(parent, ast.Call) or parent.func is not node: | ||
| return False | ||
| urlopen_calls.append(parent) | ||
| if not urlopen_calls: | ||
| return False | ||
|
|
||
| for call in urlopen_calls: | ||
| if call.keywords or len(call.args) != 1: | ||
| return False | ||
| argument = call.args[0] | ||
| if not isinstance(argument, ast.Constant) or not isinstance(argument.value, str): | ||
| return False | ||
| if not _is_official_huggingface_documented_image_url(argument.value): | ||
| return False | ||
| # The response must flow straight into `Image.open(...)` and nowhere else. | ||
| parent = parents.get(id(call)) | ||
| if ( | ||
| not isinstance(parent, ast.Call) | ||
| or parent.keywords | ||
| or len(parent.args) != 1 | ||
| or parent.args[0] is not call | ||
| or not isinstance(parent.func, ast.Attribute) | ||
| or parent.func.attr != "open" | ||
| or not isinstance(parent.func.value, ast.Name) | ||
| or parent.func.value.id != "Image" | ||
| ): | ||
| return False | ||
| return True | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a Markdown file contains many invalid Python fences mentioning
urlopen,len(spans)remains zero, so this loop can parse and walk every fence despite_MAX_README_IMAGE_EXAMPLE_FENCES. Since TextScanner accepts documentation payloads up to 100 MiB, an attacker-controlled model card can contain hundreds of thousands of small near-match fences and impose excessive CPU work during a scan. Track the number of encountered fence openings, as_index_official_readme_sample_image_fencesalready does, and fail closed after the configured limit.Useful? React with 👍 / 👎.