Use Path in ContextDir methods.

This commit is contained in:
jcamiel 2024-03-14 09:45:02 +01:00
parent 46e19d1024
commit 5ed316d014
No known key found for this signature in database
GPG Key ID: 07FF11CFD55356CC
6 changed files with 81 additions and 90 deletions

View File

@ -749,7 +749,7 @@ impl Client {
// --output is not an option of the HTTP client, we deal with it here:
match output {
Some(Output::File(filename)) => {
let filename = context_dir.get_path(&filename.to_string_lossy());
let filename = context_dir.resolved_path(filename);
arguments.push("--output".to_string());
arguments.push(filename.to_string_lossy().to_string());
}

View File

@ -16,6 +16,7 @@
*
*/
use std::collections::HashMap;
use std::path::Path;
use crate::http::core::*;
use crate::http::header::CONTENT_TYPE;
@ -125,8 +126,8 @@ fn encode_byte(b: u8) -> String {
format!("\\x{b:02x}")
}
fn encode_bytes(b: Vec<u8>) -> String {
b.iter().map(|b| encode_byte(*b)).collect()
fn encode_bytes(bytes: &[u8]) -> String {
bytes.iter().map(|b| encode_byte(*b)).collect()
}
impl Method {
@ -187,8 +188,8 @@ impl MultipartParam {
content_type,
..
}) => {
let path = context_dir.get_path(filename);
let value = format!("@{};type={}", path.to_str().unwrap(), content_type);
let path = context_dir.resolved_path(Path::new(filename));
let value = format!("@{};type={}", path.to_string_lossy(), content_type);
format!("{name}={value}")
}
}
@ -197,12 +198,12 @@ impl MultipartParam {
impl Body {
pub fn curl_arg(&self, context_dir: &ContextDir) -> String {
match self.clone() {
Body::Text(s) => encode_shell_string(&s),
match self {
Body::Text(s) => encode_shell_string(s),
Body::Binary(bytes) => format!("$'{}'", encode_bytes(bytes)),
Body::File(_, filename) => {
let path = context_dir.get_path(&filename);
format!("'@{}'", path.to_str().unwrap())
let path = context_dir.resolved_path(Path::new(filename));
format!("'@{}'", path.to_string_lossy())
}
}
}

View File

@ -73,17 +73,15 @@ pub fn eval_file(
let file = eval_template(filename, variables)?;
// In order not to leak any private date, we check that the user provided file
// is a child of the context directory.
if !context_dir.is_access_allowed(&file) {
let inner = RunnerError::UnauthorizedFileAccess {
path: PathBuf::from(file),
};
let path = PathBuf::from(file);
if !context_dir.is_access_allowed(&path) {
let inner = RunnerError::UnauthorizedFileAccess { path };
return Err(Error::new(filename.source_info, inner, false));
}
let resolved_file = context_dir.get_path(&file);
let resolved_file = context_dir.resolved_path(&path);
match std::fs::read(resolved_file) {
Ok(value) => Ok(value),
Err(_) => {
let path = PathBuf::from(&file);
let inner = RunnerError::FileReadAccess { path };
Err(Error::new(filename.source_info, inner, false))
}

View File

@ -77,10 +77,10 @@ impl Output {
let filename = match context_dir {
None => filename.clone(),
Some(context_dir) => {
if !context_dir.is_access_allowed(&filename.to_string_lossy()) {
if !context_dir.is_access_allowed(filename) {
return Err(Error::new_unauthorized_file_access(filename));
}
context_dir.get_path(&filename.to_string_lossy())
context_dir.resolved_path(filename)
}
};
let mut file = match File::create(&filename) {

View File

@ -21,7 +21,6 @@ use crate::http::{Call, Cookie};
use crate::runner::error::Error;
use crate::runner::output::Output;
use crate::runner::value::Value;
use crate::runner::RunnerError;
use crate::util::path::ContextDir;
use crate::util::term::Stdout;
@ -68,18 +67,25 @@ impl HurlResult {
}
}
/// Represents the execution result of an entry.
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct EntryResult {
/// 1-based index of the entry on the file execution.
pub entry_index: usize,
/// Source information of this entry.
pub source_info: SourceInfo,
/// List of HTTP request / response pair.
pub calls: Vec<Call>,
/// List of captures.
pub captures: Vec<CaptureResult>,
/// List of asserts.
pub asserts: Vec<AssertResult>,
/// List of errors.
pub errors: Vec<Error>,
pub time_in_ms: u128,
// The entry has been executed with `--compressed` option:
// server is requested to send compressed response, and the response should be uncompressed
// when outputted on stdout.
/// The entry has been executed with `--compressed` option:
/// server is requested to send compressed response, and the response should be uncompressed
/// when outputted on stdout.
pub compressed: bool,
}
@ -149,25 +155,11 @@ impl EntryResult {
let Some(call) = self.calls.last() else {
return Ok(());
};
// We check file access authorization for file output when a context dir has been given
if let Output::File(filename) = output {
if !context_dir.is_access_allowed(&filename.to_string_lossy()) {
let inner = RunnerError::UnauthorizedFileAccess {
path: filename.clone(),
};
return Err(Error::new(source_info, inner, false));
}
}
let response = &call.response;
if self.compressed {
let bytes = match response.uncompress_body() {
Ok(bytes) => bytes,
Err(e) => {
// TODO: pass a [`SourceInfo`] in case of error
// We may pass a [`SourceInfo`] as a parameter of this method to make
// a more accurate error (for instance a [`SourceInfo`] pointing at
// `output: foo.bin`
let source_info = SourceInfo::new(Pos::new(0, 0), Pos::new(0, 0));
return Err(Error::new(source_info, e.into(), false));
}
};

View File

@ -49,15 +49,15 @@ impl ContextDir {
}
/// Returns a path (absolute or relative), given a filename.
pub fn get_path(&self, filename: &str) -> PathBuf {
self.file_root.join(Path::new(filename))
pub fn resolved_path(&self, filename: &Path) -> PathBuf {
self.file_root.join(filename)
}
/// Checks if a given `filename` access is authorized.
/// This method is used to check if a local file can be included in POST request or if a
/// response can be outputted to a given file when using `output` option in \[Options\] sections.
pub fn is_access_allowed(&self, filename: &str) -> bool {
let file = self.get_path(filename);
pub fn is_access_allowed(&self, filename: &Path) -> bool {
let file = self.resolved_path(filename);
let absolute_file = self.current_dir.join(file);
let absolute_file_root = self.current_dir.join(&self.file_root);
is_descendant(absolute_file.as_path(), absolute_file_root.as_path())
@ -120,18 +120,18 @@ mod tests {
// ```
let current_dir = Path::new("/tmp");
let file_root = Path::new("");
let context_dir = ContextDir::new(current_dir, file_root);
assert!(context_dir.is_access_allowed("foo.bin"));
assert!(context_dir.is_access_allowed("/tmp/foo.bin"));
assert!(context_dir.is_access_allowed("a/foo.bin"));
assert!(context_dir.is_access_allowed("a/b/foo.bin"));
assert!(context_dir.is_access_allowed("../tmp/a/b/foo.bin"));
assert!(context_dir.is_access_allowed("../../../tmp/a/b/foo.bin"));
let ctx = ContextDir::new(current_dir, file_root);
assert!(ctx.is_access_allowed(Path::new("foo.bin")));
assert!(ctx.is_access_allowed(Path::new("/tmp/foo.bin")));
assert!(ctx.is_access_allowed(Path::new("a/foo.bin")));
assert!(ctx.is_access_allowed(Path::new("a/b/foo.bin")));
assert!(ctx.is_access_allowed(Path::new("../tmp/a/b/foo.bin")));
assert!(ctx.is_access_allowed(Path::new("../../../tmp/a/b/foo.bin")));
assert!(!context_dir.is_access_allowed("/file/foo.bin"));
assert!(!context_dir.is_access_allowed("../foo.bin"));
assert!(!context_dir.is_access_allowed("../../foo.bin"));
assert!(!context_dir.is_access_allowed("../../file/foo.bin"));
assert!(!ctx.is_access_allowed(Path::new("/file/foo.bin")));
assert!(!ctx.is_access_allowed(Path::new("../foo.bin")));
assert!(!ctx.is_access_allowed(Path::new("../../foo.bin")));
assert!(!ctx.is_access_allowed(Path::new("../../file/foo.bin")));
}
#[test]
@ -142,33 +142,33 @@ mod tests {
// ```
let current_dir = Path::new("/tmp");
let file_root = Path::new("/file");
let context_dir = ContextDir::new(current_dir, file_root);
assert!(context_dir.is_access_allowed("foo.bin")); // absolute path is /file/foo.bin
assert!(context_dir.is_access_allowed("/file/foo.bin"));
assert!(context_dir.is_access_allowed("a/foo.bin"));
assert!(context_dir.is_access_allowed("a/b/foo.bin"));
assert!(context_dir.is_access_allowed("../../file/foo.bin"));
let ctx = ContextDir::new(current_dir, file_root);
assert!(ctx.is_access_allowed(Path::new("foo.bin"))); // absolute path is /file/foo.bin
assert!(ctx.is_access_allowed(Path::new("/file/foo.bin")));
assert!(ctx.is_access_allowed(Path::new("a/foo.bin")));
assert!(ctx.is_access_allowed(Path::new("a/b/foo.bin")));
assert!(ctx.is_access_allowed(Path::new("../../file/foo.bin")));
assert!(!context_dir.is_access_allowed("/tmp/foo.bin"));
assert!(!context_dir.is_access_allowed("../tmp/a/b/foo.bin"));
assert!(!context_dir.is_access_allowed("../foo.bin"));
assert!(!context_dir.is_access_allowed("../../foo.bin"));
assert!(!context_dir.is_access_allowed("../../../tmp/a/b/foo.bin"));
assert!(!ctx.is_access_allowed(Path::new("/tmp/foo.bin")));
assert!(!ctx.is_access_allowed(Path::new("../tmp/a/b/foo.bin")));
assert!(!ctx.is_access_allowed(Path::new("../foo.bin")));
assert!(!ctx.is_access_allowed(Path::new("../../foo.bin")));
assert!(!ctx.is_access_allowed(Path::new("../../../tmp/a/b/foo.bin")));
let current_dir = Path::new("/tmp");
let file_root = Path::new("../file");
let context_dir = ContextDir::new(current_dir, file_root);
assert!(context_dir.is_access_allowed("foo.bin"));
assert!(context_dir.is_access_allowed("/file/foo.bin"));
assert!(context_dir.is_access_allowed("a/foo.bin"));
assert!(context_dir.is_access_allowed("a/b/foo.bin"));
assert!(context_dir.is_access_allowed("../../file/foo.bin"));
let ctx = ContextDir::new(current_dir, file_root);
assert!(ctx.is_access_allowed(Path::new("foo.bin")));
assert!(ctx.is_access_allowed(Path::new("/file/foo.bin")));
assert!(ctx.is_access_allowed(Path::new("a/foo.bin")));
assert!(ctx.is_access_allowed(Path::new("a/b/foo.bin")));
assert!(ctx.is_access_allowed(Path::new("../../file/foo.bin")));
assert!(!context_dir.is_access_allowed("/tmp/foo.bin"));
assert!(!context_dir.is_access_allowed("../tmp/a/b/foo.bin"));
assert!(!context_dir.is_access_allowed("../foo.bin"));
assert!(!context_dir.is_access_allowed("../../foo.bin"));
assert!(!context_dir.is_access_allowed("../../../tmp/a/b/foo.bin"));
assert!(!ctx.is_access_allowed(Path::new("/tmp/foo.bin")));
assert!(!ctx.is_access_allowed(Path::new("../tmp/a/b/foo.bin")));
assert!(!ctx.is_access_allowed(Path::new("../foo.bin")));
assert!(!ctx.is_access_allowed(Path::new("../../foo.bin")));
assert!(!ctx.is_access_allowed(Path::new("../../../tmp/a/b/foo.bin")));
}
#[test]
@ -179,14 +179,14 @@ mod tests {
// ```
let current_dir = Path::new("/tmp");
let file_root = Path::new("a/b");
let context_dir = ContextDir::new(current_dir, file_root);
assert!(context_dir.is_access_allowed("foo.bin"));
assert!(context_dir.is_access_allowed("c/foo.bin")); // absolute path is /tmp/a/b/c/foo.bin
assert!(context_dir.is_access_allowed("/tmp/a/b/foo.bin"));
assert!(context_dir.is_access_allowed("/tmp/a/b/c/d/foo.bin"));
assert!(context_dir.is_access_allowed("../../../tmp/a/b/foo.bin"));
let ctx = ContextDir::new(current_dir, file_root);
assert!(ctx.is_access_allowed(Path::new("foo.bin")));
assert!(ctx.is_access_allowed(Path::new("c/foo.bin"))); // absolute path is /tmp/a/b/c/foo.bin
assert!(ctx.is_access_allowed(Path::new("/tmp/a/b/foo.bin")));
assert!(ctx.is_access_allowed(Path::new("/tmp/a/b/c/d/foo.bin")));
assert!(ctx.is_access_allowed(Path::new("../../../tmp/a/b/foo.bin")));
assert!(!context_dir.is_access_allowed("/tmp/foo.bin"));
assert!(!ctx.is_access_allowed(Path::new("/tmp/foo.bin")));
}
#[test]
@ -197,18 +197,18 @@ mod tests {
// ```
let current_dir = Path::new("/tmp");
let file_root = Path::new("../tmp");
let context_dir = ContextDir::new(current_dir, file_root);
assert!(context_dir.is_access_allowed("foo.bin"));
assert!(context_dir.is_access_allowed("/tmp/foo.bin"));
assert!(context_dir.is_access_allowed("a/foo.bin"));
assert!(context_dir.is_access_allowed("a/b/foo.bin"));
assert!(context_dir.is_access_allowed("../tmp/a/b/foo.bin"));
assert!(context_dir.is_access_allowed("../../../tmp/a/b/foo.bin"));
let ctx = ContextDir::new(current_dir, file_root);
assert!(ctx.is_access_allowed(Path::new("foo.bin")));
assert!(ctx.is_access_allowed(Path::new("/tmp/foo.bin")));
assert!(ctx.is_access_allowed(Path::new("a/foo.bin")));
assert!(ctx.is_access_allowed(Path::new("a/b/foo.bin")));
assert!(ctx.is_access_allowed(Path::new("../tmp/a/b/foo.bin")));
assert!(ctx.is_access_allowed(Path::new("../../../tmp/a/b/foo.bin")));
assert!(!context_dir.is_access_allowed("/file/foo.bin"));
assert!(!context_dir.is_access_allowed("../foo.bin"));
assert!(!context_dir.is_access_allowed("../../foo.bin"));
assert!(!context_dir.is_access_allowed("../../file/foo.bin"));
assert!(!ctx.is_access_allowed(Path::new("/file/foo.bin")));
assert!(!ctx.is_access_allowed(Path::new("../foo.bin")));
assert!(!ctx.is_access_allowed(Path::new("../../foo.bin")));
assert!(!ctx.is_access_allowed(Path::new("../../file/foo.bin")));
}
#[test]