Scanframe DevOps Trial App 0.1.0
Loading...
Searching...
No Matches
Review Remarks


👀 2026-10-19T21:38 General: Junie

⚠️ 1: Misleading C++ standard comment in root CMakeLists.txt

🔎 Finding

The comment on line 54 of CMakeLists.txt:54 states "Set the C++ standard to 20 for all projects" but the actual code sets CMAKE_CXX_STANDARD to 17 on line 55 and CMAKE_C_STANDARD to 17 on line 56.

💡 Remedy/Fix

Correct the comment to reflect the actual C++17 standard being set, or update the standard values to C++20 if that was the original intent.

✅ Remediation

Modified as instructed.

⚠️ 2: Security risk — unsafe git safe.directory configuration in CI pipelines

🔎 Finding

All four CI/CD pipeline files set git config --global --add safe.directory '*' which disables Git's safe directory check globally:

This is a security risk as it allows any directory to be treated as safe, potentially enabling malicious repository execution.

💡 Remedy/Fix

Replace '*' with the actual working directory path, e.g., git config --global --add safe.directory "$(pwd)" or use git config --add safe.directory "$(pwd)" without --global to limit scope.

✅ Remediation

It is running in a GitLab build docker-container so what harm can it do?

⚠️ 3: Security risk — environment variable dump exposes secrets in CI pipelines

🔎 Finding

Multiple CI/CD files execute printenv | sort which dumps all environment variables to the log, potentially exposing sensitive credentials like NEXUS_USER, NEXUS_PASSWORD, and other tokens:

💡 Remedy/Fix

Remove the printenv | sort command from CI scripts, or filter out sensitive variables (those containing PASSWORD, TOKEN, SECRET, USER) before printing.

✅ Remediation

No problem, GitLab hides any potential passwords and tokens. It is required to do debugging when a CI/CD pipeline fails.

⚠️ 4: Missing semicolon in hello.cpp

🔎 Finding

File hello.cpp:118 is missing a semicolon after the return statement:

return {"Unknown ?.?.?"}

This will cause a compilation error.

💡 Remedy/Fix

Add the missing semicolon:

return {"Unknown ?.?.?"};

✅ Remediation

Modified as suggested.

⚠️ 5: Duplicate include with hardcoded relative path in main.cpp

🔎 Finding

File main.cpp:1 includes hello.h using a hardcoded relative path #include "../../cmake/lib/tpl/root/src/hwl/inc/hwl/hello.h", then again on line 8 with the proper include #include <hwl/hello.h>. The first include is redundant and fragile.

💡 Remedy/Fix

Remove the hardcoded relative path include on line 1 and keep only the proper include on line 8.

✅ Remediation

Modified as suggested.

⚠️ 6: Unvalidated user input in main.cpp

🔎 Finding

File main.cpp:27 uses std::strtol(argv[1], nullptr, 10) without validating that argv[1] is a valid number. If a non-numeric string is passed, the behavior is undefined and seconds will contain garbage.

💡 Remedy/Fix

Add input validation before calling strtol, or use std::stoi with proper exception handling.

✅ Remediation

Modified to std::stoi as suggested.

⚠️ 7: Missing includes in test-strings.cpp

🔎 Finding

File test-strings.cpp:7 uses std::vector<std::string> without including <vector> or <string>. While <catch2/catch_all.hpp> might transitively include these headers, relying on transitive includes is fragile and non-portable.

💡 Remedy/Fix

Add explicit includes:

#include <string>
#include <vector>

✅ Remediation

Modified as suggested.

⚠️ 8: Missing include in test-json.cpp

🔎 Finding

File test-json.cpp:51 uses std::setw without including <iomanip>.

💡 Remedy/Fix

Add the missing include:

#include <iomanip>

✅ Remediation

Modified as suggested.

⚠️ 9: Missing includes in google/test-hello.cpp

🔎 Finding

File test-hello.cpp:19 uses std::stringstream and std::string without including <sstream> and <string>.

💡 Remedy/Fix

Add the missing includes:

#include <sstream>
#include <string>

✅ Remediation

Modified as suggested.

⚠️ 10: Incorrect const qualifier on QApplication in qt/main.cpp

