Skip to content

Commit a72856b

Browse files
committed
refactor: remove path traversal check from env_file
The security check was unnecessary since: 1. Env files in committed repos are already exposed 2. Jobs can execute arbitrary commands anyway (RCE)
1 parent 353bbd6 commit a72856b

File tree

2 files changed

+6
-43
lines changed

2 files changed

+6
-43
lines changed

CLAUDE.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,6 @@ host ENV < runner.env_file < runner.env < job.env_file < job.env
165165
166166
**Shell expansion**: Values support `~` and `$VAR` / `${VAR}` expansion.
167167
168-
**Security**: `env_file` paths are validated to prevent path traversal (must stay within base directory).
169-
170168
## Constraints
171169
172170
- `cargo build` must pass

src/scheduler/executor.rs

Lines changed: 6 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -218,43 +218,6 @@ async fn run_command(job: &Job, work_dir: &PathBuf, sot_path: &PathBuf, runner:
218218
}
219219
}
220220

221-
/// Validate that a path stays within the base directory (prevents path traversal).
222-
fn validate_env_file_path(
223-
env_file_path: &str,
224-
base_dir: &PathBuf,
225-
context: &str,
226-
) -> anyhow::Result<PathBuf> {
227-
let expanded = env::expand_string(env_file_path);
228-
let full_path = base_dir.join(&expanded);
229-
230-
// Canonicalize to resolve .. and symlinks
231-
match (full_path.canonicalize(), base_dir.canonicalize()) {
232-
(Ok(resolved), Ok(base)) if resolved.starts_with(&base) => Ok(resolved),
233-
(Ok(_), Ok(_)) => {
234-
anyhow::bail!(
235-
"{} env_file '{}': path traversal detected (must be within base directory)",
236-
context,
237-
env_file_path
238-
)
239-
}
240-
(Err(e), _) => {
241-
anyhow::bail!(
242-
"{} env_file '{}': file not found or inaccessible: {}",
243-
context,
244-
env_file_path,
245-
e
246-
)
247-
}
248-
(_, Err(e)) => {
249-
anyhow::bail!(
250-
"{} base directory not accessible: {}",
251-
context,
252-
e
253-
)
254-
}
255-
}
256-
}
257-
258221
fn merge_env_vars(
259222
job: &Job,
260223
work_dir: &PathBuf,
@@ -265,8 +228,9 @@ fn merge_env_vars(
265228

266229
// 1. Start with runner.env_file (loaded from sot_path)
267230
if let Some(env_file_path) = &runner.env_file {
268-
let validated_path = validate_env_file_path(env_file_path, sot_path, "runner")?;
269-
let vars = env::load_env_from_path(&validated_path)?;
231+
let expanded = env::expand_string(env_file_path);
232+
let full_path = sot_path.join(&expanded);
233+
let vars = env::load_env_from_path(&full_path)?;
270234
env_vars.extend(vars);
271235
}
272236

@@ -279,8 +243,9 @@ fn merge_env_vars(
279243

280244
// 3. Merge job.env_file (loaded from work_dir)
281245
if let Some(env_file_path) = &job.env_file {
282-
let validated_path = validate_env_file_path(env_file_path, work_dir, &format!("job:{}", job.id))?;
283-
let vars = env::load_env_from_path(&validated_path)?;
246+
let expanded = env::expand_string(env_file_path);
247+
let full_path = work_dir.join(&expanded);
248+
let vars = env::load_env_from_path(&full_path)?;
284249
env_vars.extend(vars);
285250
}
286251

0 commit comments

Comments
 (0)