mawei/sunnypilot
1
0
Fork 0
mirror of https://github.com/sunnypilot/sunnypilot.git synced 2026-11-07 01:13:45 +08:00
Code Issues Packages Projects Releases Wiki Activity

Accept official wrapped model metadata without weakening protocol validation

Browse Source 
The exact RL navigation ONNX stores output_slices as newline-wrapped Base64.
The installer used strict Base64 decoding without removing format whitespace,
so it rejected the successfully compiled model. Synthetic metadata fixtures
were unwrapped and missed the bug.

Normalize only ASCII whitespace before strict decoding, keeping the restricted
unpickler and all shape, dtype, device, state-pair and action-slice checks. Add
an exact-source metadata fixture and run it through the actual plain artifact
writer, reader and installed-bundle validator. Keep one rejected completed build
and diagnostic snapshot outside normal registration for future investigation.

Validation: 14 model/metadata tests, 18 preparation tests including real cereal,
3 real worker subprocess tests; Ruff and diff checks pass. Invalid encoding,
executable metadata and incompatible slices remain rejected. No steering,
cooperative steering, action conversion or compiled-model source changes.
This commit is contained in:
Isaac Barham
2026-11-03 14:26:16 -05:00
parent 6bbb0e62ac
commit bf7bf7475e
6 changed files with 407 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
openpilot/sunnypilot/models/NAVIGATION_MODEL.md
+5
View File
@@ -21,6 +21,11 @@ storage. Missing device dependencies produce an error; this feature does not
install system libraries. The compiler log is
`Paths.model_root()/rl_nav_80eedcdd_build.log`. An interrupted verified download
can be reused on the next attempt; incomplete downloads are never accepted.
If a completed compilation fails protocol validation, its artifact, descriptor
and `validation_failure.json` are retained in the single
`Paths.model_root()/.rl_nav_failed_build` directory for diagnosis. A later rejected
build replaces that directory. It is outside normal model registration and is
never automatically selected or reused as a validated model.
## Exact checkpoint and build
openpilot/sunnypilot/models/build_navigation_model.py
+29 -1
View File
@@ -19,12 +19,38 @@ from openpilot.sunnypilot.models.navigation_model import (MANIFEST_NAME, SOURCE_
COMPILE_ENV = {'DEBUG': '1', 'DEV': 'USB+AMD:LLVM', 'FRAME_DEV': 'CPU', 'FLOAT16': '1',
'GMMU': '0', 'TC_OPT': '2', 'TC_MIN_GLOBALS': '32'}
COMPILER_SHA256 = '0ff7415619aa267914f858c24b3f3c32583f75aceebd89ec4f56e70e9daf32ac'
FAILED_BUILD_DIRECTORY = '.rl_nav_failed_build'
class PreparationCancelled(RuntimeError):
pass
def preserve_failed_build(temporary, root, reason):
"""Keep one completed rejected build for inspection, outside the selector root."""
diagnostic = {'reason': reason, 'sourceOnnxSha256': SOURCE_ONNX_SHA256, 'compilerSha256': COMPILER_SHA256,
'registered': False, 'automaticReuseAllowed': False}
try:
descriptor = json.loads((temporary / MANIFEST_NAME).read_text())
artifact = temporary / descriptor['artifact']['fileName']
diagnostic['artifact'] = descriptor['artifact'] | {'bytes': artifact.stat().st_size}
from openpilot.sunnypilot.modeld_v2.helpers import read_parts
_parts, extras = read_parts(str(artifact))
metadata = {key: extras.get(key) for key in ('input_specs', 'output_specs', 'metadata')}
encoded = json.dumps(metadata, default=repr)
diagnostic['compiledMetadata'] = metadata if len(encoded) <= 65536 else {'truncated': True, 'head': encoded[:65536]}
except Exception as error:
diagnostic['diagnosticReadError'] = str(error)[:2048]
(temporary / 'validation_failure.json').write_text(json.dumps(diagnostic, indent=2, default=repr) + '\n')
failed = root / FAILED_BUILD_DIRECTORY
if failed.is_symlink():
failed.unlink()
elif failed.exists():
shutil.rmtree(failed)
temporary.replace(failed)
return failed
def check_allowed(params, queued):
if not params.get_bool('IsOffroad'):
raise PreparationCancelled('Stopped because the device is not confirmed offroad. Park before preparing again.')
@@ -145,7 +171,9 @@ def prepare(root, basedir, params, hardware, *, queued=False, onnx=None, report=
output.rename(temporary / name)
(temporary / MANIFEST_NAME).write_text(json.dumps(descriptor, indent=2) + '\n')
if installed_bundle(temporary) is None:
raise RuntimeError('Compiled model failed its plain-format/navigation protocol checks. No model was registered.')
reason = 'Compiled model failed its plain-format/navigation protocol checks.'
failed = preserve_failed_build(temporary, root, reason)
raise RuntimeError(f'{reason} No model was registered. Completed build and validation details kept in {failed.name}; see the build log.')
check()
(temporary / name).replace(root / name)
check()
openpilot/sunnypilot/models/navigation_model.py
+3
View File
@@ -67,6 +67,9 @@ def compatible_metadata(extras):
if tuple(outputs[name]) != shape or tuple(actual[0]) != shape or np.dtype(actual[1]) != dtype or 'AMD' not in actual[2]:
return False
encoded = extras['metadata']['metadata']['output_slices']
# Upstream ONNX metadata uses wrapped Base64. Permit only ASCII whitespace
# around its encoded bytes; keep strict decoding and restricted unpickling.
encoded = encoded.translate(str.maketrans('', '', ' \t\r\n\v\f'))
slices = _SliceMetadata(io.BytesIO(base64.b64decode(encoded, validate=True))).load()
required = {'plan': slice(917, 1907), 'action': slice(2062, 2066),
'desire_state': slice(2054, 2062), 'desire_pred': slice(855, 887)}
openpilot/sunnypilot/models/tests/fixtures/rl_nav_80eedcdd_metadata.json
Vendored
+264
View File
@@ -0,0 +1,264 @@
{
"provenance": {
"sourceRepository": "commaai/openpilot",
"sourceRef": "afbf2e547e9319e2eb9f515e1e4330af63705cec",
"sourceModelPath": "openpilot/selfdrive/modeld/models/big_driving_supercombo.onnx",
"officialOnnxSha256": "1d18c42ea4cd95c8bffab4635844b78b8e2d4e25b9de44a9c5ac2e0961371a2e",
"officialOnnxBytes": 814591106,
"sourceCheckpoint": "cf8f6120-49ee-48f8-b2d7-d663a4708271/56320/80eedcdd-f97b-46d0-9b94-780685292acf/12864",
"extractedIoMetadataSha256": "10b55d548b2f662f214c5e7c52c5e7864b18675b295ac51064661dd207e93e39",
"scope": "Graph declarations and literal metadata only; no model weights or executable pickle."
},
"input_specs": {
"new_img": [
[
2,
6,
128,
256
],
"uint8",
"AMD"
],
"desire": [
[
8
],
"float32",
"AMD"
],
"traffic_convention": [
[
1,
2
],
"float32",
"AMD"
],
"action_t": [
[
1,
2
],
"float32",
"AMD"
],
"nav_map": [
[
1,
1,
256,
256
],
"float32",
"AMD"
],
"navigation": [
[
1,
47
],
"float32",
"AMD"
],
"state_img_q": [
[
2,
5,
6,
128,
256
],
"uint8",
"AMD"
],
"state_desire_q": [
[
132,
1,
8
],
"float32",
"AMD"
],
"state_feat_q": [
[
128,
1,
16384
],
"float32",
"AMD"
],
"state_map_features_q": [
[
128,
1,
64
],
"float32",
"AMD"
],
"state_navigation_features_q": [
[
128,
1,
64
],
"float32",
"AMD"
]
},
"output_specs": {
"outputs": [
[
1,
18580
],
"float32",
"AMD"
],
"next_state_img_q": [
[
2,
5,
6,
128,
256
],
"uint8",
"AMD"
],
"next_state_desire_q": [
[
132,
1,
8
],
"float32",
"AMD"
],
"next_state_feat_q": [
[
128,
1,
16384
],
"float32",
"AMD"
],
"next_state_map_features_q": [
[
128,
1,
64
],
"float32",
"AMD"
],
"next_state_navigation_features_q": [
[
128,
1,
64
],
"float32",
"AMD"
]
},
"metadata": {
"metadata": {
"output_slices": "gASV0gEAAAAAAAB9lCiMCmxhbmVfbGluZXOUjAhidWlsdGluc5SMBXNsaWNllJOUSwBNEAJOh5RS\nlIwPbGFuZV9saW5lc19wcm9ilGgETRACTRgCToeUUpSMCnJvYWRfZWRnZXOUaARNGAJNIANOh5RS\nlIwEbWV0YZRoBE0gA01XA06HlFKUjAtkZXNpcmVfcHJlZJRoBE1XA013A06HlFKUjARwb3NllGgE\nTXcDTYMDToeUUpSMFndpZGVfZnJvbV9kZXZpY2VfZXVsZXKUaARNgwNNiQNOh5RSlIwOcm9hZF90\ncmFuc2Zvcm2UaARNiQNNlQNOh5RSlIwEcGxhbpRoBE2VA01zB06HlFKUjARsZWFklGgETXMHTQMI\nToeUUpSMCWxlYWRfcHJvYpRoBE0DCE0GCE6HlFKUjAxkZXNpcmVfc3RhdGWUaARNBghNDghOh5RS\nlIwGYWN0aW9ulGgETQ4ITRIIToeUUpSMDGhpZGRlbl9zdGF0ZZRoBE0SCE0SSE6HlFKUjAxtYXBf\nZmVhdHVyZXOUaARNEkhNUkhOh5RSlIwTbmF2aWdhdGlvbl9mZWF0dXJlc5RoBE1SSE2SSE6HlFKU\njANwYWSUaARNkkhNlEhOh5RSlHUu\n",
"model_checkpoint": "cf8f6120-49ee-48f8-b2d7-d663a4708271/56320/80eedcdd-f97b-46d0-9b94-780685292acf/12864"
},
"input_shapes": {
"new_img": [
2,
6,
128,
256
],
"desire": [
8
],
"traffic_convention": [
1,
2
],
"action_t": [
1,
2
],
"nav_map": [
1,
1,
256,
256
],
"navigation": [
1,
47
],
"state_img_q": [
2,
5,
6,
128,
256
],
"state_desire_q": [
132,
1,
8
],
"state_feat_q": [
128,
1,
16384
],
"state_map_features_q": [
128,
1,
64
],
"state_navigation_features_q": [
128,
1,
64
]
},
"output_shapes": {
"outputs": [
1,
18580
],
"next_state_img_q": [
2,
5,
6,
128,
256
],
"next_state_desire_q": [
132,
1,
8
],
"next_state_feat_q": [
128,
1,
16384
],
"next_state_map_features_q": [
128,
1,
64
],
"next_state_navigation_features_q": [
128,
1,
64
]
}
}
}
openpilot/sunnypilot/models/tests/test_navigation_model.py
+86
View File
@@ -1,12 +1,17 @@
"""Local selector entries require an installed verified target artifact."""
import ast
import base64
import builtins
import hashlib
import importlib.util
import json
import io
import mmap
from pathlib import Path
import pickle
import sys
import struct
import subprocess
from tempfile import TemporaryDirectory
from types import SimpleNamespace
import unittest
@@ -132,6 +137,87 @@ class LocalNavigationModelTests(unittest.TestCase):
self.assertIsNone(nav.installed_bundle(self.root))
def real_navigation_metadata():
fixture = json.loads((SOURCE / 'tests/fixtures/rl_nav_80eedcdd_metadata.json').read_text())
for name, value in fixture['input_specs'].items():
fixture['input_specs'][name] = (tuple(value[0]), nav.np.dtype(value[1]).str, value[2])
for name, value in fixture['output_specs'].items():
fixture['output_specs'][name] = (tuple(value[0]), nav.np.dtype(value[1]).name, value[2])
for kind in ('input_shapes', 'output_shapes'):
fixture['metadata'][kind] = {name: tuple(shape) for name, shape in fixture['metadata'][kind].items()}
return fixture
def plain_io_functions():
"""Execute the exact plain writer/reader definitions, with no GPU imports."""
path = SOURCE.parents[0] / 'modeld_v2/helpers.py'
text = path.read_text() if path.exists() else subprocess.check_output(
['git', '-C', str(SOURCE.parents[2]), 'show', 'HEAD:openpilot/sunnypilot/modeld_v2/helpers.py'], text=True)
names = {'Blob', '_MetadataUnpickler', '_relocate', 'write_bundle', 'read_parts'}
constants = {'MAGIC', 'ALIGNMENT', 'METADATA_TYPES'}
selected = []
for node in ast.parse(text).body:
if isinstance(node, (ast.FunctionDef, ast.ClassDef)) and node.name in names:
selected.append(node)
elif isinstance(node, ast.Assign) and any(isinstance(t, ast.Name) and t.id in constants for t in node.targets):
selected.append(node)
namespace = {'hashlib': hashlib, 'builtins': builtins, 'pickle': pickle, 'struct': struct, 'json': json,
'io': io, 'mmap': mmap}
exec(compile(ast.Module(body=selected, type_ignores=[]), str(path), 'exec'), namespace)
return namespace['write_bundle'], namespace['read_parts']
class RealNavigationMetadataTests(unittest.TestCase):
def test_exact_official_wrapped_metadata_survives_actual_plain_roundtrip(self):
fixture = real_navigation_metadata()
self.assertEqual(fixture['provenance']['officialOnnxSha256'], nav.SOURCE_ONNX_SHA256)
self.assertEqual(fixture['provenance']['officialOnnxBytes'], nav.SOURCE_ONNX_BYTES)
self.assertEqual(fixture['metadata']['metadata']['output_slices'].count('\n'), 9)
write_bundle, read_parts = plain_io_functions()
with TemporaryDirectory() as directory:
path = Path(directory) / 'real_metadata_plain_fixture.pkl'
# An empty, non-executable run part is sufficient to test the actual format.
write_bundle(path, {'run': ({'calls': [], 'buffers': []}, b'')}, fixture)
parts, decoded = read_parts(path)
self.assertIn('run', parts)
self.assertEqual(decoded, fixture)
self.assertTrue(nav.compatible_metadata(decoded))
(Path(directory) / nav.MANIFEST_NAME).write_text(json.dumps({'version': 1, 'target': 'chestnut',
'sourceOnnxSha256': nav.SOURCE_ONNX_SHA256,
'artifact': {'fileName': path.name, 'sha256': nav.file_sha256(path)}}))
with patch.dict(sys.modules, {'openpilot.sunnypilot.modeld_v2.helpers': SimpleNamespace(read_parts=read_parts)}):
self.assertIsNotNone(nav.installed_bundle(directory))
def test_ascii_wrapping_is_allowed_but_corrupt_base64_is_rejected(self):
fixture = real_navigation_metadata()
original = fixture['metadata']['metadata']['output_slices']
compact = ''.join(original.split())
fixture['metadata']['metadata']['output_slices'] = ' \t\r\n'.join(compact[i:i+16] for i in range(0, len(compact), 16))
self.assertTrue(nav.compatible_metadata(fixture))
for encoded in (compact[:8] + '!' + compact[8:], compact[:-1], compact[:8] + '\u00a0' + compact[8:]):
with self.subTest(encoded=encoded[:20]):
fixture['metadata']['metadata']['output_slices'] = encoded
self.assertFalse(nav.compatible_metadata(fixture))
def test_wrapping_does_not_permit_executable_or_wrong_slice_metadata(self):
class ExecutableMetadata:
def __reduce__(self):
return builtins.eval, ('1 + 1',)
fixture = real_navigation_metadata()
payload = base64.encodebytes(pickle.dumps(ExecutableMetadata())).decode('ascii')
fixture['metadata']['metadata']['output_slices'] = payload
with patch.object(builtins, 'eval', side_effect=AssertionError('Metadata must never execute')) as execute:
self.assertFalse(nav.compatible_metadata(fixture))
execute.assert_not_called()
original = real_navigation_metadata()['metadata']['metadata']['output_slices']
slices = nav._SliceMetadata(io.BytesIO(base64.b64decode(original))).load()
slices['action'] = slice(2062, 2070)
fixture['metadata']['metadata']['output_slices'] = base64.encodebytes(pickle.dumps(slices)).decode('ascii')
self.assertFalse(nav.compatible_metadata(fixture))
fixture['metadata']['metadata']['output_slices'] = base64.encodebytes(pickle.dumps([])).decode('ascii')
self.assertFalse(nav.compatible_metadata(fixture))
class CatalogHookTests(unittest.TestCase):
def test_cached_offline_model_is_selectable_without_changing_remote_cache(self):
tree = ast.parse((SOURCE / 'fetcher.py').read_text())
openpilot/sunnypilot/models/tests/test_navigation_preparation.py
+20
View File
@@ -168,6 +168,26 @@ class BuildLifecycleTests(unittest.TestCase):
self.run_build(lambda _path: None)
self.assertFalse((self.root / nav.MANIFEST_NAME).exists())
def test_rejected_completed_build_is_preserved_once_without_normal_registration(self):
failed = self.root / builder.FAILED_BUILD_DIRECTORY
for attempt in range(2):
if attempt:
(failed / 'old-build-marker').write_text('must be replaced')
with self.assertRaisesRegex(RuntimeError, 'Completed build and validation details kept'):
self.run_build(lambda _path: None)
descriptor = json.loads((failed / nav.MANIFEST_NAME).read_text())
artifact = failed / descriptor['artifact']['fileName']
self.assertEqual(artifact.read_bytes(), b'target-plain-model')
self.assertEqual(nav.file_sha256(artifact), descriptor['artifact']['sha256'])
diagnostic = json.loads((failed / 'validation_failure.json').read_text())
self.assertFalse(diagnostic['registered'])
self.assertFalse(diagnostic['automaticReuseAllowed'])
self.assertEqual(diagnostic['artifact']['sha256'], descriptor['artifact']['sha256'])
self.assertFalse((failed / 'old-build-marker').exists())
self.assertFalse((self.root / nav.MANIFEST_NAME).exists())
self.assertIsNone(nav.installed_bundle(self.root))
self.assertEqual([path.name for path in self.root.glob('.rl_nav_failed*')], [builder.FAILED_BUILD_DIRECTORY])
def test_onroad_before_publication_never_registers(self):
def process(*args, **kwargs):
result = self.process(*args, **kwargs)