Update GitHub repo references from feedback* to feedBack*#29
Conversation
|
@coderabbitai review |
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughThis PR renames repository references from "feedback" to "feedBack" casing across README, build docs, shell scripts, and application source files (URLs, comments, and feed links). It also modifies the Linux dependency install step and ffmpeg encoder check logic, and removes content from several resource files. ChangesfeedBack Repository Rename
Unrelated Build Script and Resource Changes
Additionally, Estimated code review effort: 2 (Simple) | ~10 minutes Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/bundle-binaries.sh`:
- Around line 88-95: The ffmpeg encoder check in bundle-binaries.sh uses a fixed
/tmp/ffmpeg_enc_check path, which is predictable and vulnerable to
symlink/TOCTOU issues. Update the logic around the ffmpeg and grep check to
avoid a shared temp file entirely, preferably by piping the output of
"$BIN_DIR/ffmpeg" -hide_banner -encoders directly into grep like the Windows
build script does; if a temp file is still needed, use mktemp and ensure it is
cleaned up reliably.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d940da3f-38f7-466b-af7d-422ecd8d6454
⛔ Files ignored due to path filters (14)
resources/default-irs/1x12_Open_Fender.wavis excluded by!**/*.wavresources/default-irs/2x12_Closed_Marshall.wavis excluded by!**/*.wavresources/default-irs/4x12_Closed_V30.wavis excluded by!**/*.wavresources/default-irs/4x12_Modern_Metal.wavis excluded by!**/*.wavresources/default-irs/guitar_amp.wavis excluded by!**/*.wavresources/icons/128x128.pngis excluded by!**/*.pngresources/icons/16x16.pngis excluded by!**/*.pngresources/icons/256x256.pngis excluded by!**/*.pngresources/icons/32x32.pngis excluded by!**/*.pngresources/icons/48x48.pngis excluded by!**/*.pngresources/icons/512x512.pngis excluded by!**/*.pngresources/icons/64x64.pngis excluded by!**/*.pngresources/icons/icon.icois excluded by!**/*.icoresources/icons/icon.svgis excluded by!**/*.svg
📒 Files selected for processing (20)
README.mdWINDOWS_BUILD_REQUIREMENTS.mdresources/default-irs/.gitkeepresources/entitlements.mac.plistresources/icons/.gitkeepresources/icons/icon.icnsresources/models/README.mdresources/models/basic_pitch.onnxresources/soundfonts/LICENSEscripts/build-common.shscripts/build-linux-ubuntu.shscripts/bundle-binaries.shscripts/bundle-python.shscripts/bundle-slopsmith.shsrc/main/ipc-channels.tssrc/main/main.tssrc/main/preload.tssrc/main/soundfont-manager.tssrc/main/update-manager.tssrc/renderer/settings.html
💤 Files with no reviewable changes (3)
- resources/soundfonts/LICENSE
- resources/entitlements.mac.plist
- resources/models/README.md
| "$BIN_DIR/ffmpeg" -hide_banner -encoders >/tmp/ffmpeg_enc_check 2>/dev/null | ||
| if ! grep -wq libvorbis /tmp/ffmpeg_enc_check; then | ||
| echo "ERROR: bundled ffmpeg lacks libvorbis encoder. Sloppak conversion would fall back to the lower-quality built-in vorbis encoder on user machines." >&2 | ||
| echo "Install an ffmpeg built with --enable-libvorbis (apt's ffmpeg ships it by default; check your distro's package if this fails)." >&2 | ||
| rm -f /tmp/ffmpeg_enc_check | ||
| exit 1 | ||
| fi | ||
| rm -f /tmp/ffmpeg_enc_check |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Insecure predictable temp file path.
/tmp/ffmpeg_enc_check is a fixed, predictable path — vulnerable to symlink/TOCTOU attacks on shared build hosts (CWE-377). Use mktemp, or better, avoid the temp file entirely with a direct pipe/process substitution like the Windows build script does.
🔒 Proposed fix
-"$BIN_DIR/ffmpeg" -hide_banner -encoders >/tmp/ffmpeg_enc_check 2>/dev/null
-if ! grep -wq libvorbis /tmp/ffmpeg_enc_check; then
+if ! "$BIN_DIR/ffmpeg" -hide_banner -encoders 2>/dev/null | grep -wq libvorbis; then
echo "ERROR: bundled ffmpeg lacks libvorbis encoder. Sloppak conversion would fall back to the lower-quality built-in vorbis encoder on user machines." >&2
echo "Install an ffmpeg built with --enable-libvorbis (apt's ffmpeg ships it by default; check your distro's package if this fails)." >&2
- rm -f /tmp/ffmpeg_enc_check
exit 1
fi
-rm -f /tmp/ffmpeg_enc_check📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "$BIN_DIR/ffmpeg" -hide_banner -encoders >/tmp/ffmpeg_enc_check 2>/dev/null | |
| if ! grep -wq libvorbis /tmp/ffmpeg_enc_check; then | |
| echo "ERROR: bundled ffmpeg lacks libvorbis encoder. Sloppak conversion would fall back to the lower-quality built-in vorbis encoder on user machines." >&2 | |
| echo "Install an ffmpeg built with --enable-libvorbis (apt's ffmpeg ships it by default; check your distro's package if this fails)." >&2 | |
| rm -f /tmp/ffmpeg_enc_check | |
| exit 1 | |
| fi | |
| rm -f /tmp/ffmpeg_enc_check | |
| if ! "$BIN_DIR/ffmpeg" -hide_banner -encoders 2>/dev/null | grep -wq libvorbis; then | |
| echo "ERROR: bundled ffmpeg lacks libvorbis encoder. Sloppak conversion would fall back to the lower-quality built-in vorbis encoder on user machines." >&2 | |
| echo "Install an ffmpeg built with --enable-libvorbis (apt's ffmpeg ships it by default; check your distro's package if this fails)." >&2 | |
| exit 1 | |
| fi |
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 88-88: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/ffmpeg_enc_check
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
[warning] 91-91: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/ffmpeg_enc_check
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
[warning] 94-94: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/ffmpeg_enc_check
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/bundle-binaries.sh` around lines 88 - 95, The ffmpeg encoder check in
bundle-binaries.sh uses a fixed /tmp/ffmpeg_enc_check path, which is predictable
and vulnerable to symlink/TOCTOU issues. Update the logic around the ffmpeg and
grep check to avoid a shared temp file entirely, preferably by piping the output
of "$BIN_DIR/ffmpeg" -hide_banner -encoders directly into grep like the Windows
build script does; if a temp file is still needed, use mktemp and ensure it is
cleaned up reliably.
Source: Linters/SAST tools
|
Queue note — flagged from a full-queue integration dry-run. This is one of ~30 near-identical Recommend landing the whole rebrand as one coordinated batch (or closing them if the rename is on hold) rather than piecemeal. Flagging so it's a deliberate call, not something that quietly rots in the queue. |
Summary by CodeRabbit