Logging security

[enyst]

Logging security:

• override str magic methods

• override repr at least for dataclasses, can add LLM

• mask sensitive info in loggers - cherry-picked from feat: api_key desensitize #1940

• unit tests attempting to predict the future...
  Fixes Conceal API key when serializing config #1936

@Shimada666 I appreciate a review of this part, if you can take a look!
The most important is the masking IMHO, but there's nothing wrong with a "belt and suspenders" approach, is there? 😅

[Shimada666]

@enyst
Looks great! Please proceed with your approach, as it can handle all sensitive information. I will close my pull request.😉

[enyst]

Ah, we could have done it parallel, but then, I stole your commits here. I'm no regex wizard, but we start with it, and let's think! 😅

@Shimada666 What do you think if we try matching "api_key='" to "'", ignoring the differences inside the key? Because it looks like for many providers they... can be different.

[Shimada666]

@enyst
Yes, we can do that!
I actually prefer the approach of using logger filters because of its good compatibility. We can use regular expressions to replace all sensitive data within the filter.
Using __str__ and __repr__ has a drawback in that we have to rewrite these methods for every class that references sensitive data, which can lead to oversights and is not as elegant.
Of course, there are probably some aspects I haven't considered. These are just my personal silly thoughts, so please consider them!😉

[enyst]

Yes, I agree. Although I hope we aren't going to spread around api keys in too many places! 😅

I think this is ready for review for now...

[li-boxuan]

love it!

[neubig]

This looks great to me!

One auxiliary note, we can probably actually remove github_token from the AppConfig, as it's not used anymore (we deprecated that in favor of passing in tokens through SANDBOX_ENV_GET_TOKEN). We could do that in a future PR.

[tobitege]

A follow-up on this topic:

If a test runs into an error (in this sample an integration error run from inside WSL command line like:
TEST_ONLY=true ./tests/integration/regenerate.sh

Then whatever package does the output, it can lead to a leak of the API KEY (I redacted it here):

/home/tobias/.cache/pypoetry/virtualenvs/opendevin-oz9p9M-K-py3.11/lib/python3.11/site-packages/tenacity/__init__.py:470: in __call__
    result = fn(*args, **kwargs)
opendevin/llm/llm.py:197: in wrapper
    resp = completion_unwrapped(*args, **kwargs)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

test_name = 'test_edits', args = ()
kwargs = {'api_key': 'sk-xxxxx', 'api_version': None, 'base_url': 'https://openrouter.ai/api/v1', 'custom_llm_provider': None, ...}
messages = [{'content': 'A chat between a curious user and an artificial intelligence assistant. The assistant gives helpful, det...|No more typos!\r\n4|Enjoy!\r\n\n\nENVIRONMENT REMINDER: You have 8 turns left to complete the task.', 'role': 'user'}]
message_str = '\n\n----------\n\nA chat between a curious user and an artificial intelligence assistant. The assistant gives helpful...\r\n2|Really?\r\n3|No more typos!\r\n4|Enjoy!\r\n\n\nENVIRONMENT REMINDER: You have 8 turns left to complete the task.'
message = {'content': 'OBSERVATION:\n[File: /workspace/bad.txt (4 lines total)]\r\n1|This is a simple typo.\r\n2|Really?\r\n3|No more typos!\r\n4|Enjoy!\r\n\n\nENVIRONMENT REMINDER: You have 8 turns left to complete the task.', 'role': 'user'}
mock_response = None

It's an edge case, imo. But maybe someone else finds this worthwhile to further track down.

[tobitege]

Here ssh_password and jwt_secret are missing.

[enyst]

They're likely newer than this PR. Thanks for hunting them down!