Skip to content

Commit f365aab

Browse files
authored
Harden extra_files: warn on non-dict, resolve symlinks in path traversal check
1 parent 9bcd0b2 commit f365aab

1 file changed

Lines changed: 31 additions & 23 deletions

File tree

src/specify_cli/__init__.py

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6106,29 +6106,37 @@ def _safe_fetch(url: str) -> bytes:
61066106
# relative-path → URL. step.yml and __init__.py are ignored here (already
61076107
# written). Paths are validated to stay within the step package directory to
61086108
# prevent path-traversal attacks.
6109-
extra_files = info.get("extra_files") or {}
6110-
if isinstance(extra_files, dict):
6111-
for rel_path, file_url in extra_files.items():
6112-
if rel_path in ("step.yml", "__init__.py"):
6113-
continue # already written above
6114-
dest = (tmp_path / rel_path).resolve()
6115-
try:
6116-
dest.relative_to(tmp_path)
6117-
except ValueError:
6118-
console.print(
6119-
f"[red]Error:[/red] extra_files path '{rel_path}' is outside "
6120-
"the step package directory"
6121-
)
6122-
raise typer.Exit(1)
6123-
try:
6124-
file_content = _safe_fetch(file_url)
6125-
except Exception as exc:
6126-
console.print(
6127-
f"[red]Error:[/red] Failed to download extra file '{rel_path}': {exc}"
6128-
)
6129-
raise typer.Exit(1)
6130-
dest.parent.mkdir(parents=True, exist_ok=True)
6131-
dest.write_bytes(file_content)
6109+
extra_files = info.get("extra_files")
6110+
if extra_files is not None and not isinstance(extra_files, dict):
6111+
console.print(
6112+
"[yellow]Warning:[/yellow] Catalog entry 'extra_files' is not a mapping; "
6113+
"additional package files will not be downloaded."
6114+
)
6115+
extra_files = {}
6116+
for rel_path, file_url in (extra_files or {}).items():
6117+
if rel_path in ("step.yml", "__init__.py"):
6118+
continue # already written above
6119+
# Resolve both destination and base to handle any symlinks in tmp_path itself,
6120+
# ensuring the traversal check is robust even on non-canonical paths.
6121+
resolved_base = tmp_path.resolve()
6122+
dest = (tmp_path / rel_path).resolve()
6123+
try:
6124+
dest.relative_to(resolved_base)
6125+
except ValueError:
6126+
console.print(
6127+
f"[red]Error:[/red] extra_files path '{rel_path}' is outside "
6128+
"the step package directory"
6129+
)
6130+
raise typer.Exit(1)
6131+
try:
6132+
file_content = _safe_fetch(file_url)
6133+
except Exception as exc:
6134+
console.print(
6135+
f"[red]Error:[/red] Failed to download extra file '{rel_path}': {exc}"
6136+
)
6137+
raise typer.Exit(1)
6138+
dest.parent.mkdir(parents=True, exist_ok=True)
6139+
dest.write_bytes(file_content)
61326140

61336141
# Atomically rename the staging directory to the final location.
61346142
# Both paths are under steps_base_dir (same filesystem), so os.rename()

0 commit comments

Comments
 (0)