Add propelcode test conditions in score/evaluation by devendrapat · Pull Request #2 · devendrapat/logging

devendrapat · 2026-03-23T15:44:03Z

Notes for Reviewer

Pre-Review Checklist for the PR Author

PR title is short, expressive and meaningful
Commits are properly organized
Relevant issues are linked in the References section
Tests are conducted
Unit tests are added

Checklist for the PR Reviewer

Commits are properly organized and messages are according to the guideline
Unit tests have been written for new behavior
Public API is documented
PR title describes the changes

Post-review Checklist for the PR Author

All open points are addressed and tracked via issues

References

Closes #

propel-code-bot · 2026-03-23T15:44:38Z

Add evaluation sample modules, tests, and Bazel targets for score/evaluation

This PR introduces a large set of new evaluation-focused C++ headers under score/evaluation, covering areas like design patterns, safety, concurrency, exception handling, memory, idiomatic/modern C++, optimization, SOLID violations, security issues, and template metaprogramming. A new driver in score/evaluation/evaluation_main.cpp exercises many of these samples, and score/evaluation/BUILD adds alias targets plus an evaluation_all binary that depends on the new libraries.

It also adds a new unit_test_quality library with inline implementations and a gtest/gmock-based test suite in score/evaluation/unit_test_quality/unit_test_quality_test.cpp. Each module now has a dedicated BUILD file declaring a cc_library, and the unit test target is wired into the build graph.

This summary was automatically generated by @propel-code-bot

propel-code-bot

Review found numerous critical security, concurrency, and memory-safety defects plus API/test hygiene gaps that must be resolved before merge.

Status: Action Required | Risk: High

Issues Identified & Suggestions

Fix security flaws: security_issues.h, non_idiomatic_cpp.h
Resolve concurrency/memory safety issues: concurrency_issues.h, memory_issues.h
Fix header/ODR, logic, tests/docs, unused code: design_patterns.h, unit_test_quality.h, unit_test_quality_test.cpp

Review Details

📁 29 files reviewed | 💬 51 comments

👍 / 👎 individual comments to help improve reviews for you

propel-code-bot · 2026-03-23T16:35:51Z

score/evaluation/idiomatic/non_idiomatic_cpp.h

