Adjust dangerous command detection for docker exec #6

2025-10-28T22:17:46+01:00

quentinreytinas commented

2025-10-28 22:17:46 +01:00

(Migrated from github.com)

Summary

remove the plain sh interpreter from the dangerous command list so .sh scripts stay allowed
compare dangerous commands against individual docker exec arguments instead of substrings

Testing

pytest

https://chatgpt.com/codex/tasks/task_e_69013228a6008332a17492cc7c2cab43

## Summary - remove the plain `sh` interpreter from the dangerous command list so `.sh` scripts stay allowed - compare dangerous commands against individual docker exec arguments instead of substrings ## Testing - pytest ------ https://chatgpt.com/codex/tasks/task_e_69013228a6008332a17492cc7c2cab43

chatgpt-codex-connector[bot] (Migrated from github.com) reviewed

2025-10-28 22:20:08 +01:00

chatgpt-codex-connector[bot] (Migrated from github.com) left a comment

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

### 💡 Codex Review Here are some automated review suggestions for this pull request. <details> <summary>ℹ️ About Codex in GitHub</summary> [Your team has set up Codex to review pull requests in this repo](http://chatgpt.com/codex/settings/general). Reviews are triggered when you - Open a pull request for review - Mark a draft as ready - Comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". </details>

borgmatic_api_app/docker.py

					
				@ -78,1 +78,4 @@

				        for dangerous in settings.dangerous_commands

				        if any(dangerous == part.lower() for part in command)

				    ]

				    if blocked:

Shell detection no longer blocks absolute paths

Changing the dangerous command check to dangerous == part.lower() means only arguments that exactly match the name are rejected. Together with the no_shell loop above that also checks only for exact matches, a command like /bin/bash -c … or /bin/sh now passes validation even when ExecWhitelistEntry.no_shell is True and bash/sh are listed as dangerous. Before this change the substring check blocked absolute paths; after the change the restriction can be bypassed simply by providing the full path to the shell binary. This lets a caller spawn an interactive shell in containers that were meant to forbid shells.

Useful? React with 👍 / 👎.

**![P1 Badge](https://img.shields.io/badge/P1-orange?style=flat) Shell detection no longer blocks absolute paths** Changing the dangerous command check to `dangerous == part.lower()` means only arguments that exactly match the name are rejected. Together with the `no_shell` loop above that also checks only for exact matches, a command like `/bin/bash -c …` or `/bin/sh` now passes validation even when `ExecWhitelistEntry.no_shell` is `True` and `bash`/`sh` are listed as dangerous. Before this change the substring check blocked absolute paths; after the change the restriction can be bypassed simply by providing the full path to the shell binary. This lets a caller spawn an interactive shell in containers that were meant to forbid shells. Useful? React with 👍 / 👎.

Sign in to join this conversation.

No reviewers