🔎 Finding

File main.cpp:9 declares QApplication as const:

QApplication const app(argc, argv);

QApplication should not be const as it requires mutable state for event processing.

💡 Remedy/Fix

Remove the const qualifier:

✅ Remediation

The const is no problem at the moment.

QApplication app(argc, argv);

⚠️ 11: CMake anti-pattern — file (GLOB) used in multiple CMakeLists.txt files

🔎 Finding

The file(GLOB ...) anti-pattern is used in multiple CMake files to collect source files:

Using file(GLOB) to collect sources is discouraged by CMake because adding new files won't trigger reconfiguration, leading to build failures.

💡 Remedy/Fix

Replace file(GLOB ...) with explicit source file lists using set(...) or target_sources(...) with individual files listed.

✅ Remediation

It is designed that way and GLOB is only used in tests, so adding a test is easy.

⚠️ 12: CMakePresets.json version mismatch with CMake minimum requirement

🔎 Finding

File CMakePresets.json:2 uses preset schema version 6, which requires CMake 3.29+, but the cmakeMinimumRequired on lines 3-6 specifies CMake 3.25.0. This creates a version mismatch where the presets cannot be used with the declared minimum CMake version.

💡 Remedy/Fix

Either increase cmakeMinimumRequired to CMake 3.29 or lower the preset version to one compatible with CMake 3.25.

✅ Remediation

Increased cmakeMinimumRequired to CMake 3.29.

⚠️ 13: Broad exception handling in init-repo.py

🔎 Finding

File init-repo.py:168 uses except Exception as ex in fix_submodule_url() which silently catches all exceptions including critical ones like KeyboardInterrupt and SystemExit.

💡 Remedy/Fix

Narrow the exception handling to catch only subprocess.CalledProcessError and specific expected exceptions, and re-raise KeyboardInterrupt and SystemExit.

✅ Remediation

It is as designed that way.

⚠️ 14: Shell injection risk in build.py

🔎 Finding

File build.py:1440 passes shell=shell to subprocess.run(). If user-controlled input reaches this function with shell=True, it could enable shell injection attacks.

💡 Remedy/Fix

Ensure that shell=True is only used with fully trusted, hardcoded commands and never with user input. Add input validation or prefer list-based command invocation.

✅ Remediation

Very unlikely this happens in a situation where you are using the script to build C++ projects.

⚠️ 15: Inconsistent C++ standard between root and test CMakeLists.txt

🔎 Finding

The root CMakeLists.txt:55 sets CMAKE_CXX_STANDARD to 17, but tests/app/CMakeLists.txt:9 overrides it to 20. This inconsistency can cause subtle compilation issues when the same headers are compiled with different standards.

✅ Remediation

It is a separate project for testing purposes and the C++ standard is intentionally set to 20 for that specific project.

💡 Remedy/Fix

Align the C++ standard across all CMakeLists.txt files or document the intentional difference.

✅ Remediation

Modified as

⚠️ 16: .editorconfig max_line_length conflicts with .clang-format ColumnLimit

🔎 Finding

File .editorconfig:7 sets max_line_length = 120, but .clang-format:93 sets ColumnLimit: 160. This inconsistency can cause confusion and formatting conflicts between editors and clang-format.

💡 Remedy/Fix

Align the line length limits between .editorconfig and .clang-format to the same value.

✅ Remediation

Added this to .editorconfig file.

[*.{c++,cc,cpp,cxx,h,h++,hh,hpp,hxx,inl,ipp,tlh,tli}]
max_line_length = 160

⚠️ 17: Missing test coverage for template.h functions

🔎 Finding

File template.h contains two template functions (calculateOffset and clip), but test-template.cpp only tests calculateOffset — the clip function has no test coverage.

💡 Remedy/Fix

Add test cases for the clip function in test-template.cpp.

✅ Remediation

Only there for testing Doxygen document generator.

⚠️ 18: README.md references non-existent init-repo.sh

🔎 Finding

File README.md:111 references init-repo.sh, but the actual file is init-repo.py (Python, not shell script).

💡 Remedy/Fix

Update the reference in README.md to point to init-repo.py instead of init-repo.sh.

✅ Remediation

Modified as suggested.