Fix TestRemovePath being flagged as a possible virus #3411

cmacknz · 2023-09-13T20:38:18Z

My suspicion is this test gets flagged as malware because it writes code to disk and then compiles it by executing go build itself. This does seem like something malware could do, so I changed this to use the same build strategy the fake component uses. The mage build:testBinary target must be used to build the binaries before the test can run, which is a dependency of mage unitTest.

mergify · 2023-09-13T20:39:08Z

This pull request does not have a backport label. Could you fix it @cmacknz? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v./d./d./d is the label to automatically backport to the 8./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

elasticmachine · 2023-09-13T21:03:52Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2023-09-14T02:20:52.315+0000
Duration: 28 min 11 sec

Test stats 🧪

Test	Results
Failed	0
Passed	6301
Skipped	59
Total	6360

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages.
run integration tests : Run the Elastic Agent Integration tests.
run end-to-end tests : Generate the packages and run the E2E Tests.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

elasticmachine · 2023-09-13T21:04:01Z

🌐 Coverage report

Name	Metrics % (`covered/total`)	Diff
Packages	98.78% (`81/82`)	👍
Files	66.327% (`195/294`)	👍
Classes	65.751% (`359/546`)	👍
Methods	52.76% (`1128/2138`)	👍
Lines	38.246% (`12812/33499`)	👍 0.018
Conditionals	100.0% (`0/0`)	💚

cmacknz · 2023-09-13T23:58:40Z

/test

cmacknz · 2023-09-14T00:01:21Z

Taking out of draft, got one successful run. Will run the tests a few times before merging to confirm. I expect test may still be flaky for other reasons, I just want to eliminate the virus scanning false positive with these changes.

elasticmachine · 2023-09-14T00:01:25Z

Pinging @elastic/elastic-agent (Team:Elastic-Agent)

ycombinator

LGTM.

ycombinator · 2023-09-14T01:02:16Z

internal/pkg/agent/install/uninstall_windows_test.go

-	err = RemovePath(dir)
-	require.NoError(t, err)


Shouldn't these lines still be there? Isn't RemovePath the function being tested?

Yes they should whoops

I did this far too quickly the first time, thanks for catching this.

cmacknz · 2023-09-14T02:05:21Z

=== RUN   TestRemovePath
    uninstall_windows_test.go:43: 
        	Error Trace:	C:/Users/jenkins/workspace/_agent_elastic-agent-mbp_PR-3411/src/github.com/elastic/elastic-agent/internal/pkg/agent/install/uninstall_windows_test.go:43
        	Error:      	Received unexpected error:
        	            	fork/exec C:\Users\jenkins\AppData\Local\Temp\TestRemovePath532456694\001\subdir\testblocking.exe: Operation did not complete successfully because the file contains a virus or potentially unwanted software.
        	Test:       	TestRemovePath
--- FAIL: TestRemovePath (2.95s)

Ugh

elastic-sonarqube · 2023-09-14T02:32:06Z

SonarQube Quality Gate

0.0% Coverage on New Code (is less than 40%)

See analysis details on SonarQube

cmacknz · 2023-09-14T13:39:47Z

Merging, let's see if this problem comes back after some more test runs on other branches and adjust from there.

* Do not generate and compile blocking exe automatically. * Copy the test binary to a tmp subdir so the test actually works. * Stop exec-ing from Temp. (cherry picked from commit 72ce0fe)

* Do not generate and compile blocking exe automatically. * Copy the test binary to a tmp subdir so the test actually works. * Stop exec-ing from Temp. (cherry picked from commit 72ce0fe) Co-authored-by: Craig MacKenzie <[email protected]>

mergify bot assigned cmacknz Sep 13, 2023

mergify bot added the backport-skip label Sep 13, 2023

cmacknz force-pushed the fix-windows-uninstall-test-malware-detection branch from 2c8009e to c9afc2d Compare September 13, 2023 22:09

