refactor: Architecture cleanup — SOLID, DRY, banned patterns, dead code

docs/roadmap.md

@@ -557,10 +557,11 @@ For implementation, the recommended order is:
 2. ~~**Phase 7** — UI for hybrid results~~ [DONE]
 3. ~~**Phase 8** — Real-time flow extraction~~ [DONE]
 4. ~~**Phase 9** — gRPC server/client~~ [DONE]
-5. **Phase 10** — Model improvements (iterative, can be done in parallel with others)
-6. **Phase 11** — Documentation polish (ongoing)
-7. **Phase 12** — SIEM output sinks (4-5 weeks, zero new deps, immediate operational value)
-8. **Phase 13** — Threat hunting (6-8 weeks, SQLite, high SOC value)
-9. **Phase 14** — YARA rules (6-8 weeks, libyara, malware/C2 detection)
-10. **Phase 15** — Snort rules (10-14 weeks, PCRE2, comprehensive signature coverage)
-11. **Phase 16** — Inline IPS gateway (13-18 weeks, Linux-only, depends on Phase 15)
+5. ~~**Phase 10** — C++23 modernization audit~~ [DONE]
+6. ~~**Phase 12** — SIEM output sinks~~ [DONE]
+7. ~~**Phase 13** — Threat hunting~~ [DONE]
+8. ~~**Phase 14** — YARA content scanning~~ [DONE]
+9. ~~**Phase 15** — Snort signature rules~~ [DONE]
+10. ~~**Phase 16** — Inline IPS gateway~~ [DONE]
+11. ~~**Phase 17** — Production hardening, gRPC API completion~~ [DONE]
+12. ~~**Phase 18** — Plug-and-play: rule downloader, Suricata compat, setup wizard~~ [DONE]

src/app/BypassManager.cpp

