Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
113 changes: 113 additions & 0 deletions src/Nethermind/Nethermind.Config.Test/ConfigProviderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using FluentAssertions;
using Nethermind.Core.Extensions;
using Nethermind.JsonRpc;
using Nethermind.KeyStore.Config;
using Nethermind.Network.Config;
using NUnit.Framework;

Expand Down Expand Up @@ -102,4 +104,115 @@ public void Can_useExistingConfig()
configProvider.GetConfig<IBlocksConfig>().MinGasPrice.Should().Be(12345);
}
}

[TestFixture]
[Parallelizable(ParallelScope.None)]
public class GetNonDefaultValuesTests
{
[Test]
public void Returns_empty_when_no_overrides()
{
ConfigProvider configProvider = new();
configProvider.Initialize();

List<(string Category, string Name, object? CurrentValue, object? DefaultValue)> nonDefaults =
configProvider.GetNonDefaultValues().ToList();

Assert.That(nonDefaults, Is.Empty);
}

[Test]
public void Returns_overridden_value_with_current_and_default()
{
Dictionary<string, string> args = new()
{
{ "Network.DiscoveryPort", "12345" }
};
ConfigProvider configProvider = new();
configProvider.AddSource(new ArgsConfigSource(args));
configProvider.Initialize();

List<(string Category, string Name, object? CurrentValue, object? DefaultValue)> nonDefaults =
configProvider.GetNonDefaultValues()
.Where(static x => x.Category == "Network" && x.Name == nameof(INetworkConfig.DiscoveryPort))
.ToList();

Assert.That(nonDefaults, Has.Count.EqualTo(1));
Assert.That(nonDefaults[0].CurrentValue, Is.EqualTo(12345));
Assert.That(nonDefaults[0].DefaultValue, Is.EqualTo(30303));
}

[Test]
public void Surfaces_overrides_across_categories()
{
Dictionary<string, string> args = new()
{
{ "Network.DiscoveryPort", "12345" },
{ "JsonRpc.Enabled", "true" }
};
ConfigProvider configProvider = new();
configProvider.AddSource(new ArgsConfigSource(args));
configProvider.Initialize();

HashSet<string> keys = configProvider.GetNonDefaultValues()
.Select(static x => $"{x.Category}.{x.Name}")
.ToHashSet();

Assert.That(keys, Does.Contain("Network.DiscoveryPort"));
Assert.That(keys, Does.Contain("JsonRpc.Enabled"));
}

[Test]
public void Skips_sensitive_properties()
{
Dictionary<string, string> args = new()
{
{ "KeyStore.TestNodeKey", "0xdeadbeef" },
{ "KeyStore.Passwords", "[\"hunter2\"]" },
{ "KeyStore.KeyStoreDirectory", "custom-keystore" }
};
ConfigProvider configProvider = new();
configProvider.AddSource(new ArgsConfigSource(args));
configProvider.Initialize();

HashSet<string> keys = configProvider.GetNonDefaultValues()
.Select(static x => $"{x.Category}.{x.Name}")
.ToHashSet();

Assert.That(keys, Does.Not.Contain($"KeyStore.{nameof(IKeyStoreConfig.TestNodeKey)}"));
Assert.That(keys, Does.Not.Contain($"KeyStore.{nameof(IKeyStoreConfig.Passwords)}"));
Assert.That(keys, Does.Contain($"KeyStore.{nameof(IKeyStoreConfig.KeyStoreDirectory)}"));
}

[Test]
public void Failing_provider_for_one_type_does_not_poison_others()
{
Dictionary<string, string> args = new() { { "Network.DiscoveryPort", "12345" } };
ConfigProvider inner = new();
inner.AddSource(new ArgsConfigSource(args));
inner.Initialize();

FailingProvider failing = new(inner, typeof(IJsonRpcConfig));
List<(Type ConfigType, Exception Error)> errors = [];

HashSet<string> keys = failing.GetNonDefaultValues((t, e) => errors.Add((t, e)))
.Select(static x => $"{x.Category}.{x.Name}")
.ToHashSet();

Assert.That(keys, Does.Contain("Network.DiscoveryPort"));
Assert.That(errors, Has.Some.Matches<(Type, Exception)>(static x => x.Item1 == typeof(IJsonRpcConfig)));
}

private sealed class FailingProvider(IConfigProvider inner, Type failingType) : IConfigProvider
{
public T GetConfig<T>() where T : IConfig => (T)GetConfig(typeof(T));

public IConfig GetConfig(Type configType) =>
configType == failingType
? throw new InvalidOperationException("simulated failure")
: inner.GetConfig(configType);

public object? GetRawValue(string category, string name) => inner.GetRawValue(category, name);
}
}
}
71 changes: 71 additions & 0 deletions src/Nethermind/Nethermind.Config/ConfigExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,14 @@
// SPDX-License-Identifier: LGPL-3.0-only

