Harden dispersion entries in the artifact trust boundary - #117
Merged
Conversation
Fixes six Major findings from a three-agent security review of the artifact trust boundary in aimnet.models.artifact_validation: 1. Forbid the ptfile constructor kwarg anywhere in artifact model_yaml. DispParam.__init__ runs torch.load(ptfile, weights_only=True) on a YAML-supplied path, and the import-path walker never inspected constructor kwargs, so a default-trusted artifact could carry an arbitrary-path read/probe/DoS primitive. The exporter already strips ptfile before producing an artifact, so no legitimate artifact is affected; training-config loading is untouched. 2. Cross-check D3TS presence between model_yaml and the has_embedded_d3ts metadata flag during artifact validation. The two were previously validated independently, so a mislabeled artifact could silently double-count or entirely lose dispersion depending on which direction it was mislabeled. Also fixed a test_train_utils.py export test whose mock produced exactly that mislabeled shape, now that the boundary rejects it. 3. Validate D3TS damping parameters (a1, a2, s8, s6) supplied via artifact model_yaml: finite, non-negative real numbers only. These are plain constructor floats outside the state dict, so nothing previously stopped a NaN/Inf value or a1=a2=0 (an undamped 1/d^6 collapse) from loading silently. 4. Added an invariant test pinning every default-trusted class import path into the frozen serialization-ABI list, plus a completeness fixture proving REGISTRY_IMPORT_POLICY actually admits a DispParam+D3TS artifact shape end-to-end. 5. Documented the admission criteria for the default class-import allowlist directly above its definition. 6. Trusted the aimnet.modules.lr.D3TS submodule spelling alongside the existing barrel spelling, matching the substring-based class detection already used by the loader machinery.
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
1 participant
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.
Follow-up to #115, fixing the six Major findings from a three-agent security review of the dispersion allowlist additions:
ptfilein artifact model_yaml —DispParam.__init__runstorch.loadon that kwarg; the walker never inspected constructor kwargs, so default-trusted artifacts gained an arbitrary-path read/probe primitive. The exporter always strips it, so no legitimate artifact is affected.has_embedded_d3ts, closing the silent dispersion double-count / silent-loss window for mislabeled artifacts.a1/a2/s8/s6from YAML must be finite and non-negative (they live outside the state dict; NaN or zero damping previously loaded silently).aimnet.modules.lr.D3TSis now trusted alongside the barrel spelling (the loader detects D3TS by substring; the validator matches exactly), with the DispParam single-spelling rationale documented.Also repairs a pre-existing test mock in test_train_utils.py that produced exactly the mislabeled shape check (2) now rejects.
Verification: security+ABI suites 228 passed; model+calculator suites 111 passed; full default suite 589 passed, 0 failed; ruff clean.