Skip to content
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.

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

Sensitive object logging redaction #3828

Open
hanskokx opened this issue May 22, 2024 · 8 comments
Open

Sensitive object logging redaction #3828

hanskokx opened this issue May 22, 2024 · 8 comments
Labels
feature Proposed language feature that solves one or more problems

Comments

@hanskokx
Copy link

hanskokx commented May 22, 2024

A while back, I described this on the wrong repo. I'm finally getting around to putting it in the right place; here. :)

Consider the following scenario: You've got a data class, such as the following.

class Credentials {
  final String username;
  final String password;

  Credentials({
    required this.username,
    required this.password,
  });

 Map<String, String> toJson() => {'username': username, 'password': password};

  @override
  String toString() => toJson().toString();
}

The following is the current behavior:

final Credentials myCredentials = Credentials(
  username: "hans",
  password: "password",
);

print(myCredentials);

Output:
I/flutter (10392): {username: hans, password: password}

What I'm proposing is a new keyword, such as sensitive, that would look something like this:

class Credentials {
  final String username;
  final String password;

  Credentials({
    required this.username,
    sensitive required this.password,
  });
...

So, at a system level when you try to log/print the parameter, you'd see something like this:

I/flutter (10392): {username: hans, password: <SENSITIVE>}

@hanskokx hanskokx added the feature Proposed language feature that solves one or more problems label May 22, 2024
@jakemac53
Copy link
Contributor

The default behavior for classes would print something like Instance of Credentials. Anything else is coming from some code generator or other similar feature, which is giving you a toString based on the fields.

This is probably an issue for whatever package it is that is giving you that customized toString method, and could be solved however that package accepts configuration (probably an annotation).

@hanskokx
Copy link
Author

My apologies, I erroneously omitted the .toJson and .toString methods that I had included from my previous issue. I have updated the example code to include those.

@jakemac53
Copy link
Contributor

I would suggest just using a wrapper class here:

class Private<T> {
  final T privateValueWhichShouldNotEscape;
  
  Private(this.privateValueWhichShouldNotEscape);
}

That should send the right signal to anybody using the value, that it shouldn't be printed, assigned to other variables, etc.

@jakemac53
Copy link
Contributor

You could possibly even do this as an extension type to make it zero cost.

@lrhn
Copy link
Member

lrhn commented May 22, 2024

Not sure this is something I'd want to solve at the language level.

You have something which is not just a string, it's a password.
As @jakemac53 suggests, wrapping it in a class would avoid accidentally treating it as just any other string.

The problem here is that you both want to convert it to JSON, but also don't want to convert it to a string, but JSON is a text format.
Maybe you can choose to have the toJson function return something still containing to Password object, have Password.toString() return "<Redacted>" and pass a toEncodable function to jsonEncode that converts Password to Password.text (the real text). Then anyone accidentally printing the password, or the JSON-like map structure, will not see the password, but actually converting using jsonEncode (perhaps even directly to UTF-8) will contain the password.

@hanskokx
Copy link
Author

I wouldn't get too hung up on the fact that my example is a password or uses .toJson - these are just examples. It could very well be something like a social security number, or some other private item. The idea here is that it would function as normal, but using any print, debugPrint, or log statement would print out some sort of "redacted" message. Notably, this would not prevent you from, say, splitting the value into a list of glyphs and printing those out. My idea is that it's a simple safety check to prevent ham-fisted and careless spilling of secrets by using debug logging.

@jakemac53
Copy link
Contributor

I wouldn't get too hung up on the fact that my example is a password or uses .toJson

The advice above isn't specific to passwords, which is why I suggested a generic class.

The methods you describe all just call toString() on anything they are given, so a simple wrapper type which has a custom toString() which prints nothing is a very simple and reasonable way to prevent this information from being accidentally surfaced by things.

By naming the actual member to access the real value something scary, it should make it easy in code reviews to know when the real, unredacted version is being accessed so that such access can be carefully assessed.

@hanskokx
Copy link
Author

I wouldn't get too hung up on the fact that my example is a password or uses .toJson

The advice above isn't specific to passwords, which is why I suggested a generic class.

The methods you describe all just call toString() on anything they are given, so a simple wrapper type which has a custom toString() which prints nothing is a very simple and reasonable way to prevent this information from being accidentally surfaced by things.

By naming the actual member to access the real value something scary, it should make it easy in code reviews to know when the real, unredacted version is being accessed so that such access can be carefully assessed.

That's a valid and fair point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Proposed language feature that solves one or more problems
Projects
None yet
Development

No branches or pull requests

3 participants