Skip to content

fix: stop leaking raw upstream body in NetworkException (#41) #43

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

Draft
dobby-coder wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix: stop leaking raw upstream body in NetworkException (#41) #43

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
10 changes: 7 additions & 3 deletions src/Exceptions/PostGuardException.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,20 @@ public PostGuardException(string message, Exception inner) : base(message, inner

public class NetworkException : PostGuardException
{
// Raw upstream response body, retained for diagnostics only. Kept private
// so it is never surfaced in the exception message or via a public property,
// where it could leak sensitive upstream data into logs. See issue #41.
private readonly string _body;

@dobby-coder dobby-coder Bot Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The private _body field is assigned in the ctor but never read anywhere — no accessor, method, or ToString() override consumes it — so the "retained for diagnostics" comment is misleading: it provides no diagnostic access and only keeps the sensitive upstream body alive on the exception object. This is acceptable because the task explicitly asked to "store it in a private field," so no change is required, but as written it is effectively dead storage. If diagnostic access is genuinely wanted, expose it through a redacted/internal accessor; otherwise the comment could be trimmed. (Build stays clean — assigned from a ctor param, so no CS0414.)


public int StatusCode { get; }
public string Body { get; }
public string Url { get; }

public NetworkException(int statusCode, string body, string url)
: base($"HTTP {statusCode} at {url}: {body}")
: base($"HTTP {statusCode} received from upstream service")

@dobby-coder dobby-coder Bot Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removing the public Body property is a breaking change to the published SDK's public API, but this PR uses a fix: conventional-commit prefix. Under release-please (see PR #38 / release 0.5.1) a fix: is cut as a patch bump and will NOT signal the API break to downstream consumers who reference .Body. Please signal the break — either retitle as feat!: or add a BREAKING CHANGE: removed public NetworkException.Body property footer to the commit — so the removed member surfaces in the changelog and triggers an appropriate version bump.

{
StatusCode = statusCode;
Body = body;
Url = url;
_body = body;
}
}

Expand Down
25 changes: 18 additions & 7 deletions tests/E4A.PostGuard.Tests/NetworkExceptionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,31 +5,42 @@ namespace E4A.PostGuard.Tests;
public class NetworkExceptionTests
{
[Fact]
public void Ctor_PopulatesUrlProperty()
public void Ctor_PopulatesStatusAndUrlProperties()
{
var ex = new NetworkException(401, "Unauthorized", "https://pkg.postguard.eu/v2/irma/sign/key");

Assert.Equal(401, ex.StatusCode);
Assert.Equal("Unauthorized", ex.Body);
Assert.Equal("https://pkg.postguard.eu/v2/irma/sign/key", ex.Url);
}

[Fact]
public void Ctor_IncludesUrlAndStatusInMessage()
public void Ctor_MessageIsGenericAndIncludesStatus()
{
var ex = new NetworkException(500, "boom", "https://cryptify.postguard.eu/fileupload/init");

Assert.Contains("500", ex.Message);
Assert.Contains("https://cryptify.postguard.eu/fileupload/init", ex.Message);
Assert.Contains("boom", ex.Message);
Assert.Equal("HTTP 500 received from upstream service", ex.Message);
}

[Fact]
public void Ctor_MessageDoesNotLeakRawUpstreamBody()
{
var ex = new NetworkException(500, "sensitive-upstream-detail", "https://cryptify.postguard.eu/fileupload/init");

Assert.DoesNotContain("sensitive-upstream-detail", ex.Message);
}

[Fact]
public void NetworkException_DoesNotExposePublicBodyProperty()
{
Assert.Null(typeof(NetworkException).GetProperty("Body"));
}

[Fact]
public void Ctor_AllowsEmptyBody()
{
var ex = new NetworkException(204, "", "https://pkg.postguard.eu/v2/parameters");

Assert.Equal("", ex.Body);
Assert.Equal(204, ex.StatusCode);
Assert.Equal("https://pkg.postguard.eu/v2/parameters", ex.Url);
}
}
Loading