Mirrors/wrkflw
1
0
Fork 0
mirror of https://github.com/bahdotsh/wrkflw.git synced 2026-05-18 05:05:35 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix: actually execute Docker-based GitHub Actions instead of emulating them (#74)

Browse Source 
* fix(executor): actually execute Docker-based GitHub Actions instead of emulating them

Third-party GitHub Actions that use Docker (like super-linter) were
silently passing without ever *actually running*. The engine would
resolve the action, pick a Docker image, and then... run
`echo 'Would execute GitHub action: ...'` inside it. Every single
time. Regardless of runtime mode. Confusion ensues.

It turns out there were two separate failures conspiring here:

1. `prepare_action()` would error out on `ActionType::DockerBuild`
   with "not yet supported", fall back to `determine_action_image()`,
   and cheerfully return `node:20-slim` for super-linter. This is
   not great.

2. The `PreparedAction::Image` execution branch had three sub-paths
   for is_docker, is_local, and everything else — and *all three*
   just ran echo commands. The image was resolved correctly and then
   completely ignored.

The fix has several parts:

- Add a `NativeDocker` variant to `PreparedAction` that means "run
  this image with its built-in ENTRYPOINT, no command override."
  Docker registry actions and DockerBuild actions both use this.

- Implement DockerBuild properly: clone the repo, resolve the
  Dockerfile path from action.yml, build it, return the tag.
  Uses the existing `shallow_clone` and `runtime.build_image`.

- Fix `build_image_inner` to tar the *full context directory*
  instead of just the Dockerfile. The old code had `_context_dir`
  sitting right there, computed and unused. COPY instructions in
  Dockerfiles need the context, obviously.

- Allow empty `cmd` in `run_container` to mean "use the image's
  default ENTRYPOINT/CMD". The Docker impl now sets `config.cmd =
  None` when cmd is empty. Podman already handled this correctly.

The existing `PreparedAction::Image` path with all its special-cased
action handling (actions-rs, checkout, etc.) is completely untouched.

Closes #59

* fix(executor): fix macOS entrypoint hang, path traversal, and silent emulation pass

Three bugs in the Docker action execution path from the previous
commit:

1. The macOS emulation entrypoint override (`bash -l -c`) was applied
*unconditionally*, even when cmd was empty (NativeDocker path). That
means Docker actions running on macOS emu images would get bash with
no argument — which either hangs forever or exits immediately. The
image's real ENTRYPOINT gets discarded either way. This is not great.

Fix: capture `has_cmd` before cmd_vec is moved into the config, only
apply the bash wrapper when there's actually a command to wrap.

2. The `dockerfile_rel` extracted from action.yml's `runs.image` was
not sanitized after stripping the `docker://` prefix. A malicious
action.yml with `docker:///etc/shadow` or `../../sensitive` would
escape the action directory via Path::join's absolute-path behavior
or dotdot traversal.

Fix: strip leading slashes and reject any path containing `..`.

3. Emulation mode returned exit_code 0 for Docker actions it *didn't
actually run*. Users got a green checkmark for actions that were
silently skipped. Confusion ensues.

Fix: return exit_code 1 with a clear stderr message explaining the
action was not executed and needs --runtime docker.

While at it, add tests for all three fixes: NativeDocker variant
construction, dockerfile path sanitization (6 cases), and emulation
empty-cmd failure behavior.

* fix(executor): harden Docker action security and fix docker:// execution path

Three issues found during review, all in the Docker action plumbing:

1. The `is_docker` path in `prepare_action()` was returning
`PreparedAction::Image` instead of `NativeDocker`, which means
`docker://` prefixed actions in `uses:` went straight through the
legacy echo-command path and *never actually executed*. Same class
of bug we just fixed for DockerBuild, hiding in plain sight.

2. The path traversal check for Dockerfile paths used
`contains("..")`, which rejects perfectly legitimate directory
names like `foo..bar/`. Check for `..` as an actual path
*component* instead via `split('/').any(|c| c == "..")`.

3. `build_image_inner` was calling `append_dir_all` on untrusted
action repositories without disabling symlink following. A
malicious action repo could plant a symlink pointing at the host
filesystem and have its contents shipped into the Docker build
context. That's the kind of thing that makes security auditors
lose sleep. Set `follow_symlinks(false)` on the tar builder.

* fix(executor): wire up runs.entrypoint, runs.args, and fix local Docker dispatch

The previous commits got the NativeDocker path working for remote
actions, but left several holes that a code review correctly
identified. Let's fix them all.

