Hackathon24
Summary of changes
The app should continue to work without clearing localStorage, but a few things might not function quite as intended.
- Added routing ~~via react-router-dom~~ built-in; the new
basenameprop passed to theBadMagiccomponent defines the Bad Magic URL root - Highlighted active item in nav sidebar
- JSON keys are in alphabetical order
- Syntax highlighting is applied to code blocks in Markdown docs
- Search text is persisted to localStorage
- Added a button to clear history (both at an individual endpoint level as well as globally)
- Fixed all the eslint warnings
- Created a config provider to store the given
BadMagicProps - Fixed various bugs where the incorrect route could be "matched" for a HistoricResponse (uniqueness requires workspace + path + http method + endpoint name)
- Added deprecated label to the nav sidebar
- Routes are sorted in nav somewhat logically
Testing
via example project
To test with the included example project, follow the instructions in README.md, i.e.:
- Run
yarn && yarn linkin the root directory - Run
yarn && yarn link badmagicin the./exampledirectory- Note that I had to use a different nodejs version because of
craco, if you have issues inside theexampledirectory, runasdf install && npm i -g yarn
- Note that I had to use a different nodejs version because of
- Run
yarnin the./example-apidirectory - In the root directory,
./exampledirectory, and./example-apidirectory, runyarn start
in CMW
You should also be able to test inside of CMW.
- First, verify you have not installed
yarnvia homebrew, or it'll cause issues:which yarn- If so,
brew uninstall yarn - Since
asdfshould be set up to read the.tool-versionsfile in your working directory, cd intobadmagic, runasdf installif necessary, and install yarn vianpm i -g yarn
- If so,
- cd into the root directory of
badmagic. Runyarn -vand verify that you are running on 1.x (i.e. 1.22.22) - Run
yarn && yarn build && yarn pack - The
yarn packcommand should return a path to the generated tarball file; copy it to your clipboard - cd into the root directory of
control.smartrent.com. Runyarn -vand verify that the yarn binary is being used from the project (i.e.3.2.0-rc.13) - Run
yarn add badmagic@file:<paste clipboard>, e.g.yarn add badmagic@/Users/yourname/code/badmagic/badmagic-v0.0.39.tgz - Open
assets/js/react/bundles/devtools/BadMagicClient.tsx. Add the propbasename="/dev/api"to the<BadMagic />component returned byBadMagicClient
Markdown CSS
I made some slight adjustments to the Markdown CSS file as well, though due to the fact that the syntax highlighter tries to automatically infer the language even without a language tag, I don't think you'll actually see the CSS changes because they were only affecting some unhighlighted code elements. Regardless, if you want to also test it with the latest CSS file:
- Make sure you've run
yarn buildinside of thebadmagicrepository root so thatmarkdown.min.cssis generated and put inside of./example/public/ - Make sure the example UI server is running, i.e.
cd ./example && yarn startand verify the example UI loads at http://localhost:3000/dev/api- We only need this because the example UI server also serves the stylesheet
- In CMW, open
apps/control_room/lib/control_room_web/templates/development/react/badmagic.html.eex. Replace theunpkgURL for BadMagic's stylesheet withhttp://localhost:3000/dev/api/markdown.min.css - Open
apps/control_room/lib/control_room_web/plugs/secure_browser_headers.ex. Find the"style-src"list and append the"http://localhost:3000"string to the list of allowed domains. - Run CMW as normal!
Most of the features are also testable in the demo app. An API server is included as well.
I can't get this to work with CMW because of three dependency conflicts:
- axios@npm:0.24.0 conflicts with parent dependency axios@npm:0.26.1
- react-native-svg@npm:13.14.0 conflicts with parent dependency react-native-svg@npm:12.3.0
- react-router-dom@npm:6.24.1 conflicts with parent dependency react-router-dom@npm:5.3.4
The axios and react-native-svg conflicts will be trivial to resolve. It'd be a lot of work for CMW to upgrade to react-router-dom. Any chance we could use v5 here instead? Do we have any apps already using v6?
I haven't been able to get the example working either. This is what I get after updating the badmagic dep on the client to point to ../ so it pulls the PR version instead of the latest published version.
So I'm going to try and see if I can get a full set of steps to install this locally, because there shouldn't be a dependency conflict issue, but I think that's creeping up because of how the badmagic module's dependencies are resolved specifically when trying to link the project locally rather than the normal install process. Because these conflicts shouldn't be conflicts; they are incompatible semver ranges, so both versions of the given packages should technically be installed. This was working for me locally when I was running my copy of Bad Magic in CMW and was pretty easy to confirm, because react-router-dom v4 has a completely different API, but both worked.
I'm probably not going to have time to pick this back up for a while
I'm not following your screenshot either though... I'm not sure I understand why the example built into the repo wouldn't be working... is that after following the instructions in the README, using yarn link and yarn link badmagic?
So regarding dependency conflicts:
I never touched axios or react-native-svg, so both versions of these packages should be getting installed and bundled, which I would expect to already be happening.
I was finding react-router-dom to be a trickier situation. In practice, it was actually running just fine, with the CMW app using react-router-dom@5 and the Bad Magic component rendering with react-router-dom@6. TypeScript, on the other hand, was very unhappy with the situation. I'm not sure if this was an issue with the method I was using to "link" and/or install the badmagic package locally. But basically the problem is, react-router-dom@5 does not bundle TypeScript declarations, so we have @types/react-router-dom installed as well. react-router-dom@6, however, includes the declarations with the package. It seems that because two versions of react-router-dom are installed in the root project (CMW), TypeScript was "preferring" to get the embedded declarations from react-router-dom@6 and ignoring @types/react-router-dom. I tried a few tricks to override this without much luck.
I don't really want to install react-router-dom@5 into the badmagic project, since a newer greenfield app that wants to use Bad Magic is likely going to install the latest React Router. I also can't just make react-router-dom@* one of the peerDependencies either because of the breaking changes. Considering how little routing is actually going on in the app, and that when we added the "copy link to clipboard" feature we didn't use any library at all, I'm considering just removing react-router-dom entirely and just inline what we need.
@mmiller42 Thanks. I got it working. I didn't see those instructions in the README.
I also managed to get it running in CMW (I must've had a bad cache somewhere previously). But I think the TS issues are going to be a problem with CMW adoption.
🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎
To accept the risk, merge this PR and you will not be notified again.
| Alert | Package | Note | Source | CI |
|---|---|---|---|---|
| Shell access | npm/[email protected] |
| ⚠︎ | |
| Network access | npm/[email protected] |
| ⚠︎ | |
| Network access | npm/[email protected] |
| ⚠︎ | |
| Network access | npm/[email protected] |
| ⚠︎ | |
| Network access | npm/[email protected] |
| ⚠︎ | |
| Network access | npm/[email protected] |
| ⚠︎ | |
| Network access | npm/[email protected] |
| ⚠︎ | |
| Network access | npm/[email protected] |
| ⚠︎ | |
| Network access | npm/[email protected] |
| ⚠︎ | |
| Shell access | npm/[email protected] |
| ⚠︎ | |
| AI warning | npm/@pmmmwh/[email protected] |
| 🚫 | |
| Shell access | npm/[email protected] |
| ⚠︎ | |
| Shell access | npm/[email protected] |
| ⚠︎ | |
| Network access | npm/[email protected] |
| ⚠︎ | |
| Network access | npm/[email protected] |
| ⚠︎ | |
| Network access | npm/[email protected] |
| ⚠︎ |
Ignoring: npm/[email protected]
Next steps
What is shell access?
This module accesses the system shell. Accessing the system shell increases the risk of executing arbitrary code.
Packages should avoid accessing the shell which can reduce portability, and make it easier for malicious shell access to be introduced.
What is network access?
This module accesses the network.
Packages should remove all network access that is functionally unnecessary. Consumers should audit network access to ensure legitimate use.
What is an AI-detected potential code anomaly?
AI has identified unusual behaviors that may pose a security risk.
An AI system found a low-risk anomaly in this package. It may still be fine to use, but you should check that it is safe before proceeding.
Take a deeper look at the dependency
Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.
Remove the package
If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.
Mark a package as acceptable risk
To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/[email protected] or ignore all packages with @SocketSecurity ignore-all
@SocketSecurity ignore npm/[email protected]@SocketSecurity ignore npm/[email protected]@SocketSecurity ignore npm/[email protected]@SocketSecurity ignore npm/[email protected]@SocketSecurity ignore npm/[email protected]@SocketSecurity ignore npm/[email protected]@SocketSecurity ignore npm/@pmmmwh/[email protected]@SocketSecurity ignore npm/[email protected]@SocketSecurity ignore npm/[email protected]@SocketSecurity ignore npm/[email protected]
@SocketSecurity ignore npm/[email protected]
@justincy Hold on I had a fix for this, I know it's very simple, but there was a required code change to CMW I believe and I forgot to mention it in the PR. I'll follow up here in a minute
EDIT: Something else must have broken in my recent changes -- you can fix the routing behavior in CMW with a simple change to apps/control_room/lib/control_room_web/routes/development.ex:
scope "/api" do
pipe_through(:badmagic_layout)
get("/", Development.ReactController, :api)
match(:*, "/*path", Development.ReactController, :api)
end
However, when refreshing you'll get a blank page -- it IS rendering BadMagic though, so there's a bug somewhere with matching the route 😅
EDIT 2: I think it actually is working now? Not sure, I think I had a caching issue... @justincy can you try adding this code change and testing it again?
Thanks for the approvals! Unfortunately merging this PR is blocked by this "CodeQL" test and I have no ay around it. I am not sure who has administrative access to the repo, nor how to publish a new version if I did merge.
Contacting GitHub about this. It should not be being blocked anymore.
@matthew-kerle still not sure what's up here 😅
Any update on this?