using System;
using System.Collections;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.ComponentModel;
using System.Linq;
using System.Reflection;
using Nethermind.Core;
using Nethermind.Core.Extensions;

namespace Nethermind.Config;

Expand Down Expand Up @@ -41,4 +45,71 @@ public static T GetDefaultValue<T>(this IConfig config, string propertyName)
string? defaultValue = attribute.DefaultValue;
return (T)TypeDescriptor.GetConverter(typeof(T)).ConvertFrom(defaultValue!)!;
}

/// <summary>
/// Enumerates configuration properties whose current value differs from the
/// implementation's default. Useful for surfacing on startup which knobs the
/// operator has actually changed, rather than dumping every value.
/// </summary>
/// <remarks>
/// Defaults come from a freshly constructed instance of the implementing
/// type, so initializers and constructors are honoured exactly as production
/// wiring would do (no parsing of the <see cref="ConfigItemAttribute.DefaultValue"/> string).
/// Properties flagged <see cref="ConfigItemAttribute.IsSensitive"/> are skipped.
/// </remarks>
/// <param name="configProvider">The provider to query.</param>
/// <param name="onConfigError">
/// Optional callback invoked when a single config interface cannot be enumerated
/// (provider lookup or fresh-default construction throws). Enumeration of the
/// remaining interfaces continues regardless. If <c>null</c>, failures are silent.
/// </param>
public static IEnumerable<(string Category, string Name, object? CurrentValue, object? DefaultValue)>
GetNonDefaultValues(this IConfigProvider configProvider, Action<Type, Exception>? onConfigError = null)
{
ArgumentNullException.ThrowIfNull(configProvider);

foreach (Type configInterface in TypeDiscovery.FindNethermindBasedTypes(typeof(IConfig)))
{
if (!configInterface.IsInterface) continue;

IConfig current;
IConfig fresh;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[Medium] Incomplete exception handling

Activator.CreateInstance can also throw TargetInvocationException (if the constructor body throws), TypeLoadException, and MethodAccessException. Any of these would propagate and crash startup for what is a diagnostic-only feature. Broaden the catch:

Suggested change
IConfig fresh;
catch (Exception e) when (e is MissingMethodException or TargetInvocationException or TypeLoadException or MethodAccessException)

string category;
try
{
current = configProvider.GetConfig(configInterface);
fresh = (IConfig)Activator.CreateInstance(current.GetType())!;
category = GetCategoryName(configInterface) ?? string.Empty;
}
catch (Exception e)
{
onConfigError?.Invoke(configInterface, e);
continue;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[High] Sensitive data exposure

Properties like Seq.ApiKey and KeyStore.Passwords will be included here and logged at Info level in Program.cs. A Seq.ApiKey override would be logged into Seq itself; KeyStore.Passwords contains plaintext account unlock passwords.

Consider adding an IsSensitive flag to ConfigItemAttribute and skipping those properties:

Suggested change
// Skip sensitive properties to avoid logging credentials
foreach (PropertyInfo property in configInterface.GetProperties()
.Where(static p => p.CanRead && p.GetCustomAttribute<ConfigItemAttribute>()?.IsSensitive != true))

Or at a minimum filter out properties where the attribute is not present or HiddenFromDocs is true.

foreach (PropertyInfo property in configInterface.GetProperties())
{
if (!property.CanRead) continue;
if (property.GetCustomAttribute<ConfigItemAttribute>()?.IsSensitive == true) continue;

object? actual;
object? defaultValue;
try
{
actual = property.GetValue(current);
defaultValue = property.GetValue(fresh);
}
catch (Exception e)
{
onConfigError?.Invoke(configInterface, e);
continue;
}

if (!StructuralComparisons.StructuralEqualityComparer.Equals(actual, defaultValue))
{
yield return (category, property.Name, actual, defaultValue);
}
}
}
}
}
6 changes: 6 additions & 0 deletions src/Nethermind/Nethermind.Config/ConfigItemAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,10 @@ public class ConfigItemAttribute : Attribute
public bool IsPortOption { get; set; }

public string CliOptionAlias { get; set; } = "";

/// <summary>
/// Marks the property as containing secrets (passwords, API keys, private keys, ...).
/// Such values must never be written to logs or other diagnostic surfaces.
/// </summary>
public bool IsSensitive { get; set; }
}
2 changes: 1 addition & 1 deletion src/Nethermind/Nethermind.EthStats/IEthStatsConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public interface IEthStatsConfig : IConfig
[ConfigItem(Description = "The node name displayed on Ethstats.", DefaultValue = "Nethermind")]
string? Name { get; }

