Skip to content

Commit 4ebc320

Browse files
authored
Harden custom step loader and step remove against symlinks and OSError
1 parent 314f60d commit 4ebc320

3 files changed

Lines changed: 32 additions & 2 deletions

File tree

src/specify_cli/__init__.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6294,7 +6294,13 @@ def workflow_step_remove(
62946294
if dir_exists and not in_registry:
62956295
# No registry write needed; just delete the orphaned directory.
62966296
import shutil
6297-
shutil.rmtree(step_dir)
6297+
try:
6298+
shutil.rmtree(step_dir)
6299+
except OSError as exc:
6300+
console.print(
6301+
f"[red]Error:[/red] Failed to remove step directory {step_dir}: {exc}"
6302+
)
6303+
raise typer.Exit(1)
62986304
elif in_registry:
62996305
# Remove the registry entry FIRST so that, if the registry write fails
63006306
# (read-only filesystem, permission denied), the on-disk step directory
@@ -6306,7 +6312,13 @@ def workflow_step_remove(
63066312
raise typer.Exit(1)
63076313
if dir_exists:
63086314
import shutil
6309-
shutil.rmtree(step_dir)
6315+
try:
6316+
shutil.rmtree(step_dir)
6317+
except OSError as exc:
6318+
console.print(
6319+
f"[red]Error:[/red] Failed to remove step directory {step_dir}: {exc}"
6320+
)
6321+
raise typer.Exit(1)
63106322
console.print(f"[green]✓[/green] Step type '{step_id}' uninstalled")
63116323

63126324

src/specify_cli/workflows/__init__.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,10 +89,24 @@ def load_custom_steps(project_root: Path) -> list[str]:
8989
if not steps_dir.is_dir():
9090
return []
9191

92+
# Defense-in-depth: refuse to execute step code from a symlinked
93+
# parent directory under .specify/workflows/steps, which could redirect
94+
# the import outside the project root and bypass the install-time
95+
# symlink guard.
96+
_current = Path(project_root)
97+
for _part in (".specify", "workflows", "steps"):
98+
_current = _current / _part
99+
if _current.is_symlink():
100+
return []
101+
92102
loaded: list[str] = []
93103
for step_dir in steps_dir.iterdir():
94104
if not step_dir.is_dir():
95105
continue
106+
# Skip symlinked step directories so the imported package always
107+
# resolves to a real directory inside the project root.
108+
if step_dir.is_symlink():
109+
continue
96110
step_yml = step_dir / "step.yml"
97111
init_py = step_dir / "__init__.py"
98112
if not step_yml.exists() or not init_py.exists():

src/specify_cli/workflows/catalog.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -627,6 +627,10 @@ def _load(self) -> dict[str, Any]:
627627
# read outside the project root.
628628
if self._has_symlinked_parent():
629629
return default_registry
630+
# Defense-in-depth: also refuse to read a symlinked registry file,
631+
# which could redirect the read outside the project root.
632+
if self.registry_path.is_symlink():
633+
return default_registry
630634
if self.registry_path.exists():
631635
try:
632636
with open(self.registry_path, encoding="utf-8") as f:

0 commit comments

Comments
 (0)