diff --git a/editor/CMakeLists.txt b/editor/CMakeLists.txt index f4fbd32..a9619c9 100644 --- a/editor/CMakeLists.txt +++ b/editor/CMakeLists.txt @@ -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) diff --git a/editor/src/WatchExpressionEvaluatorHardening.h b/editor/src/WatchExpressionEvaluatorHardening.h new file mode 100644 index 0000000..a1e1cdd --- /dev/null +++ b/editor/src/WatchExpressionEvaluatorHardening.h @@ -0,0 +1,210 @@ +#pragma once +// Step 570: Watch Expression Evaluator Hardening + +#include +#include +#include +#include +#include + +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& 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 tokens; + if (!tokenize(expression, limits.maxTokens, &tokens, &result.error)) return result; + if (tokens.empty()) { + fail(result, "expression_empty"); + return result; + } + + std::vector values; + std::vector 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* 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(c))) { + flushCurrent(); + continue; + } + if (c == '+' || c == '-' || c == '*' || c == '/') { + flushCurrent(); + outTokens->push_back(std::string(1, c)); + } else if (std::isalnum(static_cast(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& 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(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 values, + std::vector operators, + WatchEvalResult* result, + const WatchEvalLimits& limits) { + std::vector reducedValues; + std::vector 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; + } +}; diff --git a/editor/tests/step570_test.cpp b/editor/tests/step570_test.cpp new file mode 100644 index 0000000..8b21515 --- /dev/null +++ b/editor/tests/step570_test.cpp @@ -0,0 +1,142 @@ +// Step 570: Watch Expression Evaluator Hardening (12 tests) + +#include "WatchExpressionEvaluatorHardening.h" + +#include +#include + +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 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 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; +} diff --git a/progress.md b/progress.md index 4c49cb0..5597c0a 100644 --- a/progress.md +++ b/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`