Step 570: harden watch expression evaluator
This commit is contained in:
Bill
@@ -4009,4 +4009,13 @@ target_link_libraries(step569_test PRIVATE
|
||||
tree_sitter_javascript tree_sitter_typescript
|
||||
tree_sitter_java tree_sitter_rust tree_sitter_go)
|
||||
|
||||
add_executable(step570_test tests/step570_test.cpp)
|
||||
target_include_directories(step570_test PRIVATE src)
|
||||
target_link_libraries(step570_test PRIVATE
|
||||
nlohmann_json::nlohmann_json
|
||||
unofficial::tree-sitter::tree-sitter
|
||||
tree_sitter_python tree_sitter_cpp tree_sitter_elisp
|
||||
tree_sitter_javascript tree_sitter_typescript
|
||||
tree_sitter_java tree_sitter_rust tree_sitter_go)
|
||||
|
||||
# Step 12: Dear ImGui shell scaffolding created (main.cpp exists but not built due to dependencies)
|
||||
|
||||
210
editor/src/WatchExpressionEvaluatorHardening.h
Normal file
210
editor/src/WatchExpressionEvaluatorHardening.h
Normal file
@@ -0,0 +1,210 @@
|
||||
#pragma once
|
||||
// Step 570: Watch Expression Evaluator Hardening
|
||||
|
||||
#include <cctype>
|
||||
#include <cstdint>
|
||||
#include <map>
|
||||
#include <string>
|
||||
#include <vector>
|
||||
|
||||
struct WatchEvalLimits {
|
||||
std::size_t maxExpressionLength = 256;
|
||||
std::size_t maxTokens = 64;
|
||||
std::size_t maxOperations = 32;
|
||||
std::uint64_t timeoutBudgetMs = 32;
|
||||
};
|
||||
|
||||
struct WatchEvalResult {
|
||||
bool ok = false;
|
||||
bool timedOut = false;
|
||||
bool rejectedUnsafe = false;
|
||||
std::int64_t value = 0;
|
||||
std::size_t operations = 0;
|
||||
std::string error;
|
||||
};
|
||||
|
||||
class WatchExpressionEvaluatorHardening {
|
||||
public:
|
||||
static WatchEvalResult evaluate(const std::string& expression,
|
||||
const std::map<std::string, std::int64_t>& scope,
|
||||
const WatchEvalLimits& limits = {}) {
|
||||
WatchEvalResult result;
|
||||
if (expression.empty()) {
|
||||
fail(result, "expression_empty");
|
||||
return result;
|
||||
}
|
||||
if (expression.size() > limits.maxExpressionLength) {
|
||||
fail(result, "expression_too_long");
|
||||
return result;
|
||||
}
|
||||
if (containsUnsafePattern(expression)) {
|
||||
result.rejectedUnsafe = true;
|
||||
fail(result, "expression_unsafe");
|
||||
return result;
|
||||
}
|
||||
|
||||
std::vector<std::string> tokens;
|
||||
if (!tokenize(expression, limits.maxTokens, &tokens, &result.error)) return result;
|
||||
if (tokens.empty()) {
|
||||
fail(result, "expression_empty");
|
||||
return result;
|
||||
}
|
||||
|
||||
std::vector<std::int64_t> values;
|
||||
std::vector<char> operators;
|
||||
for (const auto& token : tokens) {
|
||||
if (isOperatorToken(token)) {
|
||||
operators.push_back(token[0]);
|
||||
} else {
|
||||
std::int64_t value = 0;
|
||||
if (!parseValue(token, scope, &value, &result.error)) return result;
|
||||
values.push_back(value);
|
||||
}
|
||||
}
|
||||
if (values.empty()) {
|
||||
fail(result, "value_missing");
|
||||
return result;
|
||||
}
|
||||
if (values.size() != operators.size() + 1) {
|
||||
fail(result, "operator_operand_mismatch");
|
||||
return result;
|
||||
}
|
||||
|
||||
if (!applyOperations(values, operators, &result, limits)) return result;
|
||||
result.ok = true;
|
||||
return result;
|
||||
}
|
||||
|
||||
private:
|
||||
static bool fail(WatchEvalResult& result, const char* code) {
|
||||
result.error = code;
|
||||
result.ok = false;
|
||||
return false;
|
||||
}
|
||||
|
||||
static bool containsUnsafePattern(const std::string& expression) {
|
||||
static const char* badSubstrings[] = {
|
||||
";", "{", "}", "[", "]", "__", "system", "exec", "include", "while", "for("
|
||||
};
|
||||
for (const auto* pattern : badSubstrings) {
|
||||
if (expression.find(pattern) != std::string::npos) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
static bool tokenize(const std::string& expression,
|
||||
std::size_t maxTokens,
|
||||
std::vector<std::string>* outTokens,
|
||||
std::string* error) {
|
||||
std::string current;
|
||||
auto flushCurrent = [&]() {
|
||||
if (!current.empty()) {
|
||||
outTokens->push_back(current);
|
||||
current.clear();
|
||||
}
|
||||
};
|
||||
|
||||
for (char c : expression) {
|
||||
if (std::isspace(static_cast<unsigned char>(c))) {
|
||||
flushCurrent();
|
||||
continue;
|
||||
}
|
||||
if (c == '+' || c == '-' || c == '*' || c == '/') {
|
||||
flushCurrent();
|
||||
outTokens->push_back(std::string(1, c));
|
||||
} else if (std::isalnum(static_cast<unsigned char>(c)) || c == '_') {
|
||||
current.push_back(c);
|
||||
} else {
|
||||
*error = "token_invalid_character";
|
||||
return false;
|
||||
}
|
||||
if (outTokens->size() > maxTokens) {
|
||||
*error = "token_limit_exceeded";
|
||||
return false;
|
||||
}
|
||||
}
|
||||
flushCurrent();
|
||||
if (outTokens->size() > maxTokens) {
|
||||
*error = "token_limit_exceeded";
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
static bool isOperatorToken(const std::string& token) {
|
||||
return token.size() == 1 &&
|
||||
(token[0] == '+' || token[0] == '-' || token[0] == '*' || token[0] == '/');
|
||||
}
|
||||
|
||||
static bool parseValue(const std::string& token,
|
||||
const std::map<std::string, std::int64_t>& scope,
|
||||
std::int64_t* outValue,
|
||||
std::string* error) {
|
||||
if (token.empty()) {
|
||||
*error = "value_missing";
|
||||
return false;
|
||||
}
|
||||
bool numeric = true;
|
||||
for (char c : token) {
|
||||
if (!std::isdigit(static_cast<unsigned char>(c))) {
|
||||
numeric = false;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (numeric) {
|
||||
*outValue = std::stoll(token);
|
||||
return true;
|
||||
}
|
||||
auto it = scope.find(token);
|
||||
if (it == scope.end()) {
|
||||
*error = "identifier_unknown";
|
||||
return false;
|
||||
}
|
||||
*outValue = it->second;
|
||||
return true;
|
||||
}
|
||||
|
||||
static bool applyOperations(std::vector<std::int64_t> values,
|
||||
std::vector<char> operators,
|
||||
WatchEvalResult* result,
|
||||
const WatchEvalLimits& limits) {
|
||||
std::vector<std::int64_t> reducedValues;
|
||||
std::vector<char> reducedOperators;
|
||||
reducedValues.push_back(values[0]);
|
||||
|
||||
for (std::size_t i = 0; i < operators.size(); ++i) {
|
||||
const char op = operators[i];
|
||||
const std::int64_t rhs = values[i + 1];
|
||||
if (op == '*' || op == '/') {
|
||||
const std::int64_t lhs = reducedValues.back();
|
||||
if (op == '/' && rhs == 0) return fail(*result, "division_by_zero");
|
||||
reducedValues.back() = op == '*' ? lhs * rhs : lhs / rhs;
|
||||
if (!countOperation(result, limits)) return false;
|
||||
} else {
|
||||
reducedValues.push_back(rhs);
|
||||
reducedOperators.push_back(op);
|
||||
}
|
||||
}
|
||||
|
||||
std::int64_t acc = reducedValues[0];
|
||||
for (std::size_t i = 0; i < reducedOperators.size(); ++i) {
|
||||
const char op = reducedOperators[i];
|
||||
const std::int64_t rhs = reducedValues[i + 1];
|
||||
acc = op == '+' ? acc + rhs : acc - rhs;
|
||||
if (!countOperation(result, limits)) return false;
|
||||
}
|
||||
|
||||
result->value = acc;
|
||||
return true;
|
||||
}
|
||||
|
||||
static bool countOperation(WatchEvalResult* result, const WatchEvalLimits& limits) {
|
||||
++result->operations;
|
||||
if (result->operations > limits.maxOperations) return fail(*result, "operation_limit_exceeded");
|
||||
if (result->operations > limits.timeoutBudgetMs) {
|
||||
result->timedOut = true;
|
||||
return fail(*result, "evaluation_timeout");
|
||||
}
|
||||
return true;
|
||||
}
|
||||
};
|
||||
142
editor/tests/step570_test.cpp
Normal file
142
editor/tests/step570_test.cpp
Normal file
@@ -0,0 +1,142 @@
|
||||
// Step 570: Watch Expression Evaluator Hardening (12 tests)
|
||||
|
||||
#include "WatchExpressionEvaluatorHardening.h"
|
||||
|
||||
#include <iostream>
|
||||
#include <map>
|
||||
|
||||
static int passed = 0, failed = 0;
|
||||
#define TEST(name) { std::cout << " " << #name << "... "; }
|
||||
#define PASS() { std::cout << "PASS\n"; ++passed; }
|
||||
#define FAIL(msg) { std::cout << "FAIL: " << msg << "\n"; ++failed; }
|
||||
#define CHECK(cond, msg) if (!(cond)) { FAIL(msg); return; } else {}
|
||||
|
||||
void test_evaluate_literal_expression() {
|
||||
TEST(evaluate_literal_expression);
|
||||
const auto result = WatchExpressionEvaluatorHardening::evaluate("2 + 3 * 4", {});
|
||||
CHECK(result.ok, "evaluation should succeed");
|
||||
CHECK(result.value == 14, "value should honor precedence");
|
||||
PASS();
|
||||
}
|
||||
|
||||
void test_evaluate_scope_identifier_expression() {
|
||||
TEST(evaluate_scope_identifier_expression);
|
||||
std::map<std::string, std::int64_t> scope{{"x", 10}, {"y", 2}};
|
||||
const auto result = WatchExpressionEvaluatorHardening::evaluate("x - y * 3", scope);
|
||||
CHECK(result.ok, "evaluation should succeed");
|
||||
CHECK(result.value == 4, "value mismatch");
|
||||
PASS();
|
||||
}
|
||||
|
||||
void test_evaluate_rejects_empty_expression() {
|
||||
TEST(evaluate_rejects_empty_expression);
|
||||
const auto result = WatchExpressionEvaluatorHardening::evaluate("", {});
|
||||
CHECK(!result.ok, "empty expression should fail");
|
||||
CHECK(result.error == "expression_empty", "wrong error");
|
||||
PASS();
|
||||
}
|
||||
|
||||
void test_evaluate_rejects_unsafe_pattern() {
|
||||
TEST(evaluate_rejects_unsafe_pattern);
|
||||
const auto result = WatchExpressionEvaluatorHardening::evaluate("system + 1", {});
|
||||
CHECK(!result.ok, "unsafe expression should fail");
|
||||
CHECK(result.rejectedUnsafe, "unsafe flag should be set");
|
||||
CHECK(result.error == "expression_unsafe", "wrong error");
|
||||
PASS();
|
||||
}
|
||||
|
||||
void test_evaluate_rejects_unknown_identifier() {
|
||||
TEST(evaluate_rejects_unknown_identifier);
|
||||
const auto result = WatchExpressionEvaluatorHardening::evaluate("missing + 1", {});
|
||||
CHECK(!result.ok, "unknown identifier should fail");
|
||||
CHECK(result.error == "identifier_unknown", "wrong error");
|
||||
PASS();
|
||||
}
|
||||
|
||||
void test_evaluate_rejects_invalid_character() {
|
||||
TEST(evaluate_rejects_invalid_character);
|
||||
const auto result = WatchExpressionEvaluatorHardening::evaluate("2 + (3)", {});
|
||||
CHECK(!result.ok, "invalid character should fail");
|
||||
CHECK(result.error == "token_invalid_character", "wrong error");
|
||||
PASS();
|
||||
}
|
||||
|
||||
void test_evaluate_rejects_token_limit_exceeded() {
|
||||
TEST(evaluate_rejects_token_limit_exceeded);
|
||||
WatchEvalLimits limits;
|
||||
limits.maxTokens = 3;
|
||||
const auto result = WatchExpressionEvaluatorHardening::evaluate("1 + 2 + 3", {}, limits);
|
||||
CHECK(!result.ok, "token limit should fail");
|
||||
CHECK(result.error == "token_limit_exceeded", "wrong error");
|
||||
PASS();
|
||||
}
|
||||
|
||||
void test_evaluate_rejects_operation_limit_exceeded() {
|
||||
TEST(evaluate_rejects_operation_limit_exceeded);
|
||||
WatchEvalLimits limits;
|
||||
limits.maxOperations = 1;
|
||||
limits.timeoutBudgetMs = 1000;
|
||||
const auto result = WatchExpressionEvaluatorHardening::evaluate("1 + 2 + 3", {}, limits);
|
||||
CHECK(!result.ok, "operation limit should fail");
|
||||
CHECK(result.error == "operation_limit_exceeded", "wrong error");
|
||||
PASS();
|
||||
}
|
||||
|
||||
void test_evaluate_rejects_timeout_budget_exceeded() {
|
||||
TEST(evaluate_rejects_timeout_budget_exceeded);
|
||||
WatchEvalLimits limits;
|
||||
limits.maxOperations = 100;
|
||||
limits.timeoutBudgetMs = 1;
|
||||
const auto result = WatchExpressionEvaluatorHardening::evaluate("1 + 2 + 3", {}, limits);
|
||||
CHECK(!result.ok, "timeout should fail");
|
||||
CHECK(result.timedOut, "timedOut flag should be set");
|
||||
CHECK(result.error == "evaluation_timeout", "wrong error");
|
||||
PASS();
|
||||
}
|
||||
|
||||
void test_evaluate_rejects_division_by_zero() {
|
||||
TEST(evaluate_rejects_division_by_zero);
|
||||
const auto result = WatchExpressionEvaluatorHardening::evaluate("10 / 0", {});
|
||||
CHECK(!result.ok, "division by zero should fail");
|
||||
CHECK(result.error == "division_by_zero", "wrong error");
|
||||
PASS();
|
||||
}
|
||||
|
||||
void test_evaluate_detects_operator_operand_mismatch() {
|
||||
TEST(evaluate_detects_operator_operand_mismatch);
|
||||
const auto result = WatchExpressionEvaluatorHardening::evaluate("1 +", {});
|
||||
CHECK(!result.ok, "mismatch should fail");
|
||||
CHECK(result.error == "operator_operand_mismatch", "wrong error");
|
||||
PASS();
|
||||
}
|
||||
|
||||
void test_evaluate_is_deterministic_for_same_input() {
|
||||
TEST(evaluate_is_deterministic_for_same_input);
|
||||
std::map<std::string, std::int64_t> scope{{"a", 5}, {"b", 6}};
|
||||
const auto a = WatchExpressionEvaluatorHardening::evaluate("a * b + 2", scope);
|
||||
const auto b = WatchExpressionEvaluatorHardening::evaluate("a * b + 2", scope);
|
||||
CHECK(a.ok && b.ok, "both evaluations should succeed");
|
||||
CHECK(a.value == b.value, "values should match");
|
||||
CHECK(a.operations == b.operations, "operation counts should match");
|
||||
PASS();
|
||||
}
|
||||
|
||||
int main() {
|
||||
std::cout << "Step 570: Watch Expression Evaluator Hardening\n";
|
||||
|
||||
test_evaluate_literal_expression(); // 1
|
||||
test_evaluate_scope_identifier_expression(); // 2
|
||||
test_evaluate_rejects_empty_expression(); // 3
|
||||
test_evaluate_rejects_unsafe_pattern(); // 4
|
||||
test_evaluate_rejects_unknown_identifier(); // 5
|
||||
test_evaluate_rejects_invalid_character(); // 6
|
||||
test_evaluate_rejects_token_limit_exceeded(); // 7
|
||||
test_evaluate_rejects_operation_limit_exceeded(); // 8
|
||||
test_evaluate_rejects_timeout_budget_exceeded(); // 9
|
||||
test_evaluate_rejects_division_by_zero(); // 10
|
||||
test_evaluate_detects_operator_operand_mismatch(); // 11
|
||||
test_evaluate_is_deterministic_for_same_input(); // 12
|
||||
|
||||
std::cout << "\nResults: " << passed << "/" << (passed + failed) << " passed\n";
|
||||
return failed == 0 ? 0 : 1;
|
||||
}
|
||||
35
progress.md
35
progress.md
@@ -10370,3 +10370,38 @@ debug views with deterministic instruction ordering and register state access.
|
||||
- `editor/src/DisassemblyRegisterViewModel.h` within header-size limit (`125` <= `600`)
|
||||
- `editor/tests/step569_test.cpp` within test-file size guidance (`156` lines)
|
||||
- Header-only architecture and naming conventions remain aligned with `ARCHITECTURE.md`
|
||||
|
||||
### Step 570: Watch Expression Evaluator Hardening
|
||||
**Status:** PASS (12/12 tests)
|
||||
|
||||
Implements a hardened watch expression evaluator with explicit unsafe-pattern
|
||||
rejection, token/operation guardrails, deterministic timeout behavior, and
|
||||
numeric/identifier evaluation.
|
||||
|
||||
**Files added:**
|
||||
- `editor/src/WatchExpressionEvaluatorHardening.h` - evaluator hardening module:
|
||||
- expression/token safety validation
|
||||
- unsafe pattern detection (`system`, `exec`, braces, etc.)
|
||||
- token count, operation count, and timeout-budget enforcement
|
||||
- deterministic numeric/identifier expression evaluation with precedence
|
||||
- explicit error taxonomy for parser/runtime failures
|
||||
- `editor/tests/step570_test.cpp` - 12 tests covering:
|
||||
- literal and scoped identifier evaluation behavior
|
||||
- unsafe/unknown/invalid token rejection behavior
|
||||
- token/operation/timeout guard behavior
|
||||
- divide-by-zero and operand mismatch handling
|
||||
- deterministic repeated evaluation behavior
|
||||
|
||||
**Files modified:**
|
||||
- `editor/CMakeLists.txt` - `step570_test` target
|
||||
|
||||
**Verification run:**
|
||||
- `cmake -S editor -B editor/build-native` - PASS
|
||||
- `cmake --build editor/build-native --target step570_test step569_test` - PASS
|
||||
- `./editor/build-native/step570_test` - PASS (12/12)
|
||||
- `./editor/build-native/step569_test` - PASS (12/12) regression coverage
|
||||
|
||||
**Architecture gate check:**
|
||||
- `editor/src/WatchExpressionEvaluatorHardening.h` within header-size limit (`210` <= `600`)
|
||||
- `editor/tests/step570_test.cpp` within test-file size guidance (`142` lines)
|
||||
- Header-only architecture and naming conventions remain aligned with `ARCHITECTURE.md`
|
||||
|
||||
Reference in New Issue
Block a user