only use dmlast for DML files distributed near the compiler#443
only use dmlast for DML files distributed near the compiler#443mandolaerik wants to merge 3 commits into
Conversation
|
PR Verification: ✅ success |
| if (dml_filename.endswith('.dml') and os.path.exists(ast_filename) | ||
| # Trust only .dmlast files inside the compiler tree. Files outside may | ||
| # come from incompatible parser versions or untrusted sources. | ||
| and Path(__file__).parent.parent.parent in Path(ast_filename).parents): |
There was a problem hiding this comment.
Move this check down as a new link of the if chain containing the WOLDAST check. We wanna warn/error if a dmlast is rejected, not just silently skip past it.
There was a problem hiding this comment.
I still insist on this. The condition feels fragile to me, and I want to be sure we can tell when it fails.
There was a problem hiding this comment.
Sorry, I simply forgot your remark, fixed now.
mandolaerik
force-pushed
the
pr/only-use-dmlast-for-dml-files-distributed-near-the-compiler
branch
from
15d76b5 to
f6c51af
Compare
|
PR Verification: ✅ success |
lwaern-intel
left a comment
lwaern-intel
left a comment
There was a problem hiding this comment.
You have to address my concern in some way. Either do what I suggest, prove my concern is utterly unfounded, or remove the change entirely (now it's no longer needed.) As-is, all it takes is a shake-up of how dirs get -I:d (like how CMake shakes stuff up compared to Make) in a way we aren't expecting for .dmlasts to stop working, potentially in a way we wouldn't be able to easily detect.
mandolaerik
force-pushed
the
pr/only-use-dmlast-for-dml-files-distributed-near-the-compiler
branch
from
f6c51af to
ca4a943
Compare
|
DML Verification : 6: ❌ failure |
|
DML Verification : 7: ❌ failure |
mandolaerik
force-pushed
the
pr/only-use-dmlast-for-dml-files-distributed-near-the-compiler
branch
from
ca4a943 to
5b82575
Compare
|
DML Verification : 6: ❌ failure |
|
PR Verification: ❌ failure |
`pickle.loads` is unsafe for untrusted data, and although dmlast files are only used on trusted data, it is hard for the reader to deduce this so it is easier to use the safe unpickler idiom instead of suppressing security warnings.
mandolaerik
force-pushed
the
pr/only-use-dmlast-for-dml-files-distributed-near-the-compiler
branch
from
5b82575 to
61619ef
Compare
|
PR Verification: ❌ failure |
Symlink resolution caused the standard DML lib of fake pkgs in pkgs/ to appear as `<repo>/linux64/bin/dml`
|
PR Verification: ✅ success |
|
DML Verification : 6: ❌ failure |
|
DML Verification : 7: ❌ failure |
|
PR Verification: ❌ failure |
3 participants
This hopefully silences security people and bug-hunting LLM:s more efficiently, since the
# nosecapparently wasn't sufficient to do so.