+inline std::string FormatMessage(int code, const std::string& text)
+{
+    char buf[256];
+    sprintf(buf, "Code=%d Msg=%s", code, text.c_str());


[Security] sprintf is used to write into a fixed-size buffer buf[256]. If the input text is long enough, it will cause a buffer overflow, potentially leading to a crash or security vulnerability.

Suggested change

sprintf(buf, "Code=%d Msg=%s", code, text.c_str());

char buf[256];

snprintf(buf, sizeof(buf), "Code=%d Msg=%s", code, text.c_str());

Context for Agents

[**Bug**] `sprintf` is used to write into a fixed-size buffer `buf[256]`. If the input `text` is long enough, it will cause a buffer overflow, potentially leading to a crash or security vulnerability. ```suggestion char buf[256]; snprintf(buf, sizeof(buf), "Code=%d Msg=%s", code, text.c_str()); ``` File: score/evaluation/idiomatic/non_idiomatic_cpp.h Line: 59

propel-code-bot · 2026-03-23T16:35:51Z

score/evaluation/security/security_issues.h

+
+inline std::string BuildShellCommand(const std::string& file_name)
+{
+    return "cat " + file_name;


[Security] BuildShellCommand is vulnerable to command injection. If file_name contains shell metacharacters (e.g., ; rm -rf /), they will be executed by the shell when this command is eventually run.

Context for Agents

[**Bug**] `BuildShellCommand` is vulnerable to command injection. If `file_name` contains shell metacharacters (e.g., `; rm -rf /`), they will be executed by the shell when this command is eventually run. File: score/evaluation/security/security_issues.h Line: 43

propel-code-bot · 2026-03-23T16:35:51Z

score/evaluation/security/security_issues.h

+
+inline std::string BuildUserLookupQuery(const std::string& user_name)
+{
+    return "SELECT * FROM users WHERE name='" + user_name + "'";


[Security] BuildUserLookupQuery is vulnerable to SQL injection. An attacker can provide a user_name containing SQL fragments (e.g., ' OR '1'='1) to bypass authentication or extract unauthorized data.

Context for Agents

[**Bug**] `BuildUserLookupQuery` is vulnerable to SQL injection. An attacker can provide a `user_name` containing SQL fragments (e.g., `' OR '1'='1`) to bypass authentication or extract unauthorized data. File: score/evaluation/security/security_issues.h Line: 48

propel-code-bot · 2026-03-23T16:35:51Z

score/evaluation/security/security_issues.h

+
+inline std::string BuildUserFilePath(const std::string& user_fragment)
+{
+    return std::string("/var/app/data/") + user_fragment;


[Security] BuildUserFilePath is vulnerable to path traversal. An attacker can provide a user_fragment containing directory traversal sequences (e.g., ../../etc/passwd) to access sensitive files outside the intended directory.

Context for Agents

[**Bug**] `BuildUserFilePath` is vulnerable to path traversal. An attacker can provide a `user_fragment` containing directory traversal sequences (e.g., `../../etc/passwd`) to access sensitive files outside the intended directory. File: score/evaluation/security/security_issues.h Line: 53

propel-code-bot · 2026-03-23T16:35:51Z

score/evaluation/concurrency/concurrency_issues.h

+public:
+    void EnsureInitialised()
+    {
+        if (!initialised_)


[Logic] The initialised_ flag is read outside of a lock, while it may be written by another thread inside a lock. This constitutes a data race and can lead to undefined behavior. Both reads and writes to shared data must be synchronized.

Context for Agents

The `initialised_` flag is read outside of a lock, while it may be written by another thread inside a lock. This constitutes a data race and can lead to undefined behavior. Both reads and writes to shared data must be synchronized. File: score/evaluation/concurrency/concurrency_issues.h Line: 50

propel-code-bot · 2026-03-23T16:35:53Z

score/evaluation/memory/memory_issues.h

+    int* ptr_;
+};
+
+class LeakyConstructor


[Unused Code] The class LeakyConstructor is defined here but nothing in this PR constructs or references it (searched all files). Consider removing it if it’s not needed.

Context for Agents

The class `LeakyConstructor` is defined here but nothing in this PR constructs or references it (searched all files). Consider removing it if it’s not needed. File: score/evaluation/memory/memory_issues.h Line: 63

propel-code-bot · 2026-03-23T16:35:53Z

score/evaluation/optimization/code_optimization.h

+    return sum;
+}
+
+inline void ProcessItems(const std::vector<std::string>& items)


[Unused Code] The function ProcessItems is defined but never called anywhere in this PR (confirmed via full search). It appears to be unused code.

Context for Agents

The function `ProcessItems` is defined but never called anywhere in this PR (confirmed via full search). It appears to be unused code. File: score/evaluation/optimization/code_optimization.h Line: 24

propel-code-bot · 2026-03-23T16:35:53Z

score/evaluation/idiomatic/non_idiomatic_cpp.h

+
+#include <cstdio>
+#include <iostream>
+#include <numeric>


[Maintainability] No symbols from <numeric> are used in this header. Remove the include to avoid unnecessary build dependencies.

Context for Agents

No symbols from `<numeric>` are used in this header. Remove the include to avoid unnecessary build dependencies. File: score/evaluation/idiomatic/non_idiomatic_cpp.h Line: 8

propel-code-bot · 2026-03-23T16:35:53Z

score/evaluation/modern_cpp/modern_cpp_syntax.h

+#include <algorithm>
+#include <functional>
+#include <map>
+#include <memory>


[Maintainability] No symbols from <memory> are used in this header; remove the include to reduce build overhead.

Context for Agents

No symbols from `<memory>` are used in this header; remove the include to reduce build overhead. File: score/evaluation/modern_cpp/modern_cpp_syntax.h Line: 9

propel-code-bot · 2026-03-23T16:35:53Z

score/evaluation/unit_test_quality/unit_test_quality.h

+#ifndef SCORE_EVALUATION_UNIT_TEST_QUALITY_H
+#define SCORE_EVALUATION_UNIT_TEST_QUALITY_H
+
+#include <cstdint>


[Maintainability] No <cstdint> types are used in this header; remove the include to keep dependencies minimal.

Context for Agents

No `<cstdint>` types are used in this header; remove the include to keep dependencies minimal. File: score/evaluation/unit_test_quality/unit_test_quality.h Line: 6

Add propelcode test conditions in score/evaluation

ab8c251

devendrapat had a problem deploying to workflow-approval March 23, 2026 15:44 — with GitHub Actions Error

devendrapat had a problem deploying to workflow-approval March 23, 2026 15:44 — with GitHub Actions Failure

Remove docs from score/evaluation

00ff779

devendrapat had a problem deploying to workflow-approval March 23, 2026 15:57 — with GitHub Actions Error

devendrapat had a problem deploying to workflow-approval March 23, 2026 15:57 — with GitHub Actions Failure

devendrapat had a problem deploying to workflow-approval March 23, 2026 16:09 — with GitHub Actions Error

devendrapat had a problem deploying to workflow-approval March 23, 2026 16:09 — with GitHub Actions Failure

propel-code-bot bot reviewed Mar 23, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add propelcode test conditions in score/evaluation#2

Add propelcode test conditions in score/evaluation#2
devendrapat wants to merge 2 commits intomainfrom
pr1_propelcode_evaluation

devendrapat commented Mar 23, 2026

Uh oh!

propel-code-bot bot commented Mar 23, 2026 •

edited

Loading

Uh oh!

propel-code-bot bot left a comment

Uh oh!

propel-code-bot bot Mar 23, 2026

Uh oh!

propel-code-bot bot Mar 23, 2026

Uh oh!

propel-code-bot bot Mar 23, 2026

Uh oh!

propel-code-bot bot Mar 23, 2026

Uh oh!

propel-code-bot bot Mar 23, 2026

Uh oh!

propel-code-bot bot Mar 23, 2026

Uh oh!

propel-code-bot bot Mar 23, 2026

Uh oh!

propel-code-bot bot Mar 23, 2026

Uh oh!

propel-code-bot bot Mar 23, 2026

Uh oh!

propel-code-bot bot Mar 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	sprintf(buf, "Code=%d Msg=%s", code, text.c_str());
	char buf[256];
	snprintf(buf, sizeof(buf), "Code=%d Msg=%s", code, text.c_str());

Conversation

devendrapat commented Mar 23, 2026

Notes for Reviewer

Pre-Review Checklist for the PR Author

Checklist for the PR Reviewer

Post-review Checklist for the PR Author

References

Uh oh!

propel-code-bot bot commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

propel-code-bot bot left a comment

Choose a reason for hiding this comment

Uh oh!

propel-code-bot bot Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

propel-code-bot bot Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

propel-code-bot bot Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

propel-code-bot bot Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

propel-code-bot bot Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

propel-code-bot bot Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

propel-code-bot bot Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

propel-code-bot bot Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

propel-code-bot bot Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

propel-code-bot bot Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

propel-code-bot bot commented Mar 23, 2026 •

edited

Loading