Newscanner

[pauljt]

Create a new scanning engine based on acorn.walk, and new rules system

[mozfreddyb]

Please see how we do error reporting. You have to add things of type: error to the scanresults instead of just throwing exceptions. See these lines in the original scan.js:

• Errors: https://github.com/mozilla/scanjs/blob/0dabf39c16e6a1ef63ec85c89bbc9a5b042978e9/common/scan.js#L91
• Results: https://github.com/mozilla/scanjs/blob/0dabf39c16e6a1ef63ec85c89bbc9a5b042978e9/common/scan.js#L127

I am wondering if the error about invalid rules can and should be thrown at a higher level (i.e. UI). Please also make sure that your changes don't break the nodejs runner :) (haven't tested yet)

[mozfreddyb]

The additional merge & commits don't include exception handling as I suggested earlier. I can help throwing a commit on top of it, if that's useful :)

[pauljt]

In addition to fixing the error publishing, need to refactor index.js to make it use the new scanner too.

[pauljt]

Y U CLOSE :)

[pwnetrationguru]

Lol, whoops, I have no clue what happened!

[mozfreddyb]

I tacked on some additional commits to make this work with the nodejs runner¹. I'm not super happy with the rules loading but think this should be done in another commit. Would you please review, @pwnetrationguru?

¹ We should rename index.js, the name is terrible. Suggestions may go in another issue.

[pwnetrationguru]

Tests are broken. Interestingly enough, the "Advanced Tests" still run fine.

I'll be honest, there is a lot broken in this branch the last time I checked it so I was waiting for more updates to review. I will check the branch in its current state and follow up with comments.

This appears to be related to rules.js disappearing.

[pwnetrationguru]

Rules tab, as we know is broken. Before merging into master we need to resolve whether we plan to list all the rules (which I think is a very good idea since it will allow people to see what we consider good and bad) or if we just want to ditch it in the interface.

[pwnetrationguru]

We still have test rules in the scanner (bar.foo, foo.bar(), etc.).

These examples rules should not be merged into master.

[pwnetrationguru]

The experiment tab does not appear to work.

If I type in the example: bad.innerHTML = bad

I get no results. I've tested other examples like eval(), I also get no results.

[pwnetrationguru]

The errors table 'sort' functionality is broken.

Expected: I click 'Line' in the errors table header, the table is sorted based on line field value. Actual: I click 'Line' in the errors table header, the results table is sorted based on line field value.

Basically, the errors table 'sort' links sort the results table not the error table.

We could probably make a separate issue for this, and not worry about fixing before merging this branch into master.

[pwnetrationguru]

To summarize, the changes I think that need to happen before we can merge are:

• Figure out rules tab - either delete or update it so it is accurate
• Tests need to be able to run
• Example rules need to be deleted
• Experiment tab should work

These updates are lightening fast and I'm excited to get them into master!!! :boom:

[pauljt]

Rules - lets make a separate pr for this - i want to add the functionality to switch rule files by loading them etc.

On the others:

1. eval("awtaw") does work, so not sure what isn't working for you.
2. tests/index.html works for me (advanced tests dont all pass but they never did)
3. example rules are there for a reason - but once we do the rules thing above, then I propose we have several rules files - something like: examples.json securityissuesonly.json appreview.json etc

IE rule files which match use cases

[pauljt]

I'm just testing command line, then I am going to merge.

[pauljt]

Ignore my post above, I hadn't merged freddy's changes which did actually break these things (just the location of the rules I think). Fixing atm

[pauljt]

Ok fixed these, except for the rules, and command line scanner. Fixing those now.

[pauljt]

Ok so tests pass, client tabs work (except for rules), and command line scanner works. Going to test a little more, and then merge if no issues.