Add a GitHub Action to run pytest daily to detect broken urls.

[songzy12]

Inspired by #374, this PR

1. added a Github Actions with pytest, where
2. we extract urls (using regex) from php files (using glob), then
3. use python's requests lib to check their availabilities.

[digininja]

Seems like an ok idea. One thing I spotted from a quick look, it assumes that all links use single quotes which isn't guaranteed. I'll have a look at the rest of the code when on a PC later

…

On Mon, 10 Aug 2020, 14:35 Zhengyang Song, ***@***.***> wrote: Inspired by #374 <#374>, this PR 1. added a Github Actions with pytest, where 2. we extract urls (using regex) from php files (using glob), then 3. use python's requests lib to check their availabilities. ------------------------------ You can view, comment on, or merge this pull request online at: #376 Commit Summary - Add a GitHub Action to run pytest daily to detect broken urls. File Changes - *A* .github/workflows/main.yml <https://github.com/digininja/DVWA/pull/376/files#diff-90829e76e906f1c73140c7ded7e1b268> (21) - *A* tests/test_url.py <https://github.com/digininja/DVWA/pull/376/files#diff-8b270584bb5bef11f7777857af051674> (35) Patch Links: - https://github.com/digininja/DVWA/pull/376.patch - https://github.com/digininja/DVWA/pull/376.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#376>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA4SWKZ74DHDBGUC7WC33DR77ZS5ANCNFSM4PZ65JMA> .

[songzy12]

Thanks for this advice, I just added the regex to also find links with double quotes.

[digininja]

I've never set up a GitHub action, how easy are they to do? Looking at the one you've done, it looks like it has a Cron facility, that would be good to automate the checks.

…

On Thu, 13 Aug 2020, 16:24 Zhengyang Song, ***@***.***> wrote: Thanks for this advice, I just added the regex to also find links with double quotes. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#376 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA4SWJG4T5WKMYINMKBN3LSAQAR3ANCNFSM4PZ65JMA> .

[songzy12]

To set up the GitHub Action, you just need to create the file .github/workflows/main.yml like in this PR.

Then GitHub would setup the configured flow automatically, and then

1. show a tick or cross sign near your commits:
   

2. send you a email when the test fails:
   

3. you can also check the details at Action tab:
   

[digininja]

I'm not a Python user, when I try to run this locally it picks up the wrong document root so doesn't test anything. How do I point this at the correct document root?

$ python3 -m pytest -s
================================= test session starts ==================================
platform linux -- Python 3.7.3, pytest-6.0.1, py-1.9.0, pluggy-0.13.1
rootdir: /var/sites/dvwa/non-secure/htdocs/tests
collected 1 item                                                                       

test_url.py .

================================== 1 passed in 0.05s ===================================

Would the GitHub action pick up the right directory and file?

[songzy12]

hi Robin, which directory are you in when you run this command?

When I run this command directly under the top folder (i.e., directly under DVWA/) it can pick the right root dir.
And yes GitHub Actions can pick up the right dir: you can see the third screenshot at #376 (comment)

[digininja]

It wasn't working from the document root, but is now, very odd.

Should it detect every broken link or stop at the first? It has picked up one in about.php but there is at least one more that I deliberately created to see if it would spot it.

[songzy12]

Yes currently it will exit after it detects the first broken url.

But I just had an idea about how to detect all broken links at one run. I can submit the code change for this later.

[digininja]

I'll pass it back to you then

…

On Fri, 14 Aug 2020 at 15:42, Zhengyang Song ***@***.***> wrote: Yes currently it will exit after it detects the first broken url. But I just had an idea about how to detect all broken links at one run. I can submit the code change for this later. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#376 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA4SWJ4H2G5OWMZSKB52H3SAVEOHANCNFSM4PZ65JMA> .

[songzy12]

The idea is to save all the broken urls in a list and check whether its length is 0.

It should work for multiple broken urls now:

[digininja]

It doesn't seem to be working, I've created a file in the doc root called test.php with this content:

<a href="http://asdfa.notworking/eiwof">test</a>

If I then run the script in the same way you do, it only picks up the about.php page with the tmackuk domain.

[songzy12]

The reason for the above failure is that we did not check "http" links previously.
I have updated the code again, and now it should work for both "https" and "http" links.

[digininja]

It is the smallest things!

It is now picking things up, can you tidy the output a bit? This is what it looks like to me, shouldn't it just give the list of things that failed rather than the full exception and stack trace? If that is hard to do, then leave it, as long as it shows the things that need fixing, it will be obvious and it shouldn't actually trigger that often anyway.