First, local Docker actions (uses: ./my-action with a Dockerfile)
were *still* returning PreparedAction::Image instead of NativeDocker.
Same class of bug we just fixed for remote actions, hiding one
function call away. They now go through NativeDocker and parse the
local action.yml for entrypoint/args.

Second, runs.entrypoint and runs.args from action.yml were being
completely ignored. Docker actions that declare their entrypoint in
action.yml (which is, you know, *a lot of them*) would silently
use the wrong entrypoint. Add an entrypoint parameter to the
ContainerRuntime trait and thread it through all four implementations:
Docker sets Config.entrypoint, Podman passes --entrypoint, and the
emulation runtimes accept-and-ignore it.

Third, with.args from workflow steps (uses: docker://alpine with
args: "echo hello") was not being passed as container CMD. It now
overrides runs.args when present, matching GitHub Actions behavior.

While at it:
- Extract sanitize_dockerfile_rel into a real function instead of
  having the tests duplicate the logic and test their own copy.
  Testing a copy of your code instead of the actual code is not
  what I'd call confidence-inspiring.
- Add canonicalize() defense-in-depth after Dockerfile path
  resolution to catch symlink escapes.
- Document the build_image_inner context directory invariant.

* fix(executor): fix broken args parsing, empty dockerfile path, and silent entrypoint drop

Three correctness bugs found during review of the Docker action
execution path:

1. with.args was being split on whitespace like a caveman. An
   argument like "hello world" would turn into two separate args,
   which is *not* how GitHub Actions works. Use shlex::split() for
   proper shell-word parsing, with a whitespace fallback for
   malformed input that shlex chokes on.

2. sanitize_dockerfile_rel() happily accepted empty strings. Feed
   it "" or "docker://" and it would produce an empty path, which
   then joins to a directory instead of a file. The subsequent
   docker build would fail with a confusing error. Let's just
   reject empty paths upfront.

3. SecureEmulationRuntime silently swallowed the entrypoint
   override without telling anyone. If you're running in secure
   emulation mode and your action specifies runs.entrypoint, you
   deserve to know it's being ignored — not left wondering why
   your action isn't doing what you expect.

* fix(executor)!: pass explicit build context to Docker image builds

It turns out that build_image_inner was deriving the Docker build
context from dockerfile.parent(), which is *wrong* when the
Dockerfile lives in a subdirectory of the action root. An action
with runs.image: subdir/Dockerfile would get subdir/ as its build
context instead of the action root, silently breaking every COPY
instruction that references files outside that subdirectory.

The fix is straightforward: add an explicit context_dir parameter
to the ContainerRuntime::build_image trait so callers tell us what
the context is instead of us guessing from the Dockerfile path.
The DockerBuild path in engine.rs now passes &action_dir, and the
Docker inner implementation computes the Dockerfile path relative
to context_dir via strip_prefix instead of just using file_name().

While at it, add a warning log when shlex::split fails to parse
with.args (unmatched quotes). Previously this silently fell back
to naive whitespace splitting, which is the kind of thing that
makes you stare at container logs for an hour wondering why your
quoted argument got split into three pieces.

* fix(executor): reject bad dockerfile paths instead of silently guessing

Three bugs found during review:

The build_image_inner strip_prefix fallback was *silently* using just
the filename when the Dockerfile wasn't a clean descendant of the
context directory. So if something weird happened with the path,
you'd just get the wrong Dockerfile used for the build with zero
indication anything went wrong. That's not a fallback, that's a
footgun. Return an error instead.

sanitize_dockerfile_rel was happily preserving a leading "./" from
the raw path, which then caused strip_prefix to fail (because
"./build/Dockerfile" is not a prefix-match for a joined path).
Strip it early so the downstream path arithmetic actually works.

While at it, extract_docker_runs_config was using filter_map on
runs.args, which means non-string YAML values like integers and
booleans were silently dropped. GitHub Actions coerces those to
strings, so we should too.

* fix(executor): handle string-form args and reject malformed with.args

It turns out that extract_docker_runs_config only handled runs.args
as a YAML sequence. If an action.yml declared args as a plain string
(which GitHub Actions absolutely allows), we'd silently drop the
entire argument. Not great.

While at it, the with.args parser had the opposite problem — when
shlex::split hit an unmatched quote, it shrugged and fell back to
naive whitespace splitting. That's the kind of "graceful degradation"
that produces subtly wrong container invocations and makes you spend
an afternoon wondering why your action is getting the wrong flags.

Fix both: extract_docker_runs_config now handles args as either a
YAML sequence or a string (shell-tokenized via shlex). The with.args
path now returns a hard error on malformed quoting instead of
pretending everything is fine. Added tests for string-form args
including the bad-quoting edge case.

* fix(executor): close sub_path traversal hole and make args parsing consistent

It turns out that sub_path from action references (the part after
owner/repo in owner/repo/some/subdir) was being joined to the clone
directory with absolutely no sanitization. A crafted sub_path like
"../../etc" would escape the cloned repo and get passed as the
Docker *build context*. Please don't do that.

Add sanitize_sub_path() that rejects any path component equal to
"..", and apply it in both the DockerBuild and Composite action
paths. For DockerBuild, also canonicalize the resolved action_dir
and verify it's still inside the repo_dir — because symlinks exist
and trusting user-controlled paths is how we end up on HN.

While at it, fix a behavioral inconsistency in args parsing: bad
quoting in action.yml's runs.args was silently falling back to the
raw string, while the exact same bad quoting in a workflow's
with.args was a hard error. Now both are errors, because silently
doing the wrong thing is worse than loudly refusing.

* fix(executor): harden Docker build context, sanitize inputs, deduplicate mount setup

The PR review flagged several issues ranging from correctness to
performance to plain old code smell. Let's address them all.

It turns out that build_image_inner was happily tarring the *entire*
context directory and shipping it to the Docker daemon, cheerfully
ignoring any .dockerignore file. For large action repos with test
fixtures, docs, and who knows what else, this is not great. When a
.dockerignore exists, we now use the `ignore` crate's WalkBuilder to
walk only non-ignored files. Falls back to the old append_dir_all
when there's no .dockerignore, because we're not breaking anything
that already works.

The sanitize_sub_path and sanitize_dockerfile_rel functions checked
for ".." traversal but not null bytes, which can cause truncation at
OS boundaries and potentially bypass the traversal check. Please
don't do that. Added null byte rejection to both.

extract_docker_runs_config was taking &Option<T> instead of
Option<&T>, which is the Rust equivalent of wearing your shirt
inside out — it works, but everyone who sees it knows something is
wrong. Fixed the signature and all callers.

The with.args empty-string handling was also wrong: `with.args: ""`
was treated as "no override" instead of "pass zero args", which
doesn't match GitHub Actions behavior where the presence of the key
is the signal, not its value.

While at it, extracted the volume/env/mount setup boilerplate that
was copy-pasted across three execution paths into a
StepContainerContext helper. Not because I enjoy moving code around,
but because the same 12 lines in three places is not my idea of
maintainability.

* fix(executor): cap build context size, disable git hooks, add NativeDocker tests

Three security and reliability fixes from the PR review:

The build_image_inner tar buffer was completely unbounded — a
malicious or just absurdly large action repo with no .dockerignore
would happily try to load the entire thing into memory. Now we
track cumulative file sizes and bail at 500 MB. The old
append_dir_all fallback had to go since it gives us no per-file
hook; replaced it with an ignore::WalkBuilder walk (already a dep)
so both paths enforce the same limit.

shallow_clone was happily running git checkout on untrusted repos
without disabling hooks. A cloned repo's .git/hooks/post-checkout
runs automatically, which is the kind of thing that makes security
reviewers lose sleep. Pass -c core.hooksPath=/dev/null to every
git invocation so cloned repos can't execute anything on our host.

While at it, add a MockContainerRuntime and four integration tests
that exercise the NativeDocker execute_step path end-to-end:
entrypoint passthrough, with.args override + INPUT_* injection,
empty args, and step/job env propagation. This path previously had
zero test coverage for the runtime flow.

* fix(executor): deduplicate build context walker, harden sub_path, add missing tests

The build_image_inner code in docker.rs had two near-identical
~50-line walker loops — one for when .dockerignore exists and one
for when it doesn't. The *only* difference was a single
add_custom_ignore_filename() call on the builder. Copy-paste like
that drifts. Let's not.

Merged into a single loop with a conditional on the WalkBuilder
before iteration. Same behavior, half the code.

While at it, sanitize_sub_path now splits on both '/' and '\' so
a Windows-style traversal like "a\..\..\etc" doesn't sneak past
the check. Also expanded the PreparedAction::Image doc comment to
explain which code paths still produce it and why it's distinct
from NativeDocker — future contributors shouldn't have to guess.

Added tests for: unmatched-quote error path in with.args, with.args
overriding runs.args, and backslash path traversal in sub_path.

* fix(executor): close backslash traversal gap and add with.entrypoint override

It turns out that sanitize_dockerfile_rel was only splitting on '/'
to catch ".." traversal, while its sibling sanitize_sub_path was
correctly splitting on both '/' and '\\'. So a crafted Dockerfile
path like "..\\..\\etc\\shadow" would sail right past the
sanitizer.

The canonicalize() defense-in-depth below *would* catch this in
practice, but relying on one security layer to cover a hole in
another is not great. Let's just make them consistent.

While at it, the NativeDocker execution path was missing support
for with.entrypoint — a documented GitHub Actions feature that
lets workflow steps override the Docker container's ENTRYPOINT.
We were already handling with.args but silently ignoring
with.entrypoint, which is the kind of asymmetry that bites you
the moment someone actually tries to use it.

* fix(executor): close composite sub_path symlink hole and filter empty entrypoint

The DockerBuild handler had a proper canonicalize + starts_with
defense-in-depth check after resolving sub_path, but the composite
action handler just blindly trusted sanitize_sub_path() and called
repo_dir.join(p) without verifying the result stayed inside the
cloned repo. A symlink named "legit" pointing to "../../secrets"
would sail right through the string-only sanitizer.

That is not great.

Add the same canonicalize + starts_with check to the composite
action path so both handlers have identical protection.

While at it, filter empty-string entrypoint values to None in both
extract_docker_runs_config and the Docker runtime layer. An empty
runs.entrypoint in action.yml should mean "use the image default",
not "tell Docker to clear the entrypoint" — which is what passing
Some("") actually does. Added tests for both the with.entrypoint
override path and the empty entrypoint filtering.

* fix(executor): filter empty podman entrypoint and extract NativeDocker step handler

The podman runtime was happily passing `--entrypoint ""` to podman
when a workflow set `with.entrypoint: ""`, while Docker correctly
filtered it out via `.filter(|s| !s.is_empty())`. So the two
runtimes silently diverged on empty entrypoint behavior. Not great.

Add the same filter to podman's entrypoint handling so both
runtimes treat empty strings as "use the image default."

While at it, extract the ~90-line NativeDocker match arm from
execute_step into its own `execute_native_docker_step` function.
That match block was getting unwieldy, and this keeps each action
type's execution logic self-contained.

Also drop a TODO on the in-memory tar buffer in build_image_inner —
it holds the entire build context in a Vec<u8>, which gets
uncomfortable as repos approach the 500 MB cap.
This commit is contained in:
Gokul
2026-04-02 10:28:37 +05:30
committed by GitHub
parent 8a8d7e5eec
commit fd348a460e
11 changed files with 1448 additions and 104 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
Cargo.lock generated
View File

@@ -3385,6 +3385,7 @@ dependencies = [
"serde",
"serde_json",
"serde_yaml",
"shlex",
"tar",
"tempfile",
"thiserror",

1
Cargo.toml
View File

@@ -66,6 +66,7 @@ libc = "0.2"
nix = { version = "0.27.1", features = ["fs"] }
urlencoding = "2.1.3"
wiremock = "0.5"
shlex = "1.3"
[profile.release]
codegen-units = 1

1
crates/executor/Cargo.toml
View File

@@ -36,6 +36,7 @@ reqwest.workspace = true
serde.workspace = true
serde_json.workspace = true
serde_yaml.workspace = true
shlex.workspace = true
tar.workspace = true
tempfile.workspace = true
thiserror.workspace = true

179
crates/executor/src/docker.rs
View File

@@ -266,7 +266,8 @@ impl DockerRuntime {
// Build the customized image
let image_tag = format!("wrkflw-{}-{}", language, version.unwrap_or("latest"));
self.build_image(&dockerfile_path, &image_tag).await?;
self.build_image(&dockerfile_path, &image_tag, temp_dir.path())
.await?;
// Store the customized image
Self::set_language_specific_image("", language, version, &image_tag);
@@ -635,6 +636,7 @@ impl ContainerRuntime for DockerRuntime {
env_vars: &[(&str, &str)],
working_dir: &Path,
volumes: &[(&Path, &Path)],
entrypoint: Option<&str>,
) -> Result<ContainerOutput, ContainerError> {
// Print detailed debugging info
wrkflw_logging::info(&format!("Docker: Running container with image: {}", image));
@@ -645,7 +647,7 @@ impl ContainerRuntime for DockerRuntime {
// Run the entire container operation with a timeout
match tokio::time::timeout(
timeout_duration,
self.run_container_inner(image, cmd, env_vars, working_dir, volumes),
self.run_container_inner(image, cmd, env_vars, working_dir, volumes, entrypoint),
)
.await
{
@@ -676,11 +678,20 @@ impl ContainerRuntime for DockerRuntime {
}
}
async fn build_image(&self, dockerfile: &Path, tag: &str) -> Result<(), ContainerError> {
async fn build_image(
&self,
dockerfile: &Path,
tag: &str,
context_dir: &Path,
) -> Result<(), ContainerError> {
// Add a timeout for build operations
let timeout_duration = std::time::Duration::from_secs(120); // 2 minutes timeout for builds
match tokio::time::timeout(timeout_duration, self.build_image_inner(dockerfile, tag)).await
match tokio::time::timeout(
timeout_duration,
self.build_image_inner(dockerfile, tag, context_dir),
)
.await
{
Ok(result) => result,
Err(_) => {
@@ -821,7 +832,8 @@ impl ContainerRuntime for DockerRuntime {
// Build the customized image
let image_tag = format!("wrkflw-{}-{}", language, version.unwrap_or("latest"));
self.build_image(&dockerfile_path, &image_tag).await?;
self.build_image(&dockerfile_path, &image_tag, temp_dir.path())
.await?;
// Store the customized image
Self::set_language_specific_image("", language, version, &image_tag);
@@ -839,6 +851,7 @@ impl DockerRuntime {
env_vars: &[(&str, &str)],
working_dir: &Path,
volumes: &[(&Path, &Path)],
entrypoint: Option<&str>,
) -> Result<ContainerOutput, ContainerError> {
// First, try to pull the image if it's not available locally
if let Err(e) = self.pull_image_inner(image).await {
@@ -865,6 +878,7 @@ impl DockerRuntime {
// Convert command vector to Vec<String>
let cmd_vec: Vec<String> = cmd.iter().map(|&s| s.to_string()).collect();
let has_cmd = !cmd_vec.is_empty();
wrkflw_logging::debug(&format!("Running command in Docker: {:?}", cmd_vec));
wrkflw_logging::debug(&format!("Environment: {:?}", env));
@@ -915,7 +929,8 @@ impl DockerRuntime {
// Create container config with platform-specific settings
let mut config = Config {
image: Some(image.to_string()),
cmd: Some(cmd_vec),
// Empty cmd means "use the image's built-in ENTRYPOINT/CMD"
cmd: if has_cmd { Some(cmd_vec) } else { None },
env: Some(env),
working_dir: Some(working_dir.to_string_lossy().to_string()),
host_config: Some(host_config),
@@ -925,8 +940,14 @@ impl DockerRuntime {
} else {
None // Don't specify user for macOS emulation - use default root user
},
// Map appropriate entrypoint for different platforms
entrypoint: if is_macos_emu {
// Map appropriate entrypoint for different platforms.
// Priority: explicit entrypoint from action.yml > macOS bash wrapper > image default.
// Only apply the macOS bash wrapper when there is an explicit command to wrap;
// an empty cmd means "use the image's native ENTRYPOINT/CMD"
// and overriding it with bash would hang or discard the real entrypoint.
entrypoint: if let Some(ep) = entrypoint.filter(|s| !s.is_empty()) {
Some(vec![ep.to_string()])
} else if is_macos_emu && has_cmd {
// For macOS, ensure we use bash
Some(vec!["bash".to_string(), "-l".to_string(), "-c".to_string()])
} else {
@@ -1094,50 +1115,112 @@ impl DockerRuntime {
Ok(())
}
async fn build_image_inner(&self, dockerfile: &Path, tag: &str) -> Result<(), ContainerError> {
let _context_dir = dockerfile.parent().unwrap_or(Path::new("."));
/// Build a Docker image from a Dockerfile.
///
/// `context_dir` is the Docker build context directory. Files in this
/// directory are sent to the Docker daemon so that COPY instructions work.
/// The Dockerfile path is made relative to this context for the build API.
///
/// If a `.dockerignore` file exists in the context directory, its patterns
/// are honoured to exclude files from the build context — reducing the
/// amount of data sent to the daemon for large action repositories.
///
/// **Note:** `follow_symlinks(false)` protects against symlink-based
/// exfiltration but does not prevent hard-links to files outside the
/// context. This is acceptable because the context is always a freshly
/// cloned repo or tempdir that we control.
async fn build_image_inner(
&self,
dockerfile: &Path,
tag: &str,
context_dir: &Path,
) -> Result<(), ContainerError> {
if !dockerfile.exists() {
return Err(ContainerError::ImageBuild(format!(
"Cannot open Dockerfile at {}",
dockerfile.display()
)));
}
// Determine the Dockerfile path relative to the context directory.
let dockerfile_name = dockerfile
.strip_prefix(context_dir)
.map(|p| p.to_string_lossy().to_string())
.map_err(|_| {
ContainerError::ImageBuild(format!(
"Dockerfile {} is not within context directory {}",
dockerfile.display(),
context_dir.display()
))
})?;
// Maximum build context size (500 MB). Prevents OOM when an action
// repo is unexpectedly large and has no .dockerignore.
const MAX_CONTEXT_BYTES: u64 = 500 * 1024 * 1024;
// TODO: For large build contexts (approaching MAX_CONTEXT_BYTES), consider
// streaming the tar to a temporary file instead of holding it all in memory.
let tar_buffer = {
let mut tar_builder = tar::Builder::new(Vec::new());
// Add Dockerfile to tar
if let Ok(file) = std::fs::File::open(dockerfile) {
let mut header = tar::Header::new_gnu();
let metadata = file.metadata().map_err(|e| {
ContainerError::ContainerExecution(format!(
"Failed to get file metadata: {}",
e
))
})?;
let modified_time = metadata
.modified()
.map_err(|e| {
ContainerError::ContainerExecution(format!(
"Failed to get file modification time: {}",
e
))
})?
.duration_since(std::time::UNIX_EPOCH)
.map_err(|e| {
ContainerError::ContainerExecution(format!(
"Failed to convert modification time to Unix timestamp: {}",
e
))
})?
.as_secs();
header.set_size(metadata.len());
header.set_mode(0o644);
header.set_mtime(modified_time);
header.set_cksum();
// Do not follow symlinks — untrusted action repos could use symlinks
// to leak host filesystem contents into the Docker build context.
tar_builder.follow_symlinks(false);
tar_builder
.append_data(&mut header, "Dockerfile", file)
.map_err(|e| ContainerError::ImageBuild(e.to_string()))?;
} else {
return Err(ContainerError::ImageBuild(format!(
"Cannot open Dockerfile at {}",
dockerfile.display()
)));
// Track cumulative size so we can enforce MAX_CONTEXT_BYTES.
let mut total_bytes: u64 = 0;
// Build a context walker. When a .dockerignore exists, its patterns
// are used to exclude files (same glob semantics as Docker).
use ignore::WalkBuilder;
let dockerignore_path = context_dir.join(".dockerignore");
let mut walker_builder = WalkBuilder::new(context_dir);
// Disable default .gitignore / .ignore handling — we only want
// .dockerignore semantics when the file is present.
walker_builder.standard_filters(false).follow_links(false);
if dockerignore_path.exists() {
walker_builder.add_custom_ignore_filename(".dockerignore");
}
for entry in walker_builder.build() {
let entry = entry.map_err(|e| {
ContainerError::ImageBuild(format!("Failed to walk build context: {}", e))
})?;
let path = entry.path();
let rel = match path.strip_prefix(context_dir) {
Ok(r) => r,
Err(_) => continue,
};
// Skip the root directory itself — tar implicitly contains it.
if rel.as_os_str().is_empty() {
continue;
}
if path.is_dir() {
tar_builder.append_dir(rel, path).map_err(|e| {
ContainerError::ImageBuild(format!(
"Failed to add directory to build context tar: {}",
e
))
})?;
} else {
if let Ok(meta) = path.metadata() {
total_bytes += meta.len();
if total_bytes > MAX_CONTEXT_BYTES {
return Err(ContainerError::ImageBuild(format!(
"Docker build context exceeds {} MB limit. \
Add a .dockerignore file to exclude unnecessary files.",
MAX_CONTEXT_BYTES / (1024 * 1024)
)));
}
}
tar_builder.append_path_with_name(path, rel).map_err(|e| {
ContainerError::ImageBuild(format!(
"Failed to add file to build context tar: {}",
e
))
})?;
}
}
tar_builder
@@ -1146,7 +1229,7 @@ impl DockerRuntime {
};
let options = bollard::image::BuildImageOptions {
dockerfile: "Dockerfile",
dockerfile: dockerfile_name.as_str(),
t: tag,
q: false,
nocache: false,

1
crates/executor/src/docker_test.rs
View File

@@ -37,6 +37,7 @@ mod docker_cleanup_tests {
&[],
Path::new("/"),
&[],
None,
)
.await;

1246
crates/executor/src/engine.rs
View File

File diff suppressed because it is too large Load Diff

39
crates/executor/src/podman.rs
View File

@@ -470,6 +470,7 @@ impl ContainerRuntime for PodmanRuntime {
env_vars: &[(&str, &str)],
working_dir: &Path,
volumes: &[(&Path, &Path)],
entrypoint: Option<&str>,
) -> Result<ContainerOutput, ContainerError> {
// Print detailed debugging info
wrkflw_logging::info(&format!("Podman: Running container with image: {}", image));
@@ -479,7 +480,7 @@ impl ContainerRuntime for PodmanRuntime {
// Run the entire container operation with a timeout
match tokio::time::timeout(
timeout_duration,
self.run_container_inner(image, cmd, env_vars, working_dir, volumes),
self.run_container_inner(image, cmd, env_vars, working_dir, volumes, entrypoint),
)
.await
{
@@ -510,11 +511,20 @@ impl ContainerRuntime for PodmanRuntime {
}
}
async fn build_image(&self, dockerfile: &Path, tag: &str) -> Result<(), ContainerError> {
async fn build_image(
&self,
dockerfile: &Path,
tag: &str,
context_dir: &Path,
) -> Result<(), ContainerError> {
// Add a timeout for build operations
let timeout_duration = std::time::Duration::from_secs(120); // 2 minutes timeout for builds
match tokio::time::timeout(timeout_duration, self.build_image_inner(dockerfile, tag)).await
match tokio::time::timeout(
timeout_duration,
self.build_image_inner(dockerfile, tag, context_dir),
)
.await
{
Ok(result) => result,
Err(_) => {
@@ -655,7 +665,8 @@ impl ContainerRuntime for PodmanRuntime {
// Build the customized image
let image_tag = format!("wrkflw-{}-{}", language, version.unwrap_or("latest"));
self.build_image(&dockerfile_path, &image_tag).await?;
self.build_image(&dockerfile_path, &image_tag, temp_dir.path())
.await?;
// Store the customized image
Self::set_language_specific_image("", language, version, &image_tag);
@@ -673,6 +684,7 @@ impl PodmanRuntime {
env_vars: &[(&str, &str)],
working_dir: &Path,
volumes: &[(&Path, &Path)],
entrypoint: Option<&str>,
) -> Result<ContainerOutput, ContainerError> {
wrkflw_logging::debug(&format!("Running command in Podman: {:?}", cmd));
wrkflw_logging::debug(&format!("Environment: {:?}", env_vars));
@@ -720,10 +732,19 @@ impl PodmanRuntime {
args.push(volume_string);
}
// Override entrypoint if specified by action.yml
let ep_string;
if let Some(ep) = entrypoint.filter(|s| !s.is_empty()) {
ep_string = ep.to_string();
args.push("--entrypoint");
args.push(&ep_string);
}
// Add the image
args.push(image);
// Add the command
// Add the command. If cmd is empty, nothing is appended and the
// image's built-in ENTRYPOINT/CMD is used.
args.extend(cmd);
// Track the container (even though we use --rm, track it for consistency)
@@ -847,8 +868,12 @@ impl PodmanRuntime {
Ok(())
}
async fn build_image_inner(&self, dockerfile: &Path, tag: &str) -> Result<(), ContainerError> {
let context_dir = dockerfile.parent().unwrap_or(Path::new("."));
async fn build_image_inner(
&self,
dockerfile: &Path,
tag: &str,
context_dir: &Path,
) -> Result<(), ContainerError> {
let dockerfile_str = dockerfile.to_string_lossy().to_string();
let context_dir_str = context_dir.to_string_lossy().to_string();
let args = vec!["build", "-f", &dockerfile_str, "-t", tag, &context_dir_str];

16
crates/runtime/src/container.rs
View File

@@ -3,6 +3,14 @@ use std::path::Path;
#[async_trait]
pub trait ContainerRuntime {
/// Run a command inside a container.
///
/// If `cmd` is empty (`&[]`), the container runs with the image's built-in
/// ENTRYPOINT/CMD. This is used for Docker-type GitHub Actions whose
/// entrypoint is baked into the image.
///
/// `entrypoint` optionally overrides the image's ENTRYPOINT (used when an
/// action.yml declares `runs.entrypoint`).
async fn run_container(
&self,
image: &str,
@@ -10,11 +18,17 @@ pub trait ContainerRuntime {
env_vars: &[(&str, &str)],
working_dir: &Path,
volumes: &[(&Path, &Path)],
entrypoint: Option<&str>,
) -> Result<ContainerOutput, ContainerError>;
async fn pull_image(&self, image: &str) -> Result<(), ContainerError>;
async fn build_image(&self, dockerfile: &Path, tag: &str) -> Result<(), ContainerError>;
async fn build_image(
&self,
dockerfile: &Path,
tag: &str,
context_dir: &Path,
) -> Result<(), ContainerError>;
async fn prepare_language_environment(
&self,

49
crates/runtime/src/emulation.rs
View File

@@ -153,6 +153,7 @@ impl ContainerRuntime for EmulationRuntime {
env_vars: &[(&str, &str)],
working_dir: &Path,
_volumes: &[(&Path, &Path)],
_entrypoint: Option<&str>,
) -> Result<ContainerOutput, ContainerError> {
// Build command string
let mut command_str = String::new();
@@ -169,9 +170,20 @@ impl ContainerRuntime for EmulationRuntime {
wrkflw_logging::info(&format!("Command length: {}", command.len()));
if command.is_empty() {
return Err(ContainerError::ContainerExecution(
"Empty command array".to_string(),
));
// Empty cmd means "use the image's built-in ENTRYPOINT/CMD".
// Emulation mode cannot run Docker images natively, so return
// a descriptive message instead of erroring.
wrkflw_logging::warning(
"Emulation mode cannot run Docker actions with native entrypoints. \
Use --runtime docker for full Docker action support.",
);
return Ok(ContainerOutput {
stdout: String::new(),
stderr: "Emulation mode: skipped Docker action (requires --runtime docker). \
The action was NOT executed."
.to_string(),
exit_code: 1,
});
}
// Print each command part separately for debugging
@@ -391,7 +403,12 @@ impl ContainerRuntime for EmulationRuntime {
Ok(())
}
async fn build_image(&self, dockerfile: &Path, tag: &str) -> Result<(), ContainerError> {
async fn build_image(
&self,
dockerfile: &Path,
tag: &str,
_context_dir: &Path,
) -> Result<(), ContainerError> {
wrkflw_logging::info(&format!(
"🔄 Emulation: Pretending to build image {} from {}",
tag,
@@ -842,10 +859,34 @@ mod tests {
&[],
Path::new("."),
&[(Path::new("."), Path::new("/github/workspace"))],
None,
)
.await;
let output = result.expect("non-zero exit should return Ok, not Err");
assert_eq!(output.exit_code, 42);
}
#[tokio::test]
async fn test_empty_cmd_returns_failure_in_emulation() {
let runtime = EmulationRuntime::new();
let result = runtime
.run_container(
"some-action:latest",
&[],
&[],
Path::new("."),
&[(Path::new("."), Path::new("/github/workspace"))],
None,
)
.await;
let output = result.expect("empty cmd should return Ok with exit_code 1, not Err");
assert_eq!(output.exit_code, 1, "empty cmd should signal failure");
assert!(
output.stderr.contains("skipped Docker action"),
"stderr should explain the action was not executed"
);
assert!(output.stdout.is_empty(), "stdout should be empty");
}
}

1
crates/runtime/src/emulation_test.rs
View File

@@ -172,6 +172,7 @@ mod emulation_cleanup_tests {
&[],
Path::new("/"),
&[(Path::new("."), Path::new("/github/workspace"))],
None,
)
.await;

18
crates/runtime/src/secure_emulation.rs
View File

@@ -47,7 +47,16 @@ impl ContainerRuntime for SecureEmulationRuntime {
env_vars: &[(&str, &str)],
working_dir: &Path,
_volumes: &[(&Path, &Path)],
entrypoint: Option<&str>,
) -> Result<ContainerOutput, ContainerError> {
if let Some(ep) = entrypoint {
wrkflw_logging::warning(&format!(
"Secure emulation mode ignoring entrypoint override '{}' for image '{}'. \
Use --runtime docker for full Docker action support.",
ep, image
));
}
wrkflw_logging::info(&format!(
"🔒 Executing sandboxed command: {} (image: {})",
command.join(" "),
@@ -128,7 +137,12 @@ impl ContainerRuntime for SecureEmulationRuntime {
Ok(())
}
async fn build_image(&self, dockerfile: &Path, tag: &str) -> Result<(), ContainerError> {
async fn build_image(
&self,
dockerfile: &Path,
tag: &str,
_context_dir: &Path,
) -> Result<(), ContainerError> {
wrkflw_logging::info(&format!(
"🔒 Secure emulation: Pretending to build image {} from {}",
tag,
@@ -308,6 +322,7 @@ mod tests {
&[],
&PathBuf::from("."),
&[],
None,
)
.await;
@@ -328,6 +343,7 @@ mod tests {
&[],
&PathBuf::from("."),
&[],
None,
)
.await;