Severity: Warning
File: src/Servy.Core/Helpers/ServiceDtoHelper.cs (lines 27-29)
Code:
public static void ApplyDefaults(ServiceDto? dto)
{
if (dto == null) return;
// Identity & Behavior
dto.StartupType = dto.StartupType ?? (int)AppConfig.DefaultStartupType;
dto.Priority = dto.Priority ?? (int)AppConfig.DefaultPriority;
dto.RunAsLocalSystem = AppConfig.DefaultRunAsLocalSystem; // <-- UNCONDITIONAL
dto.UserAccount = null; // <-- UNCONDITIONAL
dto.Password = null; // <-- UNCONDITIONAL
dto.EnableDebugLogs = dto.EnableDebugLogs ?? AppConfig.DefaultEnableDebugLogs;
...
}
Explanation:
Note the asymmetry: every other field uses ?? to apply a default only when null. The three identity-related fields use plain = and clobber whatever the caller had loaded:
RunAsLocalSystem is unconditionally set to AppConfig.DefaultRunAsLocalSystem (which is true).
UserAccount is unconditionally set to null.
Password is unconditionally set to null.
This is called during XML/JSON deserialization (per the class XML doc — "populated during the deserialization of XML and JSON configurations"). If a user-supplied <service> element has:
<RunAsLocalSystem>false</RunAsLocalSystem>
<UserAccount>DOMAIN\svc_account</UserAccount>
then after ApplyDefaults, the DTO carries RunAsLocalSystem=true and UserAccount=null — silently switching the imported service to LocalSystem with no warning to the operator. The original imported values are gone before any validator can complain.
There are two reasonable design intents that could've been hidden behind this:
- Password security: don't carry plaintext passwords across config files. That justifies clobbering
Password, but NOT RunAsLocalSystem/UserAccount.
- First-use bootstrap: only true for new services, not for imports. The current code can't tell the difference.
Suggested fix:
- Keep
Password = null (defensible: don't accept passwords in plain config).
- Use
?? for RunAsLocalSystem and UserAccount like every other field:
dto.RunAsLocalSystem = dto.RunAsLocalSystem ?? AppConfig.DefaultRunAsLocalSystem;
dto.UserAccount = dto.UserAccount; // (no-op; remove this line)
dto.Password = null; // intentionally reset
- After import, surface a clear warning to the user via the existing message-box service: "Imported service is configured to run as 'X' — please re-enter the password before installing."
This change preserves the explicit "no plaintext credentials" posture while not silently degrading service identity to LocalSystem.
Severity: Warning
File: src/Servy.Core/Helpers/ServiceDtoHelper.cs (lines 27-29)
Code:
Explanation:
Note the asymmetry: every other field uses
??to apply a default only when null. The three identity-related fields use plain=and clobber whatever the caller had loaded:RunAsLocalSystemis unconditionally set toAppConfig.DefaultRunAsLocalSystem(which istrue).UserAccountis unconditionally set tonull.Passwordis unconditionally set tonull.This is called during XML/JSON deserialization (per the class XML doc — "populated during the deserialization of XML and JSON configurations"). If a user-supplied
<service>element has:then after
ApplyDefaults, the DTO carriesRunAsLocalSystem=trueandUserAccount=null— silently switching the imported service to LocalSystem with no warning to the operator. The original imported values are gone before any validator can complain.There are two reasonable design intents that could've been hidden behind this:
Password, but NOTRunAsLocalSystem/UserAccount.Suggested fix:
Password = null(defensible: don't accept passwords in plain config).??forRunAsLocalSystemandUserAccountlike every other field:This change preserves the explicit "no plaintext credentials" posture while not silently degrading service identity to
LocalSystem.