cmacknz changed the title ~~Do not generate and compile blocking exe automatically.~~ Fix TestRemovePath being flagged as a possible virus Sep 13, 2023

cmacknz added the Team:Elastic-Agent Label for the Agent team label Sep 14, 2023

cmacknz marked this pull request as ready for review September 14, 2023 00:01

cmacknz requested a review from a team as a code owner September 14, 2023 00:01

cmacknz requested review from AndersonQ and pchila September 14, 2023 00:01

ycombinator approved these changes Sep 14, 2023

View reviewed changes

cmacknz added the skip-changelog label Sep 14, 2023

Do not generate and compile blocking exe automatically.

0b42ffc

cmacknz force-pushed the fix-windows-uninstall-test-malware-detection branch from c9afc2d to 0b42ffc Compare September 14, 2023 01:00

cmacknz added the backport-v8.10.0 Automated backport with mergify label Sep 14, 2023

mergify bot removed the backport-skip label Sep 14, 2023

cmacknz enabled auto-merge (squash) September 14, 2023 01:01

ycombinator reviewed Sep 14, 2023

View reviewed changes

cmacknz disabled auto-merge September 14, 2023 01:02

Copy the test binary to a tmp subdir so the test actually works.

c571c68

ycombinator approved these changes Sep 14, 2023

View reviewed changes

Stop exec-ing from Temp.

d339a31

AndersonQ approved these changes Sep 14, 2023

View reviewed changes

pierrehilbert mentioned this pull request Sep 14, 2023

Add beat serverless tests & related support #3258

Closed

4 tasks

pchila approved these changes Sep 14, 2023

View reviewed changes

cmacknz merged commit 72ce0fe into elastic:main Sep 14, 2023

cmacknz deleted the fix-windows-uninstall-test-malware-detection branch September 14, 2023 13:39

mergify bot mentioned this pull request Sep 14, 2023

[8.10](backport #3411) Fix TestRemovePath being flagged as a possible virus #3418

Merged

dliappis mentioned this pull request Sep 14, 2023

[ci] Custom revision parameter for Jenkins jobs #3419

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix TestRemovePath being flagged as a possible virus #3411

Fix TestRemovePath being flagged as a possible virus #3411

cmacknz commented Sep 13, 2023 •

edited

Loading

mergify bot commented Sep 13, 2023

elasticmachine commented Sep 13, 2023 •

edited

Loading

Build stats

Test stats 🧪

elasticmachine commented Sep 13, 2023 •

edited

Loading

cmacknz commented Sep 13, 2023

cmacknz commented Sep 14, 2023

elasticmachine commented Sep 14, 2023

ycombinator left a comment

ycombinator Sep 14, 2023

cmacknz Sep 14, 2023

cmacknz Sep 14, 2023

cmacknz commented Sep 14, 2023

elastic-sonarqube bot commented Sep 14, 2023

cmacknz commented Sep 14, 2023

Fix TestRemovePath being flagged as a possible virus #3411

Fix TestRemovePath being flagged as a possible virus #3411

Conversation

cmacknz commented Sep 13, 2023 • edited Loading

mergify bot commented Sep 13, 2023

elasticmachine commented Sep 13, 2023 • edited Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

elasticmachine commented Sep 13, 2023 • edited Loading

🌐 Coverage report

cmacknz commented Sep 13, 2023

cmacknz commented Sep 14, 2023

elasticmachine commented Sep 14, 2023

ycombinator left a comment

Choose a reason for hiding this comment

ycombinator Sep 14, 2023

Choose a reason for hiding this comment

cmacknz Sep 14, 2023

Choose a reason for hiding this comment

cmacknz Sep 14, 2023

Choose a reason for hiding this comment

cmacknz commented Sep 14, 2023

elastic-sonarqube bot commented Sep 14, 2023

cmacknz commented Sep 14, 2023

cmacknz commented Sep 13, 2023 •

edited

Loading

elasticmachine commented Sep 13, 2023 •

edited

Loading

elasticmachine commented Sep 13, 2023 •

edited

Loading