Skip to content

feat: verify release asset integrity with SHA256 checksums #38

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hieuck merged 1 commit into from
Jul 8, 2026
+234 −0
Merged

feat: verify release asset integrity with SHA256 checksums #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .github/workflows/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,23 @@ jobs:
"$dist/KeePassAutoReload.Updater.exe" -Force
Copy-Item "README.md" "$dist/README.md" -Force

$dllHash = (Get-FileHash "$dist/KeePassAutoReload.dll" -Algorithm SHA256).Hash.ToLower()
$updaterHash = (Get-FileHash "$dist/KeePassAutoReload.Updater.exe" -Algorithm SHA256).Hash.ToLower()
$zipHash = $null

Compress-Archive -Path "$dist/KeePassAutoReload.dll", "$dist/KeePassAutoReload.Updater.exe", "$dist/README.md" `
-DestinationPath "$dist/KeePassAutoReload.zip" -Force
$zipHash = (Get-FileHash "$dist/KeePassAutoReload.zip" -Algorithm SHA256).Hash.ToLower()

$checksums = @(
"$dllHash KeePassAutoReload.dll",
"$updaterHash KeePassAutoReload.Updater.exe",
"$zipHash KeePassAutoReload.zip"
)
$checksums | Out-File "$dist/SHA256SUMS.txt" -Encoding utf8

Write-Host "SHA256SUMS.txt contents:"
Get-Content "$dist/SHA256SUMS.txt"

- name: Upload build artifact
uses: actions/upload-artifact@v7
Expand All @@ -100,3 +115,4 @@ jobs:
dist/KeePassAutoReload.dll
dist/KeePassAutoReload.Updater.exe
dist/KeePassAutoReload.zip
dist/SHA256SUMS.txt
70 changes: 70 additions & 0 deletions src/AssetVerifier.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Security.Cryptography;
using System.Text;

namespace KeePassAutoReload
{
internal static class AssetVerifier
{
public static string ComputeSha256(string filePath)
{
if (string.IsNullOrWhiteSpace(filePath)) throw new ArgumentException("filePath");

using (FileStream stream = File.OpenRead(filePath))
using (SHA256 sha256 = SHA256.Create())
{
byte[] hash = sha256.ComputeHash(stream);
return BytesToHex(hash);
}
Comment on lines +15 to +20

@codereviewbot-ai codereviewbot-ai Bot Jul 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ComputeSha256 method does not handle IO-related exceptions (e.g., file not found, access denied) that may occur when opening or reading the file. This can cause the application to crash if the caller does not catch these exceptions.

Recommendation:
Wrap the file operations in a try-catch block and return a meaningful error or propagate a custom exception, depending on the intended usage. For example:

try {
    using (FileStream stream = File.OpenRead(filePath))
    using (SHA256 sha256 = SHA256.Create())
    {
        byte[] hash = sha256.ComputeHash(stream);
        return BytesToHex(hash);
    }
} catch (IOException ex) {
    // Handle or log the exception as appropriate
    throw new InvalidOperationException($"Failed to compute hash for '{filePath}'", ex);
}

}

public static Dictionary<string, string> ParseChecksums(string checksumsText)
{
Dictionary<string, string> entries = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
if (string.IsNullOrWhiteSpace(checksumsText)) return entries;

using (StringReader reader = new StringReader(checksumsText))
{
string line;
while ((line = reader.ReadLine()) != null)
{
string trimmed = line.Trim();
if (string.IsNullOrEmpty(trimmed)) continue;

int separatorIndex = trimmed.IndexOf(' ');
if (separatorIndex <= 0) continue;

string hash = trimmed.Substring(0, separatorIndex).Trim();
string fileName = trimmed.Substring(separatorIndex + 1).Trim();
Comment on lines +36 to +40

@codereviewbot-ai codereviewbot-ai Bot Jul 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parsing logic in ParseChecksums splits each line on the first space character, which assumes that file names do not contain spaces. If a file name contains spaces, this will result in incorrect parsing and potentially invalid hash-to-file mappings.

Recommendation:
Consider using a more robust parsing strategy, such as splitting on the last space or using a delimiter that cannot appear in file names. Alternatively, document the expected format and enforce it during checksum file generation.


if (string.IsNullOrEmpty(hash) || string.IsNullOrEmpty(fileName)) continue;

entries[fileName] = hash;
}
}

return entries;
}

public static bool VerifyFile(string filePath, string expectedHash)
{
if (string.IsNullOrWhiteSpace(filePath) || string.IsNullOrWhiteSpace(expectedHash)) return false;
if (!File.Exists(filePath)) return false;

string actualHash = ComputeSha256(filePath);
return string.Equals(actualHash, expectedHash.Trim(), StringComparison.OrdinalIgnoreCase);
}

private static string BytesToHex(byte[] bytes)
{
StringBuilder builder = new StringBuilder(bytes.Length * 2);
foreach (byte b in bytes)
{
builder.Append(b.ToString("x2"));
}
return builder.ToString();
}
}
}
1 change: 1 addition & 0 deletions src/KeePassAutoReload.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
</Reference>
</ItemGroup>
<ItemGroup>
<Compile Include="AssetVerifier.cs" />
<Compile Include="AutoSyncPolicy.cs" />
<Compile Include="KeePassAutoReloadExt.cs" />
<Compile Include="PluginAboutInfo.cs" />
Expand Down
34 changes: 34 additions & 0 deletions src/KeePassAutoReloadExt.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Threading.Tasks;
Expand Down Expand Up @@ -179,16 +180,37 @@ private async Task InstallUpdate(UpdateInfo info)
{
string targetPath = GetPluginPackagePath();
string tempPath = targetPath + ".download";
string checksumPath = targetPath + ".sha256";

using (HttpUpdateClient client = new HttpUpdateClient())
{
await client.DownloadFileAsync(info.AssetUrl, tempPath);
if (!string.IsNullOrWhiteSpace(info.ChecksumUrl))
{
try
{
await client.DownloadFileAsync(info.ChecksumUrl, checksumPath);
}
catch (Exception checksumEx)
{
File.Delete(tempPath);
throw new InvalidOperationException("Failed to download release checksums: " + checksumEx.Message, checksumEx);
}
}
}

if (File.Exists(checksumPath) && !VerifyDownloadedAsset(tempPath, checksumPath, Path.GetFileName(info.AssetUrl)))
{
File.Delete(tempPath);
File.Delete(checksumPath);
throw new InvalidOperationException("Downloaded asset failed hash verification.");
}

try
{
File.Copy(tempPath, targetPath, true);
File.Delete(tempPath);
if (File.Exists(checksumPath)) File.Delete(checksumPath);

ShowOnUi(delegate
{
Comment on lines 180 to 216

@codereviewbot-ai codereviewbot-ai Bot Jul 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The update installation logic performs file copy and delete operations that are not atomic, which may result in a partially updated or corrupted plugin if the process is interrupted or if there is concurrent access. To improve reliability and safety, consider using atomic file replacement techniques (such as writing to a temporary file and then using File.Replace or equivalent) and ensure all file operations are robust against interruptions. Additionally, validate all file paths before performing operations to prevent potential path traversal or unintended file overwrites.

Expand All @@ -202,6 +224,7 @@ private async Task InstallUpdate(UpdateInfo info)
string pendingPath = targetPath + ".new";
File.Copy(tempPath, pendingPath, true);
File.Delete(tempPath);
if (File.Exists(checksumPath)) File.Delete(checksumPath);

bool updaterScheduled = TryScheduleUpdater(targetPath, pendingPath);

Expand Down Expand Up @@ -234,6 +257,17 @@ private async Task InstallUpdate(UpdateInfo info)
}
}

private static bool VerifyDownloadedAsset(string assetPath, string checksumPath, string assetFileName)
{
if (!File.Exists(checksumPath)) return true;

string checksumsText = File.ReadAllText(checksumPath);
Dictionary<string, string> checksums = AssetVerifier.ParseChecksums(checksumsText);
if (!checksums.ContainsKey(assetFileName)) return true;

return AssetVerifier.VerifyFile(assetPath, checksums[assetFileName]);
}

private static string GetPluginPackagePath()
{
return PluginPathResolver.ResolvePluginPackagePath(
Comment on lines 257 to 273

@codereviewbot-ai codereviewbot-ai Bot Jul 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method GetPluginPackagePath() does not validate the returned path, which could lead to security issues if the path is outside the intended plugin directory. Ensure that the resolved path is within the expected directory and does not allow for path traversal or writing to unintended locations. This is especially important when performing update operations that overwrite executable code.

Expand Down
8 changes: 8 additions & 0 deletions src/UpdateChecker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ internal sealed class UpdateInfo
public string LatestVersion;
public string ReleaseUrl;
public string AssetUrl;
public string ChecksumUrl;
public bool IsUpdateAvailable;
}

Expand Down Expand Up @@ -115,6 +116,7 @@ public static async Task<UpdateInfo> CheckLatestAsync(IUpdateClient client, Plug
info.LatestVersion = tagName;
info.ReleaseUrl = BuildReleaseUrl(tagName);
info.AssetUrl = BuildAssetUrl(tagName, format);
info.ChecksumUrl = BuildChecksumUrl(tagName);
info.IsUpdateAvailable = IsNewerVersion(GetCurrentVersion(), tagName);
return info;
}
Comment on lines 116 to 122

@codereviewbot-ai codereviewbot-ai Bot Jul 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code constructs the UpdateInfo object without validating whether tagName is null or empty. If the GitHub API response is malformed or contains no valid tags, tagName will be empty, resulting in URLs that point to the generic releases page and potentially setting IsUpdateAvailable incorrectly. This could mislead users or cause downstream errors.

Recommended solution:
Add a check after extracting tagName to ensure it is not null or empty. If it is, either throw an exception or return an UpdateInfo object indicating that update information could not be determined.

if (string.IsNullOrEmpty(tagName))
    throw new InvalidOperationException("No valid release tag found in the API response.");

Expand Down Expand Up @@ -181,6 +183,12 @@ private static string BuildReleaseUrl(string tagName)
return ReleasesUrl + "/tag/" + tagName;
}

private static string BuildChecksumUrl(string tagName)
{
if (string.IsNullOrEmpty(tagName)) return ReleasesUrl;
return "https://github.com/hieuck/KeePassAutoReload/releases/download/" + tagName + "/SHA256SUMS.txt";
}

private static List<string> ExtractJsonStrings(string json, string name)
{
List<string> values = new List<string>();
Comment on lines 193 to 194

@codereviewbot-ai codereviewbot-ai Bot Jul 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fragile JSON Parsing with Regex

The ExtractJsonStrings method uses a regular expression to extract values from JSON. Parsing JSON with regex is error-prone and fragile, especially if the JSON structure changes, contains nested objects, or includes escaped characters. This can lead to incorrect extraction or missed values, impacting the reliability of update checks.

Recommended solution:
Use a proper JSON parser such as System.Text.Json or Newtonsoft.Json to extract values robustly:

using System.Text.Json;
...
var doc = JsonDocument.Parse(json);
// Extract values using JsonElement traversal

This approach is more maintainable and less error-prone.

Expand Down
1 change: 1 addition & 0 deletions tests/KeePassAutoReload.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<IsPackable>false</IsPackable>
</PropertyGroup>
<ItemGroup>
<Compile Include="..\src\AssetVerifier.cs" Link="AssetVerifier.cs" />
<Compile Include="..\src\AutoSyncPolicy.cs" Link="AutoSyncPolicy.cs" />
<Compile Include="..\src\PluginAboutInfo.cs" Link="PluginAboutInfo.cs" />
<Compile Include="..\src\PluginPathResolver.cs" Link="PluginPathResolver.cs" />
Expand Down
104 changes: 104 additions & 0 deletions tests/Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Net;
using System.Security.Authentication;
Expand Down Expand Up @@ -532,6 +533,109 @@ public void TryScheduleUpdate_ReturnsTrueAndStartsUpdaterWhenFilesExist()
}
}

public class AssetVerifierTests
{
[Fact]
public void ComputeSha256_ReturnsHexHashOfFileContents()
{
string tempFile = Path.GetTempFileName();
try
{
File.WriteAllText(tempFile, "hello world");
string hash = AssetVerifier.ComputeSha256(tempFile);
Assert.Equal("B94D27B9934D3E08A52E52D7DA7DABFAC484EFE37A5380EE9088F7ACE2EFCDE9", hash.ToUpperInvariant());
}
finally
{
File.Delete(tempFile);
}
}

[Fact]
public void ComputeSha256_ThrowsWhenFileDoesNotExist()
{
string tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
Directory.CreateDirectory(tempDir);
string tempFile = Path.Combine(tempDir, "missing.dll");
Assert.Throws<FileNotFoundException>(() => AssetVerifier.ComputeSha256(tempFile));
}

[Fact]
public void ParseChecksums_ReturnsExpectedEntries()
{
string checksums =
"abc123 KeePassAutoReload.dll\n" +
"def456 KeePassAutoReload.Updater.exe\n";

Dictionary<string, string> entries = AssetVerifier.ParseChecksums(checksums);

Assert.Equal(2, entries.Count);
Assert.Equal("abc123", entries["KeePassAutoReload.dll"]);
Assert.Equal("def456", entries["KeePassAutoReload.Updater.exe"]);
}

[Fact]
public void ParseChecksums_IgnoresBlankLinesAndMalformedEntries()
{
string checksums =
"abc123 KeePassAutoReload.dll\n" +
"\n" +
"badline\n" +
"def456 KeePassAutoReload.Updater.exe\n";

Dictionary<string, string> entries = AssetVerifier.ParseChecksums(checksums);

Assert.Equal(2, entries.Count);
}

[Fact]
public void VerifyFile_ReturnsTrueWhenHashMatches()
{
string tempFile = Path.GetTempFileName();
try
{
File.WriteAllText(tempFile, "hello world");
string hash = AssetVerifier.ComputeSha256(tempFile);
Assert.True(AssetVerifier.VerifyFile(tempFile, hash));
}
finally
{
File.Delete(tempFile);
}
}

[Fact]
public void VerifyFile_ReturnsFalseWhenHashDoesNotMatch()
{
string tempFile = Path.GetTempFileName();
try
{
File.WriteAllText(tempFile, "hello world");
Assert.False(AssetVerifier.VerifyFile(tempFile, "0000000000000000000000000000000000000000000000000000000000000000"));
}
finally
{
File.Delete(tempFile);
}
}

[Fact]
public void VerifyFile_IsCaseInsensitive()
{
string tempFile = Path.GetTempFileName();
try
{
File.WriteAllText(tempFile, "hello world");
string hash = AssetVerifier.ComputeSha256(tempFile).ToLowerInvariant();
Assert.True(AssetVerifier.VerifyFile(tempFile, hash.ToUpperInvariant()));
}
finally
{
File.Delete(tempFile);
}
}
}

internal sealed class FakeUpdateClient : IUpdateClient
{
public string Response { get; set; }
Expand Down