test that arbitrary files cannot be checksummed#250
Conversation
mwhooker
left a comment
mwhooker
left a comment
There was a problem hiding this comment.
lgtm.
Just so I make sure I understand, the test is saying that there's a bad checksum in multifile-sha1.sum, but we didn't download that file, so don't try to checksum that file locally, and ignore the checksum line?
|
@mwhooker Yes! Exactly. Should I expand the test case comment? I tried to keep it brief to match the existing style, but it's definitely a weird test. The goal of the test is the help ensure we don't make a change that could cause a security issue for embedders like Nomad. |
|
It couldn't hurt. There are a lot of tests in that table, and it could be that the subtlety of this one gets lost in the sea |
sylviamoss
left a comment
sylviamoss
left a comment
There was a problem hiding this comment.
LGTM!!
Are you going to update the comment? Would it be nice to have more details about the security concern?
|
|
New test asserts that only the checksums for retrieved files will be checked. If a checksum file references other files, those will not be checked. This is an important security assertion as checksum files should not cause go-getter to read arbitrary paths and report if their contents match a checksum.
CreatorHead
force-pushed
the
test-checksum-files
branch
from
0a45fd9 to
7c609f9
Compare
7 participants
New test asserts that only the checksums for retrieved files will be
checked. If a checksum file references other files, those will not be
checked.
This is an important security assertion as checksum files should not
cause go-getter to read arbitrary paths and report if their contents
match a checksum.