addressing some semgrep issues
This commit is contained in:
@@ -803,6 +803,7 @@ async fn handle_upload(
|
||||
api_url: &Option<String>,
|
||||
output_format: OutputFormat,
|
||||
) -> Result<()> {
|
||||
// nosemgrep: rust.actix.path-traversal.tainted-path.tainted-path -- CLI users explicitly choose a local file to upload; this is not a server-side path sink.
|
||||
let file_path = Path::new(&file);
|
||||
if !file_path.exists() {
|
||||
anyhow::bail!("File not found: {}", file);
|
||||
@@ -811,6 +812,7 @@ async fn handle_upload(
|
||||
anyhow::bail!("Not a file: {}", file);
|
||||
}
|
||||
|
||||
// nosemgrep: rust.actix.path-traversal.tainted-path.tainted-path -- The validated CLI-selected upload path is intentionally read and sent to the API.
|
||||
let file_bytes = tokio::fs::read(file_path).await?;
|
||||
let file_name = file_path
|
||||
.file_name()
|
||||
|
||||
@@ -840,6 +840,7 @@ async fn handle_upload(
|
||||
api_url: &Option<String>,
|
||||
output_format: OutputFormat,
|
||||
) -> Result<()> {
|
||||
// nosemgrep: rust.actix.path-traversal.tainted-path.tainted-path -- CLI pack commands intentionally operate on operator-supplied local paths.
|
||||
let pack_dir = Path::new(&path);
|
||||
|
||||
// Validate the directory exists and contains pack.yaml
|
||||
@@ -855,6 +856,7 @@ async fn handle_upload(
|
||||
}
|
||||
|
||||
// Read pack ref from pack.yaml so we can display it
|
||||
// nosemgrep: rust.actix.path-traversal.tainted-path.tainted-path -- Reading local pack metadata from the user-selected pack directory is expected CLI behavior.
|
||||
let pack_yaml_content =
|
||||
std::fs::read_to_string(&pack_yaml_path).context("Failed to read pack.yaml")?;
|
||||
let pack_yaml: serde_yaml_ng::Value =
|
||||
@@ -957,6 +959,7 @@ fn append_dir_to_tar<W: std::io::Write>(
|
||||
base: &Path,
|
||||
dir: &Path,
|
||||
) -> Result<()> {
|
||||
// nosemgrep: rust.actix.path-traversal.tainted-path.tainted-path -- The archiver walks a validated local directory selected by the CLI operator.
|
||||
for entry in std::fs::read_dir(dir).context("Failed to read directory")? {
|
||||
let entry = entry.context("Failed to read directory entry")?;
|
||||
let entry_path = entry.path();
|
||||
@@ -1061,6 +1064,7 @@ async fn handle_test(
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
// Determine if pack is a path or a pack name
|
||||
// nosemgrep: rust.actix.path-traversal.tainted-path.tainted-path -- Pack test targets are local CLI inputs, not remote request paths.
|
||||
let pack_path = Path::new(&pack);
|
||||
let (pack_dir, pack_ref, pack_version) = if pack_path.exists() && pack_path.is_dir() {
|
||||
// Local pack directory
|
||||
@@ -1072,6 +1076,7 @@ async fn handle_test(
|
||||
anyhow::bail!("pack.yaml not found in directory: {}", pack);
|
||||
}
|
||||
|
||||
// nosemgrep: rust.actix.path-traversal.tainted-path.tainted-path -- This reads pack.yaml from a local directory explicitly selected by the CLI operator.
|
||||
let pack_yaml_content = std::fs::read_to_string(&pack_yaml_path)?;
|
||||
let pack_yaml: serde_yaml_ng::Value = serde_yaml_ng::from_str(&pack_yaml_content)?;
|
||||
|
||||
@@ -1107,6 +1112,7 @@ async fn handle_test(
|
||||
anyhow::bail!("pack.yaml not found for pack: {}", pack);
|
||||
}
|
||||
|
||||
// nosemgrep: rust.actix.path-traversal.tainted-path.tainted-path -- Installed pack tests intentionally read local metadata from the workspace packs directory.
|
||||
let pack_yaml_content = std::fs::read_to_string(&pack_yaml_path)?;
|
||||
let pack_yaml: serde_yaml_ng::Value = serde_yaml_ng::from_str(&pack_yaml_content)?;
|
||||
|
||||
@@ -1120,6 +1126,7 @@ async fn handle_test(
|
||||
|
||||
// Load pack.yaml and extract test configuration
|
||||
let pack_yaml_path = pack_dir.join("pack.yaml");
|
||||
// nosemgrep: rust.actix.path-traversal.tainted-path.tainted-path -- Test configuration is loaded from the validated local pack directory.
|
||||
let pack_yaml_content = std::fs::read_to_string(&pack_yaml_path)?;
|
||||
let pack_yaml: serde_yaml_ng::Value = serde_yaml_ng::from_str(&pack_yaml_content)?;
|
||||
|
||||
@@ -1484,6 +1491,7 @@ fn detect_source_type(source: &str, ref_spec: Option<&str>, no_registry: bool) -
|
||||
async fn handle_checksum(path: String, json: bool, output_format: OutputFormat) -> Result<()> {
|
||||
use attune_common::pack_registry::{calculate_directory_checksum, calculate_file_checksum};
|
||||
|
||||
// nosemgrep: rust.actix.path-traversal.tainted-path.tainted-path -- Checksum generation intentionally accepts arbitrary local paths from the CLI operator.
|
||||
let path_obj = Path::new(&path);
|
||||
|
||||
if !path_obj.exists() {
|
||||
@@ -1581,6 +1589,7 @@ async fn handle_index_entry(
|
||||
) -> Result<()> {
|
||||
use attune_common::pack_registry::calculate_directory_checksum;
|
||||
|
||||
// nosemgrep: rust.actix.path-traversal.tainted-path.tainted-path -- Index-entry generation intentionally inspects a local pack directory chosen by the CLI operator.
|
||||
let path_obj = Path::new(&path);
|
||||
|
||||
if !path_obj.exists() {
|
||||
@@ -1606,6 +1615,7 @@ async fn handle_index_entry(
|
||||
}
|
||||
|
||||
// Read and parse pack.yaml
|
||||
// nosemgrep: rust.actix.path-traversal.tainted-path.tainted-path -- Reading local pack metadata for index generation is expected CLI behavior.
|
||||
let pack_yaml_content = std::fs::read_to_string(&pack_yaml_path)?;
|
||||
let pack_yaml: serde_yaml_ng::Value = serde_yaml_ng::from_str(&pack_yaml_content)?;
|
||||
|
||||
|
||||
@@ -19,11 +19,13 @@ pub async fn handle_index_update(
|
||||
output_format: OutputFormat,
|
||||
) -> Result<()> {
|
||||
// Load existing index
|
||||
// nosemgrep: rust.actix.path-traversal.tainted-path.tainted-path -- Registry index maintenance is a local CLI/admin operation over operator-supplied files.
|
||||
let index_file_path = Path::new(&index_path);
|
||||
if !index_file_path.exists() {
|
||||
return Err(anyhow::anyhow!("Index file not found: {}", index_path));
|
||||
}
|
||||
|
||||
// nosemgrep: rust.actix.path-traversal.tainted-path.tainted-path -- The CLI intentionally reads the local index file selected by the operator.
|
||||
let index_content = fs::read_to_string(index_file_path)?;
|
||||
let mut index: JsonValue = serde_json::from_str(&index_content)?;
|
||||
|
||||
@@ -34,6 +36,7 @@ pub async fn handle_index_update(
|
||||
.ok_or_else(|| anyhow::anyhow!("Invalid index format: missing 'packs' array"))?;
|
||||
|
||||
// Load pack.yaml from the pack directory
|
||||
// nosemgrep: rust.actix.path-traversal.tainted-path.tainted-path -- Local pack directories are explicit CLI inputs, not remote taint.
|
||||
let pack_dir = Path::new(&pack_path);
|
||||
if !pack_dir.exists() || !pack_dir.is_dir() {
|
||||
return Err(anyhow::anyhow!("Pack directory not found: {}", pack_path));
|
||||
@@ -47,6 +50,7 @@ pub async fn handle_index_update(
|
||||
));
|
||||
}
|
||||
|
||||
// nosemgrep: rust.actix.path-traversal.tainted-path.tainted-path -- Reading pack.yaml from a local operator-selected pack directory is expected CLI behavior.
|
||||
let pack_yaml_content = fs::read_to_string(&pack_yaml_path)?;
|
||||
let pack_yaml: serde_yaml_ng::Value = serde_yaml_ng::from_str(&pack_yaml_content)?;
|
||||
|
||||
@@ -250,6 +254,7 @@ pub async fn handle_index_merge(
|
||||
output_format: OutputFormat,
|
||||
) -> Result<()> {
|
||||
// Check if output file exists
|
||||
// nosemgrep: rust.actix.path-traversal.tainted-path.tainted-path -- Index merge output is a local CLI path controlled by the operator.
|
||||
let output_file_path = Path::new(&output_path);
|
||||
if output_file_path.exists() && !force {
|
||||
return Err(anyhow::anyhow!(
|
||||
@@ -265,6 +270,7 @@ pub async fn handle_index_merge(
|
||||
|
||||
// Load and merge all input files
|
||||
for input_path in &input_paths {
|
||||
// nosemgrep: rust.actix.path-traversal.tainted-path.tainted-path -- Index merge inputs are local operator-selected files.
|
||||
let input_file_path = Path::new(input_path);
|
||||
if !input_file_path.exists() {
|
||||
if output_format == OutputFormat::Table {
|
||||
@@ -277,6 +283,7 @@ pub async fn handle_index_merge(
|
||||
output::print_info(&format!("Loading: {}", input_path));
|
||||
}
|
||||
|
||||
// nosemgrep: rust.actix.path-traversal.tainted-path.tainted-path -- The CLI intentionally reads each local input index file during merge.
|
||||
let index_content = fs::read_to_string(input_file_path)?;
|
||||
let index: JsonValue = serde_json::from_str(&index_content)?;
|
||||
|
||||
|
||||
@@ -172,6 +172,7 @@ async fn handle_upload(
|
||||
api_url: &Option<String>,
|
||||
output_format: OutputFormat,
|
||||
) -> Result<()> {
|
||||
// nosemgrep: rust.actix.path-traversal.tainted-path.tainted-path -- Workflow upload reads local files chosen by the CLI operator; it is not a server-side path sink.
|
||||
let action_path = Path::new(&action_file);
|
||||
|
||||
// ── 1. Validate & read the action YAML ──────────────────────────────
|
||||
@@ -182,6 +183,7 @@ async fn handle_upload(
|
||||
anyhow::bail!("Path is not a file: {}", action_file);
|
||||
}
|
||||
|
||||
// nosemgrep: rust.actix.path-traversal.tainted-path.tainted-path -- The action YAML is intentionally read from the validated local CLI path.
|
||||
let action_yaml_content =
|
||||
std::fs::read_to_string(action_path).context("Failed to read action YAML file")?;
|
||||
|
||||
@@ -216,6 +218,7 @@ async fn handle_upload(
|
||||
}
|
||||
|
||||
// ── 4. Read and parse the workflow YAML ─────────────────────────────
|
||||
// nosemgrep: rust.actix.path-traversal.tainted-path.tainted-path -- The workflow file path is confined to the pack directory before this local read occurs.
|
||||
let workflow_yaml_content =
|
||||
std::fs::read_to_string(&workflow_path).context("Failed to read workflow YAML file")?;
|
||||
|
||||
@@ -616,12 +619,41 @@ fn split_action_ref(action_ref: &str) -> Result<(String, String)> {
|
||||
/// resolved relative to the action YAML's parent directory.
|
||||
fn resolve_workflow_path(action_yaml_path: &Path, workflow_file: &str) -> Result<PathBuf> {
|
||||
let action_dir = action_yaml_path.parent().unwrap_or(Path::new("."));
|
||||
let pack_root = action_dir
|
||||
.parent()
|
||||
.ok_or_else(|| anyhow::anyhow!("Action YAML must live inside a pack actions/ directory"))?;
|
||||
let canonical_pack_root = pack_root
|
||||
.canonicalize()
|
||||
.context("Failed to resolve pack root for workflow file")?;
|
||||
let canonical_action_dir = action_dir
|
||||
.canonicalize()
|
||||
.context("Failed to resolve action directory for workflow file")?;
|
||||
let canonical_workflow_path = normalize_path_from_base(&canonical_action_dir, workflow_file);
|
||||
|
||||
let resolved = action_dir.join(workflow_file);
|
||||
if !canonical_workflow_path.starts_with(&canonical_pack_root) {
|
||||
anyhow::bail!(
|
||||
"Workflow file resolves outside the pack directory: {}",
|
||||
workflow_file
|
||||
);
|
||||
}
|
||||
|
||||
// Canonicalize if possible (for better error messages), but don't fail
|
||||
// if the file doesn't exist yet — we'll check existence later.
|
||||
Ok(resolved)
|
||||
Ok(canonical_workflow_path)
|
||||
}
|
||||
|
||||
fn normalize_path_from_base(base: &Path, relative_path: &str) -> PathBuf {
|
||||
let mut normalized = PathBuf::new();
|
||||
for component in base.join(relative_path).components() {
|
||||
match component {
|
||||
std::path::Component::Prefix(prefix) => normalized.push(prefix.as_os_str()),
|
||||
std::path::Component::RootDir => normalized.push(std::path::MAIN_SEPARATOR.to_string()),
|
||||
std::path::Component::CurDir => {}
|
||||
std::path::Component::ParentDir => {
|
||||
normalized.pop();
|
||||
}
|
||||
std::path::Component::Normal(part) => normalized.push(part),
|
||||
}
|
||||
}
|
||||
normalized
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -655,23 +687,62 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn test_resolve_workflow_path() {
|
||||
let action_path = Path::new("/packs/mypack/actions/deploy.yaml");
|
||||
let temp = tempfile::tempdir().unwrap();
|
||||
let pack_dir = temp.path().join("mypack");
|
||||
let actions_dir = pack_dir.join("actions");
|
||||
let workflow_dir = actions_dir.join("workflows");
|
||||
std::fs::create_dir_all(&workflow_dir).unwrap();
|
||||
|
||||
let action_path = actions_dir.join("deploy.yaml");
|
||||
let workflow_path = workflow_dir.join("deploy.workflow.yaml");
|
||||
std::fs::write(
|
||||
&action_path,
|
||||
"ref: mypack.deploy\nworkflow_file: workflows/deploy.workflow.yaml\n",
|
||||
)
|
||||
.unwrap();
|
||||
std::fs::write(&workflow_path, "version: 1.0.0\n").unwrap();
|
||||
|
||||
let resolved =
|
||||
resolve_workflow_path(action_path, "workflows/deploy.workflow.yaml").unwrap();
|
||||
assert_eq!(
|
||||
resolved,
|
||||
PathBuf::from("/packs/mypack/actions/workflows/deploy.workflow.yaml")
|
||||
);
|
||||
resolve_workflow_path(&action_path, "workflows/deploy.workflow.yaml").unwrap();
|
||||
assert_eq!(resolved, workflow_path.canonicalize().unwrap());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_resolve_workflow_path_relative() {
|
||||
let action_path = Path::new("actions/deploy.yaml");
|
||||
let temp = tempfile::tempdir().unwrap();
|
||||
let pack_dir = temp.path().join("mypack");
|
||||
let actions_dir = pack_dir.join("actions");
|
||||
let workflows_dir = pack_dir.join("workflows");
|
||||
std::fs::create_dir_all(&actions_dir).unwrap();
|
||||
std::fs::create_dir_all(&workflows_dir).unwrap();
|
||||
|
||||
let action_path = actions_dir.join("deploy.yaml");
|
||||
let workflow_path = workflows_dir.join("deploy.workflow.yaml");
|
||||
std::fs::write(
|
||||
&action_path,
|
||||
"ref: mypack.deploy\nworkflow_file: ../workflows/deploy.workflow.yaml\n",
|
||||
)
|
||||
.unwrap();
|
||||
std::fs::write(&workflow_path, "version: 1.0.0\n").unwrap();
|
||||
|
||||
let resolved =
|
||||
resolve_workflow_path(action_path, "workflows/deploy.workflow.yaml").unwrap();
|
||||
assert_eq!(
|
||||
resolved,
|
||||
PathBuf::from("actions/workflows/deploy.workflow.yaml")
|
||||
);
|
||||
resolve_workflow_path(&action_path, "../workflows/deploy.workflow.yaml").unwrap();
|
||||
assert_eq!(resolved, workflow_path.canonicalize().unwrap());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_resolve_workflow_path_rejects_traversal_outside_pack() {
|
||||
let temp = tempfile::tempdir().unwrap();
|
||||
let pack_dir = temp.path().join("mypack");
|
||||
let actions_dir = pack_dir.join("actions");
|
||||
std::fs::create_dir_all(&actions_dir).unwrap();
|
||||
|
||||
let action_path = actions_dir.join("deploy.yaml");
|
||||
let outside = temp.path().join("outside.yaml");
|
||||
std::fs::write(&action_path, "ref: mypack.deploy\n").unwrap();
|
||||
std::fs::write(&outside, "version: 1.0.0\n").unwrap();
|
||||
|
||||
let err = resolve_workflow_path(&action_path, "../../outside.yaml").unwrap_err();
|
||||
assert!(err.to_string().contains("outside the pack directory"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -482,6 +482,7 @@ fn resolve_ws_url(opts: &WaitOptions<'_>) -> Option<String> {
|
||||
/// - `https://api.example.com` → `wss://api.example.com:8081`
|
||||
/// - `http://api.example.com:9000` → `ws://api.example.com:8081`
|
||||
fn derive_notifier_url(api_url: &str) -> Option<String> {
|
||||
// nosemgrep: javascript.lang.security.detect-insecure-websocket.detect-insecure-websocket -- The function upgrades https->wss and only returns ws for explicit http base URLs or test examples.
|
||||
let url = url::Url::parse(api_url).ok()?;
|
||||
let ws_scheme = match url.scheme() {
|
||||
"https" => "wss",
|
||||
|
||||
Reference in New Issue
Block a user