Merge pull request #8 from carterscode/chore/openssf-passing-tier

chore: CONTRIBUTING, xUnit tests, warnings-as-errors (OpenSSF Passing groundwork)

Author: carterscode

.github/workflows/build.yml

@@ -23,3 +23,6 @@ jobs:
 
       - name: Build (Release)
         run: dotnet build GamerGuardian.sln -c Release --no-restore
+
+      - name: Test
+        run: dotnet test GamerGuardian.sln -c Release --no-build --verbosity normal

CONTRIBUTING.md

@@ -0,0 +1,124 @@
+# Contributing to GamerGuardian
+
+Thanks for your interest. This document describes how to set up, contribute, and what reviewers look for.
+
+## Quick start for contributors
+
+```powershell
+git clone https://github.com/carterscode/GamerGuardian.git
+cd GamerGuardian
+dotnet build
+dotnet test
+src\GamerGuardian\bin\Debug\net8.0-windows10.0.22000.0\GamerGuardian.exe --show-settings
+```
+
+For the installer build and CI workflow details, see [docs/wiki/Build-from-source.md](https://github.com/carterscode/GamerGuardian/blob/main/docs/wiki/Build-from-source.md).
+
+## Branching and pull requests
+
+`main` is protected — you cannot push to it directly. The flow:
+
+1. Branch off `main`: `git checkout -b feat/something-descriptive`. Use `feat/`, `fix/`, `chore/`, `ci/`, `docs/` prefixes.
+2. Commit your changes with descriptive messages (see *Commit messages* below).
+3. Push the branch: `git push -u origin feat/something-descriptive`.
+4. Open a pull request: `gh pr create --base main`.
+5. CI runs automatically — `build`, `Analyze (csharp)`, `Analyze (actions)`. All three must pass before merge.
+6. Self-merge once green: `gh pr merge --merge --delete-branch` (no required approvals for the solo-dev workflow).
+
+## Commit messages
+
+Conventional Commits format. The first line is `<type>: <imperative summary>` under 72 chars.
+
+Common types:
+- `feat:` — new functionality (new monitor, UI feature, CLI flag)
+- `fix:` — bug fix
+- `chore:` — maintenance, refactors with no behavior change
+- `ci:` — workflow / build pipeline changes
+- `docs:` — wiki, README, comments
+- `perf:` — performance improvements
+- `ui:` — UI/UX changes
+
+Multi-line bodies are encouraged for non-trivial changes — explain *why*, not *what*. Example:
+
+```
+fix(services): stop UAC spam when Windows reverts a service change
+
+Symptom: enabling auto-apply on a service Windows refuses to actually
+disable (DoSvc / Delivery Optimization is the trigger case) caused a
+UAC prompt every 30 s forever.
+
+MonitorService now backs off auto-apply for a setting whose verify
+failed for 15 minutes. Drift still surfaces as a notification.
+```
+
+## Code style
+
+- Follow the existing patterns. The codebase is small and consistent.
+- `<Nullable>enable</Nullable>` is on. Don't introduce `?` types if you can avoid them.
+- `<TreatWarningsAsErrors>true</TreatWarningsAsErrors>` is on. Build warnings break CI.
+- Default to no comments. Only comment the *why* when the *what* is obvious from the code. See examples in `Monitors/HagsMonitor.cs` for the conventional level of commenting.
+- C# expression-bodied members and pattern-matching are encouraged where they read naturally.
+- Don't introduce abstractions speculatively. Three similar lines is better than a premature framework.
+
+## Adding a new monitor
+
+The canonical example is `src/GamerGuardian/Monitors/HagsMonitor.cs` — about 30 lines.
+
+A new monitor needs:
+
+1. A class implementing `IMonitoredSetting` in `src/GamerGuardian/Monitors/`.
+2. Registration in `App.xaml.cs` in the `_allMonitors` array.
+3. A row in `SettingsWindow.xaml.cs` `LoadGlobals` (or the equivalent for your tab).
+4. A `MechanismFor` and `VerifyCommandFor` entry in `src/GamerGuardian/Services/SettingDocs.cs`.
+5. **A test** in `tests/GamerGuardian.Tests/` (see *Tests* below).
+
+If the new monitor writes to `HKLM`, route it through `ElevatedRegistry` so it shares the existing UAC-prompt behavior.
+
+## Adding a new Windows service to the catalog
+
+For the `Windows services` tab, just append to `ServiceCatalog.All` in `src/GamerGuardian/Services/ServiceCatalog.cs`. No code change required elsewhere — `WindowsServiceMonitor` is registered once per catalog entry by `App.xaml.cs`.
+
+If the service is one Windows actively protects (re-enables via `WaaSMedicSvc` etc.), set `RecommendedTarget: ServiceTargetState.Manual` rather than `Disabled`, or omit it from the catalog entirely. See `docs/wiki/Architecture-rationale.md` for the WU-protection background.
+
+## Tests
+
+We use xUnit. The test project lives at `tests/GamerGuardian.Tests/`.
+
+Run all tests:
+
+```powershell
+dotnet test
+```
+
+CI runs the same on every PR.
+
+### Test policy
+
+When you add or change behavior:
+
+- **Pure logic** (catalogs, mappings, parsers, lookup tables) — add a unit test covering the new behavior.
+- **Native API wrappers** (anything in `Native/` or `WindowsServiceController`) — add a "doesn't throw on bad input" test if practical. Full coverage isn't expected since these wrap Windows APIs that aren't easily mockable.
+- **UI** — manual verification on a dev-build artifact is the current standard. UI test automation is on the roadmap.
+- **Bug fixes** — add a regression test if the bug is reproducible from a unit test.
+
+The general rule: it's fine to merge without a test if the change can't be reasonably unit-tested (a UI tweak, a workflow change, a doc update). It's not fine to merge without a test if the change touches a class that *is* unit-tested already.
+
+## Reporting issues and requesting features
+
+- **Bug reports / feature requests:** [GitHub Issues](https://github.com/carterscode/GamerGuardian/issues). Search first; include `--test` output and your `changes.log` if relevant.
+- **Security vulnerabilities:** see [SECURITY.md](SECURITY.md). **Do not** open a public issue.
+- **Questions:** also fine in Issues; tag with `question`.
+
+## What reviewers look for
+
+- The change is scoped to one concern.
+- New behavior has a test if reasonably testable.
+- No new compiler warnings.
+- Commit messages explain *why*.
+- No secrets in the diff (GitHub push protection will catch most, but double-check).
+- Touched files have consistent style with the surrounding code.
+- For new dependencies: justified, well-maintained, license-compatible (MIT-friendly).
+
+## License
+
+By contributing you agree your contributions are licensed under the [MIT License](LICENSE), the same license the project uses.

GamerGuardian.sln

@@ -1,7 +1,11 @@
-Microsoft Visual Studio Solution File, Format Version 12.00
+Microsoft Visual Studio Solution File, Format Version 12.00
 # Visual Studio Version 17
 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "GamerGuardian", "src\GamerGuardian\GamerGuardian.csproj", "{8A1E0001-0001-0001-0001-000000000001}"
 EndProject
+Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "tests", "tests", "{6C24C775-257A-472D-A077-0A1AE0BEB2EB}"
+EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "GamerGuardian.Tests", "tests\GamerGuardian.Tests\GamerGuardian.Tests.csproj", "{7C117441-4E51-4330-AECC-C2343F5C2CC3}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -12,5 +16,12 @@ Global
 		{8A1E0001-0001-0001-0001-000000000001}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{8A1E0001-0001-0001-0001-000000000001}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{8A1E0001-0001-0001-0001-000000000001}.Release|Any CPU.Build.0 = Release|Any CPU
+		{7C117441-4E51-4330-AECC-C2343F5C2CC3}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{7C117441-4E51-4330-AECC-C2343F5C2CC3}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{7C117441-4E51-4330-AECC-C2343F5C2CC3}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{7C117441-4E51-4330-AECC-C2343F5C2CC3}.Release|Any CPU.Build.0 = Release|Any CPU
+	EndGlobalSection
+	GlobalSection(NestedProjects) = preSolution
+		{7C117441-4E51-4330-AECC-C2343F5C2CC3} = {6C24C775-257A-472D-A077-0A1AE0BEB2EB}
 	EndGlobalSection
 EndGlobal

src/GamerGuardian/GamerGuardian.csproj

@@ -27,6 +27,9 @@
          same commit produces byte-identical output across runs. -->
     <Deterministic>true</Deterministic>
     <DebugType>portable</DebugType>
+
+    <!-- Warnings break the build. Keeps regressions from sneaking in. -->
+    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
   </PropertyGroup>
 
   <PropertyGroup Condition="'$(GITHUB_ACTIONS)' == 'true'">

tests/GamerGuardian.Tests/GamerGuardian.Tests.csproj

@@ -0,0 +1,25 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <TargetFramework>net8.0-windows10.0.22000.0</TargetFramework>
+    <Nullable>enable</Nullable>
+    <ImplicitUsings>enable</ImplicitUsings>
+    <UseWPF>true</UseWPF>
+    <SupportedOSPlatformVersion>10.0.22000.0</SupportedOSPlatformVersion>
+    <IsPackable>false</IsPackable>
+    <IsTestProject>true</IsTestProject>
+    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.12.0" />
+    <PackageReference Include="xunit" Version="2.9.2" />
+    <PackageReference Include="xunit.runner.visualstudio" Version="2.8.2">
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+      <PrivateAssets>all</PrivateAssets>
+    </PackageReference>
+  </ItemGroup>
+
+  <ItemGroup>
+    <ProjectReference Include="..\..\src\GamerGuardian\GamerGuardian.csproj" />
+  </ItemGroup>
+</Project>

tests/GamerGuardian.Tests/ServiceCatalogTests.cs

@@ -0,0 +1,69 @@
+using GamerGuardian.Models;
+using GamerGuardian.Services;
+using Xunit;
+
+namespace GamerGuardian.Tests;
+
+public class ServiceCatalogTests
+{
+    [Fact]
+    public void All_ContainsServices()
+    {
+        Assert.NotEmpty(ServiceCatalog.All);
+    }
+
+    [Fact]
+    public void All_HasNoDuplicateServiceNames()
+    {
+        var names = ServiceCatalog.All.Select(d => d.Name).ToList();
+        Assert.Equal(names.Count, names.Distinct(StringComparer.OrdinalIgnoreCase).Count());
+    }
+
+    [Fact]
+    public void All_EveryEntryHasNonEmptyDisplayNameAndDescription()
+    {
+        foreach (var def in ServiceCatalog.All)
+        {
+            Assert.False(string.IsNullOrWhiteSpace(def.Name), $"empty Name for an entry");
+            Assert.False(string.IsNullOrWhiteSpace(def.DisplayName), $"empty DisplayName for {def.Name}");
+            Assert.False(string.IsNullOrWhiteSpace(def.Description), $"empty Description for {def.Name}");
+        }
+    }
+
+    [Fact]
+    public void All_DefaultStartTypeIsKnown()
+    {
+        foreach (var def in ServiceCatalog.All)
+        {
+            Assert.NotEqual(ServiceStartType.Unknown, def.DefaultStartType);
+        }
+    }
+
+    [Fact]
+    public void RecommendedTarget_NeverDefault()
+    {
+        // RecommendedTarget == Default would mean "the preset moves it to where it
+        // already is" which is meaningless. The convention is: leave RecommendedTarget
+        // null for services that aren't in the preset, otherwise specify Manual or Disabled.
+        foreach (var def in ServiceCatalog.All)
+        {
+            if (def.RecommendedTarget.HasValue)
+            {
+                Assert.NotEqual(ServiceTargetState.Default, def.RecommendedTarget.Value);
+            }
+        }
+    }
+
+    [Theory]
+    [InlineData("DiagTrack")]
+    [InlineData("MapsBroker")]
+    [InlineData("Fax")]
+    [InlineData("Spooler")]
+    [InlineData("DoSvc")]
+    [InlineData("iphlpsvc")]
+    public void All_IncludesExpectedServices(string serviceName)
+    {
+        Assert.Contains(ServiceCatalog.All, d =>
+            d.Name.Equals(serviceName, StringComparison.OrdinalIgnoreCase));
+    }
+}

tests/GamerGuardian.Tests/SettingDocsTests.cs

@@ -0,0 +1,67 @@
+using GamerGuardian.Services;
+using Xunit;
+
+namespace GamerGuardian.Tests;
+
+public class SettingDocsTests
+{
+    [Theory]
+    [InlineData("hags")]
+    [InlineData("memintegrity")]
+    [InlineData("gamemode")]
+    [InlineData("gamedvr")]
+    [InlineData("mouseaccel")]
+    [InlineData("fso")]
+    [InlineData("vrr")]
+    [InlineData("sysresponse")]
+    [InlineData("netthrottle")]
+    [InlineData("usbsuspend")]
+    [InlineData("gamestask")]
+    [InlineData("powerplan")]
+    public void MechanismFor_KnownIds_ReturnsNonEmpty(string id)
+    {
+        var mech = SettingDocs.MechanismFor(id);
+        Assert.False(string.IsNullOrWhiteSpace(mech), $"no Mechanism for {id}");
+        Assert.NotEqual("(unknown)", mech);
+    }
+
+    [Theory]
+    [InlineData("hags")]
+    [InlineData("memintegrity")]
+    [InlineData("gamemode")]
+    [InlineData("powerplan")]
+    public void VerifyCommandFor_KnownIds_ReturnsNonEmpty(string id)
+    {
+        var cmd = SettingDocs.VerifyCommandFor(id);
+        Assert.False(string.IsNullOrWhiteSpace(cmd), $"no Verify command for {id}");
+    }
+
+    [Fact]
+    public void MechanismFor_DisplayPrefixIds_RecognizesAllThree()
+    {
+        Assert.NotEqual("(unknown)", SettingDocs.MechanismFor("hdr:DISPLAY1"));
+        Assert.NotEqual("(unknown)", SettingDocs.MechanismFor("refresh:DISPLAY1"));
+        Assert.NotEqual("(unknown)", SettingDocs.MechanismFor("resolution:DISPLAY1"));
+    }
+
+    [Fact]
+    public void MechanismFor_ServicePrefix_IncludesServiceName()
+    {
+        var mech = SettingDocs.MechanismFor("service:diagtrack");
+        Assert.Contains("diagtrack", mech, StringComparison.OrdinalIgnoreCase);
+    }
+
+    [Fact]
+    public void VerifyCommandFor_ServicePrefix_IncludesScQc()
+    {
+        var cmd = SettingDocs.VerifyCommandFor("service:diagtrack");
+        Assert.Contains("sc qc", cmd);
+        Assert.Contains("diagtrack", cmd, StringComparison.OrdinalIgnoreCase);
+    }
+
+    [Fact]
+    public void MechanismFor_UnknownId_ReturnsUnknownMarker()
+    {
+        Assert.Equal("(unknown)", SettingDocs.MechanismFor("definitely_not_a_real_setting"));
+    }
+}

tests/GamerGuardian.Tests/WindowsServiceControllerTests.cs

@@ -0,0 +1,46 @@
+using GamerGuardian.Models;
+using GamerGuardian.Services;
+using Xunit;
+
+namespace GamerGuardian.Tests;
+
+public class WindowsServiceControllerTests
+{
+    private const string DefinitelyNotAService = "GamerGuardianFakeServiceForTests";
+
+    [Fact]
+    public void Exists_NonexistentService_ReturnsFalse()
+    {
+        Assert.False(WindowsServiceController.Exists(DefinitelyNotAService));
+    }
+
+    [Fact]
+    public void ReadStartType_NonexistentService_ReturnsUnknown()
+    {
+        // Should not throw; should return Unknown so callers can treat it as "skip".
+        Assert.Equal(ServiceStartType.Unknown, WindowsServiceController.ReadStartType(DefinitelyNotAService));
+    }
+
+    [Fact]
+    public void ReadStatus_NonexistentService_ReturnsNull()
+    {
+        Assert.Null(WindowsServiceController.ReadStatus(DefinitelyNotAService));
+    }
+
+    // EventLog is a service that exists on every supported Windows install and
+    // boots automatically. Reading its registry start type should succeed and
+    // never return Unknown. We don't assert the specific value because Microsoft
+    // has changed it over time (Auto vs AutoDelayed).
+    [Fact]
+    public void ReadStartType_EventLog_ReturnsKnown()
+    {
+        var start = WindowsServiceController.ReadStartType("EventLog");
+        Assert.NotEqual(ServiceStartType.Unknown, start);
+    }
+
+    [Fact]
+    public void Exists_EventLog_ReturnsTrue()
+    {
+        Assert.True(WindowsServiceController.Exists("EventLog"));
+    }
+}