[ConfigItem(Description = "The Ethstats secret.", DefaultValue = "secret")]
[ConfigItem(Description = "The Ethstats secret.", DefaultValue = "secret", IsSensitive = true)]
string? Secret { get; }

[ConfigItem(Description = "The node owner contact details displayed on Ethstats.", DefaultValue = "hello@nethermind.io")]
Expand Down
12 changes: 6 additions & 6 deletions src/Nethermind/Nethermind.KeyStore/Config/IKeystoreConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,25 +50,25 @@ public interface IKeyStoreConfig : IConfig
[ConfigItem(Description = "See [Web3 secret storage definition][web3-secret-storage].", DefaultValue = "16")]
int IVSize { get; set; }

[ConfigItem(Description = "A plaintext private key to use for testing purposes.")]
[ConfigItem(Description = "A plaintext private key to use for testing purposes.", IsSensitive = true)]
string TestNodeKey { get; set; }

[ConfigItem(Description = "An account to use as the block author (coinbase).")]
[ConfigItem(Description = "An account to use as the block author (coinbase).", IsSensitive = true)]
string BlockAuthorAccount { get; set; }

[ConfigItem(Description = $"An account to use for networking (enode). If neither this nor the `{nameof(EnodeKeyFile)}` option is specified, the key is autogenerated in `node.key.plain` file.")]
[ConfigItem(Description = $"An account to use for networking (enode). If neither this nor the `{nameof(EnodeKeyFile)}` option is specified, the key is autogenerated in `node.key.plain` file.", IsSensitive = true)]
string EnodeAccount { get; set; }

[ConfigItem(Description = $"The path to the key file to use by for networking (enode). If neither this nor the `{nameof(EnodeAccount)}` is specified, the key is autogenerated in `node.key.plain` file.")]
[ConfigItem(Description = $"The path to the key file to use by for networking (enode). If neither this nor the `{nameof(EnodeAccount)}` is specified, the key is autogenerated in `node.key.plain` file.", IsSensitive = true)]
string EnodeKeyFile { get; set; }

[ConfigItem(Description = $"An array of passwords used to unlock the accounts set with `{nameof(UnlockAccounts)}`.", DefaultValue = "[]")]
[ConfigItem(Description = $"An array of passwords used to unlock the accounts set with `{nameof(UnlockAccounts)}`.", DefaultValue = "[]", IsSensitive = true)]
string[] Passwords { get; set; }

[ConfigItem(Description = $"An array of password files paths used to unlock the accounts set with `{nameof(UnlockAccounts)}`.", DefaultValue = "[]")]
string[] PasswordFiles { get; set; }

[ConfigItem(Description = $"An array of accounts to unlock on startup using passwords either in `{nameof(PasswordFiles)}` and `{nameof(Passwords)}`.", DefaultValue = "[]")]
[ConfigItem(Description = $"An array of accounts to unlock on startup using passwords either in `{nameof(PasswordFiles)}` and `{nameof(Passwords)}`.", DefaultValue = "[]", IsSensitive = true)]
string[] UnlockAccounts { get; set; }
}

Expand Down
23 changes: 23 additions & 0 deletions src/Nethermind/Nethermind.Runner/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,29 @@ async Task<int> RunAsync(ParseResult parseResult, PluginLoader pluginLoader, Can

EthereumJsonSerializer serializer = new();

if (logger.IsInfo)
{
List<string> nonDefaultLines = [];
Action<Type, Exception> onConfigError = (configType, error) =>
{
if (logger.IsWarn) logger.Warn($"Skipped {configType.Name} in non-default config diff: {error.Message}");
};

foreach ((string category, string name, object? currentValue, object? _) in configProvider.GetNonDefaultValues(onConfigError))
{
nonDefaultLines.Add($" {category}.{name} = {serializer.Serialize(currentValue)}");
}

if (nonDefaultLines.Count == 0)
{
logger.Info("Configuration: all values at defaults.");
}
else
{
logger.Info($"Configuration: {nonDefaultLines.Count} non-default value(s):\n{string.Join('\n', nonDefaultLines)}");
}
}

if (logger.IsDebug)
{
logger.Debug($"Nethermind configuration:\n{serializer.Serialize(initConfig, true)}");
Expand Down
2 changes: 1 addition & 1 deletion src/Nethermind/Nethermind.Seq/Config/ISeqConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ public interface ISeqConfig : IConfig
[ConfigItem(Description = "The Seq instance URL.", DefaultValue = "http://localhost:5341")]
string ServerUrl { get; }

[ConfigItem(Description = "The Seq API key.", DefaultValue = "")]
[ConfigItem(Description = "The Seq API key.", DefaultValue = "", IsSensitive = true)]
string ApiKey { get; }
}