[ERA-8079] #810
[ERA-8079] #810
Conversation
.githooks/pre-commit
Outdated
| @@ -48,13 +48,13 @@ | |||
| # # If there are whitespace errors, print the offending file names and fail. | |||
| # exec git diff-index --check --cached $against -- | |||
There was a problem hiding this comment.
Once we merge this PR this should be reverted:
#809
There was a problem hiding this comment.
I'm actually going to leave this commented out for now.
https://overreacted.io/npm-audit-broken-by-design/
webpack/loader-utils#212
facebook/create-react-app#11102
https://security.snyk.io/vuln/SNYK-JS-LOADERUTILS-3043105
Even after upgrading dependencies, I'm seeing more "critical" vulnerabilities, but digging through each of them you'll see similar concerns as expressed above; namely "Prototype pollution in webpack loader-utils" being used as a mechanism for denial of service...something wholly unlikely to happen, as it's an aspect of build tools for the application and thus fully within the control/domain of the installer/developer (unless their machine access is compromised, at which point you have much bigger security issues to worry about).
We should still look at the output and investigate each, as well as diligently upgrade dependencies, but it seems like relying on that audit output for pass/fail is ill-conceived. live and learn.
| # then | ||
| # echo "\033[33mCommit failed, critical security vulnerability found in one of the project's dependencies. Please resolve as per the audit report above, then re-commit.\033[0m" | ||
| # exit 1 | ||
| # fi |
There was a problem hiding this comment.
I may be out of context. What's the reason for commenting these lines?
There was a problem hiding this comment.
See my comment above:
Once we merge this PR this should be reverted:
#809
If your pre-commit hooks are running correctly, commits currently get rejected because of critical dependency vulnerabilities which are fixed ^in that above linked PR
| const eventReportTracker = trackEventFactory(EVENT_REPORT_CATEGORY); | ||
|
|
||
| const calculateInputDisplayString = (location, gpsFormat) => { | ||
| const calculateInputDisplayString = (location, gpsFormat, placeholder) => { |
There was a problem hiding this comment.
I know you didn't add this, but honestly, having a method for a one liner feels like an overkill haha
const displayString = location ? calcGpsDisplayString(location[1], location[0], gpsFormat) : placeholder;
|
Hey @JoshuaVulcan I still see the Copy to clipboard button there 🤔 I just re-ran the pipeline in the morning and this time it seemed to work fine, but not sure if there's anything missing there 🤔
|
Funky, let me re-run the front-end build too and see what's up. |
You were right man, I just re-ran the BE pipeline to make the env reachable but I had to re-ran the FE as well haha, thanks for that |
No longer showing a "copy to clipboard" control for location point controls with placeholders displayed. A little test coverage to prove it :-)
https://allenai.atlassian.net/browse/ERA-8079
https://era-8079.pamdas.org/