[songzy12]

hi Robin,
I did a quick search for this and found this might be the intended behavior of pytest.
See: https://docs.pytest.org/en/stable/

[digininja]

OK, I'll fix up those broken links, check that it is then happy and then I'll give it a go as an action. Should be easy to test.

…

On Tue, 18 Aug 2020 at 11:38, Zhengyang Song ***@***.***> wrote: hi Robin, I did a quick search for this and found this might be the intended behavior of pytest. See: https://docs.pytest.org/en/stable/ — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#376 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA4SWPMZQ6V7TKBI7PHTUTSBJKZ5ANCNFSM4PZ65JMA> .

[digininja]

Finally fixed things up, lets try it out

[digininja]

As it merged I got this alert:

https://github.com/digininja/DVWA/actions/runs/234266677

All the links are fixed, do you need to do something so it returns 0 rather than 1?

[songzy12]

@digininja hi Robin, you can see the failure message here:

>       assert len(broken_urls) == 0, "Broken URLs Detected."
E       AssertionError: Broken URLs Detected.
E       assert 2 == 0
E        +  where 2 = len([('vulnerabilities/sqli/index.php', 'http://ferruh.mavituna.com/sql-injection-cheatsheet-oku/', 520), ('vulnerabilities/sqli_blind/index.php', 'http://ferruh.mavituna.com/sql-injection-cheatsheet-oku/', 520)])

The reason is the request for url http://ferruh.mavituna.com/sql-injection-cheatsheet-oku/ in file vulnerabilities/sqli/index.php returned 520.
By a munually check of http://ferruh.mavituna.com/sql-injection-cheatsheet-oku/, it seems redirects to https://www.netsparker.com/blog/web-security/sql-injection-cheat-sheet/.
Thus maybe after this modification it should return 0.

[digininja]

I hadn't found that bit in the UI but I did run it by hand and that URL didn't show up.

I've fixed that and a couple of others that also redirected and the build worked ok so looks like all is good.

Thanks for the work and for sticking with it while I worked out what is going on.

[songzy12]

Thanks for taking care of this!

Like what you just mentioned, sometimes the test result seems unstable.
If this would also happen frequently in the future, we can then introduce some retry mechenism to solve it.

[digininja]

Ok let's see how it goes. I can't see things changing much but at least it will catch things if there do.

…

On Tue, 1 Sep 2020, 15:53 Zhengyang Song, ***@***.***> wrote: Thanks for taking care of this! Like what you just mentioned, sometimes the test result seems unstable. If this would also happen frequently in the future, we can then introduce some retry mechenism to solve it. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#376 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA4SWPNCK7MSU5XYOIFFF3SDUDH3ANCNFSM4PZ65JMA> .

[digininja]

After working fine for a few days, the script just failed on this

tests/test_url.py vulnerabilities/xss_s/index.php	https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet	404

If I try to get that URL I get a 301 redirect to the same URL without the www. I'm hoping that it was just some downtime on the OWASP site but mentioning it just in case.

I'll also update that URL.

[songzy12]

Hi Robin,
Do you feel this feature reports false positive too frequently?
I have two ideas:

1. Make it run weekly rather than daily
2. In each run, we test a url for 3 times with 1 second interval.
   We only think one link is broken if all 3 results are broken.
   This could help to migitate some temporary failures of certain website.

[digininja]

It is getting a way too many false positives.

Running weekly could still get the false positives, we would just see them less. The multiple check might work, but some of the sites I've checked are quite slow to respond which could be causing the problems. Increasing the timeout would help with that.

[songzy12]

Sure. Let me do this.

[digininja]

Looks like you broke something, I'm getting this in the results:

Command "python setup.py egg_info" failed with error code 1 in /tmp/pip-build-lvxoui5r/iniconfig/
Error: Process completed with exit code 1.

[songzy12]

Acked. I would take a look later.

[songzy12]

Just did some search and found this,
it seems that the python setuptools is broken.

I have tested with the workaround method in the above link and it worked.
Would send a pull request for this.

[digininja]

OK, thanks

…

On Wed, 21 Oct 2020 at 14:46, Zhengyang Song ***@***.***> wrote: Just did some search and found this <pypa/setuptools#2353 (comment)>, it seems that the python setuptools is broken. I have tested with the workaround method in the above link and it worked. Would send a pull request for this. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#376 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA4SWKWVK72VXMC5HG5MFTSL3Q4LANCNFSM4PZ65JMA> .