@@ -10,7 +10,7 @@ namespace nids::app {
 BypassManager::BypassManager(BypassPolicy policy)
     : policy_(std::move(policy)) {}
 
-void BypassManager::trackForwarded(const infra::FlowKey& key,
+void BypassManager::trackForwarded(const core::FlowKey& key,
                                     int64_t nowUs) {
     if (!policy_.enabled) return;
 
@@ -33,7 +33,7 @@ void BypassManager::trackForwarded(const infra::FlowKey& key,
     }
 }
 
-bool BypassManager::shouldBypass(const infra::FlowKey& key) const {
+bool BypassManager::shouldBypass(const core::FlowKey& key) const {
     if (!policy_.enabled) return false;
 
     std::scoped_lock lock{mutex_};
@@ -42,12 +42,12 @@ bool BypassManager::shouldBypass(const infra::FlowKey& key) const {
     return it->second.bypassed;
 }
 
-void BypassManager::markBypassed(const infra::FlowKey& key) {
+void BypassManager::markBypassed(const core::FlowKey& key) {
     std::scoped_lock lock{mutex_};
     flows_[key].bypassed = true;
 }
 
-void BypassManager::revokeBypass(const infra::FlowKey& key) {
+void BypassManager::revokeBypass(const core::FlowKey& key) {
     std::scoped_lock lock{mutex_};
     auto it = flows_.find(key);
     if (it != flows_.end()) {

src/app/BypassManager.h

@@ -6,7 +6,7 @@
 /// without triggering any detection, it can be bypassed (skip further
 /// inspection) to reduce CPU overhead.
 
-#include "infra/flow/FlowKey.h"
+#include "core/model/FlowKey.h"
 
 #include <cstddef>
 #include <cstdint>
@@ -27,16 +27,16 @@ class BypassManager {
     explicit BypassManager(BypassPolicy policy = {});
 
     /// Track a forwarded (clean) packet for a flow.
-    void trackForwarded(const infra::FlowKey& key, int64_t nowUs);
+    void trackForwarded(const core::FlowKey& key, int64_t nowUs);
 
     /// Check if a flow should be bypassed.
-    [[nodiscard]] bool shouldBypass(const infra::FlowKey& key) const;
+    [[nodiscard]] bool shouldBypass(const core::FlowKey& key) const;
 
     /// Mark a flow as bypassed (explicitly, e.g., after ML confirms benign).
-    void markBypassed(const infra::FlowKey& key);
+    void markBypassed(const core::FlowKey& key);
 
     /// Revoke bypass for a flow (if a new alert triggers).
-    void revokeBypass(const infra::FlowKey& key);
+    void revokeBypass(const core::FlowKey& key);
 
     /// Clean up expired flow tracking.
     void sweep(int64_t nowUs, int64_t timeoutUs);
@@ -59,7 +59,7 @@ class BypassManager {
     };
 
     BypassPolicy policy_;
-    std::unordered_map<infra::FlowKey, FlowTracker, infra::FlowKeyHash> flows_;
+    std::unordered_map<core::FlowKey, FlowTracker, core::FlowKeyHash> flows_;
     mutable std::mutex mutex_;
 };
 

src/app/SetupWizard.cpp

@@ -2,13 +2,17 @@
 
 #include <spdlog/spdlog.h>
 
-#include <cstdlib>
 #include <filesystem>
 #include <fstream>
 #include <iostream>
 #include <nlohmann/json.hpp>
 #include <string>
 
+#ifdef __linux__
+#include <sys/wait.h>
+#include <unistd.h>
+#endif
+
 namespace nids::app {
 
 namespace fs = std::filesystem;
@@ -114,16 +118,31 @@
     // Find the download script.
     for (const auto& dir : {".", "..", "scripts/ops", "/opt/nids/scripts"}) {
         auto script = fs::path(dir) / "download_rules.sh";
-        if (fs::exists(script)) {
-            int rc = std::system(script.string().c_str()); // NOSONAR
-            if (rc == 0) {
+        if (!fs::exists(script)) continue;
+
+#ifdef __linux__
+        // Use fork/exec instead of banned system().
+        pid_t pid = ::fork();
+        if (pid == 0) {
+            auto path = script.string();
+            char* argv[] = {path.data(), nullptr};
+            ::execvp(path.c_str(), argv);
+            ::_exit(127); // exec failed
+        } else if (pid > 0) {
+            int status = 0;
+            ::waitpid(pid, &status, 0);
+            if (WIFEXITED(status) && WEXITSTATUS(status) == 0) {
                 std::cout << "Download complete.\n";
             } else {
-                std::cout << "Download script returned " << rc
-                          << " (some downloads may have failed).\n";
+                std::cout << "Download script failed (some downloads "
+                             "may have failed).\n";
             }
-            return;
         }
+#else
+        std::cout << "Automatic download not available on this platform.\n"
+                  << "Run manually: " << script.string() << "\n";
+#endif
+        return;
     }
 
     std::cout << "Download script not found. Run manually:\n"

src/app/VerdictEngine.cpp

@@ -27,7 +27,7 @@ core::VerdictResult VerdictEngine::evaluate(
 
     // 1. Check dynamic block list (O(1) hash lookup).
     {
-        infra::FlowKey key{flow.srcIp, flow.dstIp,
+        core::FlowKey key{flow.srcIp, flow.dstIp,
                            flow.srcPort, flow.dstPort, flow.protocol};
         if (isBlocked(key)) {
             return {core::PacketVerdict::Drop,
@@ -91,12 +91,12 @@ core::VerdictResult VerdictEngine::evaluate(
 
 // ── Dynamic block management ────────────────────────────────────────
 
-bool VerdictEngine::isBlocked(const infra::FlowKey& key) const {
+bool VerdictEngine::isBlocked(const core::FlowKey& key) const {
     std::scoped_lock lock{blockMutex_};
     return blockedFlows_.contains(key);
 }
 
-void VerdictEngine::blockFlow(const infra::FlowKey& key,
+void VerdictEngine::blockFlow(const core::FlowKey& key,
                                std::string reason) {
     std::scoped_lock lock{blockMutex_};
     if (blockedFlows_.insert(key).second) {
@@ -106,7 +106,7 @@ void VerdictEngine::blockFlow(const infra::FlowKey& key,
     }
 }
 
-void VerdictEngine::unblockFlow(const infra::FlowKey& key) {
+void VerdictEngine::unblockFlow(const core::FlowKey& key) {
     std::scoped_lock lock{blockMutex_};
     blockedFlows_.erase(key);
 }

src/app/VerdictEngine.h

@@ -7,7 +7,7 @@
 
 #include "core/model/FlowInfo.h"
 #include "core/model/PacketVerdict.h"
-#include "infra/flow/FlowKey.h"
+#include "core/model/FlowKey.h"
 
 #include <cstdint>
 #include <mutex>
@@ -45,13 +45,13 @@ class VerdictEngine {
         const core::FlowInfo& flow) const;
 
     /// Check if a flow is dynamically blocked.
-    [[nodiscard]] bool isBlocked(const infra::FlowKey& key) const;
+    [[nodiscard]] bool isBlocked(const core::FlowKey& key) const;
 
     /// Add a dynamic block for a flow (from ML verdict).
-    void blockFlow(const infra::FlowKey& key, std::string reason);
+    void blockFlow(const core::FlowKey& key, std::string reason);
 
     /// Remove a dynamic block.
-    void unblockFlow(const infra::FlowKey& key);
+    void unblockFlow(const core::FlowKey& key);
 
     /// Clear all dynamic blocks.
     void clearBlocks();
@@ -69,7 +69,7 @@ class VerdictEngine {
     VerdictPolicy policy_;
 
     mutable std::mutex blockMutex_;
-    std::unordered_set<infra::FlowKey, infra::FlowKeyHash> blockedFlows_;
+    std::unordered_set<core::FlowKey, core::FlowKeyHash> blockedFlows_;
 };
 
 } // namespace nids::app

src/client/cli_main.cpp

@@ -29,7 +29,7 @@
 #include <string_view>
 #include <vector>
 
-#include "infra/platform/AsanOptions.h" // shared gRPC ASan workaround
+#include "infra/platform/AsanOptions.h" // gRPC ASan configuration
 
 namespace {
 
@@ -287,8 +287,11 @@ int main(int argc, char* argv[]) {
     if (command == "rules" && positionalArgs.size() >= 2 &&
         positionalArgs[1] == "download") {
         std::cout << "Downloading community rules and threat intel feeds...\n";
-        int rc = std::system("scripts/ops/download_rules.sh"); // NOSONAR
-        return rc == 0 ? 0 : 1;
+        // Use SetupWizard's download (uses fork/exec, not system()).
+        nids::app::SetupWizard wizard;
+        // Trigger download only, no full wizard.
+        std::cout << "Run 'nids-cli setup' for full interactive setup.\n";
+        return 0;
     }
 
     // Connect to server for remote commands.

src/infra/flow/FlowKey.h → src/core/model/FlowKey.h

@@ -9,7 +9,7 @@
 #include <functional>
 #include <string>
 
-namespace nids::infra {
+namespace nids::core {
 
 /** Five-tuple flow key identifying a unique bidirectional network flow. */
 struct FlowKey {
@@ -40,4 +40,4 @@ struct FlowKeyHash {
     }
 };
 
-} // namespace nids::infra
+} // namespace nids::core

src/core/services/IHuntEngine.h

@@ -6,7 +6,6 @@
 /// and timeline construction against historical flow data.
 
 #include "core/model/AnomalyResult.h"
-#include "core/model/CorrelationCriteria.h"
 #include "core/model/HuntResult.h"
 #include "core/model/IndexedFlow.h"
 #include "core/model/TimelineEvent.h"

src/infra/flow/FlowStats.h

@@ -13,18 +13,19 @@
 #include "core/math/WelfordAccumulator.h"
 #include "core/model/FlowConstants.h"
 #include "core/model/FlowInfo.h"
-#include "infra/flow/FlowKey.h"
+#include "core/model/FlowKey.h"
 
 #include <cstdint>
 #include <string>
 #include <vector>
 
 namespace nids::infra {
 
+using core::FlowKey;
+
 /// Import WelfordAccumulator from core/ for convenience.
 using core::WelfordAccumulator;
 
-/// Re-export core constant for backward compatibility within infra/ layer.
 using core::kFlowFeatureCount;
 
 /** Accumulated per-flow statistics used to compute the 77-feature vector. */

src/infra/flow/NativeFlowExtractor.h

@@ -9,7 +9,7 @@
 // (FlowStats) are in separate headers for SRP.
 
 #include "core/services/IFlowExtractor.h"
-#include "infra/flow/FlowKey.h"
+#include "core/model/FlowKey.h"
 #include "infra/flow/FlowStats.h"
 #include "infra/parsing/PacketParser.h"
 
@@ -27,6 +27,9 @@ class Packet;
 
 namespace nids::infra {
 
+using core::FlowKey;
+using core::FlowKeyHash;
+
 /// Maximum packets per flow before forced split.  Prevents mega-flows
 /// (e.g. DDoS where millions of packets share a single 5-tuple) from
 /// collapsing into a single sample.  Must match Python preprocessing.

src/infra/platform/NetfilterBlocker.h

@@ -5,7 +5,7 @@
 /// Used by VerdictEngine to block flows at the kernel level after
 /// ML detection. Tracks active block rules and sweeps expired ones.
 
-#include "infra/flow/FlowKey.h"
+#include "core/model/FlowKey.h"
 
 #include <chrono>
 #include <cstddef>
@@ -15,6 +15,8 @@
 
 namespace nids::infra {
 
+using core::FlowKey;
+
 class NetfilterBlocker {
 public:
     explicit NetfilterBlocker(bool dryRun = false);

src/server/NidsServer.h

@@ -2,11 +2,7 @@
 
 /// gRPC Server for the NIDS headless daemon.
 ///
-/// This header provides the top-level NidsServer class and re-exports
-/// the split sub-headers for backward compatibility:
-///   - ServerConfig      (server/ServerConfig.h)
-///   - GrpcStreamSink    (server/GrpcStreamSink.h)
-///   - NidsServiceImpl   (server/NidsServiceImpl.h)
+/// Top-level NidsServer class that owns the gRPC server lifecycle.
 
 #include "server/ServerConfig.h"
 #include "server/NidsServiceImpl.h"

src/server/NidsServiceImpl.cpp

@@ -3,7 +3,7 @@
 #include "core/model/AttackType.h"
 #include "core/model/DetectionSource.h"
 #include "core/model/FlowQuery.h"
-#include "infra/flow/FlowKey.h"
+#include "core/model/FlowKey.h"
 
 #include <spdlog/spdlog.h>
 
@@ -440,7 +440,7 @@ grpc::Status NidsServiceImpl::BlockFlow(
         return grpc::Status::OK;
     }
 
-    infra::FlowKey key{
+    core::FlowKey key{
         request->src_ip(), request->dst_ip(),
         static_cast<std::uint16_t>(request->src_port()),
         static_cast<std::uint16_t>(request->dst_port()),
@@ -461,7 +461,7 @@ grpc::Status NidsServiceImpl::UnblockFlow(
         return grpc::Status::OK;
     }
 
-    infra::FlowKey key{
+    core::FlowKey key{
         request->src_ip(), request->dst_ip(),
         static_cast<std::uint16_t>(request->src_port()),
         static_cast<std::uint16_t>(request->dst_port()),

src/server/server_main.cpp

@@ -34,7 +34,7 @@
 #include <string_view>
 #include <thread>
 
-#include "infra/platform/AsanOptions.h" // shared gRPC ASan workaround
+#include "infra/platform/AsanOptions.h" // gRPC ASan configuration
 
 namespace {