Skip to content

Commit 0843417

Browse files
committed
fix: address review feedback — path traversal validation, cancellation handling, lazy tracker init
- Validate config names in add()/rm() to prevent path traversal (P1) - Devcontainer selection cancellation now errors instead of silently falling through to host-mode launch (P2) - History tracker only initialized for open/recent commands, so config/container commands don't fail on corrupt history (P2)
1 parent 92dfc55 commit 0843417

3 files changed

Lines changed: 33 additions & 15 deletions

File tree

src/config_store.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ impl ConfigStore {
115115

116116
/// Creates a minimal config with the given name.
117117
pub fn add(&self, name: &str) -> Result<PathBuf> {
118+
Self::validate_name(name)?;
118119
let root = self.dir.join(name);
119120
if root.exists() {
120121
bail!("Config '{}' already exists at: {}", name, root.display());
@@ -138,6 +139,7 @@ impl ConfigStore {
138139

139140
/// Removes a config by name.
140141
pub fn rm(&self, name: &str) -> Result<()> {
142+
Self::validate_name(name)?;
141143
let root = self.dir.join(name);
142144
if !root.exists() {
143145
bail!("Config '{}' not found", name);
@@ -163,6 +165,19 @@ impl ConfigStore {
163165
Ok(())
164166
}
165167

168+
fn validate_name(name: &str) -> Result<()> {
169+
if name.is_empty()
170+
|| name.contains('/')
171+
|| name.contains('\\')
172+
|| name.contains('\0')
173+
|| name == "."
174+
|| name == ".."
175+
{
176+
bail!("Invalid config name: '{name}'");
177+
}
178+
Ok(())
179+
}
180+
166181
/// Tries to resolve a plain name to a devcontainer.json path within the config directory.
167182
fn resolve_name(&self, name: &str) -> Option<PathBuf> {
168183
let candidate = self

src/launch.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,10 @@ impl Setup {
128128
trace!("Selected the only existing dev container.");
129129
Ok(dev_containers.into_iter().next())
130130
}
131-
_ => Ok(crate::ui::pick_devcontainer(dev_containers)?),
131+
_ => Ok(Some(
132+
crate::ui::pick_devcontainer(dev_containers)?
133+
.ok_or_else(|| eyre!("Dev container selection cancelled"))?,
134+
)),
132135
}
133136
}
134137
}

src/main.rs

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,16 @@ use crate::{
3232
workspace::Workspace,
3333
};
3434

35+
fn load_tracker(history_path: Option<PathBuf>) -> Result<Tracker> {
36+
let path = history_path.unwrap_or_else(|| {
37+
let mut p = dirs::data_local_dir().expect("Local data dir not found.");
38+
p.push("vscli");
39+
p.push("history.json");
40+
p
41+
});
42+
Tracker::load(path)
43+
}
44+
3545
fn resolve_launch_config(config: Option<&PathBuf>, store: &ConfigStore) -> Result<Option<PathBuf>> {
3646
config
3747
.map(|c| {
@@ -57,20 +67,9 @@ fn main() -> Result<()> {
5767

5868
let config_store = ConfigStore::new(opts.config_dir);
5969

60-
let mut tracker = {
61-
let tracker_path = if let Some(path) = opts.history_path {
62-
path
63-
} else {
64-
let mut tracker_path = dirs::data_local_dir().expect("Local data dir not found.");
65-
tracker_path.push("vscli");
66-
tracker_path.push("history.json");
67-
tracker_path
68-
};
69-
Tracker::load(tracker_path)?
70-
};
71-
7270
match opts.command {
7371
opts::Commands::Open { path, launch } => {
72+
let mut tracker = load_tracker(opts.history_path)?;
7473
let path = path.as_path();
7574
let ws = Workspace::from_path(path)?;
7675
let ws_name = ws.name.clone();
@@ -97,12 +96,14 @@ fn main() -> Result<()> {
9796
behavior,
9897
last_opened: Utc::now(),
9998
});
99+
tracker.store()?;
100100
}
101101
opts::Commands::Recent {
102102
launch,
103103
hide_instructions,
104104
hide_info,
105105
} => {
106+
let mut tracker = load_tracker(opts.history_path)?;
106107
let res = ui::start(&mut tracker, hide_instructions, hide_info)?;
107108
if let Some((id, mut entry)) = res {
108109
let ws = Workspace::from_path(&entry.workspace_path)?;
@@ -144,6 +145,7 @@ fn main() -> Result<()> {
144145
},
145146
);
146147
}
148+
tracker.store()?;
147149
}
148150
opts::Commands::Config { action } => {
149151
let editor = std::env::var("VSCLI_EDITOR").unwrap_or_else(|_| "code".to_string());
@@ -155,8 +157,6 @@ fn main() -> Result<()> {
155157
}
156158
}
157159

158-
tracker.store()?;
159-
160160
Ok(())
161161
}
162162

0 commit comments

Comments
 (0)