Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Husky and ESLint #82

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

kkreine
Copy link

@kkreine kkreine commented Aug 9, 2022

Issue #64

This is a draft.

I have set up ESLint and configured that to work with Prettier, with some possible minor changes. I set up Husky, as of now with a pre-commit hook for running prettier and tests (working on lint errors before adding that). Do you have a preference for the pre-commit and pre-push hooks? Let me know and I can make those changes.

I have a few questions as I've been addressing the ESLint errors.

All of the functions in public/GlobalUtitlity.js appear to be unused in facade-app. What is this file being used for?

I'm working on adding types to the props in App.tsx., including the vulnerabilityDefinitionsRes prop in App.tsx. I'm having some trouble getting the structure of this object. Do you have sample data, sample structure, or a pointer to API docs? From my understanding, that would also give some insight into the type of levelInformationRes too.

In GlobalStateProps.tsx I've started work on determining the shape of vulnerabilityDefinitionsRes.

It also may be easier to test the functions in App.tsx with the typed data if they were moved into external files App could use as a library. I could, from there, use dummy data to test the changes. Let me know if this method works for this project, and if you'd like me to handle it in this issue or another.

I made isSuccessfullyLoaded in the GlobalState interface in State.tsx an optional prop, as it was not being passed as a prop in every occurrence. Should this be a required prop?

Thank you!

@preetkaran20
Copy link
Member

Issue #64

This is a draft.

I have set up ESLint and configured that to work with Prettier, with some possible minor changes. I set up Husky, as of now with a pre-commit hook for running prettier and tests (working on lint errors before adding that). Do you have a preference for the pre-commit and pre-push hooks? Let me know and I can make those changes.

I have a few questions as I've been addressing the ESLint errors.

All of the functions in public/GlobalUtitlity.js appear to be unused in facade-app. What is this file being used for?

This filed is used in VulnerableApp project. so VulnerableApp Facade gives a template UI where clients like VulnerableApp uses that UI template and GlobalUtilities.js gives utility methods to those clients like which level is selected by the user in the template etc.

I'm working on adding types to the props in App.tsx., including the vulnerabilityDefinitionsRes prop in App.tsx. I'm having some trouble getting the structure of this object. Do you have sample data, sample structure, or a pointer to API docs? From my understanding, that would also give some insight into the type of levelInformationRes too.

The structure is quite complex. You can see the entire structure after starting application, visit: http://localhost/VulnerabilityDefinitions endpoint in browser, it will show the entire json structure. The structure is same as https://github.com/SasanLabs/VulnerableApp-facade/blob/f52fca78cda028695bfe5db87951475abf2b33f7/facade-app/src/interface/State.tsx.

Sample structure

{
   "VulnerableApp":[
      {
         "name":"CommandInjection",
         "id":"CommandInjection",
         "description":"Command injection is an attack in which the goal is execution of arbitrary commands on the host operating system via a vulnerable application. Command injection attacks are possible when an application passes unsafe user supplied data (forms, cookies, HTTP headers etc.) to a system shell. In this attack, the attacker-supplied operating system commands are usually executed with the privileges of the vulnerable application. Command injection attacks are possible largely due to insufficient input validation. <br/><br/>Important Links on Command Injection Vulnerability :<br/><ol> <li> <a href=\"https://cwe.mitre.org/data/definitions/77.html\" target=\"_blank\">CWE-77</a>  <li> <a href=\"https://owasp.org/www-community/attacks/Command_Injection\" target=\"_blank\">Owasp Wiki Link</a> </ol>",
         "vulnerabilityTypes":[
            
         ],
         "levels":[
            {
               "levelIdentifier":"LEVEL_1",
               "variant":"UNSECURE",
               "description":null,
               "resourceInformation":{
                  "htmlResource":{
                     "absolute":false,
                     "resourceType":"HTML",
                     "isAbsolute":false,
                     "uri":"/VulnerableApp/templates/CommandInjection/LEVEL_1/CI_Level1.html"
                  },
                  "staticResources":[
                     {
                        "absolute":false,
                        "resourceType":"CSS",
                        "isAbsolute":false,
                        "uri":"/VulnerableApp/templates/CommandInjection/LEVEL_1/CI_Level1.css"
                     },
                     {
                        "absolute":false,
                        "resourceType":"JAVASCRIPT",
                        "isAbsolute":false,
                        "uri":"/VulnerableApp/templates/CommandInjection/LEVEL_1/CI_Level1.js"
                     }
                  ]
               },
               "hints":[
                  {
                     "vulnerabilityTypes":[
                        {
                           "identifierType":"Custom",
                           "value":"COMMAND_INJECTION"
                        },
                        {
                           "identifierType":"CWE",
                           "value":"77"
                        },
                        {
                           "identifierType":"WASC",
                           "value":"31"
                        }
                     ],
                     "description":"\"ipaddress\" query param's value is directly executed."
                  }
               ]
            }
          ]
      }
   ]
}

In GlobalStateProps.tsx I've started work on determining the shape of vulnerabilityDefinitionsRes.

It also may be easier to test the functions in App.tsx with the typed data if they were moved into external files App could use as a library. I could, from there, use dummy data to test the changes. Let me know if this method works for this project, and if you'd like me to handle it in this issue or another.

I made isSuccessfullyLoaded in the GlobalState interface in State.tsx an optional prop, as it was not being passed as a prop in every occurrence. Should this be a required prop?

I think once the application is loaded successfully, it should be populated as true.

Thank you!

My Suggestion, these changes of removing any etc can be complex and requires good amount of testing. I would suggest for now suppress them and let's tackle them in a new ticket.

thanks,
Karan

@@ -22,15 +23,12 @@
"scripts": {
"start": "react-scripts start",
"build": "react-scripts build",
"test": "react-scripts test",
"test": "react-scripts test --watchAll=false",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for this change.

Copy link
Member

@preetkaran20 preetkaran20 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @kkreine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants