Submitter | phabricator |
---|---|
Date | April 1, 2020, 2:47 p.m. |
Message ID | <differential-rev-PHID-DREV-ykuwhajlqcate46534nk-req@mercurial-scm.org> |
Download | mbox | patch |
Permalink | /patch/45958/ |
State | Superseded |
Headers | show |
Comments
indygreg added a comment. I do not understand what the intended side-effect of running this command is supposed to be. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D8353/new/ REVISION DETAIL https://phab.mercurial-scm.org/D8353 To: durin42, #hg-reviewers Cc: indygreg, mercurial-devel
durin42 added a comment.
In D8353#124864 <https://phab.mercurial-scm.org/D8353#124864>, @indygreg wrote:
> I do not understand what the intended side-effect of running this command is supposed to be.
Well, if an AV engine isn't configured to ignore the user's clone, then this will enrage the AV engine by writing out a "virus".
This started out as a joke, but I kind of came around to the realization that it could be useful. So I'm like....90% sure I think this patch makes sense. :)
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D8353/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D8353
To: durin42, #hg-reviewers
Cc: indygreg, mercurial-devel
mharbison72 added a comment. Should it delete the file after writing it, or don't bother because the AV may have it locked? REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D8353/new/ REVISION DETAIL https://phab.mercurial-scm.org/D8353 To: durin42, #hg-reviewers Cc: mharbison72, indygreg, mercurial-devel
durin42 added a comment.
In D8353#124948 <https://phab.mercurial-scm.org/D8353#124948>, @mharbison72 wrote:
> Should it delete the file after writing it, or don't bother because the AV may have it locked?
I figured just leave it since it's tiny, but maybe we should sleep for a while and then remove it?
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D8353/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D8353
To: durin42, #hg-reviewers
Cc: mharbison72, indygreg, mercurial-devel
mharbison72 added a comment. In D8353#124949 <https://phab.mercurial-scm.org/D8353#124949>, @durin42 wrote: > In D8353#124948 <https://phab.mercurial-scm.org/D8353#124948>, @mharbison72 wrote: > >> Should it delete the file after writing it, or don't bother because the AV may have it locked? > > I figured just leave it since it's tiny, but maybe we should sleep for a while and then remove it? That might work. I don't feel too strongly about it, but it seemed weird to leave junk in there and I only realized why as I was writing out the comment. The couple of times I've worked on viruses that evaded detection, file names are what I keyed in on. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D8353/new/ REVISION DETAIL https://phab.mercurial-scm.org/D8353 To: durin42, #hg-reviewers Cc: mharbison72, indygreg, mercurial-devel
marmoute added a comment. Silly question, could make anti virus cough on the mercurial source itself? REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D8353/new/ REVISION DETAIL https://phab.mercurial-scm.org/D8353 To: durin42, #hg-reviewers Cc: marmoute, mharbison72, indygreg, mercurial-devel
marmoute added inline comments. INLINE COMMENTS > debugcommands.py:135-138 > + util.b85decode( > + b'ST#=}P$fV?P+K%yP+C|uG$>GBDK|qyDK~v2MM*<JQY}+dK~6+LQba95P' > + b'E<)&Nm5l)EmTEQR4qnHOhq9iNGnJx' > + ) I think it could be useful to have a clarification of this payload is from an inline comment. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D8353/new/ REVISION DETAIL https://phab.mercurial-scm.org/D8353 To: durin42, #hg-reviewers Cc: marmoute, mharbison72, indygreg, mercurial-devel
durin42 added a comment. I don't think so, since it's base85-armored and my understanding is that really it's only supposed to trigger in isolation... REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D8353/new/ REVISION DETAIL https://phab.mercurial-scm.org/D8353 To: durin42, #hg-reviewers Cc: marmoute, mharbison72, indygreg, mercurial-devel
This revision now requires changes to proceed. marmoute added a comment. marmoute requested changes to this revision. The approach seems good, but extra comment and cleanup would be nice. INLINE COMMENTS > debugcommands.py:139 > + ) > + ) > + As @mharbison72 pointed out, it would be nicer to remove the file after it has been scanned. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D8353/new/ REVISION DETAIL https://phab.mercurial-scm.org/D8353 To: durin42, #hg-reviewers, marmoute Cc: marmoute, mharbison72, indygreg, mercurial-devel
marmoute added a comment. marmoute accepted this revision. Seems overall good. I have some fear over the `sleep` usage, butit does not seems to depends on test, and I don't know what antivirus looks like. INLINE COMMENTS > debugcommands.py:143 > + # Give an AV engine time to scan the file. > + time.sleep(2) > + util.unlink(repo.cachevfs.join('eicar-test-file.com')) That seems short ? that might fails on heavy load system. Also, this confused me a bit. How is the antivirus failure supposed to manifest when this code is run? I have no experience with them. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D8353/new/ REVISION DETAIL https://phab.mercurial-scm.org/D8353 To: durin42, #hg-reviewers, marmoute Cc: mercurial-patches, marmoute, mharbison72, indygreg, mercurial-devel
durin42 added a comment. Best guess (given I've never hit an AV problem) is that the AV engine would lose its lunch on the EICAR file and alert the user. I figure if the AV engine isn't picking up on it after 2 seconds then it's probably also not a performance issue for us. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D8353/new/ REVISION DETAIL https://phab.mercurial-scm.org/D8353 To: durin42, #hg-reviewers, marmoute Cc: mercurial-patches, marmoute, mharbison72, indygreg, mercurial-devel
marmoute added a comment. My main question here is : what does "AV engine would lose its lunch" translate in terms of the python runtime ? REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D8353/new/ REVISION DETAIL https://phab.mercurial-scm.org/D8353 To: durin42, #hg-reviewers, marmoute Cc: mercurial-patches, marmoute, mharbison72, indygreg, mercurial-devel
durin42 added a comment. Sadly I have no idea on that. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D8353/new/ REVISION DETAIL https://phab.mercurial-scm.org/D8353 To: durin42, #hg-reviewers, marmoute Cc: mercurial-patches, marmoute, mharbison72, indygreg, mercurial-devel
marmoute added a comment.
In D8353#128838 <https://phab.mercurial-scm.org/D8353#128838>, @durin42 wrote:
> Sadly I have no idea on that.
Do we have a poor windows user that could serve an a guinea pig here ?
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D8353/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D8353
To: durin42, #hg-reviewers, marmoute
Cc: mercurial-patches, marmoute, mharbison72, indygreg, mercurial-devel
pulkit added a comment. In D8353#128839 <https://phab.mercurial-scm.org/D8353#128839>, @marmoute wrote: > In D8353#128838 <https://phab.mercurial-scm.org/D8353#128838>, @durin42 wrote: > >> Sadly I have no idea on that. > > Do we have a poor windows user that could serve an a guinea pig here ? Seems like we are not able to find a guinea pig here. How about pushing the patch and finding one in wild? :D REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D8353/new/ REVISION DETAIL https://phab.mercurial-scm.org/D8353 To: durin42, #hg-reviewers, marmoute Cc: pulkit, mercurial-patches, marmoute, mharbison72, indygreg, mercurial-devel
khanchi97 added a comment. In D8353#129493 <https://phab.mercurial-scm.org/D8353#129493>, @pulkit wrote: > In D8353#128839 <https://phab.mercurial-scm.org/D8353#128839>, @marmoute wrote: > >> In D8353#128838 <https://phab.mercurial-scm.org/D8353#128838>, @durin42 wrote: >> >>> Sadly I have no idea on that. >> >> Do we have a poor windows user that could serve an a guinea pig here ? > > Seems like we are not able to find a guinea pig here. How about pushing the patch and finding one in wild? :D Sounds good to me. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D8353/new/ REVISION DETAIL https://phab.mercurial-scm.org/D8353 To: durin42, #hg-reviewers, marmoute Cc: khanchi97, pulkit, mercurial-patches, marmoute, mharbison72, indygreg, mercurial-devel
pulkit added a comment. I will go ahead and push it before upcoming rc if I hear no objections. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D8353/new/ REVISION DETAIL https://phab.mercurial-scm.org/D8353 To: durin42, #hg-reviewers, marmoute Cc: khanchi97, pulkit, mercurial-patches, marmoute, mharbison72, indygreg, mercurial-devel
Patch
diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py --- a/mercurial/debugcommands.py +++ b/mercurial/debugcommands.py @@ -127,6 +127,18 @@ ui.write(b'%d:%s\n' % (r.rev(a), hex(a))) +@command(b'debugantivirusrunning', []) +def debugantivirusrunning(ui, repo): + """attempt to trigger an antivirus scanner to see if one is active""" + with repo.cachevfs.open('eicar-test-file.com', b'wb') as f: + f.write( + util.b85decode( + b'ST#=}P$fV?P+K%yP+C|uG$>GBDK|qyDK~v2MM*<JQY}+dK~6+LQba95P' + b'E<)&Nm5l)EmTEQR4qnHOhq9iNGnJx' + ) + ) + + @command(b'debugapplystreamclonebundle', [], b'FILE') def debugapplystreamclonebundle(ui, repo, fname): """apply a stream clone bundle file"""