Skip to content

[Correctness] ServiceValidationRules.cs — Missing Name/ExecutablePath reported as Warnings instead of Errors #785

@Christophe-Rogiers

Description

@Christophe-Rogiers

Severity: Critical
File: src/Servy.Core/Validators/ServiceValidationRules.cs lines 42-46

Description:

The null-DTO guard correctly treats missing input as a blocking Error:

if (dto == null)
{
    result.Errors.Add(Strings.Msg_ValidationError);
    return result; // Stop early for missing vital fields
}

The very next guard — for missing Name or ExecutablePath, which are also vital — adds to Warnings instead:

if (string.IsNullOrWhiteSpace(dto.Name) || string.IsNullOrWhiteSpace(dto.ExecutablePath))
{
    result.Warnings.Add(Strings.Msg_ValidationError);   // ← Warnings, not Errors
    return result; // Stop early for missing vital fields
}

Callers that gate on result.Errors.Any() (the documented contract: "errors (blocking issues) and warnings (non-blocking suggestions)") will treat a service with no Name or no ExecutablePath as valid, matching the XML-doc summary which explicitly says this method performs "fail-fast validation for null objects or missing required names/paths".

The two missing-required-field branches should both be on the same side of the Errors/Warnings line, and from the doc comment the intended side is Errors.

Suggested fix:

if (string.IsNullOrWhiteSpace(dto.Name) || string.IsNullOrWhiteSpace(dto.ExecutablePath))
{
    result.Errors.Add(Strings.Msg_ValidationError);
    return result;
}

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions