Report missing timeout DoS vulnerability in container execution#188
Report missing timeout DoS vulnerability in container execution#188Vaiditya2207 wants to merge 1 commit intomainfrom
Conversation
Updates SECURITY_ISSUE.md to report a CRITICAL DoS vulnerability caused by an unbounded wait on `docker.wait_container` in `syscore/src/docker/manager.rs`. Also records the systemic cause in the auditor journal `.jules/sentinel.md`.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
documentation
📝 WalkthroughWalkthroughThe PR updates documentation files to describe a Docker execution timeout vulnerability. The sentinel file receives a dated entry documenting the issue and proper line ending, while SECURITY_ISSUE.md is updated to replace a file write vulnerability description with a DoS vulnerability description related to unbounded container wait operations. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@SECURITY_ISSUE.md`:
- Around line 27-49: The timeout handling needs to both stop and kill the
container when the wait_future (self.docker.wait_container::<String>(&id,
None).next()) exceeds the timeout_duration used with tokio::time::timeout;
update the Err(_) branch to first attempt self.docker.stop_container(&id,
None).await, then call self.docker.kill_container(&id, None).await to ensure
unresponsive containers are terminated, log both results (stop and kill), and
return an early error (e.g., Err("Execution timed out".to_string())) so callers
know the container was forcibly terminated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d5722bfb-e6e6-4749-95a1-8d584c350903
📒 Files selected for processing (2)
.jules/sentinel.mdSECURITY_ISSUE.md
| Implement a strict execution timeout on the `wait_container` future. If the container execution exceeds the timeout threshold (e.g., 5-10 seconds), explicitly stop and kill the container, and return a "Timeout exceeded" error. | ||
|
|
||
| Example fix: | ||
| Example fix using `tokio::time::timeout`: | ||
| ```rust | ||
| let safe_filename = std::path::Path::new(&filename) | ||
| .file_name() | ||
| .and_then(|name| name.to_str()) | ||
| .ok_or((StatusCode::BAD_REQUEST, "Invalid filename".to_string()))?; | ||
|
|
||
| let file_path = version_dir.join(safe_filename); | ||
| use std::time::Duration; | ||
|
|
||
| let timeout_duration = Duration::from_secs(5); | ||
| let wait_future = self.docker.wait_container::<String>(&id, None).next(); | ||
|
|
||
| match tokio::time::timeout(timeout_duration, wait_future).await { | ||
| Ok(Some(Ok(res))) => { | ||
| tracing::debug!("[Job {}] Container exited with code {}", job_id, res.status_code); | ||
| } | ||
| Ok(_) => { | ||
| tracing::warn!("[Job {}] Wait failed or container crashed", job_id); | ||
| } | ||
| Err(_) => { | ||
| tracing::warn!("[Job {}] Execution timed out. Killing container.", job_id); | ||
| let _ = self.docker.stop_container(&id, None).await; | ||
| // Return an early error or handle the timeout condition | ||
| return Err("Execution timed out".to_string()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate manager.rs =="
fd manager.rs
echo
echo "== Inspect wait_container call site and surrounding logic =="
rg -n -C4 'wait_container|/api/execute|next\(\)\.await|timeout\(' syscore/src/docker/manager.rs
echo
echo "== Inspect docker termination APIs used in repository =="
rg -n -C2 'stop_container|kill_container|remove_container' syscore/src/docker/manager.rs
rg -n -C2 'stop_container|kill_container|remove_container' --type rustRepository: Vaiditya2207/OKernel
Length of output: 2090
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check Docker client type and imports in manager.rs =="
head -40 syscore/src/docker/manager.rs | grep -E 'use|Docker|bollard'
echo
echo "== Full imports section of manager.rs =="
rg '^use ' syscore/src/docker/manager.rs | head -20
echo
echo "== Check if stop_container or kill_container are used anywhere in the repo =="
rg 'stop_container|kill_container' --type rust
echo
echo "== Check Docker struct definition and available methods =="
rg 'struct.*Docker|impl.*Docker' syscore/src/docker/manager.rsRepository: Vaiditya2207/OKernel
Length of output: 908
🌐 Web query:
bollard rust crate docker stop_container kill_container API methods
💡 Result:
The Bollard Rust crate provides asynchronous bindings to the Docker Engine API, including methods for managing Docker containers. Key API methods on the Docker struct for stopping and killing containers: - stop_container(&self, container_name: &str, options: Option) -> impl Future<Output = Result<, Error>> - Gracefully stops a container. The StopContainerOptions struct has a field t: i64 specifying seconds to wait before killing if it doesn't stop (default 30). - kill_container(&self, container_name: &str, options: Option<KillContainerOptions>) -> impl Future<Output = Result<, Error>> where T: Into + Serialize - Force-kills a container by sending a signal. KillContainerOptions has signal: Option (e.g., "SIGKILL"). Usage example: use bollard::{Docker, container::{StopContainerOptions, KillContainerOptions}}; let docker = Docker::connect_with_local_defaults.unwrap; docker.stop_container("my_container", Some(StopContainerOptions { t: 10 })).await?; docker.kill_container("my_container", Some(KillContainerOptions { signal: "SIGKILL".to_string })).await?; These methods correspond to Docker CLI docker stop and docker kill. Source docs confirm presence in container module and Docker impl (latest v0.20.1 as of 2026-02-01, Docker API v1.52).
Citations:
- 1: https://docs.rs/bollard/latest/bollard/container/index.html
- 2: https://docs.rs/bollard/latest/bollard
- 3: https://docs.rs/bollard/latest/bollard/struct.Docker.html
- 4: https://wiki.enablingpersonalizedinterventions.nl/docs/bollard/container/struct.StopContainerOptions.html
- 5: https://docs.rs/bollard/0.11.0/bollard/struct.Docker.html
- 6: https://crates.io/crates/bollard
- 7: https://github.com/fussybeaver/bollard
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Read SECURITY_ISSUE.md lines 27-49 =="
sed -n '27,49p' SECURITY_ISSUE.md
echo
echo "== Check full context of wait_container implementation =="
sed -n '220,240p' syscore/src/docker/manager.rsRepository: Vaiditya2207/OKernel
Length of output: 1993
Remediation text and code sample are inconsistent on container termination.
The documentation requires "explicitly stop and kill the container" but the code snippet only calls stop_container. For unresponsive containers, this leaves the container running and the DoS condition unresolved. Add kill_container after the stop attempt to match the stated intent.
Suggested doc fix
Err(_) => {
tracing::warn!("[Job {}] Execution timed out. Killing container.", job_id);
let _ = self.docker.stop_container(&id, None).await;
+ let _ = self.docker.kill_container(&id, None).await;
// Return an early error or handle the timeout condition
return Err("Execution timed out".to_string());
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Implement a strict execution timeout on the `wait_container` future. If the container execution exceeds the timeout threshold (e.g., 5-10 seconds), explicitly stop and kill the container, and return a "Timeout exceeded" error. | |
| Example fix: | |
| Example fix using `tokio::time::timeout`: | |
| ```rust | |
| let safe_filename = std::path::Path::new(&filename) | |
| .file_name() | |
| .and_then(|name| name.to_str()) | |
| .ok_or((StatusCode::BAD_REQUEST, "Invalid filename".to_string()))?; | |
| let file_path = version_dir.join(safe_filename); | |
| use std::time::Duration; | |
| let timeout_duration = Duration::from_secs(5); | |
| let wait_future = self.docker.wait_container::<String>(&id, None).next(); | |
| match tokio::time::timeout(timeout_duration, wait_future).await { | |
| Ok(Some(Ok(res))) => { | |
| tracing::debug!("[Job {}] Container exited with code {}", job_id, res.status_code); | |
| } | |
| Ok(_) => { | |
| tracing::warn!("[Job {}] Wait failed or container crashed", job_id); | |
| } | |
| Err(_) => { | |
| tracing::warn!("[Job {}] Execution timed out. Killing container.", job_id); | |
| let _ = self.docker.stop_container(&id, None).await; | |
| // Return an early error or handle the timeout condition | |
| return Err("Execution timed out".to_string()); | |
| } | |
| } | |
| Err(_) => { | |
| tracing::warn!("[Job {}] Execution timed out. Killing container.", job_id); | |
| let _ = self.docker.stop_container(&id, None).await; | |
| let _ = self.docker.kill_container(&id, None).await; | |
| // Return an early error or handle the timeout condition | |
| return Err("Execution timed out".to_string()); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@SECURITY_ISSUE.md` around lines 27 - 49, The timeout handling needs to both
stop and kill the container when the wait_future
(self.docker.wait_container::<String>(&id, None).next()) exceeds the
timeout_duration used with tokio::time::timeout; update the Err(_) branch to
first attempt self.docker.stop_container(&id, None).await, then call
self.docker.kill_container(&id, None).await to ensure unresponsive containers
are terminated, log both results (stop and kill), and return an early error
(e.g., Err("Execution timed out".to_string())) so callers know the container was
forcibly terminated.
1 participant
I have updated
SECURITY_ISSUE.mdto report a CRITICAL Denial of Service vulnerability insyscore/src/docker/manager.rswhere Docker container execution is awaited without a timeout (docker.wait_container). This allows an attacker to exhaust server resources by submitting non-terminating code payloads (e.g., infinite loops).I also logged the learning in
.jules/sentinel.mdas required. No source code was modified, keeping to the Sentinel guidelines. The Rust backend tests passed successfully to ensure no regressions were introduced.PR created automatically by Jules for task 11015010953051891038 started by @Vaiditya2207
Summary by CodeRabbit
Documentation