Revert "Feature/docker security improvements"#26
Closed
Fl1riX wants to merge 2 commits into
Closed
Conversation
This reverts commit e578423.
Contributor
Руководство для ревьюера (свернуто для небольших PR)Руководство для ревьюераЭтот PR отменяет большую часть ранее добавленных мер по усилению безопасности Docker (сети, возможности, файловая система только для чтения, опции безопасности) и добавляет базовое логирование в файл, а также TODO‑комментарий, связанный с изоляцией Docker в auth‑флоу. Изменения на уровне файлов
Подсказки и командыВзаимодействие с Sourcery
Настройка работыЗайдите в свою панель управления, чтобы:
Получение помощи
Original review guide in EnglishReviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR reverts most of the previously introduced Docker security hardening (networks, capabilities, read-only FS, security options) and adds basic file-based logging, plus a TODO comment related to Docker isolation in the auth flow. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Привет — я нашёл 1 проблему и оставил несколько общих замечаний:
- Запись логов в фиксированный файл
SteelTime.logв файловой системе контейнера может привести к неконтролируемому росту диска или проблемам с правами; рассмотрите вывод логов в stdout/stderr (для Docker‑логирования) или в примонтированный том с ротацией. - Откат удаляет все
security_opt,read_only,cap_dropи выделенные сети; если проблемы вызывали только конкретные настройки, подумайте о том, чтобы вернуть минимально необходимый поднабор (например,no-new-privilegesили сброс некоторых capability), вместо полного отключения всех механизмов повышения безопасности.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Writing logs to a fixed `SteelTime.log` in the container filesystem may cause disk growth or permission issues; consider directing logs to stdout/stderr (for Docker logging) or to a mounted volume with rotation.
- The revert removes all `security_opt`, `read_only`, `cap_drop`, and dedicated networks; if only specific settings caused issues, consider reintroducing a minimal subset (e.g., `no-new-privileges` or capability drops) instead of disabling all hardening.
## Individual Comments
### Comment 1
<location path="src/logger.py" line_range="12-15" />
<code_context>
console_handler = logging.StreamHandler() # вывод логов в консоль
console_handler.setFormatter(formatter)
+handler = logging.FileHandler(filename="SteelTime.log", encoding="utf-8") # файл для записи логов
+handler.setFormatter(formatter)
+
+logger.addHandler(handler)
logger.addHandler(console_handler)
\ No newline at end of file
</code_context>
<issue_to_address>
**issue (bug_risk):** Plain FileHandler risks unbounded log file growth and path issues
Using `FileHandler` with a fixed filename will grow the log file indefinitely in the container’s working directory, which can fill the filesystem in Docker. Prefer `RotatingFileHandler`/`TimedRotatingFileHandler` with size/backup limits or logging to stdout for Docker to manage. If you must write to a file, make the log path configurable (e.g. via environment variable).
</issue_to_address>Для open source Sourcery бесплатен — если вам нравятся наши ревью, пожалуйста, расскажите о них ✨
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- Writing logs to a fixed
SteelTime.login the container filesystem may cause disk growth or permission issues; consider directing logs to stdout/stderr (for Docker logging) or to a mounted volume with rotation. - The revert removes all
security_opt,read_only,cap_drop, and dedicated networks; if only specific settings caused issues, consider reintroducing a minimal subset (e.g.,no-new-privilegesor capability drops) instead of disabling all hardening.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Writing logs to a fixed `SteelTime.log` in the container filesystem may cause disk growth or permission issues; consider directing logs to stdout/stderr (for Docker logging) or to a mounted volume with rotation.
- The revert removes all `security_opt`, `read_only`, `cap_drop`, and dedicated networks; if only specific settings caused issues, consider reintroducing a minimal subset (e.g., `no-new-privileges` or capability drops) instead of disabling all hardening.
## Individual Comments
### Comment 1
<location path="src/logger.py" line_range="12-15" />
<code_context>
console_handler = logging.StreamHandler() # вывод логов в консоль
console_handler.setFormatter(formatter)
+handler = logging.FileHandler(filename="SteelTime.log", encoding="utf-8") # файл для записи логов
+handler.setFormatter(formatter)
+
+logger.addHandler(handler)
logger.addHandler(console_handler)
\ No newline at end of file
</code_context>
<issue_to_address>
**issue (bug_risk):** Plain FileHandler risks unbounded log file growth and path issues
Using `FileHandler` with a fixed filename will grow the log file indefinitely in the container’s working directory, which can fill the filesystem in Docker. Prefer `RotatingFileHandler`/`TimedRotatingFileHandler` with size/backup limits or logging to stdout for Docker to manage. If you must write to a file, make the log path configurable (e.g. via environment variable).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Coverage Report for CI Build 26684644872Coverage remained the same at 27.887%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions2 previously-covered lines in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reverts #25
Summary by Sourcery
Отменить недавние изменения по усилению безопасности Docker и логированию, добавив при этом базовое логирование в файл.
Новые возможности:
SteelTime.logвместе с выводом в консоль.Улучшения:
Обслуживание:
Original summary in English
Summary by Sourcery
Revert recent Docker security hardening and logging changes while adding basic file logging.
New Features:
Enhancements:
Chores: