Skip to content

Commit e7aeac8

Browse files
committed
fix(cortex-cli): prevent path traversal in github install --workflow-name
Fixes bounty issue #1578 The --workflow-name argument was used directly to construct file paths without validation, allowing path traversal attacks like '../../../malicious'. This fix sanitizes the workflow name using the existing sanitize_filename() utility to remove path separators and traversal sequences before using it in file path construction. Also fixes a pre-existing bug in cortex-engine where the 'permissions' variable was shadowed in a pattern match.
1 parent 8f839ec commit e7aeac8

2 files changed

Lines changed: 33 additions & 4 deletions

File tree

cortex-cli/src/github_cmd.rs

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,10 +107,20 @@ impl GitHubCli {
107107
/// Install GitHub Actions workflow.
108108
async fn run_install(args: InstallArgs) -> Result<()> {
109109
use cortex_engine::github::{WorkflowConfig, generate_workflow};
110+
use cortex_engine::security::sanitize_filename;
111+
112+
// Sanitize workflow name to prevent path traversal attacks
113+
let sanitized_name = sanitize_filename(&args.workflow_name);
114+
if sanitized_name.is_empty() {
115+
bail!(
116+
"Invalid workflow name: '{}' contains only invalid characters",
117+
args.workflow_name
118+
);
119+
}
110120

111121
let repo_path = args.path.unwrap_or_else(|| PathBuf::from("."));
112122
let workflows_dir = repo_path.join(".github").join("workflows");
113-
let workflow_file = workflows_dir.join(format!("{}.yml", args.workflow_name));
123+
let workflow_file = workflows_dir.join(format!("{}.yml", sanitized_name));
114124

115125
// Check if workflow already exists
116126
if workflow_file.exists() && !args.force {
@@ -122,7 +132,7 @@ async fn run_install(args: InstallArgs) -> Result<()> {
122132

123133
// Generate workflow configuration
124134
let config = WorkflowConfig {
125-
name: args.workflow_name.clone(),
135+
name: sanitized_name.clone(),
126136
pr_review: args.pr_review,
127137
issue_automation: args.issue_automation,
128138
};
@@ -145,7 +155,7 @@ async fn run_install(args: InstallArgs) -> Result<()> {
145155
println!(" Settings → Secrets and variables → Actions → New repository secret");
146156
println!();
147157
println!(" 2. Commit and push the workflow file:");
148-
println!(" git add .github/workflows/{}.yml", args.workflow_name);
158+
println!(" git add .github/workflows/{}.yml", sanitized_name);
149159
println!(" git commit -m \"Add Cortex CI/CD automation\"");
150160
println!(" git push");
151161
println!();
@@ -578,6 +588,7 @@ struct InstallationStatus {
578588
#[cfg(test)]
579589
mod tests {
580590
use super::*;
591+
use cortex_engine::security::sanitize_filename;
581592

582593
#[test]
583594
fn test_parse_cortex_command() {
@@ -594,4 +605,22 @@ mod tests {
594605
assert!(help.contains("/Cortex help"));
595606
assert!(help.contains("/Cortex review"));
596607
}
608+
609+
#[test]
610+
fn test_workflow_name_sanitization() {
611+
// Path traversal attempts should be sanitized
612+
assert_eq!(sanitize_filename("../../../malicious"), "malicious");
613+
assert_eq!(sanitize_filename("..\\..\\malicious"), "malicious");
614+
assert_eq!(sanitize_filename("foo/bar"), "foobar");
615+
assert_eq!(sanitize_filename("foo\\bar"), "foobar");
616+
617+
// Normal names should pass through
618+
assert_eq!(sanitize_filename("Cortex"), "Cortex");
619+
assert_eq!(sanitize_filename("my-workflow"), "my-workflow");
620+
assert_eq!(sanitize_filename("workflow_test"), "workflow_test");
621+
622+
// Leading dots should be removed
623+
assert_eq!(sanitize_filename(".hidden"), "hidden");
624+
assert_eq!(sanitize_filename("...dots"), "dots");
625+
}
597626
}

cortex-engine/src/tasks/snapshot.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ impl SnapshotManager {
292292
match state {
293293
FileState::Exists {
294294
content,
295-
permissions: _,
295+
permissions,
296296
..
297297
} => {
298298
// Create parent directories

0 commit comments

Comments
 (0)