Feature/docker security improvements#25
Merged
Merged
Conversation
added 5 commits
May 29, 2026 17:55
…ivileges. Enable write access to the /tmp folder.
…transfer the user change to the bot container
Contributor
Руководство для рецензентаУсиливает безопасность контейнеров и изоляцию ресурсов в docker-compose, упрощает логирование до вывода только в консоль и удаляет устаревший TODO, поскольку теперь реализована изоляция на уровне Docker; также подразумеваются дополнительные изменения в Docker‑файлах, не показанные явно. Изменения по файлам
Подсказки и командыВзаимодействие с Sourcery
Настройка работы сервисаПерейдите в свою панель управления, чтобы:
Получение помощи
Original review guide in EnglishReviewer's GuideTightens container security and resource isolation in docker-compose, simplifies logging to console-only, and removes an obsolete TODO now that Docker-level isolation is in place, with additional Docker-related file changes implied but not shown. 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.
Привет — я нашёл 3 проблемы и оставил несколько общих комментариев:
- Удаление
FileHandlerизlogger.pyпереводит логирование в режим только консольного вывода; если в продакшене по‑прежнему нужны постоянные логи, имеет смысл направлять их в файл на примонтированный том или в централизованную систему логирования, а не полностью отказываться от файлового логирования. - Пометка
api_db_netкакinternal: trueи разделение сетей наapi_db_net/api_bot_netусиливает изоляцию сервисов; перепроверьте, что любые внешние инструменты (например, миграции, админ‑клиенты), которым нужен доступ к Postgres или API, либо подключены к этим сетям, либо корректно выставлены наружу, иначе можно случайно потерять коннект.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Удаление `FileHandler` из `logger.py` переводит логирование в режим только консольного вывода; если в продакшене по‑прежнему нужны постоянные логи, имеет смысл направлять их в файл на примонтированный том или в централизованную систему логирования, а не полностью отказываться от файлового логирования.
- Пометка `api_db_net` как `internal: true` и разделение сетей на `api_db_net` / `api_bot_net` усиливает изоляцию сервисов; перепроверьте, что любые внешние инструменты (например, миграции, админ‑клиенты), которым нужен доступ к Postgres или API, либо подключены к этим сетям, либо корректно выставлены наружу, иначе можно случайно потерять коннект.
## Individual Comments
### Comment 1
<location path="docker-compose.yml" line_range="94" />
<code_context>
\ No newline at end of file
+ internal: true
+
+ api_bot_net:
+ driver: bridge
\ No newline at end of file
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Рассмотрите возможность отметить `api_bot_net` как `internal: true`, если эта сеть используется только для внутреннего взаимодействия API и бота.
Поскольку `api_db_net` уже помечена как `internal` для изоляции, применение того же флага к `api_bot_net` (если она не должна быть доступна извне) сделает конфигурацию более последовательной и ещё сильнее ограничит непреднамеренное раскрытие сервисов наружу.
</issue_to_address>
### Comment 2
<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>
**suggestion:** Вместо полного удаления файлового логирования лучше сделать его настраиваемым.
Удаление `FileHandler` хорошо подходит для контейнеризованных развёртываний, которые используют stdout, но снижает гибкость для окружений, где файловые логи по‑прежнему полезны (например, bare metal или локальная разработка). Рассмотрите вариант спрятать `FileHandler` за переменной окружения или конфигурационным флагом, чтобы stdout оставался значением по умолчанию, а при необходимости можно было включить логирование в файл.
Suggested implementation:
```python
import os
console_handler = logging.StreamHandler() # вывод логов в консоль
console_handler.setFormatter(formatter)
logger.addHandler(console_handler)
# Optional file logging, controlled via environment variable
if os.getenv("ENABLE_FILE_LOGGING", "false").lower() in ("1", "true", "yes"):
log_file = os.getenv("LOG_FILE", "SteelTime.log")
file_handler = logging.FileHandler(filename=log_file, encoding="utf-8")
file_handler.setFormatter(formatter)
logger.addHandler(file_handler)
```
1. Если в `src/logger.py` модуль `os` уже импортируется в начале файла, удалите дублирующий `import os` из этого блока и оставьте один импорт.
2. Если `formatter` или `logger` не определены ранее в `src/logger.py`, убедитесь, что они создаются до этого блока (например, `logger = logging.getLogger(__name__)` и соответствующее объявление форматтера).
3. Обновите конфигурацию деплоя или документацию, чтобы упомянуть:
- `ENABLE_FILE_LOGGING` (установить в `true`/`1`/`yes` для включения файловых логов),
- `LOG_FILE` (необязательный путь/имя файла логов).
</issue_to_address>
### Comment 3
<location path="docker-compose.yml" line_range="70-72" />
<code_context>
+ read_only: true
+ security_opt:
+ - no-new-privileges:true
+ cap_drop: # Забирает все доступные привелегии у root пользователя.
+ - ALL
+ tmpfs: # Папко, доступная длоя записи инфорамции, ы случае надобности.
+ - /tmp
</code_context>
<issue_to_address>
**nitpick (typo):** Исправьте небольшие опечатки в русскоязычных комментариях, чтобы конфигурация выглядела понятно и профессионально.
В этих комментариях есть несколько орфографических ошибок (например, `привелегии` → `привилегии`, `Папко` → `Папка`, `длоя` → `для`, `инфорамции` → `информации`). Пожалуйста, исправьте их, чтобы комментарии к конфигурации оставались понятными для других.
Suggested implementation:
```
cap_drop: # Забирает все доступные привилегии у root пользователя.
- ALL
```
```
tmpfs: # Папка, доступная для записи информации, в случае необходимости.
- /tmp
```
Если в этом файле есть другие русские комментарии с похожими опечатками, примените тот же подход:
1. Исправьте орфографию ("привелегии" → "привилегии", "Папко" → "Папка", "длоя" → "для", "инфорамции" → "информации", "ы случае" → "в случае").
2. Сохраните исходный смысл и тон каждого комментария, исправляя только орфографию и лёгкие грамматические ошибки.
</issue_to_address>Sourcery бесплатно для open source — если вам нравятся наши ревью, пожалуйста, поделитесь ими ✨
Original comment in English
Hey - I've found 3 issues, and left some high level feedback:
- Removing the
FileHandlerfromlogger.pyswitches logging to console-only; if persistent logs are still needed in production, consider routing logs to a volume-mounted file or a centralized logging system instead of dropping file logging entirely. - Marking
api_db_netasinternal: trueand splitting networks intoapi_db_net/api_bot_netisolates services more strictly; double-check that any external tools (e.g., migrations, admin clients) that need to reach Postgres or the API are either attached to these networks or exposed appropriately, otherwise connectivity may be unintentionally broken.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Removing the `FileHandler` from `logger.py` switches logging to console-only; if persistent logs are still needed in production, consider routing logs to a volume-mounted file or a centralized logging system instead of dropping file logging entirely.
- Marking `api_db_net` as `internal: true` and splitting networks into `api_db_net` / `api_bot_net` isolates services more strictly; double-check that any external tools (e.g., migrations, admin clients) that need to reach Postgres or the API are either attached to these networks or exposed appropriately, otherwise connectivity may be unintentionally broken.
## Individual Comments
### Comment 1
<location path="docker-compose.yml" line_range="94" />
<code_context>
\ No newline at end of file
+ internal: true
+
+ api_bot_net:
+ driver: bridge
\ No newline at end of file
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Consider marking `api_bot_net` as `internal: true` if it’s only used for internal API–bot communication.
Since `api_db_net` is already internal for isolation, applying the same setting to `api_bot_net` (if it’s not meant to be externally reachable) would keep the configuration consistent and further limit unintended exposure.
</issue_to_address>
### Comment 2
<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>
**suggestion:** Consider making file logging configurable instead of removing it entirely.
Removing `FileHandler` works well for containerized deployments using stdout, but it removes flexibility for environments that still benefit from file logs (e.g., bare metal or local dev). Consider guarding the `FileHandler` behind an environment variable or config flag so stdout remains the default, with an option to enable file logging when needed.
Suggested implementation:
```python
import os
console_handler = logging.StreamHandler() # вывод логов в консоль
console_handler.setFormatter(formatter)
logger.addHandler(console_handler)
# Optional file logging, controlled via environment variable
if os.getenv("ENABLE_FILE_LOGGING", "false").lower() in ("1", "true", "yes"):
log_file = os.getenv("LOG_FILE", "SteelTime.log")
file_handler = logging.FileHandler(filename=log_file, encoding="utf-8")
file_handler.setFormatter(formatter)
logger.addHandler(file_handler)
```
1. If `src/logger.py` already imports `os` at the top of the file, remove the duplicate `import os` from this block and keep a single import.
2. If `formatter` or `logger` are not defined earlier in `src/logger.py`, ensure they are created before this block (e.g. `logger = logging.getLogger(__name__)` and the corresponding formatter definition).
3. Update any deployment or documentation to mention:
- `ENABLE_FILE_LOGGING` (set to `true`/`1`/`yes` to enable file logs),
- `LOG_FILE` (optional custom log file path/name).
</issue_to_address>
### Comment 3
<location path="docker-compose.yml" line_range="70-72" />
<code_context>
+ read_only: true
+ security_opt:
+ - no-new-privileges:true
+ cap_drop: # Забирает все доступные привелегии у root пользователя.
+ - ALL
+ tmpfs: # Папко, доступная длоя записи инфорамции, ы случае надобности.
+ - /tmp
</code_context>
<issue_to_address>
**nitpick (typo):** Fix minor typos in Russian comments to keep configuration clear and professional.
There are several spelling errors in these comments (e.g. `привелегии` → `привилегии`, `Папко` → `Папка`, `длоя` → `для`, `инфорамции` → `информации`). Please correct them to keep the configuration comments clear for others.
Suggested implementation:
```
cap_drop: # Забирает все доступные привилегии у root пользователя.
- ALL
```
```
tmpfs: # Папка, доступная для записи информации, в случае необходимости.
- /tmp
```
If there are any other Russian comments in this file with similar typos, apply the same approach:
1. Correct spelling ("привелегии" → "привилегии", "Папко" → "Папка", "длоя" → "для", "инфорамции" → "информации", "ы случае" → "в случае").
2. Keep the original meaning and tone of each comment intact while fixing only orthography and minor grammar.
</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 26657510954Coverage decreased (-0.2%) to 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.
Summary by Sourcery
Усилить настройки безопасности контейнеров и упростить конфигурацию логирования за счёт ограничения возможностей сервисов, изоляции сетей и отказа от файлового логирования.
Улучшения:
Original summary in English
Summary by Sourcery
Harden container security settings and simplify logging configuration by restricting service capabilities, isolating networks, and removing file-based logging.
Enhancements: