-
Notifications
You must be signed in to change notification settings - Fork 693
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
Publish a new package, loguru-hardened, with security focussed defaults #1136
Conversation
I don't know why 3.13 build fails, maybe try rebasing (a few libraries were updated since then)? |
if it's serialized be default, should we also remove level time from default format? |
c5f9775
to
92cd74c
Compare
I don't follow, could you give a code example of what you mean or explain further? |
from LOGURU_FORMAT = env(
"LOGURU_FORMAT",
str,
"<green>{time:YYYY-MM-DD HH:mm:ss.SSS}</green> | "
"<level>{level: <8}</level> | "
"<cyan>{name}</cyan>:<cyan>{function}</cyan>:<cyan>{line}</cyan> - <level>{message}</level>",
) to LOGURU_FORMAT = env("LOGURU_FORMAT", str, "{message}") |
@@ -40,6 +40,7 @@ Installation | |||
|
|||
pip install loguru | |||
|
|||
Or if you need more secure defaults, install ``loguru-hardened``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also mentioned loguru-hardened
in the README, but I could not find a release action configured, so I'm not sure how to make sure hardened is also released when a new release is created.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, since there is no Github Action, I need to do it manually for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could open up another pr with release workflows if you would like that. I don't know what currently goes into the manual release actions, but it might be a nice setup to simplify maintenance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bneijt Thanks for the proposal. If you're familiar with release automation via Github Actions, your help is most welcome. I'd like to replace manual release management.
Also sorry for not having yet merged this PR, but it seems it will be incompatible with #1140. I need to check how to create loguru-hardened
without setup.py
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just use a Python script change project.name in new pyproject.toml file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i can help write a ci file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@trim21 Yes, I suppose this should be straightforward. Unfortunately, this PR will need to be updated, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider merge 1140 first maybe....
I finally fixed CI for Python 3.5 and 3.13, can you try rebasing the PR please? |
I have a question about this, what if a package I'm using require
a possible solution here would be, instead of new package, add a extra,this extra require a new package Then in loguru, we just check if package so instead of install try:
import loguru._hardened
HARDENED_ENABLED = True
except ImportError:
HARDENED_ENABLED = False |
Yes, there will be a conflict which needs to be solved by uninstalling or ignoring the loguru package in environments that require you to use a hardened version instead. The proposed workaround of using an extra won’t work because the reason for having this package is to facilitate environments where static code analysis like sonarqube flag loguru for being difficult to use in a secure way. Which means that we can’t have loguru installed. Next week I will find some time to create a hood example of how to use loguru hardened in those situations and explain how to deal with oackages that require loguru. |
I don't think this is a solution... in some case you don't have control which package will be installed first, loguru or loguru-hardened, which mean it's possible loguru override loguru-hardened, not what we are expecting, loguru-hardened over loguru.
That's false positive problem of static code analysi themselve, right? |
Yes and no.. it is not a false positive that “the default don’t protect against line injection properly”, but you are right that that is a bad mark in the first place because that same reasoning should flag Python itself (logging has the same issue) ;) |
So with extra, the default has been changed, right? or no matter how loguru changed, it will just flag loguru anyway? |
We switched to an open telemetry implementation, so I won't be able to move forward on migrating projects to loguru at this point and I also won't be able to do the Sonar Qube validation after potential release. Apart from that, even if we create a hardened version, any package picking up the normal loguru will still result in the total environment being flagged. I still think the project should consider adding continuation characters as a solution to allowing newlines while still making them visible if injected by a third party, but as I won't be able to test this I can't see me proceeding on this. Closing the issue and PR. |
Introduce a separate script that will override a few files to harden loguru defaults and build a loguru-hardened package, then revert the changes using git.