feat: add versioned artifact bundle contract - #11
Conversation
Package an optimized graph kernel once, with everything a runtime needs to decide safely whether it may execute it, and load it without model-specific branches. The manifest (schema_version 1) records operation identity and graph fingerprint, input/output tensor signatures, candidate entry point and per-file SHA-256 digests, model/revision, GPU architecture and PyTorch/CUDA/Triton compatibility, inference/training and distributed mode, isolated benchmark evidence, full-generation validation evidence, and the promotion decision with its source campaign. - packager hashes and declares every payload file, refuses caller-supplied digests, and re-verifies the finished bundle from disk the way a consumer will - validator rejects missing, resized, altered and undeclared files - matching is keyed on graph fingerprint and tensor signature, never on a model-specific switch, with stable rejection reason codes and version ranges that fail closed on unparsable runtime versions - loader resolves bundles inside an explicit trusted root, re-hashes before import, and imports under a private module namespace instead of sys.path Metadata only: a forbidden-key walk rejects tensor values, weights, prompts and credentials anywhere in a manifest. Tests: 45 CPU tests with fake kernels; full suite 534 passed.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a versioned artifact bundle system. It defines manifest types and validation, packages and verifies hashed payloads, matches artifacts to runtime requests, selects compatible candidates deterministically, and securely loads entry points from trusted roots. ChangesPortable artifact bundles
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Packager
participant Validator
participant Matcher
participant Loader
Packager->>Validator: verify completed artifact bundle
Validator-->>Packager: return validated ArtifactManifest
Matcher->>Validator: use validated bundle manifest
Matcher-->>Loader: provide selected artifact
Loader->>Validator: re-verify bundle before import
Validator-->>Loader: return verified manifest
Loader-->>Loader: import callable entry point in private namespace
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@autokernel/artifact/compat.py`:
- Around line 22-50: Add a named NOT_SELECTED reason constant alongside the
other REASON_* constants, include it in REJECTION_REASONS, and update
match_artifact to emit that constant instead of the literal "not_selected".
In `@autokernel/artifact/loader.py`:
- Around line 61-83: Bind loading to the exact bytes verified by verify_bundle:
after resolving entry_file, preserve an immutable snapshot or open file
descriptor and ensure exec_module loads from that same content without reopening
the mutable path. Update the manifest validation to compare the optional
manifest against the complete verified manifest, rather than only artifact_id,
while preserving existing ArtifactError behavior.
In `@autokernel/artifact/packager.py`:
- Line 41: Update _COMPUTED_SECTIONS to include created_at and producer so
document.update(sections) cannot override computed values; preserve the existing
dedicated keyword-parameter behavior. Add a test beside
test_packager_refuses_caller_supplied_file_digests verifying caller-supplied
created_at in sections is rejected.
In `@autokernel/artifact/validator.py`:
- Around line 64-72: Update _bundle_files to add every symlink path to the
result before the is_dir() check, then continue skipping directories as before.
Preserve the existing ignored-directory filtering for non-symlink files while
ensuring symlinks themselves are enumerated for undeclared-content validation.
- Around line 27-29: Stop excluding "__pycache__" in "_IGNORED_DIRECTORIES" so
"_bundle_files" reports cache files as undeclared. In the loader’s "exec_module"
flow, temporarily set "sys.dont_write_bytecode" to true and restore its previous
value in a finally block, including when execution fails. Add a regression test
in "tests/test_artifact_bundle.py" that creates a file under the bundle’s
"__pycache__" directory and verifies "verify_bundle" rejects it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4471d289-4d08-4289-b05c-e05f6d04b6b7
📒 Files selected for processing (9)
CHANGELOG.mdautokernel/artifact/__init__.pyautokernel/artifact/compat.pyautokernel/artifact/loader.pyautokernel/artifact/packager.pyautokernel/artifact/types.pyautokernel/artifact/validator.pydocs/ARTIFACT_BUNDLE.mdtests/test_artifact_bundle.py
| REASON_FINGERPRINT_MISMATCH = "fingerprint_mismatch" | ||
| REASON_INPUT_SIGNATURE_MISMATCH = "input_signature_mismatch" | ||
| REASON_OUTPUT_SIGNATURE_MISMATCH = "output_signature_mismatch" | ||
| REASON_MODEL_MISMATCH = "model_mismatch" | ||
| REASON_REVISION_MISMATCH = "model_revision_mismatch" | ||
| REASON_ARCHITECTURE_MISMATCH = "gpu_architecture_mismatch" | ||
| REASON_TORCH_VERSION = "torch_version_unsupported" | ||
| REASON_CUDA_VERSION = "cuda_version_unsupported" | ||
| REASON_TRITON_VERSION = "triton_version_unsupported" | ||
| REASON_EXECUTION_MODE = "execution_mode_unsupported" | ||
| REASON_DISTRIBUTED_MODE = "distributed_mode_unsupported" | ||
| REASON_NOT_PROMOTED = "not_promoted" | ||
| REASON_EVIDENCE_INCOMPLETE = "evidence_incomplete" | ||
|
|
||
| REJECTION_REASONS = ( | ||
| REASON_FINGERPRINT_MISMATCH, | ||
| REASON_INPUT_SIGNATURE_MISMATCH, | ||
| REASON_OUTPUT_SIGNATURE_MISMATCH, | ||
| REASON_MODEL_MISMATCH, | ||
| REASON_REVISION_MISMATCH, | ||
| REASON_ARCHITECTURE_MISMATCH, | ||
| REASON_TORCH_VERSION, | ||
| REASON_CUDA_VERSION, | ||
| REASON_TRITON_VERSION, | ||
| REASON_EXECUTION_MODE, | ||
| REASON_DISTRIBUTED_MODE, | ||
| REASON_NOT_PROMOTED, | ||
| REASON_EVIDENCE_INCOMPLETE, | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Add "not_selected" to the stable reason-code contract.
Line 22-50 declares REJECTION_REASONS and documents it as a stable contract runtimes log verbatim. match_artifact at Line 299 emits a rejection with the literal "not_selected", which is not defined as a named constant and is absent from REJECTION_REASONS. This leaves one observable failure code outside the documented, enumerable contract, so a runtime that validates or logs against REJECTION_REASONS misses this legitimate reason.
♻️ Proposed fix
+REASON_NOT_SELECTED = "not_selected"
+
REJECTION_REASONS = (
REASON_FINGERPRINT_MISMATCH,
REASON_INPUT_SIGNATURE_MISMATCH,
REASON_OUTPUT_SIGNATURE_MISMATCH,
REASON_MODEL_MISMATCH,
REASON_REVISION_MISMATCH,
REASON_ARCHITECTURE_MISMATCH,
REASON_TORCH_VERSION,
REASON_CUDA_VERSION,
REASON_TRITON_VERSION,
REASON_EXECUTION_MODE,
REASON_DISTRIBUTED_MODE,
REASON_NOT_PROMOTED,
REASON_EVIDENCE_INCOMPLETE,
+ REASON_NOT_SELECTED,
) rejections.extend(
- Rejection(item.artifact_id, "not_selected", "a faster artifact was chosen")
+ Rejection(item.artifact_id, REASON_NOT_SELECTED, "a faster artifact was chosen")
for item in candidates
if item is not best
)Also applies to: 298-302
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@autokernel/artifact/compat.py` around lines 22 - 50, Add a named NOT_SELECTED
reason constant alongside the other REASON_* constants, include it in
REJECTION_REASONS, and update match_artifact to emit that constant instead of
the literal "not_selected".
| verified = verify_bundle(directory) | ||
| if manifest is not None and manifest.artifact_id != verified.artifact_id: | ||
| raise ArtifactError( | ||
| f"artifact bundle {str(directory)!r}: artifact_id changed from " | ||
| f"{manifest.artifact_id!r} to {verified.artifact_id!r} since " | ||
| "validation" | ||
| ) | ||
|
|
||
| entry_file = _resolve_inside(directory, directory / verified.entry_point.file) | ||
| module_name = f"{_MODULE_NAMESPACE}.{verified.artifact_id.replace('.', '_')}" | ||
| spec = importlib.util.spec_from_file_location(module_name, entry_file) | ||
| if spec is None or spec.loader is None: | ||
| raise ArtifactError( | ||
| f"artifact bundle {str(directory)!r}: entry_point: cannot load " | ||
| f"{verified.entry_point.file!r}" | ||
| ) | ||
| module = importlib.util.module_from_spec(spec) | ||
| # Registering before execution lets the module use dataclasses and other | ||
| # machinery that looks itself up in sys.modules. It is removed again if | ||
| # execution fails so a partial module is never reachable. | ||
| sys.modules[module_name] = module | ||
| try: | ||
| spec.loader.exec_module(module) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Bind verification to the bytes that execute.
verify_bundle(directory) hashes entry_file before exec_module() reopens that path. If another process can modify the trusted-root bundle in this interval, it can replace the verified entry file with different code that executes without a matching hash check.
Load from immutable storage, or pin and execute the verified file contents through a file descriptor or equivalent atomic mechanism. Also bind an optional prior manifest to the complete verified manifest, not only to artifact_id.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@autokernel/artifact/loader.py` around lines 61 - 83, Bind loading to the
exact bytes verified by verify_bundle: after resolving entry_file, preserve an
immutable snapshot or open file descriptor and ensure exec_module loads from
that same content without reopening the mutable path. Update the manifest
validation to compare the optional manifest against the complete verified
manifest, rather than only artifact_id, while preserving existing ArtifactError
behavior.
| "evidence", | ||
| "promotion", | ||
| ) | ||
| _COMPUTED_SECTIONS = ("files", "schema_version") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Protect created_at and producer from caller override.
_COMPUTED_SECTIONS lists only files and schema_version. Line 110 then runs document.update(sections), so a created_at or producer key inside sections silently replaces the value computed at lines 98-100. Two contracts state the opposite: the comment at lines 30-31 and docs/ARTIFACT_BUNDLE.md line 49 both declare created_at computed and refused. Both values also have dedicated keyword parameters, so accepting them through sections is a second, unvalidated path.
Add both names to _COMPUTED_SECTIONS, and add a test case next to test_packager_refuses_caller_supplied_file_digests in tests/test_artifact_bundle.py for a caller-supplied created_at.
🛠️ Proposed fix
-_COMPUTED_SECTIONS = ("files", "schema_version")
+_COMPUTED_SECTIONS = ("files", "schema_version", "created_at", "producer")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| _COMPUTED_SECTIONS = ("files", "schema_version") | |
| _COMPUTED_SECTIONS = ("files", "schema_version", "created_at", "producer") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@autokernel/artifact/packager.py` at line 41, Update _COMPUTED_SECTIONS to
include created_at and producer so document.update(sections) cannot override
computed values; preserve the existing dedicated keyword-parameter behavior. Add
a test beside test_packager_refuses_caller_supplied_file_digests verifying
caller-supplied created_at in sections is rejected.
| #: Directories ignored when enumerating bundle contents. These are produced by | ||
| #: the interpreter after packaging and carry no trust. | ||
| _IGNORED_DIRECTORIES = frozenset({"__pycache__"}) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
A forged __pycache__ entry bypasses hash verification.
_IGNORED_DIRECTORIES removes __pycache__ from the enumeration at line 89, so its contents are never treated as undeclared and never hashed. load_entry_point in autokernel/artifact/loader.py imports the entry file through the standard source loader, which prefers a valid cached .pyc. The default timestamp strategy compares the source file's size and its last-modified timestamp to the metadata stored in the corresponding .pyc file, and a pyc whose bit field is 0 carries only that timestamp and file size. Both values are known and settable by anyone who can write in the bundle directory.
An attacker with write access to the bundle can therefore drop __pycache__/kernel.cpython-3XX.pyc with arbitrary bytecode and a matching header. verify_bundle still reports the bundle as intact, and the import executes the forged bytecode instead of the verified source. This is the exact threat docs/ARTIFACT_BUNDLE.md lines 93-96 claims to close.
The fix needs two parts, because rejecting __pycache__ alone would make the second verification of a bundle fail after the first import wrote a cache:
- Stop ignoring
__pycache__in_bundle_files, so any cache file is reported as an undeclared file. - In
autokernel/artifact/loader.py, setsys.dont_write_bytecodefor the duration ofexec_module, so the trusted import never writes a cache into a verified bundle.
Add a regression test in tests/test_artifact_bundle.py that writes a file under <bundle>/__pycache__/ and asserts verify_bundle rejects it.
🔒 Proposed fix
-#: Directories ignored when enumerating bundle contents. These are produced by
-#: the interpreter after packaging and carry no trust.
-_IGNORED_DIRECTORIES = frozenset({"__pycache__"})
+#: No directory is ignored. A bytecode cache is executable content: a
+#: timestamp-based ``.pyc`` whose header repeats the verified source mtime and
+#: size is loaded in place of that source, so it must never be invisible here.
+_IGNORED_DIRECTORIES: frozenset[str] = frozenset()Companion change in autokernel/artifact/loader.py, so a trusted import never writes a cache back into the verified bundle:
sys.modules[module_name] = module
previous = sys.dont_write_bytecode
sys.dont_write_bytecode = True
try:
spec.loader.exec_module(module)
except Exception as exc:
...
finally:
sys.dont_write_bytecode = previous🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@autokernel/artifact/validator.py` around lines 27 - 29, Stop excluding
"__pycache__" in "_IGNORED_DIRECTORIES" so "_bundle_files" reports cache files
as undeclared. In the loader’s "exec_module" flow, temporarily set
"sys.dont_write_bytecode" to true and restore its previous value in a finally
block, including when execution fails. Add a regression test in
"tests/test_artifact_bundle.py" that creates a file under the bundle’s
"__pycache__" directory and verifies "verify_bundle" rejects it.
| def _bundle_files(directory: Path) -> list[Path]: | ||
| result = [] | ||
| for path in sorted(directory.rglob("*")): | ||
| if path.is_dir(): | ||
| continue | ||
| if any(part in _IGNORED_DIRECTORIES for part in path.relative_to(directory).parts): | ||
| continue | ||
| result.append(path) | ||
| return result |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Confirm the declared Python target so the pathlib symlink behavior applies.
set -euo pipefail
fd -t f -d 2 '^(pyproject.toml|setup.cfg|.python-version|tox.ini)$' --exec sh -c 'echo "== $1"; rg -n "requires-python|target-version|python_requires" "$1" || true' sh {}Repository: aryan5v/motionkernel
Length of output: 207
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate validator_test files =="
fd -t f 'validator' .
echo "== inspect autokernel/artifact/validator.py relevant sections =="
if [ -f autokernel/artifact/validator.py ]; then
wc -l autokernel/artifact/validator.py
sed -n '1,140p' autokernel/artifact/validator.py | cat -n
fi
echo "== search for bundle validation behavior/tests =="
rg -n "_bundle_files|read_manifest|bundle_files|artifact_validator|ArtifactError|undeclared|symlink" autokernel tests . 2>/dev/null | head -200Repository: aryan5v/motionkernel
Length of output: 16221
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== pathlib symlink behavior probe =="
python3 - <<'PY'
from pathlib import Path
import tempfile, os
with tempfile.TemporaryDirectory() as root:
root = Path(root)
declared = root / "declared.txt"
declared.write_text("declared")
hidden = root / "hidden"
hidden.mkdir()
hidden_file = hidden / "secret.py"
hidden_file.write_text("secret")
link = root / "secret_link"
os.symlink("hidden", link)
for name, path in [("link is_dir", link), ("link is_symlink", link), ("link is_file", link), ("hidden is_dir", hidden)]:
try:
print(f"{name}: {path.stat()}")
except Exception as exc:
print(f"{name}: stat exception {type(exc).__name__}: {exc}")
print(f"{name}: is_dir={path.is_dir()}, is_symlink={path.is_symlink()}, is_file={path.is_file()}, exists={path.exists()}")
all_paths = sorted(root.rglob("*"))
print("rglob paths:")
for p in all_paths:
print(f"{p}: is_dir={p.is_dir()}, is_symlink={p.is_symlink()}, is_file={p.is_file()}")
print("declared present via rbga:")
print(any(p == declared for p in all_paths))
print("python version:", getattr(Path, "__dataclass_fields__", False))
PY
echo "== tests around undeclared validation =="
sed -n '250,275p' tests/test_artifact_bundle.py | cat -nRepository: aryan5v/motionkernel
Length of output: 2618
Reject symlinks during bundle enumeration.
Path.rglob("*") does not follow symlinks by default, and a symlink to a directory reports is_dir() == True, so _bundle_files skips both the link and any content behind it. Add the symlink path to the present set before skipping directories so undeclared symlink content is caught as undeclared bundle content.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@autokernel/artifact/validator.py` around lines 64 - 72, Update _bundle_files
to add every symlink path to the result before the is_dir() check, then continue
skipping directories as before. Preserve the existing ignored-directory
filtering for non-symlink files while ensuring symlinks themselves are
enumerated for undeclared-content validation.
1 participant
Fork-only — not for upstream. Stacked on
agent/graph-specgen.Packages an optimized graph kernel once, with everything a runtime needs to decide safely whether it may execute it, so the runtime loads it without a single model-specific branch.
The manifest (
schema_version: 1)Records every fact the contract requires:
operationgraph_fingerprint, parent module, canonical operation listsignaturerequires_grad)entry_pointfile+symbolfilescompatibilityevidence.benchmarkevidence.generationpromotionVersioned independently of the discovery and campaign schemas.
Packager and validator
files,schema_versionandcreated_atare computed; they are refused if the caller supplies them.Matching
Keyed on graph fingerprint and tensor signature, never on a model-specific switch. Checks run cheapest-first (identity → layout → environment) and each failure returns a stable reason code:
fingerprint_mismatch,input_signature_mismatch,output_signature_mismatch,model_mismatch,model_revision_mismatch,gpu_architecture_mismatch,torch_version_unsupported,cuda_version_unsupported,triton_version_unsupported,execution_mode_unsupported,distributed_mode_unsupported,not_promoted,evidence_incomplete."*"wildcards are accepted for model id, revision and architecture. A bounded version range rejects a runtime whose version is missing or unparsable — a bound that cannot be evaluated must never be assumed to hold. Ties break on artifact id, so selection is deterministic.Trust
load_entry_pointresolves the bundle inside an explicit trusted root (defeating symlink escapes), re-hashes every declared file immediately before the import, and imports under a privateautokernel._artifacts.*name rather than touchingsys.path.Privacy
Metadata only. A forbidden-key walk rejects tensor values, weights, prompts, activations and credentials anywhere in a manifest, at any depth.
Testing
tests/test_artifact_bundle.py) using fake kernels only — no CUDA, no Triton, no model code. They cover packaging round-trip, tampered/truncated/missing/undeclared files, schema-version and traversal rejection, trusted-root enforcement, hash-before-import ordering, every rejection reason, wildcard matching, deterministic selection, and version parsing.aryan5v/FastVideo#agent/v1-generic-dispatch.Docs:
docs/ARTIFACT_BUNDLE.md.Summary by CodeRabbit