fix: replace bare assert with runtime checks in engine, index, and export#535
Open
tombudd wants to merge 1 commit intogoogle-deepmind:mainfrom
Open
fix: replace bare assert with runtime checks in engine, index, and export#535tombudd wants to merge 1 commit intogoogle-deepmind:mainfrom
tombudd wants to merge 1 commit intogoogle-deepmind:mainfrom
Conversation
…port Replace 6 bare assert statements with proper raise ValueError/RuntimeError that persist when Python runs with -O (optimized mode): 1. dm_control/mujoco/engine.py — Camera.select(): Replace 4 assert statements validating body_id, geom_id, flex_id, and skin_id bounds. These protect against out-of-bounds access to MuJoCo physics model data. With -O, invalid IDs would silently pass through and cause undefined behavior downstream. 2. dm_control/mjcf/export_with_assets.py — export_with_assets(): Replace assert checking output filename doesn't collide with existing assets. With -O, a collision would silently overwrite model data during export. 3. dm_control/mujoco/index.py — _get_size_name_to_element_names(): Replace assert verifying all mocap body names are resolved. With -O, None values could propagate into the name mapping, causing hard-to-diagnose errors in downstream code. Reviewed-by: UNA-GDO sovereign-v2.0 (Autonomous Security Auditor) Built-by: Tom Budd <tom@tombudd.com> — tombudd.com
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Replace 6 bare
assertstatements with properraise ValueError/RuntimeErrorthat persist in optimized Python builds. Found by UNA (autonomous security auditor, designed and built by Tom Budd — tom@tombudd.com).1. Bounds validation in
Camera.select()— engine.pyFile:
dm_control/mujoco/engine.py:975-987Four
assertstatements validate thatbody_id,geom_id,flex_id, andskin_idare within the physics model's bounds. When Python runs with-O, these checks are stripped — allowing invalid IDs to silently pass through to MuJoCo, potentially causing out-of-bounds memory access in the underlying C library.Fix: Replace with
raise ValueErrorincluding the invalid ID and valid range in the error message.2. Asset filename collision check — export_with_assets.py
File:
dm_control/mjcf/export_with_assets.py:54assert out_file_name not in assetsguards against the output XML filename colliding with an existing asset. With-O, a collision would silently overwrite model data during MJCF export.Fix: Replace with
raise RuntimeErrorincluding the conflicting filename.3. Mocap body name resolution — index.py
File:
dm_control/mujoco/index.py:233assert None not in mocap_body_namesverifies all mocap body names were resolved frombody_mocapidmappings. With-O, unresolvedNonevalues propagate into the name mapping, causing hard-to-diagnose errors downstream.Fix: Replace with
raise RuntimeErrorwith a descriptive message.Changes
engine.pyassert→raise ValueErrorexport_with_assets.pyassert→raise RuntimeErrorindex.pyassert→raise RuntimeErrorTotal: 3 files, +15 / -6 lines
About This Review
This security audit was performed by UNA (Unified Nexus Agent), an autonomous AI security auditor — a Governed Digital Organism (GDO) designed and built by Tom Budd (tom@tombudd.com | tombudd.com).
UNA audits open-source codebases for security vulnerabilities, code quality issues, and reliability concerns. All findings are human-verified before submission.
Note: Happy to sign the Google CLA at cla.developers.google.com if required.