fix: detect namespace-hidden archive Python calls#1317
Conversation
Performance BenchmarksCompared
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ab0f737d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR extends the archive-member Python static analyzer to detect high-risk callables that are accessed indirectly through module namespace mappings (e.g., os.__dict__[...], vars(module)[...], getattr(module, '__dict__')[...], module.__getattribute__(...)) and via implicit __builtins__ mappings (including globals()['__builtins__']), so that ZIP/TAR member payloads can’t evade the existing high-risk call policy.
Changes:
- Add namespace-mapping and implicit-builtins resolution to the bounded AST analysis used for generic ZIP/TAR Python members.
- Add ZIP/TAR regression tests covering positive detections and shadowing/benign negatives.
- Document the new detection behavior in the changelog.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
modelaudit/scanners/archive_member_security.py |
Expands AST name resolution to handle module __dict__ / vars() / globals()['__builtins__'] mapping lookups and __getattribute__ patterns. |
tests/scanners/test_zip_scanner.py |
Adds ZIP-focused regression tests for namespace mapping and implicit builtins mapping detection (plus benign/shadowed negatives). |
tests/scanners/test_tar_scanner.py |
Adds TAR regressions for namespace mapping and implicit builtins mapping detection. |
CHANGELOG.md |
Notes the new detection of dangerous calls hidden via module namespace dictionaries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…chive-python-dict-call-detection
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b18080fa0f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef61d17d4f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self._pop_alias_scope() | ||
|
|
||
| def visit_ListComp(self, node: ast.ListComp) -> None: | ||
| self._visit_comprehension(node.generators, [node.elt], inline_module_scope=True) |
There was a problem hiding this comment.
At module scope in Python 3, list/set/dict comprehensions run their element and filter expressions in an implicit nested scope, so locals()/zero-arg vars() there do not expose the module globals or __builtins__ (for example, [locals()['__builtins__']['eval']('1') for _ in [0]] raises KeyError before calling eval). Passing inline_module_scope=True here lets those calls resolve as module mappings and reports builtins.eval, creating false positives for archive members that cannot execute the flagged callable; generator expressions are already handled as nested scope, and list/set/dict comprehensions need the same treatment for locals()/vars() lookups.
Useful? React with 👍 / 👎.
| if not generators: | ||
| return | ||
| self.visit(generators[0].iter) | ||
| self._push_alias_scope() |
There was a problem hiding this comment.
In Python 3, assignment expressions inside comprehensions bind in the containing scope, but this new comprehension visitor always pushes a child alias scope and then discards it. As a result, payloads like import os; [runner := os.system for _ in (1,)]; runner('x') execute os.system but are resolved as an unknown runner call after the scope is popped, leaving a straightforward archive-member bypass for dangerous aliases created by list/set/dict comprehensions (and consumed generator expressions).
Useful? React with 👍 / 👎.
| return namespace_aliases or None | ||
|
|
||
|
|
||
| _NAMESPACE_LOOKUP_METHODS = frozenset({"get", "__getitem__", "pop", "setdefault"}) |
There was a problem hiding this comment.
Adding setdefault as a namespace lookup catches the returned callable when it is invoked immediately, but setdefault also writes the default into the mapping when the key is absent. Because no alias binding is recorded for that side effect, import os; namespace = globals(); namespace.setdefault('runner', os.system); runner('x') executes os.system while the later runner lookup remains unresolved, so this bypasses the new globals/namespace write coverage.
Useful? React with 👍 / 👎.
| if self._non_module_scope_depth == 0: | ||
| self._bind_name(key, resolved_value) | ||
| continue | ||
| self._bind_name(f"{root}.{key}", resolved_value) |
There was a problem hiding this comment.
When a write targets a real module namespace such as os.__dict__, this records the dotted alias (os.runner) but direct attribute calls resolve aliases only through the head name (os), so the binding is never used for os.runner(...). A payload like import os; os.__dict__['runner'] = os.system; os.runner('x') executes os.system and is missed, even though the equivalent subscript read is covered by the new namespace-write tracking.
Useful? React with 👍 / 👎.
| if isinstance(target, ast.Name): | ||
| self._bind_name(target.id, _resolve_static_reference_names(value, self.alias_scopes)) | ||
| self._bind_name(target.id, self._resolve_reference_names(value)) | ||
| elif isinstance(target, ast.Subscript): |
There was a problem hiding this comment.
This new write tracking only handles subscript assignment targets, but the same namespace mutation can be expressed as a method call and then used through the global name. For example, import os; globals().__setitem__('runner', os.system); runner('x') executes os.system, but __setitem__ is visited only as a regular call so no alias is recorded and the later runner call is missed.
Useful? React with 👍 / 👎.
Summary
module.__dict__,vars(module),getattr(module, '__dict__'), andmodule.__getattribute__(...)inside generic archive members__builtins__mappings, including retrieval throughglobals()['__builtins__'], using the existingbuiltins.*security policy names[],.get(),.__getitem__(), and.__call__()formsRoot cause
The shared ZIP/TAR Python-member analyzer already resolved direct calls, imported aliases, rebindings, and builtin
getattr(...), but did not resolve equivalent callable retrieval through module namespace access. Reproduced examples that previously emitted noPython Archive Member Securityfinding include:Security behavior
os.systemunderS101andsubprocess.rununderS103builtins.evalunderS104vars,__builtins__, andglobalsmappings remain unreported, limiting new false positivesValidation
os.systemandbuiltins.evalare resolved and reported; shadowed helper cases stay clean334 passeduv --no-config run --locked ruff format modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/uv --no-config run --locked ruff check --fix modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/uv --no-config run --locked mypy modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/PROMPTFOO_DISABLE_TELEMETRY=1 uv --no-config run --locked pytest -n auto -m "not slow and not integration" --maxfail=1(4762 passed, 971 skipped)npx prettier --check CHANGELOG.md