-
Notifications
You must be signed in to change notification settings - Fork 5.1k
[Guitar Tools] Add audio device selection for external audio interfaces #24280
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
base: main
Are you sure you want to change the base?
Conversation
|
Thank you for your first contribution! 🎉 🔔 @narghev you might want to have a look. You can use this guide to learn how to check out the Pull Request locally in order to test it. 📋 Quick checkout commandsBRANCH="main"
FORK_URL="https://github.com/4fun50/extensions.git"
EXTENSION_NAME="guitar-tools"
REPO_NAME="extensions"
git clone -n --depth=1 --filter=tree:0 -b $BRANCH $FORK_URL
cd $REPO_NAME
git sparse-checkout set --no-cone "extensions/$EXTENSION_NAME"
git checkout
cd "extensions/$EXTENSION_NAME"
npm install && npm run devDue to our current reduced availability, the initial review may take up to 10-15 business days. |
4fun50
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updating regex
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR adds audio device selection functionality to the Guitar Tools extension, allowing users to choose their preferred audio input device (particularly useful for external audio interfaces).
Key changes:
- New "List Audio Devices" command that parses
system_profileroutput to display available audio input devices - LocalStorage-based persistence for selected device preference
- Modified tuner to use selected device via Sox's CoreAudio interface
Critical issue found:
- Shell injection vulnerability in
note.utils.tsline 67 where device name is interpolated into shell command without proper escaping
Minor issues:
- CHANGELOG has duplicate headers and inconsistent dates
- React key using array index instead of unique identifier
Confidence Score: 2/5
- This PR has a critical security vulnerability that must be fixed before merging
- Score reflects the shell injection vulnerability in note.utils.ts where the audio device name is inserted into a shell command without proper escaping. This could allow command injection if a device name contains shell metacharacters. The rest of the implementation is sound.
- Pay critical attention to
extensions/guitar-tools/src/utils/note.utils.ts- the shell command construction needs proper escaping
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| extensions/guitar-tools/src/utils/note.utils.ts | 2/5 | Added audio device selection with shell injection vulnerability in command construction (line 67) |
| extensions/guitar-tools/src/list-audio-devices.tsx | 4/5 | New command to list and select audio devices, uses array index as React key which should be unique ID |
| extensions/guitar-tools/CHANGELOG.md | 4/5 | Updated with new features, has duplicate headers and date inconsistencies |
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
0xdhrv
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor suggestion
|
Hi 👋 Thanks for your contribution 💪 I have now tested your extension, and I have some feedback ready for you:
I'm looking forward to testing this extension again 🔥 I converted this PR into a draft until it's ready for the review, please press the button Feel free to contact me here or at Slack if you have any questions. |
Remove duplicate headers and fix date inconsistencies
Fix shell injection vulnerability in audio device selection
Co-authored-by: Dhruv Suthar <[email protected]>
Description
This PR adds the ability to select a preferred audio input device for the Guitar Tools tuner, which is particularly useful for guitarists using external audio interfaces or sound cards to connect their instruments.
Changes
How to use
Testing
Tested successfully with:
Screenshots
(Optional: you can add screenshots later if needed)