bcgov / business-filings-ui

BC Registry Services - Legal Entities - Business Dashboard and Filings
Apache License 2.0
9 stars 52 forks source link

16552 Added new eslint presets #525

Closed JazzarKarim closed 1 year ago

JazzarKarim commented 1 year ago

Issue #: /bcgov/entity#16552

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the business-filings-ui license (Apache 2.0).

JazzarKarim commented 1 year ago

Final terminal after fixing all the issues: final

codecov[bot] commented 1 year ago

Codecov Report

Merging #525 (7866239) into main (1ac27d5) will increase coverage by 0.21%. The diff coverage is 99.00%.

@@            Coverage Diff             @@
##             main     #525      +/-   ##
==========================================
+ Coverage   92.44%   92.66%   +0.21%     
==========================================
  Files         171      174       +3     
  Lines        2793     2834      +41     
  Branches      309      314       +5     
==========================================
+ Hits         2582     2626      +44     
+ Misses        210      207       -3     
  Partials        1        1              
Impacted Files Coverage Δ
src/App.vue 100.00% <ø> (ø)
src/components/AnnualReport/AGMDate.vue 100.00% <ø> (ø)
src/components/AnnualReport/ARDate.vue 100.00% <ø> (ø)
src/components/Dashboard/AddressListSm.vue 100.00% <ø> (ø)
.../components/Dashboard/Alerts/FrozenInformation.vue 100.00% <ø> (ø)
...components/Dashboard/Alerts/MissingInformation.vue 100.00% <ø> (ø)
...rc/components/Dashboard/Alerts/NotInCompliance.vue 100.00% <ø> (ø)
.../components/Dashboard/Alerts/NotInGoodStanding.vue 100.00% <ø> (ø)
src/components/Dashboard/CustodianListSm.vue 100.00% <ø> (ø)
src/components/Dashboard/DirectorListSm.vue 100.00% <ø> (ø)
... and 97 more

... and 2 files with indirect coverage changes

JazzarKarim commented 1 year ago

/gcbrun

pwei1018 commented 1 year ago

Temporary Url for review: https://business-filings-dev--pr-525-qukvvnj7.web.app

severinbeauvais commented 1 year ago

I'm clearing up some SonarCloud Security Hotspots...

Can you look into this one, please? I'm curious what the "built-in Vue sanitization" is.

image

JazzarKarim commented 1 year ago

I'm clearing up some SonarCloud Security Hotspots...

Can you look into this one, please? I'm curious what the "built-in Vue sanitization" is.

image

So Sev, I've been looking into this. Apparently there is some sort of security risk for binding the href attribute. It's something called URL Injection (REF: https://medium.com/@isaacwangethi30/vue-js-security-6e246a7613da).

URL injection URL injection 2

HOWEVER, I don't think it applies in our situation since the links we're binding to in the project are not user-provided URLs. What do you think?

severinbeauvais commented 1 year ago

HOWEVER, I don't think it applies in our situation since the links we're binding to in the project are not user-provided URLs. What do you think?

I agree. I'll mark the ones where we provide the URL as "won't fix".

severinbeauvais commented 1 year ago

HOWEVER, I don't think it applies in our situation since the links we're binding to in the project are not user-provided URLs. What do you think?

I agree. I'll mark the ones where we provide the URL as "won't fix".

On further thought, it may possible to inject a different href here (or v-html in other cases): the app code may be editable in the browser (inspecting sources). But I'm not going to worry about this right now.

JazzarKarim commented 1 year ago

I totally forgot about the code smells that SonarCloud complain about whenever I have empty functions 😞 I removed the no empty function comments in the code since there is a rule that's set to off in regards to that.

sonarcloud[bot] commented 1 year ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 52 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

JazzarKarim commented 1 year ago

@bennypc I'm going to merge. Please let me know if there still any outstanding issues, I'll fix them in a separate PR. Thanks 😄