serilog-contrib / Serilog.Enrichers.Sensitive

A Serilog LogEvent enricher that masks sensitive data
MIT License
111 stars 23 forks source link

added mask for uri as scalar value #31

Closed yadanilov19 closed 11 months ago

yadanilov19 commented 11 months ago

Hi!  I've faced problems when Uri pushed in log as param and it wasn't masked. I added an additional case section to handle that.

sandermvanvliet commented 11 months ago

I've just created a test to see if I could reproduce the issue but it shows me that if I push a Uri instance as the value of a property that it is correctly masked:

public class WhenLoggingMessageWithUriPropertyValue
{
    private readonly InMemorySink _inMemorySnk;
    private readonly Logger _logger;

    [Fact]
    public void GivenPropertyValueIsUri_UriIsMasked()
    {
        _logger.Information("Message {Prop}", new Uri("https://tempuri.org"));

        _inMemorySnk
            .Snapshot()
            .Should()
            .HaveMessage("Message {Prop}")
            .Appearing()
            .Once()
            .WithProperty("Prop")
            .WithValue("***MASKED***");
    }

    public WhenLoggingMessageWithUriPropertyValue()
    {
        _inMemorySnk = new InMemorySink();

        _logger = new LoggerConfiguration()
            .Enrich.WithSensitiveDataMasking(options => options.MaskProperties.Add("Prop"))
            .WriteTo.Sink(_inMemorySnk)
            .CreateLogger();
    }
}

Could you maybe adapt the test above to your particular situation where it fails? That would help me resolve your issue.

yadanilov19 commented 11 months ago

Sure! The problem appeared when I tried to mask info by my own realization of RegexMaskingOperator. Here is an adapted version of your test:

using System;
using System.Text.RegularExpressions;
using Serilog.Core;
using Serilog.Sinks.InMemory;
using Serilog.Sinks.InMemory.Assertions;
using Xunit;

namespace Serilog.Enrichers.Sensitive.Tests.Unit;

public class SomeMaskingOperator : RegexMaskingOperator
{
    private const string SomePattern =
        "someparam=.*?(.(?:&|$))";

    public SomeMaskingOperator() : base(SomePattern, RegexOptions.IgnoreCase | RegexOptions.Compiled)
    {
    }

    protected override string PreprocessInput(string input)
    {
        return input;
    }
}

public class WhenLoggingMessageWithUriPropertyValue
{
    private readonly InMemorySink _inMemorySnk;
    private readonly Logger _logger;

    [Fact]
    public void GivenPropertyValueIsUri_UriIsMasked()
    {
        _logger.Information("Endpoint {Endpoint}", new Uri("https://tempuri.org/someparam=test@test.com"));

        _inMemorySnk
            .Snapshot()
            .Should()
            .HaveMessage("Endpoint {Endpoint}")
            .Appearing()
            .Once()
            .WithProperty("Endpoint")
            .WithValue("https://tempuri.org/someparam=***MASKED***");
    }

    public WhenLoggingMessageWithUriPropertyValue()
    {
        _inMemorySnk = new InMemorySink();

        _logger = new LoggerConfiguration()
            .Enrich.WithSensitiveDataMasking(options =>
            {
                options.MaskingOperators.Add(new SomeMaskingOperator());
            })
            .WriteTo.Sink(_inMemorySnk)
            .CreateLogger();
    }
}
sandermvanvliet commented 11 months ago

Thanks, I’ll take a stab at that 👍

sandermvanvliet commented 11 months ago

So I've figured out why this wasn't being caught before by the string masking part. Turns out that Uri is a built-in type in Serilog and is therefore handled slightly different when rendered and retrieved as a property (see here).

sandermvanvliet commented 11 months ago

@yadanilov19 I've created a separate PR so that I could add a few tests to it and published a new release (1.7.3) that should hit NuGet shortly.

Thanks for reporting this one 👍