Skip to content

002 architecture review

Based on the provided codebase, the architectural transition from a decentralized, high-privilege PowerShell remote execution model (WinRM/WMI) to a structured Client-Server (.NET 10 Orchestrator + Minimal API Agent) model is a strong strategic decision. It inherently reduces the network attack surface by eliminating the need for open WinRM ports and provides better state tracking via the two-phase commit pattern (Agent deployment -> SQL finalization).

However, reviewing this through the lens of ISO27001, SOC2, NIS2, and the CIA triad, there are critical vulnerabilities and architectural gaps regarding supply chain security, input validation, and transport-layer confidentiality that must be addressed before this system is production-ready.

Here is the detailed architectural and security review.

1. Security Architecture & Posture Review (CIA & Compliance)#

A. Confidentiality & Transport Security (Violates HIPAA, ISO27001 A.10, SOC2 CC6.1)#

  • HTTP Cleartext Communication: In appsettings.json, Kestrel is configured to bind to HTTP ("Url": "http://0.0.0.0:5100"). Sending deployment packages (which contain proprietary IP) and API Keys via the X-Api-Key header over plain HTTP exposes the system to credential sniffing and Man-in-the-Middle (MitM) attacks.
  • TrustServerCertificate=True: In OrchestratorOptions.cs, the Orchestrator connects to SQL using TrustServerCertificate=True;Encrypt=True. This explicitly disables certificate validation, rendering the encryption completely vulnerable to MitM attacks.

B. Integrity & Supply Chain (Violates NIS2 Article 21, CRA)#

  • Lack of Payload Verification: The Agent extracts whatever ZIP file it receives. There is no cryptographic signature validation (e.g., checking an RSA signature or SHA256 hash provided by the Orchestrator) to ensure the ZIP wasn't tampered with in transit or on the CI/CD runner.
  • Path Traversal Vulnerability (Critical): The Orchestrator validates the Version string using VersionParser, but the Agent does not validate it. In FileExtractor.cs, Path.Combine(envSettings.BasePath, job.Config.Version) is used. If a malicious actor hits the Agent API directly with a payload where "Version": "../../Windows/System32", the Agent will extract files into critical system directories.

C. Availability & Resilience (Violates DORA, NIS2)#

  • Unbounded Channels: In BenefitManager.Agent/Program.cs, the queue is created using Channel.CreateUnbounded<DeploymentJob>(). If the endpoint receives a high volume of requests, it will consume all available memory on the target VM, leading to an Out-Of-Memory (OOM) crash and Denial of Service (DoS).
  • State Machine Rollback: If the deployment fails at DeploymentStatus.ConfiguringService, IIS has already been modified (ConfiguringIis and UpdatingConfig completed). There is no automated rollback defined in the Orchestrator or Agent, leaving the node in a degraded state.

2. Detailed Remediation & Code Implementations#

Vulnerability 1: Directory Traversal in the Agent#

The Agent must independently sanitize all inputs from the Orchestrator.

Fix: Add strict validation in DeployEndpoints.cs or FileExtractor.cs. 

// In FileExtractor.cs
public string Extract(DeploymentJob job)
{
    if (!options.Value.Environments.TryGetValue(job.Config.Environment, out var envSettings))
        throw new InvalidOperationException($"Environment '{job.Config.Environment}' not configured.");

    // Validate that Version contains only safe characters (e.g., alphanumeric and dots)
    if (!Regex.IsMatch(job.Config.Version, @"^[0-9a-zA-Z\.]+$"))
        throw new ArgumentException("Invalid Version format. Potential path traversal detected.");

    var targetPath = Path.Combine(envSettings.BasePath, job.Config.Version);

    // Double-check normalization to ensure it stays inside BasePath
    var fullBasePath = Path.GetFullPath(envSettings.BasePath);
    var fullTargetPath = Path.GetFullPath(targetPath);
    if (!fullTargetPath.StartsWith(fullBasePath, StringComparison.OrdinalIgnoreCase))
        throw new SecurityException("Path traversal attack detected.");

    Directory.CreateDirectory(fullTargetPath);
    ZipFile.ExtractToDirectory(job.TempPackagePath, fullTargetPath, overwriteFiles: true);

    return fullTargetPath;
}

Vulnerability 2: Unbounded Resource Consumption#

Transition from an Unbounded Channel to a Bounded Channel to enforce backpressure. If the queue is full, the API should return a 429 Too Many Requests or 503 Service Unavailable.

Fix: Update Program.cs in the Agent. 

