Conversation
There was a problem hiding this comment.
Pull request overview
This pull request introduces a new API test application for testing Firebolt C++ client interfaces. The application provides an interactive command-line tool (or automated mode with --auto flag) to exercise various Firebolt APIs including Accessibility, Advertising, Device, Discovery, Display, Lifecycle, Localization, Network, Presentation, Stats, and TextToSpeech. Additionally, the PR fixes memory leaks in the existing test utilities by properly freeing curl_slist headers.
Changes:
- Added new api-test-app under test/api-test-app/ with demo classes for 11 Firebolt interfaces
- Fixed memory leaks in test/utils.cpp by adding curl_slist_free_all() calls
- Updated CI/CD workflow to build and test the new application with mock-firebolt server
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| test/utils.cpp | Fixed memory leaks by properly freeing curl_slist headers after use |
| test/api-test-app/utils.h | Utility functions for console input and configuration management |
| test/api-test-app/utils.cpp | Implementation of console utilities including user input handling |
| test/api-test-app/main.cpp | Main application entry point with argument parsing and interface selection |
| test/api-test-app/cpp/*.h | Header files for demo classes covering 11 Firebolt interfaces |
| test/api-test-app/cpp/*.cpp | Implementation of demo classes with method invocations and event subscriptions |
| test/api-test-app/build.sh | Build script for standalone compilation of the test application |
| test/api-test-app/CMakeLists.txt | CMake configuration for building the api-test-app |
| src/CMakeLists.txt | Removed version properties that were causing issues |
| build.sh | Replaced bash-specific [[ ]] with POSIX-compliant [ ] for portability |
| CMakeLists.txt | Changed demo directory from demo/ to test/api-test-app/ |
| .github/workflows/ci.yml | Added api_test_app job to CI workflow, fixed spacing in workflow name |
| .github/scripts/run-cpp-test.sh | New script to run C++ tests with mock-firebolt server |
| .github/Dockerfile | Optimized RUN commands by combining into single layer |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3b08298 to
fe1d458
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 35 changed files in this pull request and generated 16 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
d537bfc to
b2ad80e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 34 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b2ad80e to
42dfa80
Compare
42dfa80 to
56ac177
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 34 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0c9d9c9 to
64e0064
Compare
64e0064 to
10c95bb
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 35 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
a001ce0 to
fa82bab
Compare
fa82bab to
c249c59
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 39 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
244f4d6 to
4ce672f
Compare
4ce672f to
2852e2b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 48 out of 48 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (3)
.github/scripts/run-api-test-app.sh:100
--portis parsed by this script, but the app is always launched with--mock --autoand no URL/port is passed through. Since--mockhardcodes the ws URL in the app, running with a non-default--portwon’t change the endpoint. ExportFIREBOLT_ENDPOINT=ws://127.0.0.1:$mockPort/when starting the app (or add a CLI option for URL).
test/api_test_app/build.sh:56SYSROOT_PATHis interpolated unquoted into thecmakecommand (-DSYSROOT_PATH=$SYSROOT_PATH), so shell metacharacters in the--sysrootargument orSYSROOT_PATHenvironment variable (e.g.;,&&) can break out of the intended command and execute arbitrary shell commands when this script runs. An attacker who can influenceSYSROOT_PATHcould achieve code execution in the context of the user runningbuild.sh. QuoteSYSROOT_PATH(and other path-like variables) in all command invocations so they are passed as single arguments rather than being parsed as shell syntax.
.github/scripts/run-api-test-app.sh:99- The test executable is launched via
"./$(basename "$testExe")" --mock --auto, wheretestExecomes directly from the--test-exeargument. If an attacker can control--test-exe, they can craft a basename containing a double quote and shell metacharacters so that the expanded command closes the surrounding quotes and appends arbitrary commands, leading to code execution when this script runs. Build the command using the fulltestExepath without embedding it inside an additional quoted string, or otherwise ensure the basename cannot contain characters that alter the shell syntax.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2852e2b to
323b249
Compare
323b249 to
ba9f2c4
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 48 out of 48 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Allow to invoke as `./build/api-test-app <test-suite.example`.
ba9f2c4 to
4409b80
Compare
|
🎉 This PR is included in version 0.5.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
No description provided.