// Program.cs
builder.Services.AddSingleton(Channel.CreateBounded<DeploymentJob>(new BoundedChannelOptions(10)
{
    FullMode = BoundedChannelFullMode.Wait,
    SingleReader = true,
    SingleWriter = false
}));
Note: You must also update DeployEndpoints.cs to use TryWrite or handle the wait context so the HTTP request doesn't hang indefinitely if the queue is full.

Vulnerability 3: SQL Connection Security#

Remove TrustServerCertificate=True in production. If you are using self-signed certificates internally, trust the internal Root CA on the machine running the Orchestrator rather than bypassing validation in code.

Fix: In OrchestratorOptions.cs: 

public string GetConnectionString(string environment) => environment switch
{
    "Acceptatie" => "Data Source=vmbenefitsdbap1;Initial Catalog=CommandCenter;Integrated Security=True;Encrypt=True;TrustServerCertificate=False",
    "Productie"  => "Data Source=vmbenefitsdbp1;Initial Catalog=CommandCenter;Integrated Security=True;Encrypt=True;TrustServerCertificate=False",
    _            => throw new ArgumentException($"Unknown environment: {environment}")
};

Vulnerability 4: API Authentication & Network Security#

While CryptographicOperations.FixedTimeEquals is used correctly to prevent timing attacks, relying solely on static API Keys violates SOC2 CC6.1 regarding credential rotation and identity-based access.

Strategic Recommendation: 1. Mandate HTTPS: Bind Kestrel to HTTPS (app.UseHttpsRedirection()) and issue an internal PKI certificate for the Agent VMs. 2. Mutual TLS (mTLS) or Azure Entra ID: If deploying this in Azure, wrap the VM in an Application Security Group (ASG) and restrict inbound port 5100 strictly to the Orchestrator's IP/Subnet. Alternatively, transition to Azure Entra ID JWT validation instead of static API keys.

3. Enterprise Infrastructure & Deployment Integration (IaC)#

Because you deploy to VMs (vmbenefitsbmp1), the Agent itself must be bootstrapped securely. Since the prompt specifies my domain includes Bicep/Terraform, I recommend deploying the Agent via Azure VM Custom Script Extensions or RunCommand using Bicep to ensure consistency and compliance with ISAE/ISO27001 change management.

Bicep Example: Secure Agent Bootstrapping 

resource vmAgentExtension 'Microsoft.Compute/virtualMachines/extensions@2023-03-01' = {
  parent: virtualMachine
  name: 'InstallBenefitManagerAgent'
  location: location
  properties: {
    publisher: 'Microsoft.Compute'
    type: 'CustomScriptExtension'
    typeHandlerVersion: '1.10'
    autoUpgradeMinorVersion: true
    protectedSettings: {
      // Inject API Key securely via KeyVault reference in your pipeline, NOT hardcoded
      commandToExecute: 'powershell.exe -ExecutionPolicy Unrestricted -File Install-Agent.ps1 -ApiKey ${agentApiKey}'
    }
  }
}

4. Code Quality & Maintainability Observations#

  1. Transaction Management: In SqlFinalizationService.cs, you correctly use conn.BeginTransactionAsync() and rollback on failure. This is excellent practice.
  2. Missing CancellationToken Propagation: In DeploymentWorkerService.cs, fileExtractor.Extract and iisConfigurator.EnsureApplication do not accept a CancellationToken. If a deployment is cancelled (e.g., the service shuts down), extraction and IIS configuration will become orphaned blocking calls. Update your interfaces to accept stoppingToken.
  3. Improper File Cleanup: In DeploymentWorkerService.cs's finally block, you clean up job.TempPackagePath. However, if FileStream writing in DeployEndpoints.cs throws an exception (e.g., out of disk space) before the job hits the channel, the temp file will be orphaned in F:\Temp. Fix: Wrap the file stream operation in DeployEndpoints.cs in a try/catch and delete the file if queueing fails.
  4. Resilience Policies: The Orchestrator correctly uses AddStandardResilienceHandler(). This fulfills the NIS2 requirement for resilient inter-service communication (handling network blips automatically via Polly).

Final Architectural Verdict#

The codebase demonstrates a high level of .NET engineering maturity (Channels, Minimal APIs, Dapper, proper DI, and solid testing patterns).

To meet the rigid requirements of ISO27001, NIS2, and SOC2, you must prioritize: 1. Enforcing HTTPS/TLS 1.2+ for the Agent endpoint. 2. Patching the Path Traversal vector in the File Extractor. 3. Enabling strict SQL Certificate Validation. 4. Implementing a Package Integrity Check (SHA-256 hash comparison) passed from the Orchestrator to the Agent to guarantee